Skip to content
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

Add behavioural tests for Cluster Menu K8s Resources in Sidebar menu not being shown #7280

Merged
merged 10 commits into from
Mar 22, 2023

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Copyright (c) OpenLens Authors. All rights reserved.
* Licensed under MIT License. See LICENSE in root directory for more information.
*/
import { AuthorizationV1Api } from "@kubernetes/client-node";
import type { KubeConfig } from "@kubernetes/client-node";
import { getInjectable } from "@ogre-tools/injectable";

export type CreateAuthorizationApi = (config: KubeConfig) => AuthorizationV1Api;

const createAuthorizationApiInjectable = getInjectable({
id: "create-authorization-api",
instantiate: (): CreateAuthorizationApi => (config) => config.makeApiClient(AuthorizationV1Api),
});

export default createAuthorizationApiInjectable;
42 changes: 42 additions & 0 deletions packages/core/src/common/cluster/create-can-i.injectable.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* Copyright (c) OpenLens Authors. All rights reserved.
* Licensed under MIT License. See LICENSE in root directory for more information.
*/

import type { AuthorizationV1Api, V1ResourceAttributes } from "@kubernetes/client-node";
import { getInjectable } from "@ogre-tools/injectable";
import loggerInjectable from "../logger.injectable";

/**
* Requests the permissions for actions on the kube cluster
* @param resourceAttributes The descriptor of the action that is desired to be known if it is allowed
* @returns `true` if the actions described are allowed
*/
export type CanI = (resourceAttributes: V1ResourceAttributes) => Promise<boolean>;

export type CreateCanI = (api: AuthorizationV1Api) => CanI;

const createCanIInjectable = getInjectable({
id: "create-can-i",
instantiate: (di): CreateCanI => {
const logger = di.inject(loggerInjectable);

return (api) => async (resourceAttributes: V1ResourceAttributes): Promise<boolean> => {
try {
const { body } = await api.createSelfSubjectAccessReview({
apiVersion: "authorization.k8s.io/v1",
kind: "SelfSubjectAccessReview",
spec: { resourceAttributes },
});

return body.status?.allowed ?? false;
} catch (error) {
logger.error(`[AUTHORIZATION-REVIEW]: failed to create access review: ${error}`, { resourceAttributes });

return false;
}
Comment on lines +33 to +37
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's an error do we know for sure the answer is false? Seems strange that the caller doesn't know there was an error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No we don't, but since these are currently only used for optimizations it is fine if they are turned off in case of error.

Plus this has always been the case.

};
},
});

export default createCanIInjectable;
16 changes: 16 additions & 0 deletions packages/core/src/common/cluster/create-core-api.injectable.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Copyright (c) OpenLens Authors. All rights reserved.
* Licensed under MIT License. See LICENSE in root directory for more information.
*/
import type { KubeConfig } from "@kubernetes/client-node";
import { CoreV1Api } from "@kubernetes/client-node";
import { getInjectable } from "@ogre-tools/injectable";

export type CreateCoreApi = (config: KubeConfig) => CoreV1Api;

const createCoreApiInjectable = getInjectable({
id: "create-core-api",
instantiate: (): CreateCoreApi => config => config.makeApiClient(CoreV1Api),
});

export default createCoreApiInjectable;
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/**
* Copyright (c) OpenLens Authors. All rights reserved.
* Licensed under MIT License. See LICENSE in root directory for more information.
*/

import type { AuthorizationV1Api } from "@kubernetes/client-node";
import { getInjectable } from "@ogre-tools/injectable";
import loggerInjectable from "../logger.injectable";
import type { KubeApiResource } from "../rbac";

export type CanListResource = (resource: KubeApiResource) => boolean;

/**
* Requests the permissions for actions on the kube cluster
* @param namespace The namespace of the resources
*/
export type RequestNamespaceListPermissions = (namespace: string) => Promise<CanListResource>;

export type CreateRequestNamespaceListPermissions = (api: AuthorizationV1Api) => RequestNamespaceListPermissions;

const createRequestNamespaceListPermissionsInjectable = getInjectable({
id: "create-request-namespace-list-permissions",
instantiate: (di): CreateRequestNamespaceListPermissions => {
const logger = di.inject(loggerInjectable);

return (api) => async (namespace) => {
try {
const { body: { status }} = await api.createSelfSubjectRulesReview({
apiVersion: "authorization.k8s.io/v1",
kind: "SelfSubjectRulesReview",
spec: { namespace },
});

if (!status || status.incomplete) {
logger.warn(`[AUTHORIZATION-NAMESPACE-REVIEW]: allowing all resources in namespace="${namespace}" due to incomplete SelfSubjectRulesReview: ${status?.evaluationError}`);

return () => true;
Comment on lines +35 to +37
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The caller has no idea here, or in the catch below, that api.createSelfSubjectRulesReview() failed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is safer to show all the resources of this fails then to show none.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'm really questioning whether that decision should be made here or by the caller, which might be able to show a helpful notification to the end-user. Now there is no indication to the user why there are suddenly all these resources listed in the sidebar that perhaps can't be listed.

}

const { resourceRules } = status;

return (resource) => (
resourceRules
.filter(({ apiGroups = ["*"] }) => apiGroups.includes("*") || apiGroups.includes(resource.group))
.filter(({ resources = ["*"] }) => resources.includes("*") || resources.includes(resource.apiName))
.some(({ verbs }) => verbs.includes("*") || verbs.includes("list"))
);
} catch (error) {
logger.error(`[AUTHORIZATION-NAMESPACE-REVIEW]: failed to create subject rules review`, { namespace, error });

return () => true;
}
};
},
});

export default createRequestNamespaceListPermissionsInjectable;
20 changes: 7 additions & 13 deletions packages/core/src/common/cluster/list-namespaces.injectable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,21 @@
* Copyright (c) OpenLens Authors. All rights reserved.
* Licensed under MIT License. See LICENSE in root directory for more information.
*/
import type { KubeConfig } from "@kubernetes/client-node";
import { CoreV1Api } from "@kubernetes/client-node";
import type { CoreV1Api } from "@kubernetes/client-node";
import { getInjectable } from "@ogre-tools/injectable";
import { isDefined } from "@k8slens/utilities";

export type ListNamespaces = () => Promise<string[]>;

export type CreateListNamespaces = (config: KubeConfig) => ListNamespaces;
export type CreateListNamespaces = (api: CoreV1Api) => ListNamespaces;

const createListNamespacesInjectable = getInjectable({
id: "create-list-namespaces",
instantiate: (): CreateListNamespaces => (config) => {
const coreApi = config.makeApiClient(CoreV1Api);

return async () => {
const { body: { items }} = await coreApi.listNamespace();
instantiate: (): CreateListNamespaces => (api) => async () => {
const { body: { items }} = await api.listNamespace();

return items
.map(ns => ns.metadata?.name)
.filter(isDefined);
};
return items
.map(ns => ns.metadata?.name)
.filter(isDefined);
},
});

Expand Down

This file was deleted.

Loading