Add check_certs workflow and Fastlane lane for Distribution certificate management #228

bjornoleh · 2025-01-09T14:42:35Z

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

marionbarker

Code review only. I have not tested anything yet.

.github/workflows/check_certs.yml

fastlane/Fastfile

marionbarker · 2025-01-09T18:16:16Z

Status

Tests work as expected
I suggest adding the check for certs and create if none are available as part of the build process
- I believe the check_certs workflow should remain a separate event that is needed once per year
- Having the certs creation when needed as part of the build helps minimize effort for people

Test of PR 228

Push this branch to the loopdocs-tester GitHub user name
Modify default branch to check_and_renew_certs

Test with Valid Certs

Run check_certs workflow while certs are valid

Success: run check_certs log
Success: start build action (then cancel after 5 minutes) to make sure build starts without errors

Excerpt from check_certs log:
✅ Certificate VHX89NFM74 is valid, and will not expire during the next 7 days
✅ No certificates are expired or missing. No action required.

Test without Valid Certs, no ENABLE_NUKE_CERTS variable

Revoke the Certificates with Type Distribution on https://developer.apple.com/account/resources/certificates/list
Do NOT add the new environment variable ENABLE_NUKE_CERTS

Run check_certs workflow

Success: check_certs log

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
🔔 Nuke certificates was skipped because the repository variable ENABLE_NUKE_CERTS is not set to 'true'.

