internal/relui,task: use FetchReleaseMilestone directly

Change-Id: I236f2bf46419bb9de394350ac1ff880060b7f7fd
Reviewed-on: https://go-review.googlesource.com/c/build/+/820460
Reviewed-by: Nicholas Husin <nsh@golang.org>
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Neal Patel <nealpatel@google.com>

Author: thatnealpatel

internal/relui/buildrelease_test.go

@@ -812,8 +812,8 @@ func TestMinorReleaseSecurityCoalesceRestart(t *testing.T) {
 	}
 
 	// Second run: a restart forks a new checkpoint. The branch name embeds a
-	// minute-resolution timestamp, so a same-minute restart collides on the
-	// branch name (real Gerrit 409). When the minute has rolled over, the
+	// second-resolution timestamp, so a same-second restart collides on the
+	// branch name (real Gerrit 409). When the second has rolled over, the
 	// restart succeeds with a distinct name; either way, the first run's
 	// checkpoint branch must remain exactly as it was.
 	second, err := deps.buildTasks.createSecurityCheckpoint(taskCtx, bi, cls)
@@ -822,7 +822,7 @@ func TestMinorReleaseSecurityCoalesceRestart(t *testing.T) {
 		if !errors.As(err, &httpErr) || httpErr.Res.StatusCode != http.StatusConflict {
 			t.Fatalf("second createSecurityCheckpoint: %v", err)
 		}
-		t.Logf("same-minute restart collided on the timestamped checkpoint name (expected): %v", err)
+		t.Logf("same-second restart collided on the timestamped checkpoint name (expected): %v", err)
 	} else if second == first {
 		t.Errorf("restart reused checkpoint name %q; want a distinct timestamped branch", second)
 	}

internal/relui/workflows.go

@@ -47,7 +47,6 @@ import (
 	"golang.org/x/net/context/ctxhttp"
 	"golang.org/x/vulndb/report"
 	"google.golang.org/protobuf/types/known/structpb"
-	yaml "gopkg.in/yaml.v3"
 )
 
 // DefinitionHolder holds workflow definitions.
@@ -1016,19 +1015,10 @@ func (b *BuildReleaseTasks) fetchSecurityMilestone(ctx *wf.TaskContext, mileston
 		ctx.Printf("No security milestone specified, no security milestone to fetch.")
 		return nil, nil
 	}
-	const project = "security-metadata"
-	head, err := b.PrivateGerritClient.ReadBranchHead(ctx, project, "main")
+	rm, err := task.FetchReleaseMilestone(ctx, b.PrivateGerritClient, milestoneNum)
 	if err != nil {
 		return nil, err
 	}
-	raw, err := b.PrivateGerritClient.ReadFile(ctx, project, head, path.Join("data", "milestones", milestoneNum+".yaml"))
-	if err != nil {
-		return nil, err
-	}
-	var rm relmeta.ReleaseMilestone
-	if err := yaml.Unmarshal(raw, &rm); err != nil {
-		return nil, fmt.Errorf("cannot YAML unmarshal the milestone: %v", err)
-	}
 	return &rm, nil
 }
 
@@ -1233,9 +1223,9 @@ func (b *BuildReleaseTasks) createInternalReleaseBranches(ctx *wf.TaskContext, b
 	return internalBranches, nil
 }
 
-func (b *BuildReleaseTasks) createSecurityCherryPicks(ctx *wf.TaskContext, releaseBranches []string, cls []*gerrit.ChangeInfo) ([]*gerrit.ChangeInfo, error) {
+func (b *BuildReleaseTasks) createSecurityCherryPicks(ctx *wf.TaskContext, releaseBranches []string, changes []*gerrit.ChangeInfo) ([]*gerrit.ChangeInfo, error) {
 	var cherryPicks []*gerrit.ChangeInfo
-	for _, ci := range cls {
+	for _, ci := range changes {
 		for _, releaseBranch := range releaseBranches {
 			// Check whether a non-abandoned cherry-pick of this
 			// change already exists on the target branch (e.g. from
@@ -1315,12 +1305,9 @@ func (b *BuildReleaseTasks) submitCherryPicks(ctx *wf.TaskContext, cherryPicks [
 	return submitted, nil
 }
 
-// majorFromMinor converts a release branch name from its minor version form to
-// its major version form (i.e., release-branch.go1.2.3 to release-branch.go1.2).
 func majorFromMinor(branch string) string {
 	stripped := strings.TrimPrefix(branch, "release-branch.")
-	major := goversion.Lang(stripped)
-	return "release-branch." + major
+	return "release-branch." + goversion.Lang(stripped)
 }
 
 func privateChangeURL[T int | string](clNum T) string {

internal/task/announce.go

@@ -939,7 +939,7 @@ type SecurityCommunicationTasks struct {
 //
 // It returns an empty string when there are 0 security fixes.
 func (t SecurityCommunicationTasks) GetSecuritySummary(ctx *workflow.TaskContext, milestoneNum string) (string, error) {
-	rm, err := fetchReleaseMilestone(ctx, t.PrivateGerrit, milestoneNum)
+	rm, err := FetchReleaseMilestone(ctx, t.PrivateGerrit, milestoneNum)
 	if err != nil {
 		return "", err
 	}
@@ -971,7 +971,7 @@ func (t SecurityCommunicationTasks) GetSecuritySummary(ctx *workflow.TaskContext
 // GetSecurityReleaseNotes fetches a list of descriptions, one for each distinct security fix
 // included in the release identified by milestoneNum, in Markdown format.
 func (t SecurityCommunicationTasks) GetSecurityReleaseNotes(ctx *workflow.TaskContext, milestoneNum string) (releaseNotes []string, _ error) {
-	rm, err := fetchReleaseMilestone(ctx, t.PrivateGerrit, milestoneNum)
+	rm, err := FetchReleaseMilestone(ctx, t.PrivateGerrit, milestoneNum)
 	if err != nil {
 		return nil, err
 	}
@@ -1000,7 +1000,7 @@ You can check with the security release coordinator for this release to confirm
 	numOnlyRE = regexp.MustCompile(`^\d+$`)
 )
 
-func fetchReleaseMilestone(ctx context.Context, private GerritClient, milestoneNum string) (relmeta.ReleaseMilestone, error) {
+func FetchReleaseMilestone(ctx context.Context, private GerritClient, milestoneNum string) (relmeta.ReleaseMilestone, error) {
 	const project = "security-metadata"
 	head, err := private.ReadBranchHead(ctx, project, "main")
 	if err != nil {

internal/task/privx.go

@@ -82,7 +82,7 @@ func (x *PrivXPatch) NewDefinition(tagx *TagXReposTasks) *wf.Definition {
 
 func (x *PrivXPatch) PullMilestone(ctx *wf.TaskContext, milestone string) (*relmeta.ReleaseMilestone, error) {
 	// TODO(nealpatel): Is this ceremony?
-	rm, err := fetchReleaseMilestone(ctx, x.PrivateGerrit, milestone)
+	rm, err := FetchReleaseMilestone(ctx, x.PrivateGerrit, milestone)
 	return &rm, err
 }