docs/constraints: address.proto and base.proto finalized. (#253)

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

api/BUILD

@@ -189,6 +189,7 @@ api_proto_library(
 proto_library(
     name = "api",
     deps = [
+        ":bootstrap",
         ":cds",
         ":eds",
         ":lds",

api/address.proto

@@ -6,35 +6,42 @@ import "google/protobuf/wrappers.proto";
 
 import "validate/validate.proto";
 
-// [#protodoc-title: Network related configuration elements]
+// [#protodoc-title: Network addresses]
 // [#v2-api-diff: Addresses now have .proto structure.]
 
 message Pipe {
-  string path = 1;
+  // Unix Domain Socket path.
+  string path = 1 [(validate.rules).string.min_len = 1];
 }
 
 message SocketAddress {
   enum Protocol {
     TCP = 0;
-    // [#not-implemented-warn:]
+    // [#not-implemented-hide:]
     UDP = 1;
   }
-  Protocol protocol = 1;
-  // The address for this socket.  Listeners will bind to the address.  An empty
-  // address implies a bind to 0.0.0.0 or ::. It's still possible to distinguish on
+  Protocol protocol = 1 [(validate.rules).enum = {in: [0]}];
+  // The address for this socket. :ref:`Listeners <config_listeners>` will bind
+  // to the address or outbound connections will be made. An empty address
+  // implies a bind to 0.0.0.0 or ::. It's still possible to distinguish on an
   // address via the prefix/suffix matching in FilterChainMatch after connection.
-  // For clusters, an address may be either an IP or hostname to be resolved via
-  // DNS.  If it is a hostname, resolver_name should be set.
+  // For :ref:`clusters <config_cluster_manager_cluster>`, an address may be
+  // either an IP or hostname to be resolved via DNS. If it is a hostname,
+  // :ref:`resolver_name <envoy_api_field_SocketAddress.resolver_name>` should
+  // be set unless default (i.e. DNS) resolution is expected.
   string address = 2;
   oneof port_specifier {
+    option (validate.required) = true;
     uint32 port_value = 3;
-    // This is only valid if DNS SRV or if resolver_name is specified below
-    // and the named resolver is capable of named port resolution.
+    // This is only valid if :ref:`resolver_name
+    // <envoy_api_field_SocketAddress.resolver_name>` is specified below and the
+    // named resolver is capable of named port resolution.
     string named_port = 4;
   }
   // The name of the resolver. This must have been registered with Envoy. If this is
   // empty, a context dependent default applies. If address is a hostname this
-  // should be set.  If the address is a concrete IP address, no resolution will occur.
+  // should be set for resolution other than DNS. If the address is a concrete
+  // IP address, no resolution will occur.
   string resolver_name = 5;
 }
 
@@ -48,14 +55,17 @@ message BindConfig {
 // management servers.
 message Address {
   oneof address {
+    option (validate.required) = true;
     SocketAddress socket_address = 1;
     Pipe pipe = 2;
   }
 }
 
 // CidrRange specifies an IP Address and a prefix length to construct
-// the subnet mask.
+// the subnet mask for a `CIDR <https://tools.ietf.org/html/rfc4632>`_ range.
 message CidrRange {
-  string address_prefix = 1;
-  google.protobuf.UInt32Value prefix_len = 2;
+  // IPv4 or IPv6 address, e.g. 192.0.0.0 or 2001:db8::.
+  string address_prefix = 1 [(validate.rules).string.min_len = 1];
+  // Length of prefix, e.g. 0, 32.
+  google.protobuf.UInt32Value prefix_len = 2 [(validate.rules).uint32.lte = 128];
 }

api/base.proto

@@ -9,35 +9,47 @@ import "google/protobuf/duration.proto";
 import "google/protobuf/struct.proto";
 import "google/protobuf/wrappers.proto";
 
-// [#protodoc-title: Common configuration elements]
+import "validate/validate.proto";
+
+// [#protodoc-title: Common types]
 
 // Identifies location of where either Envoy runs or where upstream hosts run.
 message Locality {
-  // Region this zone belongs to.
+  // Region this :ref:`zone <envoy_api_field_Locality.zone>` belongs to.
   string region = 1;
 
-  // Availability Zone (AZ) in AWS, Zone in GCP.
+  // `Availability Zone (AZ)
+  // <https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html>`_
+  // on AWS, `Zone <https://cloud.google.com/compute/docs/regions-zones/>`_ on
+  // GCP, etc.
   string zone = 2;
 
   // When used for locality of upstream hosts, this field further splits zone
-  // into smaller chunks of sub_zones so they can be load balanced
-  // independently
+  // into smaller chunks of sub-zones so they can be load balanced
+  // independently.
   string sub_zone = 3;
 }
 
-// Identifies a specific Envoy instance. Remote server may have per Envoy configuration.
+// Identifies a specific Envoy instance. The node identifier is presented to the
+// management server, which may use this identifier to distinguish per Envoy
+// configuration for serving.
 message Node {
-  // An opaque node identifier for the Envoy node. This must be set.
-  string id = 1;
-  // The cluster that the Envoy node belongs to. This must be set.
-  string cluster = 2;
+  // An opaque node identifier for the Envoy node.
+  string id = 1 [(validate.rules).string.min_len = 1];
+  // The cluster that the Envoy node belongs to.
+  string cluster = 2 [(validate.rules).string.min_len = 1];
+  // Opaque metadata extending the node identifier. Envoy will pass this
+  // directly to the management server.
   google.protobuf.Struct metadata = 3;
+  // Locality specifying where the Envoy instance is running.
   Locality locality = 4;
   // This is motivated by informing a management server during canary which
-  // version of Envoy is being tested in a heterogeneous fleet.
+  // version of Envoy is being tested in a heterogeneous fleet. This will be set
+  // by Envoy in management server RPCs.
   string build_version = 5;
 }
 
+// Upstream host identifier.
 message Endpoint {
   Address address = 1;
 }
@@ -57,11 +69,12 @@ message Endpoint {
 // object to match against. There are some well defined metadata used today for
 // this purpose:
 //
-// * {"envoy.lb": {"canary": <bool> }}. This indicates the canary status of an
+// * ``{"envoy.lb": {"canary": <bool> }}`` This indicates the canary status of an
 //   endpoint and is also used during header processing
 //   (x-envoy-upstream-canary) and for stats purposes.
 message Metadata {
-  // Key is the reverse DNS filter name.
+  // Key is the reverse DNS filter name, e.g. com.acme.widget. The envoy.*
+  // namespace is reserved for Envoy's built-in filters.
   map<string, google.protobuf.Struct> filter_metadata = 1;
 }
 
@@ -74,7 +87,8 @@ message RuntimeUInt32 {
   string runtime_key = 3;
 }
 
-// Envoy supports upstream priority routing both at the route and the virtual
+// Envoy supports :ref:`upstream priority routing
+// <arch_overview_http_routing_priority>` both at the route and the virtual
 // cluster level. The current priority implementation uses different connection
 // pool and circuit breaking settings for each priority level. This means that
 // even for HTTP/2 requests, two physical connections will be used to an
@@ -85,7 +99,7 @@ enum RoutingPriority {
   HIGH = 1;
 }
 
-// HTTP request method
+// HTTP request method.
 enum RequestMethod {
   METHOD_UNSPECIFIED = 0;
   GET = 1;
@@ -104,61 +118,78 @@ message HeaderValue {
   string key = 1;
 
   // Header value.
+  //
+  // The same :ref:`format specifier <config_access_log_format>` as used for
+  // :ref:`HTTP access logging <config_access_log>` applies here, however
+  // unknown header values are replaced with the empty string instead of `-`.
   string value = 2;
 }
 
 // Header name/value pair plus option to control append behavior.
 message HeaderValueOption {
-  // Header Name/Value pair that this option applies to.
-  //
-  // The same format specifier as used for HTTP access logging applies here,
-  // however unknown header values are replaced with the empty string instead
-  // of -. [V2-API-DIFF].
+  // Header name/value pair that this option applies to.
   HeaderValue header = 1;
 
   // Should the value be appended? If true (default), the value is appended to
-  // existing values [V2-API-DIFF].
+  // existing values.
   google.protobuf.BoolValue append = 2;
 }
 
+// API configuration source. This identifies the API type and cluster that Envoy
+// will use to fetch an xDS API.
 message ApiConfigSource {
   // APIs may be fetched via either REST or gRPC.
   enum ApiType {
-    // REST legacy corresponds to the v1 API.
+    // REST-JSON legacy corresponds to the v1 API.
     REST_LEGACY = 0;
+    // REST-JSON v2 API. The `canonical JSON encoding
+    // <https://developers.google.com/protocol-buffers/docs/proto3#json>`_ for
+    // the v2 protos is used.
     REST = 1;
+    // gRPC v2 API.
     GRPC = 2;
   }
   ApiType api_type = 1;
   // Multiple cluster names may be provided. If > 1 cluster is defined, clusters
   // will be cycled through if any kind of failure occurs.
-  repeated string cluster_name = 2;
+  repeated string cluster_name = 2 [(validate.rules).repeated.min_items = 1];
   // For REST APIs, the delay between successive polls.
   google.protobuf.Duration refresh_delay = 3;
 }
 
-// ADS will be used to fetch resources.
+// Aggregated Discovery Service (ADS) options. This is currently empty, but when
+// set in :ref:`ConfigSource <envoy_api_msg_ConfigSource>` can be used to
+// specify that ADS is to be used.
 message AggregatedConfigSource {
 }
 
-// Configuration for listeners, clusters, routes, endpoints etc. may either be
-// sourced from the filesystem or from an API source. Filesystem configs are
-// watched with inotify for updates.
+// Configuration for :ref:`listeners <config_listeners>`, :ref:`clusters
+// <config_cluster_manager_cluster>`, :ref:`routes
+// <config_http_conn_man_route_table>`, :ref:`endpoints
+// <arch_overview_service_discovery>` etc. may either be sourced from the
+// filesystem or from an xDS API source. Filesystem configs are watched with
+// inotify for updates.
 message ConfigSource {
   oneof config_source_specifier {
+    option (validate.required) = true;
+    // Path on the filesystem to source and watch for configuration updates.
     string path = 1;
+    // API configuration source.
     ApiConfigSource api_config_source = 2;
+    // When set, ADS will be used to fetch resources. The ADS API configuration
+    // source in the bootstrap configuration is used.
     AggregatedConfigSource ads = 3;
   }
 }
 
-// Configuration for transport socket in listeners and clusters. If the configuration is empty,
-// a default transport socket implementation and configuration will be chosen based on the
-// platform and existence of tls_context.
+// Configuration for transport socket in :ref:`listeners <config_listeners>` and
+// :ref:`clusters <config_cluster_manager_cluster>`. If the configuration is
+// empty, a default transport socket implementation and configuration will be
+// chosen based on the platform and existence of tls_context.
 message TransportSocket {
   // The name of the transport socket to instantiate. The name must match a supported transport
   // socket implementation.
-  string name = 1;
+  string name = 1 [(validate.rules).string.min_len = 1];
 
   // Implementation specific configuration which depends on the implementation being instantiated.
   // See the supported transport socket implementations for further documentation.

tools/protodoc/protodoc.py

@@ -140,6 +140,8 @@ class TypeContext(object):
     self.map_typenames = {}
     # Map from a message's oneof index to the fields sharing a oneof.
     self.oneof_fields = {}
+    # Map from a message's oneof index to the "required" bool property.
+    self.oneof_required = {}
 
   def Extend(self, path, name):
     extended = copy.deepcopy(self)
@@ -349,27 +351,33 @@ def FormatAnchor(label):
   return '.. _%s:\n\n' % label
 
 
-def FormatFieldAsDefinitionListItem(type_context, field):
+def FormatFieldAsDefinitionListItem(outer_type_context, type_context, field):
   """Format a FieldDescriptorProto as RST definition list item.
 
   Args:
+    outer_type_context: contextual information for enclosing message.
     type_context: contextual information for message/enum/field.
     field: FieldDescriptorProto.
   Returns:
     RST formatted definition list item.
   """
   if field.HasField('oneof_index'):
-    oneof_comment = '\nOnly one of %s may be set.\n' % ', '.join(
-        type_context.oneof_fields[field.oneof_index])
+    oneof_template = '\nPrecisely one of %s must be set.\n' if type_context.oneof_required[
+        field.oneof_index] else '\nOnly one of %s may be set.\n'
+    oneof_comment = oneof_template % ', '.join(
+        FormatInternalLink(
+            f, FieldCrossRefLabel(outer_type_context.Extend([], f).name))
+        for f in type_context.oneof_fields[field.oneof_index])
   else:
     oneof_comment = ''
   anchor = FormatAnchor(FieldCrossRefLabel(type_context.name))
   annotations = []
   if field.options.HasExtension(validate_pb2.rules):
     rule = field.options.Extensions[validate_pb2.rules]
-    if rule.HasField('message'):
-      if rule.message.required:
-        annotations.append('*REQUIRED*')
+    if ((rule.HasField('message') and rule.message.required) or
+        (rule.HasField('string') and rule.string.min_len > 0) or
+        (rule.HasField('repeated') and rule.repeated.min_items > 0)):
+      annotations.append('*REQUIRED*')
   leading_comment, comment_annotations = type_context.LeadingCommentPathLookup()
   if NOT_IMPLEMENTED_HIDE_ANNOTATION in comment_annotations:
     return ''
@@ -389,12 +397,17 @@ def FormatMessageAsDefinitionList(type_context, msg):
     RST formatted definition list item.
   """
   type_context.oneof_fields = defaultdict(list)
+  type_context.oneof_required = defaultdict(bool)
   for field in msg.field:
     if field.HasField('oneof_index'):
       type_context.oneof_fields[field.oneof_index].append(field.name)
+  for index, oneof_decl in enumerate(msg.oneof_decl):
+    if oneof_decl.options.HasExtension(validate_pb2.required):
+      type_context.oneof_required[index] = oneof_decl.options.Extensions[
+          validate_pb2.required]
   return '\n'.join(
       FormatFieldAsDefinitionListItem(
-          type_context.Extend([2, index], field.name), field)
+          type_context, type_context.Extend([2, index], field.name), field)
       for index, field in enumerate(msg.field)) + '\n'
 
 
@@ -516,7 +529,7 @@ def GenerateRst(proto_file):
           TypeContext(source_code_info, [5, index], package_prefix + enum.name),
           enum) for index, enum in enumerate(proto_file.enum_type))
   debug_proto = FormatProtoAsBlockComment(proto_file)
-  return header + comment + msgs + enums  #+ debug_proto
+  return header + comment + msgs + enums  # + debug_proto
 
 
 if __name__ == '__main__':