Skip to content

Commit

Permalink
Disable manage permissions when no rolebinding present for the defaul…
Browse files Browse the repository at this point in the history
…t group (#3306)
  • Loading branch information
ppadti authored Oct 11, 2024
1 parent bd03c0b commit 6ab6afd
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 31 deletions.
36 changes: 19 additions & 17 deletions frontend/src/__mocks__/mockModelRegistry.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,30 @@
import { ModelRegistryKind } from '~/k8sTypes';
import { K8sCondition, ModelRegistryKind } from '~/k8sTypes';

type MockModelRegistryType = {
name?: string;
namespace?: string;
conditions?: K8sCondition[];
};

export const mockModelRegistry = ({
name = 'modelregistry-sample',
namespace = 'odh-model-registries',
conditions = [
{
lastTransitionTime: '2024-03-22T09:30:02Z',
message: 'Deployment for custom resource modelregistry-sample was successfully created',
reason: 'CreatedDeployment',
status: 'True',
type: 'Progressing',
},
{
lastTransitionTime: '2024-03-14T08:11:26Z',
message: 'Deployment for custom resource modelregistry-sample is available',
reason: 'DeploymentAvailable',
status: 'True',
type: 'Available',
},
],
}: MockModelRegistryType): ModelRegistryKind => ({
apiVersion: 'modelregistry.opendatahub.io/v1alpha1',
kind: 'ModelRegistry',
Expand Down Expand Up @@ -39,21 +56,6 @@ export const mockModelRegistry = ({
},
},
status: {
conditions: [
{
lastTransitionTime: '2024-03-22T09:30:02Z',
message: 'Deployment for custom resource modelregistry-sample was successfully created',
reason: 'CreatedDeployment',
status: 'True',
type: 'Progressing',
},
{
lastTransitionTime: '2024-03-14T08:11:26Z',
message: 'Deployment for custom resource modelregistry-sample is available',
reason: 'DeploymentAvailable',
status: 'True',
type: 'Available',
},
],
conditions,
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ class ModelRegistrySettings {
return cy.findByTestId('modal-submit-button');
}

findManagePermissionsTooltip() {
return cy.findByRole('tooltip');
}

findTable() {
return cy.findByTestId('model-registries-table');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,17 @@ import {
asProjectAdminUser,
} from '~/__tests__/cypress/cypress/utils/mockUsers';
import { mockModelRegistry } from '~/__mocks__/mockModelRegistry';
import type { RoleBindingSubject } from '~/k8sTypes';
import { mockRoleBindingK8sResource } from '~/__mocks__/mockRoleBindingK8sResource';
import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url';

const groupSubjects: RoleBindingSubject[] = [
{
kind: 'Group',
apiGroup: 'rbac.authorization.k8s.io',
name: 'example-mr-users',
},
];

const setupMocksForMRSettingAccess = ({
hasModelRegistries = true,
Expand Down Expand Up @@ -47,7 +58,19 @@ const setupMocksForMRSettingAccess = ({
? [
mockModelRegistry({ name: 'test-registry-1' }),
mockModelRegistry({ name: 'test-registry-2' }),
mockModelRegistry({ name: 'test-registry-3' }),
mockModelRegistry({
name: 'test-registry-3',
conditions: [
{
lastTransitionTime: '2024-03-22T09:30:02Z',
message:
'Deployment for custom resource modelregistry-sample was successfully created',
reason: 'CreatedDeployment',
status: 'True',
type: 'Progressing',
},
],
}),
]
: [],
),
Expand All @@ -71,6 +94,26 @@ const setupMocksForMRSettingAccess = ({
req.reply(500); // Something went wrong on the backend when decoding the secret
},
);

cy.interceptOdh(
'GET /api/modelRegistryRoleBindings',
mockK8sResourceList([
mockRoleBindingK8sResource({
namespace: 'odh-model-registries',
name: 'test-registry-1-user',
subjects: groupSubjects,
roleRefName: 'registry-user-test-registry-1',
modelRegistryName: 'test-registry-1',
}),
mockRoleBindingK8sResource({
namespace: 'odh-model-registries',
name: 'test-registry-2-user',
subjects: groupSubjects,
roleRefName: 'registry-user-test-registry-2',
modelRegistryName: 'test-registry-2',
}),
]),
);
};

it('Model registry settings should not be available for non product admins', () => {
Expand Down Expand Up @@ -170,14 +213,25 @@ describe('ViewDatabaseConfigModal', () => {
});
});

describe('ManagePermissionsModal', () => {
beforeEach(() => {
describe('ManagePermissions', () => {
it('Manage permission is enabled, when there is a rolebinding', () => {
setupMocksForMRSettingAccess({});
modelRegistrySettings.visit(true);
modelRegistrySettings
.findModelRegistryRow('test-registry-1')
.findByText('Manage permissions')
.click();
verifyRelativeURL('/modelRegistrySettings/permissions/test-registry-1');
});

it('Manage permission is disabled, when there is no rolebinding', () => {
setupMocksForMRSettingAccess({});
modelRegistrySettings.visit(true);
modelRegistrySettings
.findModelRegistryRow('test-registry-3')
.findByText('Manage permissions')
.trigger('mouseenter');
modelRegistrySettings.findManagePermissionsTooltip().should('be.visible');
});
});

Expand Down
12 changes: 10 additions & 2 deletions frontend/src/pages/modelRegistrySettings/ModelRegistriesTable.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
import React from 'react';
import { Button, Toolbar, ToolbarContent, ToolbarItem } from '@patternfly/react-core';
import { Table } from '~/components/table';
import { ModelRegistryKind } from '~/k8sTypes';
import { ModelRegistryKind, RoleBindingKind } from '~/k8sTypes';
import { ContextResourceData } from '~/types';
import { modelRegistryColumns } from './columns';
import ModelRegistriesTableRow from './ModelRegistriesTableRow';

type ModelRegistriesTableProps = {
modelRegistries: ModelRegistryKind[];
refresh: () => Promise<unknown>;
roleBindings: ContextResourceData<RoleBindingKind>;
onCreateModelRegistryClick: () => void;
};

const ModelRegistriesTable: React.FC<ModelRegistriesTableProps> = ({
modelRegistries,
roleBindings,
refresh,
onCreateModelRegistryClick,
}) => (
Expand All @@ -36,7 +39,12 @@ const ModelRegistriesTable: React.FC<ModelRegistriesTableProps> = ({
</Toolbar>
}
rowRenderer={(mr) => (
<ModelRegistriesTableRow key={mr.metadata.name} modelRegistry={mr} refresh={refresh} />
<ModelRegistriesTableRow
key={mr.metadata.name}
modelRegistry={mr}
roleBindings={roleBindings}
refresh={refresh}
/>
)}
variant="compact"
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,34 @@
import React from 'react';
import { ActionsColumn, Td, Tr } from '@patternfly/react-table';
import { Link } from 'react-router-dom';
import { ModelRegistryKind } from '~/k8sTypes';
import { useNavigate } from 'react-router-dom';
import { Button, Tooltip } from '@patternfly/react-core';
import { ModelRegistryKind, RoleBindingKind } from '~/k8sTypes';
import ResourceNameTooltip from '~/components/ResourceNameTooltip';
import { ContextResourceData } from '~/types';
import ViewDatabaseConfigModal from './ViewDatabaseConfigModal';
import DeleteModelRegistryModal from './DeleteModelRegistryModal';
import { ModelRegistryTableRowStatus } from './ModelRegistryTableRowStatus';

type ModelRegistriesTableRowProps = {
modelRegistry: ModelRegistryKind;
roleBindings: ContextResourceData<RoleBindingKind>;
refresh: () => Promise<unknown>;
};

const ModelRegistriesTableRow: React.FC<ModelRegistriesTableRowProps> = ({
modelRegistry: mr,
roleBindings,
refresh,
}) => {
const [isDatabaseConfigModalOpen, setIsDatabaseConfigModalOpen] = React.useState(false);
const [isDeleteModalOpen, setIsDeleteModalOpen] = React.useState(false);
const navigate = useNavigate();
const filteredRoleBindings = roleBindings.data.filter(
(rb) =>
rb.metadata.labels?.['app.kubernetes.io/name'] ===
(mr.metadata.name || mr.metadata.annotations?.['openshift.io/display-name']),
);

return (
<>
<Tr>
Expand All @@ -35,12 +46,20 @@ const ModelRegistriesTableRow: React.FC<ModelRegistriesTableRowProps> = ({
<ModelRegistryTableRowStatus conditions={mr.status?.conditions} />
</Td>
<Td modifier="fitContent">
<Link
aria-label={`Manage permissions for model registry ${mr.metadata.name}`}
to={`/modelRegistrySettings/permissions/${mr.metadata.name}`}
>
Manage permissions
</Link>
{filteredRoleBindings.length === 0 ? (
<Tooltip content="You can manage permissions when the model registry becomes available.">
<Button isAriaDisabled variant="link">
Manage permissions
</Button>
</Tooltip>
) : (
<Button
variant="link"
onClick={() => navigate(`/modelRegistrySettings/permissions/${mr.metadata.name}`)}
>
Manage permissions
</Button>
)}
</Td>
<Td isActionCell>
<ActionsColumn
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,19 @@ import useModelRegistriesBackend from '~/concepts/modelRegistrySettings/useModel
import TitleWithIcon from '~/concepts/design/TitleWithIcon';
import { ProjectObjectType } from '~/concepts/design/utils';
import { ModelRegistrySelectorContext } from '~/concepts/modelRegistry/context/ModelRegistrySelectorContext';
import { useContextResourceData } from '~/utilities/useContextResourceData';
import { RoleBindingKind } from '~/k8sTypes';
import ModelRegistriesTable from './ModelRegistriesTable';
import CreateModal from './CreateModal';
import useModelRegistryRoleBindings from './useModelRegistryRoleBindings';

const ModelRegistrySettings: React.FC = () => {
const [createModalOpen, setCreateModalOpen] = React.useState(false);
const [modelRegistries, loaded, loadError, refreshModelRegistries] = useModelRegistriesBackend();
const [modelRegistries, mrloaded, loadError, refreshModelRegistries] =
useModelRegistriesBackend();
const roleBindings = useContextResourceData<RoleBindingKind>(useModelRegistryRoleBindings());
const { refreshRulesReview } = React.useContext(ModelRegistrySelectorContext);
const loaded = mrloaded && roleBindings.loaded;

const refreshAll = React.useCallback(
() => Promise.all([refreshModelRegistries(), refreshRulesReview()]),
Expand Down Expand Up @@ -65,6 +71,7 @@ const ModelRegistrySettings: React.FC = () => {
>
<ModelRegistriesTable
modelRegistries={modelRegistries}
roleBindings={roleBindings}
refresh={refreshAll}
onCreateModelRegistryClick={() => {
setCreateModalOpen(true);
Expand Down

0 comments on commit 6ab6afd

Please sign in to comment.