Skip to content

AAP-52517: Fix project.py: allow clearing credential with empty value - #16622

Open
sussen-redhat wants to merge 1 commit into
ansible:develfrom
sussen-redhat:fix-empty-credential-project
Open

sussen-redhat wants to merge 1 commit into
ansible:develfrom
sussen-redhat:fix-empty-credential-project

Conversation

@sussen-redhat

@sussen-redhat sussen-redhat commented Aug 26, 2026

Copy link
Copy Markdown
SUMMARY

This addresses the same root cause reported in #14843 (empty value not
clearing a credential field), but for the project module's credential
and signature_validation_credential fields rather than job_template's
webhook_credential.

Both fields were always passed through resolve_name_to_id() when not
None, even when the value was an empty string (""). This caused the
module to try to look up a credential literally named "" and fail with
"Request to /api/controller/v2/credentials/ returned N items, expected 1",
instead of treating an empty string as "clear this field" (which the UI
already supports).

This change treats a falsy-but-not-None value (i.e. "") as an explicit
request to clear the field, setting it to null instead of attempting name
resolution.

Note: this also changes behavior for default_environment (the third
field in the same loop) — passing default_environment: "" will now
also clear that field rather than attempting to resolve "" as an
execution environment name. This is consistent with the credential
fields above and matches expected UI behavior, but is called out
explicitly here since it wasn't the field originally reported in #14843.

A separate, module-specific fix will still be needed for #14843 itself
(job_template's webhook_credential) and for the related reports on the same
resolver behavior: #15844, #15846, #16167.

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • Collection
STEPS TO REPRODUCE AND EXTRA INFO

Before the fix, running the project module with credential: "" on a
project that already had a credential set would fail trying to resolve
"" as a credential name, instead of clearing the field.

Added test_clear_project_credential to test_project.py, which:

  1. Creates a project with a real SCM credential attached
  2. Re-runs the module with credential: ''
  3. Asserts the call succeeds and the credential is actually cleared in the
    database

Full collection test suite passes (202/202 in test/awx/), and
ansible-test sanity --python 3.11 passes cleanly on the changed module
(plugins/modules/project.py).

@github-actions github-actions Bot added component:awx_collection issues related to the collection for controlling AWX community labels Aug 26, 2026
@coderabbitai

coderabbitai Bot commented Aug 26, 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: c13f661a-5fa3-436e-b0b4-26b6e9ccac36

📥 Commits

Reviewing files that changed from the base of the PR and between 0c342ef and dd0e23b.

📒 Files selected for processing (1)
  • awx_collection/plugins/modules/project.py

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


📝 Walkthrough

Walkthrough

The project module now converts falsey association values to None. A Django test verifies that an empty SCM credential clears the credential assigned to an existing project.

Changes

Project credential clearing

Layer / File(s) Summary
Handle and validate cleared associations
awx_collection/plugins/modules/project.py, awx_collection/test/awx/test_project.py
Falsey association values now clear the association instead of triggering lookup. The test covers assigning and then clearing a project SCM credential.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to dd0e2

The project module now treats an empty credential value as a request to clear the credential, with focused tests covering the behavior; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: allowing project credentials to be cleared with an empty value.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@sussen-redhat
sussen-redhat force-pushed the fix-empty-credential-project branch 2 times, most recently from bd9375b to 1585302 Compare August 26, 2026 11:15
Signed-off-by: sussen-redhat <sussen@redhat.com>
@sussen-redhat
sussen-redhat force-pushed the fix-empty-credential-project branch from 1585302 to dd0e23b Compare August 27, 2026 05:28
@sussen-redhat sussen-redhat changed the title Fix project.py: allow clearing credential with empty value [ACA-4558] Fix project.py: allow clearing credential with empty value Aug 27, 2026
@sussen-redhat sussen-redhat changed the title [ACA-4558] Fix project.py: allow clearing credential with empty value [AAP-52517Fix project.py: allow clearing credential with empty value Aug 27, 2026
@sussen-redhat sussen-redhat changed the title [AAP-52517Fix project.py: allow clearing credential with empty value AAP-52517: Fix project.py: allow clearing credential with empty value Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community component:awx_collection issues related to the collection for controlling AWX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant