From d1699e76d7e4dbae58fb25c636fac70f535f8686 Mon Sep 17 00:00:00 2001 From: Jeff MAURY Date: Thu, 4 May 2023 16:48:05 +0200 Subject: [PATCH] fix: Refactor Kubernetes client with respect to OpenShift and Microshift (#2317) * fix: Refactor Kubernetes client with respect to OpenShift and Microshift * Code smells from @benoitf review * Add support for Restricted security profile (MicroShift) Fixes #2260 Signed-off-by: Jeff MAURY --- packages/main/src/plugin/index.ts | 4 + packages/main/src/plugin/kubernetes-client.ts | 32 ++++++-- packages/preload/src/index.ts | 4 + .../src/lib/pod/DeployPodToKube.spec.ts | 47 ++++++++++- .../src/lib/pod/DeployPodToKube.svelte | 33 +++++++- .../renderer/src/lib/pod/pod-utils.spec.ts | 81 +++++++++++++++++++ packages/renderer/src/lib/pod/pod-utils.ts | 34 ++++++++ 7 files changed, 222 insertions(+), 13 deletions(-) create mode 100644 packages/renderer/src/lib/pod/pod-utils.spec.ts diff --git a/packages/main/src/plugin/index.ts b/packages/main/src/plugin/index.ts index 626f614eb59..42f1d2898ce 100644 --- a/packages/main/src/plugin/index.ts +++ b/packages/main/src/plugin/index.ts @@ -1407,6 +1407,10 @@ export class PluginSystem { }, ); + this.ipcHandle('kubernetes-client:isAPIGroupSupported', async (_listener, group): Promise => { + return kubernetesClient.isAPIGroupSupported(group); + }); + this.ipcHandle('kubernetes-client:getCurrentContextName', async (): Promise => { return kubernetesClient.getCurrentContextName(); }); diff --git a/packages/main/src/plugin/kubernetes-client.ts b/packages/main/src/plugin/kubernetes-client.ts index efae009ff92..99e8bca1abf 100644 --- a/packages/main/src/plugin/kubernetes-client.ts +++ b/packages/main/src/plugin/kubernetes-client.ts @@ -27,8 +27,9 @@ import type { V1Ingress, V1ContainerState, V1APIResource, + V1APIGroup, } from '@kubernetes/client-node'; -import { NetworkingV1Api } from '@kubernetes/client-node'; +import { ApisApi, NetworkingV1Api } from '@kubernetes/client-node'; import { AppsV1Api } from '@kubernetes/client-node'; import { CustomObjectsApi } from '@kubernetes/client-node'; import { CoreV1Api, KubeConfig, Log, Watch, VersionApi } from '@kubernetes/client-node'; @@ -85,9 +86,7 @@ function toPodInfo(pod: V1Pod): PodInfo { }; } -const OPENSHIFT_CONSOLE_NAMESPACE = 'openshift-config-managed'; - -const OPENSHIFT_CONSOLE_CONFIG_MAP = 'console-public'; +const OPENSHIFT_PROJECT_API_GROUP = 'project.openshift.io'; /** * Handle calls to kubernetes API @@ -108,6 +107,8 @@ export class KubernetesClient { // eslint-disable-next-line @typescript-eslint/no-explicit-any private kubeWatcher: any | undefined; + private apiGroups = new Array(); + /* a Cache of API resources for the cluster. This is used to compute the plural when dealing with custom resources. The key is the apiGroup (including version) like 'networking.k8s.io/v1' @@ -214,6 +215,22 @@ export class KubernetesClient { } } + async fetchAPIGroups() { + this.apiGroups = []; + try { + if (this.kubeConfig) { + const result = await this.kubeConfig.makeApiClient(ApisApi).getAPIVersions(); + this.apiGroups = result?.body.groups; + } + } catch (err) { + console.log(`Error while fetching API groups: ${err}`); + } + } + + async isAPIGroupSupported(group: string): Promise { + return this.apiGroups.filter(g => g.name === group).length > 0; + } + getContexts(): Context[] { return this.kubeConfig.contexts; } @@ -240,11 +257,11 @@ export class KubernetesClient { let namespace; try { - const cm = await this.readNamespacedConfigMap(OPENSHIFT_CONSOLE_CONFIG_MAP, OPENSHIFT_CONSOLE_NAMESPACE); - if (cm) { + const projectGroupSupported = await this.isAPIGroupSupported(OPENSHIFT_PROJECT_API_GROUP); + if (projectGroupSupported) { const projects = await ctx .makeApiClient(CustomObjectsApi) - .listClusterCustomObject('project.openshift.io', 'v1', 'projects'); + .listClusterCustomObject(OPENSHIFT_PROJECT_API_GROUP, 'v1', 'projects'); // eslint-disable-next-line @typescript-eslint/no-explicit-any if ((projects?.body as any)?.items.length > 0) { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -287,6 +304,7 @@ export class KubernetesClient { } this.setupKubeWatcher(); this.apiResources.clear(); + await this.fetchAPIGroups(); this.apiSender.send('pod-event'); } diff --git a/packages/preload/src/index.ts b/packages/preload/src/index.ts index 7c82a455874..f07e809cf97 100644 --- a/packages/preload/src/index.ts +++ b/packages/preload/src/index.ts @@ -1114,6 +1114,10 @@ function initExposure(): void { }, ); + contextBridge.exposeInMainWorld('kubernetesIsAPIGroupSupported', async (group: string): Promise => { + return ipcInvoke('kubernetes-client:isAPIGroupSupported', group); + }); + contextBridge.exposeInMainWorld('kubernetesCreatePod', async (namespace: string, pod: V1Pod): Promise => { return ipcInvoke('kubernetes-client:createPod', namespace, pod); }); diff --git a/packages/renderer/src/lib/pod/DeployPodToKube.spec.ts b/packages/renderer/src/lib/pod/DeployPodToKube.spec.ts index 80a22796e84..714067105ed 100644 --- a/packages/renderer/src/lib/pod/DeployPodToKube.spec.ts +++ b/packages/renderer/src/lib/pod/DeployPodToKube.spec.ts @@ -35,6 +35,7 @@ const telemetryTrackMock = vi.fn(); const kubernetesCreatePodMock = vi.fn(); const kubernetesCreateIngressMock = vi.fn(); const kubernetesCreateServiceMock = vi.fn(); +const kubernetesIsAPIGroupSupported = vi.fn(); beforeEach(() => { Object.defineProperty(window, 'generatePodmanKube', { @@ -62,6 +63,9 @@ beforeEach(() => { Object.defineProperty(window, 'kubernetesCreateService', { value: kubernetesCreateServiceMock, }); + Object.defineProperty(window, 'kubernetesIsAPIGroupSupported', { + value: kubernetesIsAPIGroupSupported, + }); Object.defineProperty(window, 'telemetryTrack', { value: telemetryTrackMock, }); @@ -129,8 +133,9 @@ test('Expect to send telemetry event with OpenShift', async () => { data: { consoleURL: 'https://console-openshift-console.apps.cluster-1.example.com', }, - }), - await waitRender({}); + }); + kubernetesIsAPIGroupSupported.mockResolvedValue(true); + await waitRender({}); const createButton = screen.getByRole('button', { name: 'Deploy' }); expect(createButton).toBeInTheDocument(); expect(createButton).toBeEnabled(); @@ -173,6 +178,9 @@ test('When deploying a pod, volumes should not be added (they are deleted by pod expect(createButton).toBeInTheDocument(); expect(createButton).toBeEnabled(); + const useRestricted = screen.getByTestId('useRestricted'); + await fireEvent.click(useRestricted); + // Press the deploy button await fireEvent.click(createButton); @@ -192,6 +200,41 @@ test('When deploying a pod, volumes should not be added (they are deleted by pod ); }); +test('When deploying a pod, restricted security context is added', async () => { + await waitRender({}); + const createButton = screen.getByRole('button', { name: 'Deploy' }); + expect(createButton).toBeInTheDocument(); + expect(createButton).toBeEnabled(); + + // Press the deploy button + await fireEvent.click(createButton); + + // Expect kubernetesCreatePod to be called with default namespace and a modified bodyPod with volumes removed + await waitFor(() => + expect(kubernetesCreatePodMock).toBeCalledWith('default', { + metadata: { name: 'hello' }, + spec: { + containers: [ + { + name: 'hello', + image: 'hello-world', + securityContext: { + allowPrivilegeEscalation: false, + capabilities: { + drop: ['ALL'], + }, + runAsNonRoot: true, + seccompProfile: { + type: 'RuntimeDefault', + }, + }, + }, + ], + }, + }), + ); +}); + test('Fail to deploy ingress if service is not selected', async () => { await waitRender({}); const createButton = screen.getByRole('button', { name: 'Deploy' }); diff --git a/packages/renderer/src/lib/pod/DeployPodToKube.svelte b/packages/renderer/src/lib/pod/DeployPodToKube.svelte index 97759afd314..15cc31cd698 100644 --- a/packages/renderer/src/lib/pod/DeployPodToKube.svelte +++ b/packages/renderer/src/lib/pod/DeployPodToKube.svelte @@ -7,6 +7,7 @@ import type { V1Route } from '../../../../main/src/plugin/api/openshift-types'; import type { V1NamespaceList } from '@kubernetes/client-node/dist/api'; import ErrorMessage from '../ui/ErrorMessage.svelte'; import WarningMessage from '../ui/WarningMessage.svelte'; +import { ensureRestrictedSecurityContext } from '/@/lib/pod/pod-utils'; export let resourceId: string; export let engineId: string; @@ -22,9 +23,11 @@ let deployError = ''; let deployWarning = ''; let updatePodInterval: NodeJS.Timeout; let openshiftConsoleURL: string; +let openshiftRouteGroupSupported = false; let deployUsingServices = true; let deployUsingRoutes = true; +let deployUsingRestrictedSecurityContext = true; let createdPod = undefined; let bodyPod; @@ -62,6 +65,7 @@ onMount(async () => { // check if there is OpenShift and then grab openshift console URL try { + openshiftRouteGroupSupported = await window.kubernetesIsAPIGroupSupported('route.openshift.io'); const openshiftConfigMap = await window.kubernetesReadNamespacedConfigMap( 'console-public', 'openshift-config-managed', @@ -158,7 +162,7 @@ async function deployToKube() { }; servicesToCreate.push(service); - if (openshiftConsoleURL && deployUsingRoutes) { + if (openshiftRouteGroupSupported && deployUsingRoutes) { // Create OpenShift route object const route = { apiVersion: 'route.openshift.io/v1', @@ -250,7 +254,7 @@ async function deployToKube() { useRoutes: deployUsingRoutes, createIngress: createIngress, }; - if (openshiftConsoleURL) { + if (openshiftRouteGroupSupported) { eventProperties['isOpenshift'] = true; } @@ -281,6 +285,10 @@ async function deployToKube() { }); } + if (deployUsingRestrictedSecurityContext) { + ensureRestrictedSecurityContext(bodyPod); + } + // create pod createdPod = await window.kubernetesCreatePod(currentNamespace, bodyPod); @@ -361,8 +369,25 @@ function updateKubeResult() { policy may prevent to use hostPort. +
+ + + Update Kubernetes manifest to respect the Pod security restricted profile. +
+ - {#if !openshiftConsoleURL && deployUsingServices} + {#if !openshiftRouteGroupSupported && deployUsingServices}
@@ -401,7 +426,7 @@ function updateKubeResult() { {/if} - {#if openshiftConsoleURL} + {#if openshiftRouteGroupSupported}
diff --git a/packages/renderer/src/lib/pod/pod-utils.spec.ts b/packages/renderer/src/lib/pod/pod-utils.spec.ts new file mode 100644 index 00000000000..bdca106ab1a --- /dev/null +++ b/packages/renderer/src/lib/pod/pod-utils.spec.ts @@ -0,0 +1,81 @@ +/********************************************************************** + * Copyright (C) 2023 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + ***********************************************************************/ + +/* eslint-disable @typescript-eslint/no-explicit-any */ + +import { test, expect } from 'vitest'; +import { ensureRestrictedSecurityContext } from '/@/lib/pod/pod-utils'; + +function verifyPodSecurityContext(containers: any[], type = 'RuntimeDefault') { + containers.forEach(container => { + const securityContext = container.securityContext; + expect(securityContext).toBeDefined(); + expect(securityContext.allowPrivilegeEscalation).toBeFalsy(); + expect(securityContext.runAsNonRoot).toBeTruthy(); + expect(securityContext.seccompProfile).toBeDefined(); + expect(securityContext.seccompProfile.type).toBe(type); + expect(securityContext.capabilities).toBeDefined(); + expect(securityContext.capabilities.drop).toBeDefined(); + expect(securityContext.capabilities.drop).toContain('ALL'); + }); +} + +test('Expect security context to be added to single container pod', async () => { + const pod = { + kind: 'Pod', + apiversion: 'v1', + spec: { + containers: [{ image: 'image' }], + }, + }; + ensureRestrictedSecurityContext(pod); + verifyPodSecurityContext(pod.spec.containers); +}); + +test('Expect security context to be keep seccompProfile.type', async () => { + const pod = { + kind: 'Pod', + apiversion: 'v1', + spec: { + containers: [ + { + image: 'image', + securityContext: { + seccompProfile: { + type: 'Localhost', + }, + }, + }, + ], + }, + }; + ensureRestrictedSecurityContext(pod); + verifyPodSecurityContext(pod.spec.containers, 'Localhost'); +}); + +test('Expect security context to be added to dual containers pod', async () => { + const pod = { + kind: 'Pod', + apiversion: 'v1', + spec: { + containers: [{ image: 'image1' }, { image: 'image2' }], + }, + }; + ensureRestrictedSecurityContext(pod); + verifyPodSecurityContext(pod.spec.containers); +}); diff --git a/packages/renderer/src/lib/pod/pod-utils.ts b/packages/renderer/src/lib/pod/pod-utils.ts index f5292297469..f1ebc2ca8db 100644 --- a/packages/renderer/src/lib/pod/pod-utils.ts +++ b/packages/renderer/src/lib/pod/pod-utils.ts @@ -20,6 +20,7 @@ import moment from 'moment'; import humanizeDuration from 'humanize-duration'; import type { PodInfo } from '../../../../main/src/plugin/api/pod-info'; import type { PodInfoUI } from './PodInfoUI'; + export class PodUtils { getStatus(podinfo: PodInfo): string { return (podinfo.Status || '').toUpperCase(); @@ -64,3 +65,36 @@ export class PodUtils { }; } } + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function ensureRestrictedSecurityContext(body: any) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + body.spec?.containers?.forEach((container: any) => { + if (!container.securityContext) { + container.securityContext = {}; + } + container.securityContext.allowPrivilegeEscalation = false; + if (!container.securityContext.runAsNonRoot) { + container.securityContext.runAsNonRoot = true; + } + if (!container.securityContext.seccompProfile) { + container.securityContext.seccompProfile = {}; + } + if ( + !container.securityContext.seccompProfile.type || + (container.securityContext.seccompProfile.type !== 'RuntimeDefault' && + container.securityContext.seccompProfile.type !== 'Localhost') + ) { + container.securityContext.seccompProfile.type = 'RuntimeDefault'; + } + if (!container.securityContext.capabilities) { + container.securityContext.capabilities = {}; + } + if (!container.securityContext.capabilities.drop) { + container.securityContext.capabilities.drop = []; + } + if (container.securityContext.capabilities.drop.indexOf('ALL') === -1) { + container.securityContext.capabilities.drop.push('ALL'); + } + }); +}