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

ipatrust: Set valid choices for trust_type. #808

Merged
merged 2 commits into from
Apr 27, 2022

Conversation

rjeffman
Copy link
Member

Ensure only valid choices for trust_type ('ad') are available for the
module parameter.

@rjeffman rjeffman requested a review from t-woerner April 12, 2022 21:50
@varunmylaraiah varunmylaraiah self-requested a review April 26, 2022 13:19
@varunmylaraiah
Copy link
Collaborator

@rjeffman We need to update README-trust.md for trust_type.

Ensure only valid choices for trust_type ('ad')  are available for the
module parameter.
This patch updates the ipatrust documentation about the 'trust_type'
parameter, and changes one password to be similar to the standard
passwords used in other modules.
@rjeffman
Copy link
Member Author

@varunmylaraiah good catch. PR updated.

Copy link
Member

@t-woerner t-woerner left a comment

Choose a reason for hiding this comment

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

LGTM

@t-woerner
Copy link
Member

Do we need a test to make sure that absence trust_type and "trust_type: ad" are doing the same?

@rjeffman
Copy link
Member Author

Do we need a test to make sure that absence trust_type and "trust_type: ad" are doing the same?

I don't think we need those tests, in this PR. PR #810 enhances ipatrust tests and could add these tests. What this PR does, in fact, is to delegate the validation of the value passed to trust_type to Ansible, and the module itself has nothing else to do. If the value is invalid, that would be on Ansible side.

@varunmylaraiah
Copy link
Collaborator

LGTM

@t-woerner t-woerner merged commit 2fa9ed9 into freeipa:master Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants