Skip to content

feat: support default credential chain for AWS Secrets Manager#182

Open
yoonhyunwoo wants to merge 1 commit into
ansible:develfrom
yoonhyunwoo:feat/support-default-credentials
Open

feat: support default credential chain for AWS Secrets Manager#182
yoonhyunwoo wants to merge 1 commit into
ansible:develfrom
yoonhyunwoo:feat/support-default-credentials

Conversation

@yoonhyunwoo
Copy link
Copy Markdown
Contributor

@yoonhyunwoo yoonhyunwoo commented Apr 28, 2026

Make the access key and secret key optional to support default credentials (e.g., pod identity)
are used without exposing keys.

Also, ensure that if only one of the two is provided,
the operation fails naturally. Fallbacks to default behavior may confuse users.

Summary by CodeRabbit

  • New Features
    • AWS Secrets Manager credential plugin now treats access key and secret key as optional, allowing authentication via alternative mechanisms (e.g., IAM roles) when credentials are not provided.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

Warning

Rate limit exceeded

@yoonhyunwoo has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 46 minutes and 10 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: cfbb33d2-3f80-469b-b55a-9a067f22c038

📥 Commits

Reviewing files that changed from the base of the PR and between 8381e38 and 665bb2f.

📒 Files selected for processing (1)
  • src/awx_plugins/credentials/aws_secretsmanager.py
📝 Walkthrough

Walkthrough

The AWS Secrets Manager plugin makes aws_access_key and aws_secret_key optional, reading them via kwargs.get() and converting missing/empty values to None before boto3 client creation. A test double in tests/azure_kv_test.py updated get_secret to require out_content_type as keyword-only and accept **kwargs of type object.

Changes

Cohort / File(s) Summary
AWS Credentials Optionality
src/awx_plugins/credentials/aws_secretsmanager.py
Removed requirement for aws_access_key and aws_secret_key; backend now uses kwargs.get(...) and converts empty/missing values to None before initializing the boto3 Secrets Manager client.
Azure test signature update
tests/azure_kv_test.py
Updated _FakeSecretClient.get_secret signature to make out_content_type keyword-only and broaden **kwargs accepted value type to object; test logic/returns unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly summarizes the main change: making AWS access key and secret key optional to support default credential chains, which is the primary objective of the PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 46 minutes and 10 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@yoonhyunwoo yoonhyunwoo force-pushed the feat/support-default-credentials branch from 5555c19 to 10391e1 Compare April 28, 2026 09:25
@yoonhyunwoo
Copy link
Copy Markdown
Contributor Author

The test failure is not caused by this change

Related:

@yoonhyunwoo yoonhyunwoo force-pushed the feat/support-default-credentials branch from 8381e38 to 09a289e Compare April 29, 2026 01:19
@yoonhyunwoo yoonhyunwoo force-pushed the feat/support-default-credentials branch from 09a289e to 665bb2f Compare April 29, 2026 01:29
@yoonhyunwoo
Copy link
Copy Markdown
Contributor Author

Rebased onto latest devel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant