Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions awx/main/tasks/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@
# Django flags
from flags.state import flag_enabled

# Workload Identity imports
from ansible_base.resource_registry.workload_identity_client import (
WorkloadIdentityClient,
TokenRequestError,
get_workload_identity_client,
)

# Workload Identity
from ansible_base.lib.workload_identity.controller import AutomationControllerJobScope

Expand Down Expand Up @@ -514,6 +521,10 @@ def pre_run_hook(self, instance, private_data_dir):
# Before task is started, ensure that job_event partitions exist
create_partition(instance.event_class._meta.db_table, start=instance.created)

# Request workload identity JWT token if feature is enabled and job needs it
if flag_enabled("FEATURE_OIDC_WORKLOAD_IDENTITY_ENABLED") and self._job_needs_workload_jwt(instance):
self._request_workload_identity_token(instance)

def post_run_hook(self, instance, status):
"""
Hook for any steps to run before job/task is marked as complete.
Expand All @@ -532,6 +543,90 @@ def final_run_hook(self, instance, status, private_data_dir):
if instance.spawned_by_workflow:
ScheduleWorkflowManager().schedule()

def _job_needs_workload_jwt(self, instance):
"""
Determine if this job needs a workload identity JWT token.

Currently checks if the job has any vault credentials, as Vault is the
primary use case for workload identity JWTs (ANSTRAT-1558).

Args:
instance: The UnifiedJob instance being executed

Returns:
bool: True if job needs a JWT token
"""
# Check if job has credentials attribute (not all job types do)
if not hasattr(instance, 'credentials'):
return False

# Check if any vault credentials are present
# TODO: Extend this to check for other credential types that support workload identity
# when AAP-62694 (JWT-enabled credential plugin) is implemented
vault_creds = instance.credentials.filter(credential_type__kind='vault').exists()

if vault_creds:
logger.debug(f"Job {instance.id} has vault credentials, will request workload JWT")
return True

# TODO: Check for other credential types that require workload identity
# e.g., AWS, Azure, GCP when those integrations are implemented

return False

def _request_workload_identity_token(self, instance):
"""
Request a workload identity JWT token from Gateway for this job.

This method is called during pre_run_hook if the feature flag is enabled
and the job needs a JWT (determined by _job_needs_workload_jwt).
The JWT token is logged but not currently used for anything (see AAP-62694).

