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

Document keepers property for application and service principal passwords #572

Merged
merged 2 commits into from
Sep 22, 2021

Conversation

manicminer
Copy link
Member

Closes: #570

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

LGTM 🩹

@katbyte
Copy link
Collaborator

katbyte commented Sep 15, 2021

thou one might argue that "keepers" is a bad name for this property as how does that equate "rotate password when" 🤔

@manicminer
Copy link
Member Author

Yeah it's not the most intuitive name tbf. I took the concept and the naming from the random provider where it seems to be aiming for a generic approach. Totally happy to rename it to something more specific, though I'm struggling to think of anything good :D

@katbyte
Copy link
Collaborator

katbyte commented Sep 16, 2021

ahh given there is a keeper password manager might be best to change hah, doesn't seem like its a standard term for it? - no reason not to be verbose rotate_password_when_changed ?

@manicminer manicminer modified the milestones: v2.3.0, v2.4.0 Sep 17, 2021
@manicminer
Copy link
Member Author

ahh given there is a keeper password manager might be best to change hah, doesn't seem like its a standard term for it? - no reason not to be verbose rotate_password_when_changed ?

Sounds perfect 👌

@manicminer
Copy link
Member Author

@katbyte Have renamed this, do you think we need to support the old property name given that it was undocumented?

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

like it! thanks for making that change 🙌 LGTM

@manicminer manicminer merged commit 84797b1 into main Sep 22, 2021
@manicminer manicminer deleted the docs/password-keepers branch September 22, 2021 20:41
manicminer added a commit that referenced this pull request Sep 22, 2021
@github-actions
Copy link

This functionality has been released in v2.4.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keepers field should be documented azuread_service_principal_password
2 participants