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

[match][hotfix] remove the renew_expired_certs option introduced in #21691 and revert the default behavior while we address issues with it #21812

Merged
merged 3 commits into from
Feb 10, 2024
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
5 changes: 0 additions & 5 deletions match/lib/match/options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -292,11 +292,6 @@ def self.available_options
description: "Renew the provisioning profiles if the certificate count on the developer portal has changed. Works only for the 'development' provisioning profile type. Requires 'include_all_certificates' option to be 'true'",
type: Boolean,
default_value: false),
FastlaneCore::ConfigItem.new(key: :renew_expired_certs,
env_name: "MATCH_RENEW_EXPIRED_CERTS",
description: "Automatically renew expired certificates. Note: to renew `developer_id` and `developer_id_installer` certificates you must log in with the Account Holder account by using username and password; App Store Connect API key doesn't work in this case",
type: Boolean,
default_value: true),
FastlaneCore::ConfigItem.new(key: :skip_confirmation,
env_name: "MATCH_SKIP_CONFIRMATION",
description: "Disables confirmation prompts during nuke, answering them with yes",
Expand Down
6 changes: 3 additions & 3 deletions match/lib/match/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,12 @@ def run(params)
end

# Certificate
cert_id = fetch_certificate(params: params, renew_expired_certs: params[:renew_expired_certs])
cert_id = fetch_certificate(params: params, renew_expired_certs: false)

# Mac Installer Distribution Certificate
additional_cert_types = params[:additional_cert_types] || []
cert_ids = additional_cert_types.map do |additional_cert_type|
fetch_certificate(params: params, renew_expired_certs: params[:renew_expired_certs], specific_cert_type: additional_cert_type)
fetch_certificate(params: params, renew_expired_certs: false, specific_cert_type: additional_cert_type)
end

profile_type = Sigh.profile_type_for_distribution_type(
Expand Down Expand Up @@ -172,7 +172,7 @@ def fetch_certificate(params: nil, renew_expired_certs: false, specific_cert_typ
is_cert_renewable = is_authenticated_with_login || is_cert_renewable_via_api

# Validate existing certificate first.
if renew_expired_certs && is_cert_renewable && storage_has_certs
if renew_expired_certs && is_cert_renewable && storage_has_certs && !params[:readonly]
rogerluan marked this conversation as resolved.
Show resolved Hide resolved
cert_path = select_cert_or_key(paths: certs)

unless Utils.is_cert_valid?(cert_path)
Expand Down
93 changes: 1 addition & 92 deletions match/spec/runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@
type: "appstore",
git_url: git_url,
username: "flapple@something.com",
renew_expired_certs: false,
readonly: true
}

Expand Down Expand Up @@ -246,7 +245,7 @@

# Utils
# Ensure match validates stored certificate.
expect(Match::Utils).to receive(:is_cert_valid?).with(stored_valid_cert_path).and_return(true).twice
expect(Match::Utils).to receive(:is_cert_valid?).with(stored_valid_cert_path).and_return(true)

# Certificates
# Ensure a new certificate is not generated.
Expand All @@ -266,96 +265,6 @@
# Rely on expectations defined above.
end

it "renews an outdated certificate", requires_security: true do
# GIVEN

# Downloaded and decrypted storage location.
repo_dir = "./match/spec/fixtures/invalid"
# Invalid cert and key
stored_invalid_cert_path = "#{repo_dir}/certs/distribution/F7P4EE896K.cer"
stored_invalid_key_path = "#{repo_dir}/certs/distribution/F7P4EE896K.p12"

# Valid cert and key
new_stored_valid_cert_path = "./match/spec/fixtures/valid/certs/distribution/E7P4EE896K.cer"
new_stored_valid_key_path = "./match/spec/fixtures/valid/certs/distribution/E7P4EE896K.p12"

# match options
match_test_options = {
renew_expired_certs: true, # Current test suite.
skip_provisioning_profiles: true # We test certificate renewal, not profile.
}
match_config = create_match_config_with_git_storage(extra_values: match_test_options)

fake_cache = create_fake_cache

# EXPECTATIONS

# Storage
fake_storage = create_fake_storage(match_config: match_config, repo_dir: repo_dir)
begin # Ensure old certificates are removed from the storage and new are added.
expect(fake_storage).to receive(:save_changes!).with(
files_to_commit: [
new_stored_valid_cert_path,
new_stored_valid_key_path # this is important, as a cert consists out of 2 files
],
files_to_delete: [
stored_invalid_cert_path,
stored_invalid_key_path
]
)
end

# Encryption
fake_encryption = create_fake_encryption(storage: fake_storage)
# Ensure new files are encrypted.
expect(fake_encryption).to receive(:encrypt_files).and_return(nil)

# Certificate generator
# Ensure a new certificate is generated.
expect(Match::Generator).to receive(:generate_certificate).with(match_config, :distribution, fake_storage.working_directory, specific_cert_type: nil).and_return(new_stored_valid_cert_path)

# Spaceship ensure helper
spaceship_ensure = create_fake_spaceship_ensure
begin # Ensure match checks validity of the new certificate.
profile_type = Sigh.profile_type_for_distribution_type(
platform: match_config[:platform],
distribution_type: match_config[:type]
)

certificates_exists_params = {
username: match_config[:username],
certificate_ids: ['E7P4EE896K'],
cached_certificates: fake_cache.certificates,
platform: match_config[:platform],
profile_type: profile_type
}
expect(spaceship_ensure).to receive(:certificates_exists).with(certificates_exists_params).and_return(true)
end

# Utils
# Ensure match validates stored certificate and make it invalid for the current test suite.
expect(Match::Utils).to receive(:is_cert_valid?).with(stored_invalid_cert_path).and_return(false)

# File system
begin # Ensure old certificates are removed from the file system.
expect(File).to receive(:delete).with(stored_invalid_cert_path).and_return(nil)
expect(File).to receive(:delete).with(stored_invalid_key_path).and_return(nil)
end

# Extra
begin # Ensure profiles are not created, not installed, and not validated.
expect(Match::Generator).not_to receive(:generate_provisioning_profile)
expect(FastlaneCore::ProvisioningProfile).not_to receive(:install)
expect(spaceship_ensure).not_to receive(:profile_exists)
end

# WHEN
Match::Runner.new.run(match_config)

# THEN
# Rely on expectations defined above.
end

it "skips provisioning profiles when skip_provisioning_profiles set to true", requires_security: true do
git_url = "https://github.com/fastlane/fastlane/tree/master/certificates"
values = {
Expand Down