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
6 changes: 4 additions & 2 deletions awx/main/models/inventory.py
Original file line number Diff line number Diff line change
Expand Up @@ -1056,16 +1056,18 @@ def get_cloud_credential(self):
break
return credential

def get_extra_credentials(self):
def get_extra_credentials(self, base_qs=None):
"""Return all credentials that are not used by the inventory source injector.
These are all credentials that should run their own inject_credential logic.
"""
if base_qs is None:
base_qs = self.credentials.all()
special_cred = None
if self.source in discover_available_cloud_provider_plugin_names():
# these have special injection logic associated with them
special_cred = self.get_cloud_credential()
extra_creds = []
for cred in self.credentials.all():
for cred in base_qs:
if special_cred is None or cred.pk != special_cred.pk:
extra_creds.append(cred)
return extra_creds
Expand Down
25 changes: 23 additions & 2 deletions awx/main/tasks/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -1728,6 +1728,16 @@ def build_env(self, inventory_update, private_data_dir, private_data_files=None)
env['INVENTORY_UPDATE_ID'] = str(inventory_update.pk)
env.update(STANDARD_INVENTORY_UPDATE_ENV)

# The injector's _get_shared_env() calls inventory_update.get_cloud_credential()
# which does a fresh DB fetch, losing the OIDC context populated by
# populate_workload_identity_tokens(). Override it on this instance to return
# the credential from self._credentials which already has the context.
cloud_cred = inventory_update.get_cloud_credential()
if cloud_cred and self._credentials:
contexted_cred = next((c for c in self._credentials if c.pk == cloud_cred.pk), None)
if contexted_cred is not None:
inventory_update.get_cloud_credential = lambda _cred=contexted_cred: _cred
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is so so hacky, but I explored the code and I think there is no other way to do it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is kind of crazy. The right answer would be a deeper refactor.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You make the call here @AlanCoding , want us to dig deeper here or leave it as is for now until it becomes a problem?


injector = None
if inventory_update.source in InventorySource.injectors:
injector = InventorySource.injectors[inventory_update.source]()
Expand Down Expand Up @@ -1862,8 +1872,19 @@ def build_playbook_path_relative_to_cwd(self, inventory_update, private_data_dir
return None

def build_credentials_list(self, inventory_update):
# All credentials not used by inventory source injector
return inventory_update.get_extra_credentials()
# Use prefetched queryset so populate_workload_identity_tokens() can
# access input_sources for OIDC JWT generation.
base_qs = inventory_update.credentials.prefetch_related('input_sources__source_credential').all()
creds = inventory_update.get_extra_credentials(base_qs=base_qs)
# The cloud credential is excluded by get_extra_credentials() but must
# be in _credentials so populate_workload_identity_tokens() can generate
# an OIDC JWT for it when it has an external credential input source.
cloud_cred = inventory_update.get_cloud_credential()
if cloud_cred and cloud_cred.pk not in {c.pk for c in creds}:
# Use the prefetched instance so input_sources are already loaded
prefetched = next((c for c in base_qs if c.pk == cloud_cred.pk), cloud_cred)
creds.append(prefetched)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I can not convince myself that any of this code is needed, meaning creds could be returned without doing anything additional to it. The model method get_extra_credentials does:

        for cred in base_qs:
            if special_cred is None or cred.pk != special_cred.pk:
                extra_creds.append(cred)

So it should already be returning the credentials with the prefetched objects.

return creds

def build_project_dir(self, inventory_update, private_data_dir):
source_project = None
Expand Down
Loading