Skip to content

Commit

Permalink
[match][hotfix] remove the renew_expired_certs option introduced in #…
Browse files Browse the repository at this point in the history
…21691 and revert the default behavior while we address issues with it (#21812)

* [match] remove the renew_expired_certs option and make behaviour default

* chore: don’t auto renew cert

* chore: remove auto renewal test
  • Loading branch information
nekrich committed Feb 10, 2024
1 parent 4a43eb2 commit 9899046
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 100 deletions.
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]
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

0 comments on commit 9899046

Please sign in to comment.