pgv: bump version to 36e0c0b, validation rules pass C++ compiler checks. (#304)

Also a skeleton for testing valid/invalid Bootstrap (and later other)
filters.

Signed-off-by: Harvey Tuch <htuch@google.com>

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
   // <http://lightstep.com/>`_ 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 <envoy_api_field_Bootstrap.StaticResources.clusters>`.
-  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 <arch_overview_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];
 }

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(

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",
+    ],
 )

test/validate/pgv_test.cc

@@ -1,33 +1,70 @@
 #include <iostream>
 #include <cstdlib>
 
-#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 <class Proto> 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<envoy::api::v2::Bootstrap>{invalid_bootstrap, valid_bootstrap}.run();
 
   exit(EXIT_SUCCESS);
 }

test/validate/test.proto

@@ -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];
-}