Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[collection] Add "exists" state for credential module #13725

Conversation

l3acon
Copy link
Contributor

@l3acon l3acon commented Mar 20, 2023

SUMMARY

Add the enumeration exists to state for the credential module. This is a relatively simple change that allows the module to be used in the following situation:

  1. we want to ensure a credential resource of a particular name is available, but
  2. if the resource is already configured we do not want to modify any inputs (including secrets)

Since credentials have protections in place to safeguard their privacy, overwriting them can be (at best) annoying or (at worst) dangerous. This is an attempt to address #13169.

ISSUE TYPE
  • New or Enhanced Feature
COMPONENT NAME
  • Collection

@github-actions github-actions bot added component:awx_collection issues related to the collection for controlling AWX community labels Mar 20, 2023
@john-westcott-iv
Copy link
Member

In the collection there is an controller_api lookup that you could leverage to see if a credential exists. So this could be implemented like so:

    - name: Create credential if needed
      awx.awx.credential:
        credential_type: Machine
        inputs:
          username: John
          password: 'test'
        name: Test Machine Credential
        organization: Default
      when: "lookup('awx.awx.controller_api', 'credentials', query_params={ 'name': 'Test Machine Credential' }) | length == 0"

That being said, I'm not opposed to an exists option in the state but we would probably want to look at the other modules to see if we should add exists in other areas as well for consistency.

@john-westcott-iv
Copy link
Member

Perhaps we could discuss this at the next community meeting? If that would be something you are interested in could you leave a comment on #13687?

@willtome
Copy link

@john-westcott-iv I was unaware of the api lookup. That's pretty cool. I'm not sure how that would integrate to work with the controller_collection though.

@sean-m-sullivan do you think a lookup could work with the way the roles are today or would an additional parameter be better?

@sean-m-sullivan
Copy link
Contributor

sean-m-sullivan commented Mar 22, 2023

@willtome like this?
https://github.com/redhat-cop/controller_configuration/blob/devel/roles/object_diff/tasks/credentials.yml
However this will not remove the item from being updated if it already exists, its more for cleaning up controllers.

@sean-m-sullivan
Copy link
Contributor

sean-m-sullivan commented Mar 22, 2023

I think this state for credentials is worth it, as you would stop changing credentials in a simple way, most of the other modules its just putting in settings, this prevents a password change unless it specifically is needed, but John is right like with all things ansible, there are 20 ways of doing it.

@john-westcott-iv john-westcott-iv self-assigned this Mar 23, 2023
@l3acon
Copy link
Contributor Author

l3acon commented Mar 23, 2023

@john-westcott-iv thanks for pointing those out. I wasn't sure what the unit test should look like since the integration tests seem to cover the use-cases I have. Totally open to suggestions or ideas.

@TheRealHaoLiu TheRealHaoLiu changed the title [collection] Add existential state for credential module [collection] Add "exists" state for credential module Apr 14, 2023
@thedoubl3j
Copy link
Member

kicking CI

@TheRealHaoLiu TheRealHaoLiu merged commit b75b84e into ansible:devel Apr 19, 2023
@john-westcott-iv
Copy link
Member

I was thinking about this PR this morning and I am wondering if you were hitting the issue in #13704. tldr; the password was always being changed even when it should have been skipped because the user api page did not contain "password": "$encrypted$" which threw off the module_util logic.

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.

7 participants