From 7c2b3a141d179d84702acbc9e905fca657b16789 Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Tue, 3 Sep 2024 19:58:09 -0700 Subject: [PATCH] trim leading slash before uploading chains to S3 The stage environment went down for a while. The details of why can be found in AUT-231. One of the triggers was that aws-go-sdk-v2 no longer removes leading slashes from uploaded S3 key names like the v1 version of the library did. That caused some cascading failures leading to too many AWS CloudHSM slots being consumed. This change trims the leading slashes from the uploaded S3 key names for the contentsignaturepki certificate chains. This change also gets us closer to not needing to be careful about remembering to end chainUploadLocation strings with a slash with an application of `path.Join`. The `file://` handling elsewhere is still bugged and so are the places where we do `X5U + chainName`. Updates AUT-233 Updates AUT-231 --- signer/contentsignaturepki/upload.go | 6 +- signer/contentsignaturepki/upload_test.go | 70 +++++++++++++++++------ 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/signer/contentsignaturepki/upload.go b/signer/contentsignaturepki/upload.go index 83d87b0db..6a96bade5 100755 --- a/signer/contentsignaturepki/upload.go +++ b/signer/contentsignaturepki/upload.go @@ -9,6 +9,7 @@ import ( "net/http" "net/url" "os" + "path" "strings" "time" @@ -53,9 +54,12 @@ func (s *ContentSigner) upload(data, name string) error { } func uploadToS3(client S3UploadAPI, data, name string, target *url.URL) error { + // aws-sdk-go-v2 now includes leading slashes in the key name, where v1 did + // not. So, to keep this code compatible, we have to trim it. + keyName := strings.TrimPrefix(path.Join(target.Path, name), "/") _, err := client.Upload(context.Background(), &s3.PutObjectInput{ Bucket: aws.String(target.Host), - Key: aws.String(target.Path + name), + Key: aws.String(keyName), ACL: types.ObjectCannedACLPublicRead, Body: strings.NewReader(data), ContentType: aws.String("binary/octet-stream"), diff --git a/signer/contentsignaturepki/upload_test.go b/signer/contentsignaturepki/upload_test.go index 71e96736c..7ef9982d4 100644 --- a/signer/contentsignaturepki/upload_test.go +++ b/signer/contentsignaturepki/upload_test.go @@ -19,42 +19,78 @@ func (m mockUploadAPI) Upload(ctx context.Context, input *s3.PutObjectInput, opt func TestUploadToS3(t *testing.T) { cases := []struct { - client func(t *testing.T) S3UploadAPI - data string - name string - target string - expectErr bool + testName string + client func(t *testing.T) S3UploadAPI + data string + name string + chainUploadLocation string + expectErr bool }{ { + testName: "successful_upload", client: func(t *testing.T) S3UploadAPI { return mockUploadAPI(func(ctx context.Context, input *s3.PutObjectInput, opts ...func(*manager.Uploader)) (*manager.UploadOutput, error) { - t.Helper() + expectedBucket := "foo.bar" + if *input.Bucket != expectedBucket { + t.Errorf("bucket: want %#v, got %#v", expectedBucket, *input.Bucket) + } + if *input.Key != "somestuff/successful_chain" { + t.Errorf("key: want \"somestuff/successful_chain\", got %#v", *input.Key) + } return &manager.UploadOutput{}, nil }) }, - data: "foo", - name: "successful_upload", - target: "https://foo.bar", - expectErr: false, + data: "foo", + name: "successful_chain", + chainUploadLocation: "s3://foo.bar/somestuff/", + expectErr: false, }, { + testName: "successful_upload_with_missing_slash", client: func(t *testing.T) S3UploadAPI { return mockUploadAPI(func(ctx context.Context, input *s3.PutObjectInput, opts ...func(*manager.Uploader)) (*manager.UploadOutput, error) { - t.Helper() + expectedBucket := "foo.bar" + if *input.Bucket != expectedBucket { + t.Errorf("bucket: want %#v, got %#v", expectedBucket, *input.Bucket) + } + expectedKey := "somestuff/successful_chain" + if *input.Key != expectedKey { + t.Errorf("key: want %#v, got %#v", expectedKey, *input.Key) + } + return &manager.UploadOutput{}, nil + }) + }, + data: "foo", + name: "successful_chain", + chainUploadLocation: "s3://foo.bar/somestuff", + expectErr: false, + }, + { + testName: "failed_upload", + client: func(t *testing.T) S3UploadAPI { + return mockUploadAPI(func(ctx context.Context, input *s3.PutObjectInput, opts ...func(*manager.Uploader)) (*manager.UploadOutput, error) { + expectedBucket := "foo.quux" + if *input.Bucket != expectedBucket { + t.Errorf("bucket: want %#v, got %#v", expectedBucket, *input.Bucket) + } + expectedKey := "something/will_fail_chain" + if *input.Key != expectedKey { + t.Errorf("key: want %#v, got %#v", expectedKey, *input.Key) + } return nil, errors.New("upload failed") }) }, - data: "foo", - name: "failed_upload", - target: "https://foo.bar", - expectErr: true, + data: "foo", + name: "will_fail_chain", + chainUploadLocation: "s3://foo.quux/something/", + expectErr: true, }, } for _, tt := range cases { - t.Run(tt.name, func(t *testing.T) { + t.Run(tt.testName, func(t *testing.T) { t.Parallel() - url, err := url.Parse(tt.target) + url, err := url.Parse(tt.chainUploadLocation) if err != nil { t.Fatalf("error parsing test url: %v", err) }