Skip to content

Validate that label keys are valid k8s labels and ensure correct key splitting #3777

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.

Sign up for GitHub

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

Merged
dvail merged 1 commit into from
Nov 14, 2022
Merged

Validate that label keys are valid k8s labels and ensure correct key splitting #3777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ui/apps/platform/src/Containers/Collections/CollectionForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import * as yup from 'yup';

import { collectionsBasePath } from 'routePaths';
import { CollectionResponse } from 'services/CollectionsService';
import { getIsValidLabelKey } from 'utils/labels';
import { CollectionPageAction } from './collections.utils';
import RuleSelector from './RuleSelector';
import CollectionAttacher from './CollectionAttacher';
Expand Down Expand Up @@ -81,7 +82,7 @@ function yupResourceSelectorObject() {
rules: yup.array().of(
yup.object().shape({
operator: yup.string().required().matches(/OR/),
key: yup.string().trim().required(),
key: yup.string().trim().required().test(getIsValidLabelKey),
values: yup.array().of(yup.string().trim().required()).required(),
})
),
Expand Down
55 changes: 54 additions & 1 deletion ui/apps/platform/src/Containers/Collections/converter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { CollectionRequest, CollectionResponse } from 'services/CollectionsService';
import { generateRequest, parseCollection } from './converter';
import { Collection } from './types';
import { ByLabelResourceSelector, Collection, LabelSelectorRule } from './types';

describe('Collection parser', () => {
it('should convert between BE CollectionResponse and FE Collection', () => {
Expand Down Expand Up @@ -163,6 +163,59 @@ describe('Collection parser', () => {

expect(parseCollection(collectionResponse)).toBeInstanceOf(AggregateError);
});

it('should correctly handle label key/value splitting on `=` delimiter', () => {
const collectionResponse: CollectionResponse = {
id: 'a-b-c',
name: 'Sample',
description: 'Sample description',
inUse: false,
resourceSelectors: [
{
rules: [
{ operator: 'OR', fieldName: 'Cluster Label', values: [{ value: '' }] },
],
},
],
embeddedCollections: [],
};

// Get the resource selector we are interested in without so many type assertions
function getLabelRule(collection: CollectionResponse): LabelSelectorRule {
return (
(parseCollection(collection) as Collection).resourceSelector
.Cluster as ByLabelResourceSelector
).rules[0];
}

const firstLabelRule = collectionResponse.resourceSelectors[0].rules[0].values[0];

// Test empty label key handling (NOTE, this should be forbidden from occurring by BE)
firstLabelRule.value = '=test';
expect(getLabelRule(collectionResponse)).toMatchObject({ key: '', values: ['test'] });

// Test empty label value handling (NOTE, this should be forbidden from occurring by BE)
firstLabelRule.value = 'test=';
expect(getLabelRule(collectionResponse)).toMatchObject({ key: 'test', values: [''] });

// Test plain characters
firstLabelRule.value = 'key=value';
expect(getLabelRule(collectionResponse)).toMatchObject({ key: 'key', values: ['value'] });

// Test subdomain prefix
firstLabelRule.value = 'app.kubernetes.io/name=value';
expect(getLabelRule(collectionResponse)).toMatchObject({
key: 'app.kubernetes.io/name',
values: ['value'],
});

// Test multiple '=' characters
firstLabelRule.value = 'app.kubernetes.io/name=value=with=extra=eq';
expect(getLabelRule(collectionResponse)).toMatchObject({
key: 'app.kubernetes.io/name',
values: ['value=with=extra=eq'],
});
});
});

describe('Collection response generator', () => {
Expand Down
11 changes: 7 additions & 4 deletions ui/apps/platform/src/Containers/Collections/converter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,16 @@ export function parseCollection(data: Omit<CollectionResponse, 'id'>): Collectio
const firstValue = rule.values[0]?.value;

if (firstValue && firstValue.includes(LABEL_SEPARATOR)) {
const key = firstValue.split(LABEL_SEPARATOR)[0] ?? '';
const [key] = firstValue.split(LABEL_SEPARATOR);
selector.rules.push({
operator: 'OR',
key,
// TODO Verify with BE whether or not this is a valid method to get the label values. Is
// it possible that multiple `=` symbols will appear in the data here?
values: rule.values.map(({ value }) => value.split('=')[1] ?? ''),
values: rule.values.map(({ value }) => {
// Since the label key does not support RE2 Regex, and must be a valid k8s
// label, anything after the first '=' character is the label rule value.
const [, ...valuesPart] = value.split(LABEL_SEPARATOR);
return valuesPart.join(LABEL_SEPARATOR);
}),
});
}
break;
Expand Down