-
Notifications
You must be signed in to change notification settings - Fork 839
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 missing permission for impersonation #1651
base: master
Are you sure you want to change the base?
Add missing permission for impersonation #1651
Conversation
Signed-off-by: Varsha B <[email protected]>
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.
Looks good. Is it possible to add a unit or e2e test?
Signed-off-by: Varsha B <[email protected]>
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.
Hi @anandf, the current steps mentioned here do not work for a ns scoped instance. Could you please suggest the required modifications?
Even after adding the managed-by label, I see this error:
message: 'ComparisonError: Failed to load live state: Namespace "guestbook" for Deployment "guestbook-ui" is not managed'
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.
Instead of guestbook
namespace, can you try using argocd-test-impersonation
as the destination ns ? For a namespace scoped ArgoCD
instance, you can deploy manifests only in the same ns as the RBAC permits only that by default. Otherwise, you have to create a ns guestbook
and apply the managed-by
label. You cannot test the CreateNamespace=true
sync policy in that case.
metadata: | ||
name: test-impersonation-ns | ||
labels: | ||
argocd.argoproj.io/managed-by: openshift-gitops |
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.
argocd.argoproj.io/managed-by: openshift-gitops | |
label `argocd.argoproj.io/managed-by` not required here |
metadata: | ||
name: guestbook | ||
labels: | ||
argocd.argoproj.io/managed-by: openshift-gitops |
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.
argocd.argoproj.io/managed-by: openshift-gitops | |
argocd.argoproj.io/managed-by: test-impersonation-ns |
metadata: | ||
name: test-impersonation-ns | ||
labels: | ||
argocd.argoproj.io/managed-by: openshift-gitops |
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.
argocd.argoproj.io/managed-by: openshift-gitops | |
label `argocd.argoproj.io/managed-by` not required here |
metadata: | ||
name: guestbook | ||
labels: | ||
argocd.argoproj.io/managed-by: openshift-gitops |
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 label argocd.argoproj.io/managed-by
is set to openshift-gitops
. However, since this ArgoCD instance is deployed in the test-impersonation-ns
namespace, I suggest updating the label value to reflect that. The updated label should be argocd.argoproj.io/managed-by: test-impersonation-ns.
argocd.argoproj.io/managed-by: openshift-gitops | |
argocd.argoproj.io/managed-by: test-impersonation-ns |
Signed-off-by: Varsha B <[email protected]>
Signed-off-by: Varsha B <[email protected]>
aa57947
to
d051b9f
Compare
df2a1e6
to
c005e25
Compare
Signed-off-by: Varsha B <[email protected]>
c005e25
to
f400f35
Compare
What type of PR is this?
/kind enhancement
What does this PR do / why we need it:
Because of missing permission in application controller clusterrole, impersonation feature does not work as expected. This PR aims to fix the same
Have you updated the necessary documentation?
Which issue(s) this PR fixes:
Fixes #?
How to test changes / Special notes to the reviewer: