Allow redisVersion to change, promoted some fields to GA

mmv1/products/compute/api.yaml

@@ -1693,9 +1693,6 @@ objects:
       # TODO: make a ResourceRef to Security Policy
       - !ruby/object:Api::Type::String
         name: 'securityPolicy'
-        update_verb: :POST
-        update_url:
-          'projects/{{project}}/global/backendServices/{{name}}/setSecurityPolicy'
         description: |
           The security policy associated with this backend service.
       - !ruby/object:Api::Type::Enum
@@ -9208,8 +9205,6 @@ objects:
       - !ruby/object:Api::Type::NestedObject
         name: 'preservedState'
         description: 'The preserved state for this instance.'
-        update_verb: :POST
-        update_url: 'projects/{{project}}/zones/{{zone}}/instanceGroupManagers/{{instance_group_manager}}/updatePerInstanceConfigs'
         properties:
           - !ruby/object:Api::Type::KeyValuePairs
             name: 'metadata'
@@ -9329,7 +9324,6 @@ objects:
         name: 'preservedState'
         description: 'The preserved state for this instance.'
         update_verb: :POST
-        update_url: 'projects/{{project}}/regions/{{region}}/instanceGroupManagers/{{region_instance_group_manager}}/updatePerInstanceConfigs'
         properties:
           - !ruby/object:Api::Type::KeyValuePairs
             name: 'metadata'
@@ -18277,8 +18271,6 @@ objects:
           the BackendService. The protocol field in the BackendService
           must be set to GRPC.
         input: true
-        update_verb: :PATCH
-        update_url: projects/{{project}}/global/targetGrpcProxies
         update_id: 'urlMap'
         fingerprint_name: 'fingerprint'
       - !ruby/object:Api::Type::Boolean

mmv1/products/healthcare/api.yaml

@@ -54,7 +54,6 @@ objects:
         input: true
       - !ruby/object:Api::Type::String
         name: "timeZone"
-        update_url: 'projects/{{project}}/locations/{{location}}/datasets/{{name}}'
         description: |
           The default timezone used by this dataset. Must be a either a valid IANA time zone name such as
           "America/New_York" or empty, which defaults to UTC. This is used for parsing times in resources
@@ -110,7 +109,6 @@ objects:
       - !ruby/object:Api::Type::KeyValuePairs
         name: labels
         required: false
-        update_url: '{{dataset}}/dicomStores/{{name}}'
         description: |
           User-supplied key-value pairs used to organize DICOM stores.
 
@@ -128,7 +126,6 @@ objects:
       - !ruby/object:Api::Type::NestedObject
         name: notificationConfig
         required: false
-        update_url: '{{dataset}}/dicomStores/{{name}}'
         properties:
           - !ruby/object:Api::Type::String
             name: pubsubTopic
@@ -156,7 +153,6 @@ objects:
         name: streamConfigs
         required: false
         min_version: beta
-        update_url: '{{dataset}}/dicomStores/{{name}}'
         description: |
           To enable streaming to BigQuery, configure the streamConfigs object in your DICOM store.
           streamConfigs is an array, so you can specify multiple BigQuery destinations. You can stream metadata from a single DICOM store to up to five BigQuery tables in a BigQuery dataset.
@@ -243,7 +239,6 @@ objects:
           identifiers, those IDs will be part of the FHIR resource path recorded in Cloud audit logs and Cloud Pub/Sub
           notifications.
         required: false
-        update_url: '{{dataset}}/fhirStores/{{name}}'
       - !ruby/object:Api::Type::Boolean
         name: 'disableReferentialIntegrity'
         description: |
@@ -284,7 +279,6 @@ objects:
       - !ruby/object:Api::Type::KeyValuePairs
         name: labels
         required: false
-        update_url: '{{dataset}}/fhirStores/{{name}}'
         description: |
           User-supplied key-value pairs used to organize FHIR stores.
 
@@ -302,7 +296,6 @@ objects:
       - !ruby/object:Api::Type::NestedObject
         name: notificationConfig
         required: false
-        update_url: '{{dataset}}/fhirStores/{{name}}'
         properties:
           - !ruby/object:Api::Type::String
             name: pubsubTopic
@@ -420,7 +413,6 @@ objects:
       - !ruby/object:Api::Type::NestedObject
         name: parserConfig
         required: false
-        update_url: '{{dataset}}/hl7V2Stores/{{name}}'
         properties:
           - !ruby/object:Api::Type::Boolean
             name: allowNullHeader
@@ -463,7 +455,6 @@ objects:
       - !ruby/object:Api::Type::KeyValuePairs
         name: labels
         required: false
-        update_url: '{{dataset}}/hl7V2Stores/{{name}}'
         description: |
           User-supplied key-value pairs used to organize HL7v2 stores.
 
@@ -516,7 +507,6 @@ objects:
         removed_message: This field has been replaced by notificationConfigs
         exact_version: ga
         required: false
-        update_url: '{{dataset}}/hl7V2Stores/{{name}}'
         properties:
           - !ruby/object:Api::Type::String
             name: pubsubTopic
@@ -534,7 +524,6 @@ objects:
         deprecation_message: This field has been replaced by notificationConfigs
         exact_version: beta
         required: false
-        update_url: '{{dataset}}/hl7V2Stores/{{name}}'
         properties:
           - !ruby/object:Api::Type::String
             name: pubsubTopic

mmv1/products/redis/api.yaml

@@ -168,7 +168,6 @@ objects:
           - REDIS_5_0 for Redis 5.0 compatibility
           - REDIS_4_0 for Redis 4.0 compatibility
           - REDIS_3_2 for Redis 3.2 compatibility
-        input: true
       - !ruby/object:Api::Type::String
         name: reservedIpRange
         description: |
@@ -192,7 +191,6 @@ objects:
         input: true
       - !ruby/object:Api::Type::Enum
         name: transitEncryptionMode
-        min_version: beta
         input: true
         description: |
           The TLS mode of the Redis instance, If not provided, TLS is disabled for the instance.
@@ -204,7 +202,6 @@ objects:
         default_value: :DISABLED
       - !ruby/object:Api::Type::Array
         name: 'serverCaCerts'
-        min_version: beta
         description: |
           List of server CA certificates for the instance.
         output: true

mmv1/products/redis/terraform.yaml

@@ -67,6 +67,8 @@ overrides: !ruby/object:Overrides::ResourceOverrides
           regex: '^[a-z][a-z0-9-]{0,39}[a-z0-9]$'
       redisVersion: !ruby/object:Overrides::Terraform::PropertyOverride
         default_from_api: true
+        update_url: 'projects/{{project}}/locations/{{region}}/instances/{{name}}:upgrade'
+        update_verb: :POST
       region: !ruby/object:Overrides::Terraform::PropertyOverride
         ignore_read: true
         required: false

mmv1/provider/core.rb

@@ -325,6 +325,16 @@ def properties_by_custom_update(properties, behavior = :new)
       end
     end
 
+    # Filter the properties to keep only the ones don't have custom update
+    # method and group them by update url & verb.
+    def properties_without_custom_update(properties)
+      update_props = properties.reject do |p|
+        !(p.update_url.nil? || p.update_verb.nil? || p.update_verb == :NOOP)
+      end
+
+      update_props
+    end
+
     # Takes a update_url and returns the list of custom updatable properties
     # that can be updated at that URL. This allows flattened objects
     # to determine which parent property in the API should be updated with

mmv1/templates/terraform/resource.erb

@@ -35,8 +35,8 @@ import (
 <%
     resource_name = product_ns + object.name
     properties = object.all_user_properties
-    update_body_properties = object.settable_properties
-    update_body_properties = object.settable_properties.reject(&:input) if object.update_verb == :PATCH
+    update_body_properties = properties_without_custom_update(object.settable_properties)
+    update_body_properties = update_body_properties.reject(&:input) if object.update_verb == :PATCH
     # Handwritten TF Operation objects will be shaped like accessContextManager while the Google Go Client will have a name like accesscontextmanager
     client_name = @config.client_name || product_ns
     client_name_camel = client_name.camelize(:lower)
@@ -577,7 +577,104 @@ func resource<%= resource_name -%>Update(d *schema.ResourceData, meta interface{
     billingProject = project
 <%  end -%>
 
-<%  if object.input -%>
+
+<%  if !object.input -%>
+    obj := make(map[string]interface{})
+<%  update_body_properties.each do |prop| -%>
+    <%# flattened objects won't have something stored in state so instead nil is passed to the next expander. -%>
+    <% schemaPrefix = prop.flatten_object ? "nil" : "d.Get( \"#{prop.name.underscore}\" )" -%>
+    <%= prop.api_name -%>Prop, err := expand<%= "Nested" if object.nested_query -%><%= resource_name -%><%= titlelize_property(prop) -%>(<%= schemaPrefix -%>, d, config)
+    if err != nil {
+        return err
+<%      if prop.send_empty_value -%>
+    } else if v, ok := d.GetOkExists("<%= prop.name.underscore -%>"); ok || !reflect.DeepEqual(v, <%= prop.api_name -%>Prop) {
+<%      elsif prop.flatten_object -%>
+    } else if !isEmptyValue(reflect.ValueOf(<%= prop.api_name -%>Prop)) {
+<%      else -%>
+    } else if v, ok := d.GetOkExists("<%= prop.name.underscore -%>"); !isEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, <%= prop.api_name -%>Prop)) {
+<%      end -%>
+        obj["<%= prop.api_name -%>"] = <%= prop.api_name -%>Prop
+    }
+<%  end -%>
+
+<%# We need to decide what encoder to use here - if there's an update encoder, use that! -%>
+<%  if object.custom_code.update_encoder -%>
+    obj, err = resource<%= resource_name -%>UpdateEncoder(d, meta, obj)
+    if err != nil {
+        return err
+    }
+<%  elsif object.custom_code.encoder -%>
+    obj, err = resource<%= resource_name -%>Encoder(d, meta, obj)
+    if err != nil {
+        return err
+    }
+<%  end -%>
+
+<%  if object.mutex -%>
+    lockName, err := replaceVars(d, config, "<%= object.mutex -%>")
+    if err != nil {
+        return err
+    }
+    mutexKV.Lock(lockName)
+    defer mutexKV.Unlock(lockName)
+<%  end -%>
+
+    url, err := replaceVars(d, config, "<%= "{{#{object.__product.name}BasePath}}#{update_uri(object, object.update_url)}" -%>")
+    if err != nil {
+        return err
+    }
+
+    log.Printf("[DEBUG] Updating <%= object.name -%> %q: %#v", d.Id(), obj)
+<%= lines(compile(pwd + '/templates/terraform/update_mask.erb')) if object.update_mask -%>
+<%= lines(compile(pwd + '/' + object.custom_code.pre_update)) if object.custom_code.pre_update -%>
+<%  if object.nested_query&.modify_by_patch -%>
+<%# Keep this after mutex - patch request data relies on current resource state %>
+    obj, err = resource<%= resource_name -%>PatchUpdateEncoder(d, meta, obj)
+    if err != nil {
+        return err
+    }
+<%  end -%>
+<%  if object.supports_indirect_user_project_override -%>
+     if parts := regexp.MustCompile(`projects\/([^\/]+)\/`).FindStringSubmatch(url); parts != nil {
+        billingProject = parts[1]
+    }
+<%  end -%>
+
+    // err == nil indicates that the billing_project value was found
+    if bp, err := getBillingProject(d, config); err == nil {
+      billingProject = bp
+    }
+
+    res, err := sendRequestWithTimeout(config, "<%= object.update_verb -%>", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutUpdate)<%= object.error_retry_predicates ? ", " + object.error_retry_predicates.join(',') : "" -%>)
+
+    if err != nil {
+        return fmt.Errorf("Error updating <%= object.name -%> %q: %s", d.Id(), err)
+    } else {
+	log.Printf("[DEBUG] Finished updating <%= object.name -%> %q: %#v", d.Id(), res)
+    }
+
+<%  if object.async&.allow?('update') -%>
+<%    if object.async.is_a? Api::OpAsync -%>
+    err = <%= client_name_camel -%>OperationWaitTime(
+        config, res, <% if has_project -%> project, <% end -%> "Updating <%= object.name -%>", userAgent,
+        d.Timeout(schema.TimeoutUpdate))
+
+    if err != nil {
+        return err
+    }
+<%    elsif object.async.is_a? Provider::Terraform::PollAsync -%>
+    err = PollingWaitTime(resource<%= resource_name -%>PollRead(d, meta), <%= object.async.check_response_func_existence -%>, "Updating <%= object.name -%>", d.Timeout(schema.TimeoutUpdate), <%= object.async.target_occurrences -%>)
+    if err != nil {
+<%      if object.async.suppress_error-%>
+        log.Printf("[ERROR] Unable to confirm eventually consistent <%= object.name -%> %q finished updating: %q", d.Id(), err)
+<%      else -%>
+        return err
+<%      end -%>
+    }
+<%    end -%>
+<%  end -%>
+<%  end # if !object.input -%>
+<%    if properties_by_custom_update(object.root_properties).length() > 0 -%>
     d.Partial(true)
 
 <% properties_by_custom_update(object.root_properties)
@@ -707,103 +804,8 @@ if <%= props.map { |prop| "d.HasChange(\"#{prop.name.underscore}\")" }.join ' ||
     }
 <%  end -%>
 
-    d.Partial(false)
-<%  else # if object.input -%>
-    obj := make(map[string]interface{})
-<%  update_body_properties.each do |prop| -%>
-    <%# flattened objects won't have something stored in state so instead nil is passed to the next expander. -%>
-    <% schemaPrefix = prop.flatten_object ? "nil" : "d.Get( \"#{prop.name.underscore}\" )" -%>
-    <%= prop.api_name -%>Prop, err := expand<%= "Nested" if object.nested_query -%><%= resource_name -%><%= titlelize_property(prop) -%>(<%= schemaPrefix -%>, d, config)
-    if err != nil {
-        return err
-<%      if prop.send_empty_value -%>
-    } else if v, ok := d.GetOkExists("<%= prop.name.underscore -%>"); ok || !reflect.DeepEqual(v, <%= prop.api_name -%>Prop) {
-<%      elsif prop.flatten_object -%>
-    } else if !isEmptyValue(reflect.ValueOf(<%= prop.api_name -%>Prop)) {
-<%      else -%>
-    } else if v, ok := d.GetOkExists("<%= prop.name.underscore -%>"); !isEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, <%= prop.api_name -%>Prop)) {
-<%      end -%>
-        obj["<%= prop.api_name -%>"] = <%= prop.api_name -%>Prop
-    }
-<%  end -%>
-
-<%# We need to decide what encoder to use here - if there's an update encoder, use that! -%>
-<%  if object.custom_code.update_encoder -%>
-    obj, err = resource<%= resource_name -%>UpdateEncoder(d, meta, obj)
-    if err != nil {
-        return err
-    }
-<%  elsif object.custom_code.encoder -%>
-    obj, err = resource<%= resource_name -%>Encoder(d, meta, obj)
-    if err != nil {
-        return err
-    }
-<%  end -%>
-
-<%  if object.mutex -%>
-    lockName, err := replaceVars(d, config, "<%= object.mutex -%>")
-    if err != nil {
-        return err
-    }
-    mutexKV.Lock(lockName)
-    defer mutexKV.Unlock(lockName)
-<%  end -%>
-
-    url, err := replaceVars(d, config, "<%= "{{#{object.__product.name}BasePath}}#{update_uri(object, object.update_url)}" -%>")
-    if err != nil {
-        return err
-    }
-
-    log.Printf("[DEBUG] Updating <%= object.name -%> %q: %#v", d.Id(), obj)
-<%= lines(compile(pwd + '/templates/terraform/update_mask.erb')) if object.update_mask -%>
-<%= lines(compile(pwd + '/' + object.custom_code.pre_update)) if object.custom_code.pre_update -%>
-<%  if object.nested_query&.modify_by_patch -%>
-<%# Keep this after mutex - patch request data relies on current resource state %>
-    obj, err = resource<%= resource_name -%>PatchUpdateEncoder(d, meta, obj)
-    if err != nil {
-        return err
-    }
-<%  end -%>
-<%  if object.supports_indirect_user_project_override -%>
-     if parts := regexp.MustCompile(`projects\/([^\/]+)\/`).FindStringSubmatch(url); parts != nil {
-        billingProject = parts[1]
-    }
-<%  end -%>
-
-    // err == nil indicates that the billing_project value was found
-    if bp, err := getBillingProject(d, config); err == nil {
-      billingProject = bp
-    }
-
-    res, err := sendRequestWithTimeout(config, "<%= object.update_verb -%>", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutUpdate)<%= object.error_retry_predicates ? ", " + object.error_retry_predicates.join(',') : "" -%>)
-
-    if err != nil {
-        return fmt.Errorf("Error updating <%= object.name -%> %q: %s", d.Id(), err)
-    } else {
-	log.Printf("[DEBUG] Finished updating <%= object.name -%> %q: %#v", d.Id(), res)
-    }
-
-<%  if object.async&.allow?('update') -%>
-<%    if object.async.is_a? Api::OpAsync -%>
-    err = <%= client_name_camel -%>OperationWaitTime(
-        config, res, <% if has_project -%> project, <% end -%> "Updating <%= object.name -%>", userAgent,
-        d.Timeout(schema.TimeoutUpdate))
-
-    if err != nil {
-        return err
-    }
-<%    elsif object.async.is_a? Provider::Terraform::PollAsync -%>
-    err = PollingWaitTime(resource<%= resource_name -%>PollRead(d, meta), <%= object.async.check_response_func_existence -%>, "Updating <%= object.name -%>", d.Timeout(schema.TimeoutUpdate), <%= object.async.target_occurrences -%>)
-    if err != nil {
-<%      if object.async.suppress_error-%>
-        log.Printf("[ERROR] Unable to confirm eventually consistent <%= object.name -%> %q finished updating: %q", d.Id(), err)
-<%      else -%>
-        return err
-<%      end -%>
-    }
-<%    end -%>
-<%  end -%>
-<%  end # if object.input -%>
+  d.Partial(false)
+<% end -%>
 
 <%= lines(compile(pwd + '/' + object.custom_code.post_update)) if object.custom_code.post_update -%>
     return resource<%= resource_name -%>Read(d, meta)