Skip to content

Commit

Permalink
trim leading slash before uploading chains to S3 (#969)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jmhodges committed Sep 4, 2024
1 parent 869ae50 commit 2a91bf5
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 18 deletions.
6 changes: 5 additions & 1 deletion signer/contentsignaturepki/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/http"
"net/url"
"os"
"path"
"strings"
"time"

Expand Down Expand Up @@ -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"),
Expand Down
70 changes: 53 additions & 17 deletions signer/contentsignaturepki/upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit 2a91bf5

Please sign in to comment.