From d47034811bcd2a59a95680dbbfcc92b81450e210 Mon Sep 17 00:00:00 2001 From: William Burton Date: Wed, 18 Feb 2026 23:26:43 +0000 Subject: [PATCH 1/2] Add ExtraTags on the GCB config to indicate anlyzer builds --- analyzer/network/analyzer/main.go | 1 + analyzer/system/analyzer/main.go | 1 + pkg/build/gcb/executor.go | 17 ++++++++++++----- pkg/build/gcb/executor_test.go | 20 ++++++++++++++++++++ 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/analyzer/network/analyzer/main.go b/analyzer/network/analyzer/main.go index 3964d42b..d6054f16 100644 --- a/analyzer/network/analyzer/main.go +++ b/analyzer/network/analyzer/main.go @@ -100,6 +100,7 @@ func AnalyzerInit(ctx context.Context) (*analyzerservice.AnalyzerDeps, error) { ServiceAccount: *buildServiceAccount, LogsBucket: *logsBucket, Client: nil, // Defined depending on gcbPrivatePoolName + ExtraTags: map[string]string{"analyzer": "network"}, } if *gcbPrivatePoolName != "" { pool := &gcb.PrivatePoolConfig{ diff --git a/analyzer/system/analyzer/main.go b/analyzer/system/analyzer/main.go index dc58a14d..3195b365 100644 --- a/analyzer/system/analyzer/main.go +++ b/analyzer/system/analyzer/main.go @@ -100,6 +100,7 @@ func AnalyzerInit(ctx context.Context) (*analyzerservice.AnalyzerDeps, error) { ServiceAccount: *buildServiceAccount, LogsBucket: *logsBucket, Client: nil, // Defined depending on gcbPrivatePoolName + ExtraTags: map[string]string{"analyzer": "system"}, } if *gcbPrivatePoolName != "" { pool := &gcb.PrivatePoolConfig{ diff --git a/pkg/build/gcb/executor.go b/pkg/build/gcb/executor.go index cc19eda4..a5674e42 100644 --- a/pkg/build/gcb/executor.go +++ b/pkg/build/gcb/executor.go @@ -35,6 +35,7 @@ type Executor struct { outputBufferSize int builderName string terminateOnTimeout bool + extraTags map[string]string activeBuilds syncx.Map[string, *gcbHandle] } @@ -68,6 +69,7 @@ func NewExecutor(config ExecutorConfig) (*Executor, error) { outputBufferSize: outputBufferSize, builderName: config.BuilderName, terminateOnTimeout: config.TerminateOnTimeout, + extraTags: config.ExtraTags, activeBuilds: syncx.Map[string, *gcbHandle]{}, }, nil } @@ -83,6 +85,7 @@ type ExecutorConfig struct { OutputBufferSize int BuilderName string TerminateOnTimeout bool + ExtraTags map[string]string } // Start implements build.Executor @@ -281,16 +284,20 @@ func (e *Executor) makeBuild(t rebuild.Target, plan *Plan) *cloudbuild.Build { Name: e.privatePool.Name, } } + tags := []string{ + buildTag("ecosystem", string(t.Ecosystem)), + buildTag("package", t.Package), + buildTag("version", t.Version), + } + for k, v := range e.extraTags { + tags = append(tags, buildTag(k, v)) + } return &cloudbuild.Build{ Steps: plan.Steps, Options: buildOptions, LogsBucket: e.logsBucket, ServiceAccount: e.serviceAccount, - Tags: []string{ - buildTag("ecosystem", string(t.Ecosystem)), - buildTag("package", t.Package), - buildTag("version", t.Version), - }, + Tags: tags, } } diff --git a/pkg/build/gcb/executor_test.go b/pkg/build/gcb/executor_test.go index 2ba63556..0b7af331 100644 --- a/pkg/build/gcb/executor_test.go +++ b/pkg/build/gcb/executor_test.go @@ -57,7 +57,9 @@ func TestGCBExecutorStart(t *testing.T) { } createChan := make(chan struct{}) defer close(createChan) + var capturedTags []string mockClient.CreateBuildFunc = func(ctx context.Context, project string, build *cloudbuild.Build) (*cloudbuild.Operation, error) { + capturedTags = build.Tags <-createChan return operation, nil } @@ -72,6 +74,9 @@ func TestGCBExecutorStart(t *testing.T) { ServiceAccount: "test@test.iam.gserviceaccount.com", LogsBucket: "test-bucket", OutputBufferSize: 1024, + ExtraTags: map[string]string{ + "foo": "bar", + }, } executor, err := NewExecutor(config) @@ -138,6 +143,21 @@ func TestGCBExecutorStart(t *testing.T) { t.Fatal(result.Error) } + // Verify tags + expectedTags := []string{"ecosystem-npm", "package-test-package", "version-1.0.0", "foo-bar"} + for _, expected := range expectedTags { + found := false + for _, actual := range capturedTags { + if actual == expected { + found = true + break + } + } + if !found { + t.Errorf("Tag %s not found in %v", expected, capturedTags) + } + } + // Clean up if err := executor.Close(ctx); err != nil { t.Errorf("Close failed: %v", err) From 7f419c268b92d791deab72fbd0568b8cda810daa Mon Sep 17 00:00:00 2001 From: William Burton Date: Thu, 19 Feb 2026 18:19:48 +0000 Subject: [PATCH 2/2] Actually provide timeout setting to GCB executor stacked-commit: true --- analyzer/network/analyzerservice/analyze.go | 10 ++++++---- analyzer/system/analyzerservice/analyze.go | 10 ++++++---- internal/api/agentapiservice/iteration.go | 1 + internal/api/apiservice/rebuild.go | 5 +++-- pkg/build/gcb/executor.go | 3 +++ pkg/build/gcb/executor_test.go | 13 +++++++++---- 6 files changed, 28 insertions(+), 14 deletions(-) diff --git a/analyzer/network/analyzerservice/analyze.go b/analyzer/network/analyzerservice/analyze.go index 4bdf9f39..5e92a625 100644 --- a/analyzer/network/analyzerservice/analyze.go +++ b/analyzer/network/analyzerservice/analyze.go @@ -11,6 +11,7 @@ import ( "encoding/json" "io" "path" + "time" "github.com/google/oss-rebuild/internal/api" "github.com/google/oss-rebuild/internal/cache" @@ -68,7 +69,7 @@ func Analyze(ctx context.Context, req schema.AnalyzeRebuildRequest, deps *Analyz return nil, api.AsStatus(codes.AlreadyExists, errors.New("analysis already exists")) } } - return analyzeRebuild(ctx, t, deps) + return analyzeRebuild(ctx, t, req.Timeout, deps) } func analysisExists(ctx context.Context, store rebuild.AssetStore, t rebuild.Target) (bool, error) { @@ -81,7 +82,7 @@ func analysisExists(ctx context.Context, store rebuild.AssetStore, t rebuild.Tar return true, nil } -func analyzeRebuild(ctx context.Context, t rebuild.Target, deps *AnalyzerDeps) (*api.NoReturn, error) { +func analyzeRebuild(ctx context.Context, t rebuild.Target, timeout time.Duration, deps *AnalyzerDeps) (*api.NoReturn, error) { rebuildAttestation, err := getRebuildAttestation(ctx, deps.InputAttestationStore, t, deps.Verifier) if err != nil { return nil, errors.Wrap(err, "getting rebuild attestation") @@ -92,7 +93,7 @@ func analyzeRebuild(ctx context.Context, t rebuild.Target, deps *AnalyzerDeps) ( } ctx = context.WithValue(ctx, rebuild.HTTPBasicClientID, deps.HTTPClient) mux := meta.NewRegistryMux(httpx.NewCachedClient(deps.HTTPClient, &cache.CoalescingMemoryCache{})) - id, err := executeNetworkRebuild(ctx, deps, t, strategy, rebuildAttestation) + id, err := executeNetworkRebuild(ctx, deps, t, strategy, rebuildAttestation, timeout) if err != nil { return nil, errors.Wrap(err, "network rebuild failed") } @@ -188,7 +189,7 @@ func compareArtifacts(ctx context.Context, mux rebuild.RegistryMux, t rebuild.Ta return &rb, &up, nil } -func executeNetworkRebuild(ctx context.Context, deps *AnalyzerDeps, t rebuild.Target, strategy rebuild.Strategy, rebuildAttestation *attestation.RebuildAttestation) (string, error) { +func executeNetworkRebuild(ctx context.Context, deps *AnalyzerDeps, t rebuild.Target, strategy rebuild.Strategy, rebuildAttestation *attestation.RebuildAttestation, timeout time.Duration) (string, error) { obID := uuid.New().String() debugStore, err := deps.DebugStoreBuilder(context.WithValue(ctx, rebuild.RunID, obID)) if err != nil { @@ -220,6 +221,7 @@ func executeNetworkRebuild(ctx context.Context, deps *AnalyzerDeps, t rebuild.Ta } h, err := deps.GCBExecutor.Start(ctx, in, build.Options{ BuildID: obID, + Timeout: timeout, UseTimewarp: meta.AllRebuilders[t.Ecosystem].UsesTimewarp(in), UseNetworkProxy: true, // The whole point of the analyzer Resources: build.Resources{ diff --git a/analyzer/system/analyzerservice/analyze.go b/analyzer/system/analyzerservice/analyze.go index 45d0d618..244beacb 100644 --- a/analyzer/system/analyzerservice/analyze.go +++ b/analyzer/system/analyzerservice/analyze.go @@ -11,6 +11,7 @@ import ( "encoding/json" "io" "path" + "time" "github.com/google/oss-rebuild/internal/api" "github.com/google/oss-rebuild/internal/cache" @@ -68,7 +69,7 @@ func Analyze(ctx context.Context, req schema.AnalyzeRebuildRequest, deps *Analyz return nil, api.AsStatus(codes.AlreadyExists, errors.New("analysis already exists")) } } - return analyzeRebuild(ctx, t, deps) + return analyzeRebuild(ctx, t, req.Timeout, deps) } func analysisExists(ctx context.Context, store rebuild.AssetStore, t rebuild.Target) (bool, error) { @@ -81,7 +82,7 @@ func analysisExists(ctx context.Context, store rebuild.AssetStore, t rebuild.Tar return true, nil } -func analyzeRebuild(ctx context.Context, t rebuild.Target, deps *AnalyzerDeps) (*api.NoReturn, error) { +func analyzeRebuild(ctx context.Context, t rebuild.Target, timeout time.Duration, deps *AnalyzerDeps) (*api.NoReturn, error) { rebuildAttestation, err := getRebuildAttestation(ctx, deps.InputAttestationStore, t, deps.Verifier) if err != nil { return nil, errors.Wrap(err, "getting rebuild attestation") @@ -92,7 +93,7 @@ func analyzeRebuild(ctx context.Context, t rebuild.Target, deps *AnalyzerDeps) ( } ctx = context.WithValue(ctx, rebuild.HTTPBasicClientID, deps.HTTPClient) mux := meta.NewRegistryMux(httpx.NewCachedClient(deps.HTTPClient, &cache.CoalescingMemoryCache{})) - id, err := executeSystemTraceRebuild(ctx, deps, t, strategy, rebuildAttestation) + id, err := executeSystemTraceRebuild(ctx, deps, t, strategy, rebuildAttestation, timeout) if err != nil { return nil, errors.Wrap(err, "system trace rebuild failed") } @@ -188,7 +189,7 @@ func compareArtifacts(ctx context.Context, mux rebuild.RegistryMux, t rebuild.Ta return &rb, &up, nil } -func executeSystemTraceRebuild(ctx context.Context, deps *AnalyzerDeps, t rebuild.Target, strategy rebuild.Strategy, rebuildAttestation *attestation.RebuildAttestation) (string, error) { +func executeSystemTraceRebuild(ctx context.Context, deps *AnalyzerDeps, t rebuild.Target, strategy rebuild.Strategy, rebuildAttestation *attestation.RebuildAttestation, timeout time.Duration) (string, error) { obID := uuid.New().String() debugStore, err := deps.DebugStoreBuilder(context.WithValue(ctx, rebuild.RunID, obID)) if err != nil { @@ -220,6 +221,7 @@ func executeSystemTraceRebuild(ctx context.Context, deps *AnalyzerDeps, t rebuil } h, err := deps.GCBExecutor.Start(ctx, in, build.Options{ BuildID: obID, + Timeout: timeout, UseTimewarp: meta.AllRebuilders[t.Ecosystem].UsesTimewarp(in), UseSyscallMonitor: true, // The whole point of the analyzer Resources: build.Resources{ diff --git a/internal/api/agentapiservice/iteration.go b/internal/api/agentapiservice/iteration.go index 63db6ed0..5a1c99c5 100644 --- a/internal/api/agentapiservice/iteration.go +++ b/internal/api/agentapiservice/iteration.go @@ -118,6 +118,7 @@ func AgentCreateIteration(ctx context.Context, req schema.AgentCreateIterationRe h, err := deps.GCBExecutor.Start(ctx, input, build.Options{ BuildID: obliviousID, UseTimewarp: meta.AllRebuilders[input.Target.Ecosystem].UsesTimewarp(input), + // TODO: Should we set a Timeout? Resources: build.Resources{ AssetStore: store, ToolURLs: toolURLs, diff --git a/internal/api/apiservice/rebuild.go b/internal/api/apiservice/rebuild.go index 1d57faca..ee497046 100644 --- a/internal/api/apiservice/rebuild.go +++ b/internal/api/apiservice/rebuild.go @@ -131,7 +131,7 @@ func getStrategy(ctx context.Context, deps *RebuildPackageDeps, t rebuild.Target return strategy, entry, nil } -func buildAndAttest(ctx context.Context, deps *RebuildPackageDeps, mux rebuild.RegistryMux, a verifier.Attestor, t rebuild.Target, strategy rebuild.Strategy, entry *repoEntry, useProxy bool, useSyscallMonitor bool, mode schema.OverwriteMode) (err error) { +func buildAndAttest(ctx context.Context, deps *RebuildPackageDeps, mux rebuild.RegistryMux, a verifier.Attestor, t rebuild.Target, strategy rebuild.Strategy, entry *repoEntry, useProxy bool, useSyscallMonitor bool, timeout time.Duration, mode schema.OverwriteMode) (err error) { debugStore, err := deps.DebugStoreBuilder(ctx) if err != nil { return errors.Wrap(err, "creating debug store") @@ -184,6 +184,7 @@ func buildAndAttest(ctx context.Context, deps *RebuildPackageDeps, mux rebuild.R } h, err := deps.GCBExecutor.Start(ctx, in, build.Options{ BuildID: obID, + Timeout: timeout, UseTimewarp: meta.AllRebuilders[t.Ecosystem].UsesTimewarp(in), UseNetworkProxy: useProxy, UseSyscallMonitor: useSyscallMonitor, @@ -323,7 +324,7 @@ func rebuildPackage(ctx context.Context, req schema.RebuildPackageRequest, deps if strategy != nil { v.StrategyOneof = schema.NewStrategyOneOf(strategy) } - err = buildAndAttest(ctx, deps, mux, a, t, strategy, entry, req.UseNetworkProxy, req.UseSyscallMonitor, req.OverwriteMode) + err = buildAndAttest(ctx, deps, mux, a, t, strategy, entry, req.UseNetworkProxy, req.UseSyscallMonitor, req.BuildTimeout, req.OverwriteMode) if err != nil { v.Message = errors.Wrap(err, "executing rebuild").Error() return &v, nil diff --git a/pkg/build/gcb/executor.go b/pkg/build/gcb/executor.go index a5674e42..f5954cc7 100644 --- a/pkg/build/gcb/executor.go +++ b/pkg/build/gcb/executor.go @@ -107,6 +107,9 @@ func (e *Executor) Start(ctx context.Context, input rebuild.Input, opts build.Op } // Make the Cloud Build "Build" request gcbBuild := e.makeBuild(input.Target, plan) + if opts.Timeout > 0 { + gcbBuild.Timeout = fmt.Sprintf("%ds", int(opts.Timeout.Seconds())) + } // Create a buffered pipe for streaming output pipe := bufiox.NewBufferedPipe(bufiox.NewLineBuffer(e.outputBufferSize)) handle := &gcbHandle{ diff --git a/pkg/build/gcb/executor_test.go b/pkg/build/gcb/executor_test.go index 0b7af331..510850ef 100644 --- a/pkg/build/gcb/executor_test.go +++ b/pkg/build/gcb/executor_test.go @@ -57,9 +57,9 @@ func TestGCBExecutorStart(t *testing.T) { } createChan := make(chan struct{}) defer close(createChan) - var capturedTags []string + var gotBuild *cloudbuild.Build mockClient.CreateBuildFunc = func(ctx context.Context, project string, build *cloudbuild.Build) (*cloudbuild.Operation, error) { - capturedTags = build.Tags + gotBuild = build <-createChan return operation, nil } @@ -114,6 +114,7 @@ func TestGCBExecutorStart(t *testing.T) { Resources: build.Resources{ BaseImageConfig: baseImageConfig, }, + Timeout: 10 * time.Minute, } // Test Start method @@ -143,18 +144,22 @@ func TestGCBExecutorStart(t *testing.T) { t.Fatal(result.Error) } + if gotBuild.Timeout != "600s" { + t.Errorf("Timeout = %s, want 600s", gotBuild.Timeout) + } + // Verify tags expectedTags := []string{"ecosystem-npm", "package-test-package", "version-1.0.0", "foo-bar"} for _, expected := range expectedTags { found := false - for _, actual := range capturedTags { + for _, actual := range gotBuild.Tags { if actual == expected { found = true break } } if !found { - t.Errorf("Tag %s not found in %v", expected, capturedTags) + t.Errorf("Tag %s not found in %v", expected, gotBuild.Tags) } }