Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 11 additions & 3 deletions webapp/src/dogma/features/xds/K8sAggregatorEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ import ErrorMessageParser from 'dogma/features/services/ErrorMessageParser';
import { K8sAggregatorStatus } from 'dogma/features/xds/K8sAggregatorStatus';

// Matches the server-side resource id pattern (XdsResourceManager.RESOURCE_ID_PATTERN_STRING).
const AGGREGATOR_ID_PATTERN = /^[a-z](?:[a-z0-9-_/]*[a-z0-9])?$/;
// Dots are allowed (e.g. "my-service.v1"), but slashes are not.
const AGGREGATOR_ID_PATTERN = /^[a-z](?:[a-z0-9_.-]*[a-z0-9])?$/;

interface PropertyForm {
key: string;
Expand Down Expand Up @@ -450,9 +451,16 @@ const AggregatorFormFields = ({
<Input
placeholder="e.g. my-service"
isReadOnly={idReadOnly || readOnly}
{...register('aggregatorId', { required: true, pattern: AGGREGATOR_ID_PATTERN })}
{...register('aggregatorId', {
required: true,
// Skip the pattern check for existing resources: the ID is immutable and may contain
// slashes that were allowed before this validation was introduced.
pattern: idReadOnly ? undefined : AGGREGATOR_ID_PATTERN,
})}
/>
<FormErrorMessage>ID must match [a-z](?:[a-z0-9-_/]*[a-z0-9])?</FormErrorMessage>
<FormErrorMessage>
ID must match [a-z](?:[a-z0-9_.-]*[a-z0-9])? (dots allowed, slashes not allowed)
</FormErrorMessage>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
</FormControl>

{fields.map((field, index) => (
Expand Down
6 changes: 4 additions & 2 deletions webapp/src/dogma/features/xds/NewGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ type FormData = {
};

// A group id is also a repository name, so it follows the same naming rule.
const GROUP_ID_PATTERN = /^[a-z](?:[a-z0-9-_]*[a-z0-9])?$/;
// Dots are allowed (e.g. "my.group"), but slashes are not.
const GROUP_ID_PATTERN = /^[a-z](?:[a-z0-9_.-]*[a-z0-9])?$/;

export const NewGroup = () => {
const {
Expand Down Expand Up @@ -88,7 +89,8 @@ export const NewGroup = () => {
/>
{errors.groupId && (
<FormErrorMessage>
Group ID must match the pattern [a-z](?:[a-z0-9-_]*[a-z0-9])?
Group ID must match the pattern [a-z](?:[a-z0-9_.-]*[a-z0-9])? (lowercase letters, digits,
hyphens, underscores, and dots are allowed)
</FormErrorMessage>
)}
</FormControl>
Expand Down
21 changes: 20 additions & 1 deletion webapp/src/dogma/features/xds/ResourceEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
Button,
Flex,
FormControl,
FormErrorMessage,
FormLabel,
Heading,
HStack,
Expand Down Expand Up @@ -63,6 +64,9 @@ import { useAppDispatch } from 'dogma/hooks';
import { newNotification } from 'dogma/features/notification/notificationSlice';
import ErrorMessageParser from 'dogma/features/services/ErrorMessageParser';

// Dots are allowed (e.g. "my-service.v1"), but slashes are not.
const RESOURCE_ID_PATTERN = /^[a-z](?:[a-z0-9_.-]*[a-z0-9])?$/;

function parseJsonOrNotify(dispatch: ReturnType<typeof useAppDispatch>, value: string): object | null {
try {
return JSON.parse(value);
Expand All @@ -81,6 +85,8 @@ const NewResourceEditor = ({ group, type }: { group: string; type: XdsResourceTy
const [content, setContent] = useState(XDS_RESOURCE_TEMPLATES[type]);
const [createResource, { isLoading }] = useCreateResourceMutation();

const idIsInvalid = id.length > 0 && !RESOURCE_ID_PATTERN.test(id);

const handleCreate = async () => {
if (!hasWrite) {
return;
Expand All @@ -89,6 +95,16 @@ const NewResourceEditor = ({ group, type }: { group: string; type: XdsResourceTy
dispatch(newNotification('ID is required', `Please enter the ${meta.label} ID`, 'error'));
return;
}
if (idIsInvalid) {
dispatch(
newNotification(
'Invalid ID',
`${meta.label} ID must match [a-z](?:[a-z0-9_.-]*[a-z0-9])? (dots allowed, slashes not allowed)`,
'error',
),
);
return;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if (parseJsonOrNotify(dispatch, content) === null) {
return;
}
Expand All @@ -115,9 +131,12 @@ const NewResourceEditor = ({ group, type }: { group: string; type: XdsResourceTy

return (
<Box>
<FormControl isRequired mb={4} maxW="md">
<FormControl isRequired isInvalid={idIsInvalid} mb={4} maxW="md">
<FormLabel>{meta.label} ID</FormLabel>
<Input value={id} onChange={(e) => setId(e.target.value)} placeholder={`Enter ${meta.label} ID ...`} />
<FormErrorMessage>
ID must match [a-z](?:[a-z0-9_.-]*[a-z0-9])? (dots allowed, slashes not allowed)
</FormErrorMessage>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
</FormControl>
<JsonEditor value={content} onChange={setContent} />
<Flex mt={4}>
Expand Down
163 changes: 163 additions & 0 deletions webapp/tests/dogma/features/xds/K8sAggregatorEditor.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
/*
* Copyright 2026 LY Corporation
*
* LY Corporation licenses this file to you 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:
*
* https://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.
*/
import { screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { renderWithProviders } from 'dogma/util/test-utils';
import { K8sAggregatorEditor } from 'dogma/features/xds/K8sAggregatorEditor';
import * as xdsApiSlice from 'dogma/features/xds/xdsApiSlice';

jest.mock('next/router', () => ({
__esModule: true,
default: { push: jest.fn() },
}));

jest.mock('dogma/features/xds/useGroupWriteAccess', () => ({
useGroupWriteAccess: () => ({ hasWrite: true, isLoading: false }),
}));

// Stub out the status panel and preview modal — they make additional API calls unrelated to ID validation.
jest.mock('dogma/features/xds/K8sAggregatorStatus', () => ({
K8sAggregatorStatus: () => null,
}));
jest.mock('dogma/features/xds/K8sAggregatorPreviewModal', () => ({
K8sAggregatorPreviewModal: () => null,
}));

// chakra-react-select does not work in JSDOM; replace with a plain <select>.
jest.mock('chakra-react-select', () => ({
Select: ({ name, options, onChange, value, placeholder }: any) => (
<select
name={name}
value={value?.value || ''}
onChange={(e) => {
const selected = options?.find((o: any) => o.value === e.target.value);
onChange(selected ?? null);
}}
>
<option value="">{placeholder}</option>
{options?.map((o: any) => (
<option key={o.value} value={o.value}>
{o.label}
</option>
))}
</select>
),
}));

jest.mock('dogma/features/xds/xdsApiSlice', () => ({
// Preserve reducerPath and reducer so the Redux store initialises correctly.
...jest.requireActual('dogma/features/xds/xdsApiSlice'),
useCreateK8sAggregatorMutation: jest.fn(),
useUpdateK8sAggregatorMutation: jest.fn(),
useDeleteK8sAggregatorMutation: jest.fn(),
usePreviewK8sAggregatorMutation: jest.fn(),
useGetK8sAggregatorQuery: jest.fn(),
useListCredentialsQuery: jest.fn(),
}));

// Minimal aggregator body with one fully-populated watcher, satisfying all required watcher fields.
const VALID_WATCHER_CONTENT = {
localityLbEndpoints: [
{
watcher: {
serviceName: 'my-service',
kubeconfig: { controlPlaneUrl: 'https://kubernetes.default.svc' },
},
},
],
};

describe('K8sAggregatorEditor – aggregator ID pattern validation', () => {
let mockCreate: jest.Mock;
let mockUpdate: jest.Mock;

beforeEach(() => {
mockCreate = jest.fn().mockReturnValue({ unwrap: () => Promise.resolve({}) });
mockUpdate = jest.fn().mockReturnValue({ unwrap: () => Promise.resolve({}) });

jest
.mocked(xdsApiSlice.useCreateK8sAggregatorMutation)
.mockReturnValue([mockCreate, { isLoading: false }] as any);
jest
.mocked(xdsApiSlice.useUpdateK8sAggregatorMutation)
.mockReturnValue([mockUpdate, { isLoading: false }] as any);
jest
.mocked(xdsApiSlice.useDeleteK8sAggregatorMutation)
.mockReturnValue([jest.fn(), { isLoading: false }] as any);
jest
.mocked(xdsApiSlice.usePreviewK8sAggregatorMutation)
.mockReturnValue([jest.fn(), { isLoading: false }] as any);
jest.mocked(xdsApiSlice.useGetK8sAggregatorQuery).mockReturnValue({
data: { content: VALID_WATCHER_CONTENT },
isLoading: false,
error: undefined,
} as any);
jest.mocked(xdsApiSlice.useListCredentialsQuery).mockReturnValue({ data: [], error: null } as any);
});

describe('new aggregator', () => {
it('rejects a slash ID and shows a validation error', async () => {
const user = userEvent.setup();
renderWithProviders(<K8sAggregatorEditor group="foo" isNew />);

await user.type(screen.getByPlaceholderText('e.g. my-service'), 'foo/bar');
await user.click(screen.getByRole('button', { name: /^create$/i }));

await waitFor(() => {
expect(screen.getByText(/dots allowed, slashes not allowed/i)).toBeInTheDocument();
});
expect(mockCreate).not.toHaveBeenCalled();
});

it('accepts a dot ID and calls createAggregator', async () => {
const user = userEvent.setup();
renderWithProviders(<K8sAggregatorEditor group="foo" isNew />);

await user.type(screen.getByPlaceholderText('e.g. my-service'), 'foo.bar');
// Fill the required watcher fields so form submission proceeds past required validation.
await user.type(screen.getByPlaceholderText('k8s service name'), 'my-service');
await user.type(screen.getByPlaceholderText('https://kubernetes.default.svc'), 'https://k8s.default.svc');

await user.click(screen.getByRole('button', { name: /^create$/i }));

await waitFor(() => {
expect(mockCreate).toHaveBeenCalled();
});
expect(screen.queryByText(/dots allowed, slashes not allowed/i)).not.toBeInTheDocument();
});
});

describe('existing aggregator with a legacy slash ID', () => {
it('saves without showing a pattern error (backward compat)', async () => {
const user = userEvent.setup();
renderWithProviders(<K8sAggregatorEditor group="foo" id="foo/bar" isNew={false} />);

// Wait for the form to be populated from the fetched data.
await waitFor(() => {
expect(screen.getByDisplayValue('foo/bar')).toBeInTheDocument();
});

await user.click(screen.getByRole('button', { name: /^edit$/i }));
await user.click(screen.getByRole('button', { name: /^save$/i }));

// The update should proceed — the slash ID must not be blocked by the pattern.
await waitFor(() => {
expect(mockUpdate).toHaveBeenCalled();
});
expect(screen.queryByText(/dots allowed, slashes not allowed/i)).not.toBeInTheDocument();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

import static com.linecorp.centraldogma.server.internal.admin.auth.AuthUtil.currentAuthor;
import static com.linecorp.centraldogma.xds.internal.ControlPlaneService.CLUSTERS_DIRECTORY;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.LEGACY_RESOURCE_ID_PATTERN_STRING;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.RESOURCE_ID_PATTERN;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.RESOURCE_ID_PATTERN_STRING;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.removePrefix;

import java.util.regex.Matcher;
Expand All @@ -39,7 +39,7 @@
public final class XdsClusterService extends XdsClusterServiceImplBase {

private static final Pattern CLUSTER_NAME_PATTERN =
Pattern.compile("^groups/([^/]+)/clusters/" + RESOURCE_ID_PATTERN_STRING + '$');
Pattern.compile("^groups/([^/]+)/clusters/" + LEGACY_RESOURCE_ID_PATTERN_STRING + '$');

private final XdsResourceManager xdsResourceManager;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
import static com.linecorp.centraldogma.server.internal.admin.auth.AuthUtil.currentAuthor;
import static com.linecorp.centraldogma.xds.internal.ControlPlaneService.CLUSTERS_DIRECTORY;
import static com.linecorp.centraldogma.xds.internal.ControlPlaneService.ENDPOINTS_DIRECTORY;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.LEGACY_RESOURCE_ID_PATTERN_STRING;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.RESOURCE_ID_PATTERN;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.RESOURCE_ID_PATTERN_STRING;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.removePrefix;

import java.util.concurrent.ScheduledExecutorService;
Expand All @@ -41,7 +41,7 @@
public final class XdsEndpointService extends XdsEndpointServiceImplBase {

private static final Pattern ENDPONT_NAME_PATTERN =
Pattern.compile("^groups/([^/]+)/endpoints/(" + RESOURCE_ID_PATTERN_STRING + ")$");
Pattern.compile("^groups/([^/]+)/endpoints/(" + LEGACY_RESOURCE_ID_PATTERN_STRING + ")$");

private final XdsResourceManager xdsResourceManager;
private final XdsEndpointUpdateScheduler xdsEndpointUpdateScheduler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,14 @@

public final class XdsResourceManager {

public static final String RESOURCE_ID_PATTERN_STRING = "[a-z](?:[a-z0-9-_/]*[a-z0-9])?";
public static final String RESOURCE_ID_PATTERN_STRING = "[a-z](?:[a-z0-9_.-]*[a-z0-9])?";
public static final Pattern RESOURCE_ID_PATTERN = Pattern.compile('^' + RESOURCE_ID_PATTERN_STRING + '$');
// Allows slashes in addition to dots for backward compatibility with resources created before the
// slash was forbidden. Use this pattern only for parsing existing resource names in update/delete
// operations, not for validating new IDs in create operations.
public static final String LEGACY_RESOURCE_ID_PATTERN_STRING = "[a-z](?:[a-z0-9_/.-]*[a-z0-9])?";
public static final Pattern LEGACY_RESOURCE_ID_PATTERN =
Pattern.compile('^' + LEGACY_RESOURCE_ID_PATTERN_STRING + '$');

public static final MessageMarshaller JSON_MESSAGE_MARSHALLER;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import static com.linecorp.centraldogma.internal.CredentialUtil.credentialName;
import static com.linecorp.centraldogma.server.internal.admin.auth.AuthUtil.currentAuthor;
import static com.linecorp.centraldogma.server.internal.storage.InternalProjectConstants.INTERNAL_PROJECT_XDS;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.LEGACY_RESOURCE_ID_PATTERN_STRING;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.RESOURCE_ID_PATTERN;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.RESOURCE_ID_PATTERN_STRING;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.fileName;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.removePrefix;

Expand Down Expand Up @@ -95,7 +95,8 @@ public final class XdsKubernetesService extends XdsKubernetesServiceImplBase {
Pattern.compile("(?<=/k8s)/endpointAggregators/");

public static final Pattern K8S_ENDPOINT_AGGREGATORS_NAME_PATTERN = Pattern.compile(
"^groups/([^/]+)" + K8S_ENDPOINT_AGGREGATORS_DIRECTORY + '(' + RESOURCE_ID_PATTERN_STRING + ")$");
"^groups/([^/]+)" + K8S_ENDPOINT_AGGREGATORS_DIRECTORY +
'(' + LEGACY_RESOURCE_ID_PATTERN_STRING + ")$");

public static final CompletableFuture<?>[] EMPTY_FUTURES = new CompletableFuture[0];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

import static com.linecorp.centraldogma.server.internal.admin.auth.AuthUtil.currentAuthor;
import static com.linecorp.centraldogma.xds.internal.ControlPlaneService.LISTENERS_DIRECTORY;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.LEGACY_RESOURCE_ID_PATTERN_STRING;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.RESOURCE_ID_PATTERN;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.RESOURCE_ID_PATTERN_STRING;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.removePrefix;

import java.util.regex.Matcher;
Expand All @@ -39,7 +39,7 @@
public final class XdsListenerService extends XdsListenerServiceImplBase {

private static final Pattern LISTENER_NAME_PATTERN =
Pattern.compile("^groups/([^/]+)/listeners/" + RESOURCE_ID_PATTERN_STRING + '$');
Pattern.compile("^groups/([^/]+)/listeners/" + LEGACY_RESOURCE_ID_PATTERN_STRING + '$');

private final XdsResourceManager xdsResourceManager;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

import static com.linecorp.centraldogma.server.internal.admin.auth.AuthUtil.currentAuthor;
import static com.linecorp.centraldogma.xds.internal.ControlPlaneService.ROUTES_DIRECTORY;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.LEGACY_RESOURCE_ID_PATTERN_STRING;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.RESOURCE_ID_PATTERN;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.RESOURCE_ID_PATTERN_STRING;
import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.removePrefix;

import java.util.regex.Matcher;
Expand All @@ -39,7 +39,7 @@
public final class XdsRouteService extends XdsRouteServiceImplBase {

private static final Pattern ROUTE_NAME_PATTERN =
Pattern.compile("^groups/([^/]+)/routes/" + RESOURCE_ID_PATTERN_STRING + '$');
Pattern.compile("^groups/([^/]+)/routes/" + LEGACY_RESOURCE_ID_PATTERN_STRING + '$');

private final XdsResourceManager xdsResourceManager;

Expand Down
Loading
Loading