refactor: drop direct AWS SDK use from workflow core#744
Merged
Conversation
Workflow core no longer imports any aws-sdk-go-v2/service/*
package directly. The remaining AWS-SDK presence in go.mod is
purely indirect (modular/eventbus/v2 pulls sts + kinesis, which
is modular's concern).
Deleted (all unused by the rest of workflow; workflow-plugin-aws
is the in-tree replacement for IaC):
- provider/aws/ — full directory (plugin.go, clients.go,
deploy.go, registry.go, _test.go). cmd/server/main.go's blank
side-effect import dropped along with it.
- artifact/s3.go — S3Store impl. No callers (artifact pipeline
steps use the Store interface; FS impl in artifact/local.go is
the only concrete impl now).
- iam/aws.go — AWSIAMProvider. api/router.go's resolver
registration call removed; KubernetesProvider + OIDCProvider
remain. Plugin-side replacement landed in workflow-plugin-aws.
- plugin/rbac/aws.go — dead code; no external importers.
plugin/rbac/aws_test.go + the TestAWSIAMProvider_NameCheck
test in builtin_test.go also removed.
- iam/providers_test.go AWSProvider subtests + AWS branch of
the RegisterProvider test (latter rewired to KubernetesProvider).
Dependency footprint, before → after (direct AWS imports):
github.com/aws/aws-sdk-go-v2 6 direct
github.com/aws/aws-sdk-go-v2/config 1 direct
github.com/aws/aws-sdk-go-v2/credentials 1 direct
github.com/aws/aws-sdk-go-v2/service/cloudwatch 1 direct
github.com/aws/aws-sdk-go-v2/service/ecs 1 direct
github.com/aws/aws-sdk-go-v2/service/eks 1 direct
github.com/aws/aws-sdk-go-v2/service/iam 1 direct
github.com/aws/aws-sdk-go-v2/service/s3 1 direct
github.com/aws/aws-sdk-go-v2/service/sts 1 direct
— every one now indirect.
Verification:
GOWORK=off go test ./... — all 138 packages green
GOWORK=off go mod why aws-sdk-go-v2/service/{cloudwatch,ecs,eks,iam,s3}
— "main module does not need package …" for each
The kinesis + sts indirect pulls come from
modular/modules/eventbus/v2; reducing those is a modular-side
concern (the eventbus has multiple backends and pulls every SDK).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Removes remaining in-repo AWS SDK v2 service usage from the workflow core by deleting the now-dead AWS provider / IAM / S3 artifact implementations, and updating module dependencies accordingly. This aligns the core repo with the intended AWS feature split where AWS-specific functionality lives in workflow-plugin-aws.
Changes:
- Deleted the in-core AWS provider (
provider/aws/) and its associated tests. - Deleted AWS IAM/RBAC provider implementations (
iam/aws.go,plugin/rbac/aws.go) and updated router/tests to no longer register/test them. - Dropped direct AWS SDK v2 service dependencies from
go.mod/go.sum(leaving only indirect AWS SDK deps via other modules).
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| provider/aws/registry.go | Deleted unused ECR registry stubs. |
| provider/aws/plugin.go | Deleted AWS provider implementation that depended on AWS SDK services. |
| provider/aws/plugin_test.go | Deleted tests for the removed AWS provider. |
| provider/aws/deploy.go | Deleted ECS/EKS deploy implementation. |
| provider/aws/deploy_test.go | Deleted deploy-path tests for the removed AWS provider. |
| provider/aws/clients.go | Deleted AWS SDK client interface definitions. |
| plugin/rbac/builtin_test.go | Removed AWS IAM provider name check test. |
| plugin/rbac/aws.go | Deleted AWS IAM-backed RBAC permission provider implementation. |
| plugin/rbac/aws_test.go | Deleted tests for the removed AWS RBAC provider. |
| iam/providers_test.go | Removed AWS IAM provider tests and updated resolver registration test. |
| iam/aws.go | Deleted AWS IAM provider implementation (STS/IAM-based). |
| artifact/s3.go | Deleted unused S3-backed artifact store implementation. |
| api/router.go | Stopped registering AWS IAM provider in the default IAM resolver. |
| cmd/server/main.go | Removed blank import that previously registered the AWS provider side-effectfully. |
| go.mod | Removed direct AWS SDK service requirements; retained AWS SDK deps as indirect where needed. |
| go.sum | Removed checksums for AWS SDK service modules no longer required. |
Comment on lines
185
to
191
| resolver := iamResolver | ||
| if resolver == nil { | ||
| resolver = iam.NewIAMResolver(stores.IAM) | ||
| resolver.RegisterProvider(&iam.AWSIAMProvider{}) | ||
| // AWSIAMProvider extracted to workflow-plugin-aws; register | ||
| // it from the plugin side if needed. | ||
| resolver.RegisterProvider(&iam.KubernetesProvider{}) | ||
| resolver.RegisterProvider(&iam.OIDCProvider{}) |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Comment on lines
185
to
191
| resolver := iamResolver | ||
| if resolver == nil { | ||
| resolver = iam.NewIAMResolver(stores.IAM) | ||
| resolver.RegisterProvider(&iam.AWSIAMProvider{}) | ||
| // AWSIAMProvider extracted to workflow-plugin-aws; register | ||
| // it from the plugin side if needed. | ||
| resolver.RegisterProvider(&iam.KubernetesProvider{}) | ||
| resolver.RegisterProvider(&iam.OIDCProvider{}) |
Copilot flagged the prior comment as misleading — it suggested registration "from the plugin side" without naming the integration path. Replace with an honest note: there is no in-process AWS provider after this PR; an embedder that needs AWS IAM must construct its own iam.IAMResolver, register an AWS-aware iam.Provider impl on it, and pass it as the iamResolver argument to NewRouterWithIAM (rather than relying on the default-resolver branch). No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| // AWS IAM provider removed from workflow core; no in-process | ||
| // replacement is wired here. To re-enable AWS IAM, the | ||
| // embedder must construct an iam.IAMResolver, register an | ||
| // AWS-aware iam.Provider impl on it (e.g., from |
Copilot flagged the prior comment using `iam.Provider`; the actual type in `iam/provider.go` is `iam.IAMProvider`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The example/ sub-module had stale indirect entries for aws-sdk-go-v2/service/iam, internal/checksum, internal/s3shared, and service/s3 that were only pulled transitively via the deleted provider/aws + iam/aws.go + artifact/s3.go. Tidy now that those direct uses are gone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| @@ -44,7 +44,6 @@ import ( | |||
| allplugins "github.com/GoCodeAlone/workflow/plugins/all" | |||
| pluginpipeline "github.com/GoCodeAlone/workflow/plugins/pipelinesteps" | |||
| "github.com/GoCodeAlone/workflow/provider" | |||
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Copilot round-4 flagged that: - CICD_PLAN.md still listed provider/aws/* under "Provider Plugins" and "Files to create (Phase 6)" without acknowledging the extraction. - docs/migrations/2026-05-15-plugin-modules-on-iac.md described provider/aws/ as a deliberate AWS SDK retention seam and a go.mod-anchor for aws-sdk-go-v2. Both docs are historical (plan + phase-B/C migration); leaving them stale would mislead future readers. Added inline "Update 2026-05-20 (PR #744)" callouts that: - redirect CICD_PLAN.md readers to the migration doc for current state, - update the Phase B verification claim to say aws-sdk-go-v2 is now indirect-only (via modular/eventbus/v2 sts+kinesis), - update the final invariant statement to drop provider/aws/ from the list of retained provider-specific surfaces. No code change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Workflow core no longer imports any
aws-sdk-go-v2/service/*package directly. Five files / directories are deleted; the remaining AWS-SDK pulls ingo.modare purely indirect viamodular/eventbus/v2.Deleted (all dead in workflow core; replacement is
workflow-plugin-aws):provider/aws/(entire dir)cmd/server/main.goblank import. No call sites.artifact/s3.go(S3Store)Storeinterface;artifact/local.gois the only concrete impl now.iam/aws.go(AWSIAMProvider)api/router.goregistration call removed;KubernetesProvider+OIDCProviderremain.plugin/rbac/aws.go+plugin/rbac/aws_test.goiam/providers_test.goAWS subtests;plugin/rbac/builtin_test.goTestAWSIAMProvider_NameCheckDirect AWS imports before → after: 9 → 0.
The kinesis + sts indirect pulls come from
modular/modules/eventbus/v2; reducing those is a modular-side concern (the eventbus has multiple backends and pulls every SDK).Test plan
GOWORK=off go test ./...— all 138 packages greenGOWORK=off go build ./...— cleanGOWORK=off go mod why aws-sdk-go-v2/service/{cloudwatch,ecs,eks,iam,s3}— "main module does not need package …" for eachgofmt -l .— clean🤖 Generated with Claude Code