schubergphilis / towerlib

A python library to interface with ansible tower's (awx) api.
MIT License
43 stars 39 forks source link

Exception message triggers UnboundLocalError #118

Closed sspans-sbp closed 1 year ago

sspans-sbp commented 1 year ago

The exception message in Credential.new uses a variable 'credential_type' defined in the try block. Depending on the behaviour of the remote AWX the following UnboundLocalError might be triggered.

[ERROR] UnboundLocalError: local variable 'credential_type' referenced before assignment
Traceback (most recent call last):
  File "/opt/python/lib/python3.10/site-packages/datadog_lambda/wrapper.py", line 187, in __call__
    self.response = self.func(event, context, **kwargs)
  File "/var/task/cm_rotate_machine_process/lambda_function.py", line 59, in lambda_handler
    def lambda_handler(event, _):
  File "/opt/python/lib/python3.10/site-packages/ddtrace/contrib/aws_lambda/patch.py", line 96, in __call__
    self.response = self.func(*args, **kwargs)
  File "/var/task/cm_rotate_machine_process/lambda_function.py", line 88, in lambda_handler
    update_machine_credentials_in_awx(organization, vault, stack, environments)
  File "/var/task/cm_rotate_machine_process/library/actions.py", line 137, in update_machine_credentials_in_awx
    credentials = list(
  File "/opt/python/lib/python3.10/site-packages/towerlib/entities/core.py", line 374, in _get_entity_objects
    yield entity_object(self._tower, data)
  File "/opt/python/lib/python3.10/site-packages/towerlib/entities/credential.py", line 205, in __new__
    credential_type)
sspans-sbp commented 1 year ago

Not sure what's the best solution here, can the credential_type variable be replaced?

            LOGGER.warning('Could not dynamically load credential with type : "%s", trying a generic one.',
                           credential_type)
costastf commented 1 year ago

Assuming that the get_credential_type_by_id is well behaved, then we can check at line 198 if we actually have a value before we try to do stuff to it.

costastf commented 1 year ago

Do you mind giving https://github.com/schubergphilis/towerlib/tree/fix-unboundlocalerror a spin?

sspans-sbp commented 1 year ago

If run the diff with the code with that crashed and it works fine.

Unfortunately I cannot seem to reproduce the exact edge-case reliably - but this seems like a reasonable solution for the issue. I'll pull in the new version once relased and will let it run for a while with a production workload.

costastf commented 1 year ago

Should be fixed by v3.13.1 uploaded a short while ago.