From c47a71aae6075678f562e02f1344262e91a44fab Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Tue, 23 Feb 2021 15:46:46 +0100 Subject: [PATCH 1/6] fix delete user resource bug --- internal/resources/user.go | 4 +++- web-client/src/components/edit-user.tsx | 1 + web-client/src/constants.tsx | 7 +++++-- web-client/src/services/role.tsx | 14 +++++++++++--- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/internal/resources/user.go b/internal/resources/user.go index 7b66612e..0a06da7c 100644 --- a/internal/resources/user.go +++ b/internal/resources/user.go @@ -101,7 +101,9 @@ func (r *resourcesService) CreateUser(ctx context.Context, username string) User // the PermissionManagerUser CRD object associated to the user with the given username. func (r *resourcesService) DeleteUser(ctx context.Context, username string) { metadataName := "permissionmanager.user." + username - _, err := r.kubeclient.AppsV1().RESTClient().Delete().AbsPath(resourceURL + "/" + metadataName).DoRaw(ctx) + + _, err := r.kubeclient.AppsV1().RESTClient().Delete().AbsPath(resourceURL + "/" + metadataName + "/").DoRaw(ctx) + if err != nil { log.Printf("Failed to delete user:%s\n %v\n", username, err) } diff --git a/web-client/src/components/edit-user.tsx b/web-client/src/components/edit-user.tsx index f4ce3eca..1f2ccc34 100644 --- a/web-client/src/components/edit-user.tsx +++ b/web-client/src/components/edit-user.tsx @@ -99,6 +99,7 @@ export default function EditUser({user}: EditUserParameters) { * delete all the user-resources currently in the k8s cluster */ async function deleteUserResources() { + await httpRequests.rolebindingRequests.delete.rolebinding(rbs); await httpRequests.rolebindingRequests.delete.clusterRolebinding(crbs); } diff --git a/web-client/src/constants.tsx b/web-client/src/constants.tsx index 76b95dc0..527f9abe 100644 --- a/web-client/src/constants.tsx +++ b/web-client/src/constants.tsx @@ -63,6 +63,9 @@ export const VERBS = [ 'delete', ] -export const templateNamespacedResourceRolePrefix = 'template-namespaced-resources___' +export const resourceSeparator = '___' + +export const templateNamespacedResourceRolePrefix = 'template-namespaced-resources' + resourceSeparator + +export const templateClusterResourceRolePrefix = 'template-cluster-resources' + resourceSeparator -export const templateClusterResourceRolePrefix = 'template-cluster-resources___' diff --git a/web-client/src/services/role.tsx b/web-client/src/services/role.tsx index 482ef7a4..c5d7e382 100644 --- a/web-client/src/services/role.tsx +++ b/web-client/src/services/role.tsx @@ -1,4 +1,4 @@ -import {templateClusterResourceRolePrefix} from "../constants"; +import {resourceSeparator, templateClusterResourceRolePrefix} from "../constants"; import uuid from "uuid"; import {ClusterRoleBinding, RoleBinding} from "../hooks/useRbac"; @@ -49,11 +49,19 @@ export interface ExtractedUserRoles { */ export function extractUsersRoles(roleBindings: RoleBinding[], clusterRoleBindings: ClusterRoleBinding[], username: string): ExtractedUserRoles { const rbs = (roleBindings || []).filter(rb => { - return rb.metadata.name.startsWith(username) + const name = rb.metadata.name.split(resourceSeparator).find(e => e); + + if (!name) return false + + return name === username }) const crbs = (clusterRoleBindings || []).filter(crb => { - return crb.metadata.name.startsWith(username) + const name = crb.metadata.name.split(resourceSeparator).find(e => e); + + if (!name) return false + + return name[0] === username }) const normalizedRoleBindings: NormalizedRoleBinding[] = [...rbs, ...crbs] From 5324626ef5a02fc3830533052b22e2bf4081c4f4 Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Tue, 23 Feb 2021 15:54:02 +0100 Subject: [PATCH 2/6] improved rbs and crbs filtering --- web-client/src/services/role.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/web-client/src/services/role.tsx b/web-client/src/services/role.tsx index c5d7e382..f64a438f 100644 --- a/web-client/src/services/role.tsx +++ b/web-client/src/services/role.tsx @@ -49,17 +49,17 @@ export interface ExtractedUserRoles { */ export function extractUsersRoles(roleBindings: RoleBinding[], clusterRoleBindings: ClusterRoleBinding[], username: string): ExtractedUserRoles { const rbs = (roleBindings || []).filter(rb => { - const name = rb.metadata.name.split(resourceSeparator).find(e => e); + const name = rb.metadata.name.split(resourceSeparator); - if (!name) return false + if (name.length === 0) return false - return name === username + return name[0] === username }) const crbs = (clusterRoleBindings || []).filter(crb => { - const name = crb.metadata.name.split(resourceSeparator).find(e => e); + const name = crb.metadata.name.split(resourceSeparator); - if (!name) return false + if (name.length === 0) return false return name[0] === username }) From cb38bf22803e9493bcff466f4ea68816ea2c3159 Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Tue, 23 Feb 2021 15:55:01 +0100 Subject: [PATCH 3/6] adds documentation to cb and crb splits --- web-client/src/services/role.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/web-client/src/services/role.tsx b/web-client/src/services/role.tsx index f64a438f..4a279e30 100644 --- a/web-client/src/services/role.tsx +++ b/web-client/src/services/role.tsx @@ -53,6 +53,7 @@ export function extractUsersRoles(roleBindings: RoleBinding[], clusterRoleBindin if (name.length === 0) return false + // the first split always contains the name of the user return name[0] === username }) @@ -61,6 +62,7 @@ export function extractUsersRoles(roleBindings: RoleBinding[], clusterRoleBindin if (name.length === 0) return false + // the first split always contains the name of the user return name[0] === username }) From 3e8c0d26daacdcc3dcec6c35e9fe31ebbf148335 Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Tue, 23 Feb 2021 15:57:24 +0100 Subject: [PATCH 4/6] removes trailing slash to user delete --- internal/resources/user.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/resources/user.go b/internal/resources/user.go index 0a06da7c..706c7330 100644 --- a/internal/resources/user.go +++ b/internal/resources/user.go @@ -102,7 +102,7 @@ func (r *resourcesService) CreateUser(ctx context.Context, username string) User func (r *resourcesService) DeleteUser(ctx context.Context, username string) { metadataName := "permissionmanager.user." + username - _, err := r.kubeclient.AppsV1().RESTClient().Delete().AbsPath(resourceURL + "/" + metadataName + "/").DoRaw(ctx) + _, err := r.kubeclient.AppsV1().RESTClient().Delete().AbsPath(resourceURL + "/" + metadataName).DoRaw(ctx) if err != nil { log.Printf("Failed to delete user:%s\n %v\n", username, err) From b05284e6c0b094083c5693b58969d11ef2dc05e4 Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Tue, 23 Feb 2021 16:03:24 +0100 Subject: [PATCH 5/6] better variable naming in crb and rb --- web-client/src/services/role.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/web-client/src/services/role.tsx b/web-client/src/services/role.tsx index 4a279e30..612e0438 100644 --- a/web-client/src/services/role.tsx +++ b/web-client/src/services/role.tsx @@ -49,21 +49,21 @@ export interface ExtractedUserRoles { */ export function extractUsersRoles(roleBindings: RoleBinding[], clusterRoleBindings: ClusterRoleBinding[], username: string): ExtractedUserRoles { const rbs = (roleBindings || []).filter(rb => { - const name = rb.metadata.name.split(resourceSeparator); + const separatedResourceName = rb.metadata.name.split(resourceSeparator); - if (name.length === 0) return false + if (separatedResourceName.length === 0) return false // the first split always contains the name of the user - return name[0] === username + return separatedResourceName[0] === username }) const crbs = (clusterRoleBindings || []).filter(crb => { - const name = crb.metadata.name.split(resourceSeparator); + const separatedResourceName = crb.metadata.name.split(resourceSeparator); - if (name.length === 0) return false + if (separatedResourceName.length === 0) return false // the first split always contains the name of the user - return name[0] === username + return separatedResourceName[0] === username }) const normalizedRoleBindings: NormalizedRoleBinding[] = [...rbs, ...crbs] From 6d6a153e2744e3c1ad47a42bc5d6f98309cb4f3a Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Tue, 23 Feb 2021 16:05:22 +0100 Subject: [PATCH 6/6] refactor resource separator dynamic strings --- .../src/services/rolebindingCreateRequests.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/web-client/src/services/rolebindingCreateRequests.tsx b/web-client/src/services/rolebindingCreateRequests.tsx index c2f657eb..5587b088 100644 --- a/web-client/src/services/rolebindingCreateRequests.tsx +++ b/web-client/src/services/rolebindingCreateRequests.tsx @@ -1,4 +1,4 @@ -import {templateClusterResourceRolePrefix} from "../constants"; +import {resourceSeparator, templateClusterResourceRolePrefix} from "../constants"; import {ClusterAccess} from "../components/types"; import {AxiosInstance, AxiosResponse} from "axios"; import {AggregatedRoleBinding} from "./role"; @@ -144,7 +144,7 @@ export class RolebindingCreateRequests { // we grab all the 'ALL_NAMESPACE' rolebindings and create them on the backend for (const allNamespaceRolebinding of aggregatedRoleBindings.filter(e => e.namespaces === 'ALL_NAMESPACES')) { // we construct the resource name - const clusterRolebindingName = username + '___' + allNamespaceRolebinding.roleName + 'all_namespaces' + const clusterRolebindingName = username + resourceSeparator + allNamespaceRolebinding.roleName + 'all_namespaces' // means that we already created the resource if (consumed.includes(clusterRolebindingName)) continue; @@ -165,8 +165,8 @@ export class RolebindingCreateRequests { for (const namespace of namespacedRoleBinding.namespaces) { // we construct the resource name - const rolebindingName = username + '___' + namespacedRoleBinding.roleName + '___' + namespace - + const rolebindingName = username + resourceSeparator + namespacedRoleBinding.roleName + resourceSeparator + namespace + // means that we already created the resource if (consumed.includes(rolebindingName)) continue; @@ -191,9 +191,9 @@ export class RolebindingCreateRequests { return; } - const roleName = username + '___' + this.getRoleName(clusterAccess); + const roleName = username + resourceSeparator + this.getRoleName(clusterAccess); - const clusterRolebindingName = username + '___' + roleName + const clusterRolebindingName = username + resourceSeparator + roleName //todo this must be changed in the future to support dynamic cluster roles. Right now it's just a single api call based on a radio select await this.clusterRolebinding({