diff --git a/api/bootstrap.proto b/api/bootstrap.proto index 2fc37ec7..cff12c97 100644 --- a/api/bootstrap.proto +++ b/api/bootstrap.proto @@ -117,23 +117,23 @@ message Bootstrap { // Configuration for the LightStep tracer. message LightstepConfig { // The cluster manager cluster that hosts the LightStep collectors. - string collector_cluster = 1 [(validate.rules).string.min_len = 1]; + string collector_cluster = 1 [(validate.rules).string.min_bytes = 1]; // File containing the access token to the `LightStep // `_ API. - string access_token_file = 2 [(validate.rules).string.min_len = 1]; + string access_token_file = 2 [(validate.rules).string.min_bytes = 1]; } message ZipkinConfig { // The cluster manager cluster that hosts the Zipkin collectors. Note that the // Zipkin cluster must be defined in the :ref:`Bootstrap static cluster // resources `. - string collector_cluster = 1 [(validate.rules).string.min_len = 1]; + string collector_cluster = 1 [(validate.rules).string.min_bytes = 1]; // The API endpoint of the Zipkin service where the spans will be sent. When // using a standard Zipkin installation, the API endpoint is typically // /api/v1/spans, which is the default value. - string collector_endpoint = 2 [(validate.rules).string.min_len = 1]; + string collector_endpoint = 2 [(validate.rules).string.min_bytes = 1]; } // The :ref:`tracing ` configuration specifies global @@ -146,7 +146,7 @@ message Tracing { // The name of the HTTP trace driver to instantiate. The name must match a // supported HTTP trace driver. *envoy.lightstep* and *envoy.zipkin* are // built-in trace drivers. - string name = 1 [(validate.rules).string.min_len = 1]; + string name = 1 [(validate.rules).string.min_bytes = 1]; // Trace driver specific configuration which depends on the driver being // instantiated. See the :ref:`LightstepConfig @@ -163,7 +163,7 @@ message Tracing { message Admin { // The path to write the access log for the administration server. If no // access log is desired specify ‘/dev/null’. - string access_log_path = 1 [(validate.rules).string.min_len = 1]; + string access_log_path = 1 [(validate.rules).string.min_bytes = 1]; // The cpu profiler output path for the administration server. If no profile // path is specified, the default is ‘/var/log/envoy/envoy.prof’. @@ -358,12 +358,12 @@ message Runtime { // switched to. This parameter specifies the path to the symbolic link. Envoy // will watch the location for changes and reload the file system tree when // they happen. - string symlink_root = 1 [(validate.rules).string.min_len = 1]; + string symlink_root = 1 [(validate.rules).string.min_bytes = 1]; // Specifies the subdirectory to load within the root directory. This is // useful if multiple systems share the same delivery mechanism. Envoy // configuration elements can be contained in a dedicated subdirectory. - string subdirectory = 2 [(validate.rules).string.min_len = 1]; + string subdirectory = 2 [(validate.rules).string.min_bytes = 1]; // Specifies an optional subdirectory to load within the root directory. If // specified and the directory exists, configuration values within this @@ -379,5 +379,5 @@ message RateLimitServiceConfig { // Specifies the cluster manager cluster name that hosts the rate limit // service. The client will connect to this cluster when it needs to make rate // limit service requests. - string cluster_name = 1 [(validate.rules).string.min_len = 1]; + string cluster_name = 1 [(validate.rules).string.min_bytes = 1]; } diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index c1984620..7a8942c2 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -1,7 +1,7 @@ GOOGLEAPIS_SHA = "5c6df0cd18c6a429eab739fb711c27f6e1393366" # May 14, 2017 PROMETHEUS_SHA = "6f3806018612930941127f2a7c6c453ba2c527d2" # Nov 02, 2017 -PGV_GIT_SHA = "af35f0b0d2cee1178f77329d30252e07047918c3" +PGV_GIT_SHA = "36e0c0b5946eb7347d755b9b2f55bb3fc0967846" def api_dependencies(): native.git_repository( diff --git a/test/validate/BUILD b/test/validate/BUILD index ca1e70df..1563d47a 100644 --- a/test/validate/BUILD +++ b/test/validate/BUILD @@ -2,13 +2,26 @@ 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"], + proto_deps = [ + "//api:bootstrap", + "//api:cds", + "//api:eds", + "//api:lds", + "//api:protocol", + "//api:rds", + "//api/filter/accesslog", + "//api/filter/http:buffer", + "//api/filter/http:fault", + "//api/filter/http:health_check", + "//api/filter/http:lua", + "//api/filter/http:router", + "//api/filter/http:transcoder", + "//api/filter/network:http_connection_manager", + "//api/filter/network:mongo_proxy", + "//api/filter/network:redis_proxy", + "//api/filter/network:tcp_proxy", + ], ) diff --git a/test/validate/pgv_test.cc b/test/validate/pgv_test.cc index f52a8214..bfbd2b3f 100644 --- a/test/validate/pgv_test.cc +++ b/test/validate/pgv_test.cc @@ -1,33 +1,70 @@ #include #include -#include "test/validate/test.pb.validate.h" +// We don't use all the headers in the test below, but including them anyway as +// a cheap way to get some C++ compiler sanity checking. +#include "api/bootstrap.pb.validate.h" +#include "api/protocol.pb.validate.h" +#include "api/cds.pb.validate.h" +#include "api/eds.pb.validate.h" +#include "api/lds.pb.validate.h" +#include "api/rds.pb.validate.h" +#include "api/rds.pb.validate.h" +#include "api/filter/accesslog/accesslog.pb.validate.h" +#include "api/filter/http/buffer.pb.validate.h" +#include "api/filter/http/fault.pb.validate.h" +#include "api/filter/http/health_check.pb.validate.h" +#include "api/filter/http/lua.pb.validate.h" +#include "api/filter/http/router.pb.validate.h" +#include "api/filter/http/transcoder.pb.validate.h" +#include "api/filter/network/http_connection_manager.pb.validate.h" +#include "api/filter/network/mongo_proxy.pb.validate.h" +#include "api/filter/network/redis_proxy.pb.validate.h" +#include "api/filter/network/tcp_proxy.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; +#include "google/protobuf/text_format.h" +template struct TestCase { + void run() { std::string err; - if (Validate(empty, &err)) { - std::cout << "Unexpected successful validation of empty proto." << std::endl; + if (Validate(invalid_message, &err)) { + std::cerr << "Unexpected successful validation of invalid message: " + << invalid_message.DebugString() << std::endl; + exit(EXIT_FAILURE); + } + if (!Validate(valid_message, &err)) { + std::cerr << "Unexpected failed validation of valid message: " << valid_message.DebugString() + << ", " << err << std::endl; exit(EXIT_FAILURE); } } - { - test::validate::Foo non_empty; - non_empty.mutable_baz(); + Proto& invalid_message; + Proto& valid_message; +}; - std::string err; - if (!Validate(non_empty, &err)) { - std::cout << "Unexpected failed validation of empty proto: " << err << std::endl; - exit(EXIT_FAILURE); - } +// Basic protoc-gen-validate C++ validation header inclusion and Validate calls +// from data-plane-api. +int main(int argc, char* argv[]) { + envoy::api::v2::Bootstrap invalid_bootstrap; + // This is a baseline test of the validation features we care about. It's + // probably not worth adding in every filter and field that we want to valid + // in the API upfront, but as regressions occur, this is the place to add the + // specific case. + const std::string valid_bootstrap_text = R"EOF( + node {} + cluster_manager {} + admin { + access_log_path: "/dev/null" + address {} + } + )EOF"; + envoy::api::v2::Bootstrap valid_bootstrap; + if (!google::protobuf::TextFormat::ParseFromString(valid_bootstrap_text, &valid_bootstrap)) { + std::cerr << "Unable to parse text proto: " << valid_bootstrap_text << std::endl; + exit(EXIT_FAILURE); } + TestCase{invalid_bootstrap, valid_bootstrap}.run(); exit(EXIT_SUCCESS); } diff --git a/test/validate/test.proto b/test/validate/test.proto deleted file mode 100644 index d8374bf1..00000000 --- a/test/validate/test.proto +++ /dev/null @@ -1,13 +0,0 @@ -syntax = "proto3"; - -package test.validate; - -import "validate/validate.proto"; - -message Bar { - uint32 xyz = 1; -} - -message Foo { - Bar baz = 1 [(.validate.rules).message.required = true]; -}