Skip to content

Commit 5d27046

Browse files
dmitshurgopherbot
authored andcommitted
internal/task: add GenerateAutoSubmitChange method to CloudBuildClient
I originally planned to use the existing CloudBuildClient.RunScript API for this task, similarly to how it's used for monthly x/ repo tagging. One key limitation of the RunScript API is that it requires the caller to pass the exact files that will be copied out to Google Cloud Storage. That works fine if go.mod and go.sum files were the only ones to change, but 'go fix ./...' may update arbitrary other files too. It's not too hard to expand RunScript to be able to copy directories recursively, but that means it'd be copying the entire x/ repo content even if only a few files changed. It's possible to try to detect which files changed, and copy only those, or to create a .patch file. However, we're already relying on the Cloud Build to clone the repo and run commands to generate files, so it can work well for the Cloud Build invocation to also take on the responsibility of mailing the auto-submit CL, avoiding the need to copy anything to temporary storage only so that relui can do the mailing itself. Also reuse git-generate as a building block, pinning a known version for stability. The result is an API that's very easy to use, and has the benefit of including the script that describes the change in the commit message. For golang/go#69095. Change-Id: I78bb69567044e334fea693bf32a02de86696e7c9 Reviewed-on: https://go-review.googlesource.com/c/build/+/651336 Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Cherry Mui <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 19e4b2a commit 5d27046

File tree

4 files changed

+247
-3
lines changed

4 files changed

+247
-3
lines changed

internal/task/cloudbuild.go

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,40 @@
55
package task
66

77
import (
8+
"bytes"
89
"context"
10+
cryptorand "crypto/rand"
911
"fmt"
1012
"io/fs"
1113
"math/rand"
14+
"regexp"
1215
"strings"
16+
"time"
1317

1418
cloudbuild "cloud.google.com/go/cloudbuild/apiv1/v2"
1519
"cloud.google.com/go/cloudbuild/apiv1/v2/cloudbuildpb"
1620
"cloud.google.com/go/storage"
21+
"golang.org/x/build/gerrit"
1722
"golang.org/x/build/internal/gcsfs"
23+
"golang.org/x/build/internal/secret"
24+
wf "golang.org/x/build/internal/workflow"
1825
)
1926

