Add convenience field support to Terraform, generate Network (#1489)

Merged PR #1489.

Author: rileykarson

api/type.rb

@@ -26,6 +26,11 @@ module Fields
       attr_reader :description
       attr_reader :exclude
 
+      # Add a deprecation message for a field that's been deprecated in the API
+      # use the YAML chomping folding indicator (>-) if this is a multiline
+      # string, as providers expect a single-line one w/o a newline.
+      attr_reader :deprecation_message
+
       attr_reader :output # If set value will not be sent to server on sync
       attr_reader :input # If set to true value is used only on creation
       attr_reader :url_param_only # If, true will not be send in request body
@@ -57,6 +62,7 @@ def validate
       super
       check :description, type: ::String, required: true
       check :exclude, type: :boolean, default: false, required: true
+      check :deprecation_message, type: ::String
       check :min_version, type: ::String
       check :output, type: :boolean
       check :required, type: :boolean
@@ -214,6 +220,10 @@ def nested_properties?
       !nested_properties.empty?
     end
 
+    def deprecated?
+      !(@deprecation_message.nil? || @deprecation_message == '')
+    end
+
     private
 
     # A constant value to be provided as field

build/ansible

@@ -64,6 +64,25 @@ def self.attributes
           # Names of attributes that can't be set alongside this one
           :conflicts_with,
 
+          # ====================
+          # Schema Modifications
+          # ====================
+          # Schema modifications change the schema of a resource in some
+          # fundamental way. They're not very portable, and will be hard to
+          # generate so we should limit their use. Generally, if you're not
+          # converting existing Terraform resources, these shouldn't be used.
+          #
+          # With great power comes great responsibility.
+
+          # Flatten the child field of a NestedObject into "convenience fields"
+          # that are addressed as if they were top level fields.
+          #
+          # We need this for cases where a field inside a nested object has a
+          # default, if we can't spend a breaking change to fix a misshapen
+          # field, or if the UX is _much_ better otherwise. Nesting flattened
+          # NestedObjects is inadvisable.
+          :flatten_object,
+
           # ===========
           # Custom code
           # ===========
@@ -121,6 +140,11 @@ def apply(api_property)
                                        api_property.description)
         end
 
+        if @flatten_object && !api_property.is_a?(Api::Type::NestedObject)
+          raise 'Only NestedObjects can be flattened with flatten_object. Type'\
+            " is #{api_property.class} for property #{api_property.name}"
+        end
+
         unless api_property.is_a?(Api::Type::Array) ||
                api_property.is_a?(Api::Type::Map)
           if @is_set

build/terraform

@@ -2635,58 +2635,53 @@ objects:
     collection_url_response: !ruby/object:Api::Resource::ResponseList
       kind: 'compute#networkList'
       items: 'items'
-    update_verb: :PATCH
+    input: true
     has_self_link: true
     references: !ruby/object:Api::Resource::ReferenceLinks
       guides:
         'Official Documentation': 'https://cloud.google.com/vpc/docs/vpc'
       api: 'https://cloud.google.com/compute/docs/reference/rest/v1/networks'
     description: |
-      Represents a Network resource.
-
-      Your Cloud Platform Console project can contain multiple networks, and
-      each network can have multiple instances attached to it. A network allows
-      you to define a gateway IP and the network range for the instances
-      attached to that network. Every project is provided with a default network
-      with preset configurations and firewall rules. You can choose to customize
-      the default network by adding or removing rules, or you can create new
-      networks in that project. Generally, most users only need one network,
-      although you can have up to five networks per project by default.
-
-      A network belongs to only one project, and each instance can only belong
-      to one network. All Compute Engine networks use the IPv4 protocol. Compute
-      Engine currently does not support IPv6. However, Google is a major
-      advocate of IPv6 and it is an important future direction.
+      Manages a VPC network or legacy network resource on GCP.
 <%= indent(compile_file({}, 'templates/global_async.yaml.erb'), 4) %>
     properties:
       - !ruby/object:Api::Type::String
         name: 'description'
         description: |
