* Add custom last_job_host_summary__failed filter to HostList
The denormalized last_job_host_summary FK was removed (AAP-68024), so
filtering by ?last_job_host_summary__failed=True on /v2/hosts/ would
400 or return stale results. Add custom handling that annotates the
queryset with a Subquery on JobHostSummary (same pattern used by the
dashboard counts) and reserves the param names from FieldLookupBackend
so the generic filter skips them.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Extend last_job_host_summary failed filter to inventory and group host lists
Build on AlanCoding's HostList filter (ansible/awx#16698) by moving the
Subquery-based latest-summary logic into HostRelatedSearchMixin so
InventoryHostsList, GroupHostsList, and GroupAllHostsList honor the same
query params as the global hosts list.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address review: validate filter values and apply both summary filters
Require True for last_job_host_summary failed query params, apply failed
and not-failed filters independently (AND semantics), add GroupHostsList
coverage, and document mixin helpers.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
test_bulk_host_create_num_queries budgets 15 queries per request. The
first request in a pytest process pays one extra query for each
database-backed setting it reads (PROXY_IP_ALLOWED_LIST,
REMOTE_HOST_HEADERS, BULK_HOST_MAX_CREATE) until the settings
wrapper's in-memory cache is warm, so the budget only held when an
earlier test in the same xdist worker had already warmed it. Make one
untimed request first so the assertion measures steady state.
* AAP-96085: Decouple collection cache from project cache_id
Introduce a requirements-hash-keyed cache (reqcache_<hash>/) so that
ansible-galaxy collection/role installs are reused across project commits
when requirements files haven't changed. Previously, any commit to the
project repo invalidated the entire cache, triggering a full reinstall
of all collections — a single collection like community.general 11.0.0
(~3,500 files) adds 40-90s of I/O on SAN-backed storage.
* Address review feedback: atomic cache publish, bounded eviction, path-aware hash
- Atomic publish: _sync_requirements_cache() copies into a temp directory
and renames into place, so a failed copytree cannot leave a partial
reqcache directory that looks valid to subsequent cache-hit checks
- Bounded eviction: clear_project_cache() evicts the oldest reqcache_*
directories when the count exceeds max_reqcache (default 10), preventing
unbounded storage growth from repeated requirements changes
- Path-aware hash: requirements_hash now includes file paths alongside
checksums via zip(), so swapping content between requirements files
correctly invalidates the cache
* Protect current reqcache from eviction during cleanup
Read the requirements hash from stage/.requirements_hash and exclude
the matching reqcache_* entry from age-based eviction. Without this,
a cache hit on the oldest entry could be evicted before the job uses it.
* AAP-96176: Cache tarball for make_local_copy() to reduce small-file I/O
After populating reqcache_<hash>/, create a tarball of each subfolder
(requirements_collections.tar, requirements_roles.tar). make_local_copy()
prefers the tarball when available — copies and extracts one file instead
of shutil.copytree() across potentially thousands of individual collection
files. Falls back to copytree when no tarball exists (upgrade compatibility).
On SAN-backed storage, this significantly reduces per-job copy I/O by
eliminating per-file source reads.
* Fix direct-cache tarball cross-reference and partial extraction fallback
Remove reqcache tarball lookup on direct cache hits — the direct cache
could diverge from the reqcache (e.g. galaxy server changes not in the
requirements hash), serving stale content. The tarball optimization
still works for reqcache fallback hits where cache_subpath + '.tar'
resolves directly.
Clean up partially extracted destination before copytree fallback so a
failed tarball extraction doesn't leave behind a directory that causes
FileExistsError.
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* AAP-95887 Use SKIP LOCKED to prevent lock convoy on parent JT update
When allow_simultaneous is enabled and many jobs complete concurrently,
_update_parent_instance() serializes all sessions on the same JT row
lock. In an example customer incident, single-row UPDATEs averaged
11-25 minutes with 635+ sessions queued.
Use select_for_update(skip_locked=True) for the on_commit callback
path so concurrent updaters that cannot acquire the row lock return
immediately instead of queuing. This is safe because all fields on the
JT (current_job, status, last_job, last_job_failed) are "latest wins"
— at least one session always acquires the lock and writes the current
state.
Non-allow_simultaneous jobs are unchanged (synchronous, no skip).
* Skip lock contention test on SQLite
select_for_update(skip_locked=True) is silently a no-op on SQLite.
Mark the lock contention test to skip when running against SQLite
(the default test database). The test runs when tests are executed
against PostgreSQL.
* Use mock for lock contention test instead of threading
The test suite uses SQLite where select_for_update is a no-op.
Follow codebase convention (e.g. sqlite_copy fixture) of mocking
PostgreSQL-specific behavior: mock select_for_update to return an
empty queryset, simulating a locked row. Removes the separate
test class and threading/psycopg dependencies.
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
The Black -> Ruff migration (AAP-85334) installs a bare, unpinned
`ruff` in four places: requirements_dev.txt, both tox.ini linters
envs, and the devel image Dockerfile. CI's api-lint job resolves the
latest release on every run.
Ruff treats minor releases as breaking by policy: that is where the
yearly formatter style guide lands (0.9.0, 0.15.0) and where rules are
promoted or change behavior. Left unpinned, the next minor turns
unrelated PRs and quiet stable branches red with no code change.
Pin to ~=0.16.0 everywhere, and add `required-version` to
[tool.ruff] so a developer's mismatched local ruff (pre-commit.sh uses
whatever is on PATH) is refused instead of silently reformatting.
Patch releases within 0.16.x still flow, so bug fixes arrive without
a bump; the floor moves through a reviewed PR when 0.17 lands.
Claude-Session: https://claude.ai/code/session_011rb7ruHixj48khUNXxxqPG
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
* Update schema to add pattern fields for credential types
* Update pattern field to be camel case to match pattern field on OPTIONS
* Fix lint issues
* Add unit tests for clean text pattern hook
* [AAP-93690] Add CleanText validation and OPTIONS patterns to subscriptions endpoint
Add SubscriptionCredentialsSerializer (PlainSerializerCleanTextMixin) for
/api/v2/config/subscriptions/ so POST requests validate credential string
fields through the CleanText enforcement pipeline.
- subscriptions_client_id, subscriptions_username: Tier 2 (free-text)
- subscriptions_client_secret, subscriptions_password: excluded (secrets)
- name_fields = frozenset() ensures Tier 2, not Tier 1
- Fake Meta.model satisfies CleanTextMixin audit logging and OPTIONS metadata
- ApiV2SubscriptionView gains serializer_class and get_serializer() so the
existing metadata machinery advertises validation patterns on OPTIONS
- post() runs serializer.is_valid() before credential logic, preserving all
existing error messages, status codes, and $encrypted$ handling
- Functional tests: unsafe-input rejection, secret exclusion, safe passthrough,
OPTIONS pattern presence/absence, all existing happy/error paths still pass
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Chai Bot <chai-bot@redhat.com>
* [AAP-93690] Preserve credential whitespace and improve docstring coverage
DRF CharField strips leading/trailing whitespace by default, which
silently altered subscription credentials before passing them to
validate_rh. Set trim_whitespace=False on all four credential fields
so serializer.validated_data preserves the exact submitted values.
Add four regression tests proving whitespace is passed unchanged to
validate_rh for username, password, client_id, and client_secret.
Add docstrings to concrete_model(), check_permissions(), post(), and
the ApiV2SubscriptionView class to meet the 80% docstring coverage
threshold.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Chai Bot <chai-bot@redhat.com>
* Accept explicit JSON null on SubscriptionCredentialsSerializer fields
DRF CharField rejects explicit JSON null by default, causing a 400
"This field may not be null" error when a frontend sends null for the
unused credential pair (e.g. client_id=null, client_secret=null with a
valid username/password). Add allow_null=True on all four optional
fields so the serializer passes null through to the view, where the
existing falsy-check credential-selection logic already handles it
correctly.
Includes a focused regression test proving a valid username/password
pair with null client fields is accepted and calls validate_rh with
the basic-auth pair.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Chai Bot <chai-bot@redhat.com>
* fix(subscriptions): supply request/view context to SubscriptionCredentialsSerializer
ApiV2SubscriptionView.get_serializer() now injects the standard DRF
serializer context (request, format, view) so that CleanTextMixin's
_log_validation_failure() can attribute rejected input to the
authenticated user and client IP.
post() is updated to call self.get_serializer(data=request.data)
instead of constructing the serializer class directly, ensuring
consistent context propagation.
A focused regression test asserts that the CleanText warning emitted
for unsafe subscription input includes the authenticated username
and client IP address.
Signed-off-by: redhat-chai-bot <redhat-chai-bot@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: redhat-chai-bot <redhat-chai-bot@users.noreply.github.com>
* remove jira ref, decouple test from DAB working
---------
Signed-off-by: Chai Bot <chai-bot@redhat.com>
Signed-off-by: redhat-chai-bot <redhat-chai-bot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Add ability for a restarted controller to reconnect to in-progress work units
on the execution environment and replay their events, eliminating the need
to re-run jobs after controller restart in hybrid/mesh deployments.
Key changes:
1. Extract _process_phase() so it can be called standalone without re-transmitting
artifacts. Unified per-job loops consolidate startup/heartbeat processing
with explicit adoption/error branches for each job.
2. Add reattach_to_work_unit() to reconnect and stream events from a completed
receptor work unit. Uses hybrid memory-safe dedup (safe_threshold + collision_zone)
to avoid re-sending events already in the database to the callback receiver.
3. Unify job finalization: extract _finalize_job_run() shared between normal
execution and adoption paths. Extract _configure_runner_callback() as the
canonical callback factory used by both paths.
4. Move _receptor_release_work() after terminal status is committed to DB,
eliminating the race where a kill between release and commit left jobs
stuck in 'running' without adoptable work units.
5. Add adopt_job_async @task to run adoption in background, enabling real-time
event streaming from the EE during controller restart recovery.
Cluster-verified across 9 scenarios with all event stream data checked.
Unit and functional tests cover all adoption paths and edge cases.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
* Fix 500 error on /api/v2/schedules/ from orphaned token cleanup schedule
delete_clear_tokens_sjt() relied on cascade to remove the associated
schedule when deleting the cleanup_tokens SystemJobTemplate. In migration
context with polymorphic models, the cascade did not reliably fire,
leaving an orphaned schedule whose unified_job_template referenced a
missing row.
Explicitly delete associated schedules and clear stale next_schedule
references before deleting the SJT. Add defensive handling in
ScheduleSerializer.get_related() and get_summary_fields() so orphaned
schedules degrade gracefully instead of raising RelatedObjectDoesNotExist.
Fix the existing test assertion that used a JOIN-based query incapable of
detecting orphans, and add coverage for next_schedule reference cleanup.
Fixes: AAP-92564
* Add migration to clean up orphaned token cleanup schedules
Databases that already ran migration 0204 may have orphaned "Cleanup
Expired OAuth 2 Tokens" schedules left behind by the broken cascade.
Remove them and clear any stale UnifiedJobTemplate.next_schedule
references. Add a test that simulates the orphan scenario by deleting
the UJT via raw SQL and verifying the migration repairs it.
Ref: AAP-92564
* Re-parent migration 0212 for gateway backport to stable-2.7
Apply the gateway backport migration pattern (AAP-87599): re-parent
0212_cleanup_orphaned_token_schedules to depend on 0207_merge (the
common ancestor with tower/stable-2.7) instead of 0211. Add
0213_merge_0211_0212 to converge the parallel paths on devel.
This allows the backport to tower/stable-2.7 to use the identical
0212 migration file with no changes.
Ref: AAP-92564
* Test next_schedule cleanup on a surviving template
Use the cleanup_sessions SystemJobTemplate (which survives the deletion)
as the next_schedule reference holder, so the assertion actually proves
the UPDATE ... SET next_schedule = NULL logic works rather than being
masked by the referenced UJT itself being deleted.
Ref: AAP-92564
* Fix test: create surviving SJT instead of relying on migration data
The CI test DB does not have the cleanup_sessions SystemJobTemplate
from migration 0078. Use get_or_create to ensure the surviving UJT
exists within the test itself.
Ref: AAP-92564
* Renumber cleanup migration to 0208 and bump 0208-0211 to 0209-0212
Replace the gateway backport pattern (re-parented migration + merge
migration) with a simpler approach: slot the cleanup migration as 0208
directly after the merge point, and renumber the subsequent migrations.
This keeps the migration graph linear on all branches and preserves
the ability to backport 0209-0212 individually in the future.
Ref: AAP-92564
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* Add cleantextmixin to settings serializer
* Make excluded_fields a cached property
* Add test coverage for category_slug all
* Fix OPTIONS 500 for settings CleanTextMixin pattern injection
* Apply validation for json and list fields
---------
Co-authored-by: Peter Braun <pbraun@redhat.com>
* AAP-89067: Fix stale image purge when EE image reference form changes
The _handle_image_cleanup signal compared images by string only, so two
references to the same underlying image (e.g. tag vs digest) were treated
as different. This caused podman rmi to strip all names from a still-needed
image, breaking environments especially in disconnected installs.
Add layered guards: skip cleanup for managed images (settings and
managed=True EE objects), detect tag-vs-digest reference form changes
for the same repository, and check other EEs for cross-reference form
conflicts on delete. Genuinely different versions (v1→v2) still get
cleaned up.
* Reduce cognitive complexity of _handle_image_cleanup
Extract the delete-path loop into _other_ee_shares_repo_with_different_ref
to flatten nesting and bring cognitive complexity under the SonarQube
threshold of 15.
* Extend managed-image guard to match by repo when ref forms differ
_is_managed_image previously only did exact string comparison against
managed settings, so repo@sha256:... would not match repo:tag in
CONTROL_PLANE_EXECUTION_ENVIRONMENT or GLOBAL_JOB_EXECUTION_ENVIRONMENTS.
Now also compares by repository + reference form (tag vs digest).
* Add docstring to _handle_image_cleanup
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
[Devel] CVE-2026-15307 - Bump django to >=5.2.17
Update django in requirements.in to address CVE-2026-15307.
Regenerated requirements.txt via make docker_update_requirements.
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* feat(api): advertise validation patterns on JSON sub-key fields
Credential type inputs.fields/metadata and notification OPTIONS now include
pattern and pattern_description for non-secret string sub-keys when
ENHANCED_INPUT_VALIDATION_ENABLED is on.
* Fix lint issue
* Fix sonarqube issue
* feat(api): advertise validation patterns on top-level CharFields
Wires DAB's inject_clean_text_patterns into awx.api.metadata.Metadata so
top-level CharField OPTIONS metadata gets pattern/patternDescription/flags,
matching the JSON sub-key pattern exposure already in place. No-op until
ENHANCED_INPUT_VALIDATION_ENABLED is on and a serializer mixes in CleanTextMixin.
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
* Increase test coverage
* Fix lint issues
* Address review feedback on validation pattern injection
Import TIER2_PATTERN_DESCRIPTION from DAB instead of keeping a manually
synced duplicate, and short-circuit inject_patterns_into_field_list
before iterating fields when enhanced input validation is disabled.
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
* Avoid mutating shared field dicts in inject_patterns_into_field_list
Shallow-copy each field before injecting pattern metadata so callers
that pass module-level schema dicts (e.g. ManagedCredentialType.registry
entries) don't get them mutated in place, matching the existing
copy-before-mutate approach in inject_patterns_into_init_parameters.
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
* Add validation pattern for survey fields
---------
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
Co-authored-by: Peter Braun <pbraun@redhat.com>
fix: eliminate race condition in gateway user sync
poll_until confirms the user exists in controller after gateway
creation, but a second GET call could return empty results under
load. Return the user directly from poll_until instead.
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: AAP-92631 Only defer control nodes in mesh stability gate
When the mesh routing table is empty, the stability gate was blocking
cleanup of ALL lost instances (both control and execution nodes). However,
execution nodes don't need the mesh and can be safely reaped immediately.
This fix separates lost instances by node type:
- Control nodes: defer cleanup (they need mesh consensus)
- Execution/hop nodes: reap immediately (they don't need mesh)
The previous behavior caused stale execution node instances to accumulate
in the database indefinitely, preventing job cleanup and degrading
controller performance.
Fixes: AAP-92631
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* fix: AAP-92631 Handle Kubernetes mesh gate differently from cluster deployments
The mesh stability gate checks KnownConnectionCosts to detect if receptor
routing is established. In cluster/mesh deployments, empty routing means
the mesh is re-establishing after a controller restart, so the gate blocks
cleanup of lost instances to prevent cluster fragmentation.
However, in Kubernetes (IS_K8S=True), controller pods are stateless and
independent — they don't need peer connections. Empty routing is the
expected steady state, not a sign of instability. The gate was treating
both deployments identically, causing lost instance rows to accumulate
indefinitely in Kubernetes.
This fix adds an IS_K8S check: when Kubernetes mode is enabled, the gate
always passes (returns True), allowing normal cleanup. In cluster mode, the
gate continues to check KnownConnectionCosts.
Fixes: AAP-92631
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* fix: [AAP-92631] Return execution/hop nodes when mesh gate blocks cleanup
## Issue
When the non-K8s mesh gate blocked cleanup (empty routing table), the code
returned an empty list, preventing execution and hop node cleanup. This was
unintended—these nodes don't depend on mesh consensus and should be reaped
immediately.
## Fix
- Filter lost_instances to return only execution/hop nodes when mesh gate blocks
- Control nodes (which require mesh consensus) continue to be deferred
- K8s deployments bypass the gate entirely (empty routing is steady state)
## Test Coverage
Added three tests per reviewer request:
1. test_kubernetes_mesh_gate_allows_empty_routing (parametrized)
- Unit test verifying gate logic with different mesh_status shapes
- Confirms _mesh_all_ready_nodes_visible() returns True in K8s
2. test_non_kubernetes_mesh_gate_blocks_empty_routing
- Unit test confirming non-K8s gate blocks with empty routing
3. test_mesh_gate_defers_control_but_cleans_execution_hop
- E2E integration test with all three node types
- Verifies exec/hop nodes reach cleanup while control nodes defer
- Matches production failure scenario from baremetal-a cluster
All 18 heartbeat/mesh tests pass.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
Cryptography 50.0.0 added a self._aes attribute to the Fernet class
constructor. Since Fernet256 overrides __init__ without calling
super().__init__(), it needs to manually set this attribute.
Without this attribute, any code using Fernet256 (encryption/decryption
of credentials, secrets, etc.) fails with:
AttributeError: 'Fernet256' object has no attribute '_aes'
This fix:
- Imports algorithms from cryptography.hazmat.primitives.ciphers
- Sets self._aes = algorithms.AES(self._encryption_key) in __init__
The fix is backward compatible with cryptography < 50.0.0 since the
_aes attribute wasn't used by Fernet's methods in older versions.
Fixes: 277 test failures in AWX test suite when using cryptography 50.x
Related: pyca/cryptography#11234 (cryptography 50.0.0 changes)
Co-authored-by: Stevenson Michel <iamstevensonmichel@outlook.com>
* Fix awx-operator molecule test timeout being too tight
The inline `timeout 20m` wrapping the molecule test run in the
"Run test deployment with awx-operator" step (separate from the
never-binding outer `timeout-minutes: 60`) isn't currently causing
failures on this repo's runners (recent successful devel runs take
9.4-10.8m), but it's the same line that downstream (ansible/tower)
had to bump to `timeout 54m` after its runs started legitimately
taking 15-19.5m and getting killed with as little as ~30s of margin
under 20m.
Since downstream syncs this workflow from here, leaving it at 20m
risks a future sync silently reverting the downstream fix. Raise it
to 54m to match, well ahead of any regression in this repo's own
runtime.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Liam Allen <lallen@redhat.com>
* Raise awx-operator job-level timeout to fit the 54m inner timeout
Job-level timeout-minutes covers the whole job cumulatively
(checkout, image build, the deployment step, log collection,
artifact upload), not just the deployment step. With the inner
`timeout 54m` from the previous commit, a worst-case run (setup of
~6-8m plus the full 54m before the inner timeout fires) totals
~60-62m, which can trip the job-level 60m limit first and hard-cancel
the job before "Collect awx-operator logs on failure" or the
timed_out=true write ever execute - losing exactly the debug output
this timeout-marker pattern exists to preserve.
Raise job-level timeout-minutes to 75 to give headroom past the
worst-case inner timeout. No change to the timed_out=true or
conditional log-collection logic.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Liam Allen <lallen@redhat.com>
---------
Signed-off-by: Liam Allen <lallen@redhat.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* AAP-91428 | feat: subsystem metrics for indirect node counting
Expose Prometheus metrics for indirect node counting at
/api/v2/metrics?subsystemonly=1, following the existing DispatcherMetrics
pattern. This is a prerequisite for perf/scale testing (AAP-57413) before
the feature flag can be removed for GA in 2.7.
Metrics added:
- indirect_node_audit_records_created (counter)
- indirect_node_query_execution_seconds (accumulated float)
- indirect_node_jq_query_errors (counter)
- indirect_node_fallback_cleanup_seconds (latest-value float)
Instrumentation points in host_indirect.py:
- jq query execution timing and error counting in build_indirect_host_data
- audit record count after bulk_create in save_indirect_host_entries_of_job
- fallback cleanup job wall-clock timing
Also registers IndirectCountingMetrics in send_subsystem_metrics for
cross-node metric broadcasting.
Co-Authored-By: Claude Code <noreply@anthropic.com>
* AAP-91428 | fix: SonarQube complaints (coverage + complexity)
* AAP-91428 | fix: use jq_str and fix docstring
* AAP-91428 | test: tests to get coverage up
Co-Authored-By: Claude Code <noreply@anthropic.com>
---------
Co-authored-by: Claude Code <noreply@anthropic.com>
Previously the ATF tests pipeline only ran on `/run-atf-tests` comment.
Add on-event and on-target-branch annotations so it also triggers
automatically on pull_request events targeting devel.
Signed-off-by: Adrià Sala <22398818+adrisala@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* AAP-78694 - Add CleanTextMixin to AWX resource serializers
Applies django-ansible-base's CleanTextMixin to the AWX serializers
listed in AAP-78694: Organization, User, Team, Credential(+Create),
CredentialType, Project, JobTemplate, WorkflowJobTemplate, Inventory,
Host, Group, InventorySource, InstanceGroup, Instance,
ExecutionEnvironment, NotificationTemplate, Schedule, AdHocCommand,
WorkflowJobTemplateNode, WorkflowJobTemplateNodeCreateApproval,
JobLaunch, and WorkflowJobLaunch.
Adds a local PromptFieldCleanTextMixin for serializers whose model
derives from LaunchTimeConfigBase, where scm_branch/limit/job_tags/
skip_tags are NullablePromptPseudoField descriptors invisible to the
mixin's normal model-field auto-discovery.
Per-serializer excluded_fields/name_fields overrides are documented
inline where a field's existing behavior would otherwise conflict
with Tier 1/2 validation (Jinja2-templated fields, secret fields,
and existing colon-containing name/hostname syntax).
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
* AAP-78694 - Cover CleanTextMixin gaps in Copy, Bulk, and Job Detail paths
Three write paths were found to bypass the Tier 1/2 validation applied
elsewhere for the same resources:
- CopySerializer (used by the Copy button/endpoint for EE, Project,
Credential, JobTemplate, WorkflowJobTemplate, NotificationTemplate)
accepted a user-supplied name with zero validation. Adds a new
PlainSerializerCleanTextMixin for plain serializers.Serializer
subclasses with no backing model.
- BulkJobNodeSerializer's validate() intentionally skips
LaunchConfigurationBaseSerializer.validate() (it assumes
unified_job_template/inventory/execution_environment are model
instances, but bulk nodes pass raw PKs to avoid a per-node DB
lookup), which meant it also skipped CleanTextMixin. Adds
PromptFieldCleanTextMixin._run_clean_text_validation() so the
Tier 1/2 checks still run despite that skip. BulkJobLaunchSerializer
and WorkflowJobNodeSerializer are also covered.
- JobSerializer (backing the narrow JobDetail PUT/PATCH window while
a job's status is "new") had no coverage at all.
Confirmed live against a running controller that a Copy with a
malicious name previously succeeded (201) where the equivalent PATCH
on the same resource was already rejected (400); both are rejected
consistently after this change.
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
* AAP-78694 - Fix NotificationTemplateSerializer validate() bypass
validate() returned early without calling super().validate() whenever
notification_configuration was absent from the request (e.g. a PATCH
that only changes name/description), which skipped CleanTextMixin
entirely on that path. Same bug class as the BulkJobNodeSerializer
MRO-skip already fixed, caught by code review.
Confirmed live: a PATCH with only a malicious name (no
notification_configuration) previously succeeded; now rejected with
400, while a legitimate name-only PATCH still succeeds.
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
* AAP-78694 - Add CleanTextMixin to the previously out-of-scope API-only serializers
LabelSerializer, CredentialInputSourceSerializer, and
WorkflowApprovalTemplateSerializer were originally scoped out ("no UI
exposure") but are now in scope per updated ticket guidance.
WorkflowApprovalTemplateSerializer backs the update path for the same
model WorkflowJobTemplateNodeCreateApprovalSerializer already protects
on create -- without this, an approval name rejected at creation could
be rewritten to anything via PATCH.
Confirmed live: Label create and WorkflowApprovalTemplate update both
reject a malicious name (400) and accept a valid one.
CredentialInputSourceSerializer verified at the code level only (plain
CharField input_field_name, no custom validators; metadata is a
JSONSchemaField holding vault paths/keys, not secret values) --
setting up a live external-credential-type chain in this environment
wasn't done.
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
* AAP-78694 - Correct HostSerializer name_fields demotion comment
The comment claimed the Tier 1 -> Tier 2 demotion was needed for
"hostname:port" syntax, but that colon is stripped from attrs['name']
in validate() before it reaches CleanTextMixin -- that case never
needed the demotion. What actually needs it: names with any colon
count other than exactly 1, which _get_host_port_from_name passes
through untouched -- most notably IPv6 addresses, which its own
comment says it explicitly does not parse. Caught by code review.
Confirmed live: an IPv6-style host name (2 colons) is accepted
unmodified (201).
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
* AAP-78694 - Validate non-secret notification_configuration keys
Replaced the blanket notification_configuration exclusion with a
precise excluded_json_keys covering only the actual secret fields.
Unlike Credential.inputs (admin-definable custom types with arbitrary
secret fields), notification_type is a fixed models.CharField(choices)
-- every backend class ships in awx/main/notifications/ and is known
at code-authoring time, so the secret key set can be a static list
rather than resolved per-instance.
Enumerated every init_parameters entry with type "password" across
all 8 backends (email/irc/webhook password, slack/pagerduty token,
twilio account_token, grafana grafana_key, awssns
aws_secret_access_key/aws_session_token) and confirmed none of those
names are used for a non-secret value in a different backend.
Previously-unvalidated, UI-exposed fields like webhook url and slack
channels are now covered. Confirmed live: a javascript: webhook url
and a <script>-tagged slack channel are now rejected (400), while a
weird-looking token/password value is still accepted unchanged.
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
* AAP-78694 - Exclude extra_data on launch-config serializers
extra_data (WorkflowJobTemplateNode/WorkflowJobNode/Schedule) plays
the same role as extra_vars elsewhere and is submitted in the same
two formats -- a dict, or a raw YAML/JSON string (parse_yaml_or_json
handles both). Its model field is JSONBlob, whose get_internal_type()
reports "TextField" for legacy DB-migration reasons, so
CleanTextMixin silently skipped dict payloads (not a str) while
string payloads hit Tier 2's template-injection blocklist and
incorrectly rejected legitimate Jinja variable references -- a real
regression once enforcement is on, caught by code review.
First attempt put the exclusion on the shared
LaunchConfigurationBaseSerializer base and silently did nothing:
CleanTextMixin's own excluded_fields = frozenset() default sits
earlier in the MRO than that base class for every subclass (mixin is
first in the bases tuple), so the base class's value was always
shadowed. Verified with a standalone MRO test before settling on
setting excluded_fields directly on each leaf serializer
(WorkflowJobTemplateNodeSerializer, WorkflowJobNodeSerializer,
ScheduleSerializer), matching every other exclusion in this file.
Confirmed live on both WorkflowJobTemplateNodeSerializer and
ScheduleSerializer: a YAML-string extra_data with a Jinja reference
is accepted (previously 400), while other Tier 1/2 fields on the same
serializers are still rejected as expected.
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
* AAP-78694 - Validate non-secret Credential.inputs keys dynamically
Unlike NotificationTemplate.notification_configuration (fixed set of
backend classes, static excluded_json_keys works), CredentialType is
admin-definable via a real "Create a new custom credential type"
endpoint, so the set of secret field names in Credential.inputs can't
be known at class-definition time -- it varies per credential's own
type, via that type's own inputs schema ("secret": true flags,
exposed as CredentialType.secret_fields).
validate() resolves the credential_type from attrs (create) or the
existing instance (update) and sets excluded_json_keys as an instance
attribute before chaining into CleanTextMixin.validate(). Setting an
instance attribute rather than a class attribute sidesteps the MRO
trap found while fixing extra_data on launch-config serializers:
CleanTextMixin's own excluded_fields/excluded_json_keys class-level
defaults sit earlier in the MRO than any shared base class further
down the chain, silently shadowing it -- an instance attribute always
wins regardless of MRO position, and correctly threads through
CredentialSerializerCreate and its User/Team/Organization subclasses
for free, since each of their validate() overrides chains through
super() into this one.
Falls back to excluding the whole inputs blob (prior, safe behavior)
if credential_type can't be resolved.
Confirmed live: malicious username (non-secret, including via PATCH
with no credential_type in the payload) is rejected; a dangerous-
looking password value and a real unencrypted SSH private key
(secret fields) are both still accepted unchanged.
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
* Increase test coverage
* Cover remaining serializer branches for Sonar gate
Line coverage is already over 90%; Sonar's quality gate uses combined
line+branch coverage. These unit tests exercise previously untested
methods so the PR can clear the 90% new-code threshold.
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
* style(api): format uncovered serializer tests for ruff
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
Assisted-by: Cursor Grok 4.6 (SpaceXAI / Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
* test(api): set Mock.name as an attribute, not a constructor kwarg
unittest.mock.Mock(name=...) only sets the mock's repr name, so
getattr(obj, 'name') returned another Mock and four coverage tests
failed in api-test.
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
Assisted-by: Cursor Grok 4.6 (SpaceXAI / Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
* test(api): keep a single call inside pytest.raises blocks
Sonar python:S5918 flags exception tests that construct mocks or
serializers inside the raises context.
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
Assisted-by: Cursor Grok 4.6 (SpaceXAI / Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
* Add more test coverage
---------
Signed-off-by: Pratyush Bhandari <prbhanda@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The controller_dir property hardcoded /tmp/awx_{id} as the expected
volume mount prefix. On containerized installs the job execution
directory is at /home/ansible/aap/controller/data/job_execution/awx_{id},
causing a RuntimeError when the prefix didn't match.
Changed to exact path component matching on the awx_{id} segment,
which works for both traditional and containerized path layouts
while preventing false matches (e.g. awx_1 matching awx_10).
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After creating a user on the gateway API, the code immediately queries
the controller API to find the synced user. On containerized installs
(2.6+), the gateway-to-controller sync may not have completed yet,
causing an IndexError when .results.pop() is called on an empty list.
This affected ~32 tests across the tower-qa legacy suite on every
containerized pipeline run (2.6, 2.7, 2.8).
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AAP-90120: add _mesh_all_ready_nodes_visible() to block peer-judgment
while KnownConnectionCosts is empty after a controller restart. Gate
fails open when mesh_status is unavailable. Eliminates double receptor
status call per heartbeat.
AAP-90121: wrap each instance's reap+mark_offline block in
advisory_lock('task_manager_lock', wait=False) so the task manager
cannot dispatch to a node being concurrently marked offline. Extracts
_reap_and_mark_lost_instance() helper.
Tests: rewrite new test classes to use real Django DB (functional/)
instead of MagicMock objects.
---------
Signed-off-by: Hui Song <hsong@redhat.com>
Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
* Add PostgreSQL utility functions for metrics-utility
Creates two plpgsql functions used by metrics-utility when querying the
AWX controller database via a read-only connection:
- metrics_utility_parse_yaml_field: extracts a named field from YAML-encoded text
- metrics_utility_is_valid_json: validates whether a text value is valid JSON
GRANT EXECUTE TO PUBLIC so any database user (including ms_awx_readonly)
can call the functions without requiring an explicit per-user grant.
The migration is a no-op on SQLite so unit and functional tests are unaffected.
Assisted-by: Claude Code (claude-sonnet-4-6)
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Ciaran Shiels <cas.ireland@Gmail.com>
* Fix regex injection and broad exception in metrics utility migration
- Escape `field` parameter before interpolating into regex pattern using
regexp_replace with the whole-match `&` reference rather than bare
concatenation; prevents regex metacharacters in the field name from
altering match semantics
- Narrow `EXCEPTION WHEN others` to `WHEN invalid_text_representation`
(SQLSTATE 22P02) in metrics_utility_is_valid_json so only the expected
JSON-cast failure is swallowed and unexpected runtime errors propagate
Assisted-by: Claude Code (claude-sonnet-4-6)
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Ciaran Shiels <cshiels@redhat.com>
* Renumber migration 0206 to 0210 to avoid conflict
Re-parent create_metrics_utility_functions migration from 0205 to 0209
after other migrations (0206–0209) merged to devel since branch creation.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Renumber migration 0210 to 0211 to avoid conflict
0210_remove_host_last_job_fields arrived on devel after the last rebase,
re-parent create_metrics_utility_functions to depend on it instead.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Signed-off-by: Ciaran Shiels <cas.ireland@Gmail.com>
Signed-off-by: Ciaran Shiels <cshiels@redhat.com>
Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* [AAP-89071] Add control plane EE unit/functional tests
Add tests for EE resolution, constructed inventory model,
and ansible-sign signature validation tag injection.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* [AAP-89071] Fix control plane EE tests
- Test resolve_execution_environment on ProjectUpdate/InventoryUpdate
(job objects) instead of Project/InventorySource (templates), matching
the actual code path in BaseTask.run()
- Consolidate inventory EE resolution into single parametrized test
- Remove trivial kind-check test
- Fix OrderedManyToManyField.add() to add one inventory at a time
- Add asymmetric host counts for better aggregation validation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* [AAP-89071] Fix InventoryUpdate missing inventory relation
Add inventory=inv to InventoryUpdate.objects.create() so
resolve_execution_environment() can access self.inventory.kind.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
In particular, this brings two important changes:
- rather than opening a new PR every time it runs, it uses a stable
branch name updating its content instead; this means that at most
there is one open PR per target branch
- a new secret "OPENAPI_SPEC_PR_REVIEW_TEAM" is introduced to specify
the name of the github team to notify when creating new PRs; this
helps to make sure that PRs are reviewed
* Remove dead Host.last_job and Host.last_job_host_summary FK fields
PR #16332 replaced these denormalized ForeignKey columns with dynamic
derivation via Host.latest_summary, but left the columns in place.
Nothing writes to or reads from them anymore — drop the dead weight.
- Remove last_job and last_job_host_summary ForeignKey fields from Host
- Convert HostSerializer to use SerializerMethodField instead of FK-backed fields
- Remove redundant JobHostSummary.latest_for_host/latest_job_for_host classmethods
- Add migration 0206 to drop the columns
- Update tests to use Host.latest_summary instead of removed fields/methods
* Remove dead last_job_host_summary_id UPDATE from cleanup_jobs
The Host.last_job_host_summary FK was removed in the prior commit,
so the raw SQL UPDATE that NULLed it before deleting JHS rows now
references a non-existent column and crashes.
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
The GitHub Actions runner image update reduced available disk space,
causing the awx-operator molecule test to hang and timeout waiting for
pods that can't start due to disk pressure. Prune BuildKit cache and
dangling images after the AWX image build to reclaim space.
Same fix as ansible/tower PRs #7905 (stable-2.6), #7926 (stable-2.7),
and #7927 (release_4.6).
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Added null guard and restructured transactions
* Flag names that are None and add logging warnings for all modules
* Add test for job-deleted-during-transaction race condition
Covers the Job.DoesNotExist handler inside the transaction.atomic()
block to satisfy SonarQube's 90% new code coverage gate.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* AAP-87599 — Re-parent 0206 and update 0207 as merge point for 2.6 backport
Re-parent 0206_jobhostsummary_host_id_idx to depend on 0203 instead
of 0205, and add 0205 as a second dependency on 0207 to converge the
migration graph.
This follows the gateway backport migration pattern (AAP-80663) so that
the same 0206 migration file can be backported to stable-2.6 (where
0204/0205 do not exist) without breaking upgrade paths.
* AAP-87599 — Move merge point to 0209, restore 0207 to original
Reverts the 0205 dependency addition on 0207_alter_skip_tags and
instead adds 0209_merge_0205_0206 at the end of the chain. This
keeps 0207 pristine so it can be backported to stable-2.7 in the
future without a naming conflict.
Migration graph after this change:
0203 → 0204 → 0205 ──────────────────────────────────┐
\ ↓
→ 0206 → 0207_skip_tags → 0208_sys_auditor → 0209_merge
* AAP-87599 — Move merge point to 0207 for devel/2.7 parity
Restructure the migration graph so devel matches stable-2.7 through
the merge point:
- 0207_merge_0205_0206 is now identical on both branches (depends on
0205 + 0206, empty operations)
- Renumber 0207_alter_skip_tags → 0208, 0208_fix_system_auditor → 0209
- Both depend linearly from the merge point forward
Graph:
0205 ──────────────┐
\ ↓
0206 → 0207_merge → 0208_skip_tags → 0209_sys_auditor
* Fix test references after migration rename 0208→0209
Update test_migrations.py to reference 0209_fix_system_auditor_migration
after the merge point restructure moved it from 0208 to 0209.
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Peter Braun <pbraun@redhat.com>
Replace ResourceMixin/accessible_objects/accessible_pk_qs with DAB RBAC access_qs/access_ids_qs
Remove the ResourceMixin class and migrate all callers to the new
DAB RBAC queryset API: access_qs replaces accessible_objects and
access_ids_qs replaces accessible_pk_qs, using permission strings
(view, change, use, etc.) instead of role field names.
Also updates prefetch_page_capabilities to use permission-based
capability definitions instead of role field references.
Restore UNION pattern in TeamAccess.filtered_queryset
The access_qs migration accidentally collapsed the UNION into an OR,
which can be 48x slower at scale (AAP-83319).
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Unify activity stream format for role assignments across old and new APIs
The old API (/api/v2/users/<pk>/roles/) and new API
(/api/v2/role_user_assignments/) were producing different activity stream
entries for the same logical action. The old API created entries via the
Role.members m2m_changed signal in the legacy format, while the new API
used the new-format handler added in AAP-82996.
This change makes the old API produce the same new-format entry by:
- Skipping the legacy activity_stream_associate for Role_members changes
- Letting the mirrored RoleUserAssignment creation fire the new-format
record_role_assignment_activity_stream handler instead
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* Record DAB RBAC role assignment events in the activity stream
* The steam message was showing incorrectly, it was inverted based on the legacy setup
* object_relationship_type was missing, this is set n the legacy handler and emit_activity_stream_change emits it.
* Skip cascade deletes from non-assignment origins, same reasoning as sync_assignments_to_old_rbac_delete
The EE sidecar container creates /etc/receptor/receptor.conf on pod
startup. If the task container's dispatcherd reaches the heartbeat
before the config exists, the instance rejoins the cluster at full
capacity with no receptor — causing fatal job failures with no
recovery path.
Three layers of defense:
1. dispatcherd management command: poll for the receptor config file
before entering the main loop (10s timeout, exits with error to
let k8s restart the container).
2. Heartbeat: if get_receptor_ctl() raises FileNotFoundError (config
or socket missing), mark the instance offline so no jobs are
dispatched to it. Previously this was silently ignored and the
instance kept full capacity.
3. should_update_config(): catch FileNotFoundError and return True so
write_receptor_config() can create the file on first boot instead
of crashing.
Prior art: #12698 (closed without merge) took a broader approach with
custom exception types. See PR comment for overlap analysis.
Closes: AAP-89010
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
lgalisandCursor
Hao Liu
Dirk JulichandClaude Opus 4.6
Elijah DeLeeandJake Jackson
Pratyush Bhandari
Maeve HofferandClaude Opus 4.6
Liam Allen
Peter Braun
Hui SongandClaude Sonnet 4.6
Stevenson MichelandClaude Opus 4.6
Adrià SalaandClaude Opus 4.6
Matthew JohnsonandStevenson Michel
red-hat-konflux-kflux-prd-rh02[bot]
Hunter KepleyandClaude Code
Paul Flanagan
John Westcott
John Barker
Pino Toscano
Alan RomingerandClaude Opus 4.6
