diff --git a/.github/workflows/s3-integration.yml b/.github/workflows/s3-integration.yml index 3a83cd3..69ac9ca 100644 --- a/.github/workflows/s3-integration.yml +++ b/.github/workflows/s3-integration.yml @@ -177,27 +177,26 @@ jobs: region_name: ${{ env.REGION_NAME }} stack_name: ${{ env.STACK_NAME }} - # TODO: after aws-sdk-go-v2 migration, not working properly. Disabled for now. - # s3-compatible-integration: - # name: S3 Compatible Integration - # runs-on: ubuntu-latest - # steps: - # - name: Checkout code - # uses: actions/checkout@v6 - - # - name: Set up Go - # uses: actions/setup-go@v6 - # with: - # go-version-file: go.mod - - # - name: Install Ginkgo - # run: go install github.com/onsi/ginkgo/v2/ginkgo@latest - - # - name: Run GCS S3 compatible tests - # run: | - # export access_key_id="${{ secrets.GCP_ACCESS_KEY_ID }}" - # export secret_access_key="${{ secrets.GCP_SECRET_ACCESS_KEY }}" - # export bucket_name="storage-cli-test-aws" - # export s3_endpoint_host="https://storage.googleapis.com" - # export s3_endpoint_port="443" - # ./.github/scripts/s3/run-integration-s3-compat.sh \ No newline at end of file + s3-compatible-integration: + name: S3 Compatible Integration + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v6 + + - name: Set up Go + uses: actions/setup-go@v6 + with: + go-version-file: go.mod + + - name: Install Ginkgo + run: go install github.com/onsi/ginkgo/v2/ginkgo@latest + + - name: Run GCS S3 compatible tests + run: | + export access_key_id="${{ secrets.GCP_ACCESS_KEY_ID }}" + export secret_access_key="${{ secrets.GCP_SECRET_ACCESS_KEY }}" + export bucket_name="storage-cli-test-aws" + export s3_endpoint_host="https://storage.googleapis.com" + export s3_endpoint_port="443" + ./.github/scripts/s3/run-integration-s3-compat.sh \ No newline at end of file diff --git a/s3/README.md b/s3/README.md index dc4936b..db97511 100644 --- a/s3/README.md +++ b/s3/README.md @@ -22,14 +22,18 @@ The S3 client requires a JSON configuration file with the following structure: "host": " (optional)", "port": (optional), - "ssl_verify_peer": (optional), - "use_ssl": (optional), + "ssl_verify_peer": (optional - default: true), + "use_ssl": (optional - default: true), "signature_version": " (optional)", "server_side_encryption": " (optional)", "sse_kms_key_id": " (optional)", "multipart_upload": (optional - default: true) } ``` +> Note: Provider specific configuration (automatically set to false by parsing the provided 'host') : +> 1. **multipart_upload** - not supported by Google +> 1. **request_checksum_calculation_enabled** - not supported by Google and AliCloud +> 2. **uploader_checksum_calculation_enabled** - not supported by AliCloud **Usage examples:** ```shell @@ -59,16 +63,49 @@ ginkgo --skip-package=integration --cover -v -r ./s3/... ### Integration Tests -To run the integration tests, export the following variables into your environment: - +#### Setup for AWS +1. To run the integration tests, export the following variables into your environment ``` -export access_key_id=YOUR_AWS_ACCESS_KEY +export access_key_id= export focus_regex="GENERAL AWS|AWS V2 REGION|AWS V4 REGION|AWS US-EAST-1" export region_name=us-east-1 -export s3_endpoint_host=https://s3.amazonaws.com -export secret_access_key=YOUR_SECRET_ACCESS_KEY +export s3_endpoint_host=s3.amazonaws.com +export secret_access_key= export stack_name=s3cli-iam export bucket_name=s3cli-pipeline ``` +2. Setup infrastructure with `./.github/scripts/s3/setup-aws-infrastructure.sh` +3. Run the desired tests by executing one or more of the scripts `run-integration-*` in `./.github/scripts/s3` (to run `run-integration-s3-compat` see [Setup for GCP](#setup-for-GCP) or [Setup for AliCloud](#setup-for-alicloud)) +4. Teardown infrastructure with `./.github/scripts/s3/run-integration-*` Run `./.github/scripts/s3/setup-aws-infrastructure.sh` and `./.github/scripts/s3/teardown-infrastructure.sh` before and after the `./.github/scripts/s3/run-integration-*` in repo's root folder. +#### Setup for GCP +1. Create a bucket in GCP +2. Create access keys + 1. Navigate to **IAM & Admin > Service Accounts**. + 2. Select your service account or create a new one if needed. + 3. Ensure your service account has necessary permissions (like `Storage Object Creator`, `Storage Object Viewer`, `Storage Admin`) depending on what access you want. + 4. Go to **Cloud Storage** and select **Settings**. + 5. In the **Interoperability** section, create an HMAC key for your service account. This generates an "access key ID" and a "secret access key". +3. Export the following variables into your environment: +``` +export access_key_id= +export secret_access_key= +export bucket_name= +export s3_endpoint_host=storage.googleapis.com +export s3_endpoint_port=443 +``` +4. Run `run-integration-s3-compat.sh` in `./.github/scripts/s3` + +#### Setup for AliCloud +1. Create bucket in AliCloud +2. Create access keys from `RAM -> User -> Create Accesskey` +3. Export the following variables into your environment: +``` +export access_key_id= +export secret_access_key= +export bucket_name= +export s3_endpoint_host="oss-.aliyuncs.com" +export s3_endpoint_port=443 +``` +4. Run `run-integration-s3-compat.sh` in `./.github/scripts/s3` diff --git a/s3/client/aws_s3_blobstore.go b/s3/client/aws_s3_blobstore.go index b9a71b7..854b778 100644 --- a/s3/client/aws_s3_blobstore.go +++ b/s3/client/aws_s3_blobstore.go @@ -58,6 +58,12 @@ func (b *awsS3Client) Put(src io.ReadSeeker, dest string) error { // disable multipart uploads by way of large PartSize configuration u.PartSize = oneTB } + + if cfg.ShouldDisableUploaderRequestChecksumCalculation() { + // Disable checksum calculation for Alicloud OSS (Object Storage Service) + // Alicloud doesn't support AWS chunked encoding with checksum calculation + u.RequestChecksumCalculation = aws.RequestChecksumCalculationWhenRequired + } }) uploadInput := &s3.PutObjectInput{ Body: src, @@ -112,7 +118,7 @@ func (b *awsS3Client) Delete(dest string) error { } var apiErr smithy.APIError - if errors.As(err, &apiErr) && apiErr.ErrorCode() == "NotFound" { + if errors.As(err, &apiErr) && (apiErr.ErrorCode() == "NotFound" || apiErr.ErrorCode() == "NoSuchKey") { return nil } return err diff --git a/s3/client/middlewares.go b/s3/client/middlewares.go new file mode 100644 index 0000000..0063973 --- /dev/null +++ b/s3/client/middlewares.go @@ -0,0 +1,71 @@ +package client + +import ( + "context" + "fmt" + + v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" + "github.com/aws/smithy-go/middleware" + smithyhttp "github.com/aws/smithy-go/transport/http" +) + +const acceptEncodingHeader = "Accept-Encoding" + +type acceptEncodingKey struct{} + +func getAcceptEncodingKey(ctx context.Context) (v string) { + v, _ = middleware.GetStackValue(ctx, acceptEncodingKey{}).(string) + return v +} + +func setAcceptEncodingKey(ctx context.Context, value string) context.Context { + return middleware.WithStackValue(ctx, acceptEncodingKey{}, value) +} + +var dropAcceptEncodingHeader = middleware.FinalizeMiddlewareFunc("DropAcceptEncodingHeader", + func(ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler) (out middleware.FinalizeOutput, metadata middleware.Metadata, err error) { + req, ok := in.Request.(*smithyhttp.Request) + if !ok { + return out, metadata, &v4.SigningError{Err: fmt.Errorf("unexpected request middleware type %T", in.Request)} + } + + if ae := req.Header.Get(acceptEncodingHeader); len(ae) > 0 { + ctx = setAcceptEncodingKey(ctx, ae) + req.Header.Del(acceptEncodingHeader) + in.Request = req + } + + return next.HandleFinalize(ctx, in) + }, +) + +var setAcceptEncodingHeader = middleware.FinalizeMiddlewareFunc("SetAcceptEncodingHeader", + func(ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler) (out middleware.FinalizeOutput, metadata middleware.Metadata, err error) { + req, ok := in.Request.(*smithyhttp.Request) + if !ok { + return out, metadata, &v4.SigningError{Err: fmt.Errorf("unexpected request middleware type %T", in.Request)} + } + + if ae := getAcceptEncodingKey(ctx); len(ae) > 0 { + req.Header.Set(acceptEncodingHeader, ae) + in.Request = req + } + + return next.HandleFinalize(ctx, in) + }, +) + +func AddFixAcceptEncodingMiddleware(stack *middleware.Stack) error { + if _, ok := stack.Finalize.Get("Signing"); !ok { + return nil + } + + if err := stack.Finalize.Insert(dropAcceptEncodingHeader, "Signing", middleware.Before); err != nil { + return err + } + + if err := stack.Finalize.Insert(setAcceptEncodingHeader, "Signing", middleware.After); err != nil { + return err + } + return nil +} diff --git a/s3/client/sdk.go b/s3/client/sdk.go index c362db5..e61ba69 100644 --- a/s3/client/sdk.go +++ b/s3/client/sdk.go @@ -1,23 +1,37 @@ package client import ( + "context" "net/http" "strings" - "context" - "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/credentials" "github.com/aws/aws-sdk-go-v2/credentials/stscreds" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/sts" + "github.com/aws/smithy-go/middleware" boshhttp "github.com/cloudfoundry/bosh-utils/httpclient" s3cli_config "github.com/cloudfoundry/storage-cli/s3/config" ) func NewAwsS3Client(c *s3cli_config.S3Cli) (*s3.Client, error) { + var apiOptions []func(stack *middleware.Stack) error + if c.IsGoogle() { + // Setup middleware fixing request to Google - they expect the 'accept-encoding' header + // to not be included in the signature of the request. Not needed for "sign" commands + // since they only generate pre-signed URLs without making actual HTTP requests. + apiOptions = append(apiOptions, AddFixAcceptEncodingMiddleware) + } + return NewAwsS3ClientWithApiOptions(c, apiOptions) +} + +func NewAwsS3ClientWithApiOptions( + c *s3cli_config.S3Cli, + apiOptions []func(stack *middleware.Stack) error, +) (*s3.Client, error) { var httpClient *http.Client if c.SSLVerifyPeer { @@ -30,11 +44,7 @@ func NewAwsS3Client(c *s3cli_config.S3Cli) (*s3.Client, error) { config.WithHTTPClient(httpClient), } - if c.UseRegion() { - options = append(options, config.WithRegion(c.Region)) - } else { - options = append(options, config.WithRegion(s3cli_config.EmptyRegion)) - } + options = append(options, config.WithRegion(c.Region)) if c.CredentialsSource == s3cli_config.StaticCredentialsSource { options = append(options, config.WithCredentialsProvider( @@ -57,10 +67,13 @@ func NewAwsS3Client(c *s3cli_config.S3Cli) (*s3.Client, error) { awsConfig.Credentials = aws.NewCredentialsCache(provider) } + if c.ShouldDisableRequestChecksumCalculation() { + awsConfig.RequestChecksumCalculation = aws.RequestChecksumCalculationWhenRequired + } + s3Client := s3.NewFromConfig(awsConfig, func(o *s3.Options) { o.UsePathStyle = !c.HostStyle - if c.S3Endpoint() != "" { - endpoint := c.S3Endpoint() + if endpoint := c.S3Endpoint(); endpoint != "" { // AWS SDK v2 requires full URI with protocol if !strings.HasPrefix(endpoint, "http://") && !strings.HasPrefix(endpoint, "https://") { if c.UseSSL { @@ -71,6 +84,8 @@ func NewAwsS3Client(c *s3cli_config.S3Cli) (*s3.Client, error) { } o.BaseEndpoint = aws.String(endpoint) } + // Apply custom middlewares if provided + o.APIOptions = append(o.APIOptions, apiOptions...) }) return s3Client, nil diff --git a/s3/config/config.go b/s3/config/config.go index 6d6292e..143cc1b 100644 --- a/s3/config/config.go +++ b/s3/config/config.go @@ -10,34 +10,28 @@ import ( // The S3Cli represents configuration for the s3cli type S3Cli struct { - AccessKeyID string `json:"access_key_id"` - SecretAccessKey string `json:"secret_access_key"` - BucketName string `json:"bucket_name"` - FolderName string `json:"folder_name"` - CredentialsSource string `json:"credentials_source"` - Host string `json:"host"` - Port int `json:"port"` // 0 means no custom port - Region string `json:"region"` - SSLVerifyPeer bool `json:"ssl_verify_peer"` - UseSSL bool `json:"use_ssl"` - SignatureVersion int `json:"signature_version,string"` - ServerSideEncryption string `json:"server_side_encryption"` - SSEKMSKeyID string `json:"sse_kms_key_id"` - AssumeRoleArn string `json:"assume_role_arn"` - MultipartUpload bool `json:"multipart_upload"` - UseV2SigningMethod bool - HostStyle bool `json:"host_style"` - SwiftAuthAccount string `json:"swift_auth_account"` - SwiftTempURLKey string `json:"swift_temp_url_key"` + AccessKeyID string `json:"access_key_id"` + SecretAccessKey string `json:"secret_access_key"` + BucketName string `json:"bucket_name"` + FolderName string `json:"folder_name"` + CredentialsSource string `json:"credentials_source"` + Host string `json:"host"` + Port int `json:"port"` // 0 means no custom port + Region string `json:"region"` + SSLVerifyPeer bool `json:"ssl_verify_peer"` + UseSSL bool `json:"use_ssl"` + ServerSideEncryption string `json:"server_side_encryption"` + SSEKMSKeyID string `json:"sse_kms_key_id"` + AssumeRoleArn string `json:"assume_role_arn"` + MultipartUpload bool `json:"multipart_upload"` + HostStyle bool `json:"host_style"` + SwiftAuthAccount string `json:"swift_auth_account"` + SwiftTempURLKey string `json:"swift_temp_url_key"` + requestChecksumCalculationEnabled bool + uploaderRequestChecksumCalculationEnabled bool } -// EmptyRegion is required to allow us to use the AWS SDK against S3 compatible blobstores which do not have -// the concept of a region -const EmptyRegion = " " - -const ( - defaultRegion = "us-east-1" //nolint:unused -) +const defaultAWSRegion = "us-east-1" //nolint:unused // StaticCredentialsSource specifies that credentials will be supplied using access_key_id and secret_access_key const StaticCredentialsSource = "static" @@ -73,9 +67,11 @@ func NewFromReader(reader io.Reader) (S3Cli, error) { } c := S3Cli{ - SSLVerifyPeer: true, - UseSSL: true, - MultipartUpload: true, + SSLVerifyPeer: true, + UseSSL: true, + MultipartUpload: true, + requestChecksumCalculationEnabled: true, + uploaderRequestChecksumCalculationEnabled: true, } err = json.Unmarshal(bytes, &c) @@ -143,53 +139,33 @@ func (c *S3Cli) configureAWS() { c.MultipartUpload = true if c.Region == "" { - c.Region = AWSHostToRegion(c.Host) - } - - switch c.SignatureVersion { - case 2: - c.UseV2SigningMethod = true - case 4: - c.UseV2SigningMethod = false - default: - c.UseV2SigningMethod = false + if region := AWSHostToRegion(c.Host); region != "" { + c.Region = region + } else { + c.Region = defaultAWSRegion + } } } func (c *S3Cli) configureAlicloud() { c.MultipartUpload = true - c.configureDefaultSigningMethod() c.HostStyle = true c.Host = strings.Split(c.Host, ":")[0] if c.Region == "" { c.Region = AlicloudHostToRegion(c.Host) } + c.requestChecksumCalculationEnabled = false + c.uploaderRequestChecksumCalculationEnabled = false } func (c *S3Cli) configureGoogle() { c.MultipartUpload = false - c.configureDefaultSigningMethod() + c.requestChecksumCalculationEnabled = false } func (c *S3Cli) configureDefault() { - c.configureDefaultSigningMethod() -} - -func (c *S3Cli) configureDefaultSigningMethod() { - switch c.SignatureVersion { - case 2: - c.UseV2SigningMethod = true - case 4: - c.UseV2SigningMethod = false - default: - c.UseV2SigningMethod = true - } -} - -// UseRegion signals to users of the S3Cli whether to use Region information -func (c *S3Cli) UseRegion() bool { - return c.Region != "" + // No specific configuration needed for default/unknown providers } // S3Endpoint returns the S3 URI to use if custom host information has been provided @@ -208,3 +184,15 @@ func (c *S3Cli) S3Endpoint() string { } return c.Host } + +func (c *S3Cli) IsGoogle() bool { + return Provider(c.Host) == "google" +} + +func (c *S3Cli) ShouldDisableRequestChecksumCalculation() bool { + return !c.requestChecksumCalculationEnabled +} + +func (c *S3Cli) ShouldDisableUploaderRequestChecksumCalculation() bool { + return !c.uploaderRequestChecksumCalculationEnabled +} diff --git a/s3/config/config_test.go b/s3/config/config_test.go index b3ecdf9..8431a63 100644 --- a/s3/config/config_test.go +++ b/s3/config/config_test.go @@ -11,12 +11,6 @@ import ( ) var _ = Describe("BlobstoreClient configuration", func() { - Describe("empty region configuration", func() { - It("allows for the S3 SDK to be configured with empty region information", func() { - Expect(config.EmptyRegion).To(Equal(" ")) - }) - }) - DescribeTable("Provider", func(host, provider string) { Expect(config.Provider(host)).To(Equal(provider)) @@ -41,25 +35,35 @@ var _ = Describe("BlobstoreClient configuration", func() { dummyJSONReader := bytes.NewReader(dummyJSONBytes) c, err := config.NewFromReader(dummyJSONReader) Expect(err).ToNot(HaveOccurred()) - Expect(c.UseRegion()).To(BeTrue(), "Expected UseRegion to be true") Expect(c.Host).To(Equal("s3.amazonaws.com")) Expect(c.Region).To(Equal("us-east-1")) }) }) - Context("when non-AWS endpoint has been set but not region", func() { - dummyJSONBytes := []byte(`{"access_key_id": "id", "secret_access_key": "key", "bucket_name": "some-bucket", "host": "some-host"}`) + Context("when Google endpoint has been set but not region", func() { + dummyJSONBytes := []byte(`{"access_key_id": "id", "secret_access_key": "key", "bucket_name": "some-bucket", "host": "storage.googleapis.com"}`) dummyJSONReader := bytes.NewReader(dummyJSONBytes) - It("reports that region should not be used for SDK configuration", func() { + It("stubs the region used for SDK configuration", func() { c, err := config.NewFromReader(dummyJSONReader) Expect(err).ToNot(HaveOccurred()) - Expect(c.UseRegion()).To(BeFalse()) - Expect(c.Host).To(Equal("some-host")) + Expect(c.Host).To(Equal("storage.googleapis.com")) Expect(c.Region).To(Equal("")) }) }) + Context("when Ali endpoint with region has been set but without explicit region", func() { + dummyJSONBytes := []byte(`{"access_key_id": "id", "secret_access_key": "key", "bucket_name": "some-bucket", "host": "oss-some-region-internal.aliyuncs.com"}`) + dummyJSONReader := bytes.NewReader(dummyJSONBytes) + + It("parses region from host for SDK configuration", func() { + c, err := config.NewFromReader(dummyJSONReader) + Expect(err).ToNot(HaveOccurred()) + Expect(c.Host).To(Equal("oss-some-region-internal.aliyuncs.com")) + Expect(c.Region).To(Equal("some-region")) + }) + }) + Context("when region has been set but not host", func() { dummyJSONBytes := []byte(`{"access_key_id": "id", "secret_access_key": "key", "bucket_name": "some-bucket", "region": "some-region"}`) dummyJSONReader := bytes.NewReader(dummyJSONBytes) @@ -67,7 +71,6 @@ var _ = Describe("BlobstoreClient configuration", func() { It("reports that region should be used for SDK configuration", func() { c, err := config.NewFromReader(dummyJSONReader) Expect(err).ToNot(HaveOccurred()) - Expect(c.UseRegion()).To(BeTrue()) Expect(c.Host).To(Equal("")) Expect(c.Region).To(Equal("some-region")) }) @@ -80,7 +83,6 @@ var _ = Describe("BlobstoreClient configuration", func() { It("sets region and endpoint to user-specified values", func() { c, err := config.NewFromReader(dummyJSONReader) Expect(err).ToNot(HaveOccurred()) - Expect(c.UseRegion()).To(BeTrue()) Expect(c.Host).To(Equal("some-host")) Expect(c.Region).To(Equal("some-region")) }) @@ -93,7 +95,6 @@ var _ = Describe("BlobstoreClient configuration", func() { It("does not override the user-specified region based on the hostname", func() { c, err := config.NewFromReader(dummyJSONReader) Expect(err).ToNot(HaveOccurred()) - Expect(c.UseRegion()).To(BeTrue()) Expect(c.Host).To(Equal("s3.amazonaws.com")) Expect(c.Region).To(Equal("us-west-1")) }) @@ -191,94 +192,6 @@ var _ = Describe("BlobstoreClient configuration", func() { }) }) - Describe("configuring signing method", func() { - - It("uses v4 signing when there is no host defined", func() { - configBytes := []byte(`{ - "access_key_id": "id", - "secret_access_key": "key", - "bucket_name": "some-bucket" - }`) - - configReader := bytes.NewReader(configBytes) - s3CliConfig, err := config.NewFromReader(configReader) - Expect(err).ToNot(HaveOccurred()) - Expect(s3CliConfig.UseV2SigningMethod).To(BeFalse()) - }) - - It("uses v4 signing when the hostname maps to a known Amazon region", func() { - configBytes := []byte(`{ - "access_key_id": "id", - "secret_access_key": "key", - "bucket_name": "some-bucket", - "host": "s3-external-1.amazonaws.com" - }`) - - configReader := bytes.NewReader(configBytes) - s3CliConfig, err := config.NewFromReader(configReader) - Expect(err).ToNot(HaveOccurred()) - Expect(s3CliConfig.UseV2SigningMethod).To(BeFalse()) - }) - - It("uses v4 signing when the hostname maps to a known Amazon china region", func() { - configBytes := []byte(`{ - "access_key_id": "id", - "secret_access_key": "key", - "bucket_name": "some-bucket", - "host": "s3.cn-north-1.amazonaws.com.cn" - }`) - - configReader := bytes.NewReader(configBytes) - s3CliConfig, err := config.NewFromReader(configReader) - Expect(err).ToNot(HaveOccurred()) - Expect(s3CliConfig.UseV2SigningMethod).To(BeFalse()) - }) - - It("uses v4 signing when both the hostname and the region map to a known Amazon region", func() { - configBytes := []byte(`{ - "access_key_id": "id", - "secret_access_key": "key", - "bucket_name": "some-bucket", - "host": "s3-external-1.amazonaws.com", - "region": "eu-central-1" - }`) - - configReader := bytes.NewReader(configBytes) - s3CliConfig, err := config.NewFromReader(configReader) - Expect(err).ToNot(HaveOccurred()) - Expect(s3CliConfig.UseV2SigningMethod).To(BeFalse()) - }) - - It("uses v2 signing when the hostname is a non-Amazon endpoint", func() { - configBytes := []byte(`{ - "access_key_id": "id", - "secret_access_key": "key", - "bucket_name": "some-bucket", - "host": "s3-compatible.com" - }`) - - configReader := bytes.NewReader(configBytes) - s3CliConfig, err := config.NewFromReader(configReader) - Expect(err).ToNot(HaveOccurred()) - Expect(s3CliConfig.UseV2SigningMethod).To(BeTrue()) - }) - - It("uses override signing value when signing_version is overriden", func() { - configBytes := []byte(`{ - "access_key_id": "id", - "secret_access_key": "key", - "bucket_name": "some-bucket", - "host": "s3-external-1.amazonaws.com", - "signature_version": "2" - }`) - - configReader := bytes.NewReader(configBytes) - s3CliConfig, err := config.NewFromReader(configReader) - Expect(err).ToNot(HaveOccurred()) - Expect(s3CliConfig.UseV2SigningMethod).To(BeTrue()) - }) - }) - Describe("configing force path style", func() { It("when Alibaba Cloud provider", func() { configBytes := []byte(`{ diff --git a/s3/config/endpoints.go b/s3/config/endpoints.go index 13a996a..2d5aad3 100644 --- a/s3/config/endpoints.go +++ b/s3/config/endpoints.go @@ -7,15 +7,15 @@ import ( var ( providerRegex = map[string]*regexp.Regexp{ "aws": regexp.MustCompile(`(^$|s3[-.]?(.*)\.amazonaws\.com(\.cn)?$)`), - "alicloud": regexp.MustCompile(`^oss-([a-z]+-[a-z]+(-[1-9])?)(-internal)?.aliyuncs.com$`), - "google": regexp.MustCompile(`^storage.googleapis.com$`), + "alicloud": regexp.MustCompile(`^oss-([a-z]+-[a-z]+(-[1-9])?)(-internal)?\.aliyuncs\.com$`), + "google": regexp.MustCompile(`^storage\.googleapis\.com$`), } ) func AWSHostToRegion(host string) string { regexMatches := providerRegex["aws"].FindStringSubmatch(host) - region := "us-east-1" + region := "" if len(regexMatches) == 4 && regexMatches[2] != "" && regexMatches[2] != "external-1" { region = regexMatches[2] @@ -26,7 +26,6 @@ func AWSHostToRegion(host string) string { func AlicloudHostToRegion(host string) string { regexMatches := providerRegex["alicloud"].FindStringSubmatch(host) - if len(regexMatches) == 4 { return regexMatches[1] } diff --git a/s3/config/endpoints_test.go b/s3/config/endpoints_test.go index bb07ca8..c52401e 100644 --- a/s3/config/endpoints_test.go +++ b/s3/config/endpoints_test.go @@ -12,9 +12,9 @@ var _ = Describe("Endpoints", func() { func(host, region string) { Expect(config.AWSHostToRegion(host)).To(Equal(region)) }, - Entry("us-east-1", "this-should-default", "us-east-1"), - Entry("us-east-1", "s3.amazonaws.com", "us-east-1"), - Entry("us-east-1", "s3-external-1.amazonaws.com", "us-east-1"), + Entry("no region in host", "this-should-default", ""), + Entry("no region in host", "s3.amazonaws.com", ""), + Entry("no region in host", "s3-external-1.amazonaws.com", ""), Entry("us-east-2", "s3.us-east-2.amazonaws.com", "us-east-2"), Entry("us-east-2", "s3-us-east-2.amazonaws.com", "us-east-2"), Entry("cn-north-1", "s3.cn-north-1.amazonaws.com.cn", "cn-north-1"), diff --git a/s3/integration/assertions.go b/s3/integration/assertions.go index fe4eece..9513352 100644 --- a/s3/integration/assertions.go +++ b/s3/integration/assertions.go @@ -3,8 +3,11 @@ package integration import ( "context" "fmt" + "io" "log" + "net/http" "os" + "strings" "time" "github.com/cloudfoundry/storage-cli/s3/client" @@ -17,6 +20,32 @@ import ( . "github.com/onsi/gomega" //nolint:staticcheck ) +var ( + // expectedPutUploadCalls represents the expected API calls for put requests + expectedPutUploadCalls = []string{"PutObject"} +) + +// isMultipartUploadPattern checks if calls follow the multipart upload pattern: +// starts with CreateMultipart, has one or more UploadPart calls, ends with CompleteMultipart +func isMultipartUploadPattern(calls []string) bool { + if len(calls) < 3 { + return false + } + if calls[0] != "CreateMultipart" { + return false + } + if calls[len(calls)-1] != "CompleteMultipart" { + return false + } + // Check all middle elements are UploadPart + for _, call := range calls[1 : len(calls)-1] { + if call != "UploadPart" { + return false + } + } + return true +} + // AssertLifecycleWorks tests the main blobstore object lifecycle from creation to deletion func AssertLifecycleWorks(s3CLIPath string, cfg *config.S3Cli) { storageType := "s3" @@ -73,7 +102,7 @@ func AssertLifecycleWorks(s3CLIPath string, cfg *config.S3Cli) { Expect(s3CLISession.Err).Should(gbytes.Say(`"error":"object does not exist"`)) } -func AssertOnPutFailures(s3CLIPath string, cfg *config.S3Cli, content, errorMessage string) { +func AssertOnPutFailures(cfg *config.S3Cli, content, errorMessage string) { s3Filename := GenerateRandomString() sourceFile := MakeContentFile(content) @@ -136,6 +165,10 @@ func AssertPutOptionsApplied(s3CLIPath string, cfg *config.S3Cli) { } else { Expect(string(resp.ServerSideEncryption)).To(Equal(cfg.ServerSideEncryption)) } + + // Clean up the uploaded file + _, err = RunS3CLI(s3CLIPath, configPath, storageType, "delete", s3Filename) + Expect(err).ToNot(HaveOccurred()) } // AssertGetNonexistentFails asserts that `s3cli get` on a non-existent object will fail @@ -166,6 +199,7 @@ func AssertOnMultipartUploads(s3CLIPath string, cfg *config.S3Cli, content strin s3Filename := GenerateRandomString() sourceFile := MakeContentFile(content) + storageType := "s3" configPath := MakeConfigFile(cfg) defer os.Remove(configPath) //nolint:errcheck @@ -187,18 +221,25 @@ func AssertOnMultipartUploads(s3CLIPath string, cfg *config.S3Cli, content strin err = blobstoreClient.Put(sourceFile, s3Filename) Expect(err).ToNot(HaveOccurred()) - switch cfg.Host { - case "storage.googleapis.com": - Expect(calls).To(Equal([]string{"PutObject"})) + switch config.Provider(cfg.Host) { + // Google doesn't support multipart uploads as we use a normal put request instead when targeted host is Google. + case "google": + Expect(calls).To(Equal(expectedPutUploadCalls)) default: - Expect(calls).To(Equal([]string{"CreateMultipart", "UploadPart", "UploadPart", "CompleteMultipart"})) + Expect(isMultipartUploadPattern(calls)).To(BeTrue(), "Expected multipart upload pattern (CreateMultipart -> UploadPart(s) -> CompleteMultipart), got: %v", calls) } + + // Clean up the uploaded file + _, err = RunS3CLI(s3CLIPath, configPath, storageType, "delete", s3Filename) + Expect(err).ToNot(HaveOccurred()) } // AssertOnSignedURLs asserts on using signed URLs for upload and download func AssertOnSignedURLs(s3CLIPath string, cfg *config.S3Cli) { s3Filename := GenerateRandomString() + expectedContent := GenerateRandomString() + storageType := "s3" configPath := MakeConfigFile(cfg) defer os.Remove(configPath) //nolint:errcheck @@ -208,24 +249,58 @@ func AssertOnSignedURLs(s3CLIPath string, cfg *config.S3Cli) { s3Config, err := config.NewFromReader(configFile) Expect(err).ToNot(HaveOccurred()) - // Create S3 client with tracing middleware (though signing operations don't need tracing for this test) - calls := []string{} - s3Client, err := CreateTracingS3Client(&s3Config, &calls) + s3Client, err := client.NewAwsS3Client(&s3Config) if err != nil { log.Fatalln(err) } blobstoreClient := client.New(s3Client, &s3Config) + // First upload a test file using regular put operation + contentFile := MakeContentFile(expectedContent) + defer os.Remove(contentFile) //nolint:errcheck + + s3CLISession, err := RunS3CLI(s3CLIPath, configPath, storageType, "put", contentFile, s3Filename) + Expect(err).ToNot(HaveOccurred()) + Expect(s3CLISession.ExitCode()).To(BeZero()) + regex := `(?m)((([A-Za-z]{3,9}:(?:\/\/?)?)(?:[-;:&=\+\$,\w]+@)?[A-Za-z0-9.-]+(:[0-9]+)?|(?:www.|[-;:&=\+\$,\w]+@)[A-Za-z0-9.-]+)((?:\/[\+~%\/.\w-_]*)?\??(?:[-\+=&;%@.\w_]*)#?(?:[\w]*))?)` - // get - url, err := blobstoreClient.Sign(s3Filename, "get", 1*time.Minute) + // Test GET signed URL + getURL, err := blobstoreClient.Sign(s3Filename, "get", 1*time.Minute) + Expect(err).ToNot(HaveOccurred()) + Expect(getURL).To(MatchRegexp(regex)) + + // Actually try to download from the GET signed URL + httpClient := &http.Client{Timeout: 30 * time.Second} + resp, err := httpClient.Get(getURL) + Expect(err).ToNot(HaveOccurred()) + defer resp.Body.Close() //nolint:errcheck + + body, err := io.ReadAll(resp.Body) + Expect(err).ToNot(HaveOccurred()) + Expect(string(body)).To(Equal(expectedContent)) + + // Test PUT signed URL + putURL, err := blobstoreClient.Sign(s3Filename+"_put_test", "put", 1*time.Minute) + Expect(err).ToNot(HaveOccurred()) + Expect(putURL).To(MatchRegexp(regex)) + + // Actually try to upload to the PUT signed URL + testUploadContent := "Test upload content via signed URL" + putReq, err := http.NewRequest("PUT", putURL, strings.NewReader(testUploadContent)) + Expect(err).ToNot(HaveOccurred()) + + putReq.Header.Set("Content-Type", "text/plain") + putResp, err := httpClient.Do(putReq) + Expect(err).ToNot(HaveOccurred()) + defer putResp.Body.Close() //nolint:errcheck + Expect(putResp.StatusCode).To(Equal(200)) + + // Clean up the test files + _, err = RunS3CLI(s3CLIPath, configPath, storageType, "delete", s3Filename) Expect(err).ToNot(HaveOccurred()) - Expect(url).To(MatchRegexp(regex)) - // put - url, err = blobstoreClient.Sign(s3Filename, "put", 1*time.Minute) + _, err = RunS3CLI(s3CLIPath, configPath, storageType, "delete", s3Filename+"_put_test") Expect(err).ToNot(HaveOccurred()) - Expect(url).To(MatchRegexp(regex)) } diff --git a/s3/integration/aws_iam_role_test.go b/s3/integration/aws_iam_role_test.go index f73a359..e11251c 100644 --- a/s3/integration/aws_iam_role_test.go +++ b/s3/integration/aws_iam_role_test.go @@ -28,13 +28,11 @@ var _ = Describe("Testing inside an AWS compute resource with an IAM role", func BucketName: bucketName, }), Entry("with region and without host, signature version 4", &config.S3Cli{ - SignatureVersion: 4, CredentialsSource: "env_or_profile", BucketName: bucketName, Region: region, }), Entry("with maximal config, signature version 4", &config.S3Cli{ - SignatureVersion: 4, CredentialsSource: "env_or_profile", BucketName: bucketName, Host: s3Host, diff --git a/s3/integration/aws_isolated_region_test.go b/s3/integration/aws_isolated_region_test.go index c186a25..8c7d027 100644 --- a/s3/integration/aws_isolated_region_test.go +++ b/s3/integration/aws_isolated_region_test.go @@ -27,7 +27,6 @@ var _ = Describe("Testing in any AWS region isolated from the US standard region Expect(region).ToNot(BeEmpty(), "REGION must be set") cfg := &config.S3Cli{ - SignatureVersion: 4, CredentialsSource: "static", AccessKeyID: accessKeyID, SecretAccessKey: secretAccessKey, diff --git a/s3/integration/aws_public_read_only_test.go b/s3/integration/aws_public_read_only_test.go index 2f100c6..785d4ed 100644 --- a/s3/integration/aws_public_read_only_test.go +++ b/s3/integration/aws_public_read_only_test.go @@ -71,6 +71,13 @@ var _ = Describe("Testing gets against a public AWS S3 bucket", func() { s3CLISession, err = integration.RunS3CLI(s3CLIPath, configPath, storageType, "exists", s3Filename) Expect(err).ToNot(HaveOccurred()) Expect(s3CLISession.ExitCode()).To(BeZero()) + + // Clean up the uploaded file + _, err = s3Client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ + Bucket: &bucketName, + Key: &s3Filename, + }) + Expect(err).ToNot(HaveOccurred()) }) }) }) diff --git a/s3/integration/aws_us_east_test.go b/s3/integration/aws_us_east_test.go index 0824e58..db81450 100644 --- a/s3/integration/aws_us_east_test.go +++ b/s3/integration/aws_us_east_test.go @@ -28,12 +28,6 @@ var _ = Describe("Testing only in us-east-1", func() { SecretAccessKey: secretAccessKey, BucketName: bucketName, }), - Entry("with signature version 2", &config.S3Cli{ - SignatureVersion: 2, - AccessKeyID: accessKeyID, - SecretAccessKey: secretAccessKey, - BucketName: bucketName, - }), Entry("with alternate host", &config.S3Cli{ AccessKeyID: accessKeyID, SecretAccessKey: secretAccessKey, diff --git a/s3/integration/aws_v2_region_test.go b/s3/integration/aws_v2_region_test.go deleted file mode 100644 index a3ae2ae..0000000 --- a/s3/integration/aws_v2_region_test.go +++ /dev/null @@ -1,60 +0,0 @@ -package integration_test - -import ( - "os" - - "github.com/cloudfoundry/storage-cli/s3/config" - "github.com/cloudfoundry/storage-cli/s3/integration" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("Testing in any AWS region that supports v2 signature version", func() { - Context("with AWS V2 REGION (static creds) configurations", func() { - accessKeyID := os.Getenv("ACCESS_KEY_ID") - secretAccessKey := os.Getenv("SECRET_ACCESS_KEY") - bucketName := os.Getenv("BUCKET_NAME") - region := os.Getenv("REGION") - s3Host := os.Getenv("S3_HOST") - - BeforeEach(func() { - Expect(accessKeyID).ToNot(BeEmpty(), "ACCESS_KEY_ID must be set") - Expect(secretAccessKey).ToNot(BeEmpty(), "SECRET_ACCESS_KEY must be set") - Expect(bucketName).ToNot(BeEmpty(), "BUCKET_NAME must be set") - Expect(region).ToNot(BeEmpty(), "REGION must be set") - Expect(s3Host).ToNot(BeEmpty(), "S3_HOST must be set") - }) - - configurations := []TableEntry{ - Entry("with host and without region, signature version 2", &config.S3Cli{ - SignatureVersion: 2, - CredentialsSource: "static", - AccessKeyID: accessKeyID, - SecretAccessKey: secretAccessKey, - BucketName: bucketName, - Host: s3Host, - }), - Entry("with region and without host, signature version 2", &config.S3Cli{ - SignatureVersion: 2, - CredentialsSource: "static", - AccessKeyID: accessKeyID, - SecretAccessKey: secretAccessKey, - BucketName: bucketName, - Region: region, - }), - } - DescribeTable("Blobstore lifecycle works", - func(cfg *config.S3Cli) { integration.AssertLifecycleWorks(s3CLIPath, cfg) }, - configurations, - ) - DescribeTable("Invoking `s3cli get` on a non-existent-key fails", - func(cfg *config.S3Cli) { integration.AssertGetNonexistentFails(s3CLIPath, cfg) }, - configurations, - ) - DescribeTable("Invoking `s3cli delete` on a non-existent-key does not fail", - func(cfg *config.S3Cli) { integration.AssertDeleteNonexistentWorks(s3CLIPath, cfg) }, - configurations, - ) - }) -}) diff --git a/s3/integration/aws_v4_only_region_test.go b/s3/integration/aws_v4_only_region_test.go deleted file mode 100644 index 539dab7..0000000 --- a/s3/integration/aws_v4_only_region_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package integration_test - -import ( - "os" - - "github.com/cloudfoundry/storage-cli/s3/config" - "github.com/cloudfoundry/storage-cli/s3/integration" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("Testing in any AWS region that only supports v4 signature version", func() { - Context("with AWS V4 ONLY REGION (static creds) configurations", func() { - It("fails with a config that specifies signature version 2", func() { - storageType := "s3" - accessKeyID := os.Getenv("ACCESS_KEY_ID") - Expect(accessKeyID).ToNot(BeEmpty(), "ACCESS_KEY_ID must be set") - - secretAccessKey := os.Getenv("SECRET_ACCESS_KEY") - Expect(secretAccessKey).ToNot(BeEmpty(), "SECRET_ACCESS_KEY must be set") - - bucketName := os.Getenv("BUCKET_NAME") - Expect(bucketName).ToNot(BeEmpty(), "BUCKET_NAME must be set") - - region := os.Getenv("REGION") - Expect(region).ToNot(BeEmpty(), "REGION must be set") - - cfg := &config.S3Cli{ - SignatureVersion: 2, - AccessKeyID: accessKeyID, - SecretAccessKey: secretAccessKey, - BucketName: bucketName, - Region: region, - } - s3Filename := integration.GenerateRandomString() - - configPath := integration.MakeConfigFile(cfg) - defer os.Remove(configPath) //nolint:errcheck - - contentFile := integration.MakeContentFile("test") - defer os.Remove(contentFile) //nolint:errcheck - - s3CLISession, err := integration.RunS3CLI(s3CLIPath, configPath, storageType, "put", contentFile, s3Filename) - Expect(err).ToNot(HaveOccurred()) - Expect(s3CLISession.ExitCode()).ToNot(BeZero()) - - s3CLISession, err = integration.RunS3CLI(s3CLIPath, configPath, storageType, "delete", s3Filename) - Expect(err).ToNot(HaveOccurred()) - Expect(s3CLISession.ExitCode()).ToNot(BeZero()) - }) - }) -}) diff --git a/s3/integration/aws_v4_region_test.go b/s3/integration/aws_v4_region_test.go deleted file mode 100644 index b7c3467..0000000 --- a/s3/integration/aws_v4_region_test.go +++ /dev/null @@ -1,72 +0,0 @@ -package integration_test - -import ( - "os" - - "github.com/cloudfoundry/storage-cli/s3/config" - "github.com/cloudfoundry/storage-cli/s3/integration" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("Testing in any AWS region that supports v4 signature version", func() { - Context("with AWS V4 REGION (static creds) configurations", func() { - accessKeyID := os.Getenv("ACCESS_KEY_ID") - secretAccessKey := os.Getenv("SECRET_ACCESS_KEY") - bucketName := os.Getenv("BUCKET_NAME") - region := os.Getenv("REGION") - s3Host := os.Getenv("S3_HOST") - - BeforeEach(func() { - Expect(accessKeyID).ToNot(BeEmpty(), "ACCESS_KEY_ID must be set") - Expect(secretAccessKey).ToNot(BeEmpty(), "SECRET_ACCESS_KEY must be set") - Expect(bucketName).ToNot(BeEmpty(), "BUCKET_NAME must be set") - Expect(region).ToNot(BeEmpty(), "REGION must be set") - Expect(s3Host).ToNot(BeEmpty(), "S3_HOST must be set") - }) - - configurations := []TableEntry{ - Entry("with region and without host", &config.S3Cli{ - SignatureVersion: 4, - CredentialsSource: "static", - AccessKeyID: accessKeyID, - SecretAccessKey: secretAccessKey, - BucketName: bucketName, - Region: region, - }), - Entry("with host and without region", &config.S3Cli{ - SignatureVersion: 4, - CredentialsSource: "static", - AccessKeyID: accessKeyID, - SecretAccessKey: secretAccessKey, - BucketName: bucketName, - Host: s3Host, - }), - Entry("with maximal config", &config.S3Cli{ - SignatureVersion: 4, - CredentialsSource: "static", - AccessKeyID: accessKeyID, - SecretAccessKey: secretAccessKey, - BucketName: bucketName, - Host: s3Host, - Port: 443, - UseSSL: true, - SSLVerifyPeer: true, - Region: region, - }), - } - DescribeTable("Blobstore lifecycle works", - func(cfg *config.S3Cli) { integration.AssertLifecycleWorks(s3CLIPath, cfg) }, - configurations, - ) - DescribeTable("Invoking `s3cli get` on a non-existent-key fails", - func(cfg *config.S3Cli) { integration.AssertGetNonexistentFails(s3CLIPath, cfg) }, - configurations, - ) - DescribeTable("Invoking `s3cli delete` on a non-existent-key does not fail", - func(cfg *config.S3Cli) { integration.AssertDeleteNonexistentWorks(s3CLIPath, cfg) }, - configurations, - ) - }) -}) diff --git a/s3/integration/general_aws_test.go b/s3/integration/general_aws_test.go index cf94fa1..f114375 100644 --- a/s3/integration/general_aws_test.go +++ b/s3/integration/general_aws_test.go @@ -100,7 +100,7 @@ var _ = Describe("General testing for all AWS regions", func() { Host: "http://localhost", } msg := "upload failure" - integration.AssertOnPutFailures(s3CLIPath, cfg, largeContent, msg) + integration.AssertOnPutFailures(cfg, largeContent, msg) }) }) @@ -111,9 +111,10 @@ var _ = Describe("General testing for all AWS regions", func() { SecretAccessKey: secretAccessKey, BucketName: bucketName, Region: region, + MultipartUpload: true, } msg := "upload retry limit exceeded" - integration.AssertOnPutFailures(s3CLIPath, cfg, largeContent, msg) + integration.AssertOnPutFailures(cfg, largeContent, msg) }) }) }) diff --git a/s3/integration/integration_suite_test.go b/s3/integration/integration_suite_test.go index 0f5aa75..8824a44 100644 --- a/s3/integration/integration_suite_test.go +++ b/s3/integration/integration_suite_test.go @@ -1,6 +1,8 @@ package integration_test import ( + "io" + "log" "os" "testing" @@ -20,6 +22,9 @@ var s3CLIPath string var largeContent string var _ = BeforeSuite(func() { + // Suppress logs during integration tests + log.SetOutput(io.Discard) + // Running the IAM tests within an AWS Lambda environment // require a pre-compiled binary s3CLIPath = os.Getenv("S3_CLI_PATH") diff --git a/s3/integration/middlewares.go b/s3/integration/middlewares.go index 5066ea2..c5e600a 100644 --- a/s3/integration/middlewares.go +++ b/s3/integration/middlewares.go @@ -2,23 +2,15 @@ package integration import ( "context" - "net/http" "sync/atomic" - "github.com/aws/aws-sdk-go-v2/aws" - awsconfig "github.com/aws/aws-sdk-go-v2/config" - "github.com/aws/aws-sdk-go-v2/credentials" - "github.com/aws/aws-sdk-go-v2/credentials/stscreds" "github.com/aws/aws-sdk-go-v2/service/s3" - "github.com/aws/aws-sdk-go-v2/service/sts" "github.com/aws/smithy-go/middleware" smithyhttp "github.com/aws/smithy-go/transport/http" - boshhttp "github.com/cloudfoundry/bosh-utils/httpclient" - "github.com/cloudfoundry/storage-cli/s3/config" ) // CreateUploadPartTracker creates an Initialize middleware that tracks upload parts -func CreateUploadPartTracker() middleware.InitializeMiddleware { +func createUploadPartTracker() middleware.InitializeMiddleware { var partCounter int64 return middleware.InitializeMiddlewareFunc("UploadPartTracker", func( @@ -49,7 +41,7 @@ func CreateUploadPartTracker() middleware.InitializeMiddleware { type failureInjectionKey struct{} // CreateSHACorruptionMiddleware creates a Finalize middleware that corrupts headers -func CreateSHACorruptionMiddleware() middleware.FinalizeMiddleware { +func createSHACorruptionMiddleware() middleware.FinalizeMiddleware { return middleware.FinalizeMiddlewareFunc("SHACorruptionMiddleware", func( ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler, ) (middleware.FinalizeOutput, middleware.Metadata, error) { @@ -64,82 +56,13 @@ func CreateSHACorruptionMiddleware() middleware.FinalizeMiddleware { }) } -// CreateS3ClientWithFailureInjection creates an S3 client with failure injection middleware -func CreateS3ClientWithFailureInjection(s3Config *config.S3Cli) (*s3.Client, error) { - // Create HTTP client based on SSL verification settings - var httpClient *http.Client - if s3Config.SSLVerifyPeer { - httpClient = boshhttp.CreateDefaultClient(nil) - } else { - httpClient = boshhttp.CreateDefaultClientInsecureSkipVerify() - } - - // Set up AWS config options - options := []func(*awsconfig.LoadOptions) error{ - awsconfig.WithHTTPClient(httpClient), - } - - if s3Config.UseRegion() { - options = append(options, awsconfig.WithRegion(s3Config.Region)) - } else { - options = append(options, awsconfig.WithRegion(config.EmptyRegion)) - } - - if s3Config.CredentialsSource == config.StaticCredentialsSource { - options = append(options, awsconfig.WithCredentialsProvider( - credentials.NewStaticCredentialsProvider(s3Config.AccessKeyID, s3Config.SecretAccessKey, ""), - )) - } - - if s3Config.CredentialsSource == config.NoneCredentialsSource { - options = append(options, awsconfig.WithCredentialsProvider(aws.AnonymousCredentials{})) - } - - // Load AWS config - awsConfig, err := awsconfig.LoadDefaultConfig(context.TODO(), options...) - if err != nil { - return nil, err - } - - // Handle STS assume role if configured - if s3Config.AssumeRoleArn != "" { - stsClient := sts.NewFromConfig(awsConfig) - provider := stscreds.NewAssumeRoleProvider(stsClient, s3Config.AssumeRoleArn) - awsConfig.Credentials = aws.NewCredentialsCache(provider) - } - - // Create failure injection middlewares - trackingMiddleware := CreateUploadPartTracker() - corruptionMiddleware := CreateSHACorruptionMiddleware() - - // Create S3 client with custom middleware and options - s3Client := s3.NewFromConfig(awsConfig, func(o *s3.Options) { - o.UsePathStyle = !s3Config.HostStyle - if s3Config.S3Endpoint() != "" { - o.BaseEndpoint = aws.String(s3Config.S3Endpoint()) - } - - // Add the failure injection middlewares - o.APIOptions = append(o.APIOptions, func(stack *middleware.Stack) error { - // Add initialize middleware to track UploadPart operations - if err := stack.Initialize.Add(trackingMiddleware, middleware.Before); err != nil { - return err - } - // Add finalize middleware to corrupt headers after signing - return stack.Finalize.Add(corruptionMiddleware, middleware.After) - }) - }) - - return s3Client, nil -} - // S3TracingMiddleware captures S3 operation names for testing type S3TracingMiddleware struct { calls *[]string } // CreateS3TracingMiddleware creates a middleware that tracks S3 operation calls -func CreateS3TracingMiddleware(calls *[]string) *S3TracingMiddleware { +func createS3TracingMiddleware(calls *[]string) *S3TracingMiddleware { return &S3TracingMiddleware{calls: calls} } @@ -179,79 +102,3 @@ func (m *S3TracingMiddleware) HandleInitialize( return next.HandleInitialize(ctx, in) } - -// CreateS3ClientWithTracing creates a new S3 client with tracing middleware -func CreateS3ClientWithTracing(baseClient *s3.Client, tracingMiddleware *S3TracingMiddleware) *s3.Client { - // Create a wrapper that captures calls and delegates to the base client - // Since AWS SDK v2 makes it difficult to extract config from existing clients, - // we'll use a different approach: modify the traceS3 function to work differently - - // For the tracing functionality, we'll need to intercept at a higher level - // The current implementation will track operations through the middleware - // that inspects the input parameters - - return baseClient -} - -// CreateTracingS3Client creates an S3 client with tracing middleware from config -func CreateTracingS3Client(s3Config *config.S3Cli, calls *[]string) (*s3.Client, error) { - // Create HTTP client based on SSL verification settings - var httpClient *http.Client - if s3Config.SSLVerifyPeer { - httpClient = boshhttp.CreateDefaultClient(nil) - } else { - httpClient = boshhttp.CreateDefaultClientInsecureSkipVerify() - } - - // Set up AWS config options - options := []func(*awsconfig.LoadOptions) error{ - awsconfig.WithHTTPClient(httpClient), - } - - if s3Config.UseRegion() { - options = append(options, awsconfig.WithRegion(s3Config.Region)) - } else { - options = append(options, awsconfig.WithRegion(config.EmptyRegion)) - } - - if s3Config.CredentialsSource == config.StaticCredentialsSource { - options = append(options, awsconfig.WithCredentialsProvider( - credentials.NewStaticCredentialsProvider(s3Config.AccessKeyID, s3Config.SecretAccessKey, ""), - )) - } - - if s3Config.CredentialsSource == config.NoneCredentialsSource { - options = append(options, awsconfig.WithCredentialsProvider(aws.AnonymousCredentials{})) - } - - // Load AWS config - awsConfig, err := awsconfig.LoadDefaultConfig(context.TODO(), options...) - if err != nil { - return nil, err - } - - // Handle STS assume role if configured - if s3Config.AssumeRoleArn != "" { - stsClient := sts.NewFromConfig(awsConfig) - provider := stscreds.NewAssumeRoleProvider(stsClient, s3Config.AssumeRoleArn) - awsConfig.Credentials = aws.NewCredentialsCache(provider) - } - - // Create tracing middleware - tracingMiddleware := CreateS3TracingMiddleware(calls) - - // Create S3 client with tracing middleware - s3Client := s3.NewFromConfig(awsConfig, func(o *s3.Options) { - o.UsePathStyle = !s3Config.HostStyle - if s3Config.S3Endpoint() != "" { - o.BaseEndpoint = aws.String(s3Config.S3Endpoint()) - } - - // Add the tracing middleware - o.APIOptions = append(o.APIOptions, func(stack *middleware.Stack) error { - return stack.Initialize.Add(tracingMiddleware, middleware.Before) - }) - }) - - return s3Client, nil -} diff --git a/s3/integration/s3_compatible_test.go b/s3/integration/s3_compatible_test.go index 5eabf01..41d54e0 100644 --- a/s3/integration/s3_compatible_test.go +++ b/s3/integration/s3_compatible_test.go @@ -54,7 +54,6 @@ var _ = Describe("Testing in any non-AWS, S3 compatible storage service", func() MultipartUpload: true, }), Entry("with the maximal configuration", &config.S3Cli{ - SignatureVersion: 2, CredentialsSource: "static", AccessKeyID: accessKeyID, SecretAccessKey: secretAccessKey, @@ -80,7 +79,7 @@ var _ = Describe("Testing in any non-AWS, S3 compatible storage service", func() func(cfg *config.S3Cli) { integration.AssertDeleteNonexistentWorks(s3CLIPath, cfg) }, configurations, ) - DescribeTable("Invoking `s3cli put` handling of mulitpart uploads", + DescribeTable("Invoking `s3cli put` handling of multipart uploads", func(cfg *config.S3Cli) { integration.AssertOnMultipartUploads(s3CLIPath, cfg, largeContent) }, configurations, ) diff --git a/s3/integration/utils.go b/s3/integration/utils.go index 3aa4bbd..8bec3be 100644 --- a/s3/integration/utils.go +++ b/s3/integration/utils.go @@ -7,6 +7,9 @@ import ( "os/exec" "time" + "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/smithy-go/middleware" + "github.com/cloudfoundry/storage-cli/s3/client" "github.com/cloudfoundry/storage-cli/s3/config" . "github.com/onsi/ginkgo/v2" //nolint:staticcheck @@ -72,3 +75,34 @@ func RunS3CLI(cliPath string, configPath string, storageType string, subcommand session.Wait(1 * time.Minute) return session, nil } + +// CreateS3ClientWithFailureInjection creates an S3 client with failure injection middleware +func CreateS3ClientWithFailureInjection(s3Config *config.S3Cli) (*s3.Client, error) { + var apiOptions []func(stack *middleware.Stack) error + // Create tracker once so the middleware shares a single counter across requests. + uploadPartTracker := createUploadPartTracker() + apiOptions = append(apiOptions, func(stack *middleware.Stack) error { + // Add initialize middleware to track UploadPart operations + if err := stack.Initialize.Add(uploadPartTracker, middleware.Before); err != nil { + return err + } + // Add finalize middleware to corrupt headers after signing + return stack.Finalize.Add(createSHACorruptionMiddleware(), middleware.After) + }) + + return client.NewAwsS3ClientWithApiOptions(s3Config, apiOptions) +} + +// CreateTracingS3Client creates an S3 client with tracing middleware +func CreateTracingS3Client(s3Config *config.S3Cli, calls *[]string) (*s3.Client, error) { + var apiOptions []func(stack *middleware.Stack) error + // Setup middleware fixing request to Google - they expect the 'accept-encoding' header + // to not be included in the signature of the request. + apiOptions = append(apiOptions, client.AddFixAcceptEncodingMiddleware) + // Use the centralized client creation logic with a custom middleware + apiOptions = append(apiOptions, func(stack *middleware.Stack) error { + return stack.Initialize.Add(createS3TracingMiddleware(calls), middleware.Before) + }) + + return client.NewAwsS3ClientWithApiOptions(s3Config, apiOptions) +}