workspaces: apply roles to translated service account name #4180
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
|
Diff between rendered kustomize manifests before and after this change for Details
--- /tmp/before.yaml 2024-07-24 15:46:29.606129317 -0500
+++ /tmp/after.yaml 2024-07-24 15:46:22.390212002 -0500
@@ -383,6 +383,22 @@ rules:
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
+ name: workspaces-workspace-server-editor
+ namespace: toolchain-host-operator
+rules:
+- apiGroups:
+ - workspaces.konflux.io
+ resources:
+ - internalworkspaces
+ verbs:
+ - list
+ - get
+ - watch
+ - update
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
labels:
app.kubernetes.io/component: rbac
app.kubernetes.io/created-by: workspaces
@@ -426,22 +442,6 @@ rules:
- patch
---
apiVersion: rbac.authorization.k8s.io/v1
-kind: Role
-metadata:
- name: workspaces-workspace-server-editor
- namespace: workspaces-system
-rules:
-- apiGroups:
- - workspaces.konflux.io
- resources:
- - internalworkspaces
- verbs:
- - list
- - get
- - watch
- - update
----
-apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: workspaces-manager-role
@@ -541,7 +541,7 @@ roleRef:
name: workspaces-spacebinding-reader
subjects:
- kind: ServiceAccount
- name: rest-api-server
+ name: workspaces-rest-api-server
namespace: workspaces-system
---
apiVersion: rbac.authorization.k8s.io/v1
@@ -555,42 +555,42 @@ roleRef:
name: workspaces-usersignup-reader
subjects:
- kind: ServiceAccount
- name: rest-api-server
+ name: workspaces-rest-api-server
namespace: workspaces-system
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
- labels:
- app.kubernetes.io/component: rbac
- app.kubernetes.io/created-by: workspaces
- app.kubernetes.io/instance: leader-election-rolebinding
- app.kubernetes.io/managed-by: kustomize
- app.kubernetes.io/name: rolebinding
- app.kubernetes.io/part-of: workspaces
- name: workspaces-leader-election-rolebinding
- namespace: workspaces-system
+ name: workspaces-rest-api-server:workspace-server-editor
+ namespace: toolchain-host-operator
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
- name: workspaces-leader-election-role
+ name: workspaces-workspace-server-editor
subjects:
- kind: ServiceAccount
- name: workspaces-controller-manager
+ name: workspaces-rest-api-server
namespace: workspaces-system
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
- name: workspaces-rest-api-server:workspace-server-editor
+ labels:
+ app.kubernetes.io/component: rbac
+ app.kubernetes.io/created-by: workspaces
+ app.kubernetes.io/instance: leader-election-rolebinding
+ app.kubernetes.io/managed-by: kustomize
+ app.kubernetes.io/name: rolebinding
+ app.kubernetes.io/part-of: workspaces
+ name: workspaces-leader-election-rolebinding
namespace: workspaces-system
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
- name: workspaces-workspace-server-editor
+ name: workspaces-leader-election-role
subjects:
- kind: ServiceAccount
- name: workspaces-rest-api-server
+ name: workspaces-controller-manager
namespace: workspaces-system
---
apiVersion: rbac.authorization.k8s.io/v1
|
filariow
reviewed
Jul 24, 2024
components/workspaces/base/server/config/default/kustomization.yaml
Outdated
Show resolved
Hide resolved
The name of the RoleBinding here is not respecting the format we are using ( apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
- name: workspaces-rest-api-server:workspace-server-editor
+ labels:
+ app.kubernetes.io/component: rbac
+ app.kubernetes.io/created-by: workspaces
+ app.kubernetes.io/instance: leader-election-rolebinding
+ app.kubernetes.io/managed-by: kustomize
+ app.kubernetes.io/name: rolebinding
+ app.kubernetes.io/part-of: workspaces
+ name: workspaces-leader-election-rolebinding |
For a future PR: the format we have here for RoleBindings is I'm expecting to have something like the following in future: - name: workspaces-rest-api-server:workspace-server-editor
+ name: workspaces-rest-api-server:workspaces-workspace-server-editor |
|
AIUI it should be possible to do this using replacements. I can look into doing this tomorrow. |
|
/hold |
|
New diff in manifests: Details
diff --git a/tmp/before.yaml b/tmp/after.yaml
index 59c19ad7..9d833cc8 100644
--- a/tmp/before.yaml
+++ b/tmp/after.yaml
@@ -541,7 +541,7 @@ roleRef:
name: workspaces-spacebinding-reader
subjects:
- kind: ServiceAccount
- name: rest-api-server
+ name: workspaces-rest-api-server
namespace: workspaces-system
---
apiVersion: rbac.authorization.k8s.io/v1
@@ -555,7 +555,7 @@ roleRef:
name: workspaces-usersignup-reader
subjects:
- kind: ServiceAccount
- name: rest-api-server
+ name: workspaces-rest-api-server
namespace: workspaces-system
---
apiVersion: rbac.authorization.k8s.io/v1
</p>
</details> |
|
/unhold |
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: filariow, sadlerap The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
filariow
reviewed
Jul 25, 2024
components/workspaces/base/server/config/default/kustomization.yaml
Outdated
Show resolved
Hide resolved
|
/retest Not sure what e2e tests are failing, looks to be a flake? I don't think this change would impact them. |
|
/lgtm |
Currently, the role bindings we have for server apply to the service account `rest-api-server`, not `workspaces-rest-api-server`. We need to overwrite the name in the bindings to reflect the processing the names of resources go through as a part of the kustomization process. Signed-off-by: Andy Sadler <[email protected]>
sadlerap
force-pushed
the
service-account-rbac
branch
from
87e1ba1 to
0292e65
Compare
|
/retest |
|
/retest |
|
/retest |
|
/test appstudio-e2e-tests |
|
Filed KFLUXBUGS-1487 for recent failure. /test appstudio-e2e-tests |
openshift-merge-bot
bot
merged commit 3eeb1ba
into
redhat-appstudio:main
6 of 7 checks passed
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.
Currently, the role bindings we have for server apply to the service account
rest-api-server, notworkspaces-rest-api-server. We need to overwrite the name in the bindings to reflect the processing the names of resources go through as a part of the kustomization process.