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

Add roles required for user reset #634

Merged
merged 2 commits into from
Sep 22, 2021
Merged

Conversation

MAHDTech
Copy link
Contributor

@MAHDTech MAHDTech commented Jul 26, 2021

What does this PR do?

This PR adds the following;

  • Role for basic git management (pull/push)
  • Role for removing files based on a regex content search
  • Role for identity removal with success/fail/skip counters
  • Playbook that makes use of those roles for use in LodeStar
  • Check for leftover TODO items
  • Incorporate feedback
  • Fix regression with Ansible inventories added in Ansible Tower: Update manage-inventories #636 (first run vs updates)
  • Resolve comments

How should this be tested?

Roles can be tested individually, or demonstrated in conjunction with other changes in linked PR below.

Is there a relevant Issue open for this?

None

Other Relevant info, PRs, etc.

This is the first part of a 2-part PR, with the additional information available in lodestar-automation #66

  • Issue #796 (Internal)
  • Inventory sample (Internal)

Other related PR's include

People to notify

cc: @redhat-cop/infra-ansible

@MAHDTech MAHDTech marked this pull request as draft July 27, 2021 00:34
@MAHDTech MAHDTech force-pushed the feature/user-reset branch from 23f1f8a to ad4ae2c Compare July 27, 2021 13:24
@MAHDTech MAHDTech marked this pull request as ready for review July 27, 2021 13:26
@MAHDTech MAHDTech force-pushed the feature/user-reset branch from e816564 to 0d507e5 Compare July 30, 2021 12:20
Copy link
Contributor

@oybed oybed left a comment

Choose a reason for hiding this comment

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

some inline comments.

Also, let's find a better name for the grouping of roles to manage files - i.e.: file-management or something like that instead of files as the latter is generic and can be confused for some built-in location.

roles/scm/git/tasks/prep.yml Outdated Show resolved Hide resolved
roles/scm/git/tasks/pull.yml Outdated Show resolved Hide resolved
@MAHDTech
Copy link
Contributor Author

@oybed, I've incorporated those changes and will also update the corresponding paths in linked playbooks.

@MAHDTech MAHDTech requested a review from oybed August 18, 2021 12:57
@MAHDTech MAHDTech marked this pull request as draft August 24, 2021 09:54
@MAHDTech
Copy link
Contributor Author

Moved back to draft for additional testing

@MAHDTech MAHDTech marked this pull request as ready for review August 30, 2021 01:09
Copy link
Contributor

@oybed oybed left a comment

Choose a reason for hiding this comment

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

Overall a lot of good content. Main thread of comments is around git configuration and how we need to be careful to not step on someones existing configuration - especially avoid clobbering and/or removing it.

Some comment inline

roles/scm/git/tasks/commit.yml Outdated Show resolved Hide resolved
roles/scm/git/handlers/main.yml Outdated Show resolved Hide resolved
roles/scm/git/tasks/prep.yml Outdated Show resolved Hide resolved
roles/scm/git/tasks/pull.yml Outdated Show resolved Hide resolved
roles/scm/git/tasks/pull.yml Outdated Show resolved Hide resolved
roles/scm/git/tasks/pull.yml Outdated Show resolved Hide resolved
roles/scm/git/tasks/pull.yml Show resolved Hide resolved
roles/file-management/remove-files/tasks/remove-file.yml Outdated Show resolved Hide resolved
@MAHDTech MAHDTech requested a review from oybed September 15, 2021 04:52
roles/scm/git/handlers/remove_git_creds.yml Outdated Show resolved Hide resolved
roles/scm/git/handlers/remove_git_creds.yml Outdated Show resolved Hide resolved
roles/scm/git/tasks/pull.yml Outdated Show resolved Hide resolved
@MAHDTech MAHDTech requested a review from oybed September 20, 2021 12:43
roles/scm/git/tasks/pull.yml Outdated Show resolved Hide resolved
- Undo permissions mistake
- Dont remove global git creds
- Remove jinja bool filters
- Use bracket notation where required
- Use more descriptive filename
- Use clearer task name
Copy link
Contributor

@oybed oybed left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@oybed oybed left a comment

Choose a reason for hiding this comment

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

LGTM

@oybed oybed merged commit 11d8c29 into redhat-cop:main Sep 22, 2021
@MAHDTech MAHDTech deleted the feature/user-reset branch September 22, 2021 12:47
This was referenced Sep 23, 2021
jfilipcz pushed a commit to jfilipcz/infra-ansible that referenced this pull request Oct 20, 2021
- Undo permissions mistake
- Dont remove global git creds
- Remove jinja bool filters
- Use bracket notation where required
- Use more descriptive filename
- Use clearer task name

Co-authored-by: Øystein Bedin <oybed@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants