diff --git a/cmd/builder/signing.go b/cmd/builder/signing.go index c457fb9..e814d8f 100644 --- a/cmd/builder/signing.go +++ b/cmd/builder/signing.go @@ -395,13 +395,20 @@ func runSigningSetup(cmd *cobra.Command, args []string) error { fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", uploadErr) } - // The profile is written whatever the upload did: the files exist and the - // build that uses them is the same either way. - replaced := writeSigningProfile(cfg, profileName, typ) + // The profile is written only when its secrets reached the repository: + // builder.json must not claim a signing set the repository does not have. + replaced := "" + if uploadErr == nil { + replaced = writeSigningProfile(cfg, profileName, typ) + } if err := config.NewManager().Save(cfg); err != nil { return fmt.Errorf("failed to update config: %w", err) } - fmt.Fprintln(out, profileWritten(profileName, typ, replaced)) + if uploadErr == nil { + fmt.Fprintln(out, profileWritten(profileName, typ, replaced)) + } else { + fmt.Fprintln(out, profileNotWritten(profileName)) + } names := config.SigningSecretNames(set) fmt.Fprintln(out) diff --git a/cmd/builder/signing_auto.go b/cmd/builder/signing_auto.go index 7fe613e..695574a 100644 --- a/cmd/builder/signing_auto.go +++ b/cmd/builder/signing_auto.go @@ -146,7 +146,11 @@ func runSigningAuto(cmd *cobra.Command) error { if uploadErr != nil { fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", uploadErr) } - fmt.Fprintln(out.log, profileWritten(profileName, typ, res.ReplacedDistribution)) + if res.ProfileWritten { + fmt.Fprintln(out.log, profileWritten(profileName, typ, res.ReplacedDistribution)) + } else { + fmt.Fprintln(out.log, profileNotWritten(profileName)) + } // Everything is printed before the exit code, so finish's success-only // hook is not used. @@ -222,6 +226,12 @@ func profileWritten(name string, typ signing.Type, replaced string) string { return line + ")" } +// profileNotWritten replaces that line when the upload failed: the profile +// would claim a signing set the repository does not have. +func profileNotWritten(name string) string { + return fmt.Sprintf(" Not updated: builder.json (profile %q is written once its secrets are uploaded; rerun setup with access to the repository's secrets, or add them by hand and then the profile)", name) +} + // resolveSigningBundleID takes the flag, then builder.json, then the newest // IPA in ./dist, then asks (only in a terminal). func resolveSigningBundleID(cmd *cobra.Command, cfg *config.Config, out output) (string, error) { diff --git a/cmd/builder/signing_sets_test.go b/cmd/builder/signing_sets_test.go index bef3f89..01845c1 100644 --- a/cmd/builder/signing_sets_test.go +++ b/cmd/builder/signing_sets_test.go @@ -706,7 +706,8 @@ func signingSetupCommand(t *testing.T, store secretStore, args ...string) (cmd * // TestSigningSetupManualReportsAFailedUpload: a repository Builder cannot // write to is a message, not a dead end. The values are printed, the build -// profile is written, and only the exit code says it failed. +// profile is not written (builder.json must not claim a set the repository +// does not have), and the exit code says it failed. func TestSigningSetupManualReportsAFailedUpload(t *testing.T) { t.Chdir(t.TempDir()) cfg := &config.Config{Project: "App", Platform: "ios", GitHub: config.GitHubConfig{Owner: "o", Repo: "r"}} @@ -742,8 +743,11 @@ func TestSigningSetupManualReportsAFailedUpload(t *testing.T) { if err != nil { t.Fatal(err) } - if saved.Profiles["store"].Distribution != "store" { - t.Errorf("build profile not written: %+v", saved.Profiles) + if _, ok := saved.Profiles["store"]; ok { + t.Errorf("build profile written although its secrets were not uploaded: %+v", saved.Profiles) + } + if !strings.Contains(stdout.String(), "Not updated: builder.json") { + t.Errorf("the skipped profile is not reported:\n%s", stdout.String()) } } @@ -785,8 +789,11 @@ func TestSigningSetupAutoReportsAFailedUpload(t *testing.T) { if err != nil { t.Fatal(err) } - if saved.Profiles["store"].Distribution != "store" { - t.Errorf("build profile not written: %+v", saved.Profiles) + if _, ok := saved.Profiles["store"]; ok { + t.Errorf("build profile written although its secrets were not uploaded: %+v", saved.Profiles) + } + if !strings.Contains(stdout.String(), "Not updated: builder.json") { + t.Errorf("the skipped profile is not reported:\n%s", stdout.String()) } // --json says the same in github_upload, and still exits non-zero. diff --git a/internal/signing/sets.go b/internal/signing/sets.go index 392d661..33b0843 100644 --- a/internal/signing/sets.go +++ b/internal/signing/sets.go @@ -88,8 +88,11 @@ type SetupResult struct { SecretsUploaded bool `json:"secrets_uploaded"` // GitHubUpload is "ok" or why the upload failed. GitHubUpload string `json:"github_upload"` - // BuildProfile is the builder.json profile written with the distribution. + // BuildProfile is the builder.json profile that builds with the set. BuildProfile string `json:"build_profile"` + // ProfileWritten says BuildProfile was written to builder.json, which + // happens only when the secrets were uploaded. + ProfileWritten bool `json:"profile_written"` // ReplacedDistribution is the distribution the profile had before, when // it was a different one. ReplacedDistribution string `json:"replaced_distribution,omitempty"` @@ -135,9 +138,14 @@ func Setup(ctx context.Context, client *asc.Client, store SecretStore, storeErr res.GitHubUpload = res.UploadError.Error() } - // The profile is written whatever the upload did: the material exists and - // the build that uses it is the same either way. - res.ReplacedDistribution = WriteProfile(cfg, profileName, opts.Type) + // The profile is written only when its secrets reached the repository: a + // profile in builder.json says the set is ready to build with, and one + // whose secrets are missing fails the build late and misleads anyone, an + // agent that cannot list secrets included, who reads builder.json. + if res.SecretsUploaded { + res.ReplacedDistribution = WriteProfile(cfg, profileName, opts.Type) + res.ProfileWritten = true + } RecordDir(cfg, opts.OutDirAsGiven) if cfg.IOS.BundleID == "" { cfg.IOS.BundleID = opts.BundleID