27+
const gitGenerateVersion = "v0.0.0-20240603191855-5c202b9c66be"
28+
2029
type CloudBuildClient interface {
2130
// RunBuildTrigger runs an existing trigger in project with the given
2231
// substitutions.
2332
RunBuildTrigger(ctx context.Context, project, trigger string, substitutions map[string]string) (CloudBuild, error)
33+
34+
// GenerateAutoSubmitChange generates a change with the given metadata and
35+
// contents generated via the [git-generate] script that must be in the commit message,
36+
// starts trybots with auto-submit enabled, and returns its change ID.
37+
// If the requested contents match the state of the repository, no change
38+
// is created and the returned change ID will be empty.
39+
// Reviewers is the username part of a golang.org or google.com email address.
40+
GenerateAutoSubmitChange(ctx *wf.TaskContext, input gerrit.ChangeInput, reviewers []string) (changeID string, _ error)
41+
2442
// RunScript runs the given script under bash -eux -o pipefail in
2543
// ScriptProject. Outputs are collected into the build's ResultURL,
2644
// readable with ResultFS. The script will have the latest version of Go
@@ -35,10 +53,12 @@ type CloudBuildClient interface {
3553
// Prefer RunScript for simpler scenarios.
3654
// Reference: https://cloud.google.com/build/docs/build-config-file-schema
3755
RunCustomSteps(ctx context.Context, steps func(resultURL string) []*cloudbuildpb.BuildStep, opts *CloudBuildOptions) (CloudBuild, error)
56+
3857
// Completed reports whether a build has finished, returning an error if
3958
// it's failed. It's suitable for use with AwaitCondition.
4059
Completed(ctx context.Context, build CloudBuild) (detail string, completed bool, _ error)
4160
// ResultFS returns an FS that contains the results of the given build.
61+
// The build must've been created by RunScript or RunCustomSteps.
4262
ResultFS(ctx context.Context, build CloudBuild) (fs.FS, error)
4363
}
4464

@@ -139,6 +159,172 @@ func (c *RealCloudBuildClient) RunScript(ctx context.Context, script string, ger
139159
return c.RunCustomSteps(ctx, steps, nil)
140160
}
141161

162+
func (c *RealCloudBuildClient) GenerateAutoSubmitChange(ctx *wf.TaskContext, input gerrit.ChangeInput, reviewers []string) (changeID string, _ error) {
163+
if input.Project == "" {
164+
return "", fmt.Errorf("input.Project must be specified")
165+
} else if input.Branch == "" {
166+
return "", fmt.Errorf("input.Branch must be specified")
167+
} else if !strings.Contains(input.Subject, "\n[git-generate]\n") {
168+
return "", fmt.Errorf("a commit message with a [git-generate] script must be provided")
169+
}
170+
171+
// Add a Change-Id trailer to the commit message if it's not already present.
172+
var changeIDTrailers int
173+
if strings.HasPrefix(input.Subject, "Change-Id: ") {
174+
changeIDTrailers++
175+
}
176+
changeIDTrailers += strings.Count(input.Subject, "\nChange-Id: ")
177+
if changeIDTrailers > 1 {
178+
return "", fmt.Errorf("multiple Change-Id lines")
179+
}
180+
if changeIDTrailers == 0 {
181+
// randomBytes returns 20 random bytes suitable for use in a Change-Id line.
182+
randomBytes := func() []byte { var id [20]byte; cryptorand.Read(id[:]); return id[:] }
183+
184+
// endsWithMetadataLine reports whether the given commit message ends with a
185+
// metadata line such as "Bug: #42" or "Signed-off-by: Al <[email protected]>".
186+
metadataLineRE := regexp.MustCompile(`^[a-zA-Z0-9-]+: `)
187+
endsWithMetadataLine := func(msg string) bool {
188+
i := strings.LastIndexByte(msg, '\n')
189+
return i >= 0 && metadataLineRE.MatchString(msg[i+1:])
190+
}
191+
192+
msg := strings.TrimRight(input.Subject, "\n")
193+
sep := "\n\n"
194+
if endsWithMetadataLine(msg) {
195+
sep = "\n"
196+
}
197+
input.Subject += fmt.Sprintf("%sChange-Id: I%x", sep, randomBytes())
198+
}
199+
200+
refspec := fmt.Sprintf("HEAD:refs/for/%s%%l=Auto-Submit,l=Commit-Queue+1", input.Branch)
201+
reviewerEmails, err := coordinatorEmails(reviewers)
202+
if err != nil {
203+
return "", err
204+
}
205+
for _, r := range reviewerEmails {
206+
refspec += ",r=" + r
207+
}
208+
209+
// Create a Cloud Build that will generate and mail the CL.
210+
//
211+
// To remove the possibility of mailing multiple CLs due to
212+
// automated retries, allow only manual retries from this point.
213+
ctx.DisableRetries()
214+
op, err := c.BuildClient.CreateBuild(ctx, &cloudbuildpb.CreateBuildRequest{
215+
ProjectId: c.ScriptProject,
216+
Build: &cloudbuildpb.Build{
217+
Steps: []*cloudbuildpb.BuildStep{
218+
{
219+
Name: "bash", Script: cloudBuildClientDownloadGoScript,
220+
},
221+
{
222+
Name: "gcr.io/cloud-builders/git",
223+
Args: []string{"clone", "--branch=" + input.Branch, "--depth=1", "--",
224+
"https://go.googlesource.com/" + input.Project, "checkout"},
225+
},
226+
{
227+
Name: "gcr.io/cloud-builders/git",
228+
Args: []string{"-c", "user.name=Gopher Robot", "-c", "[email protected]",
229+
"commit", "--allow-empty", "-m", input.Subject},
230+
Dir: "checkout",
231+
},
232+
{
233+
Name: "gcr.io/cloud-builders/git",
234+
Entrypoint: "/workspace/released_go/bin/go",
235+
Args: []string{"run", "rsc.io/rf/git-generate@" + gitGenerateVersion},
236+
Dir: "checkout",
237+
},
238+
{
239+
Name: "gcr.io/cloud-builders/git",
240+
Args: []string{"-c", "user.name=Gopher Robot", "-c", "[email protected]",
241+
"commit", "--amend", "--no-edit"},
242+
Dir: "checkout",
243+
},
244+
{
245+
Name: "gcr.io/cloud-builders/git",
246+
Args: []string{"show", "HEAD"},
247+
Dir: "checkout",
248+
},
249+
{
250+
Name: "bash", Args: []string{"-c", `touch .gitcookies && chmod 0600 .gitcookies && printf ".googlesource.com\tTRUE\t/\tTRUE\t2147483647\to\tgit-gobot.golang.org=$$GOBOT_TOKEN\n" >> .gitcookies`},
251+
SecretEnv: []string{"GOBOT_TOKEN"},
252+
},
253+
{
254+
Name: "gcr.io/cloud-builders/git",
255+
Entrypoint: "bash",
256+
Args: []string{"-c", `git -c http.cookieFile=../.gitcookies push origin ` + refspec + ` 2>&1 | tee "$$BUILDER_OUTPUT/output"`},
257+
Dir: "checkout",
258+
},
259+
{
260+
Name: "bash", Args: []string{"-c", "rm .gitcookies"},
261+
},
262+
},
263+
Options: &cloudbuildpb.BuildOptions{
264+
MachineType: cloudbuildpb.BuildOptions_E2_HIGHCPU_8,
265+
Logging: cloudbuildpb.BuildOptions_CLOUD_LOGGING_ONLY,
266+
},
267+
ServiceAccount: c.ScriptAccount,
268+
AvailableSecrets: &cloudbuildpb.Secrets{
269+
SecretManager: []*cloudbuildpb.SecretManagerSecret{
270+
{
271+
VersionName: "projects/" + c.ScriptProject + "/secrets/" + secret.NameGobotPassword + "/versions/latest",
272+
Env: "GOBOT_TOKEN",
273+
},
274+
},
275+
},
276+
},
277+
})
278+
if err != nil {
279+
return "", fmt.Errorf("creating build: %w", err)
280+
}
281+
if _, err = op.Poll(ctx); err != nil {
282+
return "", fmt.Errorf("polling: %w", err)
283+
}
284+
meta, err := op.Metadata()
285+
if err != nil {
286+
return "", fmt.Errorf("reading metadata: %w", err)
287+
}
288+
build := CloudBuild{Project: c.ScriptProject, ID: meta.Build.Id}
289+
290+
// Await the Cloud Build and extract the ID of the CL that was mailed.
291+
ctx.Printf("Awaiting completion of build %q in %s.", build.ID, build.Project)
292+
return AwaitCondition(ctx, 30*time.Second, func() (changeID string, completed bool, _ error) {
293+
return c.completedGeneratingCL(ctx, build)
294+
})
295+
}
296+
297+
// completedGeneratingCL reports whether a build has finished,
298+
// returning the change ID that the given build generated.
299+
// The build must've been created by GenerateAutoSubmitChange.
300+
// It's suitable for use with AwaitCondition.
301+
func (c *RealCloudBuildClient) completedGeneratingCL(ctx context.Context, build CloudBuild) (changeID string, completed bool, _ error) {
302+
b, err := c.BuildClient.GetBuild(ctx, &cloudbuildpb.GetBuildRequest{
303+
ProjectId: build.Project,
304+
Id: build.ID,
305+
})
306+
if err != nil {
307+
return "", false, err
308+
}
309+
if b.FinishTime == nil {
310+
return "", false, nil
311+
}
312+
if b.Status != cloudbuildpb.Build_SUCCESS {
313+
return "", false, fmt.Errorf("build %q failed, see %v: %v", build.ID, b.LogUrl, b.FailureInfo)
314+
}
315+
316+
// Extract the CL number from the output using a simple regexp.
317+
re := regexp.MustCompile(`https:\/\/go-review\.googlesource\.com\/c\/([a-zA-Z0-9_\-]+)\/\+\/(\d+)`)
318+
gitPushOutput := bytes.Join(b.GetResults().GetBuildStepOutputs(), nil)
319+
if matches := re.FindSubmatch(gitPushOutput); len(matches) == 3 {
320+
changeID = fmt.Sprintf("%s~%s", matches[1], matches[2])
321+
} else {
322+
return "", false, fmt.Errorf("no match for successful mail of generated CL in git push output:\n%s", gitPushOutput)
323+
}
324+
325+
return changeID, true, nil
326+
}
327+
142328
func (c *RealCloudBuildClient) RunCustomSteps(ctx context.Context, steps func(resultURL string) []*cloudbuildpb.BuildStep, options *CloudBuildOptions) (CloudBuild, error) {
143329
resultURL := fmt.Sprintf("%v/script-build-%v", c.ScratchURL, rand.Int63())
144330
build := &cloudbuildpb.Build{

internal/task/fakes.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,17 @@ func (g *FakeGerrit) CreateAutoSubmitChange(_ *wf.TaskContext, input gerrit.Chan
350350
return changeID, nil
351351
}
352352

353+
func (g *FakeGerrit) considerCommitSubmitted(repo *FakeRepo, commit string) (changeID string) {
354+
if g.repos[repo.name] != repo {
355+
repo.t.Fatalf("FakeGerrit.createSubmittedChange: provided repo %q isn't a part of this FakeGerrit instance", repo.name)
356+
}
357+
g.changesMu.Lock()
358+
changeID = fmt.Sprintf("%s~%d", repo.name, len(g.changes)+1)
359+
g.changes[changeID] = commit
360+
g.changesMu.Unlock()
361+
return changeID
362+
}
363+
353364
func (g *FakeGerrit) Submitted(ctx context.Context, changeID, baseCommit string) (string, bool, error) {
354365
g.changesMu.Lock()
355366
commit, ok := g.changes[changeID]
@@ -924,6 +935,45 @@ func (cb *FakeCloudBuild) RunScript(ctx context.Context, script string, gerritPr
924935
return CloudBuild{Project: cb.project, ID: id, ResultURL: "file://" + resultDir}, nil
925936
}
926937

938+
func (cb *FakeCloudBuild) GenerateAutoSubmitChange(ctx *wf.TaskContext, input gerrit.ChangeInput, reviewers []string) (changeID string, _ error) {
939+
if input.Project == "" {
940+
return "", fmt.Errorf("input.Project must be specified")
941+
} else if input.Branch == "" {
942+
return "", fmt.Errorf("input.Branch must be specified")
943+
} else if !strings.Contains(input.Subject, "\n[git-generate]\n") {
944+
return "", fmt.Errorf("a commit message with a [git-generate] script must be provided")
945+
}
946+
947+
r, err := cb.gerrit.repo(input.Project)
948+
if err != nil {
949+
return "", err
950+
}
951+
if input.Branch != "master" {
952+
return "", fmt.Errorf("FakeCloudBuild.GenerateAutoSubmitChange: not implemented for branch %q", input.Branch)
953+
}
954+
955+
// Create an empty commit with the git-generate script in commit message.
956+
r.runGit("commit", "--allow-empty", "-m", input.Subject)
957+
958+
// Run git-generate.
959+
cmd := exec.CommandContext(ctx, "go", "run", "rsc.io/rf/git-generate@"+gitGenerateVersion)
960+
cmd.Dir = r.dir.dir
961+
out, err := cmd.CombinedOutput()
962+
if err != nil {
963+
return "", fmt.Errorf("git-generate failed: %v output:\n%s", err, out)
964+
}
965+
966+
// Update the empty commit with generated content.
967+
r.runGit("commit", "--amend", "--no-edit")
968+
969+
if testing.Verbose() {
970+
cb.t.Logf("FakeCloudBuild.GenerateAutoSubmitChange: generated commit content:\n%s", r.runGit("show", "HEAD"))
971+
}
972+
973+
commit := strings.TrimSpace(string(r.runGit("rev-parse", "HEAD")))
974+
return cb.gerrit.considerCommitSubmitted(r, commit), nil
975+
}
976+
927977
func (cb *FakeCloudBuild) RunCustomSteps(ctx context.Context, steps func(resultURL string) []*cloudbuildpb.BuildStep, _ *CloudBuildOptions) (CloudBuild, error) {
928978
var gerritProject, fullScript string
929979
resultURL := "file://" + cb.t.TempDir()

internal/task/privx.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,10 +195,10 @@ func (x *PrivXPatch) NewDefinition(tagx *TagXReposTasks) *wf.Definition {
195195
return nil, fmt.Errorf("git push failed: %v, stdout: %q stderr: %q", err, stdout.String(), stderr.String())
196196
}
197197

198-
// Extract the CL number from the output using a quick and dirty regex.
199-
re, err := regexp.Compile(fmt.Sprintf(`https:\/\/go-review.googlesource.com\/c\/%s\/\+\/(\d+)`, regexp.QuoteMeta(repoName)))
198+
// Extract the CL number from the output using a simple regexp.
199+
re, err := regexp.Compile(fmt.Sprintf(`https:\/\/go-review\.googlesource\.com\/c\/%s\/\+\/(\d+)`, regexp.QuoteMeta(repoName)))
200200
if err != nil {
201-
return nil, fmt.Errorf("failed to compile regex: %s", err)
201+
return nil, fmt.Errorf("failed to compile regexp: %s", err)
202202
}
203203
matches := re.FindSubmatch(stderr.Bytes())
204204
if len(matches) != 2 {

internal/workflow/workflow.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,14 @@ func (c *TaskContext) Printf(format string, v ...interface{}) {
534534
c.Logger.Printf(format, v...)
535535
}
536536

537+
// DisableRetries disables automatic retries for the remaining
538+
// task execution. It's intended to be used when interacting with
539+
// external systems, where it's preferable to leave the decision
540+
// of whether to retry to the human release coordinator instead of
541+
// automatically retrying a number of times.
542+
//
543+
// Once the current task completes, the following task runs as usual
544+
// with automatic reties enabled once again.
537545
func (c *TaskContext) DisableRetries() {
538546
c.disableRetries = true
539547
}

0 commit comments

Comments
 (0)