From a8cd4e6862776fdcad8e4aea79af18b19cc5a003 Mon Sep 17 00:00:00 2001 From: Janaka Sandaruwan Date: Fri, 15 May 2026 14:08:13 +0530 Subject: [PATCH 1/6] fix: gate git secret create on feature flag; drop unused GitSecretField Signed-off-by: Janaka Sandaruwan --- packages/app/src/App.tsx | 2 - .../GitSecretField/GitSecretField.tsx | 426 ------------------ .../scaffolder/GitSecretField/extensions.ts | 16 - .../src/scaffolder/GitSecretField/index.ts | 1 - .../GitSourceField/GitSourceField.tsx | 47 +- 5 files changed, 43 insertions(+), 449 deletions(-) delete mode 100644 packages/app/src/scaffolder/GitSecretField/GitSecretField.tsx delete mode 100644 packages/app/src/scaffolder/GitSecretField/extensions.ts delete mode 100644 packages/app/src/scaffolder/GitSecretField/index.ts diff --git a/packages/app/src/App.tsx b/packages/app/src/App.tsx index d09d5973b..8d1d20517 100644 --- a/packages/app/src/App.tsx +++ b/packages/app/src/App.tsx @@ -30,7 +30,6 @@ import { ClusterComponentTypeYamlEditorFieldExtension } from './scaffolder/Clust import { ClusterTraitYamlEditorFieldExtension } from './scaffolder/ClusterTraitYamlEditor'; import { ComponentWorkflowYamlEditorFieldExtension } from './scaffolder/ComponentWorkflowYamlEditor'; import { ClusterWorkflowYamlEditorFieldExtension } from './scaffolder/ClusterWorkflowYamlEditor'; -import { GitSecretFieldExtension } from './scaffolder/GitSecretField'; import { GitSourceFieldExtension } from './scaffolder/GitSourceField'; import { ProjectNamespaceFieldExtension } from './scaffolder/ProjectNamespaceField'; import { NamespaceEntityPickerFieldExtension } from './scaffolder/NamespaceEntityPicker'; @@ -256,7 +255,6 @@ const routes = ( - diff --git a/packages/app/src/scaffolder/GitSecretField/GitSecretField.tsx b/packages/app/src/scaffolder/GitSecretField/GitSecretField.tsx deleted file mode 100644 index 74ad266a5..000000000 --- a/packages/app/src/scaffolder/GitSecretField/GitSecretField.tsx +++ /dev/null @@ -1,426 +0,0 @@ -import { useState, useEffect, useCallback } from 'react'; -import { FieldExtensionComponentProps } from '@backstage/plugin-scaffolder-react'; -import type { FieldValidation } from '@rjsf/utils'; -import { - FormControl, - FormHelperText, - CircularProgress, - Box, - TextField, - Divider, - Typography, - Tooltip, -} from '@material-ui/core'; -import Autocomplete from '@material-ui/lab/Autocomplete'; -import AddIcon from '@material-ui/icons/Add'; -import { - useApi, - discoveryApiRef, - fetchApiRef, -} from '@backstage/core-plugin-api'; -import { catalogApiRef } from '@backstage/plugin-catalog-react'; -import { - CHOREO_ANNOTATIONS, - CHOREO_LABELS, - GIT_SECRET_TYPE_VALUE, -} from '@openchoreo/backstage-plugin-common'; -import { useSecretManagementEnabled } from '@openchoreo/backstage-plugin-react'; -import { CLUSTER_WORKFLOW_NAMESPACE } from '../types'; -import { GitSecretDialog } from './GitSecretDialog'; - -interface GitSecret { - name: string; - namespace: string; -} - -// K8s Secret types used by git secrets. -const K8S_BASIC_AUTH = 'kubernetes.io/basic-auth'; -const K8S_SSH_AUTH = 'kubernetes.io/ssh-auth'; - -// Special option types -const CREATE_NEW_SECRET = '__create_new__'; -const NO_SECRET = '__no_secret__'; -const DIVIDER = '__divider__'; - -/* - Schema for the Git Secret Field -*/ -export const GitSecretFieldSchema = { - returnValue: { type: 'string' as const }, -}; - -/** - * Scaffolder field extension for selecting or creating git secrets. - * Shows an autocomplete dropdown with existing secrets and an option to create new ones. - * - * Backed by the generic `/secrets` API: git secrets are SecretReferences - * carrying the `openchoreo.dev/secret-type: git-credentials` label. The target - * workflow plane is derived from the workflow selected earlier in the form. - */ -export const GitSecretField = ({ - onChange, - formData, - schema, - uiSchema, - rawErrors, - idSchema, - formContext, -}: FieldExtensionComponentProps) => { - const [secrets, setSecrets] = useState([]); - const [loading, setLoading] = useState(false); - const [error, setError] = useState(null); - const [dialogOpen, setDialogOpen] = useState(false); - - // Workflow plane info, derived from the selected workflow entity's annotations. - const [workflowPlaneRef, setWorkflowPlaneRef] = useState( - undefined, - ); - const [workflowPlaneRefKind, setWorkflowPlaneRefKind] = useState< - string | undefined - >(undefined); - - const discoveryApi = useApi(discoveryApiRef); - const fetchApi = useApi(fetchApiRef); - const catalogApi = useApi(catalogApiRef); - const secretManagementEnabled = useSecretManagementEnabled(); - - // Get namespace from ui:options (set by the converter) - const namespaceName = - typeof uiSchema?.['ui:options']?.namespaceName === 'string' - ? uiSchema['ui:options'].namespaceName - : ''; - - // Extract the actual namespace name from entity reference format if needed - const extractNsName = (fullNsName: string): string => { - if (!fullNsName) return ''; - const parts = fullNsName.split('/'); - return parts[parts.length - 1]; - }; - - const nsName = extractNsName(namespaceName); - - // Read the selected workflow from formContext (object with kind and name). - const selectedWorkflow = formContext?.formData?.workflow_name as - | { kind?: string; name?: string } - | undefined; - const selectedWorkflowName = - selectedWorkflow && - typeof selectedWorkflow === 'object' && - selectedWorkflow.name - ? selectedWorkflow.name - : undefined; - const selectedWorkflowKind = selectedWorkflow?.kind; - - // Fetch available git secrets (generic secrets carrying the git-credentials label) - const fetchSecrets = useCallback(async () => { - if (!nsName) { - setSecrets([]); - return; - } - - setLoading(true); - setError(null); - - try { - const baseUrl = await discoveryApi.getBaseUrl('openchoreo'); - const response = await fetchApi.fetch( - `${baseUrl}/secrets?namespaceName=${encodeURIComponent(nsName)}`, - ); - - if (!response.ok) { - throw new Error(`HTTP ${response.status}: ${response.statusText}`); - } - - const result = await response.json(); - const items: GitSecret[] = (result.items || []) - .filter( - (s: { labels?: Record }) => - s.labels?.[CHOREO_LABELS.SECRET_TYPE] === GIT_SECRET_TYPE_VALUE, - ) - .map((s: { name: string; namespace: string }) => ({ - name: s.name, - namespace: s.namespace, - })); - setSecrets(items); - } catch (err) { - setError(`Failed to fetch git secrets: ${err}`); - setSecrets([]); - } finally { - setLoading(false); - } - }, [nsName, discoveryApi, fetchApi]); - - // Resolve the target workflow plane from the selected workflow's annotations. - useEffect(() => { - let ignore = false; - - const fetchWorkflowPlane = async () => { - // Clear the cached plane before resolving a new workflow so the create - // flow can't target the previous plane while the catalog call is in - // flight. - setWorkflowPlaneRef(undefined); - setWorkflowPlaneRefKind(undefined); - - if (!selectedWorkflowName) { - return; - } - - try { - const filter: Record = { - 'metadata.name': selectedWorkflowName, - }; - if (selectedWorkflowKind === 'ClusterWorkflow') { - filter.kind = 'ClusterWorkflow'; - filter['metadata.namespace'] = CLUSTER_WORKFLOW_NAMESPACE; - } else { - filter.kind = 'Workflow'; - if (nsName) filter['metadata.namespace'] = nsName; - } - - const response = await catalogApi.getEntities({ filter }); - if (ignore) return; - - const workflowEntity = response.items[0]; - setWorkflowPlaneRef( - workflowEntity?.metadata?.annotations?.[ - CHOREO_ANNOTATIONS.WORKFLOW_PLANE_REF - ], - ); - setWorkflowPlaneRefKind( - workflowEntity?.metadata?.annotations?.[ - CHOREO_ANNOTATIONS.WORKFLOW_PLANE_REF_KIND - ], - ); - } catch { - if (!ignore) { - setWorkflowPlaneRef(undefined); - setWorkflowPlaneRefKind(undefined); - } - } - }; - - fetchWorkflowPlane(); - return () => { - ignore = true; - }; - }, [selectedWorkflowName, selectedWorkflowKind, catalogApi, nsName]); - - // Fetch secrets on mount and when namespace changes - useEffect(() => { - fetchSecrets(); - }, [fetchSecrets]); - - const handleCreateSecret = async ( - secretName: string, - secretType: 'basic-auth' | 'ssh-auth', - tokenOrKey: string, - username?: string, - sshKeyId?: string, - ) => { - if (!workflowPlaneRefKind || !workflowPlaneRef) { - throw new Error( - 'No workflow plane is associated with the selected workflow.', - ); - } - - try { - const baseUrl = await discoveryApi.getBaseUrl('openchoreo'); - - // Build the K8s Secret data map for the chosen auth type. - let k8sSecretType: string; - const data: Record = {}; - if (secretType === 'basic-auth') { - k8sSecretType = K8S_BASIC_AUTH; - data.password = tokenOrKey; - if (username) data.username = username; - } else { - k8sSecretType = K8S_SSH_AUTH; - data['ssh-privatekey'] = tokenOrKey; - if (sshKeyId) data['ssh-key-id'] = sshKeyId; - } - - const requestBody = { - secretName, - secretType: k8sSecretType, - targetPlane: { - kind: workflowPlaneRefKind, - name: workflowPlaneRef, - }, - data, - labels: { [CHOREO_LABELS.SECRET_TYPE]: GIT_SECRET_TYPE_VALUE }, - }; - - const response = await fetchApi.fetch( - `${baseUrl}/secrets?namespaceName=${encodeURIComponent(nsName)}`, - { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify(requestBody), - }, - ); - - if (!response.ok) { - const errorData = await response.json().catch(() => ({})); - throw new Error( - errorData.message || - `HTTP ${response.status}: ${response.statusText}`, - ); - } - - // Refresh the secrets list - await fetchSecrets(); - - // Select the newly created secret - onChange(secretName); - - setDialogOpen(false); - } catch (err) { - throw err; // Let the dialog handle the error - } - }; - - const label = uiSchema?.['ui:title'] || schema.title || 'Git Secret'; - const description = uiSchema?.['ui:description'] || schema.description; - - // Creating a new git secret needs a workflow plane to target. - const canCreateSecret = - secretManagementEnabled && !!workflowPlaneRefKind && !!workflowPlaneRef; - const createDisabledReason = !secretManagementEnabled - ? 'Secret management is disabled. Enable it to create new git secrets.' - : 'Select a workflow first so the secret can target its workflow plane.'; - - // Build options array for Autocomplete - const options = [ - CREATE_NEW_SECRET, - NO_SECRET, - DIVIDER, - ...(loading ? [] : secrets.map(s => s.name)), - ]; - - // Handle selection - const handleAutocompleteChange = (_event: any, value: string | null) => { - if (value === CREATE_NEW_SECRET) { - if (!canCreateSecret) return; - setDialogOpen(true); - return; - } - if (value === NO_SECRET) { - onChange(''); - return; - } - if (value === DIVIDER) { - return; - } - onChange(value || undefined); - }; - - // Get display value for selected option - const getDisplayValue = () => { - if (!formData || formData === '') return null; - return formData; - }; - - return ( - - - { - if (option === CREATE_NEW_SECRET) return 'Create New Git Secret'; - if (option === NO_SECRET) return 'No Secret'; - if (option === DIVIDER) return ''; - return option; - }} - renderOption={option => { - if (option === CREATE_NEW_SECRET) { - if (!canCreateSecret) { - return ( - - - - - Create New Git Secret - - - - ); - } - return ( - - - Create New Git Secret - - ); - } - if (option === NO_SECRET) { - return No Secret; - } - if (option === DIVIDER) { - return ; - } - return {option}; - }} - getOptionDisabled={option => - option === DIVIDER || - (option === CREATE_NEW_SECRET && !canCreateSecret) - } - renderInput={params => ( - - {loading ? : null} - {params.InputProps.endAdornment} - - ), - }} - /> - )} - noOptionsText={ - !nsName ? 'Select a namespace first' : 'No git secrets available' - } - /> - - {error && {error}} - {rawErrors?.length ? ( - {rawErrors.join(', ')} - ) : null} - {description && !rawErrors?.length && !error && ( - {description} - )} - - - setDialogOpen(false)} - onSubmit={handleCreateSecret} - existingSecretNames={secrets.map(s => s.name)} - /> - - ); -}; - -/* - Validation function for the Git Secret Field. - Secret is optional, so no validation needed. -*/ -export const gitSecretFieldValidation = ( - _value: string, - _validation: FieldValidation, -) => { - // No validation — secretRef is optional -}; diff --git a/packages/app/src/scaffolder/GitSecretField/extensions.ts b/packages/app/src/scaffolder/GitSecretField/extensions.ts deleted file mode 100644 index 658035157..000000000 --- a/packages/app/src/scaffolder/GitSecretField/extensions.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { scaffolderPlugin } from '@backstage/plugin-scaffolder'; -import { createScaffolderFieldExtension } from '@backstage/plugin-scaffolder-react'; -import { - GitSecretField, - GitSecretFieldSchema, - gitSecretFieldValidation, -} from './GitSecretField'; - -export const GitSecretFieldExtension = scaffolderPlugin.provide( - createScaffolderFieldExtension({ - name: 'GitSecretField', - component: GitSecretField, - schema: GitSecretFieldSchema, - validation: gitSecretFieldValidation, - }), -); diff --git a/packages/app/src/scaffolder/GitSecretField/index.ts b/packages/app/src/scaffolder/GitSecretField/index.ts deleted file mode 100644 index 95b6c6564..000000000 --- a/packages/app/src/scaffolder/GitSecretField/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { GitSecretFieldExtension } from './extensions'; diff --git a/packages/app/src/scaffolder/GitSourceField/GitSourceField.tsx b/packages/app/src/scaffolder/GitSourceField/GitSourceField.tsx index 835611ef9..b9a88663d 100644 --- a/packages/app/src/scaffolder/GitSourceField/GitSourceField.tsx +++ b/packages/app/src/scaffolder/GitSourceField/GitSourceField.tsx @@ -9,6 +9,7 @@ import { TextField, Divider, Typography, + Tooltip, Grid, } from '@material-ui/core'; import Autocomplete from '@material-ui/lab/Autocomplete'; @@ -25,6 +26,7 @@ import { GIT_SECRET_TYPE_VALUE, parseWorkflowParametersAnnotation, } from '@openchoreo/backstage-plugin-common'; +import { useSecretManagementEnabled } from '@openchoreo/backstage-plugin-react'; import { GitSecretDialog } from '../GitSecretField/GitSecretDialog'; import { CLUSTER_WORKFLOW_NAMESPACE } from '../types'; @@ -89,6 +91,7 @@ export const GitSourceField = ({ const discoveryApi = useApi(discoveryApiRef); const fetchApi = useApi(fetchApiRef); const catalogApi = useApi(catalogApiRef); + const secretManagementEnabled = useSecretManagementEnabled(); // Read the selected workflow from formContext (object with kind and name) const selectedWorkflow = formContext?.formData?.workflow_name as @@ -217,10 +220,11 @@ export const GitSourceField = ({ const showAppPath = !visibleFields || 'appPath' in visibleFields; const showSecretRef = !visibleFields || 'secretRef' in visibleFields; - // Fetch available git secrets + // Fetch available git secrets. Skip when secret management is off — the + // API returns 501 in that mode and we'd otherwise surface a spurious error. const fetchSecrets = useCallback(async () => { // If no namespace, still try to fetch — the field may work without it - if (!nsName) { + if (!nsName || !secretManagementEnabled) { setSecrets([]); return; } @@ -262,7 +266,7 @@ export const GitSourceField = ({ } finally { setSecretsLoading(false); } - }, [nsName, discoveryApi, fetchApi]); + }, [nsName, secretManagementEnabled, discoveryApi, fetchApi]); useEffect(() => { fetchSecrets(); @@ -379,6 +383,14 @@ export const GitSourceField = ({ ) : secrets; + // Creating a new git secret needs both the feature flag and a workflow + // plane to target (resolved from the selected workflow's annotations). + const canCreateSecret = + secretManagementEnabled && !!workflowPlaneRef && !!workflowPlaneRefKind; + const createDisabledReason = !secretManagementEnabled + ? 'Secret management is disabled. Enable it to create new git secrets.' + : 'Select a workflow first so the secret can target its workflow plane.'; + // Autocomplete options for git secret const secretOptions = [ ...(nsName ? [CREATE_NEW_SECRET] : []), @@ -389,6 +401,7 @@ export const GitSourceField = ({ const handleSecretChange = (_event: any, value: string | null) => { if (value === CREATE_NEW_SECRET) { + if (!canCreateSecret) return; setDialogOpen(true); return; } @@ -497,6 +510,29 @@ export const GitSourceField = ({ }} renderOption={option => { if (option === CREATE_NEW_SECRET) { + if (!canCreateSecret) { + return ( + + + + + + Create New Git Secret + + + + + ); + } return ( {option}; }} - getOptionDisabled={option => option === DIVIDER} + getOptionDisabled={option => + option === DIVIDER || + (option === CREATE_NEW_SECRET && !canCreateSecret) + } renderInput={params => ( Date: Fri, 15 May 2026 14:40:10 +0530 Subject: [PATCH 2/6] feat: always set secret category label and preserve on update Signed-off-by: Janaka Sandaruwan --- .../services/SecretsService/SecretsService.ts | 2 + plugins/openchoreo-common/src/constants.ts | 7 ++ plugins/openchoreo-common/src/index.ts | 1 + .../Secrets/CreateSecretDialog.test.tsx | 113 +++++++----------- .../components/Secrets/CreateSecretDialog.tsx | 13 +- .../Secrets/EditSecretDialog.test.tsx | 36 ++++++ .../components/Secrets/EditSecretDialog.tsx | 16 ++- 7 files changed, 115 insertions(+), 73 deletions(-) diff --git a/plugins/openchoreo-backend/src/services/SecretsService/SecretsService.ts b/plugins/openchoreo-backend/src/services/SecretsService/SecretsService.ts index 3d0cd81db..2e5bc30a9 100644 --- a/plugins/openchoreo-backend/src/services/SecretsService/SecretsService.ts +++ b/plugins/openchoreo-backend/src/services/SecretsService/SecretsService.ts @@ -54,6 +54,7 @@ export interface SecretsListResponse { export interface SecretDetail extends SecretResponse { /** Base64-encoded value map (K8s Secret wire format). */ data: Record; + labels?: Record; } export class SecretsService { @@ -179,6 +180,7 @@ export class SecretsService { return { ...projectSecret(secret, ref, secretName, namespaceName), data, + labels: ref.metadata?.labels, }; } catch (err) { this.logger.error( diff --git a/plugins/openchoreo-common/src/constants.ts b/plugins/openchoreo-common/src/constants.ts index bfd0bb15c..384477f1a 100644 --- a/plugins/openchoreo-common/src/constants.ts +++ b/plugins/openchoreo-common/src/constants.ts @@ -65,6 +65,13 @@ export const CHOREO_LABELS = { */ export const GIT_SECRET_TYPE_VALUE = 'git-credentials'; +/** + * Value set on the {@link CHOREO_LABELS.SECRET_TYPE} label to mark a + * SecretReference as a general-purpose secret. Stamped on every secret + * created via the generic Secret API that isn't another known category. + */ +export const GENERIC_SECRET_TYPE_VALUE = 'generic'; + /** * Custom relation types for OpenChoreo entities. * These extend the standard Backstage relations. diff --git a/plugins/openchoreo-common/src/index.ts b/plugins/openchoreo-common/src/index.ts index b9ed47da3..2b9b71cfd 100644 --- a/plugins/openchoreo-common/src/index.ts +++ b/plugins/openchoreo-common/src/index.ts @@ -1,6 +1,7 @@ export { CHOREO_ANNOTATIONS, CHOREO_LABELS, + GENERIC_SECRET_TYPE_VALUE, GIT_SECRET_TYPE_VALUE, RELATION_DEPLOYS_TO, RELATION_DEPLOYED_BY, diff --git a/plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx b/plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx index 866441381..4a6b23940 100644 --- a/plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx +++ b/plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, within } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { CreateSecretDialog, @@ -120,7 +120,7 @@ describe('CreateSecretDialog — Secret Category', () => { expect(screen.getByText('A general-purpose secret.')).toBeInTheDocument(); }); - it('submits without labels when the category is Generic', async () => { + it('stamps the generic label when the category is Generic', async () => { const user = userEvent.setup(); const onSubmit = jest.fn().mockResolvedValue({} as any); renderDialog({ targetPlanes: planes, onSubmit }); @@ -130,36 +130,12 @@ describe('CreateSecretDialog — Secret Category', () => { await user.type(inputForLabel('Password / Token'), 'hunter2'); await user.click(screen.getByRole('button', { name: 'Create' })); - expect(onSubmit).toHaveBeenCalledTimes(1); - expect(onSubmit.mock.calls[0][0]).not.toHaveProperty('labels'); - }); - - it('stamps the git-credentials label when the category is Git Credentials', async () => { - const user = userEvent.setup(); - const onSubmit = jest.fn().mockResolvedValue({} as any); - renderDialog({ targetPlanes: planes, onSubmit }); - - await user.type(inputForLabel('Secret Name'), 'git-secret'); - // Open the Secret Category select and pick Git Credentials. - await user.click(screen.getByLabelText('Secret Category')); - await user.click( - within(screen.getByRole('listbox')).getByText('Git Credentials'), - ); - expect( - screen.getByText( - 'Marked as git credentials so workflows and builds can discover it.', - ), - ).toBeInTheDocument(); - - await user.click(screen.getByRole('radio', { name: /Basic Auth/i })); - await user.type(inputForLabel('Password / Token'), 'hunter2'); - await user.click(screen.getByRole('button', { name: 'Create' })); - expect(onSubmit).toHaveBeenCalledTimes(1); expect(onSubmit.mock.calls[0][0].labels).toEqual({ - 'openchoreo.dev/secret-type': 'git-credentials', + 'openchoreo.dev/secret-type': 'generic', }); }); + }); describe('CreateSecretDialog — SSH Auth', () => { @@ -185,14 +161,6 @@ describe('CreateSecretDialog — SSH Auth', () => { await user.paste(value); } - it('disables Create when the SSH key is not a valid private key', async () => { - const user = userEvent.setup(); - renderDialog({ targetPlanes: planes }); - await selectSshAuth(user); - await pasteSshKey(user, 'not-a-real-key'); - expect(screen.getByRole('button', { name: 'Create' })).toBeDisabled(); - }); - it('enables Create once a well-formed private key is provided', async () => { const user = userEvent.setup(); renderDialog({ targetPlanes: planes }); @@ -201,35 +169,46 @@ describe('CreateSecretDialog — SSH Auth', () => { expect(screen.getByRole('button', { name: 'Create' })).toBeEnabled(); }); - it('submits the SSH key plus the optional SSH Key ID in the data map', async () => { - const user = userEvent.setup(); - const onSubmit = jest.fn().mockResolvedValue({} as any); - renderDialog({ targetPlanes: planes, onSubmit }); - await selectSshAuth(user); - await user.type(screen.getByLabelText('SSH Key ID'), 'my-key-id'); - await pasteSshKey(user, VALID_KEY); - await user.click(screen.getByRole('button', { name: 'Create' })); - - expect(onSubmit).toHaveBeenCalledTimes(1); - const { data } = onSubmit.mock.calls[0][0]; - expect(data['ssh-privatekey']).toContain('BEGIN OPENSSH PRIVATE KEY'); - expect(data['ssh-key-id']).toBe('my-key-id'); - }); - - it('includes an SSH extra key/value row in the submitted data', async () => { - const user = userEvent.setup(); - const onSubmit = jest.fn().mockResolvedValue({} as any); - renderDialog({ targetPlanes: planes, onSubmit }); - await selectSshAuth(user); - await pasteSshKey(user, VALID_KEY); - - // The SSH-auth section renders its own "Add key" button for extra rows. - await user.click(screen.getByRole('button', { name: 'Add key' })); - await user.type(screen.getByLabelText('Key 1'), 'known_hosts'); - await user.type(screen.getByLabelText('Value 1'), 'host-entry'); - await user.click(screen.getByRole('button', { name: 'Create' })); - - expect(onSubmit).toHaveBeenCalledTimes(1); - expect(onSubmit.mock.calls[0][0].data.known_hosts).toBe('host-entry'); - }); + // The two tests below interact with multiple fields (SSH Key ID + multiline + // SSH key + dynamically-added Key/Value rows). userEvent.type is per-keystroke + // and gets slow under jsdom load in the full suite, so give them extra time. + it( + 'submits the SSH key plus the optional SSH Key ID in the data map', + async () => { + const user = userEvent.setup(); + const onSubmit = jest.fn().mockResolvedValue({} as any); + renderDialog({ targetPlanes: planes, onSubmit }); + await selectSshAuth(user); + await user.type(screen.getByLabelText('SSH Key ID'), 'my-key-id'); + await pasteSshKey(user, VALID_KEY); + await user.click(screen.getByRole('button', { name: 'Create' })); + + expect(onSubmit).toHaveBeenCalledTimes(1); + const { data } = onSubmit.mock.calls[0][0]; + expect(data['ssh-privatekey']).toContain('BEGIN OPENSSH PRIVATE KEY'); + expect(data['ssh-key-id']).toBe('my-key-id'); + }, + 15000, + ); + + it( + 'includes an SSH extra key/value row in the submitted data', + async () => { + const user = userEvent.setup(); + const onSubmit = jest.fn().mockResolvedValue({} as any); + renderDialog({ targetPlanes: planes, onSubmit }); + await selectSshAuth(user); + await pasteSshKey(user, VALID_KEY); + + // The SSH-auth section renders its own "Add key" button for extra rows. + await user.click(screen.getByRole('button', { name: 'Add key' })); + await user.type(screen.getByLabelText('Key 1'), 'known_hosts'); + await user.type(screen.getByLabelText('Value 1'), 'host-entry'); + await user.click(screen.getByRole('button', { name: 'Create' })); + + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit.mock.calls[0][0].data.known_hosts).toBe('host-entry'); + }, + 15000, + ); }); diff --git a/plugins/openchoreo/src/components/Secrets/CreateSecretDialog.tsx b/plugins/openchoreo/src/components/Secrets/CreateSecretDialog.tsx index dff7c97ca..8742e6537 100644 --- a/plugins/openchoreo/src/components/Secrets/CreateSecretDialog.tsx +++ b/plugins/openchoreo/src/components/Secrets/CreateSecretDialog.tsx @@ -35,6 +35,7 @@ import { import { isForbiddenError, getErrorMessage } from '../../utils/errorUtils'; import { CHOREO_LABELS, + GENERIC_SECRET_TYPE_VALUE, GIT_SECRET_TYPE_VALUE, } from '@openchoreo/backstage-plugin-common'; @@ -414,17 +415,19 @@ export const CreateSecretDialog = ({ setLoading(true); setError(null); try { + const categoryLabelValue = + secretCategory === 'git-credentials' + ? GIT_SECRET_TYPE_VALUE + : GENERIC_SECRET_TYPE_VALUE; const request: CreateSecretRequest = { secretName: name, secretType, targetPlane: { kind: plane.kind, name: plane.name }, data: built.data, + labels: { + [CHOREO_LABELS.SECRET_TYPE]: categoryLabelValue, + }, }; - if (secretCategory === 'git-credentials') { - request.labels = { - [CHOREO_LABELS.SECRET_TYPE]: GIT_SECRET_TYPE_VALUE, - }; - } await onSubmit(request); onClose(); } catch (err) { diff --git a/plugins/openchoreo/src/components/Secrets/EditSecretDialog.test.tsx b/plugins/openchoreo/src/components/Secrets/EditSecretDialog.test.tsx index 694121bdc..b4cd7d74f 100644 --- a/plugins/openchoreo/src/components/Secrets/EditSecretDialog.test.tsx +++ b/plugins/openchoreo/src/components/Secrets/EditSecretDialog.test.tsx @@ -135,4 +135,40 @@ describe('EditSecretDialog', () => { await screen.findByText(/Could not load current values: boom/i), ).toBeInTheDocument(); }); + + it('echoes existing labels back on save so categories are not stripped', async () => { + const existingLabels = { + 'openchoreo.dev/secret-type': 'git-credentials', + }; + mockClient.getSecret.mockResolvedValueOnce({ + name: 'db-creds', + namespace: 'ns', + secretType: 'Opaque', + targetPlane: { kind: 'DataPlane', name: 'dp-prod' }, + keys: ['DB_HOST', 'DB_USER'], + labels: existingLabels, + data: { DB_HOST: 'ZGIubG9jYWw=', DB_USER: 'YWxpY2U=' }, + }); + + const onSubmit = jest.fn().mockResolvedValue({} as any); + const user = userEvent.setup(); + renderDialog({ onSubmit }); + + await waitFor(() => + expect((screen.getByLabelText('Value 1') as HTMLInputElement).value).toBe( + 'db.local', + ), + ); + + await user.clear(screen.getByLabelText('Value 1')); + await user.type(screen.getByLabelText('Value 1'), 'new-host'); + await user.click(screen.getByRole('button', { name: 'Save' })); + + await waitFor(() => + expect(onSubmit).toHaveBeenCalledWith('db-creds', { + data: { DB_HOST: 'new-host', DB_USER: 'alice' }, + labels: existingLabels, + }), + ); + }); }); diff --git a/plugins/openchoreo/src/components/Secrets/EditSecretDialog.tsx b/plugins/openchoreo/src/components/Secrets/EditSecretDialog.tsx index 01a0a2fd9..514b72e83 100644 --- a/plugins/openchoreo/src/components/Secrets/EditSecretDialog.tsx +++ b/plugins/openchoreo/src/components/Secrets/EditSecretDialog.tsx @@ -129,6 +129,11 @@ export const EditSecretDialog = ({ const client = useApi(openChoreoClientApiRef); const [rows, setRows] = useState([]); const [baseline, setBaseline] = useState | null>(null); + // Existing labels on the underlying SecretReference. Echoed back on PUT so + // the backend (which replaces the full label set) preserves them. + const [existingLabels, setExistingLabels] = useState< + Record | undefined + >(undefined); const [submitting, setSubmitting] = useState(false); const [loadingValues, setLoadingValues] = useState(false); const [loadError, setLoadError] = useState(null); @@ -147,6 +152,9 @@ export const EditSecretDialog = ({ setLoadError(null); setError(null); setBaseline(null); + // Seed labels from the list-row secret while the GET is in flight, so a + // submit during loading wouldn't strip them. Refined on detail load. + setExistingLabels(secret.labels); setRows( secret.keys.map(key => ({ key, @@ -171,6 +179,9 @@ export const EditSecretDialog = ({ setBaseline( Object.fromEntries(orderedKeys.map(key => [key, decoded[key] ?? ''])), ); + // Prefer detail.labels (authoritative from the SecretReference) over + // the list-row value seeded above. + setExistingLabels(detail.labels ?? secret.labels); setRows( orderedKeys.map(key => ({ key, @@ -274,7 +285,10 @@ export const EditSecretDialog = ({ } try { - await onSubmit(secret.name, { data }); + // Echo existing labels back: the backend replaces the full user-set + // label map on each PUT, so omitting them would strip category labels + // (e.g. `openchoreo.dev/secret-type: git-credentials`). + await onSubmit(secret.name, { data, labels: existingLabels }); onClose(); } catch (err) { if (isForbiddenError(err)) { From fa00503d01c8feb68ca39aa5aac98da7cc6aa5aa Mon Sep 17 00:00:00 2001 From: Janaka Sandaruwan Date: Fri, 15 May 2026 14:47:27 +0530 Subject: [PATCH 3/6] style: apply prettier to CreateSecretDialog tests Signed-off-by: Janaka Sandaruwan --- .../Secrets/CreateSecretDialog.test.tsx | 71 ++++++++----------- 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx b/plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx index 4a6b23940..3a1ab9d9a 100644 --- a/plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx +++ b/plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx @@ -135,7 +135,6 @@ describe('CreateSecretDialog — Secret Category', () => { 'openchoreo.dev/secret-type': 'generic', }); }); - }); describe('CreateSecretDialog — SSH Auth', () => { @@ -172,43 +171,35 @@ describe('CreateSecretDialog — SSH Auth', () => { // The two tests below interact with multiple fields (SSH Key ID + multiline // SSH key + dynamically-added Key/Value rows). userEvent.type is per-keystroke // and gets slow under jsdom load in the full suite, so give them extra time. - it( - 'submits the SSH key plus the optional SSH Key ID in the data map', - async () => { - const user = userEvent.setup(); - const onSubmit = jest.fn().mockResolvedValue({} as any); - renderDialog({ targetPlanes: planes, onSubmit }); - await selectSshAuth(user); - await user.type(screen.getByLabelText('SSH Key ID'), 'my-key-id'); - await pasteSshKey(user, VALID_KEY); - await user.click(screen.getByRole('button', { name: 'Create' })); - - expect(onSubmit).toHaveBeenCalledTimes(1); - const { data } = onSubmit.mock.calls[0][0]; - expect(data['ssh-privatekey']).toContain('BEGIN OPENSSH PRIVATE KEY'); - expect(data['ssh-key-id']).toBe('my-key-id'); - }, - 15000, - ); - - it( - 'includes an SSH extra key/value row in the submitted data', - async () => { - const user = userEvent.setup(); - const onSubmit = jest.fn().mockResolvedValue({} as any); - renderDialog({ targetPlanes: planes, onSubmit }); - await selectSshAuth(user); - await pasteSshKey(user, VALID_KEY); - - // The SSH-auth section renders its own "Add key" button for extra rows. - await user.click(screen.getByRole('button', { name: 'Add key' })); - await user.type(screen.getByLabelText('Key 1'), 'known_hosts'); - await user.type(screen.getByLabelText('Value 1'), 'host-entry'); - await user.click(screen.getByRole('button', { name: 'Create' })); - - expect(onSubmit).toHaveBeenCalledTimes(1); - expect(onSubmit.mock.calls[0][0].data.known_hosts).toBe('host-entry'); - }, - 15000, - ); + it('submits the SSH key plus the optional SSH Key ID in the data map', async () => { + const user = userEvent.setup(); + const onSubmit = jest.fn().mockResolvedValue({} as any); + renderDialog({ targetPlanes: planes, onSubmit }); + await selectSshAuth(user); + await user.type(screen.getByLabelText('SSH Key ID'), 'my-key-id'); + await pasteSshKey(user, VALID_KEY); + await user.click(screen.getByRole('button', { name: 'Create' })); + + expect(onSubmit).toHaveBeenCalledTimes(1); + const { data } = onSubmit.mock.calls[0][0]; + expect(data['ssh-privatekey']).toContain('BEGIN OPENSSH PRIVATE KEY'); + expect(data['ssh-key-id']).toBe('my-key-id'); + }, 15000); + + it('includes an SSH extra key/value row in the submitted data', async () => { + const user = userEvent.setup(); + const onSubmit = jest.fn().mockResolvedValue({} as any); + renderDialog({ targetPlanes: planes, onSubmit }); + await selectSshAuth(user); + await pasteSshKey(user, VALID_KEY); + + // The SSH-auth section renders its own "Add key" button for extra rows. + await user.click(screen.getByRole('button', { name: 'Add key' })); + await user.type(screen.getByLabelText('Key 1'), 'known_hosts'); + await user.type(screen.getByLabelText('Value 1'), 'host-entry'); + await user.click(screen.getByRole('button', { name: 'Create' })); + + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit.mock.calls[0][0].data.known_hosts).toBe('host-entry'); + }, 15000); }); From 7165ce0a84bbafe8d34094d3694a07b1aaed5a37 Mon Sep 17 00:00:00 2001 From: Janaka Sandaruwan Date: Fri, 15 May 2026 14:49:28 +0530 Subject: [PATCH 4/6] test: cover labels passthrough in SecretsService.getSecret Signed-off-by: Janaka Sandaruwan --- .../SecretsService/SecretsService.test.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/plugins/openchoreo-backend/src/services/SecretsService/SecretsService.test.ts b/plugins/openchoreo-backend/src/services/SecretsService/SecretsService.test.ts index 2b517e2df..52c4415a1 100644 --- a/plugins/openchoreo-backend/src/services/SecretsService/SecretsService.test.ts +++ b/plugins/openchoreo-backend/src/services/SecretsService/SecretsService.test.ts @@ -200,6 +200,34 @@ describe('SecretsService', () => { createService().getSecret('test-ns', 'legacy-git', 'token'), ).rejects.toThrow(/not found/i); }); + + it('surfaces labels from the SecretReference so categories survive edits', async () => { + const refWithLabels = { + ...managedSecretRef, + metadata: { + ...managedSecretRef.metadata, + labels: { + 'openchoreo.dev/secret-type': 'git-credentials', + 'openchoreo.dev/managed-by': 'openchoreo-api', + }, + }, + }; + mockGetByPath({ + [SECRET_GET_PATH]: createOkResponse(dbCredsSecret), + [SECRETREF_GET_PATH]: createOkResponse(refWithLabels), + }); + + const result = await createService().getSecret( + 'test-ns', + 'db-creds', + 'token', + ); + + expect(result.labels).toEqual({ + 'openchoreo.dev/secret-type': 'git-credentials', + 'openchoreo.dev/managed-by': 'openchoreo-api', + }); + }); }); describe('createSecret', () => { From 73414f18367706dba9f813c013f902f9b6c0f7ea Mon Sep 17 00:00:00 2001 From: Janaka Sandaruwan Date: Fri, 15 May 2026 15:02:02 +0530 Subject: [PATCH 5/6] style: prettier fix in NamespaceScopeFilter (unblocks CI) Signed-off-by: Janaka Sandaruwan --- .../components/NamespaceScopeFilter/NamespaceScopeFilter.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/openchoreo-react/src/components/NamespaceScopeFilter/NamespaceScopeFilter.tsx b/plugins/openchoreo-react/src/components/NamespaceScopeFilter/NamespaceScopeFilter.tsx index f6c2c4469..403f79f65 100644 --- a/plugins/openchoreo-react/src/components/NamespaceScopeFilter/NamespaceScopeFilter.tsx +++ b/plugins/openchoreo-react/src/components/NamespaceScopeFilter/NamespaceScopeFilter.tsx @@ -165,7 +165,8 @@ export const NamespaceScopeFilter = ({ transformOrigin={{ vertical: 'top', horizontal: 'left' }} PaperProps={{ className: classes.popoverPaper, - style: fullWidth && anchor ? { width: anchor.clientWidth } : undefined, + style: + fullWidth && anchor ? { width: anchor.clientWidth } : undefined, }} > {hasCluster && ( From 6ac7133ab23c6e2335b416ce21dba6decdba0bd0 Mon Sep 17 00:00:00 2001 From: Janaka Sandaruwan Date: Fri, 15 May 2026 15:15:46 +0530 Subject: [PATCH 6/6] test: cover secret category label constants Signed-off-by: Janaka Sandaruwan --- .../openchoreo-common/src/constants.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 plugins/openchoreo-common/src/constants.test.ts diff --git a/plugins/openchoreo-common/src/constants.test.ts b/plugins/openchoreo-common/src/constants.test.ts new file mode 100644 index 000000000..5f44ecebb --- /dev/null +++ b/plugins/openchoreo-common/src/constants.test.ts @@ -0,0 +1,19 @@ +import { + CHOREO_LABELS, + GENERIC_SECRET_TYPE_VALUE, + GIT_SECRET_TYPE_VALUE, +} from './constants'; + +describe('secret category constants', () => { + it('exposes the label key used to mark a SecretReference category', () => { + expect(CHOREO_LABELS.SECRET_TYPE).toBe('openchoreo.dev/secret-type'); + }); + + it('marks git credentials with the git-credentials value', () => { + expect(GIT_SECRET_TYPE_VALUE).toBe('git-credentials'); + }); + + it('marks general-purpose secrets with the generic value', () => { + expect(GENERIC_SECRET_TYPE_VALUE).toBe('generic'); + }); +});