Fallback profiles fix #279
Conversation
This reverts commit 5ef3991.
SummaryThis PR fixes fallback manual code signing mechanism by updating project settings when automatic code signing fails. The changes refactor code signing logic, improve error handling, and add proper project configuration updates for manual signing fallback scenarios. Walkthrough
|
bitrise-ip-bot
left a comment
bitrise-ip-bot
left a comment
There was a problem hiding this comment.
This is an AI-generated review. Please review it carefully.
Actionable comments posted: 5
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
bitrise-ip-bot
left a comment
bitrise-ip-bot
left a comment
There was a problem hiding this comment.
This is an AI-generated review. Please review it carefully.
Actionable comments posted: 1
bitrise-ip-bot
left a comment
bitrise-ip-bot
left a comment
There was a problem hiding this comment.
This is an AI-generated review. Please review it carefully.
Actionable comments posted: 1
| for _, cert := range certificates { | ||
| m.logger.Printf("- %s", cert.String()) | ||
| // Empty passphrase provided, as already parsed certificate + private key | ||
| if err := m.assetInstaller.InstallCertificate(cert); err != nil { |
There was a problem hiding this comment.
There is installCertificates already called in line 536. Seems duplicate.
There was a problem hiding this comment.
Good catch removed the latter, it was a leftover, I switched to manually installing each cert for having more control over the logs and make it similar to the profile installation logs.
Updated the code here: 314d8f0
codesign/codesign_test.go
Outdated
| wantErr: "failed to determine codesign group for development distribution: no signing assets found", | ||
| }, | ||
| { | ||
| name: "Project entitlements are not filtering the profiles", |
There was a problem hiding this comment.
Why is needed that project entitlements not filter the profiles?
There was a problem hiding this comment.
Ohh it seems we filter based on entitlements, but the filter only works with the known entitlements, whil in the test I used fake entitlements.
Hare we create a map of bundle ids and entitlements:
Lines 479 to 482 in 314d8f0
Here we pass the map to the codesign group creator function:
Line 492 in 314d8f0
Which does the entitlements filtering here:
And the entitlement filter only works with the known entitlements.
I will update the tests.
|
|
||
| assetsByDistributionType := map[autocodesign.DistributionType]autocodesign.AppCodesignAssets{} | ||
| for _, distributionType := range distributionTypes { | ||
| signingAssets, err := provider.DetermineCodesignGroup(certificates, profiles, nil, bundleIDEntitlementsMap, exportoptions.Method(distributionType), m.opts.TeamID, true) |
There was a problem hiding this comment.
Just an observation: we pass xcodeManage = true, this allows use of Xcode managed profiles (good).
We also do not set a default (Bitrise provided) profile, which should be ok (if automatic code signing is already set up, it makes no sense). (also good)
bitrise-ip-bot
left a comment
bitrise-ip-bot
left a comment
There was a problem hiding this comment.
This is an AI-generated review. Please review it carefully.
Actionable comments posted: 2
This PR fixes a bug with the fallback manual code signing mechanism of the automatic code signing logic.
After the automatic code signing logic gathers all the project required code signing files, it updates the project settings to use that signing files for the project archive. When automatic code signing fails, the step can fall back to installing manually provided signing files, but in this case the step was not updating the project settings, which led to failing project archive processes:
Here is an example build, which reproduces the bug: https://app.bitrise.io/build/25a26f21-a721-414e-a070-c4e032602e42
and here is a build with the fix: https://app.bitrise.io/build/8edd6044-0488-4b8a-984c-e21b55efd44a
PR for integrating this fix to the xcode-archive step: bitrise-steplib/steps-xcode-archive#385