-          An optional description of this resource. Provide this property when
-          you create the resource.
+          An optional description of this resource. The resource must be
+          recreated to modify this field.
         input: true
       - !ruby/object:Api::Type::String
         name: 'gateway_ipv4'
-        description: |
-          A gateway address for default routing to other networks. This value is
-          read only and is selected by the Google Compute Engine, typically as
-          the first usable address in the IPv4Range.
+        # We override this in api.yaml so that the name is more aesthetic
         api_name: 'gatewayIPv4'
         output: true
+        description: |
+          The gateway address for default routing out of the network. This value
+          is selected by GCP.
       - !ruby/object:Api::Type::Integer
         name: 'id'
         description: 'The unique identifier for the resource.'
         output: true
       - !ruby/object:Api::Type::String
         name: 'ipv4_range'
-        description: |
-          The range of internal addresses that are legal on this network. This
-          range is a CIDR specification, for example: 192.168.0.0/16. Provided
-          by the client when the network is created.
+        # We override this in api.yaml so that the name is more aesthetic
         api_name: 'IPv4Range'
+        deprecation_message: >-
+          Legacy Networks are deprecated and you will no longer be able to
+          create them using this field from Feb 1, 2020 onwards.
         input: true
         conflicts:
           - autoCreateSubnetworks
+        description: |
+          If this field is specified, a deprecated legacy network is created.
+          You will no longer be able to create a legacy network on Feb 1, 2020.
+          See the [legacy network docs](https://cloud.google.com/vpc/docs/legacy)
+          for more details.
+
+          The range of internal addresses that are legal on this legacy network.
+          This range is a CIDR specification, for example: `192.168.0.0/16`.
+          The resource must be recreated to modify this field.
       - !ruby/object:Api::Type::String
         name: 'name'
         description: |
@@ -2710,19 +2705,21 @@ objects:
       - !ruby/object:Api::Type::Boolean
         name: 'autoCreateSubnetworks'
         description: |
-          When set to true, the network is created in "auto subnet mode". When
-          set to false, the network is in "custom subnet mode".
+          When set to `true`, the network is created in "auto subnet mode" and
+          it will create a subnet for each region automatically across the
+          `10.128.0.0/9` address range.
 
-          In "auto subnet mode", a newly created network is assigned the default
-          CIDR of 10.128.0.0/9 and it automatically creates one subnetwork per
-          region.
+          When set to `false`, the network is created in "custom subnet mode" so
+          the user can explicitly connect subnetwork resources.
         input: true
       - !ruby/object:Api::Type::Time
         name: 'creationTimestamp'
         description: 'Creation timestamp in RFC3339 text format.'
         output: true
       - !ruby/object:Api::Type::NestedObject
         name: 'routingConfig'
+        update_verb: :PATCH
+        update_url:  projects/{{project}}/global/networks/{{name}}
         description: |
           The network-level routing configuration for this network. Used by Cloud
           Router to determine what type of network-wide routing behavior to
@@ -2732,9 +2729,9 @@ objects:
           name: 'routingMode'
           required: true
           description: |
-            The network-wide routing mode to use. If set to REGIONAL, this
+            The network-wide routing mode to use. If set to `REGIONAL`, this
             network's cloud routers will only advertise routes with subnetworks
-            of this network in the same region as the router. If set to GLOBAL,
+            of this network in the same region as the router. If set to `GLOBAL`,
             this network's cloud routers will advertise routes with all
             subnetworks of this network, across regions.
           values:

build/terraform-beta

@@ -509,7 +509,39 @@ overrides: !ruby/object:Overrides::ResourceOverrides
   MachineType: !ruby/object:Overrides::Terraform::ResourceOverride
     exclude: true
   Network: !ruby/object:Overrides::Terraform::ResourceOverride
-    exclude: true
+    examples:
+      - !ruby/object:Provider::Terraform::Examples
+        name: "network_basic"
+        primary_resource_id: "vpc_network"
+        vars:
+          network_name: "vpc-network"
+    properties:
+      id: !ruby/object:Overrides::Terraform::PropertyOverride
+        exclude: true
+      # We want this to be the smallest change possible, so exclude this for now.
+      # TODO: remove exclusion
+      creationTimestamp: !ruby/object:Overrides::Terraform::PropertyOverride
+        exclude: true
+      # We could include this, but it won't be correct mid-apply when subnetworks
+      # are modified, so it's probably better to exclude.
+      subnetworks: !ruby/object:Overrides::Terraform::PropertyOverride
+        exclude: true
+      autoCreateSubnetworks: !ruby/object:Overrides::Terraform::PropertyOverride
+        default_value: True
+        # autoCreateSubnetworks defaults to true, so we need to disable it explicitly
+        conflicts: []
+      ipv4_range: !ruby/object:Overrides::Terraform::PropertyOverride
+        # autoCreateSubnetworks defaults to true, so we need to disable it explicitly
+        conflicts: []
+      routingConfig: !ruby/object:Overrides::Terraform::PropertyOverride
+        flatten_object: true
+      routingConfig.routingMode: !ruby/object:Overrides::Terraform::PropertyOverride
+        # while required inside routingConfig, routingMode is an optional convenience
+        # field.
+        required: false
+        default_from_api: true
+    custom_code: !ruby/object:Provider::Terraform::CustomCode
+      encoder: templates/terraform/encoders/network.erb
   Region: !ruby/object:Overrides::Terraform::ResourceOverride
     exclude: true
   RegionAutoscaler: !ruby/object:Overrides::Terraform::ResourceOverride

build/terraform-mapper

@@ -0,0 +1,5 @@
+if _, ok := d.GetOk("ipv4_range"); !ok {
+	obj["autoCreateSubnetworks"] = d.Get("auto_create_subnetworks")
+}
+
+return obj, nil

overrides/terraform/property_override.rb

@@ -0,0 +1,3 @@
+resource "google_compute_network" "<%= ctx[:primary_resource_id] %>" {
+  name = "<%= ctx[:vars]['network_name'] %>"
+}

products/compute/api.yaml

@@ -35,17 +35,17 @@ func expand<%= prefix -%><%= titlelize_property(property) -%>(v interface{}, d T
     }
     transformed["<%= prop.api_name -%>"] = transformed<%= titlelize_property(prop) -%>
 
-<%   end -%>
+<%     end -%>
 
     m[original["<%= property.key_name -%>"].(string)] = transformed
   }
   return m, nil
 }
 
-<%   property.nested_properties.each do |prop| -%>
-<%     next if prop.name == property.key_name -%>
-<%=      lines(build_expand_method(prefix + titlelize_property(property), prop), 1) -%>
-<%   end -%>
+<%     property.nested_properties.each do |prop| -%>
+<%       next if prop.name == property.key_name -%>
+<%=        lines(build_expand_method(prefix + titlelize_property(property), prop), 1) -%>
+<%     end -%>
 <%   elsif property.is_a?(Api::Type::KeyValuePairs) -%>
 func expand<%= prefix -%><%= titlelize_property(property) -%>(v interface{}, d TerraformResourceData, config *Config) (map[string]string, error) {
   if v == nil {
@@ -57,6 +57,30 @@ func expand<%= prefix -%><%= titlelize_property(property) -%>(v interface{}, d T
   }
   return m, nil
 }
+<%   elsif property.flatten_object -%>
+func expand<%= prefix -%><%= titlelize_property(property) -%>(d TerraformResourceData, config *Config) (interface{}, error) {
+  transformed := make(map[string]interface{})
+<%     property.nested_properties.each do |prop| -%>
+  // Note that nesting flattened objects won't work because we don't handle them properly here.
+  transformed<%= titlelize_property(prop) -%>, err := expand<%= prefix -%><%= titlelize_property(property) -%><%= titlelize_property(prop) -%>(d.Get("<%= prop.name.underscore -%>"), d, config)
+  if err != nil {
+    return nil, err
+<%       unless prop.send_empty_value -%>
+  } else if val := reflect.ValueOf(transformed<%= titlelize_property(prop) -%>); val.IsValid() && !isEmptyValue(val) {
+    transformed["<%= prop.api_name -%>"] = transformed<%= titlelize_property(prop) -%>
+<%       else -%>
+  } else {
+    transformed["<%= prop.api_name -%>"] = transformed<%= titlelize_property(prop) -%>
+<%       end -%>
+  }
+
+  return transformed, nil
+<%     end -%>
+}
+
+<%     property.nested_properties.each do |prop| -%>
+<%=      lines(build_expand_method(prefix + titlelize_property(property), prop), 1) -%>
+<%     end -%>
 <%   elsif tf_types.include?(property.class) -%>
 func expand<%= prefix -%><%= titlelize_property(property) -%>(v interface{}, d TerraformResourceData, config *Config) (interface{}, error) {
 <%     if property.is_set -%>

products/compute/terraform.yaml

@@ -16,6 +16,12 @@
 <%= lines(compile_template(property.custom_flatten,
                            prefix: prefix,
                            property: property)) -%>
+<% elsif property.flatten_object -%>
+<%   if property.nested_properties? -%>
+  <%   property.nested_properties.each do |prop| -%>
+    <%=   lines(build_flatten_method(prefix + titlelize_property(property), prop), 1) -%>
+  <%   end -%>
+<%   end -%>
 <% else -%>
 <% if tf_types.include?(property.class) -%>
 func flatten<%= prefix -%><%= titlelize_property(property) -%>(v interface{}, d *schema.ResourceData) interface{} {

templates/terraform/encoders/network.erb

@@ -1,6 +1,6 @@
 
 <%
-  if property.nested_properties?
+  if property.nested_properties? && (property.flatten_object.nil? || !property.flatten_object)
 -%>
 The `<%= property.name.underscore -%>` block <%= if property.output then "contains" else "supports" end -%>:
 <%- if property.is_a?(Api::Type::Map) %>

templates/terraform/examples/network_basic.tf.erb

@@ -31,6 +31,18 @@ func Get<%= resource_name -%>CaiObject(d TerraformResourceData, config *Config)
 func Get<%= resource_name -%>ApiObject(d TerraformResourceData, config *Config) (map[string]interface{}, error) {
     obj := make(map[string]interface{})
 <%  object.settable_properties.each do |prop| -%>
+<%    if prop.flatten_object -%>
+    <%= prop.api_name -%>Prop, err := expand<%= resource_name -%><%= titlelize_property(prop) -%>(d, config)
+    if err != nil {
+        return nil, err
+<%      unless prop.send_empty_value -%>
+    } else if !isEmptyValue(reflect.ValueOf(<%= prop.api_name -%>Prop)) {
+<%      else -%>
+    } else {
+<%      end -%>
+        obj["<%= prop.api_name -%>"] = <%= prop.api_name -%>Prop
+    }
+<%    else -%>
     <%= prop.api_name -%>Prop, err := expand<%= resource_name -%><%= titlelize_property(prop) -%>(d.Get("<%= prop.name.underscore -%>"), d, config)
     if err != nil {
         return nil, err
@@ -41,6 +53,7 @@ func Get<%= resource_name -%>ApiObject(d TerraformResourceData, config *Config)
 <%      end -%>
         obj["<%= prop.api_name -%>"] = <%= prop.api_name -%>Prop
     }
+<%    end -%>
 <%  end -%>
 
 <%  if object.custom_code.encoder -%>

templates/terraform/expand_property_method.erb

@@ -8,9 +8,9 @@
 <%   end -%>
 <% else -%>
 <%   if property.required -%>
-  (Required)
+  (Required<% if property.deprecated? -%>, Deprecated<% end -%>)
 <%   elsif !property.output -%>
-  (Optional)
+  (Optional<% if property.deprecated? -%>, Deprecated<% end -%>)
 <%   end -%>
 <% end -%>
 <%= indent(property.description.strip.gsub("\n\n", "\n"), 2) -%>