From 5cdd72f06c625617bc84d4b9e50f02029cacb45c Mon Sep 17 00:00:00 2001 From: htuch Date: Thu, 30 Nov 2017 14:12:15 -0500 Subject: [PATCH] pgv: bazel plumbing for PGV support, some PGV related fixups. (#299) * Added PGV C++ generation support. This (hopefully temporarily) abandons using native proto_library in favor of pgv_cc_proto_library. We maintain build support for proto_library for the glorious future in which we write a Bazel aspect to run PGV against the native proto_library shadow graph. * Replace min_len with min_bytes on strings, until PGV gets not-empty or min_len support for C++. * Various fixups for places where the PGV plugin objected to annotations. Signed-off-by: Harvey Tuch --- api/BUILD | 2 +- api/address.proto | 4 +- api/base.proto | 8 ++-- api/cds.proto | 2 +- api/eds.proto | 6 +-- api/filter/http/fault.proto | 30 +++++++------ api/filter/http/health_check.proto | 2 +- api/filter/http/lua.proto | 2 +- api/filter/http/transcoder.proto | 2 +- .../network/http_connection_manager.proto | 6 +-- api/filter/network/mongo_proxy.proto | 2 +- api/filter/network/redis_proxy.proto | 4 +- api/filter/network/tcp_proxy.proto | 6 +-- api/health_check.proto | 4 +- api/lds.proto | 2 +- api/protocol.proto | 11 +++-- api/rds.proto | 28 ++++++------- api/sds.proto | 2 +- bazel/api_build_system.bzl | 22 ++++++++-- bazel/repositories.bzl | 42 ++++++++----------- test/validate/BUILD | 14 +++++++ test/validate/pgv_test.cc | 35 ++++++++++++++++ test/validate/test.proto | 13 ++++++ tools/protodoc/protodoc.py | 2 +- 24 files changed, 165 insertions(+), 86 deletions(-) create mode 100644 test/validate/BUILD create mode 100644 test/validate/pgv_test.cc create mode 100644 test/validate/test.proto diff --git a/api/BUILD b/api/BUILD index 36c757b9..4edf3f96 100644 --- a/api/BUILD +++ b/api/BUILD @@ -98,7 +98,7 @@ api_proto_library( has_services = 1, deps = [ ":base", - "@promotheus_metrics_model//:client_model_protos_lib", + "@promotheus_metrics_model//:client_model", ], require_py = 0, ) diff --git a/api/address.proto b/api/address.proto index 4f377e71..234acfa8 100644 --- a/api/address.proto +++ b/api/address.proto @@ -11,7 +11,7 @@ import "validate/validate.proto"; message Pipe { // Unix Domain Socket path. - string path = 1 [(validate.rules).string.min_len = 1]; + string path = 1 [(validate.rules).string.min_bytes = 1]; } message SocketAddress { @@ -65,7 +65,7 @@ message Address { // the subnet mask for a `CIDR `_ range. message CidrRange { // IPv4 or IPv6 address, e.g. 192.0.0.0 or 2001:db8::. - string address_prefix = 1 [(validate.rules).string.min_len = 1]; + string address_prefix = 1 [(validate.rules).string.min_bytes = 1]; // Length of prefix, e.g. 0, 32. google.protobuf.UInt32Value prefix_len = 2 [(validate.rules).uint32.lte = 128]; } diff --git a/api/base.proto b/api/base.proto index 464ecf26..e9132926 100644 --- a/api/base.proto +++ b/api/base.proto @@ -35,9 +35,9 @@ message Locality { // configuration for serving. message Node { // An opaque node identifier for the Envoy node. - string id = 1 [(validate.rules).string.min_len = 1]; + string id = 1 [(validate.rules).string.min_bytes = 1]; // The cluster that the Envoy node belongs to. - string cluster = 2 [(validate.rules).string.min_len = 1]; + string cluster = 2 [(validate.rules).string.min_bytes = 1]; // Opaque metadata extending the node identifier. Envoy will pass this // directly to the management server. google.protobuf.Struct metadata = 3; @@ -84,7 +84,7 @@ message RuntimeUInt32 { uint32 default_value = 2; // Runtime key to get value for comparison. This value is used if defined. - string runtime_key = 3 [(validate.rules).string.min_len = 1]; + string runtime_key = 3 [(validate.rules).string.min_bytes = 1]; } // Envoy supports :ref:`upstream priority routing @@ -189,7 +189,7 @@ message ConfigSource { message TransportSocket { // The name of the transport socket to instantiate. The name must match a supported transport // socket implementation. - string name = 1 [(validate.rules).string.min_len = 1]; + string name = 1 [(validate.rules).string.min_bytes = 1]; // Implementation specific configuration which depends on the implementation being instantiated. // See the supported transport socket implementations for further documentation. diff --git a/api/cds.proto b/api/cds.proto index 211a47f0..d341e561 100644 --- a/api/cds.proto +++ b/api/cds.proto @@ -84,7 +84,7 @@ message Cluster { // By default, the maximum length of a cluster name is limited to 60 // characters. This limit can be increased by setting the // :option:`--max-obj-name-len` command line argument to the desired value. - string name = 1 [(validate.rules).string.min_len = 1]; + string name = 1 [(validate.rules).string.min_bytes = 1]; // Refer to :ref:`service discovery type ` // for an explanation on each type. diff --git a/api/eds.proto b/api/eds.proto index 03e88376..e5cf80f7 100644 --- a/api/eds.proto +++ b/api/eds.proto @@ -201,7 +201,7 @@ message UpstreamLocalityStats { // :ref:`LoadStatsRequest` message ClusterStats { // The name of the cluster. - string cluster_name = 1 [(validate.rules).string.min_len = 1]; + string cluster_name = 1 [(validate.rules).string.min_bytes = 1]; // Need at least one. repeated UpstreamLocalityStats upstream_locality_stats = 2 [(validate.rules).repeated.min_items = 1]; @@ -243,7 +243,7 @@ message ClusterLoadAssignment { // ` value if specified // in the cluster :ref:`EdsClusterConfig // `. - string cluster_name = 1 [(validate.rules).string.min_len = 1]; + string cluster_name = 1 [(validate.rules).string.min_bytes = 1]; // List of endpoints to load balance to. repeated LocalityLbEndpoints endpoints = 2; @@ -255,7 +255,7 @@ message ClusterLoadAssignment { // recover from an outage or should they be unable to autoscale and hence // overall incoming traffic volume need to be trimmed to protect them. // [#v2-api-diff: This is known as maintenance mode in v1.] - double drop_overload = 1 [(validate.rules).fixed32 = {gte:0, lte: 100}]; + double drop_overload = 1 [(validate.rules).double = {gte:0, lte: 100}]; } // Load balancing policy settings. diff --git a/api/filter/http/fault.proto b/api/filter/http/fault.proto index 767a9501..3e062fa0 100644 --- a/api/filter/http/fault.proto +++ b/api/filter/http/fault.proto @@ -41,17 +41,21 @@ message HTTPFault { string upstream_cluster = 3; // Specifies a set of headers that the filter should match on. The fault - // injection filter can be applied selectively to requests that match a set of headers specified in - // the fault filter config. The chances of actual fault injection further depend on the value of - // the :ref:`percent ` field. The filter will check the request's headers - // against all the specified headers in the filter config. A match will happen if all the headers in - // the config are present in the request with the same values (or based on presence if the *value* - // field is not in the config). - repeated HeaderMatcher headers = 4 [(validate.rules).repeated.unique = true]; - - // Faults are injected for the specified list of downstream hosts. If this setting is - // not set, faults are injected for all downstream nodes. Downstream node name is taken from - // :ref:`the HTTP x-envoy-downstream-service-node ` - // header and compared against downstream_nodes list. - repeated string downstream_nodes = 5 [(validate.rules).repeated.unique = true]; + // injection filter can be applied selectively to requests that match a set of + // headers specified in the fault filter config. The chances of actual fault + // injection further depend on the value of the :ref:`percent + // ` field. The filter will + // check the request's headers against all the specified headers in the filter + // config. A match will happen if all the headers in the config are present in + // the request with the same values (or based on presence if the *value* field + // is not in the config). + repeated HeaderMatcher headers = 4; + + // Faults are injected for the specified list of downstream hosts. If this + // setting is not set, faults are injected for all downstream nodes. + // Downstream node name is taken from :ref:`the HTTP + // x-envoy-downstream-service-node + // ` header and compared + // against downstream_nodes list. + repeated string downstream_nodes = 5; } diff --git a/api/filter/http/health_check.proto b/api/filter/http/health_check.proto index 2dda450f..b20ba460 100644 --- a/api/filter/http/health_check.proto +++ b/api/filter/http/health_check.proto @@ -16,7 +16,7 @@ message HealthCheck { // Specifies the incoming HTTP endpoint that should be considered the // health check endpoint. For example */healthcheck*. - string endpoint = 2 [(validate.rules).string.min_len = 1]; + string endpoint = 2 [(validate.rules).string.min_bytes = 1]; // If operating in pass through mode, the amount of time in milliseconds // that the filter should cache the upstream response. diff --git a/api/filter/http/lua.proto b/api/filter/http/lua.proto index 6bfe7937..ccfb3b71 100644 --- a/api/filter/http/lua.proto +++ b/api/filter/http/lua.proto @@ -12,5 +12,5 @@ message Lua { // further loads code from disk if desired. Note that if JSON configuration is used, the code must // be properly escaped. YAML configuration may be easier to read since YAML supports multi-line // strings so complex scripts can be easily expressed inline in the configuration. - string inline_code = 1 [(validate.rules).string.min_len = 1]; + string inline_code = 1 [(validate.rules).string.min_bytes = 1]; } diff --git a/api/filter/http/transcoder.proto b/api/filter/http/transcoder.proto index 89e984cf..e77aa4f8 100644 --- a/api/filter/http/transcoder.proto +++ b/api/filter/http/transcoder.proto @@ -29,7 +29,7 @@ message GrpcJsonTranscoder { // --descriptor_set_out=proto.pb test/proto/bookstore.proto // // If you have more than one proto source files, you can pass all of them in one command. - string proto_descriptor = 1 [(validate.rules).string.min_len = 1]; + string proto_descriptor = 1 [(validate.rules).string.min_bytes = 1]; // A list of strings that supplies the service names that the // transcoder will translate. If the service name doesn't exist in ``proto_descriptor``, Envoy diff --git a/api/filter/network/http_connection_manager.proto b/api/filter/network/http_connection_manager.proto index 4d5006ee..05d4af96 100644 --- a/api/filter/network/http_connection_manager.proto +++ b/api/filter/network/http_connection_manager.proto @@ -25,7 +25,7 @@ message Rds { // API. This allows an Envoy configuration with multiple HTTP listeners (and // associated HTTP connection manager filters) to use different route // configurations. - string route_config_name = 2 [(validate.rules).string.min_len = 1]; + string route_config_name = 2 [(validate.rules).string.min_bytes = 1]; } message HttpFilter { @@ -44,7 +44,7 @@ message HttpFilter { // * :ref:`envoy.lua ` // * :ref:`envoy.rate_limit ` // * :ref:`envoy.router ` - string name = 1 [(validate.rules).string.min_len = 1]; + string name = 1 [(validate.rules).string.min_bytes = 1]; // Filter specific configuration which depends on the filter being // instantiated. See the supported filters for further documentation. @@ -85,7 +85,7 @@ message HttpConnectionManager { // The human readable prefix to use when emitting statistics for the // connection manager. See the :ref:`statistics documentation ` for // more information. - string stat_prefix = 2 [(validate.rules).string.min_len = 1]; + string stat_prefix = 2 [(validate.rules).string.min_bytes = 1]; oneof route_specifier { option (validate.required) = true; diff --git a/api/filter/network/mongo_proxy.proto b/api/filter/network/mongo_proxy.proto index 45771ae2..cd16906d 100644 --- a/api/filter/network/mongo_proxy.proto +++ b/api/filter/network/mongo_proxy.proto @@ -12,7 +12,7 @@ import "validate/validate.proto"; message MongoProxy { // The human readable prefix to use when emitting :ref:`statistics // `. - string stat_prefix = 1 [(validate.rules).string.min_len = 1]; + string stat_prefix = 1 [(validate.rules).string.min_bytes = 1]; // The optional path to use for writing Mongo access logs. If not access log // path is specified no access logs will be written. Note that access log is diff --git a/api/filter/network/redis_proxy.proto b/api/filter/network/redis_proxy.proto index ba17f443..b9a14ade 100644 --- a/api/filter/network/redis_proxy.proto +++ b/api/filter/network/redis_proxy.proto @@ -11,12 +11,12 @@ import "validate/validate.proto"; message RedisProxy { // The prefix to use when emitting :ref:`statistics `. - string stat_prefix = 1 [(validate.rules).string.min_len = 1]; + string stat_prefix = 1 [(validate.rules).string.min_bytes = 1]; // Name of cluster from cluster manager. See the :ref:`configuration section // ` of the architecture overview for recommendations on // configuring the backing cluster. - string cluster = 2 [(validate.rules).string.min_len = 1]; + string cluster = 2 [(validate.rules).string.min_bytes = 1]; // Redis connection pool settings. message ConnPoolSettings { diff --git a/api/filter/network/tcp_proxy.proto b/api/filter/network/tcp_proxy.proto index c6a97424..04c5eba8 100644 --- a/api/filter/network/tcp_proxy.proto +++ b/api/filter/network/tcp_proxy.proto @@ -18,10 +18,10 @@ import "validate/validate.proto"; message TcpProxy { // The prefix to use when emitting :ref:`statistics // `. - string stat_prefix = 1 [(validate.rules).string.min_len = 1]; + string stat_prefix = 1 [(validate.rules).string.min_bytes = 1]; // The upstream cluster to connect to. - string cluster = 2 [(validate.rules).string.min_len = 1]; + string cluster = 2 [(validate.rules).string.min_bytes = 1]; // [#not-implemented-hide:] The idle timeout for connections managed by the TCP proxy // filter. The idle timeout is defined as the period in which there is no @@ -53,7 +53,7 @@ message TcpProxy { message TCPRoute { // The cluster to connect to when a the downstream network connection // matches the specified criteria. - string cluster = 1 [(validate.rules).string.min_len = 1]; + string cluster = 1 [(validate.rules).string.min_bytes = 1]; // An optional list of IP address subnets in the form // “ip_address/xx”. The criteria is satisfied if the destination IP diff --git a/api/health_check.proto b/api/health_check.proto index 73137640..b4e280d5 100644 --- a/api/health_check.proto +++ b/api/health_check.proto @@ -46,7 +46,7 @@ message HealthCheck { option (validate.required) = true; // Hex encoded payload. E.g., "000000FF". - string text = 1 [(validate.rules).string.min_len = 1]; + string text = 1 [(validate.rules).string.min_bytes = 1]; // [#not-implemented-hide:] Binary payload. bytes binary = 2; @@ -61,7 +61,7 @@ message HealthCheck { // Specifies the HTTP path that will be requested during health checking. For example // */healthcheck*. - string path = 2 [(validate.rules).string.min_len = 1]; + string path = 2 [(validate.rules).string.min_bytes = 1]; // [#not-implemented-hide:] HTTP specific payload. Payload send = 3; diff --git a/api/lds.proto b/api/lds.proto index 5ffce685..9cc10063 100644 --- a/api/lds.proto +++ b/api/lds.proto @@ -44,7 +44,7 @@ message Filter { // * :ref:`envoy.mongo_proxy ` // * :ref:`envoy.redis_proxy ` // * :ref:`envoy.tcp_proxy ` - string name = 1 [(validate.rules).string.min_len = 1]; + string name = 1 [(validate.rules).string.min_bytes = 1]; // Filter specific configuration which depends on the filter being // instantiated. See the supported filters for further documentation. diff --git a/api/protocol.proto b/api/protocol.proto index 7d7c293d..aa60e34c 100644 --- a/api/protocol.proto +++ b/api/protocol.proto @@ -6,6 +6,8 @@ package envoy.api.v2; import "google/protobuf/wrappers.proto"; +import "validate/validate.proto"; + // [#protodoc-title: Protocol options] // [#not-implemented-hide:] @@ -30,7 +32,8 @@ message Http2ProtocolOptions { // `Maximum concurrent streams `_ // allowed for peer on one HTTP/2 connection. Valid values range from 1 to 2147483647 (2^31 - 1) // and defaults to 2147483647. - google.protobuf.UInt32Value max_concurrent_streams = 2; + google.protobuf.UInt32Value max_concurrent_streams = 2 + [(validate.rules).uint32 = {gte: 1, lte: 2147483647}]; // `Initial stream-level flow-control window // `_ size. Valid values range from 65535 @@ -43,11 +46,13 @@ message Http2ProtocolOptions { // This field also acts as a soft limit on the number of bytes Envoy will buffer per-stream in the // HTTP/2 codec buffers. Once the buffer reaches this pointer, watermark callbacks will fire to // stop the flow of data to the codec buffers. - google.protobuf.UInt32Value initial_stream_window_size = 3; + google.protobuf.UInt32Value initial_stream_window_size = 3 + [(validate.rules).uint32 = {gte: 65535, lte: 2147483647}]; // Similar to *initial_stream_window_size*, but for connection-level flow-control // window. Currently, this has the same minimum/maximum/default as *initial_stream_window_size*. - google.protobuf.UInt32Value initial_connection_window_size = 4; + google.protobuf.UInt32Value initial_connection_window_size = 4 + [(validate.rules).uint32 = {gte: 65535, lte: 2147483647}]; } // [#not-implemented-hide:] diff --git a/api/rds.proto b/api/rds.proto index ce509c25..b650927f 100644 --- a/api/rds.proto +++ b/api/rds.proto @@ -43,7 +43,7 @@ message WeightedCluster { message ClusterWeight { // Name of the upstream cluster. The cluster must exist in the // :ref:`cluster manager configuration `. - string name = 1 [(validate.rules).string.min_len = 1]; + string name = 1 [(validate.rules).string.min_bytes = 1]; // An integer between 0-100. When a request matches the route, the choice of // an upstream cluster is determined by its weight. The sum of weights @@ -242,7 +242,7 @@ message RouteAction { message RequestMirrorPolicy { // Specifies the cluster that requests will be mirrored to. The cluster must // exist in the cluster manager configuration. - string cluster = 1 [(validate.rules).string.min_len = 1]; + string cluster = 1 [(validate.rules).string.min_bytes = 1]; // If not specified, all requests to the target cluster will be mirrored. If // specified, Envoy will lookup the runtime key to get the % of requests to @@ -291,7 +291,7 @@ message RouteAction { message Header { // The name of the request header that will be used to obtain the hash // key. If the request header is not present, no hash will be produced. - string header_name = 1 [(validate.rules).string.min_len = 1]; + string header_name = 1 [(validate.rules).string.min_bytes = 1]; } // Envoy supports two types of cookie affinity: @@ -312,7 +312,7 @@ message RouteAction { // The name of the cookie that will be used to obtain the hash key. If the // cookie is not present and ttl below is not set, no hash will be // produced. - string name = 1 [(validate.rules).string.min_len = 1]; + string name = 1 [(validate.rules).string.min_bytes = 1]; // If specified, a cookie with the TTL will be generated if the cookie is // not present. @@ -410,7 +410,7 @@ message Decorator { // For ingress (inbound) requests, or egress (outbound) responses, this value may be overridden // by the :ref:`x-envoy-decorator-operation // ` header. - string operation = 1 [(validate.rules).string.min_len = 1]; + string operation = 1 [(validate.rules).string.min_bytes = 1]; } // A route is both a specification of how to match a request as well as an indication of what to do @@ -476,12 +476,12 @@ message VirtualCluster { // * The regex */rides/\d+* matches the path */rides/0* // * The regex */rides/\d+* matches the path */rides/123* // * The regex */rides/\d+* does not match the path */rides/123/456* - string pattern = 1 [(validate.rules).string.min_len = 1]; + string pattern = 1 [(validate.rules).string.min_bytes = 1]; // Specifies the name of the virtual cluster. The virtual cluster name as well // as the virtual host name are used when emitting statistics. The statistics are emitted by the // router filter and are documented :ref:`here `. - string name = 2 [(validate.rules).string.min_len = 1]; + string name = 2 [(validate.rules).string.min_bytes = 1]; // Optionally specifies the HTTP method to match on. For example GET, PUT, // etc. @@ -540,10 +540,10 @@ message RateLimit { // The header name to be queried from the request headers. The header’s // value is used to populate the value of the descriptor entry for the // descriptor_key. - string header_name = 1 [(validate.rules).string.min_len = 1]; + string header_name = 1 [(validate.rules).string.min_bytes = 1]; // The key to use in the descriptor entry. - string descriptor_key = 2 [(validate.rules).string.min_len = 1]; + string descriptor_key = 2 [(validate.rules).string.min_bytes = 1]; } // The following descriptor entry is appended to the descriptor and is populated using the @@ -561,7 +561,7 @@ message RateLimit { // ("generic_key", "") message GenericKey { // The value to use in the descriptor entry. - string descriptor_value = 1 [(validate.rules).string.min_len = 1]; + string descriptor_value = 1 [(validate.rules).string.min_bytes = 1]; } // The following descriptor entry is appended to the descriptor: @@ -571,7 +571,7 @@ message RateLimit { // ("header_match", "") message HeaderValueMatch { // The value to use in the descriptor entry. - string descriptor_value = 1 [(validate.rules).string.min_len = 1]; + string descriptor_value = 1 [(validate.rules).string.min_bytes = 1]; // If set to true, the action will append a descriptor entry when the // request matches the headers. If set to false, the action will append a @@ -616,7 +616,7 @@ message RateLimit { // cannot append a descriptor entry, no descriptor is generated for the // configuration. See :ref:`composing actions // ` for additional documentation. - repeated Action actions = 3 [(validate.rules).string.min_len = 1]; + repeated Action actions = 3 [(validate.rules).repeated.min_items = 1]; } // .. attention:: @@ -637,7 +637,7 @@ message RateLimit { // } message HeaderMatcher { // Specifies the name of the header in the request. - string name = 1 [(validate.rules).string.min_len = 1]; + string name = 1 [(validate.rules).string.min_bytes = 1]; // Specifies the value of the header. If the value is absent a request that // has the name header will match, regardless of the header’s value. @@ -665,7 +665,7 @@ message HeaderMatcher { message VirtualHost { // The logical name of the virtual host. This is used when emitting certain // statistics but is not relevant for routing. - string name = 1 [(validate.rules).string.min_len = 1]; + string name = 1 [(validate.rules).string.min_bytes = 1]; // A list of domains (host/authority header) that will be matched to this // virtual host. Wildcard hosts are supported in the form of “*.foo.com” or diff --git a/api/sds.proto b/api/sds.proto index ec0843e4..4511834d 100644 --- a/api/sds.proto +++ b/api/sds.proto @@ -31,7 +31,7 @@ message DataSource { option (validate.required) = true; // Local filesystem data source. - string filename = 1 [(validate.rules).string.min_len = 1]; + string filename = 1 [(validate.rules).string.min_bytes = 1]; // [#not-implemented-hide:] bytes inline = 2; diff --git a/bazel/api_build_system.bzl b/bazel/api_build_system.bzl index f363486e..606e91f0 100644 --- a/bazel/api_build_system.bzl +++ b/bazel/api_build_system.bzl @@ -1,4 +1,5 @@ load("@com_google_protobuf//:protobuf.bzl", "py_proto_library") +load("@com_lyft_protoc_gen_validate//bazel:pgv_proto_library.bzl", "pgv_cc_proto_library") def _CcSuffix(d): return d + "_cc" @@ -27,6 +28,12 @@ def api_py_proto_library(name, srcs = [], deps = [], has_services = 0): # TODO(htuch): has_services is currently ignored but will in future support # gRPC stub generation. def api_proto_library(name, srcs = [], deps = [], has_services = 0, require_py = 1): + # This is now vestigial, since there are no direct consumers in + # data-plane-api. However, we want to maintain native proto_library support + # in the proto graph to (1) support future C++ use of native rules with + # cc_proto_library (or some Bazel aspect that works on proto_library) when + # it can play well with the PGV plugin and (2) other language support that + # can make use of native proto_library. native.proto_library( name = name, srcs = srcs, @@ -37,14 +44,23 @@ def api_proto_library(name, srcs = [], deps = [], has_services = 0, require_py = "@com_google_protobuf//:struct_proto", "@com_google_protobuf//:timestamp_proto", "@com_google_protobuf//:wrappers_proto", - "@googleapis//:http_api_protos_lib", + "@googleapis//:http_api_protos_proto", "@com_lyft_protoc_gen_validate//validate:validate_proto", ], visibility = ["//visibility:public"], ) - native.cc_proto_library( + # Under the hood, this is just an extension of the Protobuf library's + # bespoke cc_proto_library. It doesn't consume proto_library as a proto + # provider. Hopefully one day we can move to a model where this target and + # the proto_library above are aligned. + pgv_cc_proto_library( name = _CcSuffix(name), - deps = [name], + srcs = srcs, + deps = [_CcSuffix(d) for d in deps], + external_deps = [ + "@com_google_protobuf//:cc_wkt_protos", + "@googleapis//:http_api_protos", + ], visibility = ["//visibility:public"], ) if (require_py == 1): diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index c921dcc1..c1984620 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -1,22 +1,20 @@ GOOGLEAPIS_SHA = "5c6df0cd18c6a429eab739fb711c27f6e1393366" # May 14, 2017 PROMETHEUS_SHA = "6f3806018612930941127f2a7c6c453ba2c527d2" # Nov 02, 2017 -PGV_GIT_SHA = "f3332cbd75bb28f711377dfb84761ef0d52eca0f" -PGV_TAR_SHA = "039ffa842eb62495b6aca305a4eb3d6dc3ac1dd056e228fba5e720161ddfb9c1" +PGV_GIT_SHA = "af35f0b0d2cee1178f77329d30252e07047918c3" def api_dependencies(): - native.http_archive( + native.git_repository( name = "com_lyft_protoc_gen_validate", - strip_prefix = "protoc-gen-validate-" + PGV_GIT_SHA, - sha256 = PGV_TAR_SHA, - url = "https://github.com/lyft/protoc-gen-validate/archive/" + PGV_GIT_SHA + ".tar.gz", + remote = "https://github.com/lyft/protoc-gen-validate.git", + commit = PGV_GIT_SHA, ) native.new_http_archive( name = "googleapis", strip_prefix = "googleapis-" + GOOGLEAPIS_SHA, url = "https://github.com/googleapis/googleapis/archive/" + GOOGLEAPIS_SHA + ".tar.gz", build_file_content = """ -load("@com_google_protobuf//:protobuf.bzl", "py_proto_library") +load("@com_google_protobuf//:protobuf.bzl", "cc_proto_library", "py_proto_library") filegroup( name = "http_api_protos_src", @@ -28,7 +26,7 @@ filegroup( ) proto_library( - name = "http_api_protos_lib", + name = "http_api_protos_proto", srcs = [":http_api_protos_src"], deps = ["@com_google_protobuf//:descriptor_proto"], visibility = ["//visibility:public"], @@ -36,7 +34,13 @@ proto_library( cc_proto_library( name = "http_api_protos", - deps = [":http_api_protos_lib"], + srcs = [ + "google/api/annotations.proto", + "google/api/http.proto", + ], + default_runtime = "@com_google_protobuf//:protobuf", + protoc = "@com_google_protobuf//:protoc", + deps = ["@com_google_protobuf//:cc_wkt_protos"], visibility = ["//visibility:public"], ) @@ -60,25 +64,13 @@ py_proto_library( strip_prefix = "client_model-" + PROMETHEUS_SHA, url = "https://github.com/prometheus/client_model/archive/" + PROMETHEUS_SHA + ".tar.gz", build_file_content = """ +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_library") -filegroup( - name = "client_model_protos_src", +api_proto_library( + name = "client_model", srcs = [ "metrics.proto", ], - visibility = ["//visibility:public"], - ) - -proto_library( - name = "client_model_protos_lib", - srcs = [":client_model_protos_src"], - visibility = ["//visibility:public"], -) - -cc_proto_library( - name = "client_model_protos", - deps = [":client_model_protos_lib"], - visibility = ["//visibility:public"], ) """, - ) \ No newline at end of file + ) diff --git a/test/validate/BUILD b/test/validate/BUILD new file mode 100644 index 00000000..ca1e70df --- /dev/null +++ b/test/validate/BUILD @@ -0,0 +1,14 @@ +load("//bazel:api_build_system.bzl", "api_cc_test", "api_proto_library") + +licenses(["notice"]) # Apache 2 + +api_proto_library( + name = "test", + srcs = ["test.proto"], +) + +api_cc_test( + name = "pgv_test", + srcs = ["pgv_test.cc"], + proto_deps = [":test"], +) diff --git a/test/validate/pgv_test.cc b/test/validate/pgv_test.cc new file mode 100644 index 00000000..17301750 --- /dev/null +++ b/test/validate/pgv_test.cc @@ -0,0 +1,35 @@ +#include +#include + +#include "test/validate/test.pb.validate.h" + +// Basic protoc-gen-validate C++ validation header inclusion and Validate calls +// from data-plane-api. +// TODO(htuch): Switch to using real data-plane-api protos once we can support +// the required field types. +int main(int argc, char *argv[]) { + { + test::validate::Foo empty; + + std::string err; + if (Validate(empty, &err)) { + std::cout << "Unexpected successful validation of empty proto." + << std::endl; + exit(EXIT_FAILURE); + } + } + + { + test::validate::Foo non_empty; + non_empty.mutable_baz(); + + std::string err; + if (!Validate(non_empty, &err)) { + std::cout << "Unexpected failed validation of empty proto: " << err + << std::endl; + exit(EXIT_FAILURE); + } + } + + exit(EXIT_SUCCESS); +} diff --git a/test/validate/test.proto b/test/validate/test.proto new file mode 100644 index 00000000..d8374bf1 --- /dev/null +++ b/test/validate/test.proto @@ -0,0 +1,13 @@ +syntax = "proto3"; + +package test.validate; + +import "validate/validate.proto"; + +message Bar { + uint32 xyz = 1; +} + +message Foo { + Bar baz = 1 [(.validate.rules).message.required = true]; +} diff --git a/tools/protodoc/protodoc.py b/tools/protodoc/protodoc.py index 2adf7b09..91f6b907 100755 --- a/tools/protodoc/protodoc.py +++ b/tools/protodoc/protodoc.py @@ -505,7 +505,7 @@ def FormatFieldAsDefinitionListItem(outer_type_context, type_context, field): if field.options.HasExtension(validate_pb2.rules): rule = field.options.Extensions[validate_pb2.rules] if ((rule.HasField('message') and rule.message.required) or - (rule.HasField('string') and rule.string.min_len > 0) or + (rule.HasField('string') and rule.string.min_bytes > 0) or (rule.HasField('repeated') and rule.repeated.min_items > 0)): annotations.append('*REQUIRED*') leading_comment, comment_annotations = type_context.LeadingCommentPathLookup()