From 1503a12f98e873dc06411b0527ffe26a5eb18033 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Mon, 13 Jun 2022 17:32:26 -0400 Subject: [PATCH] internal/relui: use a per-workflow directory for signing We learned the hard way that using a well-known directory creates confusion in the release process. Instead, create a "signing" directory inside the per-workflow scratch dir and use that to communicate with the internal signing tool. In addition, create a sentinel file "ready" that can be used to signal the internal tool to start working. For golang/go#51797. Change-Id: Id8236b932e22c445cc03ecec3e975d85583c914b Reviewed-on: https://go-review.googlesource.com/c/build/+/411902 Run-TryBot: Heschi Kreinick Reviewed-by: Dmitri Shuralyov TryBot-Result: Gopher Robot Reviewed-by: Dmitri Shuralyov --- cmd/relui/deployment-prod.yaml | 1 - cmd/relui/main.go | 2 - internal/relui/buildrelease_test.go | 41 ++++++++++++--- internal/relui/workflows.go | 82 +++++++++++++++++++---------- internal/relui/workflows_test.go | 2 +- 5 files changed, 88 insertions(+), 40 deletions(-) diff --git a/cmd/relui/deployment-prod.yaml b/cmd/relui/deployment-prod.yaml index fd1b3fdc09..2b4c40b83a 100644 --- a/cmd/relui/deployment-prod.yaml +++ b/cmd/relui/deployment-prod.yaml @@ -37,7 +37,6 @@ spec: - "--builder-master-key=secret:symbolic-datum-552/builder-master-key" - "--github-token=secret:symbolic-datum-552/maintner-github-token" - "--scratch-files-base=gs://golang-release-staging/relui-scratch" - - "--staging-files-base=gs://golang-release-staging" - "--serving-files-base=gs://golang" - "--edge-cache-url=https://dl.google.com/go" - "--website-upload-url=https://go.dev/dl/upload" diff --git a/cmd/relui/main.go b/cmd/relui/main.go index 2245ecd159..4d031c36c1 100644 --- a/cmd/relui/main.go +++ b/cmd/relui/main.go @@ -46,7 +46,6 @@ var ( pgConnect = flag.String("pg-connect", "", "Postgres connection string or URI. If empty, libpq connection defaults are used.") scratchFilesBase = flag.String("scratch-files-base", "", "Storage for scratch files. gs://bucket/path or file:///path/to/scratch.") - stagingFilesBase = flag.String("staging-files-base", "", "Storage for staging files. gs://bucket/path or file:///path/to/staging.") servingFilesBase = flag.String("serving-files-base", "", "Storage for serving files. gs://bucket/path or file:///path/to/serving.") edgeCacheURL = flag.String("edge-cache-url", "", "URL release files appear at when published to the CDN, e.g. https://dl.google.com/go.") websiteUploadURL = flag.String("website-upload-url", "", "URL to POST website file data to, e.g. https://go.dev/dl/upload.") @@ -130,7 +129,6 @@ func main() { CreateBuildlet: coordinator.CreateBuildlet, GCSClient: gcsClient, ScratchURL: *scratchFilesBase, - StagingURL: *stagingFilesBase, ServingURL: *servingFilesBase, DownloadURL: *edgeCacheURL, PublishFile: func(f *relui.WebsiteFile) error { diff --git a/internal/relui/buildrelease_test.go b/internal/relui/buildrelease_test.go index 3d2b93b65e..c86ef6f4f4 100644 --- a/internal/relui/buildrelease_test.go +++ b/internal/relui/buildrelease_test.go @@ -20,6 +20,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "runtime" "strings" "sync" @@ -65,9 +66,23 @@ func testRelease(t *testing.T, wantVersion string, kind task.ReleaseKind) { } // Set up the fake signing process. - stagingDir := t.TempDir() - go fakeSign(ctx, t, filepath.Join(stagingDir, wantVersion)) + scratchDir := t.TempDir() signingPollDuration = 100 * time.Millisecond + argRe := regexp.MustCompile(`--relui_staging="(.*?)"`) + outputListener := func(taskName string, output interface{}) { + if taskName != "Start signing command" { + return + } + matches := argRe.FindStringSubmatch(output.(string)) + if matches == nil { + return + } + u, err := url.Parse(matches[1]) + if err != nil { + t.Fatal(err) + } + go fakeSign(ctx, t, u.Path) + } // Set up the fake CDN publishing process. servingDir := t.TempDir() @@ -100,8 +115,7 @@ func testRelease(t *testing.T, wantVersion string, kind task.ReleaseKind) { buildTasks := &BuildReleaseTasks{ GerritURL: tarballServer.URL, GCSClient: nil, - ScratchURL: "file://" + filepath.ToSlash(t.TempDir()), - StagingURL: "file://" + filepath.ToSlash(stagingDir), + ScratchURL: "file://" + filepath.ToSlash(scratchDir), ServingURL: "file://" + filepath.ToSlash(servingDir), CreateBuildlet: fakeBuildlets.createBuildlet, DownloadURL: dlServer.URL, @@ -122,7 +136,7 @@ func testRelease(t *testing.T, wantVersion string, kind task.ReleaseKind) { if err != nil { t.Fatal(err) } - _, err = w.Run(ctx, &verboseListener{t}) + _, err = w.Run(ctx, &verboseListener{t, outputListener}) if err != nil { t.Fatal(err) } @@ -596,7 +610,10 @@ func (b *fakeBuildlet) WorkDir(ctx context.Context) (string, error) { return b.dir, nil } -type verboseListener struct{ t *testing.T } +type verboseListener struct { + t *testing.T + outputListener func(string, interface{}) +} func (l *verboseListener) TaskStateChanged(_ uuid.UUID, _ string, st *workflow.TaskState) error { switch { @@ -606,6 +623,9 @@ func (l *verboseListener) TaskStateChanged(_ uuid.UUID, _ string, st *workflow.T l.t.Logf("task %-10v: error: %v", st.Name, st.Error) default: l.t.Logf("task %-10v: done: %v", st.Name, st.Result) + if l.outputListener != nil { + l.outputListener(st.Name, st.Result) + } } return nil } @@ -632,13 +652,17 @@ func fakeSign(ctx context.Context, t *testing.T, dir string) { } func fakeSignOnce(t *testing.T, dir string, seen map[string]bool) { - contents, err := os.ReadDir(dir) + _, err := os.Stat(filepath.Join(dir, "ready")) if os.IsNotExist(err) { return } if err != nil { t.Fatal(err) } + contents, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } for _, fi := range contents { fn := fi.Name() if fn == "signed" || seen[fn] { @@ -759,6 +783,9 @@ func TestFakeSign(t *testing.T) { t.Fatal(err) } } + if err := ioutil.WriteFile(filepath.Join(dir, "ready"), nil, 0777); err != nil { + t.Fatal(err) + } fakeSignOnce(t, dir, map[string]bool{}) want := map[string]bool{} for _, f := range strings.Split(strings.TrimSpace(outputs), "\n") { diff --git a/internal/relui/workflows.go b/internal/relui/workflows.go index aa57775b61..9aca4fd482 100644 --- a/internal/relui/workflows.go +++ b/internal/relui/workflows.go @@ -307,6 +307,9 @@ func addSingleReleaseWorkflow(build *BuildReleaseTasks, milestone *task.Mileston dlclCommit := wd.Task("Wait for DL CL", version.AwaitCL, dlcl, wd.Constant("")) wd.Output("Download CL submitted", dlclCommit) + startSigner := wd.Task("Start signing command", build.startSigningCommand, nextVersion) + wd.Output("Signing command", startSigner) + // Build, test, and sign release. signedAndTestedArtifacts, err := build.addBuildTasks(wd, "go1.19", nextVersion, releaseBase, skipTests, checked) if err != nil { @@ -382,13 +385,13 @@ func (tasks *BuildReleaseTasks) addBuildTasks(wd *workflow.Definition, majorVers // BuildReleaseTasks serves as an adapter to the various build tasks in the task package. type BuildReleaseTasks struct { - GerritURL string - GCSClient *storage.Client - ScratchURL, StagingURL, ServingURL string - DownloadURL string - PublishFile func(*WebsiteFile) error - CreateBuildlet func(string) (buildlet.Client, error) - ApproveActionFunc func(taskName string) func(*workflow.TaskContext, interface{}) error + GerritURL string + GCSClient *storage.Client + ScratchURL, ServingURL string + DownloadURL string + PublishFile func(*WebsiteFile) error + CreateBuildlet func(string) (buildlet.Client, error) + ApproveActionFunc func(taskName string) func(*workflow.TaskContext, interface{}) error } func (b *BuildReleaseTasks) buildSource(ctx *workflow.TaskContext, revision, version string) (artifact, error) { @@ -530,15 +533,22 @@ func (b *BuildReleaseTasks) runBuildStep( }, nil } +// An artifact represents a file as it moves through the release process. Most +// files will appear on go.dev/dl eventually. type artifact struct { // The target platform of this artifact, or nil for source. Target *releasetargets.Target - // The scratch path of this artifact. + // The scratch path of this artifact within the scratch directory. + // /- ScratchPath string - // The path the artifact was staged to for the signing process. + // The path within the scratch directory the artifact was staged to for the + // signing process. + // /signing// StagingPath string - // The path artifact can be found at after the signing process. It may be - // the same as the staging path for artifacts that are externally signed. + // The path within the scratch directory the artifact can be found at + // after the signing process. For files not modified by the signing + // process, the staging path, or for those that are + // /signing//signed/ SignedPath string // The contents of the GPG signature for this artifact (.asc file). GPGSignature string @@ -560,15 +570,17 @@ func (w *sizeWriter) Write(p []byte) (n int, err error) { return len(p), nil } +func (tasks *BuildReleaseTasks) startSigningCommand(ctx *workflow.TaskContext, version string) (string, error) { + args := fmt.Sprintf("--relui_staging=%q", path.Join(tasks.ScratchURL, signingStagingDir(ctx, version))) + ctx.Printf("run signer with " + args) + return args, nil +} + func (tasks *BuildReleaseTasks) copyToStaging(ctx *workflow.TaskContext, version string, artifacts []artifact) ([]artifact, error) { scratchFS, err := gcsfs.FromURL(ctx, tasks.GCSClient, tasks.ScratchURL) if err != nil { return nil, err } - stagingFS, err := gcsfs.FromURL(ctx, tasks.GCSClient, tasks.StagingURL) - if err != nil { - return nil, err - } var stagedArtifacts []artifact for _, a := range artifacts { staged := a @@ -577,14 +589,14 @@ func (tasks *BuildReleaseTasks) copyToStaging(ctx *workflow.TaskContext, version } else { staged.Filename = version + "." + a.Suffix } - staged.StagingPath = path.Join(version, staged.Filename) + staged.StagingPath = path.Join(signingStagingDir(ctx, version), staged.Filename) stagedArtifacts = append(stagedArtifacts, staged) in, err := scratchFS.Open(a.ScratchPath) if err != nil { return nil, err } - out, err := gcsfs.Create(stagingFS, staged.StagingPath) + out, err := gcsfs.Create(scratchFS, staged.StagingPath) if err != nil { return nil, err } @@ -598,9 +610,20 @@ func (tasks *BuildReleaseTasks) copyToStaging(ctx *workflow.TaskContext, version return nil, err } } + out, err := gcsfs.Create(scratchFS, path.Join(signingStagingDir(ctx, version), "ready")) + if err != nil { + return nil, err + } + if err := out.Close(); err != nil { + return nil, err + } return stagedArtifacts, nil } +func signingStagingDir(ctx *workflow.TaskContext, version string) string { + return path.Join(ctx.WorkflowID.String(), "signing", version) +} + var signingPollDuration = 30 * time.Second // awaitSigned waits for all of artifacts to be signed, plus the pkgs for @@ -617,7 +640,7 @@ func (tasks *BuildReleaseTasks) awaitSigned(ctx *workflow.TaskContext, version s }) } - stagingFS, err := gcsfs.FromURL(ctx, tasks.GCSClient, tasks.StagingURL) + scratchFS, err := gcsfs.FromURL(ctx, tasks.GCSClient, tasks.ScratchURL) if err != nil { return nil, err } @@ -629,7 +652,7 @@ func (tasks *BuildReleaseTasks) awaitSigned(ctx *workflow.TaskContext, version s var signedArtifacts []artifact for { for a := range todo { - signed, ok, err := readSignedArtifact(stagingFS, version, a) + signed, ok, err := readSignedArtifact(ctx, scratchFS, version, a) if err != nil { return nil, err } @@ -653,7 +676,7 @@ func (tasks *BuildReleaseTasks) awaitSigned(ctx *workflow.TaskContext, version s } } -func readSignedArtifact(stagingFS fs.FS, version string, a artifact) (_ artifact, ok bool, _ error) { +func readSignedArtifact(ctx *workflow.TaskContext, scratchFS fs.FS, version string, a artifact) (_ artifact, ok bool, _ error) { // Our signing process has somewhat uneven behavior. In general, for things // that contain their own signature, such as MSIs and .pkgs, we don't // produce a GPG signature, just the new file. On macOS, tars can be signed @@ -684,18 +707,19 @@ func readSignedArtifact(stagingFS fs.FS, version string, a artifact) (_ artifact Filename: a.Filename, Suffix: a.Suffix, } + stagingDir := signingStagingDir(ctx, version) if modifiedBySigning { - signed.SignedPath = version + "/signed/" + a.Filename + signed.SignedPath = stagingDir + "/signed/" + a.Filename } else { - signed.SignedPath = version + "/" + a.Filename + signed.SignedPath = stagingDir + "/" + a.Filename } - fi, err := fs.Stat(stagingFS, signed.SignedPath) + fi, err := fs.Stat(scratchFS, signed.SignedPath) if err != nil { return artifact{}, false, nil } if modifiedBySigning { - hash, err := fs.ReadFile(stagingFS, version+"/signed/"+a.Filename+".sha256") + hash, err := fs.ReadFile(scratchFS, stagingDir+"/signed/"+a.Filename+".sha256") if err != nil { return artifact{}, false, nil } @@ -706,7 +730,7 @@ func readSignedArtifact(stagingFS fs.FS, version string, a artifact) (_ artifact signed.Size = a.Size } if hasGPG { - sig, err := fs.ReadFile(stagingFS, version+"/signed/"+a.Filename+".asc") + sig, err := fs.ReadFile(scratchFS, stagingDir+"/signed/"+a.Filename+".asc") if err != nil { return artifact{}, false, nil } @@ -718,7 +742,7 @@ func readSignedArtifact(stagingFS fs.FS, version string, a artifact) (_ artifact var uploadPollDuration = 30 * time.Second func (tasks *BuildReleaseTasks) uploadArtifacts(ctx *workflow.TaskContext, artifacts []artifact) error { - stagingFS, err := gcsfs.FromURL(ctx, tasks.GCSClient, tasks.StagingURL) + scratchFS, err := gcsfs.FromURL(ctx, tasks.GCSClient, tasks.ScratchURL) if err != nil { return err } @@ -729,7 +753,7 @@ func (tasks *BuildReleaseTasks) uploadArtifacts(ctx *workflow.TaskContext, artif todo := map[artifact]bool{} for _, a := range artifacts { - if err := uploadArtifact(stagingFS, servingFS, a); err != nil { + if err := uploadArtifact(scratchFS, servingFS, a); err != nil { return err } todo[a] = true @@ -762,8 +786,8 @@ func (tasks *BuildReleaseTasks) uploadArtifacts(ctx *workflow.TaskContext, artif } } -func uploadArtifact(stagingFS, servingFS fs.FS, a artifact) error { - in, err := stagingFS.Open(a.SignedPath) +func uploadArtifact(scratchFS, servingFS fs.FS, a artifact) error { + in, err := scratchFS.Open(a.SignedPath) if err != nil { return err } diff --git a/internal/relui/workflows_test.go b/internal/relui/workflows_test.go index cc326d89ad..4cecad1962 100644 --- a/internal/relui/workflows_test.go +++ b/internal/relui/workflows_test.go @@ -150,7 +150,7 @@ func runWorkflow(t *testing.T, ctx context.Context, w *workflow.Workflow, listen defer cancel() t.Helper() if listener == nil { - listener = &verboseListener{t} + listener = &verboseListener{t, nil} } return w.Run(ctx, listener) }