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.

Sign up for GitHub

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

Jump to bottom

Handle CRLF when generating fingerprint #63918

Merged
merged 3 commits into from
Mar 21, 2023
Merged

Handle CRLF when generating fingerprint #63918

merged 3 commits into from
Mar 21, 2023

Conversation

twangboy
Copy link
Contributor

@twangboy twangboy commented Mar 20, 2023 •
edited
Loading

What does this PR do?

Handles .pub files that have CRLF line endings. They are converted to LF line endings so that the fingerprint is consistent.

What issues does this PR fix or reference?

Fixes: #63742

Previous Behavior

Fingerprints generated from files with CRLF line endings did not match fingerprints from files with LF line endings. Even when the content is the same.

New Behavior

Fingerprints now match.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

@twangboy twangboy requested a review from a team as a code owner March 20, 2023 23:15
@twangboy twangboy requested review from MKLeb and removed request for a team March 20, 2023 23:15
@twangboy twangboy added the Sulfur v3006.0 release code name and version label Mar 20, 2023
@twangboy twangboy added this to the Sulfur v3006.0 milestone Mar 20, 2023
@twangboy twangboy requested a review from s0undt3ch March 21, 2023 02:39
Ch3LL
Ch3LL previously approved these changes Mar 21, 2023
View reviewed changes
s0undt3ch
s0undt3ch suggested changes Mar 21, 2023
View reviewed changes
Copy link
Collaborator

@s0undt3ch s0undt3ch left a comment

Choose a reason for hiding this comment

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

Actually, this can be a single test parametrized test

tests/pytests/unit/utils/test_crypt.py Outdated Show resolved Hide resolved
s0undt3ch
s0undt3ch previously approved these changes Mar 21, 2023
View reviewed changes
@s0undt3ch s0undt3ch merged commit eeac747 into saltstack:3006.x Mar 21, 2023
@twangboy twangboy deleted the fingerprint branch March 23, 2023 15:29
@nf-brentsaner
Copy link

Serious question, why is the Base64 checksummed to the fingerprint instead of the raw bytes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s0undt3ch s0undt3ch s0undt3ch approved these changes

@MKLeb MKLeb Awaiting requested review from MKLeb MKLeb was automatically assigned from saltstack/team-core

@Ch3LL Ch3LL Awaiting requested review from Ch3LL

Assignees
No one assigned
Labels
Sulfur v3006.0 release code name and version
Projects
None yet
Milestone
Sulfur v3006.0
Development

Successfully merging this pull request may close these issues.
4 participants
@twangboy @nf-brentsaner @s0undt3ch @Ch3LL