Skip to content

Commit

Permalink
internal/relui: run "advisory" TryBots
Browse files Browse the repository at this point in the history 
We don't have the ability to run security fixes through the real TrybBts
before we commit them to the private repo. To avoid surprises like we
just had with the wasm tests, run secondary TryBots, but don't treat
them as release-blocking. If any fail, the coordinator will be asked to
approve the results.

For golang/go#53799.

Change-Id: Icad4ece6e32f47dc81f4a8d850f56cf488c7a030
Reviewed-on: https://go-review.googlesource.com/c/build/+/419415
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Run-TryBot: Heschi Kreinick <[email protected]>
Auto-Submit: Heschi Kreinick <[email protected]>
  • Loading branch information
@heschi @gopherbot
heschi authored and gopherbot committed Jul 26, 2022
1 parent 7032644 commit baccc13
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 34 deletions.
57 changes: 53 additions & 4 deletions internal/relui/buildrelease_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,11 @@ func newReleaseTestDeps(t *testing.T, wantVersion string) *releaseTestDeps {
CreateBuildlet: fakeBuildlets.createBuildlet,
DownloadURL: dlServer.URL,
PublishFile: publishFile,
ApproveAction: func(*workflow.TaskContext, interface{}) error {
return nil
ApproveAction: func(ctx *workflow.TaskContext, _ interface{}) error {
if strings.Contains(ctx.TaskName, "Release Coordinator Approval") {
return nil
}
return fmt.Errorf("unexpected approval for %q", ctx.TaskName)
},
}
// Cleanups are called in reverse order, and we need to cancel the context
Expand Down Expand Up @@ -172,7 +175,7 @@ func testRelease(t *testing.T, wantVersion string, kind task.ReleaseKind) {
wd.Output("Published Go version", v)

w, err := workflow.Start(wd, map[string]interface{}{
"Targets to skip testing (or 'all') (optional)": []string(nil),
"Targets to skip testing (or 'all') (optional)": []string{"js-wasm"},
"Ref from the private repository to build from (optional)": "",
})
if err != nil {
Expand Down Expand Up @@ -291,7 +294,7 @@ func testSecurity(t *testing.T, mergeFixes bool) {
wd.Output("Published Go version", v)

w, err := workflow.Start(wd, map[string]interface{}{
"Targets to skip testing (or 'all') (optional)": []string(nil),
"Targets to skip testing (or 'all') (optional)": []string{"js-wasm"},
"Ref from the private repository to build from (optional)": "security-ref",
})
if err != nil {
Expand All @@ -316,6 +319,42 @@ func testSecurity(t *testing.T, mergeFixes bool) {
})
}

func TestAdvisoryTrybotFail(t *testing.T) {
deps := newReleaseTestDeps(t, "go1.18rc1")
defaultApprove := deps.buildTasks.ApproveAction
approvedTrybots := false
deps.buildTasks.ApproveAction = func(ctx *workflow.TaskContext, i interface{}) error {
if strings.Contains(ctx.TaskName, "TryBot failures") {
approvedTrybots = true
return nil
}
return defaultApprove(ctx, i)
}

// Run the release.
wd := workflow.New()
v, err := addSingleReleaseWorkflow(deps.buildTasks, deps.milestoneTasks, deps.versionTasks, wd, 18, task.KindRC)
if err != nil {
t.Fatal(err)
}
wd.Output("Published Go version", v)

w, err := workflow.Start(wd, map[string]interface{}{
"Targets to skip testing (or 'all') (optional)": []string(nil),
"Ref from the private repository to build from (optional)": "",
})
if err != nil {
t.Fatal(err)
}
if _, err := w.Run(deps.ctx, &verboseListener{t, deps.outputListener}); err != nil {
t.Fatal(err)
}
if !approvedTrybots {
t.Errorf("advisory trybots didn't need approval")
}

}

// makeScript pretends to be make.bash. It creates a fake go command that
// knows how to fake the commands the release process runs.
const makeScript = `#!/bin/bash
Expand Down Expand Up @@ -356,6 +395,12 @@ touch $GO/tool/something_orother/compile
const allScript = `#!/bin/bash -eu
echo "I'm a test! :D"
if [[ $GO_BUILDER_NAME =~ "js-wasm" ]]; then
echo "Oh no, WASM is broken"
exit 1
fi
exit 0
`

Expand All @@ -371,6 +416,8 @@ func serveSnapshot(w http.ResponseWriter, r *http.Request) {
"src/make.bat": makeScript,
"src/all.bash": allScript,
"src/all.bat": allScript,
"src/race.bash": allScript,
"src/race.bat": allScript,
}, w, r)
}

Expand All @@ -380,6 +427,8 @@ func serveSecureSnapshot(w http.ResponseWriter, r *http.Request) {
"src/make.bat": makeScript,
"src/all.bash": allScript,
"src/all.bat": allScript,
"src/race.bash": allScript,
"src/race.bat": allScript,
"security.txt": "This file makes us secure",
}, w, r)
}
Expand Down
116 changes: 86 additions & 30 deletions internal/relui/workflows.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"math/rand"
"net/http"
"path"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -772,13 +773,19 @@ func (tasks *BuildReleaseTasks) addBuildTasks(wd *workflow.Definition, major int
if target.BuildOnly {
continue
}
short := wd.Action("Run short tests", tasks.runTests, targetVal, wd.Constant(target.Builder), skipTests, bin)
short := wd.Action("Run short tests", tasks.runTests, targetVal, wd.Constant(dashboard.Builders[target.Builder]), skipTests, bin)
testsPassed = append(testsPassed, short)
if target.LongTestBuilder != "" {
long := wd.Action("Run long tests", tasks.runTests, targetVal, wd.Constant(target.LongTestBuilder), skipTests, bin)
long := wd.Action("Run long tests", tasks.runTests, targetVal, wd.Constant(dashboard.Builders[target.Builder]), skipTests, bin)
testsPassed = append(testsPassed, long)
}
}
var advisoryResults []workflow.Value
for _, bc := range advisoryTryBots(major) {
result := wd.Task("Run advisory TryBot "+bc.Name, tasks.runAdvisoryTryBot, wd.Constant(bc), skipTests, source)
advisoryResults = append(advisoryResults, result)
}
tryBotsApproved := wd.Action("Approve any TryBot failures", tasks.checkAdvisoryTrybots, wd.Slice(advisoryResults...))
if skipSigning {
builtAndTested := wd.Task("Wait for artifacts and tests", func(ctx *workflow.TaskContext, artifacts []artifact) ([]artifact, error) {
return artifacts, nil
Expand All @@ -789,10 +796,33 @@ func (tasks *BuildReleaseTasks) addBuildTasks(wd *workflow.Definition, major int
signedArtifacts := wd.Task("Wait for signed artifacts", tasks.awaitSigned, version, wd.Constant(darwinTargets), stagedArtifacts)
signedAndTested := wd.Task("Wait for signing and tests", func(ctx *workflow.TaskContext, artifacts []artifact) ([]artifact, error) {
return artifacts, nil
}, append([]workflow.TaskInput{signedArtifacts}, testsPassed...)...)
}, append([]workflow.TaskInput{signedArtifacts, tryBotsApproved}, testsPassed...)...)
return signedAndTested, nil
}

func advisoryTryBots(major int) []*dashboard.BuildConfig {
usedBuilders := map[string]bool{}
for _, t := range releasetargets.TargetsForGo1Point(major) {
usedBuilders[t.Builder] = true
usedBuilders[t.LongTestBuilder] = true
}

var extras []*dashboard.BuildConfig
for name, bc := range dashboard.Builders {
if usedBuilders[name] {
continue
}
if !bc.BuildsRepoPostSubmit("go", fmt.Sprintf("release-branch.go1.%d", major), "") {
continue
}
if !bc.IsVM() && !bc.IsContainer() {
continue
}
extras = append(extras, bc)
}
return extras
}

// BuildReleaseTasks serves as an adapter to the various build tasks in the task package.
type BuildReleaseTasks struct {
GerritHTTPClient *http.Client
Expand All @@ -807,7 +837,7 @@ type BuildReleaseTasks struct {
}

func (b *BuildReleaseTasks) buildSource(ctx *workflow.TaskContext, revision, securityRevision, version string) (artifact, error) {
return b.runBuildStep(ctx, nil, "", artifact{}, "src.tar.gz", func(_ *task.BuildletStep, _ io.Reader, w io.Writer) error {
return b.runBuildStep(ctx, nil, nil, artifact{}, "src.tar.gz", func(_ *task.BuildletStep, _ io.Reader, w io.Writer) error {
if securityRevision != "" {
return task.WriteSourceArchive(ctx, b.GerritHTTPClient, b.PrivateGerritURL, securityRevision, version, w)
}
Expand All @@ -816,7 +846,7 @@ func (b *BuildReleaseTasks) buildSource(ctx *workflow.TaskContext, revision, sec
}

func (b *BuildReleaseTasks) checkSourceMatch(ctx *workflow.TaskContext, head, version string, source artifact) error {
_, err := b.runBuildStep(ctx, nil, "", source, "", func(_ *task.BuildletStep, r io.Reader, _ io.Writer) error {
_, err := b.runBuildStep(ctx, nil, nil, source, "", func(_ *task.BuildletStep, r io.Reader, _ io.Writer) error {
branchArchive := &bytes.Buffer{}
if err := task.WriteSourceArchive(ctx, b.GerritHTTPClient, b.GerritURL, head, version, branchArchive); err != nil {
return err
Expand Down Expand Up @@ -862,74 +892,100 @@ func tarballHashes(r io.Reader) (map[string]string, error) {
}

func (b *BuildReleaseTasks) buildBinary(ctx *workflow.TaskContext, target *releasetargets.Target, source artifact) (artifact, error) {
return b.runBuildStep(ctx, target, target.Builder, source, "tar.gz", func(bs *task.BuildletStep, r io.Reader, w io.Writer) error {
build := dashboard.Builders[target.Builder]
return b.runBuildStep(ctx, target, build, source, "tar.gz", func(bs *task.BuildletStep, r io.Reader, w io.Writer) error {
return bs.BuildBinary(ctx, r, w)
})
}

func (b *BuildReleaseTasks) buildMSI(ctx *workflow.TaskContext, target *releasetargets.Target, binary artifact) (artifact, error) {
return b.runBuildStep(ctx, target, target.Builder, binary, "msi", func(bs *task.BuildletStep, r io.Reader, w io.Writer) error {
build := dashboard.Builders[target.Builder]
return b.runBuildStep(ctx, target, build, binary, "msi", func(bs *task.BuildletStep, r io.Reader, w io.Writer) error {
return bs.BuildMSI(ctx, r, w)
})
}

func (b *BuildReleaseTasks) convertToZip(ctx *workflow.TaskContext, target *releasetargets.Target, binary artifact) (artifact, error) {
return b.runBuildStep(ctx, target, "", binary, "zip", func(_ *task.BuildletStep, r io.Reader, w io.Writer) error {
return b.runBuildStep(ctx, target, nil, binary, "zip", func(_ *task.BuildletStep, r io.Reader, w io.Writer) error {
return task.ConvertTGZToZIP(r, w)
})
}

func (b *BuildReleaseTasks) runTests(ctx *workflow.TaskContext, target *releasetargets.Target, buildlet string, skipTests []string, binary artifact) error {
skipped := false
func (b *BuildReleaseTasks) runTests(ctx *workflow.TaskContext, target *releasetargets.Target, build *dashboard.BuildConfig, skipTests []string, binary artifact) error {
for _, skip := range skipTests {
if skip == "all" || target.Name == skip {
skipped = true
break
ctx.Printf("Skipping test")
return nil
}
}
if skipped {
ctx.Printf("Skipping test")
return nil
}
_, err := b.runBuildStep(ctx, target, buildlet, binary, "", func(bs *task.BuildletStep, r io.Reader, _ io.Writer) error {
_, err := b.runBuildStep(ctx, target, build, binary, "", func(bs *task.BuildletStep, r io.Reader, _ io.Writer) error {
return bs.TestTarget(ctx, r)
})
return err
}

type tryBotResult struct {
Name string
Passed bool
}

func (b *BuildReleaseTasks) runAdvisoryTryBot(ctx *workflow.TaskContext, bc *dashboard.BuildConfig, skipTests []string, source artifact) (tryBotResult, error) {
for _, skip := range skipTests {
if skip == "all" || bc.Name == skip {
ctx.Printf("Skipping test")
return tryBotResult{bc.Name, true}, nil
}
}

passed := false
_, err := b.runBuildStep(ctx, nil, bc, source, "", func(bs *task.BuildletStep, r io.Reader, w io.Writer) error {
var err error
passed, err = bs.RunTryBot(ctx, r)
return err
})
return tryBotResult{bc.Name, passed}, err
}

func (b *BuildReleaseTasks) checkAdvisoryTrybots(ctx *workflow.TaskContext, results []tryBotResult) error {
var fails []string
for _, r := range results {
if !r.Passed {
fails = append(fails, r.Name)
}
}
if len(fails) == 0 {
return nil
}
ctx.Printf("Some advisory TryBots failed. Check their logs and approve this task if it's okay:\n%v", strings.Join(fails, "\n"))
return b.ApproveAction(ctx, "")
}

// runBuildStep is a convenience function that manages resources a build step might need.
// If target and buildlet name are specified, a BuildletStep will be passed to f.
// If target and build config are specified, a BuildletStep will be passed to f.
// If inputName is specified, it will be opened and passed as a Reader to f.
// If outputSuffix is specified, a unique filename will be generated based off
// it (and the target name, if any), the file will be opened and passed as a
// Writer to f, and an artifact representing it will be returned as the result.
func (b *BuildReleaseTasks) runBuildStep(
ctx *workflow.TaskContext,
target *releasetargets.Target,
buildletName string,
build *dashboard.BuildConfig,
input artifact,
outputSuffix string,
f func(*task.BuildletStep, io.Reader, io.Writer) error,
) (artifact, error) {
var step *task.BuildletStep
if buildletName != "" {
if target == nil {
return artifact{}, fmt.Errorf("target must be specified to use a buildlet")
}
ctx.Printf("Creating buildlet %v.", buildletName)
client, err := b.CreateBuildlet(buildletName)
if build != nil {
ctx.Printf("Creating buildlet %v.", build.Name)
client, err := b.CreateBuildlet(build.Name)
if err != nil {
return artifact{}, err
}
defer client.Close()
buildConfig, ok := dashboard.Builders[buildletName]
if !ok {
return artifact{}, fmt.Errorf("unknown builder: %v", buildConfig)
}
step = &task.BuildletStep{
Target: target,
Buildlet: client,
BuildConfig: buildConfig,
BuildConfig: build,
Watch: true,
}
ctx.Printf("Buildlet ready.")
Expand Down
21 changes: 21 additions & 0 deletions internal/task/buildrelease.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,27 @@ func (b *BuildletStep) TestTarget(ctx *workflow.TaskContext, binaryArchive io.Re
})
}

func (b *BuildletStep) RunTryBot(ctx *workflow.TaskContext, sourceArchive io.Reader) (bool, error) {
ctx.Printf("Pushing source to buildlet.")
if err := b.Buildlet.PutTar(ctx, sourceArchive, ""); err != nil {
return false, fmt.Errorf("failed to put generated source tarball: %v", err)
}

if u := b.BuildConfig.GoBootstrapURL(buildenv.Production); u != "" {
ctx.Printf("Installing go1.4.")
if err := b.Buildlet.PutTarFromURL(ctx, u, go14); err != nil {
return false, err
}
}

ctx.Printf("Testing")
err := b.exec(ctx, goDir+"/"+b.BuildConfig.AllScript(), b.BuildConfig.AllScriptArgs(), buildlet.ExecOpts{})
if err != nil {
ctx.Printf("testing failed: %v", err)
}
return err == nil, nil
}

// exec runs cmd with args. Its working dir is opts.Dir, or the directory of cmd.
// Its environment is the buildlet's environment, plus a GOPATH setting, plus opts.ExtraEnv.
// If the command fails, its output is included in the returned error.
Expand Down

0 comments on commit baccc13

Please sign in to comment.