diff --git a/webapp/src/dogma/features/xds/K8sAggregatorEditor.tsx b/webapp/src/dogma/features/xds/K8sAggregatorEditor.tsx index 3327a9c4d..e995cab5a 100644 --- a/webapp/src/dogma/features/xds/K8sAggregatorEditor.tsx +++ b/webapp/src/dogma/features/xds/K8sAggregatorEditor.tsx @@ -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; @@ -450,9 +451,16 @@ const AggregatorFormFields = ({ - ID must match [a-z](?:[a-z0-9-_/]*[a-z0-9])? + + ID must match [a-z](?:[a-z0-9_.-]*[a-z0-9])? (dots allowed, slashes not allowed) + {fields.map((field, index) => ( diff --git a/webapp/src/dogma/features/xds/NewGroup.tsx b/webapp/src/dogma/features/xds/NewGroup.tsx index 1ef0fb91c..376d6429f 100644 --- a/webapp/src/dogma/features/xds/NewGroup.tsx +++ b/webapp/src/dogma/features/xds/NewGroup.tsx @@ -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 { @@ -88,7 +89,8 @@ export const NewGroup = () => { /> {errors.groupId && ( - 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) )} diff --git a/webapp/src/dogma/features/xds/ResourceEditor.tsx b/webapp/src/dogma/features/xds/ResourceEditor.tsx index 4ee0e0611..f205c27c0 100644 --- a/webapp/src/dogma/features/xds/ResourceEditor.tsx +++ b/webapp/src/dogma/features/xds/ResourceEditor.tsx @@ -23,6 +23,7 @@ import { Button, Flex, FormControl, + FormErrorMessage, FormLabel, Heading, HStack, @@ -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, value: string): object | null { try { return JSON.parse(value); @@ -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; @@ -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; + } if (parseJsonOrNotify(dispatch, content) === null) { return; } @@ -115,9 +131,12 @@ const NewResourceEditor = ({ group, type }: { group: string; type: XdsResourceTy return ( - + {meta.label} ID setId(e.target.value)} placeholder={`Enter ${meta.label} ID ...`} /> + + ID must match [a-z](?:[a-z0-9_.-]*[a-z0-9])? (dots allowed, slashes not allowed) + diff --git a/webapp/tests/dogma/features/xds/K8sAggregatorEditor.test.tsx b/webapp/tests/dogma/features/xds/K8sAggregatorEditor.test.tsx new file mode 100644 index 000000000..699c241df --- /dev/null +++ b/webapp/tests/dogma/features/xds/K8sAggregatorEditor.test.tsx @@ -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 { + const selected = options?.find((o: any) => o.value === e.target.value); + onChange(selected ?? null); + }} + > + + {options?.map((o: any) => ( + + ))} + + ), +})); + +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(); + + 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(); + + 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(); + + // 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(); + }); + }); +}); diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/cluster/v1/XdsClusterService.java b/xds/src/main/java/com/linecorp/centraldogma/xds/cluster/v1/XdsClusterService.java index 636747325..1e7a29d4f 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/cluster/v1/XdsClusterService.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/cluster/v1/XdsClusterService.java @@ -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; @@ -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; diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointService.java b/xds/src/main/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointService.java index c1ce7defe..b67542a1c 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointService.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointService.java @@ -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; @@ -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; diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/XdsResourceManager.java b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/XdsResourceManager.java index d52defe6f..2c93eeb0d 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/XdsResourceManager.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/XdsResourceManager.java @@ -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; diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesService.java b/xds/src/main/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesService.java index 204c84669..ec385e28d 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesService.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesService.java @@ -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; @@ -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]; diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/listener/v1/XdsListenerService.java b/xds/src/main/java/com/linecorp/centraldogma/xds/listener/v1/XdsListenerService.java index 9401028d0..0a572d75e 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/listener/v1/XdsListenerService.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/listener/v1/XdsListenerService.java @@ -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; @@ -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; diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/route/v1/XdsRouteService.java b/xds/src/main/java/com/linecorp/centraldogma/xds/route/v1/XdsRouteService.java index 00e0cd053..b9b0b933e 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/route/v1/XdsRouteService.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/route/v1/XdsRouteService.java @@ -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; @@ -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; diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/cluster/v1/XdsClusterServiceTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/cluster/v1/XdsClusterServiceTest.java index a585ab993..a3f078fcd 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/cluster/v1/XdsClusterServiceTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/cluster/v1/XdsClusterServiceTest.java @@ -72,15 +72,19 @@ void createClusterViaHttp() throws Exception { dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); - response = createCluster("groups/non-existent-group", "foo-cluster/1", cluster, dogma.httpClient()); + // Slashes are no longer allowed in new resource IDs. + response = createCluster("groups/foo", "foo-cluster/invalid", cluster, dogma.httpClient()); + assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); + + response = createCluster("groups/non-existent-group", "foo-cluster.1", cluster, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); - response = createCluster("groups/foo", "foo-cluster/1", cluster, dogma.httpClient()); + response = createCluster("groups/foo", "foo-cluster.1", cluster, dogma.httpClient()); assertOk(response); final Cluster.Builder clusterBuilder = Cluster.newBuilder(); JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), clusterBuilder); final Cluster actualCluster = clusterBuilder.build(); - final String clusterName = "groups/foo/clusters/foo-cluster/1"; + final String clusterName = "groups/foo/clusters/foo-cluster.1"; assertThat(actualCluster).isEqualTo(cluster.toBuilder() .setName(clusterName) .setRespectDnsTtl(true) @@ -88,7 +92,7 @@ void createClusterViaHttp() throws Exception { checkResourceViaDiscoveryRequest(actualCluster, clusterName, true); // Create the same cluster again. - response = createCluster("groups/foo", "foo-cluster/1", cluster, dogma.httpClient()); + response = createCluster("groups/foo", "foo-cluster.1", cluster, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.CONFLICT); assertThat(response.headers().get("grpc-status")) .isEqualTo(Integer.toString(Status.ALREADY_EXISTS.getCode().value())); @@ -142,16 +146,16 @@ public void onCompleted() {} @Test void updateClusterViaHttp() throws Exception { final Cluster cluster = cluster("this_cluster_name_will_be_ignored_and_replaced", 1); - AggregatedHttpResponse response = updateCluster("groups/foo", "foo-cluster/2", + AggregatedHttpResponse response = updateCluster("groups/foo", "foo-cluster.2", cluster, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); - response = createCluster("groups/foo", "foo-cluster/2", cluster, dogma.httpClient()); + response = createCluster("groups/foo", "foo-cluster.2", cluster, dogma.httpClient()); assertOk(response); final Cluster.Builder clusterBuilder = Cluster.newBuilder(); JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), clusterBuilder); final Cluster actualCluster = clusterBuilder.build(); - final String clusterName = "groups/foo/clusters/foo-cluster/2"; + final String clusterName = "groups/foo/clusters/foo-cluster.2"; assertThat(actualCluster).isEqualTo(cluster.toBuilder() .setName(clusterName) .setRespectDnsTtl(true) @@ -164,7 +168,7 @@ void updateClusterViaHttp() throws Exception { .setName(clusterName) .setRespectDnsTtl(false) .build(); - response = updateCluster("groups/foo", "foo-cluster/2", updatingCluster, dogma.httpClient()); + response = updateCluster("groups/foo", "foo-cluster.2", updatingCluster, dogma.httpClient()); assertOk(response); final Cluster.Builder clusterBuilder2 = Cluster.newBuilder(); JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), clusterBuilder2); @@ -173,18 +177,18 @@ void updateClusterViaHttp() throws Exception { checkResourceViaDiscoveryRequest(actualCluster2, clusterName, true); // Can update with the same cluster again. - response = updateCluster("groups/foo", "foo-cluster/2", updatingCluster, dogma.httpClient()); + response = updateCluster("groups/foo", "foo-cluster.2", updatingCluster, dogma.httpClient()); assertOk(response); } @Test void deleteClusterViaHttp() throws Exception { - final String clusterName = "groups/foo/clusters/foo-cluster/3/4"; + final String clusterName = "groups/foo/clusters/foo-cluster.3.4"; AggregatedHttpResponse response = deleteCluster(clusterName); assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); final Cluster cluster = cluster("this_cluster_name_will_be_ignored_and_replaced", 1); - response = createCluster("groups/foo", "foo-cluster/3/4", cluster, dogma.httpClient()); + response = createCluster("groups/foo", "foo-cluster.3.4", cluster, dogma.httpClient()); assertOk(response); final Cluster actualCluster = cluster.toBuilder() @@ -214,9 +218,9 @@ void viaStub() { HttpHeaderNames.AUTHORIZATION, "Bearer anonymous").build(XdsClusterServiceBlockingStub.class); final Cluster cluster = cluster("this_cluster_name_will_be_ignored_and_replaced", 1); Cluster response = client.createCluster(CreateClusterRequest.newBuilder().setParent("groups/foo") - .setClusterId("foo-cluster/5/6") + .setClusterId("foo-cluster.5.6") .setCluster(cluster).build()); - final String clusterName = "groups/foo/clusters/foo-cluster/5/6"; + final String clusterName = "groups/foo/clusters/foo-cluster.5.6"; assertThat(response).isEqualTo(cluster.toBuilder() .setName(clusterName) .setRespectDnsTtl(true) diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointServiceTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointServiceTest.java index b253e7316..a8b4027cb 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointServiceTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointServiceTest.java @@ -83,15 +83,19 @@ void createEndpointViaHttp() throws Exception { createEndpoint("groups/foo", "@invalid_endpoint_id", endpoint, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); - response = createEndpoint("groups/non-existent-group", "foo-endpoint/1", endpoint, dogma.httpClient()); + // Slashes are no longer allowed in new resource IDs. + response = createEndpoint("groups/foo", "foo-endpoint/invalid", endpoint, dogma.httpClient()); + assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); + + response = createEndpoint("groups/non-existent-group", "foo-endpoint.1", endpoint, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); - response = createEndpoint("groups/foo", "foo-endpoint/1", endpoint, dogma.httpClient()); + response = createEndpoint("groups/foo", "foo-endpoint.1", endpoint, dogma.httpClient()); assertOk(response); final ClusterLoadAssignment.Builder endpointBuilder = ClusterLoadAssignment.newBuilder(); JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), endpointBuilder); final ClusterLoadAssignment actualEndpoint = endpointBuilder.build(); - final String clusterName = "groups/foo/clusters/foo-endpoint/1"; + final String clusterName = "groups/foo/clusters/foo-endpoint.1"; assertThat(actualEndpoint).isEqualTo( endpoint.toBuilder().setClusterName(clusterName).build()); checkEndpointsViaDiscoveryRequest(dogma.httpClient().uri(), actualEndpoint, clusterName); @@ -150,15 +154,15 @@ public void onCompleted() {} void updateEndpointViaHttp() throws Exception { final ClusterLoadAssignment endpoint = loadAssignment("this_endpoint_name_will_be_ignored_and_replaced", "127.0.0.1", 8080); - AggregatedHttpResponse response = updateEndpoint("foo-endpoint/2", endpoint); + AggregatedHttpResponse response = updateEndpoint("foo-endpoint.2", endpoint); assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); - response = createEndpoint("groups/foo", "foo-endpoint/2", endpoint, dogma.httpClient()); + response = createEndpoint("groups/foo", "foo-endpoint.2", endpoint, dogma.httpClient()); assertOk(response); final ClusterLoadAssignment.Builder endpointBuilder = ClusterLoadAssignment.newBuilder(); JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), endpointBuilder); final ClusterLoadAssignment actualEndpoint = endpointBuilder.build(); - final String clusterName = "groups/foo/clusters/foo-endpoint/2"; + final String clusterName = "groups/foo/clusters/foo-endpoint.2"; assertThat(actualEndpoint).isEqualTo(endpoint.toBuilder().setClusterName(clusterName).build()); checkEndpointsViaDiscoveryRequest(dogma.httpClient().uri(), actualEndpoint, clusterName); @@ -167,7 +171,7 @@ void updateEndpointViaHttp() throws Exception { .addEndpoints(LocalityLbEndpoints.newBuilder() .addLbEndpoints(endpoint("127.0.0.1", 8081))) .setClusterName(clusterName).build(); - response = updateEndpoint("foo-endpoint/2", updatingEndpoint); + response = updateEndpoint("foo-endpoint.2", updatingEndpoint); assertOk(response); final ClusterLoadAssignment.Builder endpointBuilder2 = ClusterLoadAssignment.newBuilder(); JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), endpointBuilder2); @@ -180,17 +184,17 @@ void updateEndpointViaHttp() throws Exception { @Test void createEndpointReturnAlreadyExistsWhenYamlExists() throws Exception { // Pre-populate the repo with a YAML endpoint (simulating a JSON→YAML migration). - final String clusterName = "groups/foo/clusters/yaml-exists/1"; + final String clusterName = "groups/foo/clusters/yaml-exists.1"; final ClusterLoadAssignment initial = loadAssignment(clusterName, "127.0.0.1", 8080); dogma.client().forRepo(INTERNAL_PROJECT_XDS, "foo") .commit("Add YAML endpoint", - Change.ofYamlUpsert(ENDPOINTS_DIRECTORY + "yaml-exists/1.yaml", + Change.ofYamlUpsert(ENDPOINTS_DIRECTORY + "yaml-exists.1.yaml", JSON_MESSAGE_MARSHALLER.writeValueAsString(initial))) .push().join(); // A create request for the same logical resource must return ALREADY_EXISTS, not succeed. final AggregatedHttpResponse response = - createEndpoint("groups/foo", "yaml-exists/1", initial, dogma.httpClient()); + createEndpoint("groups/foo", "yaml-exists.1", initial, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.CONFLICT); assertThat(response.headers().get("grpc-status")) .isEqualTo(Integer.toString(Status.ALREADY_EXISTS.getCode().value())); @@ -198,9 +202,9 @@ void createEndpointReturnAlreadyExistsWhenYamlExists() throws Exception { // The original .yaml file must still be the only file present (no new .json created). final Repository repo = dogma.projectManager().get(INTERNAL_PROJECT_XDS).repos().get("foo"); - assertThat(repo.find(Revision.HEAD, ENDPOINTS_DIRECTORY + "yaml-exists/1.yaml", + assertThat(repo.find(Revision.HEAD, ENDPOINTS_DIRECTORY + "yaml-exists.1.yaml", FindOptions.FIND_ONE_WITHOUT_CONTENT).join()).isNotEmpty(); - assertThat(repo.find(Revision.HEAD, ENDPOINTS_DIRECTORY + "yaml-exists/1.json", + assertThat(repo.find(Revision.HEAD, ENDPOINTS_DIRECTORY + "yaml-exists.1.json", FindOptions.FIND_ONE_WITHOUT_CONTENT).join()).isEmpty(); } @@ -279,14 +283,14 @@ private static AggregatedHttpResponse updateEndpoint( @Test void deleteEndpointViaHttp() throws Exception { - final String endpointName = "groups/foo/endpoints/foo-endpoint/3/4"; - final String clusterName = "groups/foo/clusters/foo-endpoint/3/4"; + final String endpointName = "groups/foo/endpoints/foo-endpoint.3.4"; + final String clusterName = "groups/foo/clusters/foo-endpoint.3.4"; AggregatedHttpResponse response = deleteEndpoint(endpointName); assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); final ClusterLoadAssignment endpoint = loadAssignment("this_endpoint_name_will_be_ignored_and_replaced", "127.0.0.1", 8080); - response = createEndpoint("groups/foo", "foo-endpoint/3/4", endpoint, dogma.httpClient()); + response = createEndpoint("groups/foo", "foo-endpoint.3.4", endpoint, dogma.httpClient()); assertOk(response); final ClusterLoadAssignment actualEndpoint = @@ -320,10 +324,10 @@ void viaStub() throws Exception { final ClusterLoadAssignment response = client.createEndpoint( CreateEndpointRequest.newBuilder() .setParent("groups/foo") - .setEndpointId("foo-endpoint/5/6") + .setEndpointId("foo-endpoint.5.6") .setEndpoint(endpoint) .build()); - final String clusterName = "groups/foo/clusters/foo-endpoint/5/6"; + final String clusterName = "groups/foo/clusters/foo-endpoint.5.6"; assertThat(response).isEqualTo(endpoint.toBuilder().setClusterName(clusterName).build()); checkEndpointsViaDiscoveryRequest(dogma.httpClient().uri(), response, clusterName); @@ -333,7 +337,7 @@ void viaStub() throws Exception { .addLbEndpoints(endpoint("127.0.0.1", 8081))) .setClusterName(clusterName).build(); - final String endpointName = "groups/foo/endpoints/foo-endpoint/5/6"; + final String endpointName = "groups/foo/endpoints/foo-endpoint.5.6"; final ClusterLoadAssignment response2 = client.updateEndpoint( UpdateEndpointRequest.newBuilder() .setEndpointName(endpointName) diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsRegisterEndpointTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsRegisterEndpointTest.java index df0fb7c45..3dac1f289 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsRegisterEndpointTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsRegisterEndpointTest.java @@ -66,12 +66,12 @@ static void setup() { @Test void registerOrDeregister() throws Exception { - final String clusterName = "groups/foo/clusters/foo-endpoint/1"; - final String endpointName = "groups/foo/endpoints/foo-endpoint/1"; + final String clusterName = "groups/foo/clusters/foo-endpoint.1"; + final String endpointName = "groups/foo/endpoints/foo-endpoint.1"; final Locality locality1 = Locality.newBuilder().setRegion("region1").setZone("zone1").build(); ClusterLoadAssignment endpoint = loadAssignment(clusterName, locality1, endpoint("127.0.0.1", 8080)); - AggregatedHttpResponse response = createEndpoint("groups/foo", "foo-endpoint/1", + AggregatedHttpResponse response = createEndpoint("groups/foo", "foo-endpoint.1", endpoint, dogma.httpClient()); assertOk(response); checkEndpointsViaDiscoveryRequest(dogma.httpClient().uri(), endpoint, clusterName); diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/group/v1/XdsGroupServiceTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/group/v1/XdsGroupServiceTest.java index dce2d7beb..2b10713b0 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/group/v1/XdsGroupServiceTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/group/v1/XdsGroupServiceTest.java @@ -97,6 +97,15 @@ void createAndDeleteGroupViaStub() { .build())).isInstanceOf(StatusRuntimeException.class) .hasMessageContaining("Invalid group id: invalid/id"); + // Dots are allowed in group names. + final Group dotGroup = client.createGroup( + CreateGroupRequest.newBuilder() + .setGroupId("foo.bar") + .setGroup(Group.newBuilder().setName("this_will_be_ignored")) + .build()); + assertThat(dotGroup.getName()).isEqualTo("groups/foo.bar"); + client.deleteGroup(DeleteGroupRequest.newBuilder().setName("groups/foo.bar").build()); + final Group group = client.createGroup( CreateGroupRequest.newBuilder() .setGroupId("baz") diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/AggregatingMultipleKubernetesTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/AggregatingMultipleKubernetesTest.java index c3063cf81..4c4ae7860 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/AggregatingMultipleKubernetesTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/AggregatingMultipleKubernetesTest.java @@ -117,7 +117,7 @@ private static void createNodesAndPods(List nodes, Deployment deployment) @Test void aggregateMultipleKubernetes() throws Exception { - final String aggregatorId = "foo-k8s-cluster/1"; + final String aggregatorId = "foo-k8s-cluster.1"; final String clusterName = "groups/foo/k8s/clusters/" + aggregatorId; final KubernetesEndpointAggregator aggregator = aggregator(aggregatorId); final AggregatedHttpResponse response = createAggregator(aggregator, aggregatorId, dogma.httpClient()); @@ -140,7 +140,7 @@ void aggregateMultipleKubernetes() throws Exception { K8S_ENDPOINTS_DIRECTORY + aggregatorId + ".yaml")).join(); assertThatJson(endpointEntry.content()).isEqualTo( '{' + - " \"clusterName\": \"groups/foo/k8s/clusters/foo-k8s-cluster/1\"," + + " \"clusterName\": \"groups/foo/k8s/clusters/foo-k8s-cluster.1\"," + " \"endpoints\": [ {" + " \"locality\": {" + " \"zone\": \"zone1\"" + @@ -213,7 +213,7 @@ void aggregateMultipleKubernetes() throws Exception { K8S_ENDPOINTS_DIRECTORY + aggregatorId + ".yaml")).join(); assertThatJson(endpointEntry1.content()).isEqualTo( '{' + - " \"clusterName\": \"groups/foo/k8s/clusters/foo-k8s-cluster/1\"," + + " \"clusterName\": \"groups/foo/k8s/clusters/foo-k8s-cluster.1\"," + " \"endpoints\": [ {" + " \"locality\": {" + " \"zone\": \"zone1\"" + @@ -279,7 +279,7 @@ void aggregateMultipleKubernetes() throws Exception { K8S_ENDPOINTS_DIRECTORY + aggregatorId + ".yaml")).join(); assertThatJson(endpointEntry2.content()).isEqualTo( '{' + - " \"clusterName\": \"groups/foo/k8s/clusters/foo-k8s-cluster/1\"," + + " \"clusterName\": \"groups/foo/k8s/clusters/foo-k8s-cluster.1\"," + " \"endpoints\": [ {" + " \"locality\": {" + " \"zone\": \"zone1\"" + diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesServiceTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesServiceTest.java index 59ccb4a47..ab433a0f6 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesServiceTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesServiceTest.java @@ -213,7 +213,7 @@ void invalidProperty() throws IOException { @CsvSource({ "repo-credential", "project-credential" }) @ParameterizedTest void createEndpointAggregatorsRequest(String credentialId) throws IOException { - final String aggregatorId = "foo-k8s-cluster/1"; + final String aggregatorId = "foo-k8s-cluster.1"; final String clusterName = "groups/foo/k8s/clusters/" + aggregatorId; final KubernetesEndpointAggregator aggregator = aggregator(aggregatorId, credentialId); final AggregatedHttpResponse response = createAggregator(aggregator, aggregatorId); @@ -322,7 +322,7 @@ private static ClusterLoadAssignment clusterLoadAssignment(String clusterName, i @CsvSource({ "repo-credential", "project-credential" }) @ParameterizedTest void updateAggregator(String credentialId) throws IOException { - final String aggregatorId = "foo-k8s-cluster/2"; + final String aggregatorId = "foo-k8s-cluster.2"; final KubernetesEndpointAggregator aggregator = aggregator(aggregatorId, credentialId); AggregatedHttpResponse response = createAggregator(aggregator, aggregatorId); assertOk(response); @@ -363,7 +363,7 @@ static AggregatedHttpResponse updateAggregator( @CsvSource({ "repo-credential", "project-credential" }) @ParameterizedTest void deleteAggregator(String credentialId) throws IOException { - final String aggregatorId = "foo-k8s-cluster/3"; + final String aggregatorId = "foo-k8s-cluster.3"; final KubernetesEndpointAggregator aggregator = aggregator(aggregatorId, credentialId); AggregatedHttpResponse response = createAggregator(aggregator, aggregatorId); assertOk(response); @@ -378,7 +378,7 @@ void deleteAggregator(String credentialId) throws IOException { @Test void createAggregator_migratesLegacyJsonEndpoint() throws IOException { - final String aggregatorId = "k8s-mig-cluster/1"; + final String aggregatorId = "k8s-mig-cluster.1"; final String clusterName = "groups/foo/k8s/clusters/" + aggregatorId; final Repository fooGroup = dogma.projectManager().get(INTERNAL_PROJECT_XDS).repos().get("foo"); diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/listener/v1/XdsListenerServiceTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/listener/v1/XdsListenerServiceTest.java index ad80b0efd..7c500f749 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/listener/v1/XdsListenerServiceTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/listener/v1/XdsListenerServiceTest.java @@ -71,15 +71,19 @@ void createListenerViaHttp() throws Exception { listener, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); - response = createListener("groups/non-existent-group", "foo-listener/1", listener, dogma.httpClient()); + // Slashes are no longer allowed in new resource IDs. + response = createListener("groups/foo", "foo-listener/invalid", listener, dogma.httpClient()); + assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); + + response = createListener("groups/non-existent-group", "foo-listener.1", listener, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); - response = createListener("groups/foo", "foo-listener/1", listener, dogma.httpClient()); + response = createListener("groups/foo", "foo-listener.1", listener, dogma.httpClient()); assertOk(response); final Listener.Builder listenerBuilder = Listener.newBuilder(); JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), listenerBuilder); final Listener actualListener = listenerBuilder.build(); - final String listenerName = "groups/foo/listeners/foo-listener/1"; + final String listenerName = "groups/foo/listeners/foo-listener.1"; assertThat(actualListener).isEqualTo(listener.toBuilder().setName(listenerName).build()); checkResourceViaDiscoveryRequest(actualListener, listenerName, true); } @@ -135,23 +139,23 @@ public void onCompleted() {} void updateListenerViaHttp() throws Exception { final Listener listener = exampleListener("this_listener_name_will_be_ignored_and_replaced", "groups/foo/routes/foo-route", "stats"); - AggregatedHttpResponse response = updateListener("groups/foo", "foo-listener/2", listener, + AggregatedHttpResponse response = updateListener("groups/foo", "foo-listener.2", listener, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); - response = createListener("groups/foo", "foo-listener/2", listener, dogma.httpClient()); + response = createListener("groups/foo", "foo-listener.2", listener, dogma.httpClient()); assertOk(response); final Listener.Builder listenerBuilder = Listener.newBuilder(); JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), listenerBuilder); final Listener actualListener = listenerBuilder.build(); - final String listenerName = "groups/foo/listeners/foo-listener/2"; + final String listenerName = "groups/foo/listeners/foo-listener.2"; assertThat(actualListener).isEqualTo(listener.toBuilder().setName(listenerName).build()); checkResourceViaDiscoveryRequest(actualListener, listenerName, true); final Listener updatingListener = listener.toBuilder() .setStatPrefix("updated_stats") .setName(listenerName).build(); - response = updateListener("groups/foo", "foo-listener/2", updatingListener, dogma.httpClient()); + response = updateListener("groups/foo", "foo-listener.2", updatingListener, dogma.httpClient()); assertOk(response); final Listener.Builder listenerBuilder2 = Listener.newBuilder(); JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), listenerBuilder2); @@ -162,13 +166,13 @@ void updateListenerViaHttp() throws Exception { @Test void deleteListenerViaHttp() throws Exception { - final String listenerName = "groups/foo/listeners/foo-listener/3/4"; + final String listenerName = "groups/foo/listeners/foo-listener.3.4"; AggregatedHttpResponse response = deleteListener(listenerName); assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); final Listener listener = exampleListener("this_listener_name_will_be_ignored_and_replaced", "groups/foo/routes/foo-route", "stats"); - response = createListener("groups/foo", "foo-listener/3/4", listener, dogma.httpClient()); + response = createListener("groups/foo", "foo-listener.3.4", listener, dogma.httpClient()); assertOk(response); final Listener actualListener = listener.toBuilder().setName(listenerName).build(); @@ -201,10 +205,10 @@ void viaStub() { Listener response = client.createListener( CreateListenerRequest.newBuilder() .setParent("groups/foo") - .setListenerId("foo-listener/5/6") + .setListenerId("foo-listener.5.6") .setListener(listener) .build()); - final String listenerName = "groups/foo/listeners/foo-listener/5/6"; + final String listenerName = "groups/foo/listeners/foo-listener.5.6"; assertThat(response).isEqualTo(listener.toBuilder().setName(listenerName).build()); final Listener updatingListener = listener.toBuilder() diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/route/v1/XdsRouteServiceTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/route/v1/XdsRouteServiceTest.java index 8856fee0d..6bf201053 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/route/v1/XdsRouteServiceTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/route/v1/XdsRouteServiceTest.java @@ -71,15 +71,19 @@ void createRouteViaHttp() throws Exception { route, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); - response = createRoute("groups/non-existent-group", "foo-route/1", route, dogma.httpClient()); + // Slashes are no longer allowed in new resource IDs. + response = createRoute("groups/foo", "foo-route/invalid", route, dogma.httpClient()); + assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); + + response = createRoute("groups/non-existent-group", "foo-route.1", route, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); - response = createRoute("groups/foo", "foo-route/1", route, dogma.httpClient()); + response = createRoute("groups/foo", "foo-route.1", route, dogma.httpClient()); assertOk(response); final RouteConfiguration.Builder routeBuilder = RouteConfiguration.newBuilder(); JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), routeBuilder); final RouteConfiguration actualRoute = routeBuilder.build(); - final String routeName = "groups/foo/routes/foo-route/1"; + final String routeName = "groups/foo/routes/foo-route.1"; assertThat(actualRoute).isEqualTo(route.toBuilder().setName(routeName).build()); checkResourceViaDiscoveryRequest(actualRoute, routeName, true); } @@ -135,22 +139,22 @@ public void onCompleted() {} void updateRouteViaHttp() throws Exception { final RouteConfiguration route = routeConfiguration("this_route_name_will_be_ignored_and_replaced", "groups/foo/clusters/foo-cluster"); - AggregatedHttpResponse response = updateRoute("groups/foo", "foo-route/2", route, dogma.httpClient()); + AggregatedHttpResponse response = updateRoute("groups/foo", "foo-route.2", route, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); - response = createRoute("groups/foo", "foo-route/2", route, dogma.httpClient()); + response = createRoute("groups/foo", "foo-route.2", route, dogma.httpClient()); assertOk(response); final RouteConfiguration.Builder routeBuilder = RouteConfiguration.newBuilder(); JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), routeBuilder); final RouteConfiguration actualRoute = routeBuilder.build(); - final String routeName = "groups/foo/routes/foo-route/2"; + final String routeName = "groups/foo/routes/foo-route.2"; assertThat(actualRoute).isEqualTo(route.toBuilder().setName(routeName).build()); checkResourceViaDiscoveryRequest(actualRoute, routeName, true); final RouteConfiguration updatingRoute = route.toBuilder() .addInternalOnlyHeaders("internal") .setName(routeName).build(); - response = updateRoute("groups/foo", "foo-route/2", updatingRoute, dogma.httpClient()); + response = updateRoute("groups/foo", "foo-route.2", updatingRoute, dogma.httpClient()); assertOk(response); final RouteConfiguration.Builder routeBuilder2 = RouteConfiguration.newBuilder(); JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), routeBuilder2); @@ -161,13 +165,13 @@ void updateRouteViaHttp() throws Exception { @Test void deleteRouteViaHttp() throws Exception { - final String routeName = "groups/foo/routes/foo-route/3/4"; + final String routeName = "groups/foo/routes/foo-route.3.4"; AggregatedHttpResponse response = deleteRoute(routeName); assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); final RouteConfiguration route = routeConfiguration("this_route_name_will_be_ignored_and_replaced", "groups/foo/clusters/foo-cluster"); - response = createRoute("groups/foo", "foo-route/3/4", route, dogma.httpClient()); + response = createRoute("groups/foo", "foo-route.3.4", route, dogma.httpClient()); assertOk(response); final RouteConfiguration actualRoute = route.toBuilder().setName(routeName).build(); @@ -200,10 +204,10 @@ void viaStub() { RouteConfiguration response = client.createRoute( CreateRouteRequest.newBuilder() .setParent("groups/foo") - .setRouteId("foo-route/5/6") + .setRouteId("foo-route.5.6") .setRoute(route) .build()); - final String routeName = "groups/foo/routes/foo-route/5/6"; + final String routeName = "groups/foo/routes/foo-route.5.6"; assertThat(response).isEqualTo(route.toBuilder().setName(routeName).build()); final RouteConfiguration updatingRoute = route.toBuilder()