Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
com.linecorp.armeria.xds.api.StrictXdsValidatorIndex
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@

package com.linecorp.armeria.xds.it;

import com.google.protobuf.Message;

import com.linecorp.armeria.xds.validator.XdsValidatorIndex;

public class NoopXdsValidatorIndex implements XdsValidatorIndex {

@Override
public void assertValid(Object message) {
public void assertValid(Message message) {
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,40 +16,33 @@

package com.linecorp.armeria.xds.api;

import com.google.protobuf.Message;

import com.linecorp.armeria.common.annotation.UnstableApi;
import com.linecorp.armeria.xds.validator.XdsValidatorIndex;

import io.envoyproxy.pgv.ReflectiveValidatorIndex;
import io.envoyproxy.pgv.ValidationException;
import io.envoyproxy.pgv.Validator;
import io.envoyproxy.pgv.ValidatorIndex;

/**
* The default validator which uses reflection in conjunction with pgv to validate messages.
* The default validator which applies pgv (protoc-gen-validate) structural validation
* and warns usage of unsupported fields.
*/
@UnstableApi
public final class DefaultXdsValidatorIndex implements ValidatorIndex, XdsValidatorIndex {
public final class DefaultXdsValidatorIndex implements XdsValidatorIndex {

private static final DefaultXdsValidatorIndex INSTANCE = new DefaultXdsValidatorIndex();

private final ValidatorIndex delegate;
private final PgvValidator pgvValidator = PgvValidator.of();
private final SupportedFieldValidator supportedFieldValidator = SupportedFieldValidator.of();

/**
* Creates a validator.
* Returns the default singleton instance.
*/
public DefaultXdsValidatorIndex() {
delegate = new ReflectiveValidatorIndex();
}

@Override
public <T> Validator<T> validatorFor(Class clazz) {
return delegate.validatorFor(clazz);
public static DefaultXdsValidatorIndex of() {
return INSTANCE;
}

@Override
public void assertValid(Object message) {
try {
validatorFor(message).assertValid(message);
} catch (ValidationException e) {
throw new IllegalArgumentException(e);
}
public void assertValid(Message message) {
pgvValidator.assertValid(message);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
supportedFieldValidator.validate(message);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright 2025 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.armeria.xds.api;

import com.linecorp.armeria.common.annotation.UnstableApi;

import io.envoyproxy.pgv.ReflectiveValidatorIndex;
import io.envoyproxy.pgv.ValidationException;
import io.envoyproxy.pgv.ValidatorIndex;

/**
* Validates xDS protobuf messages using pgv (protoc-gen-validate) structural validation.
*/
@UnstableApi
public final class PgvValidator {

private static final PgvValidator INSTANCE = new PgvValidator();

private final ValidatorIndex delegate = new ReflectiveValidatorIndex();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static?


private PgvValidator() {}

/**
* Returns the singleton {@link PgvValidator} instance.
*/
public static PgvValidator of() {
return INSTANCE;
}

/**
* Validates the given message using pgv structural validation.
*
* @throws IllegalArgumentException if validation fails
*/
public void assertValid(Object message) {
try {
delegate.validatorFor(message).assertValid(message);
} catch (ValidationException e) {
throw new IllegalArgumentException(e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Copyright 2025 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.armeria.xds.api;

import java.util.ArrayList;
import java.util.List;

import com.google.protobuf.Message;

import com.linecorp.armeria.common.annotation.UnstableApi;
import com.linecorp.armeria.xds.validator.XdsValidatorIndex;

/**
* A strict {@link XdsValidatorIndex} that composes pgv (protoc-gen-validate) structural validation
* with supported-field validation. All unsupported field violations are collected and reported
* together in a single {@link IllegalArgumentException}.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional) What do you think of introducing a dedicated exception type to identify the validation error more easily.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that exceptions don't really impact actually request flow, so I don't really expect users to react based on the exceptions. Updated anyways.

*
* <p>This validator is intended for use in test environments via SPI to catch unsupported
* field usage early. Its {@link #priority()} is {@code 1}, which is higher than
* {@link DefaultXdsValidatorIndex} ({@code 0}), so it wins SPI selection when present
* on the classpath.
*/
@UnstableApi
public final class StrictXdsValidatorIndex implements XdsValidatorIndex {

private final PgvValidator pgvValidator = PgvValidator.of();

@Override
public void assertValid(Message message) {
pgvValidator.assertValid(message);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
final List<String> violations = new ArrayList<>();
final SupportedFieldValidator validator =
SupportedFieldValidator.of((descriptorName, fieldPath, value) ->
violations.add(descriptorName + ": " + fieldPath));
validator.validate(message);
if (!violations.isEmpty()) {
throw new IllegalArgumentException(
"Unsupported xDS fields detected: " + String.join(", ", violations));
}
}

@Override
public int priority() {
return 1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,16 @@
import com.linecorp.armeria.common.annotation.UnstableApi;

/**
* Validates protobuf messages against the {@code (armeria.xds.supported)} field annotation.
* Any set field that lacks the annotation is reported as an unsupported field violation.
* The validator walks recursively into supported message-typed fields.
* Validates protobuf messages against the {@code (armeria.xds.supported_fields)} message-level annotation.
* Any set field whose number is not listed in the message's {@code supported_fields} option is reported
* as an unsupported field violation. The validator walks recursively into supported message-typed fields.
*
* <p>Currently, supported fields are annotated inline on each field declaration in the proto files, e.g.:
* <p>Supported fields are annotated at the message level in the proto files, e.g.:
* <pre>{@code
* string exact = 1 [(armeria.xds.supported) = true];
* message StringMatcher {
* option (armeria.xds.supported_fields) = 1;
* string exact = 1;
* }
* }</pre>
*/
Comment thread
coderabbitai[bot] marked this conversation as resolved.
@UnstableApi
Expand Down Expand Up @@ -162,7 +165,9 @@ private void validateFieldValue(FieldDescriptor fd, Object value,
}

private static boolean unsupportedEnumValue(EnumValueDescriptor ev) {
return !ev.getOptions().getExtension(SupportedFieldProto.supportedValue);
final List<Integer> supportedValues =
ev.getType().getOptions().getExtension(SupportedFieldProto.supported.enumValue);
return !supportedValues.contains(ev.getNumber());
}

private static boolean unsupportedPackage(String pkg) {
Expand All @@ -171,9 +176,11 @@ private static boolean unsupportedPackage(String pkg) {

private Set<FieldDescriptor> supportedFields(Descriptors.Descriptor descriptor) {
return supportedFieldsCache.computeIfAbsent(descriptor, d -> {
final List<Integer> supportedNumbers =
d.getOptions().getExtension(SupportedFieldProto.supported.field);
final Set<FieldDescriptor> result = new HashSet<>();
for (FieldDescriptor fd : d.getFields()) {
if (fd.getOptions().getExtension(SupportedFieldProto.supported)) {
if (supportedNumbers.contains(fd.getNumber())) {
result.add(fd);
}
}
Expand Down
19 changes: 13 additions & 6 deletions xds-api/src/main/proto/armeria/xds/supported.proto
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,18 @@ option java_outer_classname = "SupportedFieldProto";

import "google/protobuf/descriptor.proto";

extend google.protobuf.FieldOptions {
optional bool supported = 50000;
}
// Enclosing message so that option usage reads as
// `(armeria.xds.supported.field)` and `(armeria.xds.supported.enum_value)`.
message supported {
extend google.protobuf.MessageOptions {
// Field numbers of supported fields in this message.
// Place each `option (armeria.xds.supported.field) = <field_number>;`
// directly above the corresponding field declaration.
repeated int32 field = 50000;
}

extend google.protobuf.EnumValueOptions {
// Distinct name from the FieldOptions extension to avoid Java codegen name collision.
optional bool supported_value = 50000;
extend google.protobuf.EnumOptions {
// Enum value numbers of supported values in this enum.
repeated int32 enum_value = 50000;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import "udpa/annotations/status.proto";
import "udpa/annotations/versioning.proto";
import "validate/validate.proto";

import "armeria/xds/supported.proto";

option java_package = "io.envoyproxy.envoy.config.bootstrap.v3";
option java_outer_classname = "BootstrapProto";
option java_multiple_files = true;
Expand All @@ -52,16 +54,19 @@ message Bootstrap {

// Static :ref:`Listeners <envoy_v3_api_msg_config.listener.v3.Listener>`. These listeners are
// available regardless of LDS configuration.
option (armeria.xds.supported.field) = 1;
repeated listener.v3.Listener listeners = 1;

// If a network based configuration source is specified for :ref:`cds_config
// <envoy_v3_api_field_config.bootstrap.v3.Bootstrap.DynamicResources.cds_config>`, it's necessary
// to have some initial cluster definitions available to allow Envoy to know
// how to speak to the management server.
option (armeria.xds.supported.field) = 2;
repeated cluster.v3.Cluster clusters = 2;

// These static secrets can be used by :ref:`SdsSecretConfig
// <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.SdsSecretConfig>`
option (armeria.xds.supported.field) = 3;
repeated envoy.extensions.transport_sockets.tls.v3.Secret secrets = 3;
}

Expand All @@ -74,6 +79,7 @@ message Bootstrap {

// All :ref:`Listeners <envoy_v3_api_msg_config.listener.v3.Listener>` are provided by a single
// :ref:`LDS <arch_overview_dynamic_config_lds>` configuration source.
option (armeria.xds.supported.field) = 1;
core.v3.ConfigSource lds_config = 1;

// xdstp:// resource locator for listener collection.
Expand All @@ -83,6 +89,7 @@ message Bootstrap {
// All post-bootstrap :ref:`Cluster <envoy_v3_api_msg_config.cluster.v3.Cluster>` definitions are
// provided by a single :ref:`CDS <arch_overview_dynamic_config_cds>`
// configuration source.
option (armeria.xds.supported.field) = 2;
core.v3.ConfigSource cds_config = 2;

// xdstp:// resource locator for cluster collection.
Expand All @@ -96,6 +103,7 @@ message Bootstrap {
// :ref:`ConfigSources <envoy_v3_api_msg_config.core.v3.ConfigSource>` that have
// the :ref:`ads <envoy_v3_api_field_config.core.v3.ConfigSource.ads>` field set will be
// streamed on the ADS channel.
option (armeria.xds.supported.field) = 3;
core.v3.ApiConfigSource ads_config = 3;
}

Expand Down Expand Up @@ -147,6 +155,7 @@ message Bootstrap {

// Node identity to present to the management server and for instance
// identification purposes (e.g. in generated headers).
option (armeria.xds.supported.field) = 1;
core.v3.Node node = 1;

// A list of :ref:`Node <envoy_v3_api_msg_config.core.v3.Node>` field names
Expand Down Expand Up @@ -184,13 +193,16 @@ message Bootstrap {
repeated string node_context_params = 26;

// Statically specified resources.
option (armeria.xds.supported.field) = 2;
StaticResources static_resources = 2;

// xDS configuration sources.
option (armeria.xds.supported.field) = 3;
DynamicResources dynamic_resources = 3;

// Configuration for the cluster manager which owns all upstream clusters
// within the server.
option (armeria.xds.supported.field) = 4;
ClusterManager cluster_manager = 4;

// Health discovery service config option.
Expand Down Expand Up @@ -485,6 +497,7 @@ message ClusterManager {
// <envoy_v3_api_field_config.bootstrap.v3.Bootstrap.StaticResources.clusters>`. This is unrelated to
// the :option:`--service-cluster` option which does not `affect zone aware
// routing <https://github.com/envoyproxy/envoy/issues/774>`_.
option (armeria.xds.supported.field) = 1;
string local_cluster_name = 1;

// Optional global configuration for outlier detection.
Expand Down
Loading
Loading