particle-os/debian-forge-composer
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

GCP: Fix panic while parsing a specific build job log

Browse source 
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 <thozza@redhat.com>
This commit is contained in:
Tomas Hozza 2021-04-27 21:08:29 +02:00 • committed by Tomas Hozza
parent 27c5aafeca
commit 5e591ccc3d
2 changed files with 65 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

12
internal/cloud/gcp/cloudbuild.go
View file

@ -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".

57
internal/cloud/gcp/cloudbuild_test.go
View file

@ -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 {