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] propagate keychain when installing wwdr certificates #21578

Merged
merged 5 commits into from Jan 15, 2024

Conversation

rabbitinspace
Copy link
Contributor

@rabbitinspace rabbitinspace commented Oct 13, 2023

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I see several green ci/circleci builds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.
  • I've added or updated relevant unit tests.

Motivation and Context

After updating from fastlane 2.202.0 to 2.216.0 fastlane match -s $KEYCHAIN -p $PASSWORD started to fail for me with the following error:

[03:54:46]: 🔓  Successfully decrypted certificates repo
[03:54:46]: Installing certificate...
[03:54:46]: $ security find-certificate -a -c 'Apple Worldwide Developer Relations' -p /Users/ec2-user/Library/Keychains/login.keychain-db
[!] Could not install WWDR certificate
🚨 Error: The command exited with status 1

I import wwdr certs manually after creating a keychain, so something is broken here. The weird thing I found is security find-certificate is invoked for the login keychain despite me specifying a separate one. After digging a bit, I figured the install_missing_wwdr_certificates method doesn't take into account the -s option.

Description

This PR makes the -s option to be propagated to the install_missing_wwdr_certificates method instead of unconditionally using wwdr_keychain. I found this issue which supposed to be fixed, but I guess this edge case isn't handled.

Also, I hope I fixed the tests the right way.

Testing Steps

KEYCHAIN_NAME=certs
KEYCHAIN_PATH="${HOME}/Library/Keychains/${KEYCHAIN_NAME}.keychain-db"
KEYCHAIN_PASS=12345HACKER

# create a new keychain first

if [[ -f "${KEYCHAIN_PATH}" ]]; then
  fastlane run delete_keychain keychain_path:"${KEYCHAIN_PATH}"
fi
fastlane run create_keychain \
  password:"${KEYCHAIN_PASS}" \
  path:"${KEYCHAIN_PATH}" \
  unlock:true \
  timeout:1800

# import all wwdr certs

certs=( AppleWWDRCA.cer AppleWWDRCAG2.cer AppleWWDRCAG3.cer AppleWWDRCAG4.cer AppleWWDRCAG5.cer AppleWWDRCAG6.cer )
for cert in "${certs[@]}"; do
  fastlane run import_certificate \
    certificate_path:"${cert}" \
    keychain_password:"${KEYCHAIN_PASS}" \
    keychain_path:"${KEYCHAIN_PATH}"
done

# run fastlane match to import certs and profiles

fastlane match development -s "${KEYCHAIN_NAME}" -p "${KEYCHAIN_PASS}"

@google-cla
Copy link

google-cla bot commented Oct 13, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@rabbitinspace rabbitinspace changed the title fix(match): propagate keychain when installing wwdr certificates [match] propagate keychain when installing wwdr certificates Oct 13, 2023
@rabbitinspace
Copy link
Contributor Author

conflicts resolved, the CI failure is possibly due to #21663

Copy link
Collaborator

@lacostej lacostej left a comment

Choose a reason for hiding this comment

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

LGTM. Anyone else?

Copy link
Member

@rogerluan rogerluan left a comment

Choose a reason for hiding this comment

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

💪 LGTM! Simple and safe. Thanks for your contribution @rabbitinspace ! 🚀

@lacostej lacostej merged commit 8da20c9 into fastlane:master Jan 15, 2024
3 checks passed
@DuMaM
Copy link
Contributor

DuMaM commented Jan 15, 2024

Yay no more changing default keychain :D thanks man

@rabbitinspace rabbitinspace deleted the match-propagate-keychain branch January 15, 2024 23:38
@rabbitinspace rabbitinspace restored the match-propagate-keychain branch January 15, 2024 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants