-
Notifications
You must be signed in to change notification settings - Fork 417
NO-JIRA: Migrate to aws-sdk-go-v2 from archived aws-sdk-go #2114
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.
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
NO-JIRA: Migrate to aws-sdk-go-v2 from archived aws-sdk-go #2114
Conversation
|
@ardaguclu: This pull request explicitly references no jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughReplaced AWS SDK for Go v1 with v2 in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🔇 Additional comments (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| url *url.URL | ||
| } | ||
|
|
||
| func (s *s3CredentialStore) IsExpired() bool { return !s.retrieved } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IsExpired was removed from the interface in v2.
|
/hold |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/cli/image/imagesource/s3.go (1)
190-201: URL-encode CopySource
CopySource must be URL-encoded by the caller before callingCopyObject(the SDK won’t do this). For example:enc := strings.ReplaceAll(url.PathEscape(sourceKey), "%2F", "/") CopySource: aws.String(enc)
🧹 Nitpick comments (4)
pkg/cli/image/imagesource/s3.go (4)
65-66: Honor caller context for config loadingUsing context.Background() drops cancellations/timeouts from upstream. Plumb ctx into newObject and use it for config.LoadDefaultConfig.
-func (d *s3Driver) newObject(server *url.URL, region string, insecure bool, securityDomain *url.URL) (*s3.Client, error) { +func (d *s3Driver) newObject(ctx context.Context, server *url.URL, region string, insecure bool, securityDomain *url.URL) (*s3.Client, error) { @@ -ctx := context.Background() +// use provided ctxAnd pass ctx from Repository.
Also applies to: 88-89
283-289: CopyObject ContentType is ignored unless MetadataDirective=REPLACESetting ContentType on CopyObject has no effect without MetadataDirective=REPLACE. Either remove ContentType to inherit from source or set the directive.
- if _, err := s.r.s3.CopyObject(s.r.ctx, &s3.CopyObjectInput{ + if _, err := s.r.s3.CopyObject(s.r.ctx, &s3.CopyObjectInput{ Bucket: aws.String(s.r.bucket), - ContentType: aws.String(mediaType), CopySource: aws.String(path.Join(s.r.bucket, blob)), Key: aws.String(fmt.Sprintf("/v2/%s/manifests/%s", s.r.repoName, tag)), + // Uncomment to override metadata and ContentType: + // MetadataDirective: s3types.MetadataDirectiveReplace, + // ContentType: aws.String(mediaType), }) ; err != nil {
37-38: Optional: guard repository cache map for concurrent useIf s3Driver can be used concurrently, wrap repositories with a mutex or use sync.Map.
209-216: Uploader defaults: consider explicit part size/concurrency for large objectsmanager.Uploader defaults are fine for small objects; for large layers/manifests, set PartSize and Concurrency to match env constraints.
Example:
u := manager.NewUploader(r.s3, func(o *manager.UploaderOptions) { o.PartSize = 8 * 1024 * 1024 // 8 MiB o.Concurrency = 5 })Based on learnings
Also applies to: 411-417
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (298)
go.sumis excluded by!**/*.sumvendor/github.com/aws/aws-sdk-go-v2/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/NOTICE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/accountid_endpoint_mode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/arn/arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/checksum.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/credential_cache.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/credentials.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/auto.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/configuration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/defaults.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaultsmode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/endpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/from_ptr.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/logging.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/logging_generate.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/osname.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/osname_go115.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/recursion_detection.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id_retriever.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/user_agent.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/debug.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/decode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/encode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/eventstreamapi/headers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/eventstreamapi/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/eventstreamapi/transport.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/eventstreamapi/transport_go117.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/header.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/header_value.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/message.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/array.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/encoder.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/map.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/object.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/value.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/restjson/decoder_util.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/xml/error_utils.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/none.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/token_bucket.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/token_rate_limit.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/request.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive_ratelimit.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive_token_bucket.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/attempt_metrics.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/jitter_backoff.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/retry.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/retryable_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/standard.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/throttle_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/timeout_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retryer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/runtime.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/cache.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/const.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/header_rules.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/headers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/hmac.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/host.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/scope.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/time.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/util.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/presign_middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/v4.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/to_ptr.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/content_type.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/response_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/response_error_middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/timeout_read_closer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/defaultsmode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/env_config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/generate.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/load_options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/local.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/resolve.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/resolve_bearer_token.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/resolve_credentials.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/shared_config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ec2rolecreds/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ec2rolecreds/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/auth.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/endpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_cached_token.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_credentials_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_token_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/static_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/assume_role_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/web_identity_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetDynamicData.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetIAMInfo.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetInstanceIdentityDocument.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetMetadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetRegion.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetToken.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetUserData.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/auth.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/endpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/internal/config/resolvers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/request_middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/token_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/api.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/bucket_region.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/buffered_read_seeker.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/default_read_seeker_write_to.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/default_read_seeker_write_to_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/default_writer_read_from.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/default_writer_read_from_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/download.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/pool.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/read_seeker_write_to.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/upload.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/writer_read_from.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/auth.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/scheme.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/bearer_token_adapter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/bearer_token_signer_adapter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/credentials_adapter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/smithy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/v4signer_adapter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/awsutil/copy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/awsutil/equal.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/awsutil/prettify.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/awsutil/string_value.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/endpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/context/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/generate.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/host.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partition.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/endpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/endpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/ini.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/parse.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/sections.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/strings.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/token.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/tokenize.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/value.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/middleware/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/rand/rand.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sdk/interfaces.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sdk/time.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sdkio/byte.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/shareddefaults/shared_config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/strings/strings.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/docs.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/singleflight.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/timeconv/duration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/credentials.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/crypto/compare.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/crypto/ecc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/const.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/header_rules.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/headers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/hmac.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/host.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/time.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/util.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/presign_middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/smithy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/v4a.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/accept_encoding_gzip.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/algorithms.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/aws_chunked_encoding.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/middleware_add.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/middleware_checksum_metrics_tracking.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/middleware_compute_input_checksum.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/middleware_setup_context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/middleware_validate_output.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/arn/accesspoint_arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/arn/arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/arn/arn_member.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/arn/outpost_arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/arn/s3_object_lambda_arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/arn_lookup.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/config/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/endpoint_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/host_id.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/metadata_retriever.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/resource_request.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/response_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/response_error_middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/s3100continue.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/update_endpoint.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/xml_utils.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_AbortMultipartUpload.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CompleteMultipartUpload.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CopyObject.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CreateBucket.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CreateBucketMetadataTableConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CreateMultipartUpload.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CreateSession.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucket.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketAnalyticsConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketCors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketEncryption.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketIntelligentTieringConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketInventoryConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketLifecycle.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketMetadataTableConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketMetricsConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketOwnershipControls.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketPolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketReplication.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketTagging.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketWebsite.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteObject.gois excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (2)
go.mod(2 hunks)pkg/cli/image/imagesource/s3.go(12 hunks)
🔇 Additional comments (2)
go.mod (2)
13-17: AWS SDK v2 modules added — align versions across SDK familyLooks good. Please ensure all aws-sdk-go-v2 modules (aws, config, s3, feature/s3/manager, smithy-go) are upgraded in lockstep to avoid subtle incompatibilities and to meet smithy-go’s minimum Go version. Your go 1.24.0 satisfies smithy-go v1.22.3 (Go ≥ 1.22). Consider a quick pass of go mod tidy after bumping to keep indirects coherent.
Based on learnings
77-90: Disregard aligning AWS SDK v2 submodule versions Each AWS v2 submodule is independently versioned; you don’t need matching minor versions across modules.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
go.mod (1)
77-90: Let go mod tidy manage indirect AWS SDK v2 modules
Multiple internal AWS SDK v2 modules are listed as// indirectbut aren’t directly imported; rungo mod tidyto remove explicit indirect pins and reduce churn.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (83)
go.sumis excluded by!**/*.sumvendor/github.com/aws/aws-sdk-go-v2/aws/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/credential_cache.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/credentials.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/user_agent.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/message.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/auth_scheme_preference.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/env_config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/load_options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/resolve.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/resolve_credentials.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/shared_config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ec2rolecreds/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_credentials_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/static_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/assume_role_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/web_identity_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/bucket_region.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/download.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/upload.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/headers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/algorithms.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/middleware_compute_input_checksum.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/middleware_validate_output.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_AbortMultipartUpload.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CompleteMultipartUpload.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CopyObject.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CreateBucket.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CreateBucketMetadataConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CreateBucketMetadataTableConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CreateMultipartUpload.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CreateSession.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucket.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketAnalyticsConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketCors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketEncryption.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketIntelligentTieringConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketInventoryConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketLifecycle.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketMetadataConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketMetadataTableConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketMetricsConfiguration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketOwnershipControls.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketPolicy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketReplication.gois excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (1)
go.mod(2 hunks)
🔇 Additional comments (1)
go.mod (1)
13-17: Approve AWS SDK v2 migration
- No remaining github.com/aws/aws-sdk-go (v1) imports.
- github.com/aws/smithy-go is directly used in pkg/cli/image/imagesource/s3.go.
- go mod tidy yields no changes.
pkg/cli/image/imagesource/s3.go
Outdated
| o.EndpointOptions.DisableHTTPS = true | ||
| } | ||
| if d.UserAgent != "" { | ||
| o.APIOptions = append(o.APIOptions, awsmiddleware.AddUserAgentKeyValue("oc", d.UserAgent)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the CodeRabbit, we can inject custom user agent by using aws sdk middleware. So that we don't need to use custom HttpClient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this use just AddKey? The original implementation uses d.UserAgent as the user agent string, there is no prefix added to that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean AddUserAgentKey
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for review. Nice catch. I'll update with this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
|
/retest |
1 similar comment
|
/retest |
| &s3CredentialStore{store: d.Creds, url: securityDomain}, | ||
| &credentials.EnvProvider{}, | ||
| &credentials.SharedCredentialsProvider{}, | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that both env and shared credentials are handled now in config.LoadDefaultConfig, so this should be ok. Just a note to myself since it may seem removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was under the same impression that this fallback mechanism had been removed under LoadDefaultConfig.
pkg/cli/image/imagesource/s3.go
Outdated
| configOpts = append(configOpts, config.WithClientLogMode(aws.LogRetries|aws.LogRequest)) | ||
| case klog.V(6).Enabled(): | ||
| awsConfig.WithLogLevel(aws.LogDebug) | ||
| configOpts = append(configOpts, config.WithClientLogMode(aws.LogRetries)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this map to the former logging easily? It seem to me v6 is usually already super debug and you would just log everything there. So shouldn't we just do, log everything on v6 or higher?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since log types in aws has changed, it is difficult to keep one to one mapping. But I updated the code to follow semantically same principal. It logs responseBody in verbosity 10. It logs response and request in verbosity 8. It logs request.
Please let me know your thoughts about this?.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's fine then. Although I don't know why it's split like that. But let's keep it if it was like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the reason is to follow k8s standards. In k8s, verbosity 10 logs everything but verbosity 6 usually does not contain any response body, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks for the explanation.
b76e65f to
40af020
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/cli/image/imagesource/s3.go (1)
65-65: Consider accepting context as a parameter instead of creating a new one.Using
context.Background()works for config loading, but accepting a context parameter would allow for better cancellation and timeout control, especially if this method is called during a long-running operation.Apply this diff if context propagation is desired:
-func (d *s3Driver) newObject(server *url.URL, region string, insecure bool, securityDomain *url.URL) (*s3.Client, error) { +func (d *s3Driver) newObject(ctx context.Context, server *url.URL, region string, insecure bool, securityDomain *url.URL) (*s3.Client, error) { key := fmt.Sprintf("%s:%s:%t:%s", server, region, insecure, securityDomain) s3obj, ok := d.repositories[key] if ok { return s3obj, nil } - ctx := context.Background() - configOpts := []func(*config.LoadOptions) error{
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
pkg/cli/image/imagesource/s3.go(12 hunks)
🔇 Additional comments (16)
pkg/cli/image/imagesource/s3.go (16)
6-6: LGTM!The addition of the
errorspackage is necessary for theerrors.Astype assertion used later in the smithy error handling (line 196).
18-23: LGTM!The AWS SDK v2 imports are correctly structured, including the necessary middleware package for User-Agent customization and smithy-go for error handling.
37-37: LGTM!The type change from
*s3.S3to*s3.Clientcorrectly reflects the AWS SDK v2 client type.
45-56: LGTM!The
Retrievemethod correctly implements the AWS SDK v2aws.CredentialsProviderinterface with context support and proper error handling.
86-93: LGTM!The S3 client configuration correctly handles insecure mode and User-Agent customization using SDK middleware as recommended in past reviews.
133-133: LGTM!The type change from
*s3.S3to*s3.Clientcorrectly reflects the AWS SDK v2 client type.
164-167: LGTM!The
HeadObjectcall correctly uses AWS SDK v2 API with context as the first parameter.
186-189: LGTM!The
CopyObjectcall correctly uses AWS SDK v2 API with context as the first parameter.
195-196: LGTM!The error handling correctly migrates from
awserrtosmithy.APIErrorusingerrors.Asfor type checking, which is the proper approach for AWS SDK v2.
205-212: LGTM!The upload logic correctly migrates to AWS SDK v2:
- Uses
*s3.PutObjectInputinstead of v1'ss3manager.UploadInput- Creates uploader with
manager.NewUploader- Passes context to
Uploadmethod
216-221: LGTM!The metadata type correctly changes from
map[string]*stringtomap[string]string, which is a simplification in AWS SDK v2 that eliminates the need for pointer values.
262-265: LGTM!The
conditionalUploadcall correctly uses*s3.PutObjectInput.
279-282: LGTM!The
CopyObjectcall correctly uses AWS SDK v2 API with context.
326-329: LGTM!The
conditionalUploadcall correctly uses*s3.PutObjectInput.
407-411: LGTM!The writer's upload logic correctly migrates to AWS SDK v2, creating the uploader from the S3 client and passing context to the
Uploadmethod.
58-100: Verify custom endpoint handling for non-AWS S3 deployments.Past review comments indicate that custom endpoint support (lines 58-60) was addressed, but the current code doesn't apply the
serverparameter when configuring the S3 client. Theserverparameter is accepted but only used for cache keying (line 59), not for settingBaseEndpointorUsePathStyle.If custom endpoints (MinIO, Ceph, or other S3-compatible services) are expected to work, the client configuration should include:
if server != nil && server.Host != "" { o.BaseEndpoint = aws.String(server.Scheme + "://" + server.Host) o.UsePathStyle = true }Run the following script to check if the
serverparameter is ever non-nil in production usage:
| } | ||
| _, err := s3manager.NewUploaderWithClient(r.s3).Upload(input) | ||
| uploader := manager.NewUploader(r.s3) | ||
| _, err := uploader.Upload(r.ctx, input) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: I think that this reads better as uploader := manager.NewUploader(r.s3).Upload(r.ctx, input)
The same thing on line 412...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be nicer. If you'll have blocker comments, I'll update these too. Otherwise, we can leave it as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's ok then.
|
/lgtm |
|
Thanks again for review. I'm unsure we should do pre-merge tests or not. Probably CI won't test these bits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/cli/image/imagesource/s3.go (1)
186-191: CopySource must be URL‑encoded — keys with ':' (digests) or spaces will failS3 requires x-amz-copy-source be URL-encoded. Digests include ':'; unencoded values can return 400 InvalidRequest.
- _, err := r.s3.CopyObject(r.ctx, &s3.CopyObjectInput{ - CopySource: aws.String(sourceKey), + _, err := r.s3.CopyObject(r.ctx, &s3.CopyObjectInput{ + CopySource: aws.String(url.PathEscape(sourceKey)), Bucket: aws.String(bucket), Key: aws.String(key), })- if _, err := s.r.s3.CopyObject(s.r.ctx, &s3.CopyObjectInput{ + if _, err := s.r.s3.CopyObject(s.r.ctx, &s3.CopyObjectInput{ Bucket: aws.String(s.r.bucket), ContentType: aws.String(mediaType), - CopySource: aws.String(path.Join(s.r.bucket, blob)), + CopySource: aws.String(url.PathEscape(path.Join(s.r.bucket, blob))), Key: aws.String(fmt.Sprintf("/v2/%s/manifests/%s", s.r.repoName, tag)), }); err != nil {Note: net/url is already imported.
Also applies to: 279-284
♻️ Duplicate comments (1)
pkg/cli/image/imagesource/s3.go (1)
86-93: Custom S3 endpoint not wired; path‑style not enabled — non‑AWS S3 likely breaksserver is ignored when creating the s3 client. This will fail for custom endpoints (MinIO/Ceph) and buckets with dots unless you set BaseEndpoint and UsePathStyle. Also respect server.Scheme for HTTPS.
Apply inside the s3.NewFromConfig options:
s3Client := s3.NewFromConfig(cfg, func(o *s3.Options) { if insecure { o.EndpointOptions.DisableHTTPS = true } + if server != nil && server.Host != "" { + // Honor explicit endpoint and favor path-style for non-AWS S3s / dotted buckets. + o.BaseEndpoint = aws.String(server.Scheme + "://" + server.Host) + o.UsePathStyle = true + if strings.EqualFold(server.Scheme, "http") { + o.EndpointOptions.DisableHTTPS = true + } + } if d.UserAgent != "" { o.APIOptions = append(o.APIOptions, awsmiddleware.AddUserAgentKey(d.UserAgent)) } })
🧹 Nitpick comments (3)
pkg/cli/image/imagesource/s3.go (3)
58-58: Plumb context instead of using Background for config loadUsing context.Background drops cancellation/timeouts. Pass the caller’s ctx through newObject and into LoadDefaultConfig.
-func (d *s3Driver) newObject(server *url.URL, region string, insecure bool, securityDomain *url.URL) (*s3.Client, error) { +func (d *s3Driver) newObject(ctx context.Context, server *url.URL, region string, insecure bool, securityDomain *url.URL) (*s3.Client, error) { key := fmt.Sprintf("%s:%s:%t:%s", server, region, insecure, securityDomain) s3obj, ok := d.repositories[key] if ok { return s3obj, nil } - ctx := context.Background() + // use passed ctx - cfg, err := config.LoadDefaultConfig(ctx, configOpts...) + cfg, err := config.LoadDefaultConfig(ctx, configOpts...)- s3obj, err := d.newObject(server, parts[0], insecure, &url.URL{Scheme: server.Scheme, Host: server.Host, Path: "/" + repoName}) + s3obj, err := d.newObject(ctx, server, parts[0], insecure, &url.URL{Scheme: server.Scheme, Host: server.Host, Path: "/" + repoName})Also applies to: 65-66, 81-81, 107-107
37-38: repositories map is unsynchronized; add a mutex to avoid data racesConcurrent Repository/newObject calls can race on map reads/writes.
type s3Driver struct { UserAgent string Region string Creds auth.CredentialStore CopyFrom []string - repositories map[string]*s3.Client + repositories map[string]*s3.Client + mu sync.Mutex }- s3obj, ok := d.repositories[key] + d.mu.Lock() + s3obj, ok := d.repositories[key] if ok { - return s3obj, nil + d.mu.Unlock() + return s3obj, nil } + d.mu.Unlock()- if d.repositories == nil { - d.repositories = make(map[string]*s3.Client) - } - d.repositories[key] = s3Client + d.mu.Lock() + if d.repositories == nil { + d.repositories = make(map[string]*s3.Client) + } + d.repositories[key] = s3Client + d.mu.Unlock()Also applies to: 60-63, 95-99
226-234: Remove unused noSeekReadernoSeekReader isn’t referenced. Delete to reduce noise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
pkg/cli/image/imagesource/s3.go(12 hunks)
|
/lgtm cancel until we research the review bot comment. |
40af020 to
4ca6440
Compare
|
/lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pkg/cli/image/imagesource/s3.go (1)
90-97: Custom S3 endpoint parameter is not applied—non-AWS S3 will fail.The
server *url.URLparameter (line 58) is never used to configure the S3 client's endpoint. This breaks support for custom S3-compatible services like MinIO or Ceph. A past review comment indicated this should be addressed, but the fix is not present in the current code.Add endpoint configuration inside the options function:
s3Client := s3.NewFromConfig(cfg, func(o *s3.Options) { if insecure { o.EndpointOptions.DisableHTTPS = true } + if server != nil && server.Host != "" { + o.BaseEndpoint = aws.String(server.Scheme + "://" + server.Host) + o.UsePathStyle = true + } if d.UserAgent != "" { o.APIOptions = append(o.APIOptions, awsmiddleware.AddUserAgentKey(d.UserAgent)) } })This ensures custom endpoints work correctly, and
UsePathStylehelps with bucket names containing dots on non-AWS endpoints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
pkg/cli/image/imagesource/s3.go(12 hunks)
🔇 Additional comments (7)
pkg/cli/image/imagesource/s3.go (7)
3-29: LGTM! Imports correctly updated for AWS SDK v2.The migration to v2 imports is complete and correct:
- AWS SDK v2 core and config modules
- S3 service client and manager
- smithy-go for error handling
- awsmiddleware for User-Agent customization
40-56: LGTM! Credential store correctly implements AWS SDK v2 interface.The
Retrievemethod signature and return type correctly match the v2aws.CredentialsProviderinterface. SettingSourceto "DockerCfg" provides useful traceability for credential resolution.
76-83: LGTM! Logging configuration appropriately maps klog verbosity to AWS SDK v2 log modes.The verbosity-to-log-mode mapping preserves the desired level of detail at each klog level while using v2's structured logging flags.
167-207: LGTM! HeadObject and CopyObject correctly migrated to AWS SDK v2.The error handling properly uses
smithy.APIErrorwitherrors.Asto check for "NoSuchKey", and all S3 operations pass context correctly.
209-216: LGTM! Upload flow correctly migrated to AWS SDK v2 manager.The signature change to
*s3.PutObjectInputand usage ofmanager.NewUploaderwith context-awareUploadcall is correct for v2.
220-222: LGTM! Metadata type correctly updated for AWS SDK v2.The change from
map[string]*stringtomap[string]stringaligns with v2's simplified metadata API.
283-291: LGTM! All S3 operations correctly use AWS SDK v2 APIs.The manifest Put, blob Put, and writer ReadFrom methods all properly use v2 client calls with context propagation and correct input types.
Also applies to: 330-338, 411-417
pkg/cli/image/imagesource/s3.go
Outdated
| } | ||
|
|
||
| credStore := &s3CredentialStore{store: d.Creds, url: securityDomain} | ||
| if _, err := credStore.Retrieve(ctx); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this should be err == nil as the bot noticed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Today, bot is definitely clever than me :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Today is literally the first time I find the bot useful 😅
|
/lgtm |
|
I was too fast, one more time 😅 /lgtm |
|
/hold cancel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
pkg/cli/image/imagesource/s3.go(11 hunks)
🔇 Additional comments (12)
pkg/cli/image/imagesource/s3.go (12)
3-29: LGTM! SDK v2 imports are correct.The imports correctly reflect the AWS SDK for Go v2 migration, including the necessary middleware, config, manager, and smithy-go packages for error handling.
45-56: LGTM! Credential provider correctly implements v2 interface.The
Retrievemethod correctly implements the AWS SDK v2CredentialsProviderinterface, accepting a context and returningaws.Credentials. The logic properly handles missing credentials and sets the credential source for observability.
65-65: Verify if context.Background() is appropriate for client initialization.The method uses
context.Background()for config loading and credential retrieval, even thoughRepositoryreceives a context from the caller. Since the client is cached and reused, this means the initial credential retrieval doesn't respect the caller's context cancellation or deadline.This may be intentional for long-lived client initialization, but consider if the context should be passed through from the caller for consistency and to respect timeouts during initial setup.
67-74: LGTM! Credential provider logic is correct.The conditional logic correctly adds the custom credential provider only when Docker config credentials exist (
err == nil), allowing the SDK to fall back to the default credential chain (environment variables, shared config, IMDS, SSO) when credentials are absent. This properly addresses the critical issue from previous reviews.Based on learnings
76-83: LGTM! Log mode configuration follows v2 SDK patterns.The log mode configuration correctly uses v2 SDK constants and follows Kubernetes logging conventions, with verbosity level 10 logging response bodies, level 8 logging requests/responses without bodies, and level 6 logging requests only.
94-95: LGTM! User-Agent middleware correctly preserves SDK UA.The implementation correctly uses
awsmiddleware.AddUserAgentKeyto append the custom User-Agent value, preserving the AWS SDK's built-in UA and instrumentation. This addresses the concern from previous reviews about not overriding the SDK's UA.Based on learnings
167-207: LGTM! S3 operations correctly use v2 SDK patterns.The S3 operations properly use the v2 SDK with:
- Correct client type (
*s3.Client)- Proper context propagation using
r.ctx- v2 error handling with
smithy.APIErrorandErrorCode()method for checkingNoSuchKeyerrors
209-216: LGTM! Upload logic correctly uses v2 manager.The
conditionalUploadmethod properly uses the v2 SDK'smanager.NewUploaderwith the correct*s3.PutObjectInputtype and context propagation.Based on learnings
218-228: LGTM! Metadata type correctly updated for v2.The initialization properly uses
map[string]stringfor metadata, which is the correct type in AWS SDK Go v2 (changed frommap[string]*stringin v1).
255-293: LGTM! Manifest operations correctly migrated to v2.The manifest Put method properly uses v2 SDK patterns with correct input types, context propagation, and proper URL encoding for
CopySource.
325-339: LGTM! Blob store operations correctly use v2 SDK.The blob store Put method consistently uses v2 SDK patterns with the correct
s3.PutObjectInputtype.
399-422: LGTM! Writer upload correctly uses v2 manager.The writer's
ReadFrommethod properly uses the v2 SDK'smanager.NewUploaderwith correct context propagation fromw.driver.ctx.Based on learnings
pkg/cli/image/imagesource/s3.go
Outdated
| Bucket: aws.String(s.r.bucket), | ||
| ContentType: aws.String(mediaType), | ||
| CopySource: aws.String(path.Join(s.r.bucket, blob)), | ||
| CopySource: aws.String(s.r.bucket + "/" + url.QueryEscape(blob)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there any reason to remove path.Join ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not actually. Critical change is using the QueryEscape
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want me still using path.Join?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it would be better to keep it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
f9b2fa2 to
07a9b47
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
go.mod (1)
77-90: Remove unimported AWS SDK v2 submodules
- Drop these from go.mod; let Go MVS resolve them transitively:
• github.com/aws/aws-sdk-go-v2/internal/configsources
• github.com/aws/aws-sdk-go-v2/internal/endpoints/v2
• github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding
• github.com/aws/aws-sdk-go-v2/service/internal/presigned-url
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (275)
go.sumis excluded by!**/*.sumvendor/github.com/aws/aws-sdk-go-v2/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/NOTICE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/accountid_endpoint_mode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/arn/arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/checksum.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/credential_cache.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/credentials.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/auto.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/configuration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/defaults.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaultsmode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/endpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/from_ptr.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/logging.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/logging_generate.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/osname.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/osname_go115.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/recursion_detection.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id_retriever.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/user_agent.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/debug.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/decode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/encode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/eventstreamapi/headers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/eventstreamapi/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/eventstreamapi/transport.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/eventstreamapi/transport_go117.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/header.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/header_value.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/message.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/array.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/encoder.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/map.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/object.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/value.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/restjson/decoder_util.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/xml/error_utils.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/none.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/token_bucket.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/token_rate_limit.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/request.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive_ratelimit.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive_token_bucket.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/attempt_metrics.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/jitter_backoff.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/retry.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/retryable_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/standard.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/throttle_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/timeout_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retryer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/runtime.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/cache.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/const.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/header_rules.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/headers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/hmac.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/host.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/scope.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/time.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/util.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/presign_middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/stream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/v4.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/to_ptr.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/content_type.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/response_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/response_error_middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/timeout_read_closer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/auth_scheme_preference.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/defaultsmode.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/env_config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/generate.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/load_options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/local.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/resolve.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/resolve_bearer_token.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/resolve_credentials.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/shared_config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ec2rolecreds/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ec2rolecreds/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/auth.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/endpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_cached_token.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_credentials_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_token_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/static_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/assume_role_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/web_identity_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetDynamicData.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetIAMInfo.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetInstanceIdentityDocument.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetMetadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetRegion.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetToken.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetUserData.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/auth.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/endpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/internal/config/resolvers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/request_middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/token_provider.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/api.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/bucket_region.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/buffered_read_seeker.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/default_read_seeker_write_to.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/default_read_seeker_write_to_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/default_writer_read_from.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/default_writer_read_from_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/download.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/pool.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/read_seeker_write_to.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/upload.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/writer_read_from.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/auth.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/scheme.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/bearer_token_adapter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/bearer_token_signer_adapter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/credentials_adapter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/smithy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/v4signer_adapter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/awsutil/copy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/awsutil/equal.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/awsutil/prettify.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/awsutil/string_value.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/endpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/context/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/generate.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/host.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partition.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/endpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/endpoints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/ini.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/parse.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/sections.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/strings.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/token.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/tokenize.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/ini/value.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/middleware/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/rand/rand.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sdk/interfaces.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sdk/time.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sdkio/byte.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/shareddefaults/shared_config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/strings/strings.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/docs.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/singleflight.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/timeconv/duration.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/credentials.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/crypto/compare.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/crypto/ecc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/const.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/header_rules.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/headers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/hmac.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/host.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/time.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/util.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/presign_middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/smithy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/v4a.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/accept_encoding_gzip.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/algorithms.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/aws_chunked_encoding.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/middleware_add.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/middleware_checksum_metrics_tracking.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/middleware_compute_input_checksum.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/middleware_setup_context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/middleware_validate_output.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/arn/accesspoint_arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/arn/arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/arn/arn_member.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/arn/outpost_arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/arn/s3_object_lambda_arn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/arn_lookup.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/config/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/endpoint_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/go_module_metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/host_id.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/metadata.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/metadata_retriever.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/resource_request.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/response_error.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/response_error_middleware.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/s3100continue.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/update_endpoint.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/xml_utils.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/s3/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (2)
go.mod(2 hunks)pkg/cli/image/imagesource/s3.go(11 hunks)
🔇 Additional comments (3)
pkg/cli/image/imagesource/s3.go (2)
71-74: LGTM: Credential provider logic is correct.The conditional logic correctly adds the custom credentials provider only when Docker config credentials are present, allowing the SDK to fall back to the default credential chain (environment variables, shared config, IMDS, SSO) otherwise.
94-96: LGTM: User-Agent middleware usage is correct.The implementation correctly uses
awsmiddleware.AddUserAgentKeyto append the custom User-Agent, which preserves the SDK's own UA and instrumentation, as recommended in previous reviews.Based on learnings
go.mod (1)
13-17: No legacy v1 imports; keep explicit smithy-go require
All aws-sdk-go v1 imports are gone. pkg/cli/image/imagesource/s3.go directly imports github.com/aws/smithy-go, so the explicit require is intentional. LGTM.
| s3Client := s3.NewFromConfig(cfg, func(o *s3.Options) { | ||
| if insecure { | ||
| o.EndpointOptions.DisableHTTPS = true | ||
| } | ||
| if d.UserAgent != "" { | ||
| o.APIOptions = append(o.APIOptions, awsmiddleware.AddUserAgentKey(d.UserAgent)) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Server endpoint parameter is still not applied—breaks non-AWS S3.
The server *url.URL parameter (line 58) is never used when creating the S3 client, which will break MinIO, Ceph, and other S3-compatible services. A previous review flagged this issue and it was marked as addressed in commit f94c527, but the fix is not present in the current code.
Apply this diff to configure custom endpoints:
s3Client := s3.NewFromConfig(cfg, func(o *s3.Options) {
if insecure {
o.EndpointOptions.DisableHTTPS = true
}
+ if server != nil && server.Host != "" {
+ o.BaseEndpoint = aws.String(server.Scheme + "://" + server.Host)
+ o.UsePathStyle = true
+ }
if d.UserAgent != "" {
o.APIOptions = append(o.APIOptions, awsmiddleware.AddUserAgentKey(d.UserAgent))
}
})This ensures non-AWS endpoints are properly configured with path-style addressing.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| s3Client := s3.NewFromConfig(cfg, func(o *s3.Options) { | |
| if insecure { | |
| o.EndpointOptions.DisableHTTPS = true | |
| } | |
| if d.UserAgent != "" { | |
| o.APIOptions = append(o.APIOptions, awsmiddleware.AddUserAgentKey(d.UserAgent)) | |
| } | |
| }) | |
| s3Client := s3.NewFromConfig(cfg, func(o *s3.Options) { | |
| if insecure { | |
| o.EndpointOptions.DisableHTTPS = true | |
| } | |
| if server != nil && server.Host != "" { | |
| o.BaseEndpoint = aws.String(server.Scheme + "://" + server.Host) | |
| o.UsePathStyle = true | |
| } | |
| if d.UserAgent != "" { | |
| o.APIOptions = append(o.APIOptions, awsmiddleware.AddUserAgentKey(d.UserAgent)) | |
| } | |
| }) |
07a9b47 to
9e4292b
Compare
|
@ardaguclu: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ardaguclu, tchap The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/label qe-approved |
|
/verified by @ardaguclu @zhouying7780 |
|
@ardaguclu: This PR has been marked as verified by In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
oc depends on aws-sdk-go for s3 based operations (these are image related operations, such as mirroring to image hosted in a s3 bucket with
s3://prefix url).However, https://github.com/aws/aws-sdk-go package has been archived. It is recommended to migrate to https://github.com/aws/aws-sdk-go-v2. But this change brings about breaking changes and requires to reimplement the s3.go in oc.
This PR migrates to aws-sdk-go-v2 by applying the required changes. We need to be careful to not introduce any regressions. This PR should keep the functionality as is.