Args:
instance: The UnifiedJob instance being executed
"""
try:
# Populate JWT claims from the job
claims = populate_claims_for_workload(instance)

# TODO: Make scope and audience configurable
# For now, hardcode the scope as defined in the OIDC spec
scope = "aap_controller_automation_job"
# TODO: Audience should be configured per integration (Vault, AWS, etc.)
audience = "https://vault.example.com"

# Get the workload identity client (uses service token auth)
client = get_workload_identity_client()

# Request the JWT token
logger.info(f"Requesting workload identity token for job {instance.id} ({instance.name})")
response = client.request_workload_jwt(
claims=claims,
scope=scope,
audience=audience
)

logger.info(
f"Successfully obtained workload identity token for job {instance.id}. "
f"JWT length: {len(response.jwt)} characters"
)
logger.debug(f"JWT token: {response.jwt[:50]}...")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid logging any portion of a JWT, even at DEBUG level.

Auth tokens should not appear in log files. Logging the first 50 characters still exposes part of the payload (base64-decoded claims like organization or job metadata). Remove or replace with a non-sensitive token fingerprint (e.g., length only).

🛡️ Proposed fix
-            logger.debug(f"JWT token: {response.jwt[:50]}...")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
logger.debug(f"JWT token: {response.jwt[:50]}...")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@awx/main/tasks/jobs.py` at line 613, The current logger.debug call exposes a
sensitive value by logging response.jwt (even partially); remove any logging of
response.jwt and replace it with a non-sensitive indicator such as the token
length or a stable fingerprint (e.g., log f"JWT length: {len(response.jwt)}" or
a one-way hash/fingerprint of response.jwt) so no JWT characters or payload are
written to logs; update the logger.debug invocation that references response.jwt
to emit only the safe indicator.


# TODO (AAP-62694): Store JWT for use by credential plugins
# For now, we just log that we got it

except TokenRequestError as e:
logger.error(
f"Failed to obtain workload identity token for job {instance.id}: {e}"
)
# Don't fail the job if JWT request fails - this is optional functionality

except Exception as e:
logger.exception(
f"Unexpected error requesting workload identity token for job {instance.id}: {e}"
)
# Don't fail the job on unexpected errors
Comment on lines +618 to +628
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Address both Ruff warnings: TRY400 and TRY401.

  1. except TokenRequestError (line 618): logger.error discards the exception traceback — use logger.exception so the full stack is captured.
  2. except Exception as e (line 624): passing e to logger.exception is redundant — logger.exception already attaches the active exception.
🐛 Proposed fix
         except TokenRequestError as e:
-            logger.error(
-                f"Failed to obtain workload identity token for job {instance.id}: {e}"
-            )
+            logger.exception(
+                f"Failed to obtain workload identity token for job {instance.id}: {e}"
+            )
             # Don't fail the job if JWT request fails - this is optional functionality

-        except Exception as e:
+        except Exception:
             logger.exception(
-                f"Unexpected error requesting workload identity token for job {instance.id}: {e}"
+                f"Unexpected error requesting workload identity token for job {instance.id}"
             )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
except TokenRequestError as e:
logger.error(
f"Failed to obtain workload identity token for job {instance.id}: {e}"
)
# Don't fail the job if JWT request fails - this is optional functionality
except Exception as e:
logger.exception(
f"Unexpected error requesting workload identity token for job {instance.id}: {e}"
)
# Don't fail the job on unexpected errors
except TokenRequestError as e:
logger.exception(
f"Failed to obtain workload identity token for job {instance.id}: {e}"
)
# Don't fail the job if JWT request fails - this is optional functionality
except Exception:
logger.exception(
f"Unexpected error requesting workload identity token for job {instance.id}"
)
# Don't fail the job on unexpected errors
🧰 Tools
🪛 Ruff (0.15.1)

[warning] 619-621: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


[warning] 626-626: Redundant exception object included in logging.exception call

(TRY401)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@awx/main/tasks/jobs.py` around lines 618 - 628, Change the two exception
handlers so they both log full tracebacks: replace the logger.error in the
TokenRequestError except block with logger.exception to include the traceback
(keep the same non-failing behavior), and in the generic except Exception
handler remove the redundant exception argument so the call is
logger.exception(f"Unexpected error requesting workload identity token for job
{instance.id}") — reference TokenRequestError, the existing except blocks,
logger.exception, and instance.id when making the edits.


def should_use_fact_cache(self):
return False

Expand Down
112 changes: 112 additions & 0 deletions awx/main/tests/unit/tasks/test_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,3 +427,115 @@ def test_populate_claims_for_adhoc_command(workload_attrs, expected_claims):

claims = jobs.populate_claims_for_workload(adhoc_command)
assert claims == expected_claims


@pytest.mark.django_db
@mock.patch('awx.main.tasks.jobs.get_workload_identity_client')
def test_request_workload_identity_token_success(mock_get_client, job_template_factory, credential):
"""Test successful workload identity token request."""
# Mock the client and response
from ansible_base.resource_registry.workload_identity_client import WorkloadIdentityTokenResponse
mock_client = mock.Mock()
mock_client.request_workload_jwt.return_value = WorkloadIdentityTokenResponse(
jwt='eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.test.signature'
)
mock_get_client.return_value = mock_client

# Create a test job with vault credential
jt = job_template_factory('test-jt')
job = jt.create_unified_job()
job.id = 123
job.name = 'Test Job'
job.credentials.add(credential)

# Create a BaseTask instance and call the method
task = jobs.BaseTask()
task._request_workload_identity_token(job)

# Verify client was called with claims
mock_client.request_workload_jwt.assert_called_once()
call_args = mock_client.request_workload_jwt.call_args
assert 'claims' in call_args.kwargs
assert 'scope' in call_args.kwargs
assert 'audience' in call_args.kwargs
assert call_args.kwargs['scope'] == 'aap_controller_automation_job'


@pytest.mark.django_db
@mock.patch('awx.main.tasks.jobs.get_workload_identity_client')
@mock.patch('awx.main.tasks.jobs.flag_enabled')
@mock.patch('awx.main.tasks.jobs.logger')
def test_request_workload_identity_token_failure(mock_logger, mock_flag_enabled, mock_get_client):
"""Test workload identity token request failure handling."""
# Enable the feature flag
mock_flag_enabled.return_value = True

# Mock the client to raise an error
from ansible_base.resource_registry.workload_identity_client import TokenRequestError
mock_client = mock.Mock()
mock_client.request_workload_jwt.side_effect = TokenRequestError("Gateway unavailable")
mock_get_client.return_value = mock_client

# Create a test job
job = Job(id=456, name='Failing Job')

# Create a BaseTask instance and call the method
task = jobs.BaseTask()
task._request_workload_identity_token(job)

# Verify error was logged
mock_logger.error.assert_called()
error_message = mock_logger.error.call_args[0][0]
assert 'Failed to obtain workload identity token' in error_message
assert '456' in error_message


@pytest.mark.django_db
@mock.patch('awx.main.tasks.jobs.get_workload_identity_client')
@mock.patch('awx.main.tasks.jobs.flag_enabled')
def test_request_workload_identity_token_feature_flag_disabled(mock_flag_enabled, mock_get_client):
"""Test that JWT request is skipped when feature flag is disabled."""
# Disable the feature flag
mock_flag_enabled.return_value = False

# Create a test job
job = Job(id=789, name='Test Job')

# Create a BaseTask instance and call pre_run_hook
task = jobs.BaseTask()
task.pre_run_hook(job, '/tmp/private_data')

# Verify client was never called
mock_get_client.assert_not_called()
Comment on lines +493 to +509
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

create_partition is not mocked — test_request_workload_identity_token_feature_flag_disabled may fail before reaching the assertion.

pre_run_hook calls create_partition(instance.event_class._meta.db_table, start=instance.created) before the flag check. For a bare unsaved Job(id=789), instance.created is None, which may cause create_partition to error or behave unexpectedly. Add the same create_partition mock used in other pre/post hook tests.

🐛 Proposed fix
 `@pytest.mark.django_db`
 `@mock.patch`('awx.main.tasks.jobs.get_workload_identity_client')
 `@mock.patch`('awx.main.tasks.jobs.flag_enabled')
+@mock.patch('awx.main.tasks.jobs.create_partition', return_value=True)
-def test_request_workload_identity_token_feature_flag_disabled(mock_flag_enabled, mock_get_client):
+def test_request_workload_identity_token_feature_flag_disabled(mock_create_partition, mock_flag_enabled, mock_get_client):
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@pytest.mark.django_db
@mock.patch('awx.main.tasks.jobs.get_workload_identity_client')
@mock.patch('awx.main.tasks.jobs.flag_enabled')
def test_request_workload_identity_token_feature_flag_disabled(mock_flag_enabled, mock_get_client):
"""Test that JWT request is skipped when feature flag is disabled."""
# Disable the feature flag
mock_flag_enabled.return_value = False
# Create a test job
job = Job(id=789, name='Test Job')
# Create a BaseTask instance and call pre_run_hook
task = jobs.BaseTask()
task.pre_run_hook(job, '/tmp/private_data')
# Verify client was never called
mock_get_client.assert_not_called()
`@pytest.mark.django_db`
`@mock.patch`('awx.main.tasks.jobs.get_workload_identity_client')
`@mock.patch`('awx.main.tasks.jobs.flag_enabled')
`@mock.patch`('awx.main.tasks.jobs.create_partition', return_value=True)
def test_request_workload_identity_token_feature_flag_disabled(mock_create_partition, mock_flag_enabled, mock_get_client):
"""Test that JWT request is skipped when feature flag is disabled."""
# Disable the feature flag
mock_flag_enabled.return_value = False
# Create a test job
job = Job(id=789, name='Test Job')
# Create a BaseTask instance and call pre_run_hook
task = jobs.BaseTask()
task.pre_run_hook(job, '/tmp/private_data')
# Verify client was never called
mock_get_client.assert_not_called()
🧰 Tools
🪛 Ruff (0.15.1)

[error] 506-506: Probable insecure usage of temporary file or directory: "/tmp/private_data"

(S108)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@awx/main/tests/unit/tasks/test_jobs.py` around lines 493 - 509, The test
test_request_workload_identity_token_feature_flag_disabled can error before the
assertion because pre_run_hook on jobs.BaseTask calls
create_partition(instance.event_class._meta.db_table, start=instance.created);
mock create_partition the same way other pre/post hook tests do so
instance.created=None won’t trigger real partition logic. Update the test to
patch/mocking create_partition (the function used by pre_run_hook) before
invoking jobs.BaseTask().pre_run_hook, leaving mocks for flag_enabled and
get_workload_identity_client unchanged so the test can reach the
mock_get_client.assert_not_called() assertion.



@pytest.mark.django_db
def test_job_needs_workload_jwt_with_vault_credential(job_template_factory, vault_credential):
"""Test that jobs with vault credentials need JWT tokens."""
jt = job_template_factory('test-jt')
job = jt.create_unified_job()
job.credentials.add(vault_credential)

task = jobs.BaseTask()
assert task._job_needs_workload_jwt(job) is True


@pytest.mark.django_db
def test_job_needs_workload_jwt_without_vault_credential(job_template_factory, machine_credential):
"""Test that jobs without vault credentials don't need JWT tokens."""
jt = job_template_factory('test-jt')
job = jt.create_unified_job()
job.credentials.add(machine_credential)

task = jobs.BaseTask()
assert task._job_needs_workload_jwt(job) is False


@pytest.mark.django_db
def test_job_needs_workload_jwt_no_credentials(job_template_factory):
"""Test that jobs without any credentials don't need JWT tokens."""
jt = job_template_factory('test-jt')
job = jt.create_unified_job()

task = jobs.BaseTask()
assert task._job_needs_workload_jwt(job) is False
3 changes: 3 additions & 0 deletions awx/settings/development_defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,6 @@
# ======================!!!!!!! FOR DEVELOPMENT ONLY !!!!!!!=================================

FEATURE_INDIRECT_NODE_COUNTING_ENABLED = True

# Feature flag for workload identity JWT integration (AAP-62693)
FEATURE_OIDC_WORKLOAD_IDENTITY_ENABLED = True
Comment on lines +73 to +74
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check all settings files for FEATURE_OIDC_WORKLOAD_IDENTITY_ENABLED
rg -n "FEATURE_OIDC_WORKLOAD_IDENTITY_ENABLED" --type py -A1

Repository: ansible/awx

Length of output: 367


🏁 Script executed:

#!/bin/bash
# Find all settings files in the directory
fd -e py settings/ awx/settings/ | head -20
# Also check specifically for production or defaults
rg -l "production|PRODUCTION" awx/settings/ --type py

