Skip to content

Commit

Permalink
Fix ObjectExistsWithSize s3 implementation.
Browse files Browse the repository at this point in the history
- We're not correctly returning the size of the file when there's no error
  • Loading branch information
Dylan Guedes authored and Dylan Guedes committed Oct 1, 2024
1 parent 11b92ee commit 82cdd4b
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 6 deletions.
8 changes: 2 additions & 6 deletions pkg/storage/chunk/client/aws/s3_storage_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ func (a *S3ObjectClient) ObjectExistsWithSize(ctx context.Context, objectKey str
return nil
})
if lastErr == nil {
return true, 0, nil
return true, objectSize, nil
}

if a.IsObjectNotFoundErr(lastErr) {
Expand All @@ -348,11 +348,7 @@ func (a *S3ObjectClient) ObjectExistsWithSize(ctx context.Context, objectKey str
retries.Wait()
}

if lastErr != nil {
return false, 0, lastErr
}

return true, objectSize, nil
return false, 0, lastErr
}

// DeleteObject deletes the specified objectKey from the appropriate S3 bucket
Expand Down
32 changes: 32 additions & 0 deletions pkg/storage/chunk/client/aws/s3_storage_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,38 @@ func (m *MockS3Client) HeadObject(input *s3.HeadObjectInput) (*s3.HeadObjectOutp
return m.HeadObjectFunc(input)
}

func Test_ExistsWithSize(t *testing.T) {
mockS3 := &MockS3Client{
HeadObjectFunc: func(_ *s3.HeadObjectInput) (*s3.HeadObjectOutput, error) {
var size int64

Check failure on line 315 in pkg/storage/chunk/client/aws/s3_storage_client_test.go

View workflow job for this annotation

GitHub Actions / check / golangciLint

S1021: should merge variable declaration with assignment on next line (gosimple)
size = 128
return &s3.HeadObjectOutput{ContentLength: &size}, nil
},
}

c, err := NewS3ObjectClient(S3Config{
AccessKeyID: "foo",
SecretAccessKey: flagext.SecretWithValue("bar"),
BackoffConfig: backoff.Config{MaxRetries: 3},
BucketNames: "foo",
Inject: func(_ http.RoundTripper) http.RoundTripper {
return RoundTripperFunc(func(_ *http.Request) (*http.Response, error) {
return &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(bytes.NewReader([]byte("object content"))),
}, nil
})
},
}, hedging.Config{})
require.NoError(t, err)
c.S3 = mockS3

exists, size, err := c.ObjectExistsWithSize(context.Background(), "abc")
require.NoError(t, err)
require.EqualValues(t, 128, size)
require.True(t, exists)
}

func Test_RetryLogic(t *testing.T) {
for _, tc := range []struct {
name string
Expand Down

0 comments on commit 82cdd4b

Please sign in to comment.