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

argocd-util should allow editing project policies in bulk #2615

Merged
merged 3 commits into from Dec 5, 2019

Conversation

alexmt
Copy link
Collaborator

@alexmt alexmt commented Oct 31, 2019

Closes #2614

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Optional. My organization is added to the README.
  • I've signed the CLA and my build is green (troubleshooting builds).

CLI example:

# add permission to execute any action to `deployer` role in all projects
argocd-util projects update-role-policy '*' set 'action/*' --resource applications --scope '*' --permission allow --dry-run=false --role  '*deployer*'
# remove permission to override resources from `deployer` role in all projects
argocd-util projects update-role-policy '*' remove override --role '*deployer*'

@codecov
Copy link

codecov bot commented Nov 1, 2019

Codecov Report

Merging #2615 into master will decrease coverage by 0.01%.
The diff coverage is 36.43%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2615      +/-   ##
==========================================
- Coverage   37.93%   37.92%   -0.02%     
==========================================
  Files         116      117       +1     
  Lines       16130    16259     +129     
==========================================
+ Hits         6119     6166      +47     
- Misses       9200     9274      +74     
- Partials      811      819       +8
Impacted Files Coverage Δ
cmd/argocd-util/main.go 0.79% <0%> (-0.01%) ⬇️
cmd/argocd-util/projects.go 37% <37%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7065229...623ad65. Read the comment docs.

@alexec alexec self-assigned this Nov 1, 2019
cmd/argocd-util/projects.go Outdated Show resolved Hide resolved
cmd/argocd-util/projects.go Outdated Show resolved Hide resolved
cmd/argocd-util/projects.go Outdated Show resolved Hide resolved
alexec
alexec previously requested changes Nov 1, 2019
cmd/argocd-util/projects.go Outdated Show resolved Hide resolved
@alexmt
Copy link
Collaborator Author

alexmt commented Nov 18, 2019

@alexec can you take a look again please. your comments were addressed

@alexec alexec dismissed their stale review November 21, 2019 02:34

changes made

Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

Some minor comments - but I think we should sit down and discus this, as I don't fully understand

return command
}

func globMatch(pattern string, val string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

rename val to something else? path?

}

func globMatch(pattern string, val string) bool {
if pattern == "*" {
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't this get picked up by the filepath.Match?


func getModification(modification string, resource string, scope string, permission string) (func(string, string) string, error) {
switch modification {
case "set":
Copy link
Contributor

Choose a reason for hiding this comment

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

add unit test please

}
actionPolicyIndex := -1
for i := range role.Policies {
parts := split(role.Policies[i], ",")
Copy link
Contributor

Choose a reason for hiding this comment

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

untested?

updated = true
role.Policies = append(role.Policies[:actionPolicyIndex], role.Policies[actionPolicyIndex+1:]...)
} else if actionPolicyIndex > -1 && policyPermission != "" {
updated = true
Copy link
Contributor

Choose a reason for hiding this comment

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

untested?

Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

Ops. I believe I request changes for this.

Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

Conditional approval: please create an issue to complete the tests for this.

@alexmt alexmt merged commit 5077be9 into argoproj:master Dec 5, 2019
@alexmt alexmt deleted the projects-backfill-util branch December 5, 2019 21:35
alexmt pushed a commit that referenced this pull request Dec 5, 2019
* Implement 'argocd-util projects update-role-policy' command which allows to update multiple project policies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

argocd-util should allow editing project policies in bulk
2 participants