Repository: ansible/awx

Length of output: 193


🏁 Script executed:

#!/bin/bash
# Check production_defaults.py for the flag
echo "=== production_defaults.py ==="
rg -n "FEATURE_OIDC_WORKLOAD_IDENTITY_ENABLED" awx/settings/production_defaults.py -A1
echo "Exit code: $?"

# Check defaults.py as well
echo -e "\n=== defaults.py ==="
rg -n "FEATURE_OIDC_WORKLOAD_IDENTITY_ENABLED" awx/settings/defaults.py -A1
echo "Exit code: $?"

# Also check if there's any inheritance or defaults mechanism
echo -e "\n=== All settings file references to FEATURE_OIDC ==="
rg -n "FEATURE_OIDC" awx/settings/ --type py

Repository: ansible/awx

Length of output: 272


🏁 Script executed:

#!/bin/bash
# Find the flag_enabled function definition
echo "=== Finding flag_enabled function ==="
rg -n "def flag_enabled" --type py -A5

# Check how settings files are loaded/inherited
echo -e "\n=== Checking settings/__init__.py for initialization logic ==="
head -50 awx/settings/__init__.py

# Look for which settings file is loaded by default
echo -e "\n=== Check for DJANGO_SETTINGS_MODULE or settings loading ==="
rg -n "development_defaults\|production_defaults" awx/settings/ --type py | head -20

Repository: ansible/awx

Length of output: 2071


🏁 Script executed:

#!/bin/bash
# Search for flag_enabled in entire codebase
echo "=== Searching for flag_enabled definition ==="
rg -n "flag_enabled" --type py | grep -E "(def|from|import)" | head -20

# Search specifically in main directory
echo -e "\n=== Checking awx/main for flag_enabled ==="
rg -n "def flag_enabled\|from.*flag_enabled" awx/main/ --type py

# Check if it's from an external library
echo -e "\n=== Check imports in jobs.py ==="
head -40 awx/main/tasks/jobs.py | grep -E "(import|from)"

Repository: ansible/awx

Length of output: 1509


🏁 Script executed:

#!/bin/bash
# Check the test for feature flag disabled
echo "=== Test for workload identity token with feature flag disabled ==="
sed -n '496,520p' awx/main/tests/unit/tasks/test_jobs.py

# Check if there's any documentation or setup in the test file about flag defaults
echo -e "\n=== Check test file setup and imports ==="
head -100 awx/main/tests/unit/tasks/test_jobs.py | grep -A5 -B5 "flag_enabled\|mock"

# Search for how flags library is configured
echo -e "\n=== Search for FLAGS configuration in settings ==="
rg -n "FLAGS" awx/settings/ --type py

Repository: ansible/awx

Length of output: 3943


🏁 Script executed:

#!/bin/bash
# Search for FLAGS related settings that might define defaults
echo "=== Search for FLAGS or flag defaults ==="
rg -n "FLAGS\|flag.*default" awx/settings/ --type py -i

# Check if production_defaults.py explicitly handles this flag
echo -e "\n=== Check production_defaults.py content around feature flags ==="
rg -n "FEATURE_" awx/settings/production_defaults.py --type py | head -20

# Check defaults.py for any feature flags
echo -e "\n=== Check defaults.py for FEATURE_ flags ==="
rg -n "FEATURE_" awx/settings/defaults.py --type py | head -20

Repository: ansible/awx

Length of output: 305


🏁 Script executed:

#!/bin/bash
# Check the context around FEATURE_ flags in defaults.py
echo "=== Context around FEATURE_ flags in defaults.py ==="
sed -n '1115,1140p' awx/settings/defaults.py

# Check if there are more FEATURE_ flags
echo -e "\n=== All FEATURE_ flags in codebase ==="
rg -n "FEATURE_.*=" --type py | grep -E "defaults|settings"

Repository: ansible/awx

Length of output: 2406


Add FEATURE_OIDC_WORKLOAD_IDENTITY_ENABLED = False to awx/settings/defaults.py for consistency and production safety.

The flag is currently defined only in development_defaults.py:74 as True, but is missing from the base defaults.py. This breaks the established pattern where all FEATURE_* flags are explicitly defined in defaults.py as False (like FEATURE_POLICY_AS_CODE_ENABLED and FEATURE_INDIRECT_NODE_COUNTING_ENABLED). Without an explicit False in the base settings, production deployments lack a guaranteed default state for this flag. Add the flag to defaults.py to match the pattern and ensure production explicitly disables it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@awx/settings/development_defaults.py` around lines 73 - 74, Add
FEATURE_OIDC_WORKLOAD_IDENTITY_ENABLED = False to the base defaults so the
feature flag has an explicit production-safe default; mirror the pattern used
for other flags like
FEATURE_POLICY_AS_CODE_ENABLED/FEATURE_INDIRECT_NODE_COUNTING_ENABLED in
defaults.py, leaving the existing FEATURE_OIDC_WORKLOAD_IDENTITY_ENABLED = True
in development_defaults.py to override the base for development.

Loading