internal/task: use milestone metadata to derive security patch changelists
Updates golang/go#76157
Change-Id: I76f6b9c6f75dc3dd331657967641194c84fe2998
Reviewed-on: https://go-review.googlesource.com/c/build/+/696936
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
diff --git a/go.mod b/go.mod
index 7173b51..c417eb6 100644
--- a/go.mod
+++ b/go.mod
@@ -71,6 +71,7 @@
google.golang.org/grpc v1.67.1
google.golang.org/protobuf v1.34.2
gopkg.in/inf.v0 v0.9.1
+ gopkg.in/yaml.v3 v3.0.1
rsc.io/github v0.3.1-0.20240418182958-01bebb0c456a
rsc.io/markdown v0.0.0-20240306144322-0bf8f97ee8ef
)
diff --git a/internal/task/security_release_coalesce.go b/internal/task/security_release_coalesce.go
index 8f0f4f6..58ad3dc 100644
--- a/internal/task/security_release_coalesce.go
+++ b/internal/task/security_release_coalesce.go
@@ -5,10 +5,12 @@
package task
import (
+ "bytes"
"errors"
"fmt"
goversion "go/version"
"net/http"
+ "path"
"regexp"
"strings"
"time"
@@ -16,6 +18,7 @@
"golang.org/x/build/gerrit"
"golang.org/x/build/internal/relui/groups"
wf "golang.org/x/build/internal/workflow"
+ yaml "gopkg.in/yaml.v3"
)
// SecurityReleaseCoalesceTask is the workflow used to preparing patches for
@@ -46,23 +49,21 @@
var numOnlyRe = regexp.MustCompile(`^\d+$`)
wd := wf.New(wf.ACL{Groups: []string{groups.SecurityTeam}})
- clNums := wf.Param(wd, wf.ParamDef[[]string]{
- Name: "Security Patch CL Numbers",
- ParamType: wf.SliceShort,
- Doc: `Gerrit CL numbers for each security patch in a release`,
+ milestoneNum := wf.Param(wd, wf.ParamDef[string]{
+ Name: "Release Milestone",
+ ParamType: wf.BasicString,
+ Doc: `Release milestone for the security patch(es) being released.`,
Example: "123456",
- Check: func(nums []string) error {
- for _, num := range nums {
- if !numOnlyRe.MatchString(num) {
- return errors.New("CL numbers must contain only numbers")
- }
+ Check: func(num string) error {
+ if !numOnlyRe.MatchString(num) {
+ return errors.New("Milestone number must contain only numbers")
}
return nil
},
})
// check CLs are ready
- cls := wf.Task1(wd, "Check changes", x.CheckChanges, clNums)
+ cls := wf.Task1(wd, "Check changes", x.CheckChanges, milestoneNum)
// look up branch names
branchInfo := wf.Task0(wd, "Get branch names", x.GetBranchNames, wf.After(cls))
// create checkpoint branch
@@ -86,6 +87,8 @@
}
func (x *SecurityReleaseCoalesceTask) GetBranchNames(ctx *wf.TaskContext) (branchInfo, error) {
+ // TODO: consider using the release milestone to derive
+ // the active version patch and backports?
currentMajor, _, err := x.Version.GetCurrentMajor(ctx)
if err != nil {
return branchInfo{}, err
@@ -123,24 +126,60 @@
}
}
-func (x *SecurityReleaseCoalesceTask) CheckChanges(ctx *wf.TaskContext, clNums []string) ([]*gerrit.ChangeInfo, error) {
+// ReleaseMilestone contains all
+// patches and their respective
+// metadata for a given release.
+type ReleaseMilestone struct {
+ BuganizerID int `yaml:"buganizer_id"`
+ Patches []*SecurityPatch `yaml:"security_patches"`
+}
+
+// SecurityPatch is a subset of the
+// required metadata to release all
+// patches contained by a milestone.
+type SecurityPatch struct {
+ Changelists []string `yaml:"changelists"`
+ TargetedReleases []string `yaml:"target_releases"`
+}
+
+func (x *SecurityReleaseCoalesceTask) CheckChanges(ctx *wf.TaskContext, milestoneNum string) ([]*gerrit.ChangeInfo, error) {
+ const project = "security-metadata"
var cls []*gerrit.ChangeInfo
- for _, num := range clNums {
- ci, err := x.PrivateGerrit.GetChange(ctx, num, gerrit.QueryChangesOpt{Fields: []string{"SUBMITTABLE"}})
- if err != nil {
- return nil, err
+
+ head, err := x.PrivateGerrit.ReadBranchHead(ctx, project, "main")
+ if err != nil {
+ return nil, err
+ }
+
+ buf, err := x.PrivateGerrit.ReadFile(ctx, project, head, path.Join("data", "milestones", milestoneNum+".yaml"))
+ if err != nil {
+ return nil, err
+ }
+
+ var rm ReleaseMilestone
+ if err := yaml.NewDecoder(bytes.NewReader(buf)).Decode(&rm); err != nil {
+ return nil, fmt.Errorf("cannot read milestone: %v", err)
+ }
+
+ for _, patch := range rm.Patches {
+ for _, url := range patch.Changelists {
+ _, num, _ := strings.Cut(url, "/+/")
+ ci, err := x.PrivateGerrit.GetChange(ctx, num, gerrit.QueryChangesOpt{Fields: []string{"SUBMITTABLE"}})
+ if err != nil {
+ return nil, err
+ }
+ if !ci.Submittable {
+ return nil, fmt.Errorf("change %s is not submittable", internalGerritChangeURL(num))
+ }
+ ra, err := x.PrivateGerrit.GetRevisionActions(ctx, num, "current")
+ if err != nil {
+ return nil, err
+ }
+ if ra["submit"] == nil || !ra["submit"].Enabled {
+ return nil, fmt.Errorf("change %s is not submittable", internalGerritChangeURL(num))
+ }
+ cls = append(cls, ci)
}
- if !ci.Submittable {
- return nil, fmt.Errorf("Change %s is not submittable", internalGerritChangeURL(num))
- }
- ra, err := x.PrivateGerrit.GetRevisionActions(ctx, num, "current")
- if err != nil {
- return nil, err
- }
- if ra["submit"] == nil || !ra["submit"].Enabled {
- return nil, fmt.Errorf("Change %s is not submittable", internalGerritChangeURL(num))
- }
- cls = append(cls, ci)
}
return cls, nil
}
@@ -232,6 +271,9 @@
var internalReleaseBranchPrefix = "internal-"
func (x *SecurityReleaseCoalesceTask) CreateInternalReleaseBranches(ctx *wf.TaskContext, bi branchInfo) ([]string, error) {
+ // TODO: update step to commit the metadata
+ // about the submitted changes and their
+ // branch hashes to security-metadata.
var internalBranches []string
for _, next := range bi.PublicReleaseBranches {
publicHead, err := x.PrivateGerrit.ReadBranchHead(ctx, "go", majorFromMinor(next))
diff --git a/internal/task/security_release_coalesce_test.go b/internal/task/security_release_coalesce_test.go
index f456628..260999f 100644
--- a/internal/task/security_release_coalesce_test.go
+++ b/internal/task/security_release_coalesce_test.go
@@ -9,6 +9,7 @@
"crypto/rand"
"errors"
"fmt"
+ "path"
"slices"
"strings"
"testing"
@@ -105,10 +106,8 @@
if project != "go" {
return gerrit.TagInfo{}, gerrit.ErrResourceNotExist
}
- for _, t := range c.tags {
- if tag == t {
- return gerrit.TagInfo{Created: gerrit.TimeStamp(time.Now())}, nil
- }
+ if slices.Contains(c.tags, tag) {
+ return gerrit.TagInfo{Created: gerrit.TimeStamp(time.Now())}, nil
}
return gerrit.TagInfo{}, gerrit.ErrResourceNotExist
}
@@ -132,14 +131,25 @@
})
}
+const milestoneYAML = `buganizer_id: 100001
+security_patches:
+ - is_toolchain: false
+ package: runtime
+ changelists:
+ - https://go-internal-review.git.corp.google.com/c/security-metadata/+/1234
+ - https://go-internal-review.git.corp.google.com/c/security-metadata/+/5678
+ target_releases:
+ - go1.3.1
+ - go1.4.1`
+
func testSecurityReleaseCoalesceTask(t *testing.T, withNextReleaseBranch bool) {
publicTags := []string{"go1.3", "go1.3.1", "go1.4", "go1.4.1"}
publicBranches := []string{"release-branch.go1.3", "release-branch.go1.4"}
if withNextReleaseBranch {
publicBranches = append(publicBranches, "release-branch.go1.5")
}
- privRepo := NewFakeRepo(t, "go")
- privGerrit := &fakeCoalesceGerrit{FakeGerrit: NewFakeGerrit(t, privRepo), cherryPicks: map[string][]cherryPickedCommit{}, commitMessages: map[string]string{}}
+ privGoRepo, smRepo := NewFakeRepo(t, "go"), NewFakeRepo(t, "security-metadata")
+ privGerrit := &fakeCoalesceGerrit{FakeGerrit: NewFakeGerrit(t, privGoRepo, smRepo), cherryPicks: map[string][]cherryPickedCommit{}, commitMessages: map[string]string{}}
task := &SecurityReleaseCoalesceTask{
PrivateGerrit: privGerrit,
Version: &VersionTasks{
@@ -176,17 +186,21 @@
other body`,
}
- head := privRepo.History()[0]
- privRepo.Branch("public", head)
- privRepo.Branch("release-branch.go1.3", head)
- privRepo.Branch("release-branch.go1.4", head)
+ head := privGoRepo.History()[0]
+ privGoRepo.Branch("public", head)
+ privGoRepo.Branch("release-branch.go1.3", head)
+ privGoRepo.Branch("release-branch.go1.4", head)
if withNextReleaseBranch {
- privRepo.Branch("release-branch.go1.5", head)
+ privGoRepo.Branch("release-branch.go1.5", head)
}
+ head = smRepo.History()[0]
+ smRepo.Branch("main", head)
+ smRepo.CommitOnBranch("main", map[string]string{path.Join("data", "milestones", "100001.yaml"): milestoneYAML})
+
wd := task.NewDefinition()
w, err := wf.Start(wd, map[string]any{
- "Security Patch CL Numbers": []string{"1234", "5678"},
+ "Release Milestone": "100001",
})
if err != nil {
t.Fatal(err)
@@ -203,7 +217,7 @@
if withNextReleaseBranch {
checkpointBranch = "go1.5rc1-go1.4.2-go1.3.2-checkpoint"
}
- commits := len(strings.Split(string(privRepo.runGit("log", checkpointBranch, "--format=%H")), "\n")) - 1
+ commits := len(strings.Split(string(privGoRepo.runGit("log", checkpointBranch, "--format=%H")), "\n")) - 1
if commits != 3 {
t.Errorf("unexpected number of commits on checkpoint branch: got %d, want 3", commits)
}
