diff --git a/backend/src/routes/api/modelRegistries/index.ts b/backend/src/routes/api/modelRegistries/index.ts index 50afeb71fc..1e2bccc961 100644 --- a/backend/src/routes/api/modelRegistries/index.ts +++ b/backend/src/routes/api/modelRegistries/index.ts @@ -162,7 +162,7 @@ export default async (fastify: KubeFastifyInstance): Promise => { const { modelRegistryName } = request.params; try { const modelRegistryNamespace = getModelRegistryNamespace(fastify); - deleteModelRegistryAndSecret( + return deleteModelRegistryAndSecret( fastify, modelRegistryName, modelRegistryNamespace, diff --git a/backend/src/routes/api/modelRegistryRoleBindings/modelRegistryRolebindingsUtils.ts b/backend/src/routes/api/modelRegistryRoleBindings/modelRegistryRolebindingsUtils.ts index 77a3808868..46dff66eb3 100644 --- a/backend/src/routes/api/modelRegistryRoleBindings/modelRegistryRolebindingsUtils.ts +++ b/backend/src/routes/api/modelRegistryRoleBindings/modelRegistryRolebindingsUtils.ts @@ -27,12 +27,19 @@ export const createModelRegistryRoleBinding = async ( rbRequest: V1RoleBinding, mrNamespace: string, ): Promise => { + // Re-inject the namespace value that was omitted by the client + // (see createModelRegistryRoleBinding in frontend/src/services/modelRegistrySettingsService.ts) + // This will be unnecessary when we remove the backend service as part of https://issues.redhat.com/browse/RHOAIENG-12077 + const roleBindingWithNamespace = { + ...rbRequest, + metadata: { ...rbRequest.metadata, namespace: mrNamespace }, + }; const response = await (fastify.kube.customObjectsApi.createNamespacedCustomObject( MODEL_REGISTRY_ROLE_BINDING_API_GROUP, MODEL_REGISTRY_ROLE_BINDING_API_VERSION, mrNamespace, MODEL_REGISTRY_ROLE_BINDING_PLURAL, - rbRequest, + roleBindingWithNamespace, ) as Promise<{ body: V1RoleBinding }>); return response.body; }; diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistrySettings/modelRegistryPermissions.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistrySettings/modelRegistryPermissions.cy.ts index 7214d73710..3f4d3d7d0d 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistrySettings/modelRegistryPermissions.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistrySettings/modelRegistryPermissions.cy.ts @@ -430,7 +430,6 @@ describe('MR Permissions', () => { cy.interceptOdh( 'POST /api/modelRegistryRoleBindings', mockRoleBindingK8sResource({ - namespace: MODEL_REGISTRY_DEFAULT_NAMESPACE, subjects: projectSubjects, roleRefName: 'registry-user-example-mr', modelRegistryName: 'example-mr', @@ -443,9 +442,6 @@ describe('MR Permissions', () => { cy.wait('@addProject').then((interception) => { expect(interception.request.body).to.containSubset({ - metadata: { - namespace: 'odh-model-registries', - }, roleRef: { apiGroup: 'rbac.authorization.k8s.io', kind: 'Role', @@ -484,9 +480,6 @@ describe('MR Permissions', () => { cy.wait('@editProject').then((interception) => { expect(interception.request.body).to.containSubset({ - metadata: { - namespace: 'odh-model-registries', - }, roleRef: { apiGroup: 'rbac.authorization.k8s.io', kind: 'Role', diff --git a/frontend/src/api/modelRegistry/errorUtils.ts b/frontend/src/api/modelRegistry/errorUtils.ts index 1282802199..bc5c723e06 100644 --- a/frontend/src/api/modelRegistry/errorUtils.ts +++ b/frontend/src/api/modelRegistry/errorUtils.ts @@ -18,7 +18,6 @@ export const handleModelRegistryFailures = (promise: Promise): Promise } if (isCommonStateError(e)) { // Common state errors are handled by useFetchState at storage level, let them deal with it - // TODO: check whether we need this or not throw e; } // eslint-disable-next-line no-console diff --git a/frontend/src/services/modelRegistrySettingsService.ts b/frontend/src/services/modelRegistrySettingsService.ts index 465ee7aa84..4d5218099a 100644 --- a/frontend/src/services/modelRegistrySettingsService.ts +++ b/frontend/src/services/modelRegistrySettingsService.ts @@ -1,3 +1,4 @@ +import * as _ from 'lodash-es'; import { K8sStatus } from '@openshift/dynamic-plugin-sdk-utils'; import axios from '~/utilities/axios'; import { ModelRegistryKind, RoleBindingKind } from '~/k8sTypes'; @@ -66,13 +67,21 @@ export const listModelRegistryRoleBindings = (): Promise => throw new Error(e.response.data.message); }); -export const createModelRegistryRoleBinding = (data: RoleBindingKind): Promise => - axios - .post(mrRoleBindingsUrl, data) +export const createModelRegistryRoleBinding = (data: RoleBindingKind): Promise => { + // Don't include the namespace in the object we pass because it would get rejected by requestSecurityGuard. + // (see backend/src/utils/route-security.ts) + // Instead the namespace will be reinjected by the backend + // (see backend/src/routes/api/modelRegistries/modelRegistryUtils.ts) + // This will be unnecessary when we remove this service as part of https://issues.redhat.com/browse/RHOAIENG-12077 + const roleBindingWithoutNamespace: RoleBindingKind & { metadata: { namespace?: string } } = + _.omit(data, 'metadata.namespace'); + return axios + .post(mrRoleBindingsUrl, roleBindingWithoutNamespace) .then((response) => response.data) .catch((e) => { throw new Error(e.response.data.message); }); +}; export const deleteModelRegistryRoleBinding = (roleBindingName: string): Promise => axios