Skip to content

Commit

Permalink
support static prefix for k8s resource name (#3447)
Browse files Browse the repository at this point in the history
* support static prefix for k8s resource name

* rename isStaticPrefix -> staticPrefix
  • Loading branch information
christianvogt authored Nov 7, 2024
1 parent 5ec3d22 commit 0e4420f
Show file tree
Hide file tree
Showing 7 changed files with 189 additions and 28 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import * as React from 'react';
import { FormGroup, HelperText, TextInput, ValidatedOptions } from '@patternfly/react-core';
import {
FormGroup,
HelperText,
InputGroup,
InputGroupItem,
InputGroupText,
TextInput,
ValidatedOptions,
} from '@patternfly/react-core';
import ResourceNameDefinitionTooltip from '~/concepts/k8s/ResourceNameDefinitionTooltip';
import {
HelperTextItemMaxLength,
Expand Down Expand Up @@ -45,17 +53,34 @@ const ResourceNameField: React.FC<ResourceNameFieldProps> = ({
validated = ValidatedOptions.success;
}

const usePrefix = k8sName.state.staticPrefix && !!k8sName.state.safePrefix;
const textInput = (
<TextInput
id={`${dataTestId}-resourceName`}
data-testid={`${dataTestId}-resourceName`}
name={`${dataTestId}-resourceName`}
isRequired
value={
usePrefix
? k8sName.value.replace(new RegExp(`^${k8sName.state.safePrefix}`), '')
: k8sName.value
}
onChange={(event, value) =>
onDataChange?.('k8sName', usePrefix ? `${k8sName.state.safePrefix}${value}` : value)
}
validated={validated}
/>
);
return (
<FormGroup {...formGroupProps} isRequired>
<TextInput
id={`${dataTestId}-resourceName`}
data-testid={`${dataTestId}-resourceName`}
name={`${dataTestId}-resourceName`}
isRequired
value={k8sName.value}
onChange={(event, value) => onDataChange?.('k8sName', value)}
validated={validated}
/>
{usePrefix ? (
<InputGroup>
<InputGroupText>{k8sName.state.safePrefix}</InputGroupText>
<InputGroupItem isFill>{textInput}</InputGroupItem>
</InputGroup>
) : (
textInput
)}
<HelperText>
<HelperTextItemMaxLength k8sName={k8sName} />
<HelperTextItemValidCharacters k8sName={k8sName} />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import * as React from 'react';
import '@testing-library/jest-dom';
import { render, screen } from '@testing-library/react';
import ResourceNameField from '~/concepts/k8s/K8sNameDescriptionField/ResourceNameField';

describe('ResourceNameField', () => {
it('should render immutable name', () => {
render(
<ResourceNameField
allowEdit={false}
dataTestId="test"
k8sName={{
value: 'test',
state: {
immutable: true,
invalidCharacters: false,
invalidLength: false,
maxLength: 0,
touched: false,
},
}}
/>,
);

expect(screen.queryByTestId('test-resourceName')).not.toBeInTheDocument();
});

it('should render nothing if not editable and not immutable', () => {
const result = render(
<ResourceNameField
allowEdit={false}
dataTestId="test"
k8sName={{
value: 'test',
state: {
immutable: false,
invalidCharacters: false,
invalidLength: false,
maxLength: 0,
touched: false,
},
}}
/>,
);

expect(result.container).toBeEmptyDOMElement();
});

it('should render editable name', () => {
render(
<ResourceNameField
allowEdit
dataTestId="test"
k8sName={{
value: 'test',
state: {
immutable: false,
invalidCharacters: false,
invalidLength: false,
maxLength: 0,
touched: false,
},
}}
/>,
);

expect(screen.queryByTestId('test-resourceName')).toBeInTheDocument();
});

it('should render editable name with static prefix', () => {
render(
<ResourceNameField
allowEdit
dataTestId="test"
k8sName={{
value: 'test',
state: {
immutable: false,
invalidCharacters: false,
invalidLength: false,
maxLength: 0,
touched: false,
safePrefix: 'wb-',
staticPrefix: true,
},
}}
/>,
);

expect(screen.getByText('wb-')).toBeInTheDocument();
expect(screen.queryByTestId('test-resourceName')).toHaveValue('test');
});
});
10 changes: 10 additions & 0 deletions frontend/src/concepts/k8s/K8sNameDescriptionField/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ export type K8sNameDescriptionFieldData = {
* @see AdditionalCriteriaForTranslation
*/
safePrefix?: string;
/**
* If the safe prefix is to be statically applied
* @see AdditionalCriteriaForTranslation
*/
staticPrefix?: boolean;
/** Max length for the K8s name */
maxLength: number;
/** The user is now in control of the value; do not auto generate */
Expand All @@ -42,6 +47,11 @@ export type UseK8sNameDescriptionDataConfiguration = {
* @see AdditionalCriteriaForTranslation
*/
safePrefix?: string;
/**
* If the safe prefix is to be statically applied
* @see AdditionalCriteriaForTranslation
*/
staticPrefix?: boolean;
};

type K8sNameDescriptionFieldUpdateFunctionTemplate<T> = (
Expand Down
6 changes: 5 additions & 1 deletion frontend/src/concepts/k8s/K8sNameDescriptionField/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const setupDefaults = ({
initialData,
limitNameResourceType,
safePrefix,
staticPrefix,
}: UseK8sNameDescriptionDataConfiguration): K8sNameDescriptionFieldData => {
let initialName = '';
let initialDescription = '';
Expand Down Expand Up @@ -74,6 +75,7 @@ export const setupDefaults = ({
invalidLength: false,
maxLength: configuredMaxLength,
safePrefix,
staticPrefix,
touched: false,
},
},
Expand All @@ -90,13 +92,15 @@ export const handleUpdateLogic =
case 'name': {
changedData.name = value;

const { touched, immutable, maxLength, safePrefix } = existingData.k8sName.state;
const { touched, immutable, maxLength, safePrefix, staticPrefix } =
existingData.k8sName.state;
// When name changes, we want to update resource name if applicable
if (!touched && !immutable) {
// Update the generated name
const k8sValue = translateDisplayNameForK8s(value, {
maxLength,
safeK8sPrefix: safePrefix,
staticPrefix,
});
changedData.k8sName = {
value: k8sValue,
Expand Down
36 changes: 25 additions & 11 deletions frontend/src/concepts/k8s/__tests__/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,24 +59,38 @@ describe('translateDisplayNameForK8sAndReport', () => {
it('should NOT generate name if provided nothing', () => {
expect(translateDisplayNameForK8sAndReport('')).toEqual([
'',
{ autoGenerated: false, safeK8sPrefix: false, maxLength: false },
{ autoGenerated: false, safeK8sPrefix: false, maxLength: false, staticPrefix: false },
] satisfies ReturnArrayType);

expect(translateDisplayNameForK8sAndReport(' ')).toEqual([
'',
{ autoGenerated: false, safeK8sPrefix: false, maxLength: false },
{ autoGenerated: false, safeK8sPrefix: false, maxLength: false, staticPrefix: false },
] satisfies ReturnArrayType);

expect(
translateDisplayNameForK8sAndReport('', { safeK8sPrefix: 'wb-', staticPrefix: true }),
).toEqual([
'',
{ autoGenerated: false, safeK8sPrefix: false, maxLength: false, staticPrefix: false },
] satisfies ReturnArrayType);
});

it('should handle cases where it applied additional criteria', () => {
expect(translateDisplayNameForK8sAndReport('1234', { safeK8sPrefix: 'wb-' })).toEqual([
'wb-1234',
{ autoGenerated: false, safeK8sPrefix: true, maxLength: false },
{ autoGenerated: false, safeK8sPrefix: true, maxLength: false, staticPrefix: false },
] satisfies ReturnArrayType);

expect(translateDisplayNameForK8sAndReport('foobarbaz', { maxLength: 3 })).toEqual([
'foo',
{ autoGenerated: false, safeK8sPrefix: false, maxLength: true },
{ autoGenerated: false, safeK8sPrefix: false, maxLength: true, staticPrefix: false },
] satisfies ReturnArrayType);

expect(
translateDisplayNameForK8sAndReport('test', { safeK8sPrefix: 'wb-', staticPrefix: true }),
).toEqual([
'wb-test',
{ autoGenerated: false, safeK8sPrefix: true, maxLength: false, staticPrefix: true },
] satisfies ReturnArrayType);
});

Expand All @@ -85,21 +99,21 @@ describe('translateDisplayNameForK8sAndReport', () => {
translateDisplayNameForK8sAndReport('111allnumbers', { maxLength: 3, safeK8sPrefix: 'f-' }),
).toEqual([
'f-1',
{ autoGenerated: false, safeK8sPrefix: true, maxLength: true },
{ autoGenerated: false, safeK8sPrefix: true, maxLength: true, staticPrefix: false },
] satisfies ReturnArrayType);

expect(
translateDisplayNameForK8sAndReport('111allnumbers', { maxLength: 3, safeK8sPrefix: 'foo-' }),
).toEqual([
'foo', // known exception to providing a too long safeK8sPrefix
{ autoGenerated: false, safeK8sPrefix: true, maxLength: true },
{ autoGenerated: false, safeK8sPrefix: true, maxLength: true, staticPrefix: false },
] satisfies ReturnArrayType);
});

it('should report nothing happened if no criteria was provided', () => {
expect(translateDisplayNameForK8sAndReport('foobarbaz')).toEqual([
'foobarbaz',
{ autoGenerated: false, safeK8sPrefix: false, maxLength: false },
{ autoGenerated: false, safeK8sPrefix: false, maxLength: false, staticPrefix: false },
] satisfies ReturnArrayType);
});

Expand All @@ -109,24 +123,24 @@ describe('translateDisplayNameForK8sAndReport', () => {

expect(translateDisplayNameForK8sAndReport('-')).toEqual([
`gen-${GENERATED_STRING}`,
{ autoGenerated: true, safeK8sPrefix: false, maxLength: false },
{ autoGenerated: true, safeK8sPrefix: false, maxLength: false, staticPrefix: false },
] satisfies ReturnArrayType);

expect(translateDisplayNameForK8sAndReport('$%*($%()*')).toEqual([
`gen-${GENERATED_STRING}`,
{ autoGenerated: true, safeK8sPrefix: false, maxLength: false },
{ autoGenerated: true, safeK8sPrefix: false, maxLength: false, staticPrefix: false },
] satisfies ReturnArrayType);

expect(translateDisplayNameForK8sAndReport('*(&(*&(*', { maxLength: 3 })).toEqual([
`gen`, // known exception to providing a too short maxLength
{ autoGenerated: true, safeK8sPrefix: false, maxLength: true },
{ autoGenerated: true, safeK8sPrefix: false, maxLength: true, staticPrefix: false },
] satisfies ReturnArrayType);

expect(
translateDisplayNameForK8sAndReport('*(&(*&(*', { maxLength: 3, safeK8sPrefix: 'foo-' }),
).toEqual([
`foo`, // known exception to providing a too long safeK8sPrefix
{ autoGenerated: true, safeK8sPrefix: true, maxLength: true },
{ autoGenerated: true, safeK8sPrefix: true, maxLength: true, staticPrefix: false },
] satisfies ReturnArrayType);
});
});
Expand Down
21 changes: 16 additions & 5 deletions frontend/src/concepts/k8s/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ type AdditionalCriteriaForTranslation = {
* Note: Do not exceed maxLength otherwise it will replace content with it
*/
safeK8sPrefix?: string;
/** If the safe prefix is to be statically applied */
staticPrefix?: boolean;
/**
* Cap the characters allowed.
* Note: This value can be problematic at very short sizes (< 3)
Expand All @@ -44,12 +46,13 @@ type AdditionalCriteriaApplied = Record<
*/
export const translateDisplayNameForK8sAndReport = (
name: string,
{ safeK8sPrefix, maxLength }: AdditionalCriteriaForTranslation = {},
{ safeK8sPrefix, staticPrefix, maxLength }: AdditionalCriteriaForTranslation = {},
): [string, AdditionalCriteriaApplied] => {
const appliedCriteria: AdditionalCriteriaApplied = {
autoGenerated: false,
safeK8sPrefix: false,
maxLength: false,
staticPrefix: false,
};

let translatedName = name
Expand All @@ -75,10 +78,18 @@ export const translateDisplayNameForK8sAndReport = (

keepLength();

if (safeK8sPrefix && /^\d+$/.test(translatedName)) {
// Avoid pure digit names
translatedName = `${safeK8sPrefix}${translatedName}`;
appliedCriteria.safeK8sPrefix = true;
if (safeK8sPrefix) {
if (staticPrefix && translatedName) {
if (!translatedName.startsWith(safeK8sPrefix)) {
translatedName = `${safeK8sPrefix}${translatedName}`;
}
appliedCriteria.safeK8sPrefix = true;
appliedCriteria.staticPrefix = true;
} else if (/^\d+$/.test(translatedName)) {
// Avoid pure digit names
translatedName = `${safeK8sPrefix}${translatedName}`;
appliedCriteria.safeK8sPrefix = true;
}
}

keepLength();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,11 @@ const ManageConnectionTypePage: React.FC<Props> = ({ prefill, isEdit, onSave })
const { category, enabled, fields, username } = data;

const { data: connectionNameDesc, onDataChange: setConnectionNameDesc } =
useK8sNameDescriptionFieldData({ initialData: prefill });
useK8sNameDescriptionFieldData({
initialData: prefill,
safePrefix: 'ct-',
staticPrefix: true,
});

const isDataDirty = React.useRef(data).current !== data;
const isNameDirty = React.useRef(connectionNameDesc).current !== connectionNameDesc;
Expand Down

0 comments on commit 0e4420f

Please sign in to comment.