Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add VPC SC error handling and propagate uid in user facing error message #932

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add VPC SC error handling and propagate uid in user facing error message #932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions gcs-fetcher/pkg/fetcher/fetcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0
http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
Expand Down Expand Up @@ -67,6 +67,7 @@ var (
noTimeout = 0 * time.Second
errGCSTimeout = errors.New("GCS timeout")

vpcscRegex = regexp.MustCompile(`<Details>(.+vpcServiceControlsUniqueIdentifier: .+)<\/Details>`)
robotRegex = regexp.MustCompile(`<Details>(\S+@\S+)\s`)
nonHexRegex = regexp.MustCompile(`[^0-9a-f]`)
)
Expand Down Expand Up @@ -160,6 +161,15 @@ type Fetcher struct {
Stderr io.Writer
}

type vpcscError struct {
bucket string
details string
}

func (e *vpcscError) Error() string {
return fmt.Sprintf("Access to bucket %s denied. %s.", e.bucket, e.details)
}

type permissionError struct {
bucket string
robot string
Expand Down Expand Up @@ -268,11 +278,14 @@ func (gf *Fetcher) fetchObject(ctx context.Context, j job) *jobReport {
allowedGCSTimeout := gf.timeout(j.filename, retrynum)
size, err := gf.fetchObjectOnceWithTimeout(ctx, j, allowedGCSTimeout, tmpfile)
if err != nil {
// Allow permissionError to bubble up.
// Allow permissionError and vpcscError to bubble up.
e := err
if _, ok := err.(*permissionError); !ok {
switch e.(type) {
case *permissionError, *vpcscError:
default:
e = fmt.Errorf("fetching %q with timeout %v to temp file %q: %v", formatGCSName(j.bucket, j.object, j.generation), allowedGCSTimeout, tmpfile, err)
}

gf.recordFailure(j, started, allowedGCSTimeout, e, report)
continue
}
Expand Down Expand Up @@ -352,6 +365,11 @@ func (gf *Fetcher) fetchObjectOnce(ctx context.Context, j job, dest string, brea
if err != nil {
// Check for AccessDenied failure here and return a useful error message on Stderr and exit immediately.
if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == http.StatusForbidden {
// Check if VPC SC is the source of Access Denial
if match := vpcscRegex.FindStringSubmatch(err.Error()); len(match) == 2 {
result.err = &vpcscError{bucket: j.bucket, details: match[1]}
return result
}
// Try to parse out the robot name.
match := robotRegex.FindStringSubmatch(err.Error())
robot := "your Cloud Build service account"
Expand Down Expand Up @@ -564,8 +582,9 @@ func (gf *Fetcher) fetchFromManifest(ctx context.Context) (err error) {
report := gf.fetchObject(ctx, j)
gf.Retries, gf.Backoff = oretries, obackoff
if !report.success {
if err, ok := report.err.(*permissionError); ok {
gf.logErr(err.Error())
switch report.err.(type) {
case *permissionError, *vpcscError:
gf.logErr(report.err.Error())
os.Exit(1)
}
return fmt.Errorf("failed to download manifest %s: %v", formatGCSName(gf.Bucket, gf.Object, gf.Generation), report.err)
Expand Down
32 changes: 31 additions & 1 deletion gcs-fetcher/pkg/fetcher/fetcher_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0
http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
Expand Down Expand Up @@ -49,6 +49,7 @@ const (
efile2 = "efile2"
efile3 = "efile3"
efile4 = "efile4"
efile5 = "efile5"
errorManifest = "error-manifest.json"
errorZipfile = "error-source.zip"

Expand Down Expand Up @@ -81,6 +82,7 @@ var (
errMkdirAll = fmt.Errorf("instrumented os.MkdirAll error")
errOpen = fmt.Errorf("instrumented os.Open error")
errGCS403 = fmt.Errorf("instrumented GCS AccessDenied error")
errVPCSC = fmt.Errorf("instrumented VPC SC error")
)

type fakeGCSErrorReader struct {
Expand Down Expand Up @@ -127,6 +129,15 @@ func (f *fakeGCS) NewReader(context context.Context, bucket, object string) (io.
return ioutil.NopCloser(bytes.NewReader([]byte(""))), err
}

if response.err == errVPCSC {
message := "<Xml><Error><Code>SecurityPolicyViolated</Code><Details>Request is prohibited by organization's policy. vpcServiceControlsUniqueIdentifier: 1234567890_ABCDefg</Details></Error>"
err := &googleapi.Error{
Code: 403,
Body: message,
}
return ioutil.NopCloser(bytes.NewReader([]byte(""))), err
}

if response.err == errGCSRead {
return ioutil.NopCloser(fakeGCSErrorReader{err: response.err}), nil
}
Expand Down Expand Up @@ -225,6 +236,7 @@ func buildManifestTestContext(t *testing.T) (tc *testContext, teardown func()) {
formatGCSName(errorBucket, efile2, generation): {err: errGCSRead},
formatGCSName(errorBucket, efile3, generation): {err: errGCSSlowRead},
formatGCSName(errorBucket, efile4, generation): {err: errGCS403},
formatGCSName(errorBucket, efile5, generation): {err: errVPCSC},
formatGCSName(successBucket, goodManifest, generation): {content: goodManifestContents},
formatGCSName(successBucket, malformedManifest, generation): {content: malformedManifestContents},
formatGCSName(errorBucket, errorManifest, generation): {err: errGCSRead},
Expand Down Expand Up @@ -284,6 +296,24 @@ func TestFetchObjectOnceStoresFile(t *testing.T) {
}
}

func TestVPCSCDenial(t *testing.T) {
tc, teardown := buildManifestTestContext(t)
defer teardown()
j := job{bucket: errorBucket, object: efile5}
result := tc.gf.fetchObjectOnce(context.Background(), j, filepath.Join(tc.workDir, "efile5.tmp"), make(chan struct{}, 1))
if result.err == nil {
t.Fatalf("fetchObjectOnce did not fail, got err=nil, want err!=nil")
}
if err, ok := result.err.(*vpcscError); ok {
want := `Access to bucket error-bucket denied. Request is prohibited by organization's policy. vpcServiceControlsUniqueIdentifier: 1234567890_ABCDefg.`
if err.Error() != want {
t.Fatalf("incorrect error message, got %q, want %q", err.Error(), want)
}
} else {
t.Fatalf("got err=%q, want vpcscError", result.err)
}
}

func TestGCSAccessDenied(t *testing.T) {
tc, teardown := buildManifestTestContext(t)
defer teardown()
Expand Down