I know build will fail (so don't bother to run)
I also expect Create Certificate to fail - so test that one
- Yup - it failed with Certificate 'VHX89NFM74' (stored in your storage) is not available on the Developer Portal

Test without Valid Certs, add ENABLE_NUKE_CERTS variable set to true

Status is no valid Certificate of Type Distribution
Added ENABLE_NUKE_CERTS to environment and set it to true

Run check_certs workflow

Success: check_certs log

Excerpt from check_certs annotations:
⚠️⚠️⚠️ All Distribution certificates and TestFlight profiles have been revoked.
⚠️⚠️⚠️ Certificates have been recreated successfully.
⚠️⚠️⚠️ If you have other apps being distributed by GitHub Actions / Fastlane / TestFlight,
⚠️⚠️⚠️ please run the '3. Create Certificates' workflow for each of these apps to allow these apps to be built.
✅✅✅ But don't worry about your existing TestFlight builds, they will keep working!

Test Build after check_certs completes

Status: check_certs did the nuke_certs step and created new certs for LoopWorkspace repository

Expect that build loop will succeed for this repository, but other repositories will require create cert then build to succeed.

Success: (yes) Build Loop
Failure: (yes) Build LoopFollow (as expected)
Success: (yes) Create Certs for LoopFollow
Success; (yes) Build LoopFollow

- 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

bjornoleh · 2025-01-09T19:23:18Z

I think I have addressed you feedback now, thanks!

Please see commit log for details.

bjornoleh · 2025-01-09T21:08:04Z

This should be ready for review now.

marionbarker · 2025-01-10T01:51:19Z

Status

Success
LGTM. (@ps2 - please review - this is a big improvement to the build process)
Should also add to main and tidepool-merge as well as dev

1. Test Build without valid Certs & with ENABLE_NUKE_CERTS = false

Steps:

Revoke the Certificates with Type Distribution on https://developer.apple.com/account/resources/certificates/list
Expect Action: Build Loop to fail

Result:

Action: Build Loop failed with this message

❌ 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 = true

Steps:

Continue from previous test
Expect Action: Build Loop to succeed after automatic creation of new certs

Result:

Action: Build Loop succeeded with Annotations:

⚠️⚠️⚠️ All Distribution certificates and TestFlight profiles have been revoked.
⚠️⚠️⚠️ Certificates have been recreated successfully.
⚠️⚠️⚠️ If you have other apps being distributed by GitHub Actions / Fastlane / TestFlight, please run the '3. Create Certificates' workflow for each of these apps to allow these apps to be built.
✅✅✅ But don't worry about your existing TestFlight builds, they will keep working!

3. Test check_certs with valid certs & ENABLE_NUKE_CERTS = true

Steps:

Continue from previous test
Expect Action: Check Certificates to succeed with no need for certs message

Result:

Action: Check Certificates succeeds with annotation

✅ Distribution certificate is present and valid. No action required.

4. Test check_certs with valid certs & ENABLE_NUKE_CERTS = true & FORCE_NUKE_CERTS = true

Steps:

Continue from previous test
Expect Action: Check Certificates to succeed after revoking and creating new certs

Results:

Action: Check Certificates succeeded, certificates were revoked and new ones issued for Loop Workspace with annotation:

⚠️⚠️⚠️ All Distribution certificates and TestFlight profiles have been revoked.
⚠️⚠️⚠️ Certificates have been recreated successfully.
⚠️⚠️⚠️ If you have other apps being distributed by GitHub Actions / Fastlane / TestFlight, please run the '3. Create Certificates' workflow for each of these apps to allow these apps to be built.
✅✅✅ But don't worry about your existing TestFlight builds, they will keep working!
✅ Distribution certificate is present and valid. No action required.

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.

marionbarker

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

bjornoleh · 2025-01-11T16:56:36Z

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.

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.

marionbarker · 2025-01-11T19:43:31Z

.github/workflows/validate_secrets.yml

-              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."


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.

marionbarker

I revoke my approval until we work out a few details.
Thanks much @bjornoleh.

marionbarker · 2025-01-12T00:07:04Z

.github/workflows/build_loop.yml

@@ -21,6 +21,14 @@ jobs:
    name: Validate
    uses: ./.github/workflows/validate_secrets.yml
    secrets: inherit
+


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.

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

marionbarker suggested changes Jan 9, 2025

View reviewed changes

.github/workflows/check_certs.yml Outdated Show resolved Hide resolved

.github/workflows/check_certs.yml Outdated Show resolved Hide resolved

fastlane/Fastfile Outdated Show resolved Hide resolved

check_certs.yml: use checkout@v4

0a9d876

marionbarker mentioned this pull request Jan 9, 2025

Add workflow (nuke_certs) for Annual Certificates Reset #191

Closed

bjornoleh added 3 commits January 9, 2025 20:19

Require check_certs before building

4e8ed2e

Checks if Distribution certificate is present and valid, optionally nukes and reates new certs if the repository variable ENABLE_NUKE_CERTS == 'true'

Set error when no valid certs and ENABLE_NUKE_CERTS is not 'true'.

063d602

Remove warnings about other apps from Fastfile, as these are displayed as annotations from check_certs.yml

Annotation for valid certs

5432ec6

bjornoleh requested a review from marionbarker January 9, 2025 21:07

marionbarker approved these changes Jan 10, 2025

View reviewed changes

marionbarker reviewed Jan 11, 2025

View reviewed changes

marionbarker suggested changes Jan 11, 2025

View reviewed changes

bjornoleh mentioned this pull request Jan 11, 2025

feat: Add automated certificate check and renewal workflows nightscout/Trio#451

Draft

marionbarker reviewed Jan 12, 2025

View reviewed changes

Remove unused env

90cc934

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add check_certs workflow and Fastlane lane for Distribution certificate management #228

Add check_certs workflow and Fastlane lane for Distribution certificate management #228

bjornoleh commented Jan 9, 2025 •

edited

Loading

marionbarker left a comment

marionbarker commented Jan 9, 2025

bjornoleh commented Jan 9, 2025

bjornoleh commented Jan 9, 2025

marionbarker commented Jan 10, 2025

marionbarker left a comment

bjornoleh commented Jan 11, 2025

marionbarker Jan 11, 2025

marionbarker left a comment

marionbarker Jan 12, 2025

bjornoleh Jan 12, 2025

Add check_certs workflow and Fastlane lane for Distribution certificate management #228

Are you sure you want to change the base?

Add check_certs workflow and Fastlane lane for Distribution certificate management #228

Conversation

bjornoleh commented Jan 9, 2025 • edited Loading

marionbarker left a comment

Choose a reason for hiding this comment

marionbarker commented Jan 9, 2025

Status

Test of PR 228

Test with Valid Certs

Test without Valid Certs, no ENABLE_NUKE_CERTS variable

Test without Valid Certs, add ENABLE_NUKE_CERTS variable set to true

Test Build after check_certs completes

bjornoleh commented Jan 9, 2025

bjornoleh commented Jan 9, 2025

marionbarker commented Jan 10, 2025

Status

1. Test Build without valid Certs & with ENABLE_NUKE_CERTS = false

2. Test Build without valid Certs & with ENABLE_NUKE_CERTS = true

3. Test check_certs with valid certs & ENABLE_NUKE_CERTS = true

4. Test check_certs with valid certs & ENABLE_NUKE_CERTS = true & FORCE_NUKE_CERTS = true

Comments:

marionbarker left a comment

Choose a reason for hiding this comment

bjornoleh commented Jan 11, 2025

marionbarker Jan 11, 2025

Choose a reason for hiding this comment

marionbarker left a comment

Choose a reason for hiding this comment

marionbarker Jan 12, 2025

Choose a reason for hiding this comment

bjornoleh Jan 12, 2025

Choose a reason for hiding this comment

bjornoleh commented Jan 9, 2025 •

edited

Loading