-
Notifications
You must be signed in to change notification settings - Fork 272
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
fix: missing namespace on clusterrolebinding in install manifests #860
Conversation
Signed-off-by: Cheng Fang <[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.
lgtm!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #860 +/- ##
==========================================
+ Coverage 73.50% 74.39% +0.89%
==========================================
Files 31 31
Lines 3144 3144
==========================================
+ Hits 2311 2339 +28
+ Misses 695 669 -26
+ Partials 138 136 -2 ☔ View full report in Codecov by Sentry. |
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.
LGTM!!
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.
warnings/notes must be indented properly
Co-authored-by: Jann Fischer <[email protected]> Signed-off-by: Cheng Fang <[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.
LGTM, thanks!
…goproj-labs#860) Signed-off-by: Cheng Fang <[email protected]> Co-authored-by: Jann Fischer <[email protected]> Signed-off-by: Tchoupinax <[email protected]>
Fixes #859
PR #848 added a clusterrolebinding without specifying SA namespace to the install manifests, causing the installation to fail. This PR fixes the problem by adding a default SA namespace
argocd
to the clusterrolebinding, and also updates the docs with a reminder/warning for the need to change the namespace if necessary. Tested with installing both plain manifest and kustomization.