-
Notifications
You must be signed in to change notification settings - Fork 108
add service account with allow-app-sharing-role permissions #2917
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
Draft
Adam-D-Lewis
wants to merge
41
commits into
main
Choose a base branch
from
jhub_apps_user
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 11 commits
Commits
Show all changes
41 commits
Select commit
Hold shift + click to select a range
6bc13de
add jhub apps service account with admin permissions
Adam-D-Lewis a2e1620
Merge branch 'main' into jhub_apps_user
Adam-D-Lewis 234baa2
reduce permissions
Adam-D-Lewis d609271
cleanup
Adam-D-Lewis 01d1d5d
consolidate calls
Adam-D-Lewis 1bfe644
revert to non service account user for jhub apps startup apps
Adam-D-Lewis a4943bb
cleanup
Adam-D-Lewis 5f9834a
hacky, but works
Adam-D-Lewis 7e6204a
add role to service account + cleanup
Adam-D-Lewis 2a3e49b
try to set service account auth state, but I don't think it's working
Adam-D-Lewis 110b0ee
fix bug and set auth state for service account
Adam-D-Lewis a0f4efe
cleanup
Adam-D-Lewis f180f07
cleanup
Adam-D-Lewis 6406e82
cleanup
Adam-D-Lewis 325a601
make service account name a variable
Adam-D-Lewis 64d3e0b
rename id to uuid for clarity
Adam-D-Lewis cb775e0
remove unneeded code
Adam-D-Lewis 59078cc
fix
Adam-D-Lewis f799f3e
cleanup
Adam-D-Lewis 21d0880
clarify docstring
Adam-D-Lewis 0be3851
clarify docstring
Adam-D-Lewis fedf7ae
Merge branch 'main' into jhub_apps_user
Adam-D-Lewis 2fb4fa8
fix buffer full deadlock
Adam-D-Lewis 8cb0e63
ensure binary raw string
Adam-D-Lewis 556661f
strip all ansi formatting sequences
Adam-D-Lewis 7e5c2b0
Revert "strip all ansi formatting sequences"
Adam-D-Lewis 37bd636
Revert "ensure binary raw string"
Adam-D-Lewis b6e75de
Revert "fix buffer full deadlock"
Adam-D-Lewis 1fce666
fix fstring
Adam-D-Lewis 865c8d6
add comment with jupyter/oauth code we are mimicking
Adam-D-Lewis fad0155
add keycloak service account name format comment
Adam-D-Lewis 8569ee8
merge with main
Adam-D-Lewis 80456c5
test that jupyterhub service account gets needed roles
Adam-D-Lewis 627c4aa
add a startup app to ci deployment
Adam-D-Lewis 6de7c1d
assert startup server is created
Adam-D-Lewis 48eae29
fix test_startup_apps_created test
Adam-D-Lewis fbaec09
remove breakpoint
Adam-D-Lewis 708f753
refactor keycloak command cli
Adam-D-Lewis e7da0aa
make test-user an admin
Adam-D-Lewis de43a81
fix test ids
Adam-D-Lewis 9810fdb
update tests since test-user is now an admin
Adam-D-Lewis File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
9 changes: 9 additions & 0 deletions
9
...ri/stages/kubernetes_services/template/modules/kubernetes/services/jupyterhub/versions.tf
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| terraform { | ||
| required_providers { | ||
| keycloak = { | ||
| source = "mrparkers/keycloak" | ||
| version = "3.7.0" | ||
| } | ||
| } | ||
| required_version = ">= 1.0" | ||
| } |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,29 +67,54 @@ data "keycloak_realm" "master" { | |
| realm = "nebari" | ||
| } | ||
|
|
||
| data "keycloak_openid_client" "realm_management" { | ||
| realm_id = var.realm_id | ||
| client_id = "realm-management" | ||
| } | ||
|
|
||
| data "keycloak_role" "main-service" { | ||
| for_each = toset(var.service-account-roles) | ||
| # Get client data for each service account client | ||
| data "keycloak_openid_client" "service_clients" { | ||
|
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. Before we only allowed service accounts to get roles from the realm-management client. This PR allows us to set roles by any client. This functionality was needed to be able to set the allow-app-sharing-role on the jupyterhub service account. |
||
| for_each = var.service-account-roles | ||
|
|
||
| realm_id = data.keycloak_realm.master.id | ||
| client_id = data.keycloak_openid_client.realm_management.id | ||
| name = each.key | ||
| } | ||
| realm_id = var.realm_id | ||
| client_id = each.key | ||
| depends_on = [keycloak_openid_client.main] | ||
| } | ||
|
|
||
| # Get role data for each client's roles | ||
| data "keycloak_role" "client_roles" { | ||
| for_each = { | ||
| for pair in flatten([ | ||
| for client, roles in var.service-account-roles : [ | ||
| for role in roles : { | ||
| key = "${client}-${role}" | ||
| client = client | ||
| role = role | ||
| } | ||
| ] | ||
| ]) : pair.key => pair | ||
| } | ||
|
|
||
| resource "keycloak_openid_client_service_account_role" "main" { | ||
| for_each = toset(var.service-account-roles) | ||
| realm_id = var.realm_id | ||
| client_id = data.keycloak_openid_client.service_clients[each.value.client].id | ||
| name = each.value.role | ||
| } | ||
|
|
||
| resource "keycloak_openid_client_service_account_role" "client_roles" { | ||
| for_each = { | ||
| for pair in flatten([ | ||
| for client, roles in var.service-account-roles : [ | ||
| for role in roles : { | ||
| key = "${client}-${role}" | ||
| client = client | ||
| role = role | ||
| } | ||
| ] | ||
| ]) : pair.key => pair | ||
| } | ||
|
|
||
| realm_id = var.realm_id | ||
| service_account_user_id = keycloak_openid_client.main.service_account_user_id | ||
| client_id = data.keycloak_openid_client.realm_management.id | ||
| role = data.keycloak_role.main-service[each.key].name | ||
| client_id = data.keycloak_openid_client.service_clients[each.value.client].id | ||
| role = data.keycloak_role.client_roles[each.key].name | ||
| } | ||
|
|
||
|
|
||
| resource "keycloak_role" "main" { | ||
| for_each = toset(flatten(values(var.role_mapping))) | ||
|
|
||
|
|
||
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
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.
We should add a note here and link to the JupyterHub code for posterity, incase something changes in JupyterHub, we can catch-up with that.
https://github.com/jupyterhub/oauthenticator/blob/d31bb193e84e7cda58b16f2f5d385c9b8affda4f/oauthenticator/oauth2.py#L1436
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.
done