-
Notifications
You must be signed in to change notification settings - Fork 110
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
Removes md5 and sha1 from checksum defaults #1123
Conversation
Attached issue: https://pulp.plan.io/issues/8246 |
docs/settings.rst
Outdated
See :ref:`Configuration` for details on how to change configuration-options. | ||
ALLOWED_CONTENT_CHECKSUMS = ["sha224", "sha256", "sha384", "sha512"] | ||
|
||
The entire set of possible checksums are: ``md5``, ``sha1``, ``sha224``, ``sha256``, ``sha384``, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/possible/supported
@@ -34,7 +34,7 @@ if [[ "$TEST" = "docs" || "$TEST" = "publish" ]]; then | |||
echo "Validating OpenAPI schema..." | |||
cat $PWD/.ci/scripts/schema.py | cmd_stdin_prefix bash -c "cat > /tmp/schema.py" | |||
cmd_prefix bash -c "python3 /tmp/schema.py" | |||
# cmd_prefix bash -c "pulpcore-manager spectacular --file pulp_schema.yml --validate" | |||
cmd_prefix bash -c "pulpcore-manager spectacular --file pulp_schema.yml --validate" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there changes in plugin_template?
Basically is it a custom change or rolled out from the plugin template?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kind of surprisingly, there weren't changes needed in the plugin template. I reapplied the HEAD of plugin_template and it produced this. This particular line is just some unrelated change being brought in from its reapplication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea this check was added a while ago but enabled post-3.10.
CHANGES/8246.removal
Outdated
@@ -0,0 +1,3 @@ | |||
Adjusts the ``ALLOWED_CONTENT_CHECKSUMS`` setting to remove ``md5`` and ``sha1`` since they are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick, changelogs should use past tense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always mistakenly use imperative, but that's for commit messages.
The md5 and sha1 checksums are not considered secure and therefore should not be included as available hashers for Pulp to perform Artifact integrity checks with. This PR: * Removes them from the default in settings * Updates the `ALLOWED_CONTENT_CHECKSUMS` settings documentation. * Re-applies the plugin_tepmlate to no longer modify the set of allowed checksums in the CI environment. closes #8246
95e6989
to
8dae0b5
Compare
The md5 and sha1 checksums are not considered secure and therefore
should not be included as available hashers for Pulp to perform Artifact
integrity checks with.
This PR:
ALLOWED_CONTENT_CHECKSUMS
settings documentation.checksums in the CI environment.
closes #8246