Migrate to aws-sdk-go-v2 (fixes #555, resolves CVE scanner noise) - #634
Open
xoanmm wants to merge 8 commits into
Open
Migrate to aws-sdk-go-v2 (fixes #555, resolves CVE scanner noise)#634xoanmm wants to merge 8 commits into
xoanmm wants to merge 8 commits into
Conversation
- Session: require http/https scheme (url.Parse accepts foobar:1234 as scheme=foobar, which silently passed the previous check). - session_test: switch to t.Setenv so failed tests do not leak HELM_S3_REGION/AWS_ENDPOINT into other tests; drop unused os import; update expected cn-test-bucket region from ap-southeast-2 to eu-central-1 to match current bucket location returned by GetBucketLocation. - hack/test-e2e-local.sh: use http://localhost:9000 as AWS_ENDPOINT to satisfy the new scheme validation. - tests/e2e/main_test: strip scheme from AWS_ENDPOINT before passing to minio.New, which expects a bare host:port. - go.mod/go.sum: go mod tidy (promotes golang.org/x/text to direct). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Ran `go get -u ./... && go mod tidy` against Go 1.26.3 toolchain. Notable upgrades: - helm.sh/helm/v3 v3.21.0 -> v3.21.3 - k8s.io/* v0.35.1 -> v0.36.2 - golang.org/x/crypto v0.49.0 -> v0.54.0 - golang.org/x/net v0.52.0 -> v0.57.0 - google.golang.org/grpc v1.80.0 -> v1.82.1 - klauspost/compress v1.18.0 -> v1.19.1 - sirupsen/logrus v1.9.3 -> v1.9.4 Reduces open CVE surface. Unit + e2e (minio) tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Go: 1.25.5 -> 1.25.12 (latest 1.25.x patch) - alpine/helm: 3.21.0 -> 3.21.3 (latest 3.21.x patch) Reduces CVE surface. Local docker build not verified due to network TLS interception; verify in CI. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
|
@hypnoglow could you review please? |
Author
|
Hello @hypnoglow any chance to review it? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates helm-s3 from the end-of-life
aws-sdk-gov1 toaws-sdk-go-v2, and refreshes remaining Go dependencies and Docker base images. Closes #555.Builds on top of the excellent prior work in #571 (@monwolf) and #578 (@alekc) — both stale ~6 months — by resolving conflicts against current
main, fixing a few tests, and adding a full dep refresh.Motivation
Two independent drivers:
AWS SDK for Go v1 reached end of support on 2025-07-31.
See AWS announcement. No more security patches from AWS — every future CVE against v1 will remain unfixed upstream.
CVE scanner noise on every downstream helm-s3 install.
Image scanners (Grype/Trivy/etc.) currently flag CVE-2020-8911 and CVE-2020-8912 against
github.com/aws/aws-sdk-go v1.55.7in the shippedhelm-s3binary. Both live inservice/s3/s3crypto(CBC padding oracle in client-side encryption). helm-s3 does not imports3crypto(verified:grep -rn "s3crypto"= 0 hits — helm-s3 only uses server-side SSE via theAWS_S3_SSEheader), so the CVEs are technically non-exploitable here. But because these CVEs haveFixed In: NotAvailablein v1 (they were only fixed by moving to v2), every downstream user has to either waive them per-scanner or accept persistent noise. Migrating to v2 makes both CVEs disappear from every downstream scan simultaneously.Between v1 EOL and the CVE noise, the migration is now a shared blocker for anyone shipping helm-s3 inside a compliance-scanned container image.
Changes
AWS SDK v2 migration (rebased from #578)
internal/awsutil/session.go—session.Session→aws.Config+config.LoadDefaultConfig;HELM_S3_DYNAMIC_REGION_ENABLEDpreservedinternal/awsutil/url.go—EscapePathreimplemented (v1'sprivate/protocol/rest.EscapePathhas no v2 public equivalent)internal/awss3/storage.go— S3 client ops migrated to v2 context-native APIs;s3manager→feature/s3/manager;awserr.Error/.Code()→errors.Aswith*types.NoSuchBucket/*types.NoSuchKey/*smithy.GenericAPIErrorcmd/helm-s3/{delete,download,init,push,reindex}.go— session return-type touch-ups4cec5dd)Fixes on top of the rebased #578
url.Parse("foobar:1234")returnsScheme="foobar", so the previousparsedURL.Scheme == ""check silently passed invalid input. Tightened to requirehttporhttps.os.Setenv/os.Unsetenvtot.Setenvinsession_test.go. At.FatalfinTestSessionWithInvalidEndpointwas leakingHELM_S3_REGION=us-west-2intoTestDynamicBucketRegion, causing false failures.TestDynamicBucketRegion: updated expected region forcn-test-bucketfromap-southeast-2→eu-central-1. v2 usesGetBucketLocationwhich returns the bucket's actual current region; the demo bucket appears to have moved.hack/test-e2e-local.sh:AWS_ENDPOINT=localhost:9000→http://localhost:9000to satisfy the new scheme check.tests/e2e/main_test.go:minio.Newrequires a barehost:port, so strip the scheme when reusingAWS_ENDPOINTfor the test-side minio client.Dependency refresh
go get -u ./... && go mod tidy— notablyhelm.sh/helm/v33.21.0 → 3.21.3,k8s.io/*0.35.1 → 0.36.2,golang.org/x/crypto0.49.0 → 0.54.0,google.golang.org/grpc1.80.0 → 1.82.1.Docker base images
GO_VERSION1.25.5 → 1.25.12 (latest 1.25.x patch)HELM_VERSION3.21.0 → 3.21.3-mod=vendoralready dropped by Feature/aws sdk v2 #578's33956bb(no vendor/ directory is checked in)Test plan
go build ./cmd/helm-s3— clean compilego vet ./...— cleango test -short -count=1 ./internal/...— all pass (awsutil,helmutil)hack/test-e2e-local.sh— 19/19 tests pass against local minio + docker (basic delete, delete--relative, delete provenance, delete multi-version, delete multi-version one-missing, helm dependency update, init, init--force, init--ignore-if-exists, init both flags, init repo-not-found, push, push--content-type, push--dry-run, push--force, push--ignore-if-exists, push both flags, push--relative, push provenance)Commits
Rebased from #578 (preserving @monwolf and @alekc authorship):
feat: migrate to AWS SDK v2fix: remove deprecated depsMinor tweaks for aws sdk v2 migrationchore: update Go version to 1.25.5 in Dockerfile, fix for --vendoring crashfeat: add support for custom AWS endpoint with scheme validationAdded on top:
fix(awsutil): tighten endpoint scheme check + stabilize testschore(deps): bump Go dependencies to latest compatible with Go 1.25.5chore(docker): bump base image versionsHappy to squash / split / rebase however you prefer.