Add check_certs workflow and Fastlane lane for Distribution certificate management #228
Conversation
bjornoleh
commented
Jan 9, 2025
•
bjornoleh
commented
- Introduces a new GitHub Actions workflow check_certs.yml for certificate validation and renewal.
- Adds a Fastlane lane check_and_renew_certificates to handle certificate checks, expiration warnings, and flag creation for automated renewal.
- Updates create_certs.yml to respond to both workflow_dispatch and workflow_call triggers for compatibility with the new workflow.
- Certificates are created or renewed if missing or expired
- Annotations added after check_certs, nuke_certs and create_certs
- Nuke certs only if repository variable ENABLE_NUKE_CERTS == 'true'
- Set failure and output annotation if nuke_certs were skipped due to ENABLE_NUKE_CERTS != true
…te management. - Introduces a new GitHub Actions workflow check_certs.yml for certificate validation and renewal. - Adds a Fastlane lane check_and_renew_certificates to handle certificate checks, expiration warnings, and flag creation for automated renewal. - Updates create_certs.yml to respond to both workflow_dispatch and workflow_call triggers for compatibility with the new workflow. - Certificates are renewed if less than 7 days to expiry - Annotations added after nuke and create certs - Nuke certs only if ENABLE_NUKE_CERTS == 'true' - Output annotation if nuke_certs were skipped due to ENABLE_NUKE_CERTS != true
Status
Test of PR 228
Test with Valid CertsRun
Excerpt from check_certs log: Test without Valid Certs, no ENABLE_NUKE_CERTS variable
Run
Comment - I think if no valid certs are found AND the environment does not have the secret - this should show up as a failure not a success. I think people will miss the subtle information message: check_certs
Test without Valid Certs, add ENABLE_NUKE_CERTS variable set to true
Run
Excerpt from check_certs annotations: Test Build after check_certs completesStatus: check_certs did the nuke_certs step and created new certs for LoopWorkspace repository Expect that
|
- Only emit warning for certs close to expiration, do not nuke valid certs. - Introduce optional repository variable FORCE_NUKE_CERTS - Nuke Certs if needed, and if the repository variable ENABLE_NUKE_CERTS is set to 'true', or if FORCE_NUKE_CERTS is set to 'true', which will always force certs to be nuked - Emit annotations for FORCE_NUKE_CERTS
Checks if Distribution certificate is present and valid, optionally nukes and reates new certs if the repository variable ENABLE_NUKE_CERTS == 'true'
Remove warnings about other apps from Fastfile, as these are displayed as annotations from check_certs.yml
|
I think I have addressed you feedback now, thanks! Please see commit log for details. |
|
This should be ready for review now. |
StatusSuccess 1. Test Build without valid Certs & with ENABLE_NUKE_CERTS = falseSteps:
Result:
❌ No valid distribution certificate found. Automated renewal of certificates was skipped because the repository variable ENABLE_NUKE_CERTS is not set to 'true'. 2. Test Build without valid Certs & with ENABLE_NUKE_CERTS = trueSteps:
Result:
3. Test check_certs with valid certs & ENABLE_NUKE_CERTS = trueSteps:
Result:
✅ Distribution certificate is present and valid. No action required. 4. Test check_certs with valid certs & ENABLE_NUKE_CERTS = true & FORCE_NUKE_CERTS = trueSteps:
Results:
Comments:The use of FORCE_NUKE_CERTS = true is for the special case of using more than one GitHub account to build for the same Apple Developer ID. Most people would never set this. I restored the value to false immediately after testing. |
There was a problem hiding this comment.
This is a very nice change and I approve what is here. I also posted in zulipchat so we can get a few more eyes on this.
Another issue that makes it hard for people updating their app is that when there are Apple Developer agreements not completed, this annotation (from validate_secrets) tells people Unable to create a valid authorization token for the App Store Connect API. Verify that the FASTLANE_ISSUER_ID, FASTLANE_KEY_ID, and FASTLANE_KEY secrets are set correctly and try again. In fact, there is nothing wrong with their secrets. This causes a lot of unnecessary confusion and perhaps could be modified with this PR as well.
- Include the possibility of missing signing of agreements in the check for "No code signing identity found" or "Could not install WWDR certificate". - Break up some long annotation strings into several messages - Add ❗️-emoji to emphasise the suggested actions to take
Please see if the last commit resolved this. There are no code changes except for some rearrangement of annotation strings. I made some effort into checking if we can more accurately detect missing signing agreements, but I did not come up with anything. |
| echo "::error::Unable to create a valid authorization token for the App Store Connect API. Verify that the FASTLANE_ISSUER_ID, FASTLANE_KEY_ID, and FASTLANE_KEY secrets are set correctly and try again." | ||
| echo "::error::Unable to create a valid authorization token for the App Store Connect API." | ||
| echo "::error::❗️ Verify that the latest developer program license agreement has been accepted at https://developer.apple.com/account (review and accept any updated agreement), then wait a few minutes for changes to take effect and try again." | ||
| echo "::error::❗️ Also verify that the FASTLANE_ISSUER_ID, FASTLANE_KEY_ID, and FASTLANE_KEY secrets are set correctly and try again." |
There was a problem hiding this comment.
Perhaps "If you created a new FASTLANE KEY or have not previously succeeded with validate secrets, then check that FASTLANE_ISSUER_ID, FASTLANE_KEY_ID, and FASTLANE_KEY secrets were entered correctly.
There was a problem hiding this comment.
I revoke my approval until we work out a few details.
Thanks much @bjornoleh.
.github/workflows/build_loop.yml
Outdated
| @@ -21,6 +21,14 @@ jobs: | |||
| name: Validate | |||
| uses: ./.github/workflows/validate_secrets.yml | |||
| secrets: inherit | |||
|
|
|||
There was a problem hiding this comment.
I believe this change to build is not quite complete. I tested this after revoking certs and then building 2 different apps. I copied over the check_certs.yml file and modified the Fastfile, build_xxx.yml and create_certs.yml to match these changes in LoopFollow. Then added to LF_Second and LF_Third so I have 3 fast building apps to test.
The manual revoke Distribution cert, followed by build app (for the first app works - for any of the apps).
For each successive app, I must manually create certs and then it works.
Perhaps create certs could be modified to encompass some parts of check certs so that build works every time, with no need for a separate create certs step.
There was a problem hiding this comment.
Thanks, yes I know, we need a check for valid distribution profiles during build if we want to avoid the manual create certs here. Or just run create certs on each build. But that doesn’t sound like the right solution
There was a problem hiding this comment.
This should be resolved by fd9acda.
The certs lane is now changed to not force renewal of profiles, and is launched before every build to check if certificates and profiles are valid, and creates new one as needed. The nuke_certs lane is used if necessary.
When we get these changes into all of the other DIY repositories, everything around certs and profiles will be very easy for the users and supporting volunteers.
The current changes does not break much for the other apps, but until automation is included elsewhere, users will have to run the Create certificates workflow for each app after cert renewals. This is explained in annotations in the run summary.
I have also replaced the old Create certificates with the new one, currently named 3. Check certificates, but with the same old create_certs.yml file name. This simplifies things when merging into dev then master (or the other way around?), as we won’t have to explain how new workflow files aren’t visible outside of the default branch.
Summary:To be explicit about what doesn't work with current build_loop.yml: I have a Test:Revoke Distribution type certificate. The problem with the first LF_2 build, without the separate create certs step, is there are valid certs, just not for the app I'm trying to build. |
Details: Workflows: Removed the validate job dependency where unnecessary. Adjusted needs dependencies in check_alive_and_permissions, check_latest_from_upstream, and build jobs to optimize execution order. Consolidated redundant steps in check_certs.yml, reducing complexity. Enhanced clarity by explicitly listing required secrets and improving step naming. Added annotations for better debugging and user feedback during certificate operations. Fastlane Configuration: Changed match to disable forced certificate updates (force: false) and enabled verbose output. Improved certificate expiration handling and logging for better feedback. Fixed a typo in comments regarding certificate renewal flags.
Rename new workflow to create_certificates.yml Using the old filename simplifies transitions when syncing branches. New workflow names are not visible in GitHub UI unless they are in the default branch.
| @@ -190,7 +190,7 @@ jobs: | |||
| failed=true | |||
| echo "::error::Unable to create a valid authorization token for the App Store Connect API." | |||
| echo "::error::❗️ Verify that the latest developer program license agreement has been accepted at https://developer.apple.com/account (review and accept any updated agreement), then wait a few minutes for changes to take effect and try again." | |||
| echo "::error::❗️ Also verify that the FASTLANE_ISSUER_ID, FASTLANE_KEY_ID, and FASTLANE_KEY secrets are set correctly and try again." | |||
| echo "::error::❗️ If you created a new FASTLANE KEY or have not previously succeeded with validate secrets, then check that FASTLANE_ISSUER_ID, FASTLANE_KEY_ID, and FASTLANE_KEY secrets were entered correctly." | |||
There was a problem hiding this comment.
@marionbarker , was this what you had in mind?
|
Thank you @bjornoleh. I tested this code as provided in this PR through commit
|
There was a problem hiding this comment.
I prefer the original filename and run name be maintained for create_certs. The reason is that simplifies the documentation changes - no need to edit graphics.
I tested this by moving create_certificates.yml to create_certs.yml and editing the run name.; and modified build_loop to use create_certs.yml.
I did a quick test of this: see check_and_renew_certs_v2 in loopdocs-tester GitHub username.
Changed to reduce need for updating docs and instructions. The workflow for users will be the same as before this PR, but missing or invalid certificates or profiles will be updated automatically. Update job and step names in create_certs.yml Rename job check_certs to create_certs (original name) - Keep step name check_certs - Update step name under nuke_certs - add comments for set -e at fastlane nuke_certs and fastlane certs create_certs.yml job create_certs: name: Certificates
|
Thanks, the file name was intended to be the same as the original, this was just a mishap. Should be adressed in the latest commit. I have also named everything like before, hopefully very little changes to docs are needed now. The build workflow has a genuinely new job, now called "Check certificates", which launch the create_certs.yml workflow. This matches the existing job names well: 
What's missing is that testflight.md has not been looked at yet, and some changes are required here (mention of the repo/org variable ENABLE_NUKE_CERTS, and possibly more). |
