Skip to content

AAP-89036: Prefer direct input value over external input_source during PATCH - #16613

Open
asjha-rh wants to merge 1 commit into
ansible:develfrom
asjha-rh:fix/AAP-89036-credential-input-source-override
Open

asjha-rh wants to merge 1 commit into
ansible:develfrom
asjha-rh:fix/AAP-89036-credential-input-source-override

Conversation

@asjha-rh

@asjha-rh asjha-rh commented Aug 21, 2026

Copy link
Copy Markdown

Summary

  • Fixes a bug where Credential.get_input() unconditionally delegates to the external credential backend (e.g. HashiCorp Vault) when an input_source is attached, even when a direct value is being provided via PATCH.
  • This caused HTTP 500 if the backend was unreachable, or silently discarded the new value if the backend was reachable.
  • The fix checks for a non-placeholder direct value in self.inputs before falling through to _get_dynamic_input(), allowing PATCH requests to override externally-sourced fields.

Issue

AAP-89036

Root Cause

In awx/main/models/credential.py, get_input() checks dynamic_input_fields first and always routes to _get_dynamic_input() which makes a live network call to the external backend. There was no guard to check whether a direct value was being supplied in an update context.

Changes

  • awx/main/models/credential.py: Modified get_input() to check if a direct (non-placeholder) value exists in self.inputs before delegating to the dynamic input source. If a direct value is present, it is returned (with decryption for secret fields), bypassing the external backend call.
  • awx/main/tests/functional/test_credential.py: Added two regression tests:
    • test_get_input_prefers_direct_value_over_dynamic_source — verifies direct values take precedence
    • test_get_input_falls_back_to_dynamic_when_no_direct_value — verifies the dynamic path still works when no direct value is present

Test plan

  • New unit tests pass
  • Manual test: Create credential with Vault input_source, PATCH with new direct ssh_key_data — should succeed without contacting Vault
  • Manual test: Credential with input_source and no direct override — should still fetch from Vault as before
  • Existing credential test suite passes

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes

    • Credential values now prioritize directly provided inputs over dynamically retrieved values.
    • Secret inputs are decrypted when possible, with graceful fallback to the provided value if decryption fails.
    • Dynamic input retrieval remains available when no direct value is supplied.
  • Tests

    • Added regression coverage for direct input handling and dynamic lookup fallback.

…g PATCH

When a credential has an input_source (e.g. HashiCorp Vault) attached
to a field, get_input() unconditionally delegates to the external
backend — even when a direct value is being provided via PATCH. This
causes HTTP 500 if the backend is unreachable, or silently discards the
new value if it is reachable.

Fix get_input() to check for a non-placeholder direct value in
self.inputs before falling through to _get_dynamic_input(). This lets
PATCH requests override an externally-sourced field without triggering
a live call to the external credential backend.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7b047287-d1e9-4feb-a45d-3a93b51cb953

📥 Commits

Reviewing files that changed from the base of the PR and between 2a73b96 and fa1729e.

📒 Files selected for processing (2)
  • awx/main/models/credential.py
  • awx/main/tests/functional/test_credential.py

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Credential.get_input now prioritizes direct inputs, decrypts secret values when possible, and falls back to dynamic input resolution when no direct value exists. Functional tests cover direct-value precedence and dynamic fallback.

Changes

Credential input resolution

Layer / File(s) Summary
Direct input precedence and fallback validation
awx/main/models/credential.py, awx/main/tests/functional/test_credential.py
Credential.get_input returns eligible direct inputs before dynamic lookup, decrypts secret values when possible, and preserves dynamic resolution when no direct value exists. Regression tests verify both behaviors and confirm when dynamic lookup is not called.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to fa172

PATCH requests can now use a supplied direct credential value while credentials without an override continue using the external source; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: fosterseth

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the issue and the main change: preferring direct input values over external input sources during PATCH requests.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.
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.
✨ 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

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant