-
Notifications
You must be signed in to change notification settings - Fork 700
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
Implement pwquality macro #12656
Implement pwquality macro #12656
Conversation
Hi @alanmcanonical. Thanks for your PR. I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
ef863ef
to
718293b
Compare
718293b
to
d0c1d87
Compare
Issue 1For sle15 test, I saw the oval log:
It indicates that system-auth doesn't exist in sle15, and this patch could solve this issue but need someone from suse to confirm
Issue 2As of Automatus UBI8 test It shows that |
@mpurg is currently testing it, we will take some time before merging this |
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.
Minor changes
{{{ bash_ensure_pam_module_options('/etc/pam.d/common-password', 'password', 'requisite', 'pam_pwquality.so', 'retry', "$var_password_pam_retry", "$var_password_pam_retry") }}} | ||
{{{ bash_pam_pwquality_enable() }}} | ||
PAM_FILE_PATH=/usr/share/pam-configs/cac_pwquality | ||
if grep -qE 'pam_pwquality\.so.*retry=[^[:space:]]' "$PAM_FILE_PATH"; then |
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.
We don't need to remove the retry parameter from cac_pwquality
. The file is created in bash_pam_pwquality_enable
macro and doesn't contain the parameter.
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.
Remove Done, delete these line.
@@ -15,6 +15,9 @@ for file in ${configuration_files[@]}; do | |||
"/etc/authselect/custom/testingProfile/$file" | |||
done | |||
authselect select --force custom/testingProfile | |||
{{% elif 'ubuntu' in product %}} | |||
rm -f /usr/share/pam-configs/*pwquality |
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.
Should we remove all pwquality configs or just cac_pwquality
?
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.
The container has no cac_pwquality config at default, so no need to delete it at all.
Code Climate has analyzed commit 73b8561 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 60.9% (0.0% change). View more on Code Climate. |
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.
lgtm, thanks!
Description:
Rationale: