From 5e591ccc3d2b4b4eac305a0a3ad744fd22f061cf Mon Sep 17 00:00:00 2001 From: Tomas Hozza Date: Tue, 27 Apr 2021 21:08:29 +0200 Subject: [PATCH] GCP: Fix panic while parsing a specific build job log The `cloudbuildResourcesFromBuildLog()` function from the internal GCP package could cause panic while parsing Build job log which failed early and didn't create any Compute Engine resources. The function relied on the `Regexp.FindStringSubmatch()` method to always return a match while being used on the build log. Accessing a member of a nil slice would cause a panic in `osbuild-worker`, such as: Stack trace of thread 185316: #0 0x0000564e5393b5e1 runtime.raise (osbuild-worker) #1 0x0000564e5391fa1e runtime.sigfwdgo (osbuild-worker) #2 0x0000564e5391e354 runtime.sigtrampgo (osbuild-worker) #3 0x0000564e5393b953 runtime.sigtramp (osbuild-worker) #4 0x00007f37e98e3b20 __restore_rt (libpthread.so.0) #5 0x0000564e5393b5e1 runtime.raise (osbuild-worker) #6 0x0000564e5391f5ea runtime.crash (osbuild-worker) #7 0x0000564e53909306 runtime.fatalpanic (osbuild-worker) #8 0x0000564e53908ca1 runtime.gopanic (osbuild-worker) #9 0x0000564e53906b65 runtime.goPanicIndex (osbuild-worker) #10 0x0000564e5420b36e github.com/osbuild/osbuild-composer/internal/cloud/gcp.cloudbuildResourcesFromBuildLog (osbuild-worker) #11 0x0000564e54209ebb github.com/osbuild/osbuild-composer/internal/cloud/gcp.(*GCP).CloudbuildBuildCleanup (osbuild-worker) #12 0x0000564e54b05a9b main.(*OSBuildJobImpl).Run (osbuild-worker) #13 0x0000564e54b08854 main.main (osbuild-worker) #14 0x0000564e5390b722 runtime.main (osbuild-worker) #15 0x0000564e53939a11 runtime.goexit (osbuild-worker) Add a unit test testing this scenario. Make the `cloudbuildResourcesFromBuildLog()` function more robust and not blindly expect to find matches in the build log. As a result the `cloudbuildBuildResources` struct instance returned from the function may be empty. Subsequently make sure that the `CloudbuildBuildCleanup()` method handles an empty `cloudbuildBuildResources` instance correctly. Specifically the `storageCacheDir.bucket` may be an empty string and thus won't exist. Ensure that this does not result in infinite loop by checking for `storage.ErrBucketNotExist` while iterating the bucket objects. Signed-off-by: Tomas Hozza --- internal/cloud/gcp/cloudbuild.go | 12 ++++-- internal/cloud/gcp/cloudbuild_test.go | 57 +++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/internal/cloud/gcp/cloudbuild.go b/internal/cloud/gcp/cloudbuild.go index 075a15b9e..fd878aaa0 100644 --- a/internal/cloud/gcp/cloudbuild.go +++ b/internal/cloud/gcp/cloudbuild.go @@ -148,7 +148,7 @@ func (g *GCP) CloudbuildBuildCleanup(ctx context.Context, buildID string) ([]str objects := bucket.Objects(ctx, &storage.Query{Prefix: resources.storageCacheDir.dir}) for { objAttrs, err := objects.Next() - if err == iterator.Done { + if err == iterator.Done || err == storage.ErrBucketNotExist { break } if err != nil { @@ -178,7 +178,9 @@ func cloudbuildResourcesFromBuildLog(buildLog string) (*cloudbuildBuildResources return &resources, err } zoneMatch := zoneRe.FindStringSubmatch(buildLog) - resources.zone = zoneMatch[1] + if zoneMatch != nil { + resources.zone = zoneMatch[1] + } // extract Storage cache directory // [inflate]: 2021-03-12T13:13:10Z Workflow GCSPath: gs://ascendant-braid-303513-daisy-bkt-us-central1/gce-image-import-2021-03-12T13:13:08Z-btgtd @@ -187,8 +189,10 @@ func cloudbuildResourcesFromBuildLog(buildLog string) (*cloudbuildBuildResources return &resources, err } cacheDirMatch := cacheDirRe.FindStringSubmatch(buildLog) - resources.storageCacheDir.bucket = cacheDirMatch[1] - resources.storageCacheDir.dir = cacheDirMatch[2] + if cacheDirMatch != nil { + resources.storageCacheDir.bucket = cacheDirMatch[1] + resources.storageCacheDir.dir = cacheDirMatch[2] + } // extract Compute disks // [inflate.setup-disks]: 2021-03-12T13:13:11Z CreateDisks: Creating disk "disk-importer-inflate-7366y". diff --git a/internal/cloud/gcp/cloudbuild_test.go b/internal/cloud/gcp/cloudbuild_test.go index 6cb842708..d8f3a0267 100644 --- a/internal/cloud/gcp/cloudbuild_test.go +++ b/internal/cloud/gcp/cloudbuild_test.go @@ -407,6 +407,63 @@ ERROR: build step 0 "gcr.io/compute-image-tools/gce_vm_image_import:release" fai }, }, }, + { + buildLog: `starting build "21eb22bb-b92e-41f7-972d-35e75dae2a2c" + +FETCHSOURCE +BUILD +Pulling image: gcr.io/compute-image-tools/gce_vm_image_import:release +release: Pulling from compute-image-tools/gce_vm_image_import +a352db2f02b6: Pulling fs layer +8e0ed4351c49: Pulling fs layer +7ef2d30124da: Pulling fs layer +7558c9498dac: Pulling fs layer +ac1adc2a272f: Pulling fs layer +1bfe08dab915: Pulling fs layer +d7a35a584f97: Pulling fs layer +14ef19cde991: Pulling fs layer +e3a2159de935: Pulling fs layer +7558c9498dac: Waiting +ac1adc2a272f: Waiting +1bfe08dab915: Waiting +d7a35a584f97: Waiting +14ef19cde991: Waiting +e3a2159de935: Waiting +7ef2d30124da: Verifying Checksum +7ef2d30124da: Download complete +7558c9498dac: Verifying Checksum +7558c9498dac: Download complete +ac1adc2a272f: Verifying Checksum +ac1adc2a272f: Download complete +a352db2f02b6: Verifying Checksum +a352db2f02b6: Download complete +8e0ed4351c49: Verifying Checksum +8e0ed4351c49: Download complete +14ef19cde991: Verifying Checksum +14ef19cde991: Download complete +1bfe08dab915: Verifying Checksum +1bfe08dab915: Download complete +e3a2159de935: Verifying Checksum +e3a2159de935: Download complete +d7a35a584f97: Verifying Checksum +d7a35a584f97: Download complete +a352db2f02b6: Pull complete +8e0ed4351c49: Pull complete +7ef2d30124da: Pull complete +7558c9498dac: Pull complete +ac1adc2a272f: Pull complete +1bfe08dab915: Pull complete +d7a35a584f97: Pull complete +14ef19cde991: Pull complete +e3a2159de935: Pull complete +Digest: sha256:63ab233c04139087154f27797efbebc9e55302f465ffb56e2dce34c2b5bf5d8a +Status: Downloaded newer image for gcr.io/compute-image-tools/gce_vm_image_import:release +gcr.io/compute-image-tools/gce_vm_image_import:release +[import-image]: 2021-04-27T08:08:40Z The resource 'image-8c27cb332db33890146b290a3989198d829ae456dabf96e5a4461147' already exists. Please pick an image name that isn't already used. +ERROR +ERROR: build step 0 "gcr.io/compute-image-tools/gce_vm_image_import:release" failed: step exited with non-zero status: 1`, + resources: cloudbuildBuildResources{}, + }, } for i, tc := range testCases {