feat: add AWS Secrets Manager provider#2402
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds AWS Secrets Manager configuration and a resolver supporting regional clients, version options, binary values, and JSON field extraction. Updates schemas, dependencies, tests, provider registration assertions, documentation, and chatbridge monitor test timeouts. ChangesAWS Secrets Manager support
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Adds an AWS Secrets Manager-backed secret resolver to Dagu’s secret management subsystem, integrating with existing provider registry and configuration loading so DAGs can resolve runtime secrets via AWS SDK default credentials.
Changes:
- Added
aws-secrets-managersecret provider with support for name/ARN lookups, version selectors, binary values, and optional JSON field extraction. - Extended global config + schema to include
secrets.aws.region(andDAGU_SECRETS_AWS_REGION) as a default AWS region for secret lookups. - Added unit tests for the new resolver and JSON field selection helper; updated provider registry tests and README.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates documented secret management providers list to include AWS Secrets Manager. |
| internal/cmn/secrets/resolver_test.go | Updates provider registry tests to expect the new AWS provider. |
| internal/cmn/secrets/field.go | Adds helper for extracting a top-level field from a JSON secret payload. |
| internal/cmn/secrets/field_test.go | Adds tests for JSON field extraction behavior and error safety. |
| internal/cmn/secrets/aws_secrets_manager.go | Implements the AWS Secrets Manager resolver and client caching. |
| internal/cmn/secrets/aws_secrets_manager_test.go | Adds tests for AWS resolver behavior, options, and caching. |
| internal/cmn/schema/config.schema.json | Extends config schema with secrets.aws.region. |
| internal/cmn/config/loader.go | Loads secrets.aws.region into runtime config and binds env var. |
| internal/cmn/config/loader_test.go | Tests YAML/env loading for the new AWS secrets config section. |
| internal/cmn/config/definition.go | Adds mapstructure definition types for secrets.aws.*. |
| internal/cmn/config/config.go | Adds runtime config struct for AWS secrets defaults. |
| go.mod | Adds AWS SDK v2 modules required for Secrets Manager integration. |
| go.sum | Updates dependency checksums for the new AWS SDK modules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/cmn/secrets/aws_secrets_manager.go`:
- Around line 104-124: Update awsSecretsManagerResolver.getClient to avoid
calling factory(ctx, region) while holding r.mu: first check r.clients under the
lock, release it, create the client outside the lock, then reacquire the lock
and recheck the region before caching and returning the existing or newly
created client. Preserve the existing error wrapping and ensure concurrent
initialization cannot overwrite the client selected in the second check.
In `@internal/cmn/secrets/field.go`:
- Around line 31-39: Update the JSON handling in the visible field-processing
function to validate raw JSON without unmarshaling into any or re-marshaling it;
return the original json.RawMessage content directly after validation,
preserving exact numeric representation and formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29ce3df0-8c99-48c4-983c-c1b48c7703ba
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
README.mdgo.modinternal/cmn/config/config.gointernal/cmn/config/definition.gointernal/cmn/config/loader.gointernal/cmn/config/loader_test.gointernal/cmn/schema/config.schema.jsoninternal/cmn/secrets/aws_secrets_manager.gointernal/cmn/secrets/aws_secrets_manager_test.gointernal/cmn/secrets/field.gointernal/cmn/secrets/field_test.gointernal/cmn/secrets/resolver_test.go
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
Testing
Part of #798
Summary by cubic
Adds an AWS Secrets Manager secret provider for DAG secrets using AWS SDK default credentials. Adds a global default region, strict ARN/region validation, and a non-blocking per‑region client cache.
New Features
aws-secrets-manager.version_idandversion_stage.region, globalsecrets.aws.region, orDAGU_SECRETS_AWS_REGION; blocks ARN/region mismatches.field.Bug Fixes
version_idandversion_stageexactly as provided; normalize only key andregion.Written for commit 9c781aa. Summary will update on new commits.
Summary by CodeRabbit
secrets.aws.region, per-secret region overrides,version_id/version_stage, binary secrets, and selecting a specific JSON field from a secret value.DAGU_SECRETS_AWS_REGION.