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

htpasswd: deprecate crypt_scheme #6841

Merged

Conversation

russoz
Copy link
Collaborator

@russoz russoz commented Jul 3, 2023

SUMMARY

Technical nit-picking. What htpasswd does with the .htpasswd fiels used by Apache httpd is hashing, not encrypting. This PR renames the parameter crypt_scheme to hash_scheme, and add crypt_scheme as a deprecated alias of that option. It also updates the docs.

ISSUE TYPE
  • Docs Pull Request
  • Feature Pull Request
COMPONENT NAME

htpasswd

@russoz
Copy link
Collaborator Author

russoz commented Jul 3, 2023

This PR will need a rebase after #6840 is merged.

@ansibullbot
Copy link
Collaborator

cc @None
click here for bot help

@ansibullbot ansibullbot added docs module module plugins plugin (any type) labels Jul 3, 2023
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-7 labels Jul 3, 2023
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Since the htpasswd module is very old, I would proceed differently:

  1. Only do the rename in this PR, add the old name as an alias.
  2. In 8.0.0, deprecate the alias.
  3. In 10.0.0, remove the alias.

What do you think?

@ansibullbot ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jul 3, 2023
@russoz russoz force-pushed the htpasswd-hash-scheme branch from 738c8e0 to 7cb6e81 Compare July 3, 2023 20:17
@russoz
Copy link
Collaborator Author

russoz commented Jul 3, 2023

Since the htpasswd module is very old, I would proceed differently:

  1. Only do the rename in this PR, add the old name as an alias.
  2. In 8.0.0, deprecate the alias.
  3. In 10.0.0, remove the alias.

What do you think?

Okie, slowing down :-)

@ansibullbot ansibullbot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jul 3, 2023
@felixfontein felixfontein merged commit 4d704c0 into ansible-collections:main Jul 6, 2023
@patchback
Copy link

patchback bot commented Jul 6, 2023

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/4d704c03dfb26bfab75970310f2c81a05f919b96/pr-6841

Backported as #6858

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jul 6, 2023
patchback bot pushed a commit that referenced this pull request Jul 6, 2023
* htpasswd: rename crypt_scheme with hash_scheme

* add changelog frag

* fixed chglog frag

* adjusted code for parameter name change

(cherry picked from commit 4d704c0)
@felixfontein
Copy link
Collaborator

@russoz thanks for working on this!

felixfontein pushed a commit that referenced this pull request Jul 6, 2023
…me (#6858)

htpasswd: deprecate crypt_scheme (#6841)

* htpasswd: rename crypt_scheme with hash_scheme

* add changelog frag

* fixed chglog frag

* adjusted code for parameter name change

(cherry picked from commit 4d704c0)

Co-authored-by: Alexei Znamensky <[email protected]>
@russoz russoz deleted the htpasswd-hash-scheme branch July 6, 2023 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs module module plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants