Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[deliver] Implements verify with altool for Xcode 14 validation #20738

Merged
merged 14 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions deliver/lib/deliver/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,15 @@ def verify_binary
package_path: "/tmp",
platform: platform
)
result = transporter.verify(package_path: package_path)
result = transporter.verify(package_path: package_path, asset_path: ipa_path, platform: platform)
when "osx"
package_path = FastlaneCore::PkgUploadPackageBuilder.new.generate(
app_id: Deliver.cache[:app].id,
pkg_path: pkg_path,
package_path: "/tmp",
platform: platform
)
result = transporter.verify(package_path: package_path)
result = transporter.verify(package_path: package_path, asset_path: pkg_path, platform: platform)
else
UI.user_error!("No suitable file found for verify for platform: #{options[:platform]}")
end
Expand All @@ -206,7 +206,7 @@ def upload_binary
pkg_path = options[:pkg]

platform = options[:platform]
transporter = transporter_for_selected_team(upload: true)
transporter = transporter_for_selected_team

case platform
when "ios", "appletvos"
Expand Down Expand Up @@ -266,7 +266,7 @@ def submit_for_review
# If itc_provider was explicitly specified, use it.
# If there are multiple teams, infer the provider from the selected team name.
# If there are fewer than two teams, don't infer the provider.
def transporter_for_selected_team(upload: false)
def transporter_for_selected_team
# Use JWT auth
api_token = Spaceship::ConnectAPI.token
api_key = if options[:api_key].nil? && !api_token.nil?
Expand All @@ -281,20 +281,20 @@ def transporter_for_selected_team(upload: false)

unless api_token.nil?
api_token.refresh! if api_token.expired?
return FastlaneCore::ItunesTransporter.new(nil, nil, false, nil, api_token.text, upload: upload, api_key: api_key)
return FastlaneCore::ItunesTransporter.new(nil, nil, false, nil, api_token.text, altool_compatible_command: true, api_key: api_key)
end

tunes_client = Spaceship::ConnectAPI.client.tunes_client

generic_transporter = FastlaneCore::ItunesTransporter.new(options[:username], nil, false, options[:itc_provider], upload: upload, api_key: api_key)
generic_transporter = FastlaneCore::ItunesTransporter.new(options[:username], nil, false, options[:itc_provider], altool_compatible_command: true, api_key: api_key)
return generic_transporter unless options[:itc_provider].nil? && tunes_client.teams.count > 1

begin
team = tunes_client.teams.find { |t| t['providerId'].to_s == tunes_client.team_id }
name = team['name']
provider_id = generic_transporter.provider_ids[name]
UI.verbose("Inferred provider id #{provider_id} for team #{name}.")
return FastlaneCore::ItunesTransporter.new(options[:username], nil, false, provider_id, upload: upload, api_key: api_key)
return FastlaneCore::ItunesTransporter.new(options[:username], nil, false, provider_id, altool_compatible_command: true, api_key: api_key)
rescue => ex
UI.verbose("Couldn't infer a provider short name for team with id #{tunes_client.team_id} automatically: #{ex}. Proceeding without provider short name.")
return generic_transporter
Expand Down
6 changes: 3 additions & 3 deletions deliver/spec/runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def provider_ids
expect_any_instance_of(FastlaneCore::IpaUploadPackageBuilder).to receive(:generate)
.with(app_id: 'YI8C2AS', ipa_path: 'ACME.ipa', package_path: '/tmp', platform: 'ios')
.and_return('path')
expect(transporter).to receive(:verify).with(package_path: 'path').and_return(true)
expect(transporter).to receive(:verify).with(asset_path: "ACME.ipa", package_path: 'path', platform: "ios").and_return(true)
runner.verify_binary
end
end
Expand All @@ -124,7 +124,7 @@ def provider_ids
expect_any_instance_of(FastlaneCore::IpaUploadPackageBuilder).to receive(:generate)
.with(app_id: 'YI8C2AS', ipa_path: 'ACME.ipa', package_path: '/tmp', platform: 'appletvos')
.and_return('path')
expect(transporter).to receive(:verify).with(package_path: 'path').and_return(true)
expect(transporter).to receive(:verify).with(asset_path: "ACME.ipa", package_path: 'path', platform: "appletvos").and_return(true)
runner.verify_binary
end
end
Expand All @@ -140,7 +140,7 @@ def provider_ids
expect_any_instance_of(FastlaneCore::PkgUploadPackageBuilder).to receive(:generate)
.with(app_id: 'YI8C2AS', pkg_path: 'ACME.pkg', package_path: '/tmp', platform: 'osx')
.and_return('path')
expect(transporter).to receive(:verify).with(package_path: 'path').and_return(true)
expect(transporter).to receive(:verify).with(asset_path: "ACME.pkg", package_path: 'path', platform: "osx").and_return(true)
runner.verify_binary
end
end
Expand Down
62 changes: 48 additions & 14 deletions fastlane_core/lib/fastlane_core/itunes_transporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def build_upload_command(username, password, source = "/tmp", provider_short_nam
not_implemented(__method__)
end

def build_verify_command(username, password, source = "/tmp", provider_short_name = "", jwt = nil)
def build_verify_command(username, password, source = "/tmp", provider_short_name = "", **kwargs)
not_implemented(__method__)
end

Expand Down Expand Up @@ -307,8 +307,22 @@ def build_download_command(username, password, apple_id, destination = "/tmp", p
raise "This feature has not been implemented yet with altool for Xcode 14"
end

def build_verify_command(username, password, source = "/tmp", provider_short_name = "", jwt = nil)
raise "This feature has not been implemented yet with altool for Xcode 14"
def build_verify_command(username, password, source = "/tmp", provider_short_name = "", **kwargs)
api_key = kwargs[:api_key]
platform = kwargs[:platform]
use_api_key = !api_key.nil?
[
("API_PRIVATE_KEYS_DIR=#{api_key[:key_dir]}" if use_api_key),
"xcrun altool",
"--validate-app",
("-u #{username.shellescape}" unless use_api_key),
("-p #{password.shellescape}" unless use_api_key),
("--apiKey #{api_key[:key_id]}" if use_api_key),
("--apiIssuer #{api_key[:issuer_id]}" if use_api_key),
("--asc-provider #{provider_short_name}" unless use_api_key || provider_short_name.to_s.empty?),
platform_option(platform),
file_upload_option(source)
].compact.join(' ')
end

def additional_upload_parameters
Expand Down Expand Up @@ -409,7 +423,8 @@ def build_provider_ids_command(username, password, jwt = nil, api_key = nil)
].compact.join(' ')
end

def build_verify_command(username, password, source = "/tmp", provider_short_name = "", jwt = nil)
def build_verify_command(username, password, source = "/tmp", provider_short_name = "", **kwargs)
jwt = kwargs[:jwt]
use_jwt = !jwt.to_s.empty?
[
'"' + Helper.transporter_path + '"',
Expand Down Expand Up @@ -504,7 +519,8 @@ def build_upload_command(username, password, source = "/tmp", provider_short_nam
end
end

def build_verify_command(username, password, source = "/tmp", provider_short_name = "", jwt = nil)
def build_verify_command(username, password, source = "/tmp", provider_short_name = "", **kwargs)
jwt = kwargs[:jwt]
use_jwt = !jwt.to_s.empty?
if !Helper.user_defined_itms_path? && Helper.mac? && Helper.xcode_at_least?(11)
[
Expand Down Expand Up @@ -660,7 +676,7 @@ def self.hide_transporter_output?
# see: https://github.com/fastlane/fastlane/issues/1524#issuecomment-196370628
# for more information about how to use the iTMSTransporter to list your provider
# short names
def initialize(user = nil, password = nil, use_shell_script = false, provider_short_name = nil, jwt = nil, upload: false, api_key: nil)
def initialize(user = nil, password = nil, use_shell_script = false, provider_short_name = nil, jwt = nil, altool_compatible_command: false, api_key: nil)
# Xcode 6.x doesn't have the same iTMSTransporter Java setup as later Xcode versions, so
# we can't default to using the newer direct Java invocation strategy for those versions.
use_shell_script ||= Helper.is_mac? && Helper.xcode_version.start_with?('6.')
Expand All @@ -675,7 +691,7 @@ def initialize(user = nil, password = nil, use_shell_script = false, provider_sh
@jwt = jwt
@api_key = api_key

if should_use_altool?(upload, use_shell_script)
if should_use_altool?(altool_compatible_command, use_shell_script)
UI.verbose("Using altool as transporter.")
@transporter_executor = AltoolTransporterExecutor.new
else
Expand Down Expand Up @@ -800,10 +816,20 @@ def upload(app_id = nil, dir = nil, package_path: nil, asset_path: nil, platform
# @return (Bool) True if everything worked fine
# @raise [Deliver::TransporterTransferError] when something went wrong
# when transferring
def verify(app_id = nil, dir = nil, package_path: nil)
raise "Either a combination of app id and directory or a package_path are required" if (app_id.nil? || dir.nil?) && package_path.nil?
def verify(app_id = nil, dir = nil, package_path: nil, asset_path: nil, platform: nil)
raise "app_id and dir are required or package_path or asset_path is required" if (app_id.nil? || dir.nil?) && package_path.nil? && asset_path.nil?

force_itmsp = FastlaneCore::Env.truthy?("ITMSTRANSPORTER_FORCE_ITMS_PACKAGE_UPLOAD")
can_use_asset_path = Helper.is_mac? && asset_path

actual_dir = if package_path
actual_dir = if can_use_asset_path && !force_itmsp
# The asset gets deleted upon completion so copying to a temp directory
# (with randomized filename, for multibyte-mixed filename upload fails)
new_file_name = "#{SecureRandom.uuid}#{File.extname(asset_path)}"
tmp_asset_path = File.join(Dir.tmpdir, new_file_name)
FileUtils.cp(asset_path, tmp_asset_path)
tmp_asset_path
giginet marked this conversation as resolved.
Show resolved Hide resolved
giginet marked this conversation as resolved.
Show resolved Hide resolved
elsif package_path
package_path
else
File.join(dir, "#{app_id}.itmsp")
Expand All @@ -812,8 +838,16 @@ def verify(app_id = nil, dir = nil, package_path: nil)
password_placeholder = @jwt.nil? ? 'YourPassword' : nil
jwt_placeholder = @jwt.nil? ? nil : 'YourJWT'

command = @transporter_executor.build_verify_command(@user, @password, actual_dir, @provider_short_name, @jwt)
UI.verbose(@transporter_executor.build_verify_command(@user, password_placeholder, actual_dir, @provider_short_name, jwt_placeholder))
# Handle AppStore Connect API
use_api_key = !@api_key.nil?

# Masking credentials for verbose outputs
api_key_placeholder = use_api_key ? { key_id: "YourKeyID", issuer_id: "YourIssuerID", key_dir: "YourTmpP8KeyDir" } : nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to be commented for code readability 👍

Suggested change
api_key_placeholder = use_api_key ? { key_id: "YourKeyID", issuer_id: "YourIssuerID", key_dir: "YourTmpP8KeyDir" } : nil
# Masking credentials for verbose outputs
api_key_placeholder = use_api_key ? { key_id: "YourKeyID", issuer_id: "YourIssuerID", key_dir: "YourTmpP8KeyDir" } : nil


api_key = api_key_with_p8_file_path(@api_key) if use_api_key

command = @transporter_executor.build_verify_command(@user, @password, actual_dir, @provider_short_name, jwt: @jwt, platform: platform, api_key: api_key)
UI.verbose(@transporter_executor.build_verify_command(@user, password_placeholder, actual_dir, @provider_short_name, jwt: jwt_placeholder, platform: platform, api_key: api_key_placeholder))

begin
result = @transporter_executor.execute(command, ItunesTransporter.hide_transporter_output?)
Expand Down Expand Up @@ -883,9 +917,9 @@ def api_key_with_p8_file_path(original_api_key)
end

# Returns whether altool should be used or ItunesTransporter should be used
def should_use_altool?(upload, use_shell_script)
def should_use_altool?(altool_compatible_command, use_shell_script)
# Xcode 14 no longer supports iTMSTransporter. Use altool instead
!use_shell_script && upload && !Helper.user_defined_itms_path? && Helper.mac? && Helper.xcode_at_least?(14)
!use_shell_script && altool_compatible_command && !Helper.user_defined_itms_path? && Helper.mac? && Helper.xcode_at_least?(14)
end

# Returns the password to be used with the transporter
Expand Down
10 changes: 5 additions & 5 deletions fastlane_core/spec/itunes_transporter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1157,14 +1157,14 @@ def xcrun_download_command(provider_short_name = nil, jwt: nil)
end
context "upload command generation" do
it 'generates a call to altool' do
transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', upload: true)
transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', altool_compatible_command: true)
expect(transporter.upload('my.app.id', '/tmp', package_path: '/tmp/my.app.id.itmsp', platform: "osx")).to eq(altool_upload_command(provider_short_name: 'abcd123'))
end
end

context "provider IDs command generation" do
it 'generates a call to altool' do
transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', upload: true)
transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', altool_compatible_command: true)
expect(transporter.provider_ids).to eq(altool_provider_id_command)
end
end
Expand All @@ -1177,7 +1177,7 @@ def xcrun_download_command(provider_short_name = nil, jwt: nil)
end
context "upload command generation" do
it 'generates a call to xcrun iTMSTransporter instead altool' do
transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', upload: true)
transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', altool_compatible_command: true)
expect(transporter.upload('my.app.id', '/tmp', platform: "osx")).to eq(java_upload_command(provider_short_name: 'abcd123', classpath: false))
end
end
Expand All @@ -1194,15 +1194,15 @@ def xcrun_download_command(provider_short_name = nil, jwt: nil)
end
context "upload command generation" do
it 'generates a call to altool' do
transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', upload: true, api_key: api_key)
transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', altool_compatible_command: true, api_key: api_key)
expected = Regexp.new("API_PRIVATE_KEYS_DIR=#{Regexp.escape(Dir.tmpdir)}.*\s#{Regexp.escape(altool_upload_command(api_key: api_key, provider_short_name: 'abcd123'))}")
expect(transporter.upload('my.app.id', '/tmp', platform: "osx")).to match(expected)
end
end

context "provider IDs command generation" do
it 'generates a call to altool' do
transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', upload: true, api_key: api_key)
transporter = FastlaneCore::ItunesTransporter.new(email, password, false, 'abcd123', altool_compatible_command: true, api_key: api_key)
expected = Regexp.new("API_PRIVATE_KEYS_DIR=#{Regexp.escape(Dir.tmpdir)}.*\s#{Regexp.escape(altool_provider_id_command(api_key: api_key))}")
expect(transporter.provider_ids).to match(expected)
end
Expand Down
6 changes: 3 additions & 3 deletions pilot/lib/pilot/build_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -401,13 +401,13 @@ def transporter_for_selected_team(options)

unless api_token.nil?
api_token.refresh! if api_token.expired?
return FastlaneCore::ItunesTransporter.new(nil, nil, false, nil, api_token.text, upload: true, api_key: api_key)
return FastlaneCore::ItunesTransporter.new(nil, nil, false, nil, api_token.text, altool_compatible_command: true, api_key: api_key)
end

# Otherwise use username and password
tunes_client = Spaceship::ConnectAPI.client ? Spaceship::ConnectAPI.client.tunes_client : nil

generic_transporter = FastlaneCore::ItunesTransporter.new(options[:username], nil, false, options[:itc_provider], upload: true, api_key: api_key)
generic_transporter = FastlaneCore::ItunesTransporter.new(options[:username], nil, false, options[:itc_provider], altool_compatible_command: true, api_key: api_key)
return generic_transporter if options[:itc_provider] || tunes_client.nil?
return generic_transporter unless tunes_client.teams.count > 1

Expand All @@ -416,7 +416,7 @@ def transporter_for_selected_team(options)
name = team['name']
provider_id = generic_transporter.provider_ids[name]
UI.verbose("Inferred provider id #{provider_id} for team #{name}.")
return FastlaneCore::ItunesTransporter.new(options[:username], nil, false, provider_id, upload: true, api_key: api_key)
return FastlaneCore::ItunesTransporter.new(options[:username], nil, false, provider_id, altool_compatible_command: true, api_key: api_key)
rescue => ex
STDERR.puts(ex.to_s)
UI.verbose("Couldn't infer a provider short name for team with id #{tunes_client.team_id} automatically: #{ex}. Proceeding without provider short name.")
Expand Down