-
Notifications
You must be signed in to change notification settings - Fork 6.7k
fix(appset): add applicationset leader election to roles and clean up (#14369) #23976
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
Changes from all commits
327104d
2798ae3
9aa6c4c
ac8d4d9
e88b9d5
ff31957
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -1,90 +1,77 @@ | ||||
| apiVersion: rbac.authorization.k8s.io/v1 | ||||
| kind: ClusterRole | ||||
| metadata: | ||||
| name: argocd-applicationset-controller | ||||
| labels: | ||||
| app.kubernetes.io/name: argocd-applicationset-controller | ||||
| app.kubernetes.io/part-of: argocd | ||||
| app.kubernetes.io/component: applicationset-controller | ||||
| name: argocd-applicationset-controller | ||||
| rules: | ||||
| - apiGroups: | ||||
| - argoproj.io | ||||
| resources: | ||||
| - applications | ||||
| - applicationsets | ||||
| - applicationsets/finalizers | ||||
| verbs: | ||||
| - create | ||||
| - delete | ||||
| - get | ||||
| - list | ||||
| - patch | ||||
| - update | ||||
| - watch | ||||
| - apiGroups: | ||||
| - argoproj.io | ||||
| resources: | ||||
| - applicationsets/status | ||||
| verbs: | ||||
| - get | ||||
| - patch | ||||
| - update | ||||
| - apiGroups: | ||||
| - argoproj.io | ||||
| resources: | ||||
| - appprojects | ||||
| verbs: | ||||
| - get | ||||
| - list | ||||
| - watch | ||||
| - apiGroups: | ||||
| - "" | ||||
| resources: | ||||
| - events | ||||
| verbs: | ||||
| - create | ||||
| - get | ||||
| - list | ||||
| - patch | ||||
| - watch | ||||
| - apiGroups: | ||||
| - "" | ||||
| resources: | ||||
| - configmaps | ||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leader election no longer requires config maps - kubernetes-sigs/controller-runtime#1773 |
||||
| verbs: | ||||
| - create | ||||
| - update | ||||
| - delete | ||||
| - get | ||||
| - list | ||||
| - patch | ||||
| - watch | ||||
| - apiGroups: | ||||
| - "" | ||||
| resources: | ||||
| - secrets | ||||
| verbs: | ||||
| - get | ||||
| - list | ||||
| - watch | ||||
| - apiGroups: | ||||
| - apps | ||||
| - extensions | ||||
| resources: | ||||
| - deployments | ||||
| verbs: | ||||
| - get | ||||
| - list | ||||
| - watch | ||||
| - apiGroups: | ||||
| - coordination.k8s.io | ||||
| resources: | ||||
| - leases | ||||
| verbs: | ||||
| - create | ||||
| - delete | ||||
| - get | ||||
| - list | ||||
| - patch | ||||
| - update | ||||
| - watch | ||||
| - apiGroups: | ||||
| - argoproj.io | ||||
| resources: | ||||
| - applications | ||||
| - applicationsets | ||||
| - applicationsets/finalizers | ||||
| verbs: | ||||
| - create | ||||
| - delete | ||||
| - get | ||||
| - list | ||||
| - patch | ||||
| - update | ||||
| - watch | ||||
| - apiGroups: | ||||
| - argoproj.io | ||||
| resources: | ||||
| - appprojects | ||||
| verbs: | ||||
| - get | ||||
| - list | ||||
| - watch | ||||
| - apiGroups: | ||||
| - argoproj.io | ||||
| resources: | ||||
| - applicationsets/status | ||||
| verbs: | ||||
| - get | ||||
| - patch | ||||
| - update | ||||
| - apiGroups: | ||||
| - "" | ||||
| resources: | ||||
| - events | ||||
| verbs: | ||||
| - create | ||||
| - get | ||||
| - list | ||||
| - patch | ||||
| - watch | ||||
| - apiGroups: | ||||
| - "" | ||||
| resources: | ||||
| - secrets | ||||
| - configmaps | ||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think the applicationset controller needs any config map perms now?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. some appset plugin generator still needs access to configmaps eg.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But I don't see any need for write perms in the code too, so this should be fine |
||||
| verbs: | ||||
| - get | ||||
| - list | ||||
| - watch | ||||
| # argocd-applicationset-controller leader election rules | ||||
| # Create with resourceNames fails, so use a separate rule for the lease creation | ||||
| - apiGroups: | ||||
| - coordination.k8s.io | ||||
| resources: | ||||
| - leases | ||||
| verbs: | ||||
| - create | ||||
| - apiGroups: | ||||
| - coordination.k8s.io | ||||
| resources: | ||||
| - leases | ||||
| resourceNames: | ||||
| # Defined in `cmd/argocd-applicationset-controller/commands/applicationset_controller.go` | ||||
| - 58ac56fa.applicationsets.argoproj.io | ||||
| verbs: | ||||
| - get | ||||
| - update | ||||
| - create | ||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 am confused why the applicationset controller needs get, list, watch for deployments. I removed it but can add it back.
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.
Agreed, I checked these permissions have been there from the very beginning but couldn't find any usage of these perms in the appset code so far.