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

Update check_password function for newer RabbitMQ versions. #56193

Merged
merged 10 commits into from
Apr 11, 2020

Conversation

11chrisadams11
Copy link
Contributor

@11chrisadams11 11chrisadams11 commented Feb 18, 2020

What does this PR do?

Newer versions of RabbitMQ have a differing output to older versions that breaks the search regex in the check_password function. This caused rabbitmq_user states to fail. This PR adds a check for the new status output as well, so the function will work for all versions of RabbitMQ.

What issues does this PR fix or reference?

Follow up to #33588
Fixes issues #56258 & #55376

Previous Behavior

The check_password function would only search the regex against the status output of older versions of RabbitMQ, which would cause errors on newer versions of RabbitMQ.

New Behavior

The check_password function will now search for the regex of the status output for both new and old versions of RabbitMQ.

Tests written?

Yes

Commits signed with GPG?

No

Newer versions of RabbitMQ have a differing output to older versions that breaks the search regex in the check_password function. This caused rabbitmq_user states to fail. This adds a check for the new status output as well, so the function will work for all versions of RabbitMQ.
@11chrisadams11 11chrisadams11 requested a review from a team as a code owner February 18, 2020 21:29
@ghost ghost requested a review from xeacott February 18, 2020 21:29
DmitryKuzmenko
DmitryKuzmenko previously approved these changes Apr 6, 2020
@DmitryKuzmenko DmitryKuzmenko added the Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Apr 6, 2020
@DmitryKuzmenko
Copy link
Contributor

@11chrisadams11 Could you please add a test case for it?

Add unit tests to verify older and newer versions of RabbitMQ are both working.
@11chrisadams11
Copy link
Contributor Author

@DmitryKuzmenko Added test cases for the different versions.

@DmitryKuzmenko
Copy link
Contributor

Perfect! Thank you for updating this with tests. I've blackened the code now tests should be green. Let's wait for ci.

@dwoz dwoz merged commit d0e0ed6 into saltstack:master Apr 11, 2020
@sagetherage sagetherage added the ZRelease-Sodium retired label label May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases ZRelease-Sodium retired label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants