-
Notifications
You must be signed in to change notification settings - Fork 61
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
✨ auth: use synthetic user/group when service account is not defined #1816
base: main
Are you sure you want to change the base?
✨ auth: use synthetic user/group when service account is not defined #1816
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
dfc5ccb
to
fbcc4a5
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1816 +/- ##
=======================================
Coverage 68.98% 68.99%
=======================================
Files 66 67 +1
Lines 5243 5276 +33
=======================================
+ Hits 3617 3640 +23
- Misses 1395 1403 +8
- Partials 231 233 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
11210fc
to
e44c4d2
Compare
I'd like to have a discussion on the |
1c2f38f
to
73e6080
Compare
I put next to no critical thought into the name and group names. @thetechnick you propose the following?
|
73e6080
to
fb13084
Compare
@joelanford precisely. 👍 |
59b818a
to
c9575d3
Compare
hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml
Outdated
Show resolved
Hide resolved
c9575d3
to
76ca00e
Compare
cd6d528
to
7546680
Compare
279322a
to
48cb945
Compare
} | ||
} | ||
|
||
func ClusterExtensionUserRestConfigMapper(tokenGetter *authentication.TokenGetter) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) { |
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.
Should we pass a enableSyntheticUserAuthentication
function parameter so that we can reference the feature gate only in main.go
?
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.
is that a general goal that we have that FGs should only be referenced in main? I might need to update the Single-OwnNamespace FG if that's the case. My only worry about it is that if it's only checked somewhere down the stack, we end up having to thread it all the way down, which could be painful. What is the value of having it in main? In the end I end up searching the code for usages of the FG anyway. Is it helpful in other contexts as well?
Signed-off-by: Joe Lanford <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
48cb945
to
9904073
Compare
Description
Today at a meeting among maintainers of OLMv1, we discussed an idea that @thetechnick proposed awhile back. That is: stop using service accounts and service account tokens. Instead use synthetic names with impersonation.
While we are now 1.0.0 with support for service accounts, we can deprecate that feature and recommend attaching permissions to synthetic users/groups instead.
This PR demonstrates how we might do this. But with the API change, we should write up a detailed design and gain consensus.
This PR uses:
"olm:clusterextensions:<clusterExtensionName>"
["olm:clusterextensions", "system:authenticated"]
(not sure we need to be explicit aboutsystem:authenticated
, but it can't hurt)Reviewer Checklist