From 27d22eb9da8cacdf66bffa514c111b10622aa67d Mon Sep 17 00:00:00 2001 From: minwoox Date: Mon, 20 Jul 2026 14:21:17 +0900 Subject: [PATCH 1/6] Migrate xDS resource services from gRPC to YAML-over-HTTP annotated services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Motivation: The xDS resource management APIs (cluster, endpoint, listener, route, group, kubernetes) were implemented as gRPC services backed by service-specific `.proto` files. Migrating them to Armeria annotated HTTP services that consume `application/yaml` removes the protobuf service layer, simplifies the API surface, and makes the YAML-native storage format a first-class concern — clients send YAML directly and it is stored as-is (with `name`/`clusterName` injected). Modifications: - Convert `XdsClusterService`, `XdsEndpointService`, `XdsListenerService`, `XdsRouteService` from `*ImplBase` gRPC stubs to plain annotated HTTP services (`@Post`/`@Put`/`@Delete`, `@Consumes("application/yaml")`). - Add `RequiresXdsGroupRole` annotation and `RequiresXdsGroupRoleDecorator` to replace the inline `checkWritePermission` calls with a declarative, per-method authorization mechanism. - Refactor `XdsResourceManager` so that `push`/`update`/`delete` operate on raw YAML strings instead of protobuf messages, and add helpers `parseYaml`, `normalizeYamlKeys`, `injectYamlField`, and `errorResponse`. Result: - xDS resource CRUD is now served via plain HTTP with YAML bodies, removing the gRPC service layer and the associated proto wrapper definitions. --- .../k8s/XdsKubernetesNodeIpExtractorTest.java | 11 +- webapp/src/dogma/features/xds/XdsTypes.ts | 1 - .../xds/cluster/v1/XdsClusterService.java | 156 ++--- .../xds/endpoint/v1/XdsEndpointService.java | 283 +++++---- .../v1/XdsEndpointUpdateScheduler.java | 124 ++-- .../xds/group/v1/XdsGroupService.java | 124 ++-- .../xds/internal/ControlPlaneService.java | 91 +-- .../xds/internal/RequiresXdsGroupRole.java | 46 ++ .../RequiresXdsGroupRoleDecorator.java | 114 ++++ .../xds/internal/XdsResourceManager.java | 421 ++++++++------ .../xds/k8s/v1/XdsKubernetesService.java | 545 ++++++++++-------- .../xds/listener/v1/XdsListenerService.java | 148 +++-- .../xds/route/v1/XdsRouteService.java | 145 +++-- .../xds/cluster/v1/xds_cluster.proto | 99 ---- .../xds/endpoint/v1/xds_endpoint.proto | 115 +--- .../centraldogma/xds/group/v1/xds_group.proto | 63 -- .../xds/listener/v1/xds_listener.proto | 99 ---- .../centraldogma/xds/route/v1/xds_route.proto | 99 ---- .../xds/cluster/v1/XdsClusterServiceTest.java | 42 +- .../endpoint/v1/XdsEndpointServiceTest.java | 70 +-- .../endpoint/v1/XdsRegisterEndpointTest.java | 18 +- .../xds/group/v1/XdsGroupServiceTest.java | 61 +- .../MtlsDiscoveryAuthorizationTest.java | 4 +- .../TokenDiscoveryAuthorizationTest.java | 6 +- .../XdsEndpointReadPermissionTest.java | 6 +- .../XdsGroupDeletePermissionTest.java | 6 +- .../XdsLegacyJsonCompatibilityTest.java | 10 +- .../xds/internal/XdsResourceManagerTest.java | 262 +++++++++ .../xds/internal/XdsTestUtil.java | 47 +- .../xds/internal/XdsWritePermissionTest.java | 20 +- .../internal/XdsYamlCompatibilityTest.java | 18 +- .../v1/KubernetesEndpointMetadataTest.java | 2 - .../xds/k8s/v1/XdsKubernetesServiceTest.java | 66 ++- .../listener/v1/XdsListenerServiceTest.java | 43 +- .../xds/route/v1/XdsRouteServiceTest.java | 43 +- 35 files changed, 1717 insertions(+), 1691 deletions(-) create mode 100644 xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRole.java create mode 100644 xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRoleDecorator.java delete mode 100644 xds/src/main/proto/centraldogma/xds/cluster/v1/xds_cluster.proto delete mode 100644 xds/src/main/proto/centraldogma/xds/group/v1/xds_group.proto delete mode 100644 xds/src/main/proto/centraldogma/xds/listener/v1/xds_listener.proto delete mode 100644 xds/src/main/proto/centraldogma/xds/route/v1/xds_route.proto create mode 100644 xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsResourceManagerTest.java diff --git a/it/xds-k8s-node-ip-extractor/src/test/java/com/linecorp/centraldogma/it/xds/k8s/XdsKubernetesNodeIpExtractorTest.java b/it/xds-k8s-node-ip-extractor/src/test/java/com/linecorp/centraldogma/it/xds/k8s/XdsKubernetesNodeIpExtractorTest.java index bb0475395..9c1d9c6b0 100644 --- a/it/xds-k8s-node-ip-extractor/src/test/java/com/linecorp/centraldogma/it/xds/k8s/XdsKubernetesNodeIpExtractorTest.java +++ b/it/xds-k8s-node-ip-extractor/src/test/java/com/linecorp/centraldogma/it/xds/k8s/XdsKubernetesNodeIpExtractorTest.java @@ -43,6 +43,8 @@ import com.linecorp.centraldogma.common.Entry; import com.linecorp.centraldogma.common.Query; import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.internal.Jackson; +import com.linecorp.centraldogma.internal.Yaml; import com.linecorp.centraldogma.server.storage.repository.Repository; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; import com.linecorp.centraldogma.xds.internal.XdsResourceManager; @@ -147,7 +149,6 @@ void extractsNodeIpFromLabel() throws Exception { final AggregatedHttpResponse response = createAggregator(aggregator, aggregatorId); assertThat(response.status()).isSameAs(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); final Repository fooGroup = dogma.projectManager().get(INTERNAL_PROJECT_XDS) .repos().get("foo"); @@ -223,7 +224,6 @@ void fallsBackToInternalIpWhenLabelKeyIsAbsent() throws Exception { final AggregatedHttpResponse response = createAggregator(aggregator, aggregatorId); assertThat(response.status()).isSameAs(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); final Repository fooGroup = dogma.projectManager().get(INTERNAL_PROJECT_XDS) .repos().get("foo"); @@ -315,11 +315,12 @@ private static AggregatedHttpResponse createAggregator( RequestHeaders.builder(HttpMethod.POST, "/api/v1/xds/groups/foo/k8s/endpointAggregators?" + "aggregator_id=" + aggregatorId) - .contentType(MediaType.JSON_UTF_8) + .contentType(MediaType.parse("application/yaml")) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") .build(); - return dogma.httpClient().blocking().execute( - headers, XdsResourceManager.JSON_MESSAGE_MARSHALLER.writeValueAsString(aggregator)); + final String yaml = Yaml.writeValueAsString( + Jackson.readTree(XdsResourceManager.JSON_MESSAGE_MARSHALLER.writeValueAsString(aggregator))); + return dogma.httpClient().blocking().execute(headers, yaml); } private static Node newNodeWithLabel(String internalIp, String labelKey, String labelValue) { diff --git a/webapp/src/dogma/features/xds/XdsTypes.ts b/webapp/src/dogma/features/xds/XdsTypes.ts index 27c3989f9..861b740c2 100644 --- a/webapp/src/dogma/features/xds/XdsTypes.ts +++ b/webapp/src/dogma/features/xds/XdsTypes.ts @@ -125,7 +125,6 @@ export const XDS_RESOURCE_TEMPLATES: Record = { }, }, ], - respectDnsTtl: true, }, null, 2, 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 a66c56bd6..70c77977c 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 @@ -20,24 +20,30 @@ 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.removePrefix; -import java.util.regex.Matcher; +import java.io.IOException; +import java.util.concurrent.CompletableFuture; import java.util.regex.Pattern; -import com.google.protobuf.Empty; +import org.jspecify.annotations.Nullable; -import com.linecorp.centraldogma.xds.cluster.v1.XdsClusterServiceGrpc.XdsClusterServiceImplBase; +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.server.annotation.Consumes; +import com.linecorp.armeria.server.annotation.Delete; +import com.linecorp.armeria.server.annotation.Param; +import com.linecorp.armeria.server.annotation.Post; +import com.linecorp.armeria.server.annotation.Put; +import com.linecorp.centraldogma.common.RepositoryRole; +import com.linecorp.centraldogma.xds.internal.RequiresXdsGroupRole; import com.linecorp.centraldogma.xds.internal.XdsResourceManager; import io.envoyproxy.envoy.config.cluster.v3.Cluster; -import io.grpc.Status; -import io.grpc.stub.StreamObserver; /** - * Service for managing clusters. + * Annotated service object for managing clusters. */ -public final class XdsClusterService extends XdsClusterServiceImplBase { +public final class XdsClusterService { private static final Pattern CLUSTER_NAME_PATTERN = Pattern.compile("^groups/([^/]+)/clusters/" + LEGACY_RESOURCE_ID_PATTERN_STRING + '$'); @@ -51,69 +57,89 @@ public XdsClusterService(XdsResourceManager xdsResourceManager) { this.xdsResourceManager = xdsResourceManager; } - @Override - public void createCluster(CreateClusterRequest request, StreamObserver responseObserver) { - final String parent = request.getParent(); - final String group = removePrefix("groups/", parent); - xdsResourceManager.checkWritePermission(group); - - final String clusterId = request.getClusterId(); + /** + * POST /xds/groups/{group}/clusters + * + *

Creates a new cluster. + */ + @Post("/xds/groups/{group}/clusters") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture createCluster( + @Param("group") String group, + @Param("cluster_id") String clusterId, + @Param("summary") @Nullable String summary, + String body) { if (!RESOURCE_ID_PATTERN.matcher(clusterId).matches()) { - throw Status.INVALID_ARGUMENT.withDescription("Invalid cluster_id: " + clusterId + - " (expected: " + RESOURCE_ID_PATTERN + ')') - .asRuntimeException(); + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid cluster ID: " + clusterId)); } - - final String clusterName = parent + CLUSTERS_DIRECTORY + clusterId; - final Cluster cluster - = request.getCluster() - .toBuilder() - // Ignore the specified name in the cluster and set the name with the format of - // "groups/{group}/clusters/{cluster}". - // https://github.com/aip-dev/google.aip.dev/blob/master/aip/general/0133.md#user-specified-ids - .setName(clusterName) - // Respect the DNS TTL would be more efficient in terms of DNS resolution. - // https://github.com/envoyproxy/envoy/issues/6876 - // `respect_dns_ttl` is a `bool` field so it is not possible to check whether a value - // has not been set for the field. Until we create our own proto file, the value only - // can be set to false via the update API. - .setRespectDnsTtl(true) - .build(); - final String createSummary = isNullOrEmpty(request.getSummary()) ? - "Create cluster: " + clusterName : request.getSummary(); - xdsResourceManager.push(responseObserver, group, clusterName, CLUSTERS_DIRECTORY + clusterId + ".yaml", - createSummary, cluster, currentAuthor(), true); - } - - @Override - public void updateCluster(UpdateClusterRequest request, StreamObserver responseObserver) { - final Cluster cluster = request.getCluster(); - final String clusterName = cluster.getName(); - final String group = checkClusterName(clusterName).group(1); - xdsResourceManager.checkWritePermission(group); - final String updateSummary = isNullOrEmpty(request.getSummary()) ? - "Update cluster: " + clusterName : request.getSummary(); - xdsResourceManager.update(responseObserver, group, clusterName, - updateSummary, cluster, currentAuthor()); + final String clusterName = "groups/" + group + CLUSTERS_DIRECTORY + clusterId; + try { + XdsResourceManager.parseYaml(body, Cluster.newBuilder()); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); + } + final String createSummary = isNullOrEmpty(summary) ? "Create cluster: " + clusterName : summary; + final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); + final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", clusterName); + return xdsResourceManager.push(group, clusterName, CLUSTERS_DIRECTORY + clusterId + ".yaml", + createSummary, currentAuthor(), true, bodyToStore); } - @Override - public void deleteCluster(DeleteClusterRequest request, StreamObserver responseObserver) { - final String clusterName = request.getName(); - final String group = checkClusterName(clusterName).group(1); - xdsResourceManager.checkWritePermission(group); - final String deleteSummary = isNullOrEmpty(request.getSummary()) ? - "Delete cluster: " + clusterName : request.getSummary(); - xdsResourceManager.delete(responseObserver, group, clusterName, deleteSummary, currentAuthor()); + /** + * PUT /xds/groups/{group}/clusters/{cluster_id} + * + *

Updates an existing cluster. + */ + @Put("/xds/groups/{group}/clusters/{*cluster_id}") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture updateCluster( + @Param("group") String group, + @Param("cluster_id") String clusterId, + @Param("summary") @Nullable String summary, + String body) { + final String clusterName = "groups/" + group + "/clusters/" + clusterId; + if (!CLUSTER_NAME_PATTERN.matcher(clusterName).matches()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid cluster name: " + clusterName)); + } + try { + XdsResourceManager.parseYaml(body, Cluster.newBuilder()); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); + } + final String updateSummary = isNullOrEmpty(summary) ? "Update cluster: " + clusterName : summary; + final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); + final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", clusterName); + return xdsResourceManager.update(group, clusterName, updateSummary, currentAuthor(), bodyToStore); } - private static Matcher checkClusterName(String clusterName) { - final Matcher matcher = CLUSTER_NAME_PATTERN.matcher(clusterName); - if (!matcher.matches()) { - throw Status.INVALID_ARGUMENT.withDescription("Invalid cluster name: " + clusterName + - " (expected: " + CLUSTER_NAME_PATTERN + ')') - .asRuntimeException(); + /** + * DELETE /xds/groups/{group}/clusters/{cluster_id} + * + *

Removes a cluster. + */ + @Delete("/xds/groups/{group}/clusters/{*cluster_id}") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture deleteCluster( + @Param("group") String group, + @Param("cluster_id") String clusterId, + @Param("summary") @Nullable String summary) { + final String clusterName = "groups/" + group + "/clusters/" + clusterId; + if (!CLUSTER_NAME_PATTERN.matcher(clusterName).matches()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid cluster name: " + clusterName)); } - return matcher; + final String deleteSummary = isNullOrEmpty(summary) ? "Delete cluster: " + clusterName : summary; + return xdsResourceManager.delete(group, clusterName, deleteSummary, currentAuthor()); } } 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 4fae0e651..dcbc7e7a4 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 @@ -21,27 +21,35 @@ 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.removePrefix; +import java.io.IOException; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ScheduledExecutorService; import java.util.regex.Matcher; import java.util.regex.Pattern; -import com.google.protobuf.Empty; - -import com.linecorp.centraldogma.xds.endpoint.v1.XdsEndpointServiceGrpc.XdsEndpointServiceImplBase; +import org.jspecify.annotations.Nullable; + +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.server.annotation.Consumes; +import com.linecorp.armeria.server.annotation.Delete; +import com.linecorp.armeria.server.annotation.Param; +import com.linecorp.armeria.server.annotation.Patch; +import com.linecorp.armeria.server.annotation.Post; +import com.linecorp.armeria.server.annotation.Put; +import com.linecorp.centraldogma.common.RepositoryRole; +import com.linecorp.centraldogma.xds.internal.RequiresXdsGroupRole; import com.linecorp.centraldogma.xds.internal.XdsResourceManager; import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment; -import io.grpc.Status; -import io.grpc.stub.StreamObserver; /** - * Service for managing endpoints. + * Annotated service object for managing endpoints. */ -public final class XdsEndpointService extends XdsEndpointServiceImplBase { +public final class XdsEndpointService { - private static final Pattern ENDPONT_NAME_PATTERN = + private static final Pattern ENDPOINT_NAME_PATTERN = Pattern.compile("^groups/([^/]+)/endpoints/(" + LEGACY_RESOURCE_ID_PATTERN_STRING + ")$"); private final XdsResourceManager xdsResourceManager; @@ -53,122 +61,179 @@ public final class XdsEndpointService extends XdsEndpointServiceImplBase { public XdsEndpointService(XdsResourceManager xdsResourceManager, ScheduledExecutorService controlPlaneExecutor) { this.xdsResourceManager = xdsResourceManager; - xdsEndpointUpdateScheduler = new XdsEndpointUpdateScheduler(xdsResourceManager, controlPlaneExecutor); + xdsEndpointUpdateScheduler = new XdsEndpointUpdateScheduler(xdsResourceManager, + controlPlaneExecutor); } - @Override - public void createEndpoint(CreateEndpointRequest request, - StreamObserver responseObserver) { - final String parent = request.getParent(); - final String group = removePrefix("groups/", parent); - xdsResourceManager.checkWritePermission(group); + /** + * Returns the batch update task size. + */ + public int batchUpdateTaskSize() { + return xdsEndpointUpdateScheduler.batchUpdateTaskSize(); + } - final String endpointId = request.getEndpointId(); + /** + * POST /xds/groups/{group}/endpoints + * + *

Creates a new endpoint. + */ + @Post("/xds/groups/{group}/endpoints") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture createEndpoint( + @Param("group") String group, + @Param("endpoint_id") String endpointId, + @Param("summary") @Nullable String summary, + String body) { if (!RESOURCE_ID_PATTERN.matcher(endpointId).matches()) { - throw Status.INVALID_ARGUMENT.withDescription("Invalid endpoint_id: " + endpointId + - " (expected: " + RESOURCE_ID_PATTERN + ')') - .asRuntimeException(); + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid endpoint ID: " + endpointId)); } - - final String clusterName = clusterName(parent, endpointId); - // Ignore the specified name in the endpoint and set the name - // with the format of "groups/{group}/clusters/{endpoint}". - // https://github.com/aip-dev/google.aip.dev/blob/master/aip/general/0133.md#user-specified-ids - final ClusterLoadAssignment endpoint = request.getEndpoint() - .toBuilder() - .setClusterName(clusterName) - .build(); - final String createSummary = - isNullOrEmpty(request.getSummary()) ? "Create endpoint: " + clusterName : request.getSummary(); - xdsResourceManager.push(responseObserver, group, clusterName, fileName(endpointId), - createSummary, endpoint, currentAuthor(), true); - } - - private static String clusterName(String parent, String endpointId) { - // Use /clusters/ instead of /endpoints/ for the cluster name. - // /endpoints/ will be used for the file name. - return parent + CLUSTERS_DIRECTORY + endpointId; - } - - @Override - public void updateEndpoint(UpdateEndpointRequest request, - StreamObserver responseObserver) { - final String endpointName = request.getEndpointName(); - final Matcher matcher = checkEndpointName(endpointName); - final String group = matcher.group(1); - xdsResourceManager.checkWritePermission(group); - - final ClusterLoadAssignment endpoint = request.getEndpoint(); - final String endpointId = matcher.group(2); - final String updateSummary = - isNullOrEmpty(request.getSummary()) ? "Update endpoint: " + endpointName : request.getSummary(); - xdsResourceManager.update(responseObserver, group, endpointName, - fileName(endpointId), updateSummary, - endpoint.toBuilder() - .setClusterName(clusterName("groups/" + group, endpointId)) - .build(), currentAuthor()); - } - - @Override - public void deleteEndpoint(DeleteEndpointRequest request, StreamObserver responseObserver) { - final String endpointName = request.getName(); - final Matcher matcher = checkEndpointName(endpointName); - final String group = matcher.group(1); - xdsResourceManager.checkWritePermission(group); - final String deleteSummary = - isNullOrEmpty(request.getSummary()) ? "Delete endpoint: " + endpointName : request.getSummary(); - xdsResourceManager.delete(responseObserver, group, endpointName, fileName(matcher.group(2)), - deleteSummary, currentAuthor()); + final String endpointName = "groups/" + group + ENDPOINTS_DIRECTORY + endpointId; + final String clusterName = "groups/" + group + CLUSTERS_DIRECTORY + endpointId; + try { + XdsResourceManager.parseYaml(body, ClusterLoadAssignment.newBuilder()); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); + } + final String createSummary = isNullOrEmpty(summary) ? "Create endpoint: " + endpointName : summary; + final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); + final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, + "clusterName", clusterName); + return xdsResourceManager.push(group, endpointName, ENDPOINTS_DIRECTORY + endpointId + ".yaml", + createSummary, currentAuthor(), true, bodyToStore); } - private static Matcher checkEndpointName(String endpointName) { - final Matcher matcher = ENDPONT_NAME_PATTERN.matcher(endpointName); + /** + * PUT /xds/groups/{group}/endpoints/{endpoint_id} + * + *

Updates an existing endpoint. + */ + @Put("/xds/groups/{group}/endpoints/{*endpoint_id}") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture updateEndpoint( + @Param("group") String group, + @Param("endpoint_id") String endpointId, + @Param("summary") @Nullable String summary, + String body) { + final String endpointName = "groups/" + group + "/endpoints/" + endpointId; + final Matcher matcher = ENDPOINT_NAME_PATTERN.matcher(endpointName); if (!matcher.matches()) { - throw Status.INVALID_ARGUMENT.withDescription("Invalid endpoint name: " + endpointName + - " (expected: " + ENDPONT_NAME_PATTERN + ')') - .asRuntimeException(); + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid endpoint name: " + endpointName)); } - return matcher; - } - - private static String fileName(String endpointId) { - return ENDPOINTS_DIRECTORY + endpointId + ".yaml"; - } - - @Override - public void registerLocalityLbEndpoint(RegisterLocalityLbEndpointRequest request, - StreamObserver responseObserver) { - final String endpointName = request.getEndpointName(); - final LocalityLbEndpoint localityLbEndpoint = request.getLocalityLbEndpoint(); - handleRegisterOrDeregister(endpointName, localityLbEndpoint, responseObserver, true); + final String clusterName = "groups/" + group + CLUSTERS_DIRECTORY + endpointId; + try { + XdsResourceManager.parseYaml(body, ClusterLoadAssignment.newBuilder()); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); + } + final String updateSummary = isNullOrEmpty(summary) ? "Update endpoint: " + endpointName : summary; + final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); + final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, + "clusterName", clusterName); + return xdsResourceManager.update(group, endpointName, updateSummary, currentAuthor(), bodyToStore); } - @Override - public void deregisterLocalityLbEndpoint(DeregisterLocalityLbEndpointRequest request, - StreamObserver responseObserver) { - final String endpointName = request.getEndpointName(); - final LocalityLbEndpoint localityLbEndpoint = request.getLocalityLbEndpoint(); - handleRegisterOrDeregister(endpointName, localityLbEndpoint, responseObserver, false); + /** + * DELETE /xds/groups/{group}/endpoints/{endpoint_id} + * + *

Removes an endpoint. + */ + @Delete("/xds/groups/{group}/endpoints/{*endpoint_id}") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture deleteEndpoint( + @Param("group") String group, + @Param("endpoint_id") String endpointId, + @Param("summary") @Nullable String summary) { + final String endpointName = "groups/" + group + "/endpoints/" + endpointId; + if (!ENDPOINT_NAME_PATTERN.matcher(endpointName).matches()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid endpoint name: " + endpointName)); + } + final String deleteSummary = isNullOrEmpty(summary) ? "Delete endpoint: " + endpointName : summary; + return xdsResourceManager.delete(group, endpointName, deleteSummary, currentAuthor()); } - private void handleRegisterOrDeregister(String endpointName, - LocalityLbEndpoint localityLbEndpoint, - StreamObserver responseObserver, - boolean register) { - final Matcher matcher = checkEndpointName(endpointName); - final String group = matcher.group(1); - xdsResourceManager.checkWritePermission(group); - - final String endpointId = matcher.group(2); - final String fileName = fileName(endpointId); - xdsEndpointUpdateScheduler.schedule(group, endpointName, fileName, - localityLbEndpoint, responseObserver, register); + /** + * PATCH /xds/groups/{group}/endpoints/{endpointId}:registerLocalityLbEndpoint + * + *

Registers a locality lb endpoint into an existing endpoint resource. + */ + // Named capture groups in Java regex do not allow underscores, so endpointId uses camelCase + // here even though the rest of the path parameters follow snake_case (AIP-140). + @Patch("regex:/xds/groups/(?[^/]+)/endpoints/(?.+):registerLocalityLbEndpoint") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture registerLocalityLbEndpoint( + @Param("group") String group, + @Param("endpointId") String endpointId, + String body) { + final String endpointName = "groups/" + group + "/endpoints/" + endpointId; + final Matcher matcher = ENDPOINT_NAME_PATTERN.matcher(endpointName); + if (!matcher.matches()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid endpoint name: " + endpointName)); + } + final LocalityLbEndpoint localityLbEndpoint; + try { + localityLbEndpoint = parseLocalityLbEndpoint(body); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); + } + final String fileName = ENDPOINTS_DIRECTORY + endpointId + ".yaml"; + final CompletableFuture future = new CompletableFuture<>(); + xdsEndpointUpdateScheduler.schedule(group, endpointName, fileName, localityLbEndpoint, future, true); + return future; } /** - * Returns the number of tasks that are currently scheduled for batch updates. + * DELETE /xds/groups/{group}/endpoints/{endpointId}:deregisterLocalityLbEndpoint + * + *

Deregisters a locality lb endpoint from an existing endpoint resource. */ - public int batchUpdateTaskSize() { - return xdsEndpointUpdateScheduler.batchUpdateTaskSize(); + // See the comment on registerLocalityLbEndpoint for why endpointId uses camelCase. + @Delete("regex:/xds/groups/(?[^/]+)/endpoints/(?.+):deregisterLocalityLbEndpoint") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture deregisterLocalityLbEndpoint( + @Param("group") String group, + @Param("endpointId") String endpointId, + String body) { + final String endpointName = "groups/" + group + "/endpoints/" + endpointId; + final Matcher matcher = ENDPOINT_NAME_PATTERN.matcher(endpointName); + if (!matcher.matches()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid endpoint name: " + endpointName)); + } + final LocalityLbEndpoint localityLbEndpoint; + try { + localityLbEndpoint = parseLocalityLbEndpoint(body); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); + } + final String fileName = ENDPOINTS_DIRECTORY + endpointId + ".yaml"; + final CompletableFuture future = new CompletableFuture<>(); + xdsEndpointUpdateScheduler.schedule(group, endpointName, fileName, localityLbEndpoint, future, + false); + return future; + } + + private static LocalityLbEndpoint parseLocalityLbEndpoint(String body) throws IOException { + return XdsResourceManager.parseYaml(body, LocalityLbEndpoint.newBuilder()); } } diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointUpdateScheduler.java b/xds/src/main/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointUpdateScheduler.java index 77eb1fe67..03af9abfd 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointUpdateScheduler.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointUpdateScheduler.java @@ -24,6 +24,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ScheduledExecutorService; @@ -36,11 +37,10 @@ import com.fasterxml.jackson.databind.JsonNode; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; -import com.google.protobuf.Empty; +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.util.Exceptions; -import com.linecorp.armeria.common.util.SafeCloseable; -import com.linecorp.armeria.internal.common.RequestContextUtil; import com.linecorp.armeria.internal.common.util.ReentrantShortLock; import com.linecorp.centraldogma.common.Author; import com.linecorp.centraldogma.common.Change; @@ -63,9 +63,6 @@ import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment.Builder; import io.envoyproxy.envoy.config.endpoint.v3.LbEndpoint; import io.envoyproxy.envoy.config.endpoint.v3.LocalityLbEndpoints; -import io.grpc.Status; -import io.grpc.StatusRuntimeException; -import io.grpc.stub.StreamObserver; final class XdsEndpointUpdateScheduler { @@ -84,18 +81,9 @@ int batchUpdateTaskSize() { return batchUpdateTasks.size(); } - private static String alternativeFileName(String fileName) { - if (fileName.endsWith(".json")) { - return fileName.substring(0, fileName.length() - 5) + ".yaml"; - } - if (fileName.endsWith(".yaml")) { - return fileName.substring(0, fileName.length() - 5) + ".json"; - } - return fileName; - } - void schedule(String group, String endpointName, String fileName, - LocalityLbEndpoint localityLbEndpoint, StreamObserver streamObserver, boolean register) { + LocalityLbEndpoint localityLbEndpoint, CompletableFuture future, + boolean register) { final EndpointIdentifier identifier = EndpointIdentifier.of(localityLbEndpoint); batchUpdateTasks.compute(endpointName, (key, task) -> { @@ -103,7 +91,7 @@ void schedule(String group, String endpointName, String fileName, task = new BatchUpdateTask(group, key, fileName); } - task.addOperationAndSchedule(identifier, register, localityLbEndpoint, streamObserver); + task.addOperationAndSchedule(identifier, register, localityLbEndpoint, future); return task; }); } @@ -126,12 +114,13 @@ private final class BatchUpdateTask { } void addOperationAndSchedule(EndpointIdentifier identifier, boolean register, - LocalityLbEndpoint localityLbEndpoint, StreamObserver streamObserver) { + LocalityLbEndpoint localityLbEndpoint, + CompletableFuture future) { final PendingUpdate previous; lock.lock(); try { previous = pendingUpdates.put(identifier, - new PendingUpdate(register, localityLbEndpoint, streamObserver)); + new PendingUpdate(register, localityLbEndpoint, future)); if (scheduledFuture == null) { scheduledFuture = scheduler.schedule(this::flush, 3, TimeUnit.SECONDS); } @@ -140,12 +129,10 @@ void addOperationAndSchedule(EndpointIdentifier identifier, boolean register, } if (previous != null) { - try (SafeCloseable ignored = RequestContextUtil.pop()) { - previous.streamObserver.onError( - Status.ABORTED - .withDescription("Aborted due to a new update for the same endpoint") - .asRuntimeException()); - } + previous.future.complete( + XdsResourceManager.errorResponse( + HttpStatus.CONFLICT, + "Aborted due to a new update for the same endpoint")); } } @@ -183,19 +170,28 @@ private void flush() { } } - final Repository repository = xdsResourceManager.xdsProject().repos().get(group); - final String altFileName = alternativeFileName(fileName); + final Repository repository; + try { + repository = xdsResourceManager.xdsProject().repos().get(group); + } catch (Exception e) { + copied.forEach(u -> u.future.complete( + XdsResourceManager.errorResponse(HttpStatus.NOT_FOUND, + "Group not found: " + group))); + return; + } + final String altFileName = XdsResourceManager.alternativeFileName(fileName); repository.find(Revision.HEAD, fileName + ',' + altFileName, FIND_ONE_WITHOUT_CONTENT) .handle((entries, cause) -> { if (cause != null) { - copied.forEach(pendingUpdate -> pendingUpdate.streamObserver.onError(cause)); + final Throwable peeled = Exceptions.peel(cause); + copied.forEach(u -> u.future.complete( + XdsResourceManager.errorResponse(HttpStatus.INTERNAL_SERVER_ERROR, peeled))); return null; } if (entries.isEmpty()) { - final StatusRuntimeException runtimeException = - Status.NOT_FOUND.withDescription("Resource not found: " + fileName) - .asRuntimeException(); - copied.forEach(pendingUpdate -> pendingUpdate.streamObserver.onError(runtimeException)); + copied.forEach(u -> u.future.complete( + XdsResourceManager.errorResponse(HttpStatus.NOT_FOUND, + "Resource not found: " + fileName))); return null; } @@ -234,8 +230,9 @@ private void flush() { toDeregister, copied); return null; } - copied.forEach(pendingUpdate -> pendingUpdate - .streamObserver.onError(peeled)); + copied.forEach(u -> u.future.complete( + XdsResourceManager.errorResponse( + HttpStatus.INTERNAL_SERVER_ERROR, peeled))); return null; } completeUpdates(copied); @@ -261,8 +258,16 @@ private void executeYamlTransform(String commitMessage, List .handle((result, cause) -> { if (cause != null) { final Throwable peeled = Exceptions.peel(cause); + if (peeled instanceof EntryNotFoundException) { + copied.forEach(u -> u.future.complete( + XdsResourceManager.errorResponse( + HttpStatus.NOT_FOUND, peeled))); + return null; + } if (!(peeled instanceof RedundantChangeException)) { - copied.forEach(u -> u.streamObserver.onError(peeled)); + copied.forEach(u -> u.future.complete( + XdsResourceManager.errorResponse( + HttpStatus.INTERNAL_SERVER_ERROR, peeled))); return null; } } @@ -272,29 +277,32 @@ private void executeYamlTransform(String commitMessage, List } private void completeUpdates(List copied) { - copied.forEach(pendingUpdate -> { - final StreamObserver streamObserver = pendingUpdate.streamObserver; + for (PendingUpdate pendingUpdate : copied) { if (pendingUpdate.register) { - //noinspection unchecked - ((StreamObserver) streamObserver).onNext(pendingUpdate.endpoint); + try { + pendingUpdate.future.complete( + XdsResourceManager.toYamlResponse(pendingUpdate.endpoint)); + } catch (IOException e) { + pendingUpdate.future.complete( + XdsResourceManager.errorResponse(HttpStatus.INTERNAL_SERVER_ERROR, e)); + } } else { - //noinspection unchecked - ((StreamObserver) streamObserver).onNext(Empty.getDefaultInstance()); + pendingUpdate.future.complete(HttpResponse.of(HttpStatus.OK)); } - streamObserver.onCompleted(); - }); + } } } private static class PendingUpdate { final boolean register; final LocalityLbEndpoint endpoint; - final StreamObserver streamObserver; + final CompletableFuture future; - PendingUpdate(boolean register, LocalityLbEndpoint endpoint, StreamObserver streamObserver) { + PendingUpdate(boolean register, LocalityLbEndpoint endpoint, + CompletableFuture future) { this.register = register; this.endpoint = endpoint; - this.streamObserver = streamObserver; + this.future = future; } } @@ -303,7 +311,8 @@ private static final class BatchUpdateTransformer implements BiFunction toRegister; private final List toDeregister; - BatchUpdateTransformer(List toRegister, List toDeregister) { + BatchUpdateTransformer(List toRegister, + List toDeregister) { this.toRegister = toRegister; this.toDeregister = toDeregister; } @@ -359,12 +368,14 @@ public JsonNode apply(Revision revision, JsonNode oldJsonNode) { } } + // TransformingChangesApplier parses the stored YAML via Jackson before passing + // oldJsonNode to this transformer, which strips all user comments regardless. + // Re-serializing the full proto here is therefore equivalent in practice. return toJsonNode(builder); } private static ClusterLoadAssignment.Builder toClusterLoadAssignmentBuilder(JsonNode oldJsonNode) { - final Builder clusterLoadAssignmentBuilder = - ClusterLoadAssignment.newBuilder(); + final Builder clusterLoadAssignmentBuilder = ClusterLoadAssignment.newBuilder(); try { JSON_MESSAGE_MARSHALLER.mergeValue(oldJsonNode.traverse(), clusterLoadAssignmentBuilder); } catch (Throwable t) { @@ -381,34 +392,29 @@ private static ClusterLoadAssignment.Builder toClusterLoadAssignmentBuilder(Json */ private static int findLocalityAndPriorityIndex(Builder clusterLoadAssignmentBuilder, LocalityLbEndpoint localityLbEndpoint) { - int sameLocalityIndex = -1; - final List localityLbEndpointsList = clusterLoadAssignmentBuilder.getEndpointsList(); for (int i = 0; i < localityLbEndpointsList.size(); i++) { final LocalityLbEndpoints localityLbEndpoints = localityLbEndpointsList.get(i); if (localityLbEndpoints.getLocality().equals(localityLbEndpoint.getLocality()) && localityLbEndpoints.getPriority() == localityLbEndpoint.getPriority()) { - sameLocalityIndex = i; - break; + return i; } } - return sameLocalityIndex; + return -1; } private static int findLbEndpointIndex(LocalityLbEndpoints targetLocalityLbEndpoints, LocalityLbEndpoint localityLbEndpoint) { - int sameLbEndpointIndex = -1; final List lbEndpointsList = targetLocalityLbEndpoints.getLbEndpointsList(); for (int i = 0; i < lbEndpointsList.size(); i++) { final LbEndpoint lbEndpoint = lbEndpointsList.get(i); if (lbEndpoint.getEndpoint().getAddress().equals( localityLbEndpoint.getLbEndpoint().getEndpoint().getAddress())) { - sameLbEndpointIndex = i; - break; + return i; } } - return sameLbEndpointIndex; + return -1; } private static JsonNode toJsonNode(ClusterLoadAssignment.Builder clusterLoadAssignmentBuilder) { diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/group/v1/XdsGroupService.java b/xds/src/main/java/com/linecorp/centraldogma/xds/group/v1/XdsGroupService.java index 16b0e162d..746e3f53b 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/group/v1/XdsGroupService.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/group/v1/XdsGroupService.java @@ -15,18 +15,22 @@ */ package com.linecorp.centraldogma.xds.group.v1; -import static com.linecorp.centraldogma.server.internal.admin.auth.AuthUtil.currentAuthor; +import static com.linecorp.centraldogma.internal.Util.PROJECT_AND_REPO_NAME_PATTERN; import static com.linecorp.centraldogma.server.internal.admin.auth.AuthUtil.currentUser; import static com.linecorp.centraldogma.server.internal.admin.auth.AuthUtil.getAuthor; import static com.linecorp.centraldogma.server.internal.api.RepositoryServiceUtil.createRepository; import static com.linecorp.centraldogma.server.internal.api.RepositoryServiceUtil.removeRepository; import static com.linecorp.centraldogma.server.internal.storage.InternalProjectConstants.INTERNAL_PROJECT_XDS; -import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.checkGroupId; -import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.removePrefix; +import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.errorResponse; -import com.google.protobuf.Empty; +import java.util.concurrent.CompletableFuture; +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.util.Exceptions; +import com.linecorp.armeria.server.annotation.Delete; +import com.linecorp.armeria.server.annotation.Param; +import com.linecorp.armeria.server.annotation.Post; import com.linecorp.centraldogma.common.RepositoryExistsException; import com.linecorp.centraldogma.common.RepositoryRole; import com.linecorp.centraldogma.server.command.CommandExecutor; @@ -34,15 +38,11 @@ import com.linecorp.centraldogma.server.metadata.ProjectMetadata; import com.linecorp.centraldogma.server.metadata.User; import com.linecorp.centraldogma.server.storage.project.Project; -import com.linecorp.centraldogma.xds.group.v1.XdsGroupServiceGrpc.XdsGroupServiceImplBase; - -import io.grpc.Status; -import io.grpc.stub.StreamObserver; /** - * An {@link XdsGroupServiceImplBase} implementation that provides methods to manage XDS groups. + * Annotated service object for managing xDS groups. */ -public final class XdsGroupService extends XdsGroupServiceImplBase { +public final class XdsGroupService { private final Project xdsProject; private final CommandExecutor commandExecutor; @@ -57,76 +57,80 @@ public XdsGroupService(Project xdsProject, CommandExecutor commandExecutor, Meta this.mds = mds; } - @Override - public void createGroup(CreateGroupRequest request, - StreamObserver responseObserver) { - final String groupId = request.getGroupId(); - checkGroupId(groupId); + /** + * POST /xds/groups + * + *

Creates a new xDS group. + */ + @Post("/xds/groups") + public CompletableFuture createGroup(@Param("group_id") String groupId) { + if (!PROJECT_AND_REPO_NAME_PATTERN.matcher(groupId).matches()) { + return CompletableFuture.completedFuture( + errorResponse(HttpStatus.BAD_REQUEST, "Invalid group ID: " + groupId)); + } + if (Project.isInternalRepo(groupId)) { + return CompletableFuture.completedFuture( + errorResponse(HttpStatus.FORBIDDEN, "Cannot create internal repository: " + groupId)); + } if (xdsProject.repos().exists(groupId)) { - throw alreadyExistsException(groupId); + return CompletableFuture.completedFuture( + errorResponse(HttpStatus.CONFLICT, "Group already exists: " + groupId)); + } + final User createUser = currentUser(); + if (createUser == null) { + return CompletableFuture.completedFuture( + errorResponse(HttpStatus.UNAUTHORIZED, "Authentication required")); } - createRepository(commandExecutor, mds, currentAuthor(), INTERNAL_PROJECT_XDS, groupId, false, null) + return createRepository(commandExecutor, mds, getAuthor(createUser), INTERNAL_PROJECT_XDS, groupId, + false, null) .handle((unused, cause) -> { if (cause != null) { final Throwable peeled = Exceptions.peel(cause); if (peeled instanceof RepositoryExistsException) { - responseObserver.onError(alreadyExistsException(groupId)); - } else { - responseObserver.onError( - Status.INTERNAL.withCause(peeled).asRuntimeException()); + return errorResponse(HttpStatus.CONFLICT, + "Group already exists: " + groupId); } - return null; + return errorResponse(HttpStatus.INTERNAL_SERVER_ERROR, peeled); } - responseObserver.onNext(Group.newBuilder().setName("groups/" + groupId).build()); - responseObserver.onCompleted(); - return null; + return HttpResponse.of(HttpStatus.OK); }); } - private static RuntimeException alreadyExistsException(String groupName) { - return Status.ALREADY_EXISTS.withDescription("Group already exists: " + groupName) - .asRuntimeException(); - } - - @Override - public void deleteGroup(DeleteGroupRequest request, StreamObserver responseObserver) { - final String groupName = request.getName(); - final String name = removePrefix("groups/", groupName); - if (!xdsProject.repos().exists(name)) { - throw Status.NOT_FOUND.withDescription("Group does not exist: " + groupName) - .asRuntimeException(); + /** + * DELETE /xds/groups/{group_name} + * + *

Removes an xDS group. + */ + @Delete("/xds/groups/{group_name}") + public CompletableFuture deleteGroup(@Param("group_name") String groupName) { + if (!xdsProject.repos().exists(groupName)) { + return CompletableFuture.completedFuture( + errorResponse(HttpStatus.NOT_FOUND, "Group not found: " + groupName)); } - if (Project.isInternalRepo(name)) { - throw Status.PERMISSION_DENIED.withDescription("Now allowed to delete " + groupName) - .asRuntimeException(); + if (Project.isInternalRepo(groupName)) { + return CompletableFuture.completedFuture( + errorResponse(HttpStatus.FORBIDDEN, "Cannot delete internal repository: " + groupName)); } - final User user = currentUser(); if (user == null) { - responseObserver.onError(Status.UNAUTHENTICATED.withDescription( - "You must be authenticated to delete " + groupName).asRuntimeException()); - return; + return CompletableFuture.completedFuture( + errorResponse(HttpStatus.UNAUTHORIZED, "Authentication required")); } final ProjectMetadata metadata = xdsProject.metadata(); - final RepositoryRole role = - metadata != null ? MetadataService.findRepositoryRole(metadata, name, user) : null; + // @xds is not the internal dogma project, so metadata is always initialized — never null. + assert metadata != null; + final RepositoryRole role = MetadataService.findRepositoryRole(metadata, groupName, user); if (role != RepositoryRole.ADMIN) { - responseObserver.onError(Status.PERMISSION_DENIED.withDescription( - "You must have the ADMIN repository role to delete " + groupName) - .asRuntimeException()); - return; + return CompletableFuture.completedFuture( + errorResponse(HttpStatus.FORBIDDEN, "No admin permission for group: " + groupName)); } - removeRepository(commandExecutor, mds, getAuthor(user), INTERNAL_PROJECT_XDS, name) - .handle((unused, cause1) -> { - if (cause1 != null) { - responseObserver.onError( - Status.INTERNAL.withDescription("Failed to delete " + groupName) - .withCause(cause1).asRuntimeException()); - return null; + return removeRepository(commandExecutor, mds, getAuthor(user), INTERNAL_PROJECT_XDS, groupName) + .handle((unused, cause) -> { + if (cause != null) { + return errorResponse(HttpStatus.INTERNAL_SERVER_ERROR, + Exceptions.peel(cause)); } - responseObserver.onNext(Empty.getDefaultInstance()); - responseObserver.onCompleted(); - return null; + return HttpResponse.of(HttpStatus.OK); }); } } diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/ControlPlaneService.java b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/ControlPlaneService.java index 45f053fee..a999df3f2 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/ControlPlaneService.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/ControlPlaneService.java @@ -30,7 +30,6 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.function.Function; -import org.curioswitch.common.protobuf.json.MessageMarshaller; import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,10 +41,10 @@ import com.google.common.collect.ImmutableSet; import com.google.protobuf.Message; +import com.linecorp.armeria.common.DependencyInjector; import com.linecorp.armeria.common.HttpRequest; import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.common.util.Exceptions; -import com.linecorp.armeria.internal.common.grpc.DefaultJsonMarshaller; import com.linecorp.armeria.server.HttpService; import com.linecorp.armeria.server.ServerBuilder; import com.linecorp.armeria.server.ServiceRequestContext; @@ -69,6 +68,7 @@ import com.linecorp.centraldogma.xds.cluster.v1.XdsClusterService; import com.linecorp.centraldogma.xds.endpoint.v1.XdsEndpointService; import com.linecorp.centraldogma.xds.group.v1.XdsGroupService; +import com.linecorp.centraldogma.xds.internal.RequiresXdsGroupRoleDecorator.RequiresXdsGroupRoleDecoratorFactory; import com.linecorp.centraldogma.xds.k8s.v1.XdsKubernetesService; import com.linecorp.centraldogma.xds.listener.v1.XdsListenerService; import com.linecorp.centraldogma.xds.route.v1.XdsRouteService; @@ -79,9 +79,6 @@ import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment; import io.envoyproxy.envoy.config.listener.v3.Listener; import io.envoyproxy.envoy.config.route.v3.RouteConfiguration; -import io.grpc.MethodDescriptor; -import io.grpc.MethodDescriptor.Marshaller; -import io.grpc.MethodDescriptor.PrototypeMarshaller; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.binder.jvm.ExecutorServiceMetrics; import io.netty.util.concurrent.DefaultThreadFactory; @@ -116,9 +113,9 @@ public final class ControlPlaneService extends XdsResourceWatchingService { private final ControlPlaneMetrics metrics; // Mutated only from the controlPlaneExecutor. private final CentralDogmaXdsResources centralDogmaXdsResources = new CentralDogmaXdsResources(); + private volatile boolean stop; @Nullable private volatile XdsEndpointService xdsEndpointService; - private volatile boolean stop; ControlPlaneService(Project xdsProject, MeterRegistry meterRegistry) { super(xdsProject, "xds.control.plane.service.", meterRegistry); @@ -152,43 +149,23 @@ void start(PluginInitContext pluginInitContext) { final ServerBuilder serverBuilder = pluginInitContext.serverBuilder(); serverBuilder.service(grpcService, optionalAppIdentityAuth(mds)); final XdsResourceManager xdsResourceManager = new XdsResourceManager(xdsProject(), commandExecutor); + serverBuilder.dependencyInjector( + DependencyInjector.ofSingletons( + new RequiresXdsGroupRoleDecoratorFactory(xdsProject())), false); + serverBuilder.annotatedService().pathPrefix("/api/v1").build( + new XdsClusterService(xdsResourceManager)); + serverBuilder.annotatedService().pathPrefix("/api/v1").build( + new XdsListenerService(xdsResourceManager)); + serverBuilder.annotatedService().pathPrefix("/api/v1").build( + new XdsRouteService(xdsResourceManager)); final XdsEndpointService xdsEndpointService = new XdsEndpointService(xdsResourceManager, controlPlaneExecutor); this.xdsEndpointService = xdsEndpointService; - final GrpcService xdsApplicationService = - GrpcService.builder() - .addService(new XdsGroupService(xdsProject(), commandExecutor, mds)) - .addService(new XdsListenerService(xdsResourceManager)) - .addService(new XdsRouteService(xdsResourceManager)) - .addService(new XdsClusterService(xdsResourceManager)) - .addService(xdsEndpointService) - .addService(new XdsKubernetesService(xdsResourceManager)) - .exceptionHandler(new ControlPlaneExceptionHandlerFunction()) - .jsonMarshallerFactory( - serviceDescriptor -> { - // Use JSON_MESSAGE_MARSHALLER not to parse Envoy extensions twice. - final MessageMarshaller.Builder builder = - JSON_MESSAGE_MARSHALLER.toBuilder(); - for (MethodDescriptor method : ImmutableList.copyOf( - serviceDescriptor.getMethods())) { - final Message reqPrototype = - marshallerPrototype(method.getRequestMarshaller()); - final Message resPrototype = - marshallerPrototype(method.getResponseMarshaller()); - if (reqPrototype != null) { - builder.register(reqPrototype); - } - if (resPrototype != null) { - builder.register(resPrototype); - } - } - return new DefaultJsonMarshaller(builder.build()); - }) - .enableHttpJsonTranscoding(true).build(); - // The global /api/v1/** route decorator in CentralDogma covers the HTTP-transcoded routes, but - // the native gRPC paths (e.g. /centraldogma.xds.*.v1.*Service/Method) bypass it. The per-service - // decorator ensures both paths require authentication. - serverBuilder.service(xdsApplicationService, pluginInitContext.authService()); + serverBuilder.annotatedService().pathPrefix("/api/v1").build(xdsEndpointService); + serverBuilder.annotatedService().pathPrefix("/api/v1").build( + new XdsGroupService(xdsProject(), commandExecutor, mds)); + serverBuilder.annotatedService().pathPrefix("/api/v1").build( + new XdsKubernetesService(xdsResourceManager)); // Endpoints (EDS) are not access-controlled: any authenticated user can read the endpoints of every // group regardless of its READ access. @@ -316,17 +293,6 @@ private static XdsResourceTypeDto toResourceTypeDto(String version, return new XdsResourceTypeDto(version, builder.build()); } - @Nullable - private static Message marshallerPrototype(Marshaller marshaller) { - if (marshaller instanceof MethodDescriptor.PrototypeMarshaller) { - final Object prototype = ((PrototypeMarshaller) marshaller).getMessagePrototype(); - if (prototype instanceof Message) { - return (Message) prototype; - } - } - return null; - } - @Override protected ScheduledExecutorService executor() { return controlPlaneExecutor; @@ -511,21 +477,20 @@ protected boolean isStopped() { void stop() { stop = true; metrics.onStopped(); + final XdsEndpointService xdsEndpointService = this.xdsEndpointService; - if (xdsEndpointService != null) { - if (xdsEndpointService.batchUpdateTaskSize() > 0) { - logger.info("Waiting for {} xDS endpoint batch update tasks to finish up to 5 seconds...", - xdsEndpointService.batchUpdateTaskSize()); - for (int i = 0; i < 5; i++) { - try { - if (xdsEndpointService.batchUpdateTaskSize() == 0) { - break; - } - Thread.sleep(1000); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); + if (xdsEndpointService != null && xdsEndpointService.batchUpdateTaskSize() > 0) { + logger.info("Waiting for {} xDS endpoint batch update tasks to finish up to 5 seconds...", + xdsEndpointService.batchUpdateTaskSize()); + for (int i = 0; i < 5; i++) { + try { + if (xdsEndpointService.batchUpdateTaskSize() == 0) { break; } + Thread.sleep(1000); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + break; } } } diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRole.java b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRole.java new file mode 100644 index 000000000..b71cf7ae9 --- /dev/null +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRole.java @@ -0,0 +1,46 @@ +/* + * Copyright 2024 LINE Corporation + * + * LINE 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. + */ +package com.linecorp.centraldogma.xds.internal; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import com.linecorp.armeria.server.annotation.Decorator; +import com.linecorp.armeria.server.annotation.DecoratorFactory; +import com.linecorp.centraldogma.common.RepositoryRole; +import com.linecorp.centraldogma.xds.internal.RequiresXdsGroupRoleDecorator.RequiresXdsGroupRoleDecoratorFactory; + +/** + * A {@link Decorator} to allow a request from a user granted the specified {@link RepositoryRole} + * on the xDS group identified by the {@code {group}} path variable. + */ +@DecoratorFactory(RequiresXdsGroupRoleDecoratorFactory.class) +@Retention(RetentionPolicy.RUNTIME) +@Target({ ElementType.TYPE, ElementType.METHOD }) +public @interface RequiresXdsGroupRole { + + /** + * The required {@link RepositoryRole}. Defaults to {@link RepositoryRole#WRITE}. + */ + RepositoryRole value(); + + /** + * A special parameter in order to specify the order of a {@link Decorator}. + */ + int order() default 0; +} diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRoleDecorator.java b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRoleDecorator.java new file mode 100644 index 000000000..b01920dcb --- /dev/null +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRoleDecorator.java @@ -0,0 +1,114 @@ +/* + * Copyright 2024 LINE Corporation + * + * LINE 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. + */ +package com.linecorp.centraldogma.xds.internal; + +import static com.google.common.base.Strings.isNullOrEmpty; +import static com.linecorp.centraldogma.internal.Util.PROJECT_AND_REPO_NAME_PATTERN; +import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.errorResponse; +import static java.util.Objects.requireNonNull; + +import java.util.function.Function; + +import com.linecorp.armeria.common.HttpRequest; +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.server.HttpService; +import com.linecorp.armeria.server.ServiceRequestContext; +import com.linecorp.armeria.server.SimpleDecoratingHttpService; +import com.linecorp.armeria.server.annotation.Decorator; +import com.linecorp.armeria.server.annotation.DecoratorFactoryFunction; +import com.linecorp.centraldogma.common.RepositoryRole; +import com.linecorp.centraldogma.server.internal.admin.auth.AuthUtil; +import com.linecorp.centraldogma.server.metadata.MetadataService; +import com.linecorp.centraldogma.server.metadata.ProjectMetadata; +import com.linecorp.centraldogma.server.metadata.User; +import com.linecorp.centraldogma.server.storage.project.Project; + +/** + * A {@link Decorator} that enforces the specified {@link RepositoryRole} on the xDS group identified + * by the {@code {group}} path variable. Performs the following checks in order: + *

    + *
  1. Group name matches the allowed pattern → 400 if not
    2. + *
  2. Group repository exists in the xDS project → 404 if not
    3. + *
  3. Request is authenticated → 401 if not
    4. + *
  4. User is a system administrator, or has the required role on the group → 403 if not
    5. + *
+ */ +public final class RequiresXdsGroupRoleDecorator extends SimpleDecoratingHttpService { + + private final Project xdsProject; + private final RepositoryRole requiredRole; + + RequiresXdsGroupRoleDecorator(HttpService delegate, Project xdsProject, + RepositoryRole requiredRole) { + super(delegate); + this.xdsProject = requireNonNull(xdsProject, "xdsProject"); + this.requiredRole = requireNonNull(requiredRole, "requiredRole"); + } + + @Override + public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exception { + final String group = ctx.pathParam("group"); + if (isNullOrEmpty(group)) { + return errorResponse(HttpStatus.BAD_REQUEST, "group path variable is missing"); + } + if (!PROJECT_AND_REPO_NAME_PATTERN.matcher(group).matches()) { + return errorResponse(HttpStatus.BAD_REQUEST, "Invalid group: " + group); + } + if (!xdsProject.repos().exists(group)) { + return errorResponse(HttpStatus.NOT_FOUND, "Group not found: " + group); + } + + final User user = AuthUtil.currentUser(ctx); + if (user == null) { + return errorResponse(HttpStatus.UNAUTHORIZED, "Authentication required"); + } + if (user.isSystemAdmin()) { + return unwrap().serve(ctx, req); + } + + final ProjectMetadata metadata = xdsProject.metadata(); + // @xds is not the internal dogma project, so metadata is always initialized — never null. + assert metadata != null; + final RepositoryRole role = + MetadataService.findRepositoryRole(metadata, group, user); + if (role != null && role.has(requiredRole)) { + return unwrap().serve(ctx, req); + } + return errorResponse(HttpStatus.FORBIDDEN, "No " + requiredRole + + " permission for group: " + group); + } + + /** + * A {@link DecoratorFactoryFunction} that creates a {@link RequiresXdsGroupRoleDecorator}. + */ + public static final class RequiresXdsGroupRoleDecoratorFactory + implements DecoratorFactoryFunction { + + private final Project xdsProject; + + public RequiresXdsGroupRoleDecoratorFactory(Project xdsProject) { + this.xdsProject = requireNonNull(xdsProject, "xdsProject"); + } + + @Override + public Function + newDecorator(RequiresXdsGroupRole parameter) { + return delegate -> new RequiresXdsGroupRoleDecorator( + delegate, xdsProject, parameter.value()); + } + } +} 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 2c93eeb0d..c3e5008f2 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 @@ -15,59 +15,58 @@ */ package com.linecorp.centraldogma.xds.internal; -import static com.linecorp.centraldogma.internal.Util.PROJECT_AND_REPO_NAME_PATTERN; import static com.linecorp.centraldogma.server.internal.storage.InternalProjectConstants.INTERNAL_PROJECT_XDS; import static com.linecorp.centraldogma.server.storage.repository.FindOptions.FIND_ONE_WITHOUT_CONTENT; import static java.util.Objects.requireNonNull; import java.io.IOException; +import java.io.StringReader; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.util.function.Consumer; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.function.Function; import java.util.regex.Pattern; import org.curioswitch.common.protobuf.json.MessageMarshaller; import org.jspecify.annotations.Nullable; import org.reflections.Reflections; import org.reflections.scanners.SubTypesScanner; +import org.yaml.snakeyaml.nodes.MappingNode; +import org.yaml.snakeyaml.nodes.Node; +import org.yaml.snakeyaml.nodes.NodeTuple; +import org.yaml.snakeyaml.nodes.ScalarNode; +import org.yaml.snakeyaml.nodes.SequenceNode; import com.fasterxml.jackson.databind.JsonNode; import com.google.common.collect.ImmutableList; -import com.google.protobuf.Empty; import com.google.protobuf.GeneratedMessageV3; import com.google.protobuf.Message; +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.util.Exceptions; import com.linecorp.centraldogma.common.Author; import com.linecorp.centraldogma.common.Change; -import com.linecorp.centraldogma.common.ChangeConflictException; import com.linecorp.centraldogma.common.Markup; import com.linecorp.centraldogma.common.RedundantChangeException; -import com.linecorp.centraldogma.common.RepositoryRole; import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.internal.Jackson; +import com.linecorp.centraldogma.internal.Yaml; import com.linecorp.centraldogma.server.command.Command; import com.linecorp.centraldogma.server.command.CommandExecutor; -import com.linecorp.centraldogma.server.internal.admin.auth.AuthUtil; -import com.linecorp.centraldogma.server.metadata.MetadataService; -import com.linecorp.centraldogma.server.metadata.ProjectMetadata; -import com.linecorp.centraldogma.server.metadata.User; import com.linecorp.centraldogma.server.storage.project.Project; import com.linecorp.centraldogma.server.storage.repository.Repository; -import com.linecorp.centraldogma.xds.endpoint.v1.DeregisterLocalityLbEndpointRequest; -import com.linecorp.centraldogma.xds.endpoint.v1.RegisterLocalityLbEndpointRequest; -import com.linecorp.centraldogma.xds.group.v1.CreateGroupRequest; -import com.linecorp.centraldogma.xds.k8s.v1.CreateKubernetesEndpointAggregatorRequest; -import com.linecorp.centraldogma.xds.k8s.v1.DeleteKubernetesEndpointAggregatorRequest; -import com.linecorp.centraldogma.xds.k8s.v1.UpdateKubernetesEndpointAggregatorRequest; +import com.linecorp.centraldogma.xds.endpoint.v1.LocalityLbEndpoint; +import com.linecorp.centraldogma.xds.k8s.v1.KubernetesEndpointAggregator; import io.envoyproxy.envoy.config.cluster.v3.Cluster; import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment; import io.envoyproxy.envoy.config.listener.v3.Listener; import io.envoyproxy.envoy.config.route.v3.RouteConfiguration; import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager; -import io.grpc.Status; -import io.grpc.stub.StreamObserver; public final class XdsResourceManager { @@ -80,21 +79,19 @@ public final class XdsResourceManager { public static final Pattern LEGACY_RESOURCE_ID_PATTERN = Pattern.compile('^' + LEGACY_RESOURCE_ID_PATTERN_STRING + '$'); + public static final MediaType MEDIA_TYPE_YAML = MediaType.parse("application/yaml"); + public static final MessageMarshaller JSON_MESSAGE_MARSHALLER; static { final MessageMarshaller.Builder builder = MessageMarshaller.builder().omittingInsignificantWhitespace(true); - builder.register(CreateGroupRequest.getDefaultInstance()) - .register(Listener.getDefaultInstance()) + builder.register(Listener.getDefaultInstance()) .register(Cluster.getDefaultInstance()) .register(ClusterLoadAssignment.getDefaultInstance()) - .register(RegisterLocalityLbEndpointRequest.getDefaultInstance()) - .register(DeregisterLocalityLbEndpointRequest.getDefaultInstance()) .register(RouteConfiguration.getDefaultInstance()) - .register(CreateKubernetesEndpointAggregatorRequest.getDefaultInstance()) - .register(UpdateKubernetesEndpointAggregatorRequest.getDefaultInstance()) - .register(DeleteKubernetesEndpointAggregatorRequest.getDefaultInstance()); + .register(KubernetesEndpointAggregator.getDefaultInstance()) + .register(LocalityLbEndpoint.getDefaultInstance()); envoyExtension(builder); JSON_MESSAGE_MARSHALLER = builder.build(); } @@ -119,22 +116,6 @@ private static boolean hasGetDefaultInstanceMethod(Class clazz) { } } - public static String removePrefix(String prefix, String name) { - if (!name.startsWith(prefix)) { - throw Status.INVALID_ARGUMENT.withDescription(name + " does not start with prefix: " + prefix) - .asRuntimeException(); - } - return name.substring(prefix.length()); - } - - public static void checkGroupId(String groupId) { - if (!PROJECT_AND_REPO_NAME_PATTERN.matcher(groupId).matches()) { - throw Status.INVALID_ARGUMENT.withDescription("Invalid group id: " + groupId + - " (expected: " + PROJECT_AND_REPO_NAME_PATTERN + ')') - .asRuntimeException(); - } - } - private final Project xdsProject; private final CommandExecutor commandExecutor; @@ -154,187 +135,267 @@ public CommandExecutor commandExecutor() { return commandExecutor; } - public void checkWritePermission(String group) { - checkGroupId(group); - if (!xdsProject.repos().exists(group)) { - throw Status.NOT_FOUND.withDescription("Group not found: " + group).asRuntimeException(); - } - checkWritePermission0(group); + public static String fileName(String group, String resourceName) { + // Remove groups/{group} + return resourceName.substring(7 + group.length()) + ".yaml"; + } + + public static HttpResponse errorResponse(HttpStatus status, String message) { + return HttpResponse.of(status, MediaType.PLAIN_TEXT_UTF_8, message); + } + + public static HttpResponse errorResponse(HttpStatus status, Throwable cause) { + final String message = cause.getMessage(); + return errorResponse(status, message != null ? message : cause.toString()); + } + + public static HttpResponse toYamlResponse(Message resource) throws IOException { + return HttpResponse.of(MEDIA_TYPE_YAML, toYamlBodyString(resource)); + } + + public static HttpResponse toYamlResponse(String yaml) { + return HttpResponse.of(MEDIA_TYPE_YAML, yaml); + } + + public static String toYamlBodyString(Message resource) throws IOException { + final String json = JSON_MESSAGE_MARSHALLER.writeValueAsString(resource); + final JsonNode node = Jackson.readTree(json); + return Yaml.writeValueAsString(node); + } + + @SuppressWarnings("unchecked") + public static T parseYaml(String body, Message.Builder builder) throws IOException { + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(body).traverse(), builder); + return (T) builder.build(); } - private void checkWritePermission0(String group) { - final User user = AuthUtil.currentUserOrNull(); - if (user == null) { - throw Status.UNAUTHENTICATED.withDescription( - "You must be authenticated to modify resources in the group: " + group) - .asRuntimeException(); + // Matches a snake_case identifier: at least one underscore, starts with lowercase. + // Used to detect mapping keys that need camelCase conversion. + private static final Pattern SNAKE_CASE_IDENTIFIER = + Pattern.compile("[a-z][a-z0-9]*(?:_[a-z0-9]+)+"); + + /** + * Converts all snake_case mapping keys in the given YAML string to camelCase, preserving + * values, comments, block scalars, and formatting. Uses snakeyaml's AST to locate real + * mapping keys, so block-scalar content lines are never mistaken for keys. + * + *

Call this on user-supplied bodies before storing them so that all stored YAML uses + * camelCase keys consistently, matching what the proto3 JSON serializer produces. + */ + public static String normalizeYamlKeys(String yaml) { + if (!yaml.contains("_")) { + return yaml; } - if (user.isSystemAdmin()) { - return; + final Node root = composeYaml(yaml); + if (root == null) { + return yaml; } - final ProjectMetadata metadata = xdsProject.metadata(); - if (metadata != null) { - final RepositoryRole role = MetadataService.findRepositoryRole(metadata, group, user); - if (role != null && role.has(RepositoryRole.WRITE)) { - return; - } + final List ranges = new ArrayList<>(); + collectSnakeCaseKeyRanges(root, ranges); + if (ranges.isEmpty()) { + return yaml; + } + // Replace from end to start so earlier offsets stay valid. + ranges.sort((a, b) -> Integer.compare(b[0], a[0])); + final StringBuilder sb = new StringBuilder(yaml); + for (int[] range : ranges) { + sb.replace(range[0], range[1], snakeToCamel(yaml.substring(range[0], range[1]))); } - throw Status.PERMISSION_DENIED.withDescription( - "You must have the WRITE repository role to modify resources in the group: " + group) - .asRuntimeException(); + return sb.toString(); } - public void push( - StreamObserver responseObserver, String group, String resourceName, String fileName, - String summary, T resource, Author author, boolean create) { - if (create) { - // Before attempting to create, verify that neither the requested file nor its alternative - // extension (.json ↔ .yaml) already exists, so a migrated .yaml resource is not silently - // shadowed by a newly created .json file. - final Repository repository = xdsProject.repos().get(group); - final String altFileName = alternativeFileName(fileName); - repository.find(Revision.HEAD, fileName + ',' + altFileName, FIND_ONE_WITHOUT_CONTENT) - .handle((entries, cause) -> { - if (cause != null) { - responseObserver.onError(cause); - return null; + private static void collectSnakeCaseKeyRanges(Node node, List ranges) { + if (node instanceof MappingNode) { + for (NodeTuple tuple : ((MappingNode) node).getValue()) { + final Node keyNode = tuple.getKeyNode(); + if (keyNode instanceof ScalarNode) { + final String key = ((ScalarNode) keyNode).getValue(); + if (SNAKE_CASE_IDENTIFIER.matcher(key).matches()) { + final int start = keyNode.getStartMark().getIndex(); + // Use key.length() rather than endMark to avoid trailing-whitespace + // ambiguity (e.g. "key : value" where the space before ':' is trimmed + // from the value but may or may not be included in endMark). + ranges.add(new int[] { start, start + key.length() }); + } } - if (!entries.isEmpty()) { - responseObserver.onError( - Status.ALREADY_EXISTS - .withDescription("Resource already exists: " + resourceName) - .asRuntimeException()); - return null; + // Recurse into the value — never into ScalarNode leaves (block scalar content). + collectSnakeCaseKeyRanges(tuple.getValueNode(), ranges); + } + } else if (node instanceof SequenceNode) { + for (Node item : ((SequenceNode) node).getValue()) { + collectSnakeCaseKeyRanges(item, ranges); + } + } + } + + private static String snakeToCamel(String snake) { + final StringBuilder sb = new StringBuilder(snake.length()); + boolean upper = false; + for (int i = 0; i < snake.length(); i++) { + final char c = snake.charAt(i); + if (c == '_') { + upper = true; + } else { + sb.append(upper ? Character.toUpperCase(c) : c); + upper = false; + } + } + return sb.toString(); + } + + /** + * Injects or replaces a top-level YAML field in the given YAML string, preserving all other + * content including comments. If {@code fieldName} is not found as a top-level key, it is + * prepended. Uses snakeyaml's AST so that block-scalar values (e.g. {@code name: |}) are + * replaced in their entirety — not just the indicator line. + * + *

Call {@link #normalizeYamlKeys(String)} before this method so that the key name is + * already in camelCase. + */ + public static String injectYamlField(String yaml, String fieldName, String fieldValue) { + final Node root = composeYaml(yaml); + if (root instanceof MappingNode) { + for (NodeTuple tuple : ((MappingNode) root).getValue()) { + final Node keyNode = tuple.getKeyNode(); + if (keyNode instanceof ScalarNode && + fieldName.equals(((ScalarNode) keyNode).getValue())) { + final int keyStart = keyNode.getStartMark().getIndex(); + // End of the value node; skip one trailing newline so we don't leave a blank + // line when the value's endMark lands on the '\n' that separates entries. + int valueEnd = tuple.getValueNode().getEndMark().getIndex(); + if (valueEnd < yaml.length() && yaml.charAt(valueEnd) == '\n') { + valueEnd++; + } + return yaml.substring(0, keyStart) + + fieldName + ": " + fieldValue + '\n' + + yaml.substring(valueEnd); } - doPush(responseObserver, group, resourceName, fileName, summary, resource, author, true, null); - return null; - }); - return; + } } - doPush(responseObserver, group, resourceName, fileName, summary, resource, author, false, null); + return fieldName + ": " + fieldValue + '\n' + yaml; } - private void doPush( - StreamObserver responseObserver, String group, String resourceName, String fileName, - String summary, T resource, Author author, boolean create, - @Nullable String legacyFileToRemove) { - final Change change; + @Nullable + private static Node composeYaml(String yaml) { try { - final String jsonText = JSON_MESSAGE_MARSHALLER.writeValueAsString(resource); - final JsonNode jsonNode = Jackson.readTree(jsonText); - if (create) { - change = Change.ofJsonPatch(fileName, null, jsonNode); - } else if (fileName.endsWith(".yaml")) { - change = Change.ofYamlUpsert(fileName, jsonNode); - } else { - change = Change.ofJsonUpsert(fileName, jsonNode); + return new org.yaml.snakeyaml.Yaml().compose(new StringReader(yaml)); + } catch (Exception ignored) { + return null; + } + } + + public CompletableFuture push( + String group, String resourceName, String fileName, String summary, + Author author, boolean create, String originalBody) { + if (create) { + final Repository repository; + try { + repository = xdsProject.repos().get(group); + } catch (Exception e) { + return CompletableFuture.completedFuture( + errorResponse(HttpStatus.NOT_FOUND, "Group not found: " + group)); } - } catch (IOException e) { - // This could happen when the message has a type that isn't registered to JSON_MESSAGE_MARSHALLER. - responseObserver.onError(Status.INTERNAL.withCause(new IllegalStateException( - "failed to convert message to JSON: " + resource, e)).asRuntimeException()); - return; + final String altFileName = alternativeFileName(fileName); + // Note: There is a TOCTOU race between this check and the subsequent ofYamlUpsert — + // two concurrent creates can both pass this check and the second will silently overwrite + // the first. This is acceptable for xDS resources where concurrent creates of the same + // resource are extremely rare. + return repository.find(Revision.HEAD, fileName + ',' + altFileName, FIND_ONE_WITHOUT_CONTENT) + .thenCompose(entries -> { + if (!entries.isEmpty()) { + return CompletableFuture.completedFuture( + errorResponse(HttpStatus.CONFLICT, + "Resource already exists: " + resourceName)); + } + return doPush(group, fileName, summary, author, + true, null, originalBody); + }); } + return doPush(group, fileName, summary, author, false, null, originalBody); + } + + private CompletableFuture doPush( + String group, String fileName, String summary, + Author author, boolean create, @Nullable String legacyFileToRemove, String originalBody) { + // Store the original YAML body as-is (server-set fields are injected by the caller before + // this method is invoked). Respond with the same body so the client sees exactly what is stored. + final Change change = Change.ofYamlUpsert(fileName, originalBody); final ImmutableList> changes = legacyFileToRemove != null ? ImmutableList.of(Change.ofRemoval(legacyFileToRemove), change) : ImmutableList.of(change); - commandExecutor.execute(Command.push(author, INTERNAL_PROJECT_XDS, group, Revision.HEAD, - summary, "", Markup.PLAINTEXT, changes)) - .handle((unused, cause) -> { - if (cause != null) { - final Throwable peeled = Exceptions.peel(cause); - if (create && peeled instanceof ChangeConflictException) { - responseObserver.onError( - Status.ALREADY_EXISTS - .withCause(peeled) - .withDescription("Resource already exists: " + resourceName) - .asRuntimeException()); - return null; - } - if (!create && peeled instanceof RedundantChangeException) { - // Updating with the same resource. Return the resource as is. - responseObserver.onNext(resource); - responseObserver.onCompleted(); - return null; - } - responseObserver.onError(cause); - return null; - } - responseObserver.onNext(resource); - responseObserver.onCompleted(); - return null; - }); + return commandExecutor.execute(Command.push(author, INTERNAL_PROJECT_XDS, group, Revision.HEAD, + summary, "", Markup.PLAINTEXT, changes)) + .handle((unused, cause) -> { + if (cause != null) { + final Throwable peeled = Exceptions.peel(cause); + if (!create && peeled instanceof RedundantChangeException) { + return toYamlResponse(originalBody); + } + return errorResponse(HttpStatus.INTERNAL_SERVER_ERROR, peeled); + } + return toYamlResponse(originalBody); + }); } - public static String fileName(String group, String resourceName) { - // Remove groups/{group} - return resourceName.substring(7 + group.length()) + ".yaml"; + public CompletableFuture update( + String group, String resourceName, String summary, Author author, String originalBody) { + return update(group, resourceName, fileName(group, resourceName), summary, author, originalBody); } - public void update(StreamObserver responseObserver, String group, - String resourceName, String summary, T resource, Author author) { - update(responseObserver, group, resourceName, fileName(group, resourceName), summary, resource, author); - } - - public void update(StreamObserver responseObserver, String group, - String resourceName, String fileName, String summary, T resource, - Author author) { - updateOrDelete(responseObserver, group, resourceName, fileName, resolvedFileName -> { + public CompletableFuture update( + String group, String resourceName, String fileName, String summary, + Author author, String originalBody) { + return updateOrDelete(group, resourceName, fileName, resolvedFileName -> { final String legacyFileToRemove = resolvedFileName.endsWith(".json") ? resolvedFileName : null; final String targetFileName = legacyFileToRemove != null ? fileName : resolvedFileName; - doPush(responseObserver, group, resourceName, targetFileName, summary, resource, author, false, - legacyFileToRemove); + return doPush(group, targetFileName, summary, author, false, legacyFileToRemove, originalBody); }); } - public void delete(StreamObserver responseObserver, String group, - String resourceName, String summary, Author author) { - delete(responseObserver, group, resourceName, fileName(group, resourceName), summary, author); + public CompletableFuture delete( + String group, String resourceName, String summary, Author author) { + return delete(group, resourceName, fileName(group, resourceName), summary, author); } - public void delete(StreamObserver responseObserver, String group, - String resourceName, String fileName, String summary, Author author) { - updateOrDelete(responseObserver, group, resourceName, fileName, resolvedFileName -> + public CompletableFuture delete( + String group, String resourceName, String fileName, String summary, Author author) { + return updateOrDelete(group, resourceName, fileName, resolvedFileName -> commandExecutor.execute(Command.push(author, INTERNAL_PROJECT_XDS, group, Revision.HEAD, summary, "", Markup.PLAINTEXT, ImmutableList.of(Change.ofRemoval(resolvedFileName)))) .handle((unused, cause) -> { if (cause != null) { - responseObserver.onError( - Status.INTERNAL.withCause(cause).asRuntimeException()); - return null; + return errorResponse(HttpStatus.INTERNAL_SERVER_ERROR, + Exceptions.peel(cause)); } - responseObserver.onNext(Empty.getDefaultInstance()); - responseObserver.onCompleted(); - return null; + return HttpResponse.of(HttpStatus.OK); })); } - public void updateOrDelete(StreamObserver responseObserver, String group, String resourceName, - String fileName, Consumer taskProvider) { - final Repository repository = xdsProject.repos().get(group); - // Search for both the requested filename and its alternative extension (.json ↔ .yaml) - // to support files that may have been written in either format. + public CompletableFuture updateOrDelete( + String group, String resourceName, String fileName, + Function> taskProvider) { + final Repository repository; + try { + repository = xdsProject.repos().get(group); + } catch (Exception e) { + return CompletableFuture.completedFuture( + errorResponse(HttpStatus.NOT_FOUND, "Group not found: " + group)); + } final String altFileName = alternativeFileName(fileName); - repository.find(Revision.HEAD, fileName + ',' + altFileName, FIND_ONE_WITHOUT_CONTENT) - .handle((entries, cause) -> { - if (cause != null) { - responseObserver.onError(cause); - return null; - } - if (entries.isEmpty()) { - responseObserver.onError( - Status.NOT_FOUND.withDescription("Resource not found: " + resourceName) - .asRuntimeException()); - return null; - } - final String resolvedFileName = entries.keySet().iterator().next(); - taskProvider.accept(resolvedFileName); - return null; - }); + return repository.find(Revision.HEAD, fileName + ',' + altFileName, FIND_ONE_WITHOUT_CONTENT) + .thenCompose(entries -> { + if (entries.isEmpty()) { + return CompletableFuture.completedFuture( + errorResponse(HttpStatus.NOT_FOUND, + "Resource not found: " + resourceName)); + } + final String resolvedFileName = entries.keySet().iterator().next(); + return taskProvider.apply(resolvedFileName); + }); } - private static String alternativeFileName(String fileName) { + public static String alternativeFileName(String fileName) { if (fileName.endsWith(".json")) { return fileName.substring(0, fileName.length() - 5) + ".yaml"; } 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 de6d20ec0..e3110d99e 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 @@ -22,8 +22,8 @@ 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.fileName; -import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.removePrefix; +import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.ServiceLoader; @@ -31,6 +31,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Supplier; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -39,33 +40,38 @@ import org.slf4j.LoggerFactory; import com.google.common.collect.ImmutableList; -import com.google.protobuf.Empty; import com.linecorp.armeria.client.Endpoint; import com.linecorp.armeria.client.kubernetes.endpoints.KubernetesEndpointGroup; import com.linecorp.armeria.client.kubernetes.endpoints.KubernetesEndpointGroupBuilder; import com.linecorp.armeria.common.ContextAwareBlockingTaskExecutor; +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.util.Exceptions; import com.linecorp.armeria.server.ServiceRequestContext; import com.linecorp.armeria.server.annotation.Blocking; +import com.linecorp.armeria.server.annotation.Consumes; +import com.linecorp.armeria.server.annotation.Delete; +import com.linecorp.armeria.server.annotation.Param; +import com.linecorp.armeria.server.annotation.Post; +import com.linecorp.armeria.server.annotation.Put; import com.linecorp.centraldogma.common.Author; import com.linecorp.centraldogma.common.EntryNotFoundException; +import com.linecorp.centraldogma.common.RepositoryRole; import com.linecorp.centraldogma.server.internal.credential.AccessTokenCredential; import com.linecorp.centraldogma.server.storage.repository.MetaRepository; +import com.linecorp.centraldogma.xds.internal.RequiresXdsGroupRole; import com.linecorp.centraldogma.xds.internal.XdsResourceManager; -import com.linecorp.centraldogma.xds.k8s.v1.XdsKubernetesServiceGrpc.XdsKubernetesServiceImplBase; import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment; import io.envoyproxy.envoy.config.endpoint.v3.LocalityLbEndpoints; import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.ConfigBuilder; -import io.grpc.Status; -import io.grpc.stub.StreamObserver; /** - * A gRPC service that handles Kubernetes resources. + * Annotated service object for managing Kubernetes endpoint aggregators. */ -public final class XdsKubernetesService extends XdsKubernetesServiceImplBase { +public final class XdsKubernetesService { private static final Logger logger = LoggerFactory.getLogger(XdsKubernetesService.class); @@ -111,55 +117,206 @@ public XdsKubernetesService(XdsResourceManager xdsResourceManager) { this.xdsResourceManager = xdsResourceManager; } + /** + * POST /xds/groups/{group}/k8s/endpointAggregators + * + *

Creates a new Kubernetes endpoint aggregator. + */ @Blocking - @Override - public void createKubernetesEndpointAggregator( - CreateKubernetesEndpointAggregatorRequest request, - StreamObserver responseObserver) { - final String parent = request.getParent(); - final String group = removePrefix("groups/", parent); - xdsResourceManager.checkWritePermission(group); - final String aggregatorId = request.getAggregatorId(); + @Post("/xds/groups/{group}/k8s/endpointAggregators") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture createKubernetesEndpointAggregator( + @Param("group") String group, + @Param("aggregator_id") String aggregatorId, + @Param("summary") @Nullable String summary, + String body) { if (!RESOURCE_ID_PATTERN.matcher(aggregatorId).matches()) { - throw Status.INVALID_ARGUMENT.withDescription("Invalid aggregator_id: " + aggregatorId + - " (expected: " + RESOURCE_ID_PATTERN + ')') - .asRuntimeException(); + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid aggregator ID: " + aggregatorId)); + } + final String kubernetesEndpointName = + "groups/" + group + K8S_ENDPOINT_AGGREGATORS_DIRECTORY + aggregatorId; + final String clusterName = "groups/" + group + "/k8s/clusters/" + aggregatorId; + final KubernetesEndpointAggregator aggregator; + try { + aggregator = XdsResourceManager.parseYaml(body, KubernetesEndpointAggregator.newBuilder()); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); + } + final List kubernetesLocalityLbEndpointsList = + aggregator.getLocalityLbEndpointsList(); + if (kubernetesLocalityLbEndpointsList.isEmpty()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "locality_lb_endpoints must not be empty")); } + final Author author = currentAuthor(); + final String aggregatorFileName = K8S_ENDPOINT_AGGREGATORS_DIRECTORY + aggregatorId + ".yaml"; + final String createSummary = isNullOrEmpty(summary) ? + "Create kubernetes endpoint: " + kubernetesEndpointName : summary; + final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); + String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", + kubernetesEndpointName); + bodyToStore = XdsResourceManager.injectYamlField(bodyToStore, "clusterName", clusterName); + final String finalBodyToStore = bodyToStore; + return validateKubernetesEndpointAndPushHttp( + kubernetesLocalityLbEndpointsList, group, aggregatorFileName, + () -> xdsResourceManager.push(group, kubernetesEndpointName, + aggregatorFileName, createSummary, author, + true, finalBodyToStore)); + } - final String kubernetesEndpointName = parent + K8S_ENDPOINT_AGGREGATORS_DIRECTORY + aggregatorId; - final String clusterName = parent + "/k8s/clusters/" + aggregatorId; - final KubernetesEndpointAggregator aggregator = request.getKubernetesEndpointAggregator().toBuilder() - .setName(kubernetesEndpointName) - .setClusterName(clusterName) - .build(); + /** + * PUT /xds/groups/{group}/k8s/endpointAggregators/{aggregator_id} + * + *

Updates an existing Kubernetes endpoint aggregator. + */ + @Blocking + @Put("/xds/groups/{group}/k8s/endpointAggregators/{*aggregator_id}") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture updateKubernetesEndpointAggregator( + @Param("group") String group, + @Param("aggregator_id") String aggregatorId, + @Param("summary") @Nullable String summary, + String body) { + final String aggregatorName = "groups/" + group + K8S_ENDPOINT_AGGREGATORS_DIRECTORY + aggregatorId; + final Matcher matcher = K8S_ENDPOINT_AGGREGATORS_NAME_PATTERN.matcher(aggregatorName); + if (!matcher.matches()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid aggregator name: " + aggregatorName)); + } + final String clusterName = AGGREGATORS_REPLCACE_PATTERN.matcher(aggregatorName) + .replaceFirst("/clusters/"); + final KubernetesEndpointAggregator aggregator; + try { + aggregator = XdsResourceManager.parseYaml(body, KubernetesEndpointAggregator.newBuilder()); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); + } final List kubernetesLocalityLbEndpointsList = aggregator.getLocalityLbEndpointsList(); if (kubernetesLocalityLbEndpointsList.isEmpty()) { - throw Status.INVALID_ARGUMENT.withDescription("kubernetes locality lb endpoints are empty.") - .asRuntimeException(); + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "locality_lb_endpoints must not be empty")); } final Author author = currentAuthor(); - final String fileName = K8S_ENDPOINT_AGGREGATORS_DIRECTORY + aggregatorId + ".yaml"; - final String createSummary = - isNullOrEmpty(request.getSummary()) ? "Create kubernetes endpoint: " + kubernetesEndpointName - : request.getSummary(); - validateKubernetesEndpointAndPush( - responseObserver, kubernetesLocalityLbEndpointsList, group, fileName, - () -> xdsResourceManager.push( - responseObserver, group, kubernetesEndpointName, - fileName, createSummary, aggregator, author, true)); + final String updateSummary = isNullOrEmpty(summary) ? + "Update kubernetes endpoint aggregator: " + aggregatorName : summary; + // Both name and clusterName are server-derived; inject them so the stored YAML is always correct + // regardless of what the client sent in the body. + final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); + String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", aggregatorName); + bodyToStore = XdsResourceManager.injectYamlField(bodyToStore, "clusterName", clusterName); + final String finalBodyToStore = bodyToStore; + return validateKubernetesEndpointAndPushHttp( + kubernetesLocalityLbEndpointsList, group, fileName(group, aggregatorName), + () -> xdsResourceManager.update(group, aggregatorName, updateSummary, author, + finalBodyToStore)); + } + + /** + * DELETE /xds/groups/{group}/k8s/endpointAggregators/{aggregator_id} + * + *

Removes a Kubernetes endpoint aggregator. + */ + @Delete("/xds/groups/{group}/k8s/endpointAggregators/{*aggregator_id}") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture deleteKubernetesEndpointAggregator( + @Param("group") String group, + @Param("aggregator_id") String aggregatorId, + @Param("summary") @Nullable String summary) { + final String aggregatorName = "groups/" + group + K8S_ENDPOINT_AGGREGATORS_DIRECTORY + aggregatorId; + if (!K8S_ENDPOINT_AGGREGATORS_NAME_PATTERN.matcher(aggregatorName).matches()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid aggregator name: " + aggregatorName)); + } + final String deleteSummary = isNullOrEmpty(summary) ? + "Delete kubernetes endpoint aggregator: " + aggregatorName : summary; + return xdsResourceManager.delete(group, aggregatorName, deleteSummary, currentAuthor()); } - private void validateKubernetesEndpointAndPush( - StreamObserver responseObserver, + /** + * POST /xds/groups/{group}/k8s/endpointAggregators:preview + * + *

Previews the endpoints that would be resolved for a Kubernetes endpoint aggregator + * without persisting it. + */ + @Blocking + @Post("/xds/groups/{group}/k8s/endpointAggregators:preview") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture previewKubernetesEndpointAggregator( + @Param("group") String group, + String body) { + final KubernetesEndpointAggregator aggregator; + try { + aggregator = XdsResourceManager.parseYaml( + body, KubernetesEndpointAggregator.newBuilder()); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); + } + final List localityLbEndpointsList = + aggregator.getLocalityLbEndpointsList(); + if (localityLbEndpointsList.isEmpty()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "locality_lb_endpoints must not be empty")); + } + final MetaRepository metaRepository = xdsResourceManager.xdsProject().metaRepo(); + final ContextAwareBlockingTaskExecutor taskExecutor = + ServiceRequestContext.current().blockingTaskExecutor(); + + final List> futures = new ArrayList<>(); + for (KubernetesLocalityLbEndpoints localityLbEndpoints : localityLbEndpointsList) { + futures.add(resolvePreview(localityLbEndpoints, group, metaRepository, taskExecutor)); + } + + return CompletableFuture.allOf(futures.toArray(EMPTY_FUTURES)) + .handle((unused, cause) -> { + if (cause != null) { + return toErrorHttpResponse(cause); + } + final ClusterLoadAssignment.Builder cla = + ClusterLoadAssignment.newBuilder(); + if (!aggregator.getClusterName().isEmpty()) { + cla.setClusterName(aggregator.getClusterName()); + } + for (CompletableFuture future : futures) { + cla.addEndpoints(future.join()); + } + try { + return XdsResourceManager.toYamlResponse(cla.build()); + } catch (IOException e) { + return XdsResourceManager.errorResponse( + HttpStatus.INTERNAL_SERVER_ERROR, e); + } + }); + } + + private CompletableFuture validateKubernetesEndpointAndPushHttp( List kubernetesLocalityLbEndpointsList, - String group, String fileName, Runnable onSuccess) { + String group, String fileNameForLookup, + Supplier> onSuccess) { for (KubernetesLocalityLbEndpoints kubernetesLocalityLbEndpoints : kubernetesLocalityLbEndpointsList) { - validateMetadataMappings(kubernetesLocalityLbEndpoints.getWatcher()); + try { + validateMetadataMappings(kubernetesLocalityLbEndpoints.getWatcher()); + } catch (IllegalArgumentException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, e)); + } } - // Create a KubernetesEndpointGroup to check if the watcher is valid. - // We use KubernetesEndpointGroup for simplicity, but we will implement a custom implementation - // for better debugging and error handling in the future. final ContextAwareBlockingTaskExecutor taskExecutor = ServiceRequestContext.current().blockingTaskExecutor(); @@ -171,31 +328,21 @@ private void validateKubernetesEndpointAndPush( final ServiceEndpointWatcher watcher = kubernetesLocalityLbEndpoints.getWatcher(); final CompletableFuture endpointGroupFuture = createKubernetesEndpointGroup(watcher, xdsResourceManager.xdsProject().metaRepo(), - group, fileName, false); + group, fileNameForLookup, false); endpointGroupFuture.handle((kubernetesEndpointGroup, cause) -> { if (cause != null) { - cause = Exceptions.peel(cause); - if (cause instanceof IllegalArgumentException || cause instanceof EntryNotFoundException) { - future.completeExceptionally(Status.INVALID_ARGUMENT.withCause(cause) - .withDescription(cause.getMessage()) - .asRuntimeException()); - } else { - future.completeExceptionally(Status.INTERNAL.withCause(cause).asRuntimeException()); - } + future.completeExceptionally(Exceptions.peel(cause)); return null; } final AtomicBoolean completed = new AtomicBoolean(); final CompletableFuture> whenReady = kubernetesEndpointGroup.whenReady(); - // Use a schedule to time out the watcher creation until we implement a custom implementation. final ScheduledFuture scheduledFuture = taskExecutor.schedule(() -> { if (!completed.compareAndSet(false, true)) { return; } kubernetesEndpointGroup.closeAsync(); - future.completeExceptionally( - Status.INTERNAL.withDescription( - "Failed to retrieve k8s endpoints within 5 seconds. watcher: " + - watcher).asRuntimeException()); + future.completeExceptionally(new IllegalStateException( + "Failed to retrieve k8s endpoints within 5 seconds. watcher: " + watcher)); }, 5, TimeUnit.SECONDS); whenReady.handle((endpoints, cause1) -> { @@ -205,63 +352,125 @@ private void validateKubernetesEndpointAndPush( scheduledFuture.cancel(false); kubernetesEndpointGroup.closeAsync(); if (cause1 != null) { - // Specific types. - responseObserver.onError( - Status.INTERNAL.withDescription("Failed to retrieve k8s endpoints") - .withCause(cause1).asRuntimeException()); - return null; + future.completeExceptionally( + new IllegalStateException("Failed to retrieve k8s endpoints", cause1)); + } else { + logger.debug("Successfully retrieved k8s endpoints: {}, watcher: {}", + endpoints, watcher); + future.complete(null); } - logger.debug("Successfully retrieved k8s endpoints: {}, watcher: {}", endpoints, watcher); - future.complete(null); return null; }); return null; }); } - final CompletableFuture allOfFuture = - CompletableFuture.allOf(futures.toArray(EMPTY_FUTURES)); - allOfFuture.handle((unused, cause) -> { - if (cause != null) { - responseObserver.onError(cause); - } else { - onSuccess.run(); - } - return null; - }); + return CompletableFuture.allOf(futures.toArray(EMPTY_FUTURES)) + .handle((unused, cause) -> { + if (cause != null) { + return toErrorHttpResponse(cause); + } + return null; + }) + .thenCompose(errorResponse -> { + if (errorResponse != null) { + return CompletableFuture.completedFuture(errorResponse); + } + return onSuccess.get(); + }); + } + + private static HttpResponse toErrorHttpResponse(Throwable cause) { + final Throwable peeled = Exceptions.peel(cause); + if (peeled instanceof IllegalArgumentException || peeled instanceof EntryNotFoundException) { + return XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, peeled); + } + return XdsResourceManager.errorResponse(HttpStatus.INTERNAL_SERVER_ERROR, peeled); + } + + private static CompletableFuture resolvePreview( + KubernetesLocalityLbEndpoints localityLbEndpoints, String group, + MetaRepository metaRepository, ContextAwareBlockingTaskExecutor taskExecutor) { + final CompletableFuture result = new CompletableFuture<>(); + final ServiceEndpointWatcher watcher = localityLbEndpoints.getWatcher(); + createKubernetesEndpointGroup(watcher, metaRepository, group, "(preview)", false) + .handle((endpointGroup, cause) -> { + if (cause != null) { + result.completeExceptionally(Exceptions.peel(cause)); + return null; + } + final AtomicBoolean completed = new AtomicBoolean(); + final ScheduledFuture timeout = taskExecutor.schedule(() -> { + if (completed.compareAndSet(false, true)) { + endpointGroup.closeAsync(); + result.completeExceptionally(new IllegalStateException( + "Timed out after " + PREVIEW_TIMEOUT_SECONDS + + "s resolving '" + watcher.getServiceName() + '\'')); + } + }, PREVIEW_TIMEOUT_SECONDS, TimeUnit.SECONDS); + endpointGroup.whenReady().handle((endpoints, readyCause) -> { + if (!completed.compareAndSet(false, true)) { + return null; + } + timeout.cancel(false); + try { + if (readyCause != null) { + result.completeExceptionally(Exceptions.peel(readyCause)); + } else { + result.complete(toLocalityLbEndpoints(endpointGroup, localityLbEndpoints)); + } + } finally { + endpointGroup.closeAsync(); + } + return null; + }); + return null; + }); + return result; + } + + private static LocalityLbEndpoints toLocalityLbEndpoints( + KubernetesEndpointGroup endpointGroup, KubernetesLocalityLbEndpoints localityLbEndpoints) { + final LocalityLbEndpoints.Builder builder = LocalityLbEndpoints.newBuilder(); + if (localityLbEndpoints.hasLocality()) { + builder.setLocality(localityLbEndpoints.getLocality()); + } + if (localityLbEndpoints.hasLoadBalancingWeight()) { + builder.setLoadBalancingWeight(localityLbEndpoints.getLoadBalancingWeight()); + } + builder.setPriority(localityLbEndpoints.getPriority()); + KubernetesEndpointConverter.addLbEndpoints(builder, endpointGroup.endpoints(), + localityLbEndpoints.getWatcher()); + return builder.build(); } private static void validateMetadataMappings(ServiceEndpointWatcher watcher) { for (MetadataMapping mapping : watcher.getMetadataMappingList()) { if (mapping.getResourceType() == MetadataMapping.ResourceType.RESOURCE_TYPE_UNSPECIFIED) { - throw Status.INVALID_ARGUMENT.withDescription( - "resource_type must be specified in metadata_mapping: " + mapping) - .asRuntimeException(); + throw new IllegalArgumentException( + "resource_type must be specified in metadata_mapping: " + mapping); } if (mapping.getEntryType() == MetadataMapping.EntryType.ENTRY_TYPE_UNSPECIFIED) { - throw Status.INVALID_ARGUMENT.withDescription( - "entry_type must be specified in metadata_mapping: " + mapping) - .asRuntimeException(); + throw new IllegalArgumentException( + "entry_type must be specified in metadata_mapping: " + mapping); } switch (mapping.getSourceCase()) { case SOURCE_KEY: if (mapping.getSourceKey().isEmpty()) { - throw Status.INVALID_ARGUMENT.withDescription( - "source_key must not be empty in metadata_mapping: " + mapping) - .asRuntimeException(); + throw new IllegalArgumentException( + "source_key must not be empty in metadata_mapping: " + mapping); } break; case SOURCE_KEY_PREFIX: if (mapping.getSourceKeyPrefix().isEmpty()) { - throw Status.INVALID_ARGUMENT.withDescription( - "source_key_prefix must not be empty in metadata_mapping: " + mapping) - .asRuntimeException(); + throw new IllegalArgumentException( + "source_key_prefix must not be empty in metadata_mapping: " + mapping); } break; default: - throw Status.INVALID_ARGUMENT.withDescription( + throw new IllegalArgumentException( "either source_key or source_key_prefix must be set in metadata_mapping: " + - mapping).asRuntimeException(); + mapping); } } } @@ -355,166 +564,4 @@ private static CompletableFuture toConfig(Kubeconfig kubeconfig, MetaRep }); return future; } - - @Blocking - @Override - public void updateKubernetesEndpointAggregator( - UpdateKubernetesEndpointAggregatorRequest request, - StreamObserver responseObserver) { - final KubernetesEndpointAggregator aggregator = request.getKubernetesEndpointAggregator(); - final String aggregatorName = aggregator.getName(); - final String group = checkAggregatorName(aggregatorName).group(1); - xdsResourceManager.checkWritePermission(group); - final List kubernetesLocalityLbEndpointsList = - aggregator.getLocalityLbEndpointsList(); - if (kubernetesLocalityLbEndpointsList.isEmpty()) { - throw Status.INVALID_ARGUMENT.withDescription("kubernetes locality lb endpoints are empty.") - .asRuntimeException(); - } - - // Update the cluster name just in case it's mistakenly set by the user. - final KubernetesEndpointAggregator aggregator0 = aggregator.toBuilder().setClusterName( - AGGREGATORS_REPLCACE_PATTERN.matcher(aggregatorName).replaceFirst("/clusters/")).build(); - final Author author = currentAuthor(); - final String updateSummary = - isNullOrEmpty(request.getSummary()) ? "Update kubernetes endpoint aggregator: " + aggregatorName - : request.getSummary(); - validateKubernetesEndpointAndPush( - responseObserver, kubernetesLocalityLbEndpointsList, group, fileName(group, aggregatorName), - () -> xdsResourceManager.update( - responseObserver, group, aggregatorName, updateSummary, aggregator0, author)); - } - - private static Matcher checkAggregatorName(String aggregatorName) { - final Matcher matcher = K8S_ENDPOINT_AGGREGATORS_NAME_PATTERN.matcher(aggregatorName); - if (!matcher.matches()) { - throw Status.INVALID_ARGUMENT.withDescription( - "Invalid kubernetes endpoint aggregator name: " + aggregatorName + - " (expected: " + K8S_ENDPOINT_AGGREGATORS_NAME_PATTERN + ')') - .asRuntimeException(); - } - return matcher; - } - - @Override - public void deleteKubernetesEndpointAggregator(DeleteKubernetesEndpointAggregatorRequest request, - StreamObserver responseObserver) { - final String aggregatorName = request.getName(); - final String group = checkAggregatorName(aggregatorName).group(1); - xdsResourceManager.checkWritePermission(group); - final String deleteSummary = - isNullOrEmpty(request.getSummary()) ? "Delete kubernetes endpoint aggregator: " + aggregatorName - : request.getSummary(); - xdsResourceManager.delete(responseObserver, group, aggregatorName, deleteSummary, currentAuthor()); - } - - @Blocking - @Override - public void previewKubernetesEndpointAggregator( - PreviewKubernetesEndpointAggregatorRequest request, - StreamObserver responseObserver) { - final String parent = request.getParent(); - final String group = removePrefix("groups/", parent); - xdsResourceManager.checkWritePermission(group); - - final KubernetesEndpointAggregator aggregator = request.getKubernetesEndpointAggregator(); - final List localityLbEndpointsList = - aggregator.getLocalityLbEndpointsList(); - if (localityLbEndpointsList.isEmpty()) { - throw Status.INVALID_ARGUMENT.withDescription("kubernetes locality lb endpoints are empty.") - .asRuntimeException(); - } - - final MetaRepository metaRepository = xdsResourceManager.xdsProject().metaRepo(); - final ContextAwareBlockingTaskExecutor taskExecutor = - ServiceRequestContext.current().blockingTaskExecutor(); - - final List> futures = new ArrayList<>(); - for (KubernetesLocalityLbEndpoints localityLbEndpoints : localityLbEndpointsList) { - futures.add(resolvePreview(localityLbEndpoints, group, metaRepository, taskExecutor)); - } - - CompletableFuture.allOf(futures.toArray(EMPTY_FUTURES)) - .handle((unused, cause) -> { - if (cause != null) { - final Throwable peeled = Exceptions.peel(cause); - final Status status = - peeled instanceof IllegalArgumentException || - peeled instanceof EntryNotFoundException ? - Status.INVALID_ARGUMENT : Status.INTERNAL; - responseObserver.onError( - status.withDescription(peeled.getMessage()) - .asRuntimeException()); - } else { - final ClusterLoadAssignment.Builder cla = - ClusterLoadAssignment.newBuilder(); - if (!aggregator.getClusterName().isEmpty()) { - cla.setClusterName(aggregator.getClusterName()); - } - for (CompletableFuture future : futures) { - cla.addEndpoints(future.join()); - } - responseObserver.onNext(cla.build()); - responseObserver.onCompleted(); - } - return null; - }); - } - - private static CompletableFuture resolvePreview( - KubernetesLocalityLbEndpoints localityLbEndpoints, String group, - MetaRepository metaRepository, ContextAwareBlockingTaskExecutor taskExecutor) { - final CompletableFuture result = new CompletableFuture<>(); - final ServiceEndpointWatcher watcher = localityLbEndpoints.getWatcher(); - createKubernetesEndpointGroup(watcher, metaRepository, group, "(preview)", false) - .handle((endpointGroup, cause) -> { - if (cause != null) { - result.completeExceptionally(Exceptions.peel(cause)); - return null; - } - final AtomicBoolean completed = new AtomicBoolean(); - final ScheduledFuture timeout = taskExecutor.schedule(() -> { - if (completed.compareAndSet(false, true)) { - endpointGroup.closeAsync(); - result.completeExceptionally(new IllegalStateException( - "Timed out after " + PREVIEW_TIMEOUT_SECONDS + - "s resolving '" + watcher.getServiceName() + '\'')); - } - }, PREVIEW_TIMEOUT_SECONDS, TimeUnit.SECONDS); - endpointGroup.whenReady().handle((endpoints, readyCause) -> { - if (!completed.compareAndSet(false, true)) { - return null; - } - timeout.cancel(false); - try { - if (readyCause != null) { - result.completeExceptionally(Exceptions.peel(readyCause)); - } else { - result.complete(toLocalityLbEndpoints(endpointGroup, - localityLbEndpoints)); - } - } finally { - endpointGroup.closeAsync(); - } - return null; - }); - return null; - }); - return result; - } - - private static LocalityLbEndpoints toLocalityLbEndpoints( - KubernetesEndpointGroup endpointGroup, KubernetesLocalityLbEndpoints localityLbEndpoints) { - final LocalityLbEndpoints.Builder builder = LocalityLbEndpoints.newBuilder(); - if (localityLbEndpoints.hasLocality()) { - builder.setLocality(localityLbEndpoints.getLocality()); - } - if (localityLbEndpoints.hasLoadBalancingWeight()) { - builder.setLoadBalancingWeight(localityLbEndpoints.getLoadBalancingWeight()); - } - builder.setPriority(localityLbEndpoints.getPriority()); - KubernetesEndpointConverter.addLbEndpoints(builder, endpointGroup.endpoints(), - localityLbEndpoints.getWatcher()); - return builder.build(); - } } 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 80cee44cd..f821a6e3a 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 @@ -20,24 +20,30 @@ 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.removePrefix; -import java.util.regex.Matcher; +import java.io.IOException; +import java.util.concurrent.CompletableFuture; import java.util.regex.Pattern; -import com.google.protobuf.Empty; +import org.jspecify.annotations.Nullable; +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.server.annotation.Consumes; +import com.linecorp.armeria.server.annotation.Delete; +import com.linecorp.armeria.server.annotation.Param; +import com.linecorp.armeria.server.annotation.Post; +import com.linecorp.armeria.server.annotation.Put; +import com.linecorp.centraldogma.common.RepositoryRole; +import com.linecorp.centraldogma.xds.internal.RequiresXdsGroupRole; import com.linecorp.centraldogma.xds.internal.XdsResourceManager; -import com.linecorp.centraldogma.xds.listener.v1.XdsListenerServiceGrpc.XdsListenerServiceImplBase; import io.envoyproxy.envoy.config.listener.v3.Listener; -import io.grpc.Status; -import io.grpc.stub.StreamObserver; /** - * Service for managing listeners. + * Annotated service object for managing listeners. */ -public final class XdsListenerService extends XdsListenerServiceImplBase { +public final class XdsListenerService { private static final Pattern LISTENER_NAME_PATTERN = Pattern.compile("^groups/([^/]+)/listeners/" + LEGACY_RESOURCE_ID_PATTERN_STRING + '$'); @@ -51,60 +57,90 @@ public XdsListenerService(XdsResourceManager xdsResourceManager) { this.xdsResourceManager = xdsResourceManager; } - @Override - public void createListener(CreateListenerRequest request, StreamObserver responseObserver) { - final String parent = request.getParent(); - final String group = removePrefix("groups/", parent); - xdsResourceManager.checkWritePermission(group); - - final String listenerId = request.getListenerId(); + /** + * POST /xds/groups/{group}/listeners + * + *

Creates a new listener. + */ + @Post("/xds/groups/{group}/listeners") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture createListener( + @Param("group") String group, + @Param("listener_id") String listenerId, + @Param("summary") @Nullable String summary, + String body) { if (!RESOURCE_ID_PATTERN.matcher(listenerId).matches()) { - throw Status.INVALID_ARGUMENT.withDescription("Invalid listener_id: " + listenerId + - " (expected: " + RESOURCE_ID_PATTERN + ')') - .asRuntimeException(); + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid listener ID: " + listenerId)); } - - final String listenerName = parent + LISTENERS_DIRECTORY + listenerId; - // Ignore the specified name in the listener and set the name - // with the format of "groups/{group}/listeners/{listener}". - // https://github.com/aip-dev/google.aip.dev/blob/master/aip/general/0133.md#user-specified-ids - final Listener listener = request.getListener().toBuilder().setName(listenerName).build(); - final String createSummary = - isNullOrEmpty(request.getSummary()) ? "Create listener: " + listenerName : request.getSummary(); - xdsResourceManager.push(responseObserver, group, listenerName, - LISTENERS_DIRECTORY + listenerId + ".yaml", - createSummary, listener, currentAuthor(), true); - } - - @Override - public void updateListener(UpdateListenerRequest request, StreamObserver responseObserver) { - final Listener listener = request.getListener(); - final String listenerName = listener.getName(); - final String group = checkListenerName(listenerName).group(1); - xdsResourceManager.checkWritePermission(group); - final String updateSummary = - isNullOrEmpty(request.getSummary()) ? "Update listener: " + listenerName : request.getSummary(); - xdsResourceManager.update(responseObserver, group, listenerName, - updateSummary, listener, currentAuthor()); + final String listenerName = "groups/" + group + LISTENERS_DIRECTORY + listenerId; + try { + XdsResourceManager.parseYaml(body, Listener.newBuilder()); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); + } + final String createSummary = isNullOrEmpty(summary) ? "Create listener: " + listenerName : summary; + final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); + final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", listenerName); + return xdsResourceManager.push(group, listenerName, + LISTENERS_DIRECTORY + listenerId + ".yaml", + createSummary, currentAuthor(), true, bodyToStore); } - private static Matcher checkListenerName(String listenerName) { - final Matcher matcher = LISTENER_NAME_PATTERN.matcher(listenerName); - if (!matcher.matches()) { - throw Status.INVALID_ARGUMENT.withDescription("Invalid listener name: " + listenerName + - " (expected: " + LISTENER_NAME_PATTERN + ')') - .asRuntimeException(); + /** + * PUT /xds/groups/{group}/listeners/{listener_id} + * + *

Updates an existing listener. + */ + @Put("/xds/groups/{group}/listeners/{*listener_id}") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture updateListener( + @Param("group") String group, + @Param("listener_id") String listenerId, + @Param("summary") @Nullable String summary, + String body) { + final String listenerName = "groups/" + group + "/listeners/" + listenerId; + if (!LISTENER_NAME_PATTERN.matcher(listenerName).matches()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid listener name: " + listenerName)); + } + try { + XdsResourceManager.parseYaml(body, Listener.newBuilder()); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); } - return matcher; + final String updateSummary = isNullOrEmpty(summary) ? "Update listener: " + listenerName : summary; + final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); + final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", listenerName); + return xdsResourceManager.update(group, listenerName, updateSummary, currentAuthor(), bodyToStore); } - @Override - public void deleteListener(DeleteListenerRequest request, StreamObserver responseObserver) { - final String listenerName = request.getName(); - final String group = checkListenerName(listenerName).group(1); - xdsResourceManager.checkWritePermission(group); - final String deleteSummary = - isNullOrEmpty(request.getSummary()) ? "Delete listener: " + listenerName : request.getSummary(); - xdsResourceManager.delete(responseObserver, group, listenerName, deleteSummary, currentAuthor()); + /** + * DELETE /xds/groups/{group}/listeners/{listener_id} + * + *

Removes a listener. + */ + @Delete("/xds/groups/{group}/listeners/{*listener_id}") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture deleteListener( + @Param("group") String group, + @Param("listener_id") String listenerId, + @Param("summary") @Nullable String summary) { + final String listenerName = "groups/" + group + "/listeners/" + listenerId; + if (!LISTENER_NAME_PATTERN.matcher(listenerName).matches()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid listener name: " + listenerName)); + } + final String deleteSummary = isNullOrEmpty(summary) ? "Delete listener: " + listenerName : summary; + return xdsResourceManager.delete(group, listenerName, deleteSummary, currentAuthor()); } } 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 df33a18be..437b51c86 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 @@ -20,24 +20,30 @@ 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.removePrefix; -import java.util.regex.Matcher; +import java.io.IOException; +import java.util.concurrent.CompletableFuture; import java.util.regex.Pattern; -import com.google.protobuf.Empty; +import org.jspecify.annotations.Nullable; +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.server.annotation.Consumes; +import com.linecorp.armeria.server.annotation.Delete; +import com.linecorp.armeria.server.annotation.Param; +import com.linecorp.armeria.server.annotation.Post; +import com.linecorp.armeria.server.annotation.Put; +import com.linecorp.centraldogma.common.RepositoryRole; +import com.linecorp.centraldogma.xds.internal.RequiresXdsGroupRole; import com.linecorp.centraldogma.xds.internal.XdsResourceManager; -import com.linecorp.centraldogma.xds.route.v1.XdsRouteServiceGrpc.XdsRouteServiceImplBase; import io.envoyproxy.envoy.config.route.v3.RouteConfiguration; -import io.grpc.Status; -import io.grpc.stub.StreamObserver; /** - * Service for managing routes. + * Annotated service object for managing routes. */ -public final class XdsRouteService extends XdsRouteServiceImplBase { +public final class XdsRouteService { private static final Pattern ROUTE_NAME_PATTERN = Pattern.compile("^groups/([^/]+)/routes/" + LEGACY_RESOURCE_ID_PATTERN_STRING + '$'); @@ -51,58 +57,89 @@ public XdsRouteService(XdsResourceManager xdsResourceManager) { this.xdsResourceManager = xdsResourceManager; } - @Override - public void createRoute(CreateRouteRequest request, StreamObserver responseObserver) { - final String parent = request.getParent(); - final String group = removePrefix("groups/", parent); - xdsResourceManager.checkWritePermission(group); - - final String routeId = request.getRouteId(); + /** + * POST /xds/groups/{group}/routes + * + *

Creates a new route. + */ + @Post("/xds/groups/{group}/routes") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture createRoute( + @Param("group") String group, + @Param("route_id") String routeId, + @Param("summary") @Nullable String summary, + String body) { if (!RESOURCE_ID_PATTERN.matcher(routeId).matches()) { - throw Status.INVALID_ARGUMENT.withDescription("Invalid route_id: " + routeId + - " (expected: " + RESOURCE_ID_PATTERN + ')') - .asRuntimeException(); + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid route ID: " + routeId)); } - - final String routeName = parent + ROUTES_DIRECTORY + routeId; - // Ignore the specified name in the route and set the name - // with the format of "groups/{group}/routes/{route}". - // https://github.com/aip-dev/google.aip.dev/blob/master/aip/general/0133.md#user-specified-ids - final RouteConfiguration route = request.getRoute().toBuilder().setName(routeName).build(); - final String createSummary = isNullOrEmpty(request.getSummary()) ? - "Create route: " + routeName : request.getSummary(); - xdsResourceManager.push(responseObserver, group, routeName, ROUTES_DIRECTORY + routeId + ".yaml", - createSummary, route, currentAuthor(), true); - } - - @Override - public void updateRoute(UpdateRouteRequest request, StreamObserver responseObserver) { - final RouteConfiguration route = request.getRoute(); - final String routeName = route.getName(); - final String group = checkRouteName(routeName).group(1); - xdsResourceManager.checkWritePermission(group); - final String updateSummary = isNullOrEmpty(request.getSummary()) ? - "Update route: " + routeName : request.getSummary(); - xdsResourceManager.update(responseObserver, group, routeName, updateSummary, route, currentAuthor()); + final String routeName = "groups/" + group + ROUTES_DIRECTORY + routeId; + try { + XdsResourceManager.parseYaml(body, RouteConfiguration.newBuilder()); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); + } + final String createSummary = isNullOrEmpty(summary) ? "Create route: " + routeName : summary; + final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); + final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", routeName); + return xdsResourceManager.push(group, routeName, ROUTES_DIRECTORY + routeId + ".yaml", + createSummary, currentAuthor(), true, bodyToStore); } - private static Matcher checkRouteName(String routeName) { - final Matcher matcher = ROUTE_NAME_PATTERN.matcher(routeName); - if (!matcher.matches()) { - throw Status.INVALID_ARGUMENT.withDescription("Invalid route name: " + routeName + - " (expected: " + ROUTE_NAME_PATTERN + ')') - .asRuntimeException(); + /** + * PUT /xds/groups/{group}/routes/{route_id} + * + *

Updates an existing route. + */ + @Put("/xds/groups/{group}/routes/{*route_id}") + @Consumes("application/yaml") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture updateRoute( + @Param("group") String group, + @Param("route_id") String routeId, + @Param("summary") @Nullable String summary, + String body) { + final String routeName = "groups/" + group + "/routes/" + routeId; + if (!ROUTE_NAME_PATTERN.matcher(routeName).matches()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid route name: " + routeName)); + } + try { + XdsResourceManager.parseYaml(body, RouteConfiguration.newBuilder()); + } catch (IOException e) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid request body: " + e.getMessage())); } - return matcher; + final String updateSummary = isNullOrEmpty(summary) ? "Update route: " + routeName : summary; + final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); + final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", routeName); + return xdsResourceManager.update(group, routeName, updateSummary, currentAuthor(), bodyToStore); } - @Override - public void deleteRoute(DeleteRouteRequest request, StreamObserver responseObserver) { - final String routeName = request.getName(); - final String group = checkRouteName(routeName).group(1); - xdsResourceManager.checkWritePermission(group); - final String deleteSummary = isNullOrEmpty(request.getSummary()) ? - "Delete route: " + routeName : request.getSummary(); - xdsResourceManager.delete(responseObserver, group, routeName, deleteSummary, currentAuthor()); + /** + * DELETE /xds/groups/{group}/routes/{route_id} + * + *

Removes a route. + */ + @Delete("/xds/groups/{group}/routes/{*route_id}") + @RequiresXdsGroupRole(RepositoryRole.WRITE) + public CompletableFuture deleteRoute( + @Param("group") String group, + @Param("route_id") String routeId, + @Param("summary") @Nullable String summary) { + final String routeName = "groups/" + group + "/routes/" + routeId; + if (!ROUTE_NAME_PATTERN.matcher(routeName).matches()) { + return CompletableFuture.completedFuture( + XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, + "Invalid route name: " + routeName)); + } + final String deleteSummary = isNullOrEmpty(summary) ? "Delete route: " + routeName : summary; + return xdsResourceManager.delete(group, routeName, deleteSummary, currentAuthor()); } } diff --git a/xds/src/main/proto/centraldogma/xds/cluster/v1/xds_cluster.proto b/xds/src/main/proto/centraldogma/xds/cluster/v1/xds_cluster.proto deleted file mode 100644 index 4a5995459..000000000 --- a/xds/src/main/proto/centraldogma/xds/cluster/v1/xds_cluster.proto +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright 2024 LINE Corporation -// -// LINE 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. -syntax = "proto3"; - -package centraldogma.xds.cluster.v1; - -option java_multiple_files = true; -option java_outer_classname = "XdsClusterProto"; -option java_package = "com.linecorp.centraldogma.xds.cluster.v1"; - -import "envoy/config/cluster/v3/cluster.proto"; - -import "google/api/annotations.proto"; -import "google/api/client.proto"; -import "google/api/field_behavior.proto"; -import "google/protobuf/empty.proto"; - -// An XdsClusterService provides methods to manage clusters. -service XdsClusterService { - - // Creates a new cluster. - rpc CreateCluster(CreateClusterRequest) returns (envoy.config.cluster.v3.Cluster) { - option (google.api.http) = { - post: "/api/v1/xds/{parent=groups/*}/clusters" - body: "cluster" - }; - } - - // Updates an existing cluster. - rpc UpdateCluster(UpdateClusterRequest) returns (envoy.config.cluster.v3.Cluster) { - option (google.api.http) = { - patch: "/api/v1/xds/{cluster.name=groups/*/clusters/**}" - body: "cluster" - }; - } - - // Deletes a cluster. - rpc DeleteCluster(DeleteClusterRequest) returns (google.protobuf.Empty) { - option (google.api.http) = { - delete: "/api/v1/xds/{name=groups/*/clusters/**}" - }; - } -} - -message CreateClusterRequest { - // The parent resource where this cluster will be created. - // Format: groups/{group} - string parent = 1 [(google.api.field_behavior) = REQUIRED]; - - // The ID to use for the cluster, which will become the final component of - // the cluster's resource name. - // - // Valid pattern is "^[a-z]([a-z0-9-/]*[a-z0-9])?$" - string cluster_id = 2 [(google.api.field_behavior) = REQUIRED]; - envoy.config.cluster.v3.Cluster cluster = 3 [(google.api.field_behavior) = REQUIRED]; - - // The commit summary for this change. If not specified, a default message is used. - string summary = 4 [(google.api.field_behavior) = OPTIONAL]; -} - -message UpdateClusterRequest { - // The cluster's `name` field is used to identify the cluster to update. - // Format: groups/{group}/clusters/{cluster} - envoy.config.cluster.v3.Cluster cluster = 1 [(google.api.field_behavior) = REQUIRED]; - - // TODO(minwoox): Add the following fields. - // The list of fields to be updated. - // google.protobuf.FieldMask update_mask = 2; - - // If set to true, and the cluster is not found, a new cluster will be created. - // In this situation, `update_mask` is ignored. - // bool allow_missing = 3; - - // The commit summary for this change. If not specified, a default message is used. - string summary = 4 [(google.api.field_behavior) = OPTIONAL]; -} - -message DeleteClusterRequest { - // Format: groups/{group}/clusters/{cluster} - string name = 1 [(google.api.field_behavior) = IDENTIFIER]; - - // If set to true, and the cluster is not found, the request will succeed - // but no action will be taken on the server - // bool allow_missing = 2; - - // The commit summary for this change. If not specified, a default message is used. - string summary = 3 [(google.api.field_behavior) = OPTIONAL]; -} diff --git a/xds/src/main/proto/centraldogma/xds/endpoint/v1/xds_endpoint.proto b/xds/src/main/proto/centraldogma/xds/endpoint/v1/xds_endpoint.proto index fb3929af7..8e1a5fc98 100644 --- a/xds/src/main/proto/centraldogma/xds/endpoint/v1/xds_endpoint.proto +++ b/xds/src/main/proto/centraldogma/xds/endpoint/v1/xds_endpoint.proto @@ -20,120 +20,11 @@ option java_outer_classname = "XdsEndpointProto"; option java_package = "com.linecorp.centraldogma.xds.endpoint.v1"; import "envoy/config/core/v3/base.proto"; -import "envoy/config/endpoint/v3/endpoint.proto"; import "envoy/config/endpoint/v3/endpoint_components.proto"; -import "google/api/annotations.proto"; -import "google/api/client.proto"; -import "google/api/field_behavior.proto"; -import "google/protobuf/empty.proto"; - -import "validate/validate.proto"; - -// An XdsEndpointService provides methods to manage endpoints. -service XdsEndpointService { - - // Creates a new endpoint. - rpc CreateEndpoint(CreateEndpointRequest) returns (envoy.config.endpoint.v3.ClusterLoadAssignment) { - option (google.api.http) = { - post: "/api/v1/xds/{parent=groups/*}/endpoints" - body: "endpoint" - }; - } - - // Updates an existing endpoint. - rpc UpdateEndpoint(UpdateEndpointRequest) returns (envoy.config.endpoint.v3.ClusterLoadAssignment) { - option (google.api.http) = { - // ClusterLoadAssignment does not have the name field, so use endpoint_name. - patch: "/api/v1/xds/{endpoint_name=groups/*/endpoints/**}" - body: "endpoint" - }; - } - - // Deletes a endpoint. - rpc DeleteEndpoint(DeleteEndpointRequest) returns (google.protobuf.Empty) { - option (google.api.http) = { - delete: "/api/v1/xds/{name=groups/*/endpoints/**}" - }; - } - - rpc RegisterLocalityLbEndpoint(RegisterLocalityLbEndpointRequest) returns (LocalityLbEndpoint) { - option (google.api.http) = { - patch: "/api/v1/xds/{endpoint_name=groups/*/endpoints/**}:registerLocalityLbEndpoint" - body: "locality_lb_endpoint" - }; - } - - rpc DeregisterLocalityLbEndpoint(DeregisterLocalityLbEndpointRequest) returns (google.protobuf.Empty) { - option (google.api.http) = { - delete: "/api/v1/xds/{endpoint_name=groups/*/endpoints/**}:deregisterLocalityLbEndpoint" - body: "locality_lb_endpoint" - }; - } -} - -message CreateEndpointRequest { - // The parent resource where this endpoint will be created. - // Format: groups/{group} - string parent = 1 [(google.api.field_behavior) = REQUIRED]; - - // The ID to use for the endpoint, which will become the final component of - // the endpoint's resource name. - // - // Valid pattern is "^[a-z]([a-z0-9-/]*[a-z0-9])?$" - string endpoint_id = 2 [(google.api.field_behavior) = REQUIRED]; - envoy.config.endpoint.v3.ClusterLoadAssignment endpoint = 3 [(google.api.field_behavior) = REQUIRED]; - - // The commit summary for this change. If not specified, a default message is used. - string summary = 4 [(google.api.field_behavior) = OPTIONAL]; -} - -message UpdateEndpointRequest { - // Format: groups/{group}/endpoints/{endpoint} - string endpoint_name = 1 [(google.api.field_behavior) = REQUIRED]; - - envoy.config.endpoint.v3.ClusterLoadAssignment endpoint = 2 [(google.api.field_behavior) = REQUIRED]; - - // The commit summary for this change. If not specified, a default message is used. - string summary = 3 [(google.api.field_behavior) = OPTIONAL]; - - // TODO(minwoox): Add the following fields. - // The list of fields to be updated. - // google.protobuf.FieldMask update_mask = 4; - - // If set to true, and the endpoint is not found, a new endpoint will be created. - // In this situation, `update_mask` is ignored. - // bool allow_missing = 5; -} - -message DeleteEndpointRequest { - // Format: groups/{group}/endpoints/{endpoint} - string name = 1 [(google.api.field_behavior) = IDENTIFIER]; - - // The commit summary for this change. If not specified, a default message is used. - string summary = 2 [(google.api.field_behavior) = OPTIONAL]; - - // If set to true, and the endpoint is not found, the request will succeed - // but no action will be taken on the server - // bool allow_missing = 3; -} - -message RegisterLocalityLbEndpointRequest { - // Format: groups/{group}/endpoints/{endpoint} - string endpoint_name = 1 [(google.api.field_behavior) = REQUIRED]; - - LocalityLbEndpoint locality_lb_endpoint = 2 [(google.api.field_behavior) = REQUIRED]; -} - +// Represents a single locality lb endpoint entry for register/deregister operations. message LocalityLbEndpoint { envoy.config.core.v3.Locality locality = 1; - uint32 priority = 2 [(validate.rules).uint32 = {lte: 128}]; - envoy.config.endpoint.v3.LbEndpoint lb_endpoint = 3 [(google.api.field_behavior) = REQUIRED]; -} - -message DeregisterLocalityLbEndpointRequest { - // Format: groups/{group}/endpoints/{endpoint} - string endpoint_name = 1 [(google.api.field_behavior) = REQUIRED]; - - LocalityLbEndpoint locality_lb_endpoint = 2 [(google.api.field_behavior) = REQUIRED]; + uint32 priority = 2; + envoy.config.endpoint.v3.LbEndpoint lb_endpoint = 3; } diff --git a/xds/src/main/proto/centraldogma/xds/group/v1/xds_group.proto b/xds/src/main/proto/centraldogma/xds/group/v1/xds_group.proto deleted file mode 100644 index 570daaae0..000000000 --- a/xds/src/main/proto/centraldogma/xds/group/v1/xds_group.proto +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright 2024 LINE Corporation -// -// LINE 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. -syntax = "proto3"; - -package centraldogma.xds.group.v1; - -option java_multiple_files = true; -option java_outer_classname = "XdsGroupProto"; -option java_package = "com.linecorp.centraldogma.xds.group.v1"; - -import "google/api/annotations.proto"; -import "google/api/client.proto"; -import "google/api/field_behavior.proto"; -import "google/protobuf/empty.proto"; - -// An XdsGroupService provides methods to manage groups. -service XdsGroupService { - - // Creates a new group. - rpc CreateGroup(CreateGroupRequest) returns (Group) { - option (google.api.http) = { - post: "/api/v1/xds/groups" - body: "group" - }; - } - - // Deletes an group. - rpc DeleteGroup(DeleteGroupRequest) returns (google.protobuf.Empty) { - option (google.api.http) = { - delete: "/api/v1/xds/{name=groups/*}" - }; - } -} - -message CreateGroupRequest { - string group_id = 1 [(google.api.field_behavior) = REQUIRED]; - Group group = 2 [(google.api.field_behavior) = REQUIRED]; -} - -message DeleteGroupRequest { - // Format: groups/{group} - string name = 1 [(google.api.field_behavior) = IDENTIFIER]; - - // If set to true, any xds Resources from this group will also be deleted. - // (Otherwise, the request will only work if the group has no xDS resources.) - // bool force = 2; -} - -message Group { - // Format: groups/{group} - string name = 1 [(google.api.field_behavior) = IDENTIFIER]; -} diff --git a/xds/src/main/proto/centraldogma/xds/listener/v1/xds_listener.proto b/xds/src/main/proto/centraldogma/xds/listener/v1/xds_listener.proto deleted file mode 100644 index 8b518f917..000000000 --- a/xds/src/main/proto/centraldogma/xds/listener/v1/xds_listener.proto +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright 2024 LINE Corporation -// -// LINE 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. -syntax = "proto3"; - -package centraldogma.xds.listener.v1; - -option java_multiple_files = true; -option java_outer_classname = "XdsListenerProto"; -option java_package = "com.linecorp.centraldogma.xds.listener.v1"; - -import "envoy/config/listener/v3/listener.proto"; - -import "google/api/annotations.proto"; -import "google/api/client.proto"; -import "google/api/field_behavior.proto"; -import "google/protobuf/empty.proto"; - -// An XdsListenerService provides methods to manage listeners. -service XdsListenerService { - - // Creates a new listener. - rpc CreateListener(CreateListenerRequest) returns (envoy.config.listener.v3.Listener) { - option (google.api.http) = { - post: "/api/v1/xds/{parent=groups/*}/listeners" - body: "listener" - }; - } - - // Updates an existing listener. - rpc UpdateListener(UpdateListenerRequest) returns (envoy.config.listener.v3.Listener) { - option (google.api.http) = { - patch: "/api/v1/xds/{listener.name=groups/*/listeners/**}" - body: "listener" - }; - } - - // Deletes a listener. - rpc DeleteListener(DeleteListenerRequest) returns (google.protobuf.Empty) { - option (google.api.http) = { - delete: "/api/v1/xds/{name=groups/*/listeners/**}" - }; - } -} - -message CreateListenerRequest { - // The parent resource where this listener will be created. - // Format: groups/{group} - string parent = 1 [(google.api.field_behavior) = REQUIRED]; - - // The ID to use for the listener, which will become the final component of - // the listener's resource name. - // - // Valid pattern is "^[a-z]([a-z0-9-/]*[a-z0-9])?$" - string listener_id = 2 [(google.api.field_behavior) = REQUIRED]; - envoy.config.listener.v3.Listener listener = 3 [(google.api.field_behavior) = REQUIRED]; - - // The commit summary for this change. If not specified, a default message is used. - string summary = 4 [(google.api.field_behavior) = OPTIONAL]; -} - -message UpdateListenerRequest { - // The listener's `name` field is used to identify the listener to update. - // Format: groups/{group}/listeners/{listener} - envoy.config.listener.v3.Listener listener = 1 [(google.api.field_behavior) = REQUIRED]; - - // The commit summary for this change. If not specified, a default message is used. - string summary = 2 [(google.api.field_behavior) = OPTIONAL]; - - // TODO(minwoox): Add the following fields. - // The list of fields to be updated. - // google.protobuf.FieldMask update_mask = 3; - - // If set to true, and the listener is not found, a new listener will be created. - // In this situation, `update_mask` is ignored. - // bool allow_missing = 4; -} - -message DeleteListenerRequest { - // Format: groups/{group}/listeners/{listener} - string name = 1 [(google.api.field_behavior) = IDENTIFIER]; - - // The commit summary for this change. If not specified, a default message is used. - string summary = 2 [(google.api.field_behavior) = OPTIONAL]; - - // If set to true, and the listener is not found, the request will succeed - // but no action will be taken on the server - // bool allow_missing = 3; -} diff --git a/xds/src/main/proto/centraldogma/xds/route/v1/xds_route.proto b/xds/src/main/proto/centraldogma/xds/route/v1/xds_route.proto deleted file mode 100644 index 242dcc645..000000000 --- a/xds/src/main/proto/centraldogma/xds/route/v1/xds_route.proto +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright 2024 LINE Corporation -// -// LINE 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. -syntax = "proto3"; - -package centraldogma.xds.route.v1; - -option java_multiple_files = true; -option java_outer_classname = "XdsRouteProto"; -option java_package = "com.linecorp.centraldogma.xds.route.v1"; - -import "envoy/config/route/v3/route.proto"; - -import "google/api/annotations.proto"; -import "google/api/client.proto"; -import "google/api/field_behavior.proto"; -import "google/protobuf/empty.proto"; - -// An XdsRouteService provides methods to manage routes. -service XdsRouteService { - - // Creates a new route. - rpc CreateRoute(CreateRouteRequest) returns (envoy.config.route.v3.RouteConfiguration) { - option (google.api.http) = { - post: "/api/v1/xds/{parent=groups/*}/routes" - body: "route" - }; - } - - // Updates an existing route. - rpc UpdateRoute(UpdateRouteRequest) returns (envoy.config.route.v3.RouteConfiguration) { - option (google.api.http) = { - patch: "/api/v1/xds/{route.name=groups/*/routes/**}" - body: "route" - }; - } - - // Deletes a route. - rpc DeleteRoute(DeleteRouteRequest) returns (google.protobuf.Empty) { - option (google.api.http) = { - delete: "/api/v1/xds/{name=groups/*/routes/**}" - }; - } -} - -message CreateRouteRequest { - // The parent resource where this route will be created. - // Format: groups/{group} - string parent = 1 [(google.api.field_behavior) = REQUIRED]; - - // The ID to use for the route, which will become the final component of - // the route's resource name. - // - // Valid pattern is "^[a-z]([a-z0-9-/]*[a-z0-9])?$" - string route_id = 2 [(google.api.field_behavior) = REQUIRED]; - envoy.config.route.v3.RouteConfiguration route = 3 [(google.api.field_behavior) = REQUIRED]; - - // The commit summary for this change. If not specified, a default message is used. - string summary = 4 [(google.api.field_behavior) = OPTIONAL]; -} - -message UpdateRouteRequest { - // The route's `name` field is used to identify the route to update. - // Format: groups/{group}/routes/{route} - envoy.config.route.v3.RouteConfiguration route = 1 [(google.api.field_behavior) = REQUIRED]; - - // The commit summary for this change. If not specified, a default message is used. - string summary = 2 [(google.api.field_behavior) = OPTIONAL]; - - // TODO(minwoox): Add the following fields. - // The list of fields to be updated. - // google.protobuf.FieldMask update_mask = 3; - - // If set to true, and the route is not found, a new route will be created. - // In this situation, `update_mask` is ignored. - // bool allow_missing = 4; -} - -message DeleteRouteRequest { - // Format: groups/{group}/routes/{route} - string name = 1 [(google.api.field_behavior) = IDENTIFIER]; - - // The commit summary for this change. If not specified, a default message is used. - string summary = 2 [(google.api.field_behavior) = OPTIONAL]; - - // If set to true, and the Route is not found, the request will succeed - // but no action will be taken on the server - // bool allow_missing = 3; -} 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 a3f078fcd..539109229 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 @@ -34,7 +34,6 @@ import com.google.protobuf.Any; import com.google.protobuf.Duration; -import com.google.protobuf.Empty; import com.google.protobuf.InvalidProtocolBufferException; import com.linecorp.armeria.client.grpc.GrpcClients; @@ -43,15 +42,14 @@ import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.centraldogma.internal.Yaml; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; -import com.linecorp.centraldogma.xds.cluster.v1.XdsClusterServiceGrpc.XdsClusterServiceBlockingStub; import io.envoyproxy.controlplane.cache.Resources.V3; import io.envoyproxy.envoy.config.cluster.v3.Cluster; import io.envoyproxy.envoy.service.cluster.v3.ClusterDiscoveryServiceGrpc.ClusterDiscoveryServiceStub; import io.envoyproxy.envoy.service.discovery.v3.DiscoveryRequest; import io.envoyproxy.envoy.service.discovery.v3.DiscoveryResponse; -import io.grpc.Status; import io.grpc.stub.StreamObserver; class XdsClusterServiceTest { @@ -82,25 +80,21 @@ void createClusterViaHttp() throws Exception { 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); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), clusterBuilder); final Cluster actualCluster = clusterBuilder.build(); final String clusterName = "groups/foo/clusters/foo-cluster.1"; assertThat(actualCluster).isEqualTo(cluster.toBuilder() .setName(clusterName) - .setRespectDnsTtl(true) .build()); checkResourceViaDiscoveryRequest(actualCluster, clusterName, true); // Create the same cluster again. 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())); } private static void assertOk(AggregatedHttpResponse response) { assertThat(response.status()).isSameAs(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); } private static void checkResourceViaDiscoveryRequest(Cluster actualCluster, String resourceName, @@ -153,12 +147,11 @@ void updateClusterViaHttp() throws Exception { 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); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), clusterBuilder); final Cluster actualCluster = clusterBuilder.build(); final String clusterName = "groups/foo/clusters/foo-cluster.2"; assertThat(actualCluster).isEqualTo(cluster.toBuilder() .setName(clusterName) - .setRespectDnsTtl(true) .build()); checkResourceViaDiscoveryRequest(actualCluster, clusterName, true); @@ -166,12 +159,11 @@ void updateClusterViaHttp() throws Exception { .setConnectTimeout( Duration.newBuilder().setSeconds(2).build()) .setName(clusterName) - .setRespectDnsTtl(false) .build(); 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); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), clusterBuilder2); final Cluster actualCluster2 = clusterBuilder2.build(); assertThat(actualCluster2).isEqualTo(updatingCluster.toBuilder().setName(clusterName).build()); checkResourceViaDiscoveryRequest(actualCluster2, clusterName, true); @@ -193,7 +185,6 @@ void deleteClusterViaHttp() throws Exception { final Cluster actualCluster = cluster.toBuilder() .setName(clusterName) - .setRespectDnsTtl(true) .build(); checkResourceViaDiscoveryRequest(actualCluster, clusterName, true); @@ -201,7 +192,6 @@ void deleteClusterViaHttp() throws Exception { response = deleteCluster(clusterName); assertOk(response); - assertThat(response.contentUtf8()).isEqualTo("{}"); checkResourceViaDiscoveryRequest(actualCluster, clusterName, false); } @@ -211,28 +201,4 @@ private static AggregatedHttpResponse deleteCluster(String clusterName) { .build(); return dogma.httpClient().execute(headers).aggregate().join(); } - - @Test - void viaStub() { - final XdsClusterServiceBlockingStub client = GrpcClients.builder(dogma.httpClient().uri()).setHeader( - 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") - .setCluster(cluster).build()); - final String clusterName = "groups/foo/clusters/foo-cluster.5.6"; - assertThat(response).isEqualTo(cluster.toBuilder() - .setName(clusterName) - .setRespectDnsTtl(true) - .build()); - - final Cluster updatingCluster = cluster.toBuilder().setConnectTimeout( - Duration.newBuilder().setSeconds(2).build()).setName(clusterName).build(); - response = client.updateCluster(UpdateClusterRequest.newBuilder().setCluster(updatingCluster).build()); - assertThat(response).isEqualTo(updatingCluster); - - // No exception is thrown. - final Empty ignored = client.deleteCluster( - DeleteClusterRequest.newBuilder().setName(clusterName).build()); - } } 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 a8b4027cb..78d511cee 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 @@ -38,7 +38,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import com.google.protobuf.Any; -import com.google.protobuf.Empty; import com.google.protobuf.InvalidProtocolBufferException; import com.linecorp.armeria.client.grpc.GrpcClients; @@ -50,10 +49,11 @@ import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.centraldogma.common.Change; import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.internal.Yaml; import com.linecorp.centraldogma.server.storage.repository.FindOptions; import com.linecorp.centraldogma.server.storage.repository.Repository; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; -import com.linecorp.centraldogma.xds.endpoint.v1.XdsEndpointServiceGrpc.XdsEndpointServiceBlockingStub; +import com.linecorp.centraldogma.xds.internal.XdsTestUtil; import io.envoyproxy.controlplane.cache.Resources.V3; import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment; @@ -61,7 +61,6 @@ import io.envoyproxy.envoy.service.discovery.v3.DiscoveryRequest; import io.envoyproxy.envoy.service.discovery.v3.DiscoveryResponse; import io.envoyproxy.envoy.service.endpoint.v3.EndpointDiscoveryServiceGrpc.EndpointDiscoveryServiceStub; -import io.grpc.Status; import io.grpc.stub.StreamObserver; public class XdsEndpointServiceTest { @@ -93,7 +92,7 @@ void createEndpointViaHttp() throws Exception { 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); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), endpointBuilder); final ClusterLoadAssignment actualEndpoint = endpointBuilder.build(); final String clusterName = "groups/foo/clusters/foo-endpoint.1"; assertThat(actualEndpoint).isEqualTo( @@ -103,7 +102,6 @@ void createEndpointViaHttp() throws Exception { static void assertOk(AggregatedHttpResponse response) { assertThat(response.status()).isSameAs(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); } public static void checkEndpointsViaDiscoveryRequest( @@ -160,7 +158,7 @@ void updateEndpointViaHttp() throws Exception { 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); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), endpointBuilder); final ClusterLoadAssignment actualEndpoint = endpointBuilder.build(); final String clusterName = "groups/foo/clusters/foo-endpoint.2"; assertThat(actualEndpoint).isEqualTo(endpoint.toBuilder().setClusterName(clusterName).build()); @@ -174,7 +172,7 @@ void updateEndpointViaHttp() throws Exception { response = updateEndpoint("foo-endpoint.2", updatingEndpoint); assertOk(response); final ClusterLoadAssignment.Builder endpointBuilder2 = ClusterLoadAssignment.newBuilder(); - JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), endpointBuilder2); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), endpointBuilder2); final ClusterLoadAssignment actualEndpoint2 = endpointBuilder2.build(); assertThat(actualEndpoint2).isEqualTo( updatingEndpoint.toBuilder().setClusterName(clusterName).build()); @@ -196,8 +194,9 @@ void createEndpointReturnAlreadyExistsWhenYamlExists() throws Exception { final AggregatedHttpResponse response = 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())); + // The conflict message must reference the endpoint name, not the cluster name. + assertThat(response.contentUtf8()).contains("groups/foo/endpoints/yaml-exists.1"); + assertThat(response.contentUtf8()).doesNotContain("groups/foo/clusters/yaml-exists.1"); // The original .yaml file must still be the only file present (no new .json created). final Repository repo = @@ -239,7 +238,7 @@ void updateYamlEndpointViaHttp() throws Exception { FindOptions.FIND_ONE_WITHOUT_CONTENT).join()).isEmpty(); final ClusterLoadAssignment.Builder endpointBuilder = ClusterLoadAssignment.newBuilder(); - JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), endpointBuilder); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), endpointBuilder); checkEndpointsViaDiscoveryRequest(dogma.httpClient().uri(), endpointBuilder.build(), clusterName); } @@ -259,7 +258,6 @@ void deleteYamlEndpointViaHttp() throws Exception { // Delete via the HTTP API — updateOrDelete must locate and remove the .yaml file. final AggregatedHttpResponse response = deleteEndpoint(endpointName); assertOk(response); - assertThat(response.contentUtf8()).isEqualTo("{}"); // The .yaml file must be gone. final Repository repo = @@ -273,11 +271,11 @@ void deleteYamlEndpointViaHttp() throws Exception { private static AggregatedHttpResponse updateEndpoint( String endpointId, ClusterLoadAssignment endpoint) throws IOException { - final RequestHeaders headers = RequestHeaders.builder(HttpMethod.PATCH, + final RequestHeaders headers = RequestHeaders.builder(HttpMethod.PUT, "/api/v1/xds/groups/foo/endpoints/" + endpointId) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8).build(); - return dogma.httpClient().execute(headers, JSON_MESSAGE_MARSHALLER.writeValueAsString(endpoint)) + .contentType(MediaType.parse("application/yaml")).build(); + return dogma.httpClient().execute(headers, XdsTestUtil.toYaml(endpoint)) .aggregate().join(); } @@ -297,11 +295,8 @@ void deleteEndpointViaHttp() throws Exception { endpoint.toBuilder().setClusterName(clusterName).build(); checkEndpointsViaDiscoveryRequest(dogma.httpClient().uri(), actualEndpoint, clusterName); - // Add permission test. - response = deleteEndpoint(endpointName); assertOk(response); - assertThat(response.contentUtf8()).isEqualTo("{}"); checkEndpointsViaDiscoveryRequest(dogma.httpClient().uri(), null, clusterName); } @@ -312,45 +307,4 @@ private static AggregatedHttpResponse deleteEndpoint(String endpointName) { .build(); return dogma.httpClient().execute(headers).aggregate().join(); } - - @Test - void viaStub() throws Exception { - final XdsEndpointServiceBlockingStub client = - GrpcClients.builder(dogma.httpClient().uri()) - .setHeader(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .build(XdsEndpointServiceBlockingStub.class); - final ClusterLoadAssignment endpoint = loadAssignment("this_endpoint_name_will_be_ignored_and_replaced", - "127.0.0.1", 8080); - final ClusterLoadAssignment response = client.createEndpoint( - CreateEndpointRequest.newBuilder() - .setParent("groups/foo") - .setEndpointId("foo-endpoint.5.6") - .setEndpoint(endpoint) - .build()); - final String clusterName = "groups/foo/clusters/foo-endpoint.5.6"; - assertThat(response).isEqualTo(endpoint.toBuilder().setClusterName(clusterName).build()); - checkEndpointsViaDiscoveryRequest(dogma.httpClient().uri(), response, clusterName); - - final ClusterLoadAssignment updatingEndpoint = - endpoint.toBuilder() - .addEndpoints(LocalityLbEndpoints.newBuilder() - .addLbEndpoints(endpoint("127.0.0.1", 8081))) - .setClusterName(clusterName).build(); - - final String endpointName = "groups/foo/endpoints/foo-endpoint.5.6"; - final ClusterLoadAssignment response2 = client.updateEndpoint( - UpdateEndpointRequest.newBuilder() - .setEndpointName(endpointName) - .setEndpoint(updatingEndpoint) - .build()); - assertThat(response2).isEqualTo(updatingEndpoint); - checkEndpointsViaDiscoveryRequest(dogma.httpClient().uri(), response2, clusterName); - - // No exception is thrown. - final Empty ignored = client.deleteEndpoint( - DeleteEndpointRequest.newBuilder() - .setName(endpointName) - .build()); - checkEndpointsViaDiscoveryRequest(dogma.httpClient().uri(), null, clusterName); - } } 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 3dac1f289..5e800e844 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 @@ -45,6 +45,7 @@ import com.linecorp.centraldogma.common.Query; import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.internal.Jackson; +import com.linecorp.centraldogma.internal.Yaml; import com.linecorp.centraldogma.server.storage.repository.Repository; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; @@ -100,7 +101,9 @@ void registerOrDeregister() throws Exception { LocalityLbEndpoint.newBuilder().setLocality(locality1) .setLbEndpoint(endpoint("127.0.0.1", 8080)) .build(), false); - assertOk(registerFuture1.join()); + final AggregatedHttpResponse registerResponse1 = registerFuture1.join(); + assertOk(registerResponse1); + assertThat(registerResponse1.headers().contentType()).hasToString("application/yaml"); assertOk(registerFuture2.join()); assertOk(deregister.join()); // localityLbEndpoint1 and localityLbEndpoint2 are registered together so the major version should @@ -192,7 +195,6 @@ void registerOrDeregister() throws Exception { // Deregister the endpoint. response = registerOrDeregister(endpointName, localityLbEndpoint4, false); assertOk(response); - assertThat(response.contentUtf8()).isEqualTo("{}"); endpoint = endpoint.toBuilder() .removeEndpoints(2) .build(); @@ -204,7 +206,6 @@ void registerOrDeregister() throws Exception { response = registerOrDeregister(endpointName, localityLbEndpoint3, false); assertOk(response); - assertThat(response.contentUtf8()).isEqualTo("{}"); endpoint = endpoint.toBuilder() .removeEndpoints(1) .build(); @@ -228,9 +229,6 @@ void registerOrDeregister() throws Exception { assertOk(deregisterFuture2.join()); assertThat(fooRepository.normalizeNow(Revision.HEAD).major()).isEqualTo(prevMajor + 1); - - assertThat(deregisterFuture1.join().contentUtf8()).isEqualTo("{}"); - assertThat(deregisterFuture2.join().contentUtf8()).isEqualTo("{}"); endpoint = endpoint.toBuilder() .removeEndpoints(0) .build(); @@ -250,10 +248,10 @@ private static CompletableFuture registerOrDeregisterAsy (register ? ":registerLocalityLbEndpoint" : ":deregisterLocalityLbEndpoint")) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8).build(); - return dogma.httpClient().execute(headers, - JSON_MESSAGE_MARSHALLER.writeValueAsString(localityLbEndpoint)) - .aggregate(); + .contentType(MediaType.parse("application/yaml")).build(); + final String yaml = Yaml.writeValueAsString( + Jackson.readTree(JSON_MESSAGE_MARSHALLER.writeValueAsString(localityLbEndpoint))); + return dogma.httpClient().execute(headers, yaml).aggregate(); } @Test 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 2b10713b0..73da6ab68 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 @@ -18,25 +18,15 @@ import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.createGroup; import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.createGroupAsync; import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.deleteGroup; -import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -import com.google.protobuf.Empty; - -import com.linecorp.armeria.client.grpc.GrpcClients; import com.linecorp.armeria.common.AggregatedHttpResponse; -import com.linecorp.armeria.common.HttpHeaderNames; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.centraldogma.server.CentralDogmaBuilder; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; -import com.linecorp.centraldogma.xds.group.v1.XdsGroupServiceGrpc.XdsGroupServiceBlockingStub; - -import io.grpc.Status; -import io.grpc.StatusRuntimeException; final class XdsGroupServiceTest { @@ -55,18 +45,24 @@ protected void configure(CentralDogmaBuilder builder) { void createGroupViaHttp() { AggregatedHttpResponse response = createGroupAsync("foo", dogma.httpClient()).join(); assertOk(response); - assertThatJson(response.contentUtf8()).isEqualTo("{\"name\":\"groups/foo\"}"); // Cannot create with the same name. response = createGroupAsync("foo", dogma.httpClient()).join(); assertThat(response.status()).isSameAs(HttpStatus.CONFLICT); - assertThat(response.headers().get("grpc-status")) - .isEqualTo(Integer.toString(Status.ALREADY_EXISTS.getCode().value())); + + // Cannot create a group with an internal repository name. + response = createGroupAsync("dogma", dogma.httpClient()).join(); + assertThat(response.status()).isSameAs(HttpStatus.FORBIDDEN); + response = createGroupAsync("meta", dogma.httpClient()).join(); + assertThat(response.status()).isSameAs(HttpStatus.FORBIDDEN); + + // Cannot create a group with an invalid ID format. + response = createGroupAsync("@invalid!", dogma.httpClient()).join(); + assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); } private static void assertOk(AggregatedHttpResponse response) { assertThat(response.status()).isSameAs(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); } @Test @@ -77,44 +73,7 @@ void deleteGroupViaHttp() { response = createGroup("bar", dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.OK); - // Add permission test. - response = deleteGroup("groups/bar", dogma.httpClient()); assertOk(response); - assertThat(response.contentUtf8()).isEqualTo("{}"); - } - - @Test - void createAndDeleteGroupViaStub() { - final XdsGroupServiceBlockingStub client = - GrpcClients.builder(dogma.httpClient().uri()) - .setHeader(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .build(XdsGroupServiceBlockingStub.class); - assertThatThrownBy(() -> client.createGroup( - CreateGroupRequest.newBuilder() - .setGroupId("invalid/id") - .setGroup(Group.newBuilder().setName("this_will_be_ignored")) - .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") - .setGroup(Group.newBuilder().setName("this_will_be_ignored")) - .build()); - assertThat(group.getName()).isEqualTo("groups/baz"); - // No exception is thrown. - final Empty ignored = client.deleteGroup(DeleteGroupRequest.newBuilder() - .setName("groups/baz") - .build()); } } diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/MtlsDiscoveryAuthorizationTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/MtlsDiscoveryAuthorizationTest.java index b3b0f7b23..427777972 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/MtlsDiscoveryAuthorizationTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/MtlsDiscoveryAuthorizationTest.java @@ -18,7 +18,6 @@ import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.PASSWORD; import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.USERNAME; import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.getAccessToken; -import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.JSON_MESSAGE_MARSHALLER; import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.cluster; import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.exampleListener; import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.routeConfiguration; @@ -299,7 +298,6 @@ private static void createGroup(WebClient admin, String group) { admin.prepare() .post("/api/v1/xds/groups") .queryParam("group_id", group) - .content(MediaType.JSON, "{\"name\":\"groups/" + group + "\"}") .execute().aggregate().join(); assertThat(response.status()).isEqualTo(HttpStatus.OK); } @@ -325,7 +323,7 @@ private static void post(WebClient admin, String path, String idParam, String id admin.prepare() .post(path) .queryParam(idParam, id) - .content(MediaType.JSON, JSON_MESSAGE_MARSHALLER.writeValueAsString(body)) + .content(MediaType.parse("application/yaml"), XdsTestUtil.toYaml(body)) .execute().aggregate().join(); assertThat(response.status()).isEqualTo(HttpStatus.OK); } diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/TokenDiscoveryAuthorizationTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/TokenDiscoveryAuthorizationTest.java index 4ae329185..306a27b43 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/TokenDiscoveryAuthorizationTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/TokenDiscoveryAuthorizationTest.java @@ -18,7 +18,6 @@ import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.PASSWORD; import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.USERNAME; import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.getAccessToken; -import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.JSON_MESSAGE_MARSHALLER; import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.cluster; import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.loadAssignment; import static org.assertj.core.api.Assertions.assertThat; @@ -308,7 +307,6 @@ private static void createGroup(WebClient admin, String group) { admin.prepare() .post("/api/v1/xds/groups") .queryParam("group_id", group) - .content(MediaType.JSON, "{\"name\":\"groups/" + group + "\"}") .execute().aggregate().join(); assertThat(response.status()).isEqualTo(HttpStatus.OK); } @@ -319,7 +317,7 @@ private static void createCluster(WebClient admin, String group, String clusterI admin.prepare() .post("/api/v1/xds/groups/" + group + "/clusters") .queryParam("cluster_id", clusterId) - .content(MediaType.JSON, JSON_MESSAGE_MARSHALLER.writeValueAsString(cluster)) + .content(MediaType.parse("application/yaml"), XdsTestUtil.toYaml(cluster)) .execute().aggregate().join(); assertThat(response.status()).isEqualTo(HttpStatus.OK); } @@ -330,7 +328,7 @@ private static void createEndpoint(WebClient admin, String group, String endpoin admin.prepare() .post("/api/v1/xds/groups/" + group + "/endpoints") .queryParam("endpoint_id", endpointId) - .content(MediaType.JSON, JSON_MESSAGE_MARSHALLER.writeValueAsString(endpoint)) + .content(MediaType.parse("application/yaml"), XdsTestUtil.toYaml(endpoint)) .execute().aggregate().join(); assertThat(response.status()).isEqualTo(HttpStatus.OK); } diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsEndpointReadPermissionTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsEndpointReadPermissionTest.java index 31796dcc8..00cd5b2c3 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsEndpointReadPermissionTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsEndpointReadPermissionTest.java @@ -18,7 +18,6 @@ import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.PASSWORD; import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.USERNAME; import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.getAccessToken; -import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.JSON_MESSAGE_MARSHALLER; import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.cluster; import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.loadAssignment; import static org.assertj.core.api.Assertions.assertThat; @@ -100,7 +99,6 @@ private static AggregatedHttpResponse createGroup(WebClient client, String group return client.prepare() .post("/api/v1/xds/groups") .queryParam("group_id", group) - .content(MediaType.JSON, "{\"name\":\"groups/" + group + "\"}") .execute().aggregate().join(); } @@ -109,7 +107,7 @@ private static AggregatedHttpResponse createCluster(WebClient client, String gro return client.prepare() .post("/api/v1/xds/groups/" + group + "/clusters") .queryParam("cluster_id", clusterId) - .content(MediaType.JSON, JSON_MESSAGE_MARSHALLER.writeValueAsString(cluster)) + .content(MediaType.parse("application/yaml"), XdsTestUtil.toYaml(cluster)) .execute().aggregate().join(); } @@ -118,7 +116,7 @@ private static AggregatedHttpResponse createEndpoint(WebClient client, String gr return client.prepare() .post("/api/v1/xds/groups/" + group + "/endpoints") .queryParam("endpoint_id", endpointId) - .content(MediaType.JSON, JSON_MESSAGE_MARSHALLER.writeValueAsString(endpoint)) + .content(MediaType.parse("application/yaml"), XdsTestUtil.toYaml(endpoint)) .execute().aggregate().join(); } } diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsGroupDeletePermissionTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsGroupDeletePermissionTest.java index 3b59ee800..119a36752 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsGroupDeletePermissionTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsGroupDeletePermissionTest.java @@ -66,17 +66,16 @@ void shouldRequireAdminRoleToDeleteGroup() throws Exception { assertThat(createGroup(admin, "foo").status()).isEqualTo(HttpStatus.OK); // A principal with no role cannot delete the group. - assertThat(deleteGroup(noRole, "foo").headers().get("grpc-status")).isEqualTo("7"); // PERMISSION_DENIED + assertThat(deleteGroup(noRole, "foo").status()).isEqualTo(HttpStatus.FORBIDDEN); // A WRITE role is insufficient; deletion requires ADMIN. grantRole(admin, "foo", "writer", "WRITE"); - assertThat(deleteGroup(writer, "foo").headers().get("grpc-status")).isEqualTo("7"); + assertThat(deleteGroup(writer, "foo").status()).isEqualTo(HttpStatus.FORBIDDEN); // An ADMIN role can delete the group. grantRole(admin, "foo", "group-admin", "ADMIN"); final AggregatedHttpResponse deleted = deleteGroup(groupAdmin, "foo"); assertThat(deleted.status()).isEqualTo(HttpStatus.OK); - assertThat(deleted.headers().get("grpc-status")).isEqualTo("0"); // The group is really gone. assertThat(deleteGroup(admin, "foo").status()).isEqualTo(HttpStatus.NOT_FOUND); @@ -91,7 +90,6 @@ private static AggregatedHttpResponse createGroup(WebClient client, String group return client.prepare() .post("/api/v1/xds/groups") .queryParam("group_id", group) - .content(MediaType.JSON, "{\"name\":\"groups/" + group + "\"}") .execute().aggregate().join(); } diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsLegacyJsonCompatibilityTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsLegacyJsonCompatibilityTest.java index 01d49525e..737992ce7 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsLegacyJsonCompatibilityTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsLegacyJsonCompatibilityTest.java @@ -61,7 +61,6 @@ import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment; import io.envoyproxy.envoy.config.listener.v3.Listener; import io.envoyproxy.envoy.config.route.v3.RouteConfiguration; -import io.grpc.Status; class XdsLegacyJsonCompatibilityTest { @@ -227,25 +226,22 @@ private static AggregatedHttpResponse deleteResource(String path) { private static AggregatedHttpResponse updateEndpoint(String endpointId, ClusterLoadAssignment endpoint) throws IOException { return dogma.httpClient() - .execute(RequestHeaders.builder(HttpMethod.PATCH, + .execute(RequestHeaders.builder(HttpMethod.PUT, "/api/v1/xds/groups/" + GROUP + "/endpoints/" + endpointId) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8) + .contentType(MediaType.parse("application/yaml")) .build(), - JSON_MESSAGE_MARSHALLER.writeValueAsString(endpoint)) + XdsTestUtil.toYaml(endpoint)) .aggregate() .join(); } private static void assertOk(AggregatedHttpResponse response) { assertThat(response.status()).isSameAs(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); } private static void assertAlreadyExists(AggregatedHttpResponse response) { assertThat(response.status()).isSameAs(HttpStatus.CONFLICT); - assertThat(response.headers().get("grpc-status")) - .isEqualTo(Integer.toString(Status.ALREADY_EXISTS.getCode().value())); } } diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsResourceManagerTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsResourceManagerTest.java new file mode 100644 index 000000000..c3b191b0b --- /dev/null +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsResourceManagerTest.java @@ -0,0 +1,262 @@ +/* + * 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. + */ +package com.linecorp.centraldogma.xds.internal; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class XdsResourceManagerTest { + + // ---- normalizeYamlKeys ----------------------------------------------- + + @Test + void normalizeYamlKeys_simpleSnakeCaseKey() { + assertThat(XdsResourceManager.normalizeYamlKeys("cluster_name: foo\n")) + .isEqualTo("clusterName: foo\n"); + } + + @Test + void normalizeYamlKeys_multipleUnderscoreSegments() { + assertThat(XdsResourceManager.normalizeYamlKeys("locality_lb_endpoints: []\n")) + .isEqualTo("localityLbEndpoints: []\n"); + } + + @Test + void normalizeYamlKeys_snakeCaseValueNotConverted() { + // The value after the colon must NOT be touched, even if it looks like a snake_case + // identifier. The regex only matches at the start of a line followed by a colon. + assertThat(XdsResourceManager.normalizeYamlKeys("someField: some_snake_value\n")) + .isEqualTo("someField: some_snake_value\n"); + } + + @Test + void normalizeYamlKeys_snakeCaseKeyWithSnakeCaseValue() { + // The key portion is converted; the value is preserved verbatim. + assertThat(XdsResourceManager.normalizeYamlKeys("cluster_name: my_cluster_name\n")) + .isEqualTo("clusterName: my_cluster_name\n"); + } + + @Test + void normalizeYamlKeys_valueIdenticalToKey() { + // The value happens to be the same snake_case word as the key — only the key is + // converted. + assertThat(XdsResourceManager.normalizeYamlKeys("cluster_name: cluster_name\n")) + .isEqualTo("clusterName: cluster_name\n"); + } + + @Test + void normalizeYamlKeys_commentLineNotConverted() { + // Lines beginning with '#' do not start with [a-z], so they are never matched. + final String yaml = "# cluster_name: should stay as-is\ncluster_name: bar\n"; + assertThat(XdsResourceManager.normalizeYamlKeys(yaml)) + .isEqualTo("# cluster_name: should stay as-is\nclusterName: bar\n"); + } + + @Test + void normalizeYamlKeys_inlineCommentNotConverted() { + // The regex stops at the colon; the rest of the line (value + inline comment) is + // never part of the match and is left unchanged. + assertThat(XdsResourceManager.normalizeYamlKeys( + "cluster_name: foo # cluster_name note\n")) + .isEqualTo("clusterName: foo # cluster_name note\n"); + } + + @Test + void normalizeYamlKeys_indentedSnakeCaseKey() { + final String yaml = "edsClusterConfig:\n service_name: foo\n"; + assertThat(XdsResourceManager.normalizeYamlKeys(yaml)) + .isEqualTo("edsClusterConfig:\n serviceName: foo\n"); + } + + @Test + void normalizeYamlKeys_deeplyNestedSnakeCaseKeys() { + final String yaml = + "locality_lb_endpoints:\n" + + " lb_endpoints:\n" + + " socket_address:\n" + + " port_value: 8080\n"; + assertThat(XdsResourceManager.normalizeYamlKeys(yaml)) + .isEqualTo( + "localityLbEndpoints:\n" + + " lbEndpoints:\n" + + " socketAddress:\n" + + " portValue: 8080\n"); + } + + @Test + void normalizeYamlKeys_camelCaseKeyUnchanged() { + // camelCase keys have no underscore, so the pattern never matches them. + assertThat(XdsResourceManager.normalizeYamlKeys("clusterName: foo\n")) + .isEqualTo("clusterName: foo\n"); + } + + @Test + void normalizeYamlKeys_singleWordKeyUnchanged() { + // Single-word keys (no underscore) are not touched. + final String yaml = "name: foo\nport: 8080\n"; + assertThat(XdsResourceManager.normalizeYamlKeys(yaml)).isEqualTo(yaml); + } + + @Test + void normalizeYamlKeys_mixedCamelAndSnakeKeys() { + final String yaml = + "clusterName: already_camel\n" + + "connect_timeout: 5s\n" + + "name: foo\n"; + assertThat(XdsResourceManager.normalizeYamlKeys(yaml)) + .isEqualTo( + "clusterName: already_camel\n" + + "connectTimeout: 5s\n" + + "name: foo\n"); + } + + @Test + void normalizeYamlKeys_noSnakeCaseKeys() { + // Nothing to convert — result must be identical to input. + final String yaml = "name: foo\nclusterName: bar\n serviceName: baz\n"; + assertThat(XdsResourceManager.normalizeYamlKeys(yaml)).isEqualTo(yaml); + } + + @Test + void normalizeYamlKeys_emptyString() { + assertThat(XdsResourceManager.normalizeYamlKeys("")).isEmpty(); + } + + @Test + void normalizeYamlKeys_keyWithSpaceBeforeColon() { + // The pattern allows optional whitespace before the colon (group 3 is \s*:). + assertThat(XdsResourceManager.normalizeYamlKeys("connect_timeout : 5s\n")) + .isEqualTo("connectTimeout : 5s\n"); + } + + @Test + void normalizeYamlKeys_realWorldClusterYaml() { + // A typical cluster body a user might POST using snake_case field names. + final String input = + "# EDS cluster\n" + + "type: EDS\n" + + "connect_timeout: 5s\n" + + "eds_cluster_config:\n" + + " eds_config:\n" + + " ads: {}\n" + + " service_name: my_service_name\n"; + final String expected = + "# EDS cluster\n" + + "type: EDS\n" + + "connectTimeout: 5s\n" + + "edsClusterConfig:\n" + + " edsConfig:\n" + + " ads: {}\n" + + " serviceName: my_service_name\n"; + assertThat(XdsResourceManager.normalizeYamlKeys(input)).isEqualTo(expected); + } + + @Test + void normalizeYamlKeys_realWorldEndpointYaml() { + // A ClusterLoadAssignment body with snake_case field names and snake_case values. + final String input = + "cluster_name: my_cluster\n" + + "endpoints:\n" + + " - locality:\n" + + " region: us_east_1\n" + // 'region' is single-word, value unchanged + " lb_endpoints:\n" + + " - endpoint:\n" + + " address:\n" + + " socket_address:\n" + + " address: 127.0.0.1\n" + + " port_value: 8080\n"; + final String expected = + "clusterName: my_cluster\n" + + "endpoints:\n" + + " - locality:\n" + + " region: us_east_1\n" + + " lbEndpoints:\n" + // lb_endpoints converted + " - endpoint:\n" + + " address:\n" + + " socketAddress:\n" + + " address: 127.0.0.1\n" + + " portValue: 8080\n"; + assertThat(XdsResourceManager.normalizeYamlKeys(input)).isEqualTo(expected); + } + + // ---- injectYamlField ------------------------------------------------- + + @Test + void injectYamlField_replacesExistingField() { + final String yaml = "name: old\ntype: EDS\n"; + assertThat(XdsResourceManager.injectYamlField(yaml, "name", "new_name")) + .isEqualTo("name: new_name\ntype: EDS\n"); + } + + @Test + void injectYamlField_prependsWhenFieldAbsent() { + final String yaml = "type: EDS\n"; + assertThat(XdsResourceManager.injectYamlField(yaml, "name", "my_cluster")) + .isEqualTo("name: my_cluster\ntype: EDS\n"); + } + + @Test + void injectYamlField_preservesComments() { + final String yaml = "# important comment\nname: old\ntype: EDS\n"; + assertThat(XdsResourceManager.injectYamlField(yaml, "name", "new_name")) + .isEqualTo("# important comment\nname: new_name\ntype: EDS\n"); + } + + @Test + void injectYamlField_doesNotMatchNestedField() { + // A 'name:' that appears only as a value nested under another key is not matched + // as the top-level field; it is prepended instead. + final String yaml = "someField:\n name: nested\n"; + assertThat(XdsResourceManager.injectYamlField(yaml, "name", "top_name")) + .isEqualTo("name: top_name\nsomeField:\n name: nested\n"); + } + + @Test + void injectYamlField_replacesBlockScalarField() { + // When the existing field value is a block scalar (|), the injection must replace + // the entire block — indicator line AND content lines — not just the indicator line. + final String yaml = "name: |\n groups/foo/clusters/bar\ntype: EDS\n"; + assertThat(XdsResourceManager.injectYamlField(yaml, "name", "groups/new/clusters/new")) + .isEqualTo("name: groups/new/clusters/new\ntype: EDS\n"); + } + + @Test + void normalizeYamlKeys_blockScalarContentNotConverted() { + // Content lines of a block scalar must not be converted even if a line happens to + // look like a snake_case key. Only real mapping keys (as identified by the YAML AST) + // are touched. + final String yaml = + "filterMetadata:\n" + + " inlineString: |\n" + + " connect_timeout: 5s\n" + + " retry_on: 5xx\n"; + // 'inlineString' is already camelCase; the block scalar content lines must be + // left completely unchanged. + assertThat(XdsResourceManager.normalizeYamlKeys(yaml)).isEqualTo(yaml); + } + + @Test + void normalizeYamlKeys_blockScalarKeyConverted() { + // The key of a block scalar field IS a real mapping key and must be converted; + // the block scalar content must not be touched. + final String yaml = + "inline_string: |\n" + + " connect_timeout: 5s\n"; + assertThat(XdsResourceManager.normalizeYamlKeys(yaml)) + .isEqualTo("inlineString: |\n connect_timeout: 5s\n"); + } +} diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsTestUtil.java b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsTestUtil.java index f49aba68d..129e774c3 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsTestUtil.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsTestUtil.java @@ -15,8 +15,6 @@ */ package com.linecorp.centraldogma.xds.internal; -import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.JSON_MESSAGE_MARSHALLER; - import java.io.IOException; import java.net.URI; import java.util.concurrent.CompletableFuture; @@ -24,6 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.protobuf.Any; +import com.google.protobuf.Message; import com.google.protobuf.util.Durations; import com.linecorp.armeria.client.WebClient; @@ -68,6 +67,10 @@ public final class XdsTestUtil { static final String CONFIG_SOURCE_CLUSTER_NAME = "dogma/cluster"; + public static String toYaml(Message message) throws IOException { + return XdsResourceManager.toYamlBodyString(message); + } + public static AggregatedHttpResponse createGroup(String groupId, WebClient webClient) { return createGroupAsync(groupId, webClient).join(); } @@ -77,9 +80,8 @@ public static CompletableFuture createGroupAsync( final RequestHeaders headers = RequestHeaders.builder(HttpMethod.POST, "/api/v1/xds/groups?group_id=" + groupId) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8).build(); - return webClient.execute(headers, "{\"name\":\"groups/" + groupId + "\"}") - .aggregate(); + .build(); + return webClient.execute(headers).aggregate(); } public static AggregatedHttpResponse deleteGroup(String groupName, WebClient webClient) { @@ -202,7 +204,6 @@ static Cluster cluster(String clusterName, ConfigSource configSource, .setEdsConfig(configSource) .setServiceName(clusterName)) .setType(Cluster.DiscoveryType.EDS) - .setRespectDnsTtl(true) .build(); } @@ -273,8 +274,8 @@ public static AggregatedHttpResponse createEndpoint( "/api/v1/xds/" + groupName + "/endpoints?endpoint_id=" + endpointId) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8).build(); - return webClient.execute(headers, JSON_MESSAGE_MARSHALLER.writeValueAsString(endpoint)) + .contentType(MediaType.parse("application/yaml")).build(); + return webClient.execute(headers, toYaml(endpoint)) .aggregate().join(); } @@ -285,19 +286,19 @@ public static AggregatedHttpResponse createCluster( RequestHeaders.builder(HttpMethod.POST, "/api/v1/xds/" + groupName + "/clusters?cluster_id=" + clusterId) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8).build(); - return webClient.execute(headers, JSON_MESSAGE_MARSHALLER.writeValueAsString(cluster)) + .contentType(MediaType.parse("application/yaml")).build(); + return webClient.execute(headers, toYaml(cluster)) .aggregate().join(); } public static AggregatedHttpResponse updateCluster( String groupName, String clusterId, Cluster cluster, WebClient webClient) throws IOException { final RequestHeaders headers = - RequestHeaders.builder(HttpMethod.PATCH, + RequestHeaders.builder(HttpMethod.PUT, "/api/v1/xds/" + groupName + "/clusters/" + clusterId) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8).build(); - return webClient.execute(headers, JSON_MESSAGE_MARSHALLER.writeValueAsString(cluster)) + .contentType(MediaType.parse("application/yaml")).build(); + return webClient.execute(headers, toYaml(cluster)) .aggregate().join(); } @@ -309,19 +310,19 @@ public static AggregatedHttpResponse createListener( "/api/v1/xds/" + groupName + "/listeners?listener_id=" + listenerId) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8).build(); - return webClient.execute(headers, JSON_MESSAGE_MARSHALLER.writeValueAsString(listener)) + .contentType(MediaType.parse("application/yaml")).build(); + return webClient.execute(headers, toYaml(listener)) .aggregate().join(); } public static AggregatedHttpResponse updateListener( String groupName, String listenerId, Listener listener, WebClient webClient) throws IOException { final RequestHeaders headers = - RequestHeaders.builder(HttpMethod.PATCH, + RequestHeaders.builder(HttpMethod.PUT, "/api/v1/xds/" + groupName + "/listeners/" + listenerId) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8).build(); - return webClient.execute(headers, JSON_MESSAGE_MARSHALLER.writeValueAsString(listener)) + .contentType(MediaType.parse("application/yaml")).build(); + return webClient.execute(headers, toYaml(listener)) .aggregate().join(); } @@ -333,19 +334,19 @@ public static AggregatedHttpResponse createRoute( "/api/v1/xds/" + groupName + "/routes?route_id=" + routeId) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8).build(); - return webClient.execute(headers, JSON_MESSAGE_MARSHALLER.writeValueAsString(route)) + .contentType(MediaType.parse("application/yaml")).build(); + return webClient.execute(headers, toYaml(route)) .aggregate().join(); } public static AggregatedHttpResponse updateRoute( String groupName, String routeId, RouteConfiguration route, WebClient webClient) throws IOException { - final RequestHeaders headers = RequestHeaders.builder(HttpMethod.PATCH, + final RequestHeaders headers = RequestHeaders.builder(HttpMethod.PUT, "/api/v1/xds/" + groupName + "/routes/" + routeId) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8).build(); - return webClient.execute(headers, JSON_MESSAGE_MARSHALLER.writeValueAsString(route)) + .contentType(MediaType.parse("application/yaml")).build(); + return webClient.execute(headers, toYaml(route)) .aggregate().join(); } diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsWritePermissionTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsWritePermissionTest.java index 4d8e98a89..e65c8ca47 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsWritePermissionTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsWritePermissionTest.java @@ -18,7 +18,6 @@ import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.PASSWORD; import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.USERNAME; import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.getAccessToken; -import static com.linecorp.centraldogma.xds.internal.XdsResourceManager.JSON_MESSAGE_MARSHALLER; import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.cluster; import static org.assertj.core.api.Assertions.assertThat; @@ -67,9 +66,10 @@ void shouldRequireWriteRoleToModifyResources() throws Exception { final Cluster cluster = cluster("groups/foo/clusters/c1", 1); - // 'writer' has no role on 'foo' yet -> denied. + // 'writer' has no role on 'foo' yet -> create and delete are both denied. AggregatedHttpResponse response = createCluster(writer, "foo", "c1", cluster); - assertThat(response.headers().get("grpc-status")).isEqualTo("7"); // PERMISSION_DENIED + assertThat(response.status()).isEqualTo(HttpStatus.FORBIDDEN); + assertThat(deleteCluster(writer, "foo", "c1").status()).isEqualTo(HttpStatus.FORBIDDEN); // The denied request must not have created the resource. assertThat(getClusterStatus(admin, "foo", "c1")).isEqualTo(HttpStatus.NOT_FOUND); @@ -85,7 +85,6 @@ void shouldRequireWriteRoleToModifyResources() throws Exception { // Now 'writer' can create the cluster. response = createCluster(writer, "foo", "c1", cluster); assertThat(response.status()).isEqualTo(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); // The resource is now actually present. assertThat(getClusterStatus(admin, "foo", "c1")).isEqualTo(HttpStatus.OK); @@ -104,8 +103,7 @@ void readRoleIsInsufficientAndAdminCanWrite() throws Exception { // A READ role is insufficient to modify resources. grantRole(admin, "bar", "reader", "READ"); - assertThat(createCluster(reader, "bar", "c1", cluster).headers().get("grpc-status")) - .isEqualTo("7"); // PERMISSION_DENIED + assertThat(createCluster(reader, "bar", "c1", cluster).status()).isEqualTo(HttpStatus.FORBIDDEN); // The denied request must not have created the resource. assertThat(getClusterStatus(admin, "bar", "c1")).isEqualTo(HttpStatus.NOT_FOUND); @@ -113,7 +111,6 @@ void readRoleIsInsufficientAndAdminCanWrite() throws Exception { grantRole(admin, "bar", "group-admin", "ADMIN"); final AggregatedHttpResponse response = createCluster(groupAdmin, "bar", "c1", cluster); assertThat(response.status()).isEqualTo(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); assertThat(getClusterStatus(admin, "bar", "c1")).isEqualTo(HttpStatus.OK); } @@ -141,7 +138,6 @@ private static AggregatedHttpResponse createGroup(WebClient client, String group return client.prepare() .post("/api/v1/xds/groups") .queryParam("group_id", group) - .content(MediaType.JSON, "{\"name\":\"groups/" + group + "\"}") .execute().aggregate().join(); } @@ -150,7 +146,13 @@ private static AggregatedHttpResponse createCluster(WebClient client, String gro return client.prepare() .post("/api/v1/xds/groups/" + group + "/clusters") .queryParam("cluster_id", clusterId) - .content(MediaType.JSON, JSON_MESSAGE_MARSHALLER.writeValueAsString(cluster)) + .content(MediaType.parse("application/yaml"), XdsTestUtil.toYaml(cluster)) + .execute().aggregate().join(); + } + + private static AggregatedHttpResponse deleteCluster(WebClient client, String group, String clusterId) { + return client.prepare() + .delete("/api/v1/xds/groups/" + group + "/clusters/" + clusterId) .execute().aggregate().join(); } } diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsYamlCompatibilityTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsYamlCompatibilityTest.java index 61ab237a0..7c1670941 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsYamlCompatibilityTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsYamlCompatibilityTest.java @@ -95,9 +95,8 @@ void updateYamlCluster() throws Exception { final Cluster updated = cluster.toBuilder() .setConnectTimeout(Duration.newBuilder().setSeconds(2).build()) .build(); - final AggregatedHttpResponse response = patchCluster("foo", "yaml-update-cluster", updated); + final AggregatedHttpResponse response = putCluster("foo", "yaml-update-cluster", updated); assertThat(response.status()).isSameAs(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); // The .yaml file must be updated in place; no new .json file should appear. final Repository repo = xdsRepo("foo"); @@ -123,8 +122,6 @@ void deleteYamlCluster() throws Exception { // Delete via the HTTP API — updateOrDelete must locate and remove the .yaml file. final AggregatedHttpResponse response = deleteCluster(clusterName); assertThat(response.status()).isSameAs(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); - assertThat(response.contentUtf8()).isEqualTo("{}"); // The .yaml file must be gone. final Repository repo = xdsRepo("foo"); @@ -185,8 +182,7 @@ void jsonFileStillWorksAlongsideYaml() throws Exception { pushYamlCluster("yaml-alongside-json", yamlCluster); // Both must be served by the control plane. - final Cluster expectedJson = cluster(jsonClusterName, 1).toBuilder() - .setRespectDnsTtl(true).build(); + final Cluster expectedJson = cluster(jsonClusterName, 1); await().pollInterval(100, TimeUnit.MILLISECONDS).untilAsserted(() -> { checkClusterViaDiscovery(jsonClusterName, expectedJson, true); checkClusterViaDiscovery(yamlClusterName, yamlCluster, true); @@ -220,19 +216,19 @@ private static AggregatedHttpResponse createCluster(String group, String cluster .path("/api/v1/xds/groups/" + group + "/clusters") .queryParam("cluster_id", clusterId) .header(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .content(MediaType.JSON_UTF_8, JSON_MESSAGE_MARSHALLER.writeValueAsString(cluster)) + .content(MediaType.parse("application/yaml"), XdsTestUtil.toYaml(cluster)) .execute().aggregate().join(); } - private static AggregatedHttpResponse patchCluster(String group, String clusterId, Cluster cluster) + private static AggregatedHttpResponse putCluster(String group, String clusterId, Cluster cluster) throws Exception { final RequestHeaders headers = - RequestHeaders.builder(HttpMethod.PATCH, + RequestHeaders.builder(HttpMethod.PUT, "/api/v1/xds/groups/" + group + "/clusters/" + clusterId) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8).build(); + .contentType(MediaType.parse("application/yaml")).build(); return dogma.httpClient() - .execute(headers, JSON_MESSAGE_MARSHALLER.writeValueAsString(cluster)) + .execute(headers, XdsTestUtil.toYaml(cluster)) .aggregate().join(); } diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/KubernetesEndpointMetadataTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/KubernetesEndpointMetadataTest.java index 03cdf302b..7ba4bd23c 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/KubernetesEndpointMetadataTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/KubernetesEndpointMetadataTest.java @@ -20,7 +20,6 @@ import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.endpoint; import static com.linecorp.centraldogma.xds.k8s.v1.XdsKubernetesServiceTest.assertOk; import static com.linecorp.centraldogma.xds.k8s.v1.XdsKubernetesServiceTest.createAggregator; -import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; import java.io.IOException; @@ -217,7 +216,6 @@ private static void assertInvalidArgument(String aggregatorId, MetadataMapping.B final AggregatedHttpResponse response = createAggregator(aggregator(aggregatorId, watcher), aggregatorId, dogma.httpClient()); assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); - assertThatJson(response.contentUtf8()).node("grpc-code").isEqualTo("INVALID_ARGUMENT"); } private static String createAndGetClusterName(String aggregatorId, ServiceEndpointWatcher watcher) 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 ab433a0f6..feab285d7 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 @@ -24,7 +24,6 @@ import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.createGroup; import static com.linecorp.centraldogma.xds.internal.XdsTestUtil.endpoint; import static com.linecorp.centraldogma.xds.k8s.v1.XdsKubernetesService.K8S_ENDPOINT_AGGREGATORS_DIRECTORY; -import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; @@ -60,10 +59,12 @@ import com.linecorp.centraldogma.common.Query; import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.internal.Jackson; +import com.linecorp.centraldogma.internal.Yaml; import com.linecorp.centraldogma.server.credential.CreateCredentialRequest; import com.linecorp.centraldogma.server.internal.credential.AccessTokenCredential; import com.linecorp.centraldogma.server.storage.repository.Repository; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; +import com.linecorp.centraldogma.xds.internal.XdsTestUtil; import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment; import io.envoyproxy.envoy.config.endpoint.v3.LocalityLbEndpoints; @@ -192,22 +193,34 @@ void clearQueue() { } @Test - void invalidProperty() throws IOException { + void invalidAggregatorId() throws IOException { + // An aggregator ID that does not match the allowed pattern is rejected immediately with 400 + // without making any Kubernetes API calls. + final KubernetesEndpointAggregator aggregator = aggregator("valid-cluster", "repo-credential"); + final AggregatedHttpResponse response = createAggregator(aggregator, "@invalid!"); + assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); + } + + @Test + void invalidK8sConfigProperties() throws IOException { + // A valid aggregator ID but a K8s service that does not exist causes a 500. final String aggregatorId = "foo-cluster"; KubernetesEndpointAggregator aggregator = aggregator(aggregatorId, "invalid-service-name", "repo-credential"); AggregatedHttpResponse response = createAggregator(aggregator, aggregatorId); assertThat(response.status()).isSameAs(HttpStatus.INTERNAL_SERVER_ERROR); - assertThat(response.contentUtf8()).contains("Failed to retrieve k8s endpoints"); + // A credential that does not exist causes a 400. aggregator = aggregator(aggregatorId, "nginx-service", "invalid-credential"); response = createAggregator(aggregator, aggregatorId); assertThat(response.status()).isSameAs(HttpStatus.BAD_REQUEST); - assertThatJson(response.contentUtf8()) - .node("grpc-code").isEqualTo("INVALID_ARGUMENT") - .node("message").isEqualTo( - "failed to find credential file " + - "'/credentials/invalid-credential.json' in @xds/dogma"); + } + + @Test + void deleteAggregator_nonExistentReturns404() { + final AggregatedHttpResponse response = + deleteAggregator0("groups/foo/k8s/endpointAggregators/does-not-exist"); + assertThat(response.status()).isSameAs(HttpStatus.NOT_FOUND); } @CsvSource({ "repo-credential", "project-credential" }) @@ -289,22 +302,21 @@ static AggregatedHttpResponse createAggregator( RequestHeaders.builder(HttpMethod.POST, "/api/v1/xds/groups/foo/k8s/endpointAggregators?" + "aggregator_id=" + aggregatorId) - .contentType(MediaType.JSON_UTF_8) + .contentType(MediaType.parse("application/yaml")) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") .build(); - return webClient.blocking().execute(headers, JSON_MESSAGE_MARSHALLER.writeValueAsString(aggregator)); + return webClient.blocking().execute(headers, XdsTestUtil.toYaml(aggregator)); } static void assertOk(AggregatedHttpResponse response) { assertThat(response.status()).isSameAs(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); } static void assertAggregator( String json, KubernetesEndpointAggregator expected) throws IOException { final KubernetesEndpointAggregator.Builder responseBuilder = KubernetesEndpointAggregator.newBuilder(); - JSON_MESSAGE_MARSHALLER.mergeValue(json, responseBuilder); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(json).traverse(), responseBuilder); assertThat(responseBuilder.build()).isEqualTo(expected); } @@ -352,14 +364,37 @@ static AggregatedHttpResponse updateAggregator( KubernetesEndpointAggregator aggregator, String aggregatorId, WebClient webClient) throws IOException { final RequestHeaders headers = - RequestHeaders.builder(HttpMethod.PATCH, + RequestHeaders.builder(HttpMethod.PUT, "/api/v1/xds/groups/foo/k8s/endpointAggregators/" + aggregatorId) .set(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .contentType(MediaType.JSON_UTF_8).build(); - return webClient.execute(headers, JSON_MESSAGE_MARSHALLER.writeValueAsString(aggregator)) + .contentType(MediaType.parse("application/yaml")).build(); + return webClient.execute(headers, XdsTestUtil.toYaml(aggregator)) .aggregate().join(); } + @Test + void updateAggregator_responseHasServerDerivedName() throws IOException { + final String aggregatorId = "foo-k8s-cluster.name-check"; + final String expectedName = "groups/foo/k8s/endpointAggregators/" + aggregatorId; + final String expectedClusterName = "groups/foo/k8s/clusters/" + aggregatorId; + final KubernetesEndpointAggregator aggregator = aggregator(aggregatorId, "repo-credential"); + assertOk(createAggregator(aggregator, aggregatorId)); + + // Send an update with a mismatched name in the body — the server must override it. + final KubernetesEndpointAggregator wrongNameAggregator = + aggregator.toBuilder().setName("groups/foo/k8s/endpointAggregators/WRONG-NAME").build(); + final AggregatedHttpResponse response = + updateAggregator(wrongNameAggregator, aggregatorId, dogma.httpClient()); + assertOk(response); + assertAggregator(response.contentUtf8(), + aggregator.toBuilder() + .setName(expectedName) + .setClusterName(expectedClusterName) + .build()); + + assertOk(deleteAggregator0(aggregator.getName())); + } + @CsvSource({ "repo-credential", "project-credential" }) @ParameterizedTest void deleteAggregator(String credentialId) throws IOException { @@ -372,7 +407,6 @@ void deleteAggregator(String credentialId) throws IOException { checkEndpointsViaDiscoveryRequest(dogma.httpClient().uri(), loadAssignment, clusterName); response = deleteAggregator0(aggregator.getName()); assertOk(response); - assertThat(response.contentUtf8()).isEqualTo("{}"); checkEndpointsViaDiscoveryRequest(dogma.httpClient().uri(), null, clusterName); } 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 7c500f749..52d236813 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 @@ -33,7 +33,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import com.google.protobuf.Any; -import com.google.protobuf.Empty; import com.google.protobuf.InvalidProtocolBufferException; import com.linecorp.armeria.client.grpc.GrpcClients; @@ -42,8 +41,8 @@ import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.centraldogma.internal.Yaml; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; -import com.linecorp.centraldogma.xds.listener.v1.XdsListenerServiceGrpc.XdsListenerServiceBlockingStub; import io.envoyproxy.controlplane.cache.Resources.V3; import io.envoyproxy.envoy.config.listener.v3.Listener; @@ -81,7 +80,7 @@ void createListenerViaHttp() throws Exception { 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); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), listenerBuilder); final Listener actualListener = listenerBuilder.build(); final String listenerName = "groups/foo/listeners/foo-listener.1"; assertThat(actualListener).isEqualTo(listener.toBuilder().setName(listenerName).build()); @@ -90,7 +89,6 @@ void createListenerViaHttp() throws Exception { private static void assertOk(AggregatedHttpResponse response) { assertThat(response.status()).isSameAs(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); } private static void checkResourceViaDiscoveryRequest(Listener actualListener, String resourceName, @@ -146,7 +144,7 @@ void updateListenerViaHttp() throws Exception { 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); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), listenerBuilder); final Listener actualListener = listenerBuilder.build(); final String listenerName = "groups/foo/listeners/foo-listener.2"; assertThat(actualListener).isEqualTo(listener.toBuilder().setName(listenerName).build()); @@ -158,7 +156,7 @@ void updateListenerViaHttp() throws Exception { 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); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), listenerBuilder2); final Listener actualListener2 = listenerBuilder2.build(); assertThat(actualListener2).isEqualTo(updatingListener.toBuilder().setName(listenerName).build()); checkResourceViaDiscoveryRequest(actualListener2, listenerName, true); @@ -182,7 +180,6 @@ void deleteListenerViaHttp() throws Exception { response = deleteListener(listenerName); assertOk(response); - assertThat(response.contentUtf8()).isEqualTo("{}"); checkResourceViaDiscoveryRequest(actualListener, listenerName, false); } @@ -193,36 +190,4 @@ private static AggregatedHttpResponse deleteListener(String listenerName) { .build(); return dogma.httpClient().execute(headers).aggregate().join(); } - - @Test - void viaStub() { - final XdsListenerServiceBlockingStub client = - GrpcClients.builder(dogma.httpClient().uri()) - .setHeader(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .build(XdsListenerServiceBlockingStub.class); - final Listener listener = exampleListener("this_listener_name_will_be_ignored_and_replaced", - "groups/foo/routes/foo-route", "stats"); - Listener response = client.createListener( - CreateListenerRequest.newBuilder() - .setParent("groups/foo") - .setListenerId("foo-listener.5.6") - .setListener(listener) - .build()); - final String listenerName = "groups/foo/listeners/foo-listener.5.6"; - assertThat(response).isEqualTo(listener.toBuilder().setName(listenerName).build()); - - final Listener updatingListener = listener.toBuilder() - .setStatPrefix("updated_stats") - .setName(listenerName).build(); - response = client.updateListener(UpdateListenerRequest.newBuilder() - .setListener(updatingListener) - .build()); - assertThat(response).isEqualTo(updatingListener); - - // No exception is thrown. - final Empty ignored = client.deleteListener( - DeleteListenerRequest.newBuilder() - .setName(listenerName) - .build()); - } } 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 6bf201053..700c8497f 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 @@ -33,7 +33,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import com.google.protobuf.Any; -import com.google.protobuf.Empty; import com.google.protobuf.InvalidProtocolBufferException; import com.linecorp.armeria.client.grpc.GrpcClients; @@ -42,8 +41,8 @@ import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.RequestHeaders; +import com.linecorp.centraldogma.internal.Yaml; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; -import com.linecorp.centraldogma.xds.route.v1.XdsRouteServiceGrpc.XdsRouteServiceBlockingStub; import io.envoyproxy.controlplane.cache.Resources.V3; import io.envoyproxy.envoy.config.route.v3.RouteConfiguration; @@ -81,7 +80,7 @@ void createRouteViaHttp() throws Exception { 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); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), routeBuilder); final RouteConfiguration actualRoute = routeBuilder.build(); final String routeName = "groups/foo/routes/foo-route.1"; assertThat(actualRoute).isEqualTo(route.toBuilder().setName(routeName).build()); @@ -90,7 +89,6 @@ void createRouteViaHttp() throws Exception { private static void assertOk(AggregatedHttpResponse response) { assertThat(response.status()).isSameAs(HttpStatus.OK); - assertThat(response.headers().get("grpc-status")).isEqualTo("0"); } private static void checkResourceViaDiscoveryRequest(RouteConfiguration actualRoute, String resourceName, @@ -145,7 +143,7 @@ void updateRouteViaHttp() throws Exception { 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); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), routeBuilder); final RouteConfiguration actualRoute = routeBuilder.build(); final String routeName = "groups/foo/routes/foo-route.2"; assertThat(actualRoute).isEqualTo(route.toBuilder().setName(routeName).build()); @@ -157,7 +155,7 @@ void updateRouteViaHttp() throws Exception { 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); + JSON_MESSAGE_MARSHALLER.mergeValue(Yaml.readTree(response.contentUtf8()).traverse(), routeBuilder2); final RouteConfiguration actualRoute2 = routeBuilder2.build(); assertThat(actualRoute2).isEqualTo(updatingRoute.toBuilder().setName(routeName).build()); checkResourceViaDiscoveryRequest(actualRoute2, routeName, true); @@ -181,7 +179,6 @@ void deleteRouteViaHttp() throws Exception { response = deleteRoute(routeName); assertOk(response); - assertThat(response.contentUtf8()).isEqualTo("{}"); checkResourceViaDiscoveryRequest(actualRoute, routeName, false); } @@ -192,36 +189,4 @@ private static AggregatedHttpResponse deleteRoute(String routeName) { .build(); return dogma.httpClient().execute(headers).aggregate().join(); } - - @Test - void viaStub() { - final XdsRouteServiceBlockingStub client = - GrpcClients.builder(dogma.httpClient().uri()) - .setHeader(HttpHeaderNames.AUTHORIZATION, "Bearer anonymous") - .build(XdsRouteServiceBlockingStub.class); - final RouteConfiguration route = routeConfiguration("this_route_name_will_be_ignored_and_replaced", - "groups/foo/clusters/foo-cluster"); - RouteConfiguration response = client.createRoute( - CreateRouteRequest.newBuilder() - .setParent("groups/foo") - .setRouteId("foo-route.5.6") - .setRoute(route) - .build()); - final String routeName = "groups/foo/routes/foo-route.5.6"; - assertThat(response).isEqualTo(route.toBuilder().setName(routeName).build()); - - final RouteConfiguration updatingRoute = route.toBuilder() - .addInternalOnlyHeaders("internal") - .setName(routeName).build(); - response = client.updateRoute(UpdateRouteRequest.newBuilder() - .setRoute(updatingRoute) - .build()); - assertThat(response).isEqualTo(updatingRoute); - - // No exception is thrown. - final Empty ignored = client.deleteRoute( - DeleteRouteRequest.newBuilder() - .setName(routeName) - .build()); - } } From 509a4160be9f227c3634403b1aa6a193f999c24f Mon Sep 17 00:00:00 2001 From: minwoox Date: Tue, 21 Jul 2026 13:17:56 +0900 Subject: [PATCH 2/6] AI comment --- .../xds/cluster/v1/XdsClusterService.java | 6 +- .../xds/endpoint/v1/XdsEndpointService.java | 8 +- .../RequiresXdsGroupRoleDecorator.java | 9 +- .../xds/internal/XdsResourceManager.java | 125 ++--------- .../xds/k8s/v1/XdsKubernetesService.java | 7 +- .../xds/listener/v1/XdsListenerService.java | 6 +- .../xds/route/v1/XdsRouteService.java | 6 +- .../xds/internal/XdsResourceManagerTest.java | 202 +----------------- 8 files changed, 36 insertions(+), 333 deletions(-) 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 70c77977c..a168fac5e 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 @@ -84,8 +84,7 @@ public CompletableFuture createCluster( "Invalid request body: " + e.getMessage())); } final String createSummary = isNullOrEmpty(summary) ? "Create cluster: " + clusterName : summary; - final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); - final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", clusterName); + final String bodyToStore = XdsResourceManager.injectYamlField(body, "name", clusterName); return xdsResourceManager.push(group, clusterName, CLUSTERS_DIRECTORY + clusterId + ".yaml", createSummary, currentAuthor(), true, bodyToStore); } @@ -117,8 +116,7 @@ public CompletableFuture updateCluster( "Invalid request body: " + e.getMessage())); } final String updateSummary = isNullOrEmpty(summary) ? "Update cluster: " + clusterName : summary; - final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); - final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", clusterName); + final String bodyToStore = XdsResourceManager.injectYamlField(body, "name", clusterName); return xdsResourceManager.update(group, clusterName, updateSummary, currentAuthor(), bodyToStore); } 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 dcbc7e7a4..8ed15216f 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 @@ -100,9 +100,7 @@ public CompletableFuture createEndpoint( "Invalid request body: " + e.getMessage())); } final String createSummary = isNullOrEmpty(summary) ? "Create endpoint: " + endpointName : summary; - final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); - final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, - "clusterName", clusterName); + final String bodyToStore = XdsResourceManager.injectYamlField(body, "clusterName", clusterName); return xdsResourceManager.push(group, endpointName, ENDPOINTS_DIRECTORY + endpointId + ".yaml", createSummary, currentAuthor(), true, bodyToStore); } @@ -136,9 +134,7 @@ public CompletableFuture updateEndpoint( "Invalid request body: " + e.getMessage())); } final String updateSummary = isNullOrEmpty(summary) ? "Update endpoint: " + endpointName : summary; - final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); - final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, - "clusterName", clusterName); + final String bodyToStore = XdsResourceManager.injectYamlField(body, "clusterName", clusterName); return xdsResourceManager.update(group, endpointName, updateSummary, currentAuthor(), bodyToStore); } diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRoleDecorator.java b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRoleDecorator.java index b01920dcb..80b40c1c3 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRoleDecorator.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRoleDecorator.java @@ -61,6 +61,11 @@ public final class RequiresXdsGroupRoleDecorator extends SimpleDecoratingHttpSer @Override public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exception { + final User user = AuthUtil.currentUser(ctx); + if (user == null) { + return errorResponse(HttpStatus.UNAUTHORIZED, "Authentication required"); + } + final String group = ctx.pathParam("group"); if (isNullOrEmpty(group)) { return errorResponse(HttpStatus.BAD_REQUEST, "group path variable is missing"); @@ -72,10 +77,6 @@ public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exc return errorResponse(HttpStatus.NOT_FOUND, "Group not found: " + group); } - final User user = AuthUtil.currentUser(ctx); - if (user == null) { - return errorResponse(HttpStatus.UNAUTHORIZED, "Authentication required"); - } if (user.isSystemAdmin()) { return unwrap().serve(ctx, req); } 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 c3e5008f2..1f9902f46 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 @@ -20,24 +20,17 @@ import static java.util.Objects.requireNonNull; import java.io.IOException; -import java.io.StringReader; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.util.ArrayList; -import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.function.Function; +import java.util.regex.Matcher; import java.util.regex.Pattern; import org.curioswitch.common.protobuf.json.MessageMarshaller; import org.jspecify.annotations.Nullable; import org.reflections.Reflections; import org.reflections.scanners.SubTypesScanner; -import org.yaml.snakeyaml.nodes.MappingNode; -import org.yaml.snakeyaml.nodes.Node; -import org.yaml.snakeyaml.nodes.NodeTuple; -import org.yaml.snakeyaml.nodes.ScalarNode; -import org.yaml.snakeyaml.nodes.SequenceNode; import com.fasterxml.jackson.databind.JsonNode; import com.google.common.collect.ImmutableList; @@ -169,119 +162,29 @@ public static T parseYaml(String body, Message.Builder build return (T) builder.build(); } - // Matches a snake_case identifier: at least one underscore, starts with lowercase. - // Used to detect mapping keys that need camelCase conversion. - private static final Pattern SNAKE_CASE_IDENTIFIER = - Pattern.compile("[a-z][a-z0-9]*(?:_[a-z0-9]+)+"); - - /** - * Converts all snake_case mapping keys in the given YAML string to camelCase, preserving - * values, comments, block scalars, and formatting. Uses snakeyaml's AST to locate real - * mapping keys, so block-scalar content lines are never mistaken for keys. - * - *

Call this on user-supplied bodies before storing them so that all stored YAML uses - * camelCase keys consistently, matching what the proto3 JSON serializer produces. - */ - public static String normalizeYamlKeys(String yaml) { - if (!yaml.contains("_")) { - return yaml; - } - final Node root = composeYaml(yaml); - if (root == null) { - return yaml; - } - final List ranges = new ArrayList<>(); - collectSnakeCaseKeyRanges(root, ranges); - if (ranges.isEmpty()) { - return yaml; - } - // Replace from end to start so earlier offsets stay valid. - ranges.sort((a, b) -> Integer.compare(b[0], a[0])); - final StringBuilder sb = new StringBuilder(yaml); - for (int[] range : ranges) { - sb.replace(range[0], range[1], snakeToCamel(yaml.substring(range[0], range[1]))); - } - return sb.toString(); - } - - private static void collectSnakeCaseKeyRanges(Node node, List ranges) { - if (node instanceof MappingNode) { - for (NodeTuple tuple : ((MappingNode) node).getValue()) { - final Node keyNode = tuple.getKeyNode(); - if (keyNode instanceof ScalarNode) { - final String key = ((ScalarNode) keyNode).getValue(); - if (SNAKE_CASE_IDENTIFIER.matcher(key).matches()) { - final int start = keyNode.getStartMark().getIndex(); - // Use key.length() rather than endMark to avoid trailing-whitespace - // ambiguity (e.g. "key : value" where the space before ':' is trimmed - // from the value but may or may not be included in endMark). - ranges.add(new int[] { start, start + key.length() }); - } - } - // Recurse into the value — never into ScalarNode leaves (block scalar content). - collectSnakeCaseKeyRanges(tuple.getValueNode(), ranges); - } - } else if (node instanceof SequenceNode) { - for (Node item : ((SequenceNode) node).getValue()) { - collectSnakeCaseKeyRanges(item, ranges); - } - } - } - - private static String snakeToCamel(String snake) { - final StringBuilder sb = new StringBuilder(snake.length()); - boolean upper = false; - for (int i = 0; i < snake.length(); i++) { - final char c = snake.charAt(i); - if (c == '_') { - upper = true; - } else { - sb.append(upper ? Character.toUpperCase(c) : c); - upper = false; - } - } - return sb.toString(); - } - /** * Injects or replaces a top-level YAML field in the given YAML string, preserving all other * content including comments. If {@code fieldName} is not found as a top-level key, it is * prepended. Uses snakeyaml's AST so that block-scalar values (e.g. {@code name: |}) are * replaced in their entirety — not just the indicator line. - * - *

Call {@link #normalizeYamlKeys(String)} before this method so that the key name is - * already in camelCase. */ public static String injectYamlField(String yaml, String fieldName, String fieldValue) { - final Node root = composeYaml(yaml); - if (root instanceof MappingNode) { - for (NodeTuple tuple : ((MappingNode) root).getValue()) { - final Node keyNode = tuple.getKeyNode(); - if (keyNode instanceof ScalarNode && - fieldName.equals(((ScalarNode) keyNode).getValue())) { - final int keyStart = keyNode.getStartMark().getIndex(); - // End of the value node; skip one trailing newline so we don't leave a blank - // line when the value's endMark lands on the '\n' that separates entries. - int valueEnd = tuple.getValueNode().getEndMark().getIndex(); - if (valueEnd < yaml.length() && yaml.charAt(valueEnd) == '\n') { - valueEnd++; - } - return yaml.substring(0, keyStart) + - fieldName + ": " + fieldValue + '\n' + - yaml.substring(valueEnd); - } - } + final String replacement = fieldName + ": " + fieldValue + '\n'; + // Match a top-level key (no leading whitespace) in either camelCase or snake_case form, + // plus its entire value — including any indented block-scalar continuation lines. + final Pattern pattern = Pattern.compile( + "^(?:" + Pattern.quote(fieldName) + '|' + + Pattern.quote(camelToSnake(fieldName)) + ")\\s*:.*\\n(?:[ \\t]+.*\\n)*", + Pattern.MULTILINE); + final Matcher matcher = pattern.matcher(yaml); + if (matcher.find()) { + return matcher.replaceFirst(Matcher.quoteReplacement(replacement)); } - return fieldName + ": " + fieldValue + '\n' + yaml; + return replacement + yaml; } - @Nullable - private static Node composeYaml(String yaml) { - try { - return new org.yaml.snakeyaml.Yaml().compose(new StringReader(yaml)); - } catch (Exception ignored) { - return null; - } + private static String camelToSnake(String camel) { + return camel.replaceAll("([A-Z])", "_$1").toLowerCase(); } public CompletableFuture push( 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 e3110d99e..ad2eb4b01 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 @@ -158,9 +158,7 @@ public CompletableFuture createKubernetesEndpointAggregator( final String aggregatorFileName = K8S_ENDPOINT_AGGREGATORS_DIRECTORY + aggregatorId + ".yaml"; final String createSummary = isNullOrEmpty(summary) ? "Create kubernetes endpoint: " + kubernetesEndpointName : summary; - final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); - String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", - kubernetesEndpointName); + String bodyToStore = XdsResourceManager.injectYamlField(body, "name", kubernetesEndpointName); bodyToStore = XdsResourceManager.injectYamlField(bodyToStore, "clusterName", clusterName); final String finalBodyToStore = bodyToStore; return validateKubernetesEndpointAndPushHttp( @@ -213,8 +211,7 @@ public CompletableFuture updateKubernetesEndpointAggregator( "Update kubernetes endpoint aggregator: " + aggregatorName : summary; // Both name and clusterName are server-derived; inject them so the stored YAML is always correct // regardless of what the client sent in the body. - final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); - String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", aggregatorName); + String bodyToStore = XdsResourceManager.injectYamlField(body, "name", aggregatorName); bodyToStore = XdsResourceManager.injectYamlField(bodyToStore, "clusterName", clusterName); final String finalBodyToStore = bodyToStore; return validateKubernetesEndpointAndPushHttp( 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 f821a6e3a..e259b54d5 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 @@ -84,8 +84,7 @@ public CompletableFuture createListener( "Invalid request body: " + e.getMessage())); } final String createSummary = isNullOrEmpty(summary) ? "Create listener: " + listenerName : summary; - final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); - final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", listenerName); + final String bodyToStore = XdsResourceManager.injectYamlField(body, "name", listenerName); return xdsResourceManager.push(group, listenerName, LISTENERS_DIRECTORY + listenerId + ".yaml", createSummary, currentAuthor(), true, bodyToStore); @@ -118,8 +117,7 @@ public CompletableFuture updateListener( "Invalid request body: " + e.getMessage())); } final String updateSummary = isNullOrEmpty(summary) ? "Update listener: " + listenerName : summary; - final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); - final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", listenerName); + final String bodyToStore = XdsResourceManager.injectYamlField(body, "name", listenerName); return xdsResourceManager.update(group, listenerName, updateSummary, currentAuthor(), bodyToStore); } 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 437b51c86..68d3056ab 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 @@ -84,8 +84,7 @@ public CompletableFuture createRoute( "Invalid request body: " + e.getMessage())); } final String createSummary = isNullOrEmpty(summary) ? "Create route: " + routeName : summary; - final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); - final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", routeName); + final String bodyToStore = XdsResourceManager.injectYamlField(body, "name", routeName); return xdsResourceManager.push(group, routeName, ROUTES_DIRECTORY + routeId + ".yaml", createSummary, currentAuthor(), true, bodyToStore); } @@ -117,8 +116,7 @@ public CompletableFuture updateRoute( "Invalid request body: " + e.getMessage())); } final String updateSummary = isNullOrEmpty(summary) ? "Update route: " + routeName : summary; - final String normalizedBody = XdsResourceManager.normalizeYamlKeys(body); - final String bodyToStore = XdsResourceManager.injectYamlField(normalizedBody, "name", routeName); + final String bodyToStore = XdsResourceManager.injectYamlField(body, "name", routeName); return xdsResourceManager.update(group, routeName, updateSummary, currentAuthor(), bodyToStore); } diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsResourceManagerTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsResourceManagerTest.java index c3b191b0b..92b8a57dd 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsResourceManagerTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsResourceManagerTest.java @@ -21,178 +21,6 @@ class XdsResourceManagerTest { - // ---- normalizeYamlKeys ----------------------------------------------- - - @Test - void normalizeYamlKeys_simpleSnakeCaseKey() { - assertThat(XdsResourceManager.normalizeYamlKeys("cluster_name: foo\n")) - .isEqualTo("clusterName: foo\n"); - } - - @Test - void normalizeYamlKeys_multipleUnderscoreSegments() { - assertThat(XdsResourceManager.normalizeYamlKeys("locality_lb_endpoints: []\n")) - .isEqualTo("localityLbEndpoints: []\n"); - } - - @Test - void normalizeYamlKeys_snakeCaseValueNotConverted() { - // The value after the colon must NOT be touched, even if it looks like a snake_case - // identifier. The regex only matches at the start of a line followed by a colon. - assertThat(XdsResourceManager.normalizeYamlKeys("someField: some_snake_value\n")) - .isEqualTo("someField: some_snake_value\n"); - } - - @Test - void normalizeYamlKeys_snakeCaseKeyWithSnakeCaseValue() { - // The key portion is converted; the value is preserved verbatim. - assertThat(XdsResourceManager.normalizeYamlKeys("cluster_name: my_cluster_name\n")) - .isEqualTo("clusterName: my_cluster_name\n"); - } - - @Test - void normalizeYamlKeys_valueIdenticalToKey() { - // The value happens to be the same snake_case word as the key — only the key is - // converted. - assertThat(XdsResourceManager.normalizeYamlKeys("cluster_name: cluster_name\n")) - .isEqualTo("clusterName: cluster_name\n"); - } - - @Test - void normalizeYamlKeys_commentLineNotConverted() { - // Lines beginning with '#' do not start with [a-z], so they are never matched. - final String yaml = "# cluster_name: should stay as-is\ncluster_name: bar\n"; - assertThat(XdsResourceManager.normalizeYamlKeys(yaml)) - .isEqualTo("# cluster_name: should stay as-is\nclusterName: bar\n"); - } - - @Test - void normalizeYamlKeys_inlineCommentNotConverted() { - // The regex stops at the colon; the rest of the line (value + inline comment) is - // never part of the match and is left unchanged. - assertThat(XdsResourceManager.normalizeYamlKeys( - "cluster_name: foo # cluster_name note\n")) - .isEqualTo("clusterName: foo # cluster_name note\n"); - } - - @Test - void normalizeYamlKeys_indentedSnakeCaseKey() { - final String yaml = "edsClusterConfig:\n service_name: foo\n"; - assertThat(XdsResourceManager.normalizeYamlKeys(yaml)) - .isEqualTo("edsClusterConfig:\n serviceName: foo\n"); - } - - @Test - void normalizeYamlKeys_deeplyNestedSnakeCaseKeys() { - final String yaml = - "locality_lb_endpoints:\n" + - " lb_endpoints:\n" + - " socket_address:\n" + - " port_value: 8080\n"; - assertThat(XdsResourceManager.normalizeYamlKeys(yaml)) - .isEqualTo( - "localityLbEndpoints:\n" + - " lbEndpoints:\n" + - " socketAddress:\n" + - " portValue: 8080\n"); - } - - @Test - void normalizeYamlKeys_camelCaseKeyUnchanged() { - // camelCase keys have no underscore, so the pattern never matches them. - assertThat(XdsResourceManager.normalizeYamlKeys("clusterName: foo\n")) - .isEqualTo("clusterName: foo\n"); - } - - @Test - void normalizeYamlKeys_singleWordKeyUnchanged() { - // Single-word keys (no underscore) are not touched. - final String yaml = "name: foo\nport: 8080\n"; - assertThat(XdsResourceManager.normalizeYamlKeys(yaml)).isEqualTo(yaml); - } - - @Test - void normalizeYamlKeys_mixedCamelAndSnakeKeys() { - final String yaml = - "clusterName: already_camel\n" + - "connect_timeout: 5s\n" + - "name: foo\n"; - assertThat(XdsResourceManager.normalizeYamlKeys(yaml)) - .isEqualTo( - "clusterName: already_camel\n" + - "connectTimeout: 5s\n" + - "name: foo\n"); - } - - @Test - void normalizeYamlKeys_noSnakeCaseKeys() { - // Nothing to convert — result must be identical to input. - final String yaml = "name: foo\nclusterName: bar\n serviceName: baz\n"; - assertThat(XdsResourceManager.normalizeYamlKeys(yaml)).isEqualTo(yaml); - } - - @Test - void normalizeYamlKeys_emptyString() { - assertThat(XdsResourceManager.normalizeYamlKeys("")).isEmpty(); - } - - @Test - void normalizeYamlKeys_keyWithSpaceBeforeColon() { - // The pattern allows optional whitespace before the colon (group 3 is \s*:). - assertThat(XdsResourceManager.normalizeYamlKeys("connect_timeout : 5s\n")) - .isEqualTo("connectTimeout : 5s\n"); - } - - @Test - void normalizeYamlKeys_realWorldClusterYaml() { - // A typical cluster body a user might POST using snake_case field names. - final String input = - "# EDS cluster\n" + - "type: EDS\n" + - "connect_timeout: 5s\n" + - "eds_cluster_config:\n" + - " eds_config:\n" + - " ads: {}\n" + - " service_name: my_service_name\n"; - final String expected = - "# EDS cluster\n" + - "type: EDS\n" + - "connectTimeout: 5s\n" + - "edsClusterConfig:\n" + - " edsConfig:\n" + - " ads: {}\n" + - " serviceName: my_service_name\n"; - assertThat(XdsResourceManager.normalizeYamlKeys(input)).isEqualTo(expected); - } - - @Test - void normalizeYamlKeys_realWorldEndpointYaml() { - // A ClusterLoadAssignment body with snake_case field names and snake_case values. - final String input = - "cluster_name: my_cluster\n" + - "endpoints:\n" + - " - locality:\n" + - " region: us_east_1\n" + // 'region' is single-word, value unchanged - " lb_endpoints:\n" + - " - endpoint:\n" + - " address:\n" + - " socket_address:\n" + - " address: 127.0.0.1\n" + - " port_value: 8080\n"; - final String expected = - "clusterName: my_cluster\n" + - "endpoints:\n" + - " - locality:\n" + - " region: us_east_1\n" + - " lbEndpoints:\n" + // lb_endpoints converted - " - endpoint:\n" + - " address:\n" + - " socketAddress:\n" + - " address: 127.0.0.1\n" + - " portValue: 8080\n"; - assertThat(XdsResourceManager.normalizeYamlKeys(input)).isEqualTo(expected); - } - // ---- injectYamlField ------------------------------------------------- @Test @@ -235,28 +63,12 @@ void injectYamlField_replacesBlockScalarField() { } @Test - void normalizeYamlKeys_blockScalarContentNotConverted() { - // Content lines of a block scalar must not be converted even if a line happens to - // look like a snake_case key. Only real mapping keys (as identified by the YAML AST) - // are touched. - final String yaml = - "filterMetadata:\n" + - " inlineString: |\n" + - " connect_timeout: 5s\n" + - " retry_on: 5xx\n"; - // 'inlineString' is already camelCase; the block scalar content lines must be - // left completely unchanged. - assertThat(XdsResourceManager.normalizeYamlKeys(yaml)).isEqualTo(yaml); - } - - @Test - void normalizeYamlKeys_blockScalarKeyConverted() { - // The key of a block scalar field IS a real mapping key and must be converted; - // the block scalar content must not be touched. - final String yaml = - "inline_string: |\n" + - " connect_timeout: 5s\n"; - assertThat(XdsResourceManager.normalizeYamlKeys(yaml)) - .isEqualTo("inlineString: |\n connect_timeout: 5s\n"); + void injectYamlField_replacesSnakeCaseKey() { + // A user-supplied snake_case key (e.g. cluster_name) must be found, replaced, and + // rewritten as the canonical camelCase field name. + final String yaml = "cluster_name: old\nendpoints: []\n"; + assertThat(XdsResourceManager.injectYamlField(yaml, "clusterName", + "groups/g1/clusters/ep1")) + .isEqualTo("clusterName: groups/g1/clusters/ep1\nendpoints: []\n"); } } From 741077d4c0e667073cb40a03fa054ce5607280cd Mon Sep 17 00:00:00 2001 From: minwoox Date: Tue, 21 Jul 2026 13:35:00 +0900 Subject: [PATCH 3/6] AI comment --- .../xds/internal/XdsResourceManager.java | 4 +++- .../xds/internal/XdsResourceManagerTest.java | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) 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 1f9902f46..251b43780 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 @@ -172,9 +172,11 @@ public static String injectYamlField(String yaml, String fieldName, String field final String replacement = fieldName + ": " + fieldValue + '\n'; // Match a top-level key (no leading whitespace) in either camelCase or snake_case form, // plus its entire value — including any indented block-scalar continuation lines. + // (?:\r?\n|\z) handles LF, CRLF, and a missing trailing newline at end of file. final Pattern pattern = Pattern.compile( "^(?:" + Pattern.quote(fieldName) + '|' + - Pattern.quote(camelToSnake(fieldName)) + ")\\s*:.*\\n(?:[ \\t]+.*\\n)*", + Pattern.quote(camelToSnake(fieldName)) + + ")\\s*:.*(?:\\r?\\n|\\z)(?:[ \\t]+.*(?:\\r?\\n|\\z))*", Pattern.MULTILINE); final Matcher matcher = pattern.matcher(yaml); if (matcher.find()) { diff --git a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsResourceManagerTest.java b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsResourceManagerTest.java index 92b8a57dd..d1fbcfcf7 100644 --- a/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsResourceManagerTest.java +++ b/xds/src/test/java/com/linecorp/centraldogma/xds/internal/XdsResourceManagerTest.java @@ -71,4 +71,28 @@ void injectYamlField_replacesSnakeCaseKey() { "groups/g1/clusters/ep1")) .isEqualTo("clusterName: groups/g1/clusters/ep1\nendpoints: []\n"); } + + @Test + void injectYamlField_handlesCrlfLineEndings() { + // CRLF bodies must find the existing key rather than prepending a duplicate. + final String yaml = "name: old\r\ntype: EDS\r\n"; + assertThat(XdsResourceManager.injectYamlField(yaml, "name", "new_name")) + .isEqualTo("name: new_name\ntype: EDS\r\n"); + } + + @Test + void injectYamlField_handlesNoTrailingNewline() { + // A body with no trailing newline must still find and replace the existing key. + final String yaml = "name: old"; + assertThat(XdsResourceManager.injectYamlField(yaml, "name", "new_name")) + .isEqualTo("name: new_name\n"); + } + + @Test + void injectYamlField_prependsWhenFieldAbsentAndNoTrailingNewline() { + // When the field is absent and the body has no trailing newline, prepend normally. + final String yaml = "type: EDS"; + assertThat(XdsResourceManager.injectYamlField(yaml, "name", "my_cluster")) + .isEqualTo("name: my_cluster\ntype: EDS"); + } } From 18ce744a9f45fae3fa185ba8108c732608bec054 Mon Sep 17 00:00:00 2001 From: minwoox Date: Tue, 21 Jul 2026 14:23:13 +0900 Subject: [PATCH 4/6] Cleanup --- .../xds/cluster/v1/XdsClusterService.java | 10 +++++--- .../xds/endpoint/v1/XdsEndpointService.java | 22 +++++++++++------ .../xds/internal/ControlPlaneService.java | 24 ++++++++++--------- .../xds/internal/RequiresXdsGroupRole.java | 6 ++--- .../RequiresXdsGroupRoleDecorator.java | 9 ++++--- .../xds/internal/XdsResourceManager.java | 12 +++++----- .../xds/k8s/v1/XdsKubernetesService.java | 4 +++- .../xds/listener/v1/XdsListenerService.java | 10 +++++--- .../xds/route/v1/XdsRouteService.java | 10 +++++--- 9 files changed, 65 insertions(+), 42 deletions(-) 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 a168fac5e..6abecd432 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 @@ -75,7 +75,7 @@ public CompletableFuture createCluster( XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, "Invalid cluster ID: " + clusterId)); } - final String clusterName = "groups/" + group + CLUSTERS_DIRECTORY + clusterId; + final String clusterName = clusterName(group, clusterId); try { XdsResourceManager.parseYaml(body, Cluster.newBuilder()); } catch (IOException e) { @@ -102,7 +102,7 @@ public CompletableFuture updateCluster( @Param("cluster_id") String clusterId, @Param("summary") @Nullable String summary, String body) { - final String clusterName = "groups/" + group + "/clusters/" + clusterId; + final String clusterName = clusterName(group, clusterId); if (!CLUSTER_NAME_PATTERN.matcher(clusterName).matches()) { return CompletableFuture.completedFuture( XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, @@ -131,7 +131,7 @@ public CompletableFuture deleteCluster( @Param("group") String group, @Param("cluster_id") String clusterId, @Param("summary") @Nullable String summary) { - final String clusterName = "groups/" + group + "/clusters/" + clusterId; + final String clusterName = clusterName(group, clusterId); if (!CLUSTER_NAME_PATTERN.matcher(clusterName).matches()) { return CompletableFuture.completedFuture( XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, @@ -140,4 +140,8 @@ public CompletableFuture deleteCluster( final String deleteSummary = isNullOrEmpty(summary) ? "Delete cluster: " + clusterName : summary; return xdsResourceManager.delete(group, clusterName, deleteSummary, currentAuthor()); } + + private static String clusterName(String group, String clusterId) { + return "groups/" + group + CLUSTERS_DIRECTORY + clusterId; + } } 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 8ed15216f..022b2f72a 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 @@ -90,8 +90,8 @@ public CompletableFuture createEndpoint( XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, "Invalid endpoint ID: " + endpointId)); } - final String endpointName = "groups/" + group + ENDPOINTS_DIRECTORY + endpointId; - final String clusterName = "groups/" + group + CLUSTERS_DIRECTORY + endpointId; + final String endpointName = endpointName(group, endpointId); + final String clusterName = clusterName(group, endpointId); try { XdsResourceManager.parseYaml(body, ClusterLoadAssignment.newBuilder()); } catch (IOException e) { @@ -118,14 +118,14 @@ public CompletableFuture updateEndpoint( @Param("endpoint_id") String endpointId, @Param("summary") @Nullable String summary, String body) { - final String endpointName = "groups/" + group + "/endpoints/" + endpointId; + final String endpointName = endpointName(group, endpointId); + final String clusterName = clusterName(group, endpointId); final Matcher matcher = ENDPOINT_NAME_PATTERN.matcher(endpointName); if (!matcher.matches()) { return CompletableFuture.completedFuture( XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, "Invalid endpoint name: " + endpointName)); } - final String clusterName = "groups/" + group + CLUSTERS_DIRECTORY + endpointId; try { XdsResourceManager.parseYaml(body, ClusterLoadAssignment.newBuilder()); } catch (IOException e) { @@ -149,7 +149,7 @@ public CompletableFuture deleteEndpoint( @Param("group") String group, @Param("endpoint_id") String endpointId, @Param("summary") @Nullable String summary) { - final String endpointName = "groups/" + group + "/endpoints/" + endpointId; + final String endpointName = endpointName(group, endpointId); if (!ENDPOINT_NAME_PATTERN.matcher(endpointName).matches()) { return CompletableFuture.completedFuture( XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, @@ -173,7 +173,7 @@ public CompletableFuture registerLocalityLbEndpoint( @Param("group") String group, @Param("endpointId") String endpointId, String body) { - final String endpointName = "groups/" + group + "/endpoints/" + endpointId; + final String endpointName = endpointName(group, endpointId); final Matcher matcher = ENDPOINT_NAME_PATTERN.matcher(endpointName); if (!matcher.matches()) { return CompletableFuture.completedFuture( @@ -207,7 +207,7 @@ public CompletableFuture deregisterLocalityLbEndpoint( @Param("group") String group, @Param("endpointId") String endpointId, String body) { - final String endpointName = "groups/" + group + "/endpoints/" + endpointId; + final String endpointName = endpointName(group, endpointId); final Matcher matcher = ENDPOINT_NAME_PATTERN.matcher(endpointName); if (!matcher.matches()) { return CompletableFuture.completedFuture( @@ -232,4 +232,12 @@ public CompletableFuture deregisterLocalityLbEndpoint( private static LocalityLbEndpoint parseLocalityLbEndpoint(String body) throws IOException { return XdsResourceManager.parseYaml(body, LocalityLbEndpoint.newBuilder()); } + + private static String clusterName(String group, String endpointId) { + return "groups/" + group + CLUSTERS_DIRECTORY + endpointId; + } + + private static String endpointName(String group, String endpointId) { + return "groups/" + group + ENDPOINTS_DIRECTORY + endpointId; + } } diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/ControlPlaneService.java b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/ControlPlaneService.java index a999df3f2..263eb8254 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/ControlPlaneService.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/ControlPlaneService.java @@ -113,9 +113,9 @@ public final class ControlPlaneService extends XdsResourceWatchingService { private final ControlPlaneMetrics metrics; // Mutated only from the controlPlaneExecutor. private final CentralDogmaXdsResources centralDogmaXdsResources = new CentralDogmaXdsResources(); - private volatile boolean stop; @Nullable private volatile XdsEndpointService xdsEndpointService; + private volatile boolean stop; ControlPlaneService(Project xdsProject, MeterRegistry meterRegistry) { super(xdsProject, "xds.control.plane.service.", meterRegistry); @@ -479,18 +479,20 @@ void stop() { metrics.onStopped(); final XdsEndpointService xdsEndpointService = this.xdsEndpointService; - if (xdsEndpointService != null && xdsEndpointService.batchUpdateTaskSize() > 0) { - logger.info("Waiting for {} xDS endpoint batch update tasks to finish up to 5 seconds...", - xdsEndpointService.batchUpdateTaskSize()); - for (int i = 0; i < 5; i++) { - try { - if (xdsEndpointService.batchUpdateTaskSize() == 0) { + if (xdsEndpointService != null) { + if (xdsEndpointService.batchUpdateTaskSize() > 0) { + logger.info("Waiting for {} xDS endpoint batch update tasks to finish up to 5 seconds...", + xdsEndpointService.batchUpdateTaskSize()); + for (int i = 0; i < 5; i++) { + try { + if (xdsEndpointService.batchUpdateTaskSize() == 0) { + break; + } + Thread.sleep(1000); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); break; } - Thread.sleep(1000); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - break; } } } diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRole.java b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRole.java index b71cf7ae9..42404d82c 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRole.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRole.java @@ -1,7 +1,7 @@ /* - * Copyright 2024 LINE Corporation + * Copyright 2026 LY Corporation * - * LINE Corporation licenses this file to you under the Apache License, + * 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: * @@ -35,7 +35,7 @@ public @interface RequiresXdsGroupRole { /** - * The required {@link RepositoryRole}. Defaults to {@link RepositoryRole#WRITE}. + * The required {@link RepositoryRole}. */ RepositoryRole value(); diff --git a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRoleDecorator.java b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRoleDecorator.java index 80b40c1c3..e4e1ae712 100644 --- a/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRoleDecorator.java +++ b/xds/src/main/java/com/linecorp/centraldogma/xds/internal/RequiresXdsGroupRoleDecorator.java @@ -1,7 +1,7 @@ /* - * Copyright 2024 LINE Corporation + * Copyright 2026 LY Corporation * - * LINE Corporation licenses this file to you under the Apache License, + * 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: * @@ -41,9 +41,9 @@ * A {@link Decorator} that enforces the specified {@link RepositoryRole} on the xDS group identified * by the {@code {group}} path variable. Performs the following checks in order: *

    + *
  1. Request is authenticated → 401 if not
    2. *
  2. Group name matches the allowed pattern → 400 if not
    3. *
  3. Group repository exists in the xDS project → 404 if not
    4. - *
  4. Request is authenticated → 401 if not
    5. *
  5. User is a system administrator, or has the required role on the group → 403 if not
    6. *
*/ @@ -84,8 +84,7 @@ public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exc final ProjectMetadata metadata = xdsProject.metadata(); // @xds is not the internal dogma project, so metadata is always initialized — never null. assert metadata != null; - final RepositoryRole role = - MetadataService.findRepositoryRole(metadata, group, user); + final RepositoryRole role = MetadataService.findRepositoryRole(metadata, group, user); if (role != null && role.has(requiredRole)) { return unwrap().serve(ctx, req); } 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 251b43780..cd3ef5fb1 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 @@ -165,8 +165,7 @@ public static T parseYaml(String body, Message.Builder build /** * Injects or replaces a top-level YAML field in the given YAML string, preserving all other * content including comments. If {@code fieldName} is not found as a top-level key, it is - * prepended. Uses snakeyaml's AST so that block-scalar values (e.g. {@code name: |}) are - * replaced in their entirety — not just the indicator line. + * prepended. */ public static String injectYamlField(String yaml, String fieldName, String fieldValue) { final String replacement = fieldName + ": " + fieldValue + '\n'; @@ -279,7 +278,7 @@ public CompletableFuture delete( public CompletableFuture updateOrDelete( String group, String resourceName, String fileName, - Function> taskProvider) { + Function> task) { final Repository repository; try { repository = xdsProject.repos().get(group); @@ -296,16 +295,17 @@ public CompletableFuture updateOrDelete( "Resource not found: " + resourceName)); } final String resolvedFileName = entries.keySet().iterator().next(); - return taskProvider.apply(resolvedFileName); + return task.apply(resolvedFileName); }); } public static String alternativeFileName(String fileName) { + final String baseFileName = fileName.substring(0, fileName.length() - 5); if (fileName.endsWith(".json")) { - return fileName.substring(0, fileName.length() - 5) + ".yaml"; + return baseFileName + ".yaml"; } if (fileName.endsWith(".yaml")) { - return fileName.substring(0, fileName.length() - 5) + ".json"; + return baseFileName + ".json"; } return fileName; } 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 ad2eb4b01..db9c7fdfb 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 @@ -190,7 +190,7 @@ public CompletableFuture updateKubernetesEndpointAggregator( "Invalid aggregator name: " + aggregatorName)); } final String clusterName = AGGREGATORS_REPLCACE_PATTERN.matcher(aggregatorName) - .replaceFirst("/clusters/"); + .replaceFirst("/clusters/"); final KubernetesEndpointAggregator aggregator; try { aggregator = XdsResourceManager.parseYaml(body, KubernetesEndpointAggregator.newBuilder()); @@ -379,9 +379,11 @@ private CompletableFuture validateKubernetesEndpointAndPushHttp( private static HttpResponse toErrorHttpResponse(Throwable cause) { final Throwable peeled = Exceptions.peel(cause); + // invalid credential not found if (peeled instanceof IllegalArgumentException || peeled instanceof EntryNotFoundException) { return XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, peeled); } + return XdsResourceManager.errorResponse(HttpStatus.INTERNAL_SERVER_ERROR, peeled); } 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 e259b54d5..0eabcd278 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 @@ -75,7 +75,7 @@ public CompletableFuture createListener( XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, "Invalid listener ID: " + listenerId)); } - final String listenerName = "groups/" + group + LISTENERS_DIRECTORY + listenerId; + final String listenerName = listenerName(group, listenerId); try { XdsResourceManager.parseYaml(body, Listener.newBuilder()); } catch (IOException e) { @@ -103,7 +103,7 @@ public CompletableFuture updateListener( @Param("listener_id") String listenerId, @Param("summary") @Nullable String summary, String body) { - final String listenerName = "groups/" + group + "/listeners/" + listenerId; + final String listenerName = listenerName(group, listenerId); if (!LISTENER_NAME_PATTERN.matcher(listenerName).matches()) { return CompletableFuture.completedFuture( XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, @@ -132,7 +132,7 @@ public CompletableFuture deleteListener( @Param("group") String group, @Param("listener_id") String listenerId, @Param("summary") @Nullable String summary) { - final String listenerName = "groups/" + group + "/listeners/" + listenerId; + final String listenerName = listenerName(group, listenerId); if (!LISTENER_NAME_PATTERN.matcher(listenerName).matches()) { return CompletableFuture.completedFuture( XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, @@ -141,4 +141,8 @@ public CompletableFuture deleteListener( final String deleteSummary = isNullOrEmpty(summary) ? "Delete listener: " + listenerName : summary; return xdsResourceManager.delete(group, listenerName, deleteSummary, currentAuthor()); } + + private static String listenerName(String group, String listenerId) { + return "groups/" + group + LISTENERS_DIRECTORY + listenerId; + } } 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 68d3056ab..9279b58fa 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 @@ -75,7 +75,7 @@ public CompletableFuture createRoute( XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, "Invalid route ID: " + routeId)); } - final String routeName = "groups/" + group + ROUTES_DIRECTORY + routeId; + final String routeName = routeName(group, routeId); try { XdsResourceManager.parseYaml(body, RouteConfiguration.newBuilder()); } catch (IOException e) { @@ -102,7 +102,7 @@ public CompletableFuture updateRoute( @Param("route_id") String routeId, @Param("summary") @Nullable String summary, String body) { - final String routeName = "groups/" + group + "/routes/" + routeId; + final String routeName = routeName(group, routeId); if (!ROUTE_NAME_PATTERN.matcher(routeName).matches()) { return CompletableFuture.completedFuture( XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, @@ -131,7 +131,7 @@ public CompletableFuture deleteRoute( @Param("group") String group, @Param("route_id") String routeId, @Param("summary") @Nullable String summary) { - final String routeName = "groups/" + group + "/routes/" + routeId; + final String routeName = routeName(group, routeId); if (!ROUTE_NAME_PATTERN.matcher(routeName).matches()) { return CompletableFuture.completedFuture( XdsResourceManager.errorResponse(HttpStatus.BAD_REQUEST, @@ -140,4 +140,8 @@ public CompletableFuture deleteRoute( final String deleteSummary = isNullOrEmpty(summary) ? "Delete route: " + routeName : summary; return xdsResourceManager.delete(group, routeName, deleteSummary, currentAuthor()); } + + private static String routeName(String group, String routeId) { + return "groups/" + group + ROUTES_DIRECTORY + routeId; + } } From 5cecafde6acd55375ab720696a30893b332d0b65 Mon Sep 17 00:00:00 2001 From: minwoox Date: Tue, 21 Jul 2026 16:39:43 +0900 Subject: [PATCH 5/6] Address comment from @ikhoon --- .../linecorp/centraldogma/xds/k8s/v1/XdsKubernetesService.java | 2 -- 1 file changed, 2 deletions(-) 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 db9c7fdfb..3b3d9937f 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 @@ -122,7 +122,6 @@ public XdsKubernetesService(XdsResourceManager xdsResourceManager) { * *

Creates a new Kubernetes endpoint aggregator. */ - @Blocking @Post("/xds/groups/{group}/k8s/endpointAggregators") @Consumes("application/yaml") @RequiresXdsGroupRole(RepositoryRole.WRITE) @@ -248,7 +247,6 @@ public CompletableFuture deleteKubernetesEndpointAggregator( *

Previews the endpoints that would be resolved for a Kubernetes endpoint aggregator * without persisting it. */ - @Blocking @Post("/xds/groups/{group}/k8s/endpointAggregators:preview") @Consumes("application/yaml") @RequiresXdsGroupRole(RepositoryRole.WRITE) From ebe1fb0bae9ab5e31d8b05800e1b6919ab84a5dc Mon Sep 17 00:00:00 2001 From: minwoox Date: Tue, 21 Jul 2026 19:05:00 +0900 Subject: [PATCH 6/6] Support more types --- .../xds/internal/XdsResourceManager.java | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) 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 cd3ef5fb1..2927e3f92 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 @@ -31,6 +31,8 @@ import org.jspecify.annotations.Nullable; import org.reflections.Reflections; import org.reflections.scanners.SubTypesScanner; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.fasterxml.jackson.databind.JsonNode; import com.google.common.collect.ImmutableList; @@ -55,14 +57,12 @@ import com.linecorp.centraldogma.xds.endpoint.v1.LocalityLbEndpoint; import com.linecorp.centraldogma.xds.k8s.v1.KubernetesEndpointAggregator; -import io.envoyproxy.envoy.config.cluster.v3.Cluster; -import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment; -import io.envoyproxy.envoy.config.listener.v3.Listener; -import io.envoyproxy.envoy.config.route.v3.RouteConfiguration; import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager; public final class XdsResourceManager { + private static final Logger logger = LoggerFactory.getLogger(XdsResourceManager.class); + 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 @@ -79,11 +79,9 @@ public final class XdsResourceManager { static { final MessageMarshaller.Builder builder = MessageMarshaller.builder().omittingInsignificantWhitespace(true); - builder.register(Listener.getDefaultInstance()) - .register(Cluster.getDefaultInstance()) - .register(ClusterLoadAssignment.getDefaultInstance()) - .register(RouteConfiguration.getDefaultInstance()) - .register(KubernetesEndpointAggregator.getDefaultInstance()) + // These Central Dogma-specific types are not in the io.envoyproxy.envoy package, so they + // must be registered explicitly; envoyExtension() does not pick them up. + builder.register(KubernetesEndpointAggregator.getDefaultInstance()) .register(LocalityLbEndpoint.getDefaultInstance()); envoyExtension(builder); JSON_MESSAGE_MARSHALLER = builder.build(); @@ -91,13 +89,26 @@ public final class XdsResourceManager { private static void envoyExtension(MessageMarshaller.Builder builder) { final Reflections reflections = new Reflections( - "io.envoyproxy.envoy.extensions", HttpConnectionManager.class.getClassLoader(), + "io.envoyproxy.envoy", HttpConnectionManager.class.getClassLoader(), new SubTypesScanner(true)); reflections.getSubTypesOf(GeneratedMessageV3.class) .stream() - .filter(c -> !c.getName().contains("$")) // exclude subclasses + .filter(c -> !c.getName().contains("$")) // exclude inner classes .filter(XdsResourceManager::hasGetDefaultInstanceMethod) - .forEach(builder::register); + .forEach(c -> { + // register() does not throw; build() does. A test build per class is needed + // to detect unsupported types before adding them to the real builder. + try { + MessageMarshaller.builder() + .omittingInsignificantWhitespace(true) + .register(c) + .build(); + builder.register(c); + } catch (Exception e) { + logger.debug("Skipping proto type not supported by MessageMarshaller: {}", + c.getName()); + } + }); } private static boolean hasGetDefaultInstanceMethod(Class clazz) {