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

use var values for pam_faillock #52

Merged
merged 1 commit into from
Apr 26, 2023
Merged

use var values for pam_faillock #52

merged 1 commit into from
Apr 26, 2023

Conversation

jayolinares
Copy link
Contributor

Overall Review of Changes:
Replace hard coded values for deny and unlock_time with variables.

github-actions[bot]
github-actions bot reviewed Apr 13, 2023
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on opening your first pull request and thank you for taking the time to help improve Ansible-Lockdown!
Please join in the conversation happening on the Discord Server as well.

@uk-bolly uk-bolly self-assigned this Apr 13, 2023
@uk-bolly
Copy link
Member

Hi @jayolinares

Thank you for taking the time for the PR, great catch and improvement.
Unfortunately it is not meeting DCO requirements, this does required GPG signature and signed-off by in order to pass these. https://github.com/ansible-lockdown/RHEL9-CIS/pull/52/checks?check_run_id=12710469440
If you could amend those happy to accept this.

Many thanks

uk-bolly

@jayolinares
Copy link
Contributor Author

Hi @uk-bolly
My bad, should have done that earlier. Anyways it has been signed-off and I can see the checks are successful.
Thanks.

@MrSteve81 MrSteve81 self-assigned this Apr 13, 2023
uk-bolly
uk-bolly previously approved these changes Apr 13, 2023
View reviewed changes
Copy link
Member

@uk-bolly uk-bolly left a comment

Choose a reason for hiding this comment

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

Great catch

@MrSteve81 MrSteve81 self-requested a review April 13, 2023 13:49
MrSteve81
MrSteve81 previously approved these changes Apr 13, 2023
View reviewed changes
Copy link
Contributor

@MrSteve81 MrSteve81 left a comment

Choose a reason for hiding this comment

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

Looks Good! Great Find

@MrSteve81 MrSteve81 added the enhancement New feature or request label Apr 13, 2023
@uk-bolly
Copy link
Member

Hi @uk-bolly My bad, should have done that earlier. Anyways it has been signed-off and I can see the checks are successful. Thanks.

hi @jayolinares

Apologies, It appears there is still an issue. Signed-off by appears good but GPG doesn't appear to be there?

thanks

uk-bolly

@jayolinares jayolinares dismissed stale reviews from MrSteve81 and uk-bolly via fb4216b April 15, 2023 12:24
@jayolinares
Copy link
Contributor Author

Hi @uk-bolly,

I have signed it (sorry wasn't aware that it's needed and missed the note above).
The commit is now marked as 'verified'. However the pipeline/tf build is failing, if you could check that please?

Thanks

MrSteve81
MrSteve81 reviewed Apr 26, 2023
View reviewed changes
Copy link
Contributor

@MrSteve81 MrSteve81 left a comment

Choose a reason for hiding this comment

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

Looks Good Here and thanks for fixing the sign off.

@MrSteve81 MrSteve81 requested a review from uk-bolly April 26, 2023 14:53
georgenalen
georgenalen approved these changes Apr 26, 2023
View reviewed changes
Copy link
Contributor

@georgenalen georgenalen left a comment

Choose a reason for hiding this comment

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

Looks good!

@MrSteve81 MrSteve81 removed the request for review from uk-bolly April 26, 2023 14:57
@MrSteve81 MrSteve81 merged commit c20b5ed into ansible-lockdown:devel Apr 26, 2023
@jayolinares jayolinares deleted the faillock-vars branch April 27, 2023 00:38
@uk-bolly uk-bolly mentioned this pull request Jun 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MrSteve81 MrSteve81 MrSteve81 left review comments

@github-actions github-actions[bot] github-actions[bot] left review comments

@georgenalen georgenalen georgenalen approved these changes

@uk-bolly uk-bolly uk-bolly left review comments

Assignees

@uk-bolly uk-bolly

@MrSteve81 MrSteve81

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jayolinares @uk-bolly @georgenalen @MrSteve81