feat: Wire core server for Impersonation #1612
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Callisto13
force-pushed
the
refactor-sa
branch
2 times, most recently
from
da22626
to
f56c707
Compare
Callisto13
changed the title
|
@JamWils @sympatheticmoose could you 2 look at the Chart changes in commit 337d703? |
yitsushi
approved these changes
Mar 9, 2022
|
No-one get keen and merge anything - looking into something curious in the superuser flow. |
jpellizzari
approved these changes
Mar 9, 2022
| @@ -74,6 +74,8 @@ To set up a development environment for the CLI | |||
| 5. Run `./bin/gitops install --config-repo=<repo url>` (or just `flux install -n flux-system` if you don't care about doing the whole dance.) | |||
| 6. Start the in-cluster API replacement job (powered by [http://tilt.dev](tilt.dev)) with `make cluster-dev` | |||
| 7. make or make unit-tests to ensure everything built correctly. | |||
| 8. The UI will start immediately on port `9001`. Auth is now always on (I do not recommend | |||
| turning it off). The password is `dev`. | |||
1. Strip out the service account (mostly) in the Helm Chart. To keep the app's scope limited we are making it "impersonate only". The caveat to this is that the Profiles functionality is always on, with queries not made by the user. Therefore in this case there is nothing to impersonate. So *for now*, the wego-app service account has an extra role which will let it query those resources. 2. Add an Admin user to the Chart. With the "impersonate only" plan, we require users to enable auth by default. I have therefore added a template to create an Admin user with the required permissions. The actually password is configurable via values and is left empty. 3. Update the Tiltfile to deploy this Chart locally. All current local dev instructions remain true. The dev password is `dev`. 4. Create a default const to hold the value of the default claims subject (admin).
1. Move the `ImpersonatingConfigGetter` into the `kube` package. This felt like a "natural" place to me, feel free to suggest elsewhere if you disagree. It has to be moved _somewhere_ to avoid import cycles with t'other server. 2. Update the core server to use the default client getter rather than our placeholder. With the import cycle fixed this is now doable. This default ClientGetter takes a ConfigGetter, so now core will Impersonate. 3. Update the server wiring to receive a `CoreServerConfig`, following the same pattern as seen in the `ApplicationsServer` and the `ProfilesServer`. I have included a `Logger` on the server, which I intend to use in my next ticket, but feel free to complain that it can technically be dropped from this PR.
false alarm, all good |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
2 commits here:
337d703:
feat: Refactor chart service account
app's scope limited we are making it "impersonate only". The caveat
to this is that the Profiles functionality is always on, with queries
not made by the user. Therefore in this case there is nothing to
impersonate. So for now, the wego-app service account has an
extra role which will let it query those resources.
require users to enable auth by default. I have therefore added a
template to create an Admin user with the required permissions.
The actual password is configurable via values and is left empty.
dev instructions remain true. The dev password is
dev.subject (admin).
f56c707:
feat: Wire the core server to Impersonate queries
ImpersonatingConfigGetterinto thekubepackage. Thisfelt like a "natural" place to me, feel free to suggest elsewhere if
you disagree. It has to be moved somewhere to avoid import cycles
with t'other server.
our placeholder. With the import cycle fixed this is now doable.
This default ClientGetter takes a ConfigGetter, so now core will
Impersonate.
CoreServerConfig, followingthe same pattern as seen in the
ApplicationsServerand theProfilesServer. I have included aLoggeron the server, which Iintend to use in my next ticket, but feel free to complain that it
can technically be dropped from this PR.