Skip to content

Commit

Permalink
Make terraform_labels field immutable for immutable resources (Google…
Browse files Browse the repository at this point in the history
…CloudPlatform#9394)

* Make terraform_labels field immutable for immutable resources

* Revert change for dataflow job

* Skip updatable resources

* Use empty update for the handwritten immutable resources

* Update upgrade guide

* Remove dataflow tests

* Address comments

* Add forceNew to all subfields
  • Loading branch information
zli82016 authored and jialei-chen committed Nov 29, 2023
1 parent cc5c696 commit 4cd06bd
Show file tree
Hide file tree
Showing 9 changed files with 315 additions and 12 deletions.
10 changes: 6 additions & 4 deletions mmv1/api/resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ def add_labels_fields(props, parent, labels)
@custom_diff.append('tpgresource.SetMetadataLabelsDiff')
end

props << build_terraform_labels_field('labels', labels.field_min_version)
props << build_terraform_labels_field('labels', labels)
props << build_effective_labels_field('labels', labels)
end

Expand Down Expand Up @@ -521,7 +521,7 @@ def build_effective_labels_field(name, labels)
)
end

def build_terraform_labels_field(name, min_version)
def build_terraform_labels_field(name, labels)
description = "The combination of #{name} configured directly on the resource
and default #{name} configured on the provider."

Expand All @@ -530,8 +530,10 @@ def build_terraform_labels_field(name, min_version)
output: true,
api_name: name,
description:,
min_version:,
ignore_write: true
min_version: labels.field_min_version,
ignore_write: true,
update_url: labels.update_url,
immutable: labels.immutable
)
end

Expand Down
1 change: 1 addition & 0 deletions mmv1/provider/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@ def field_specific_update_methods(properties)
def properties_by_custom_update(properties)
update_props = properties.reject do |p|
p.update_url.nil? || p.update_verb.nil? || p.update_verb == :NOOP ||
p.is_a?(Api::Type::KeyValueTerraformLabels) ||
p.is_a?(Api::Type::KeyValueLabels) # effective_labels is used for update
end

Expand Down
6 changes: 4 additions & 2 deletions mmv1/provider/terraform.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,13 @@ def updatable?(resource, properties)
end

def force_new?(property, resource)
(!property.output || property.is_a?(Api::Type::KeyValueEffectiveLabels)) &&
((!property.output || property.is_a?(Api::Type::KeyValueEffectiveLabels)) &&
(property.immutable || (resource.immutable && property.update_url.nil? &&
property.immutable.nil? &&
(property.parent.nil? ||
force_new?(property.parent, resource))))
force_new?(property.parent, resource))))) ||
(property.is_a?(Api::Type::KeyValueTerraformLabels) &&
!updatable?(resource, resource.all_user_properties))
end

# Returns tuples of (fieldName, list of update masks) for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,35 @@ resource "google_cloud_run_domain_mapping" "default" {
}
`, context)
}

func TestAccCloudRunDomainMapping_migration(t *testing.T) {
acctest.SkipIfVcr(t)
t.Parallel()

context := map[string]interface{}{
"namespace": envvar.GetTestProjectFromEnv(),
"random_suffix": acctest.RandString(t, 10),
}

oldVersion := map[string]resource.ExternalProvider{
"google": {
VersionConstraint: "4.84.0", // a version that doesn't separate user defined labels and system labels
Source: "registry.terraform.io/hashicorp/google",
},
}

acctest.VcrTest(t, resource.TestCase{
PreCheck: func() { acctest.AccTestPreCheck(t) },
CheckDestroy: testAccCheckCloudRunDomainMappingDestroyProducer(t),
Steps: []resource.TestStep{
{
Config: testAccCloudRunDomainMapping_cloudRunDomainMappingUpdated2(context),
ExternalProviders: oldVersion,
},
{
Config: testAccCloudRunDomainMapping_cloudRunDomainMappingUpdated2(context),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
},
},
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func ResourceComputeInstanceTemplate() *schema.Resource {
return &schema.Resource{
Create: resourceComputeInstanceTemplateCreate,
Read: resourceComputeInstanceTemplateRead,
Update: resourceComputeInstanceTemplateUpdate,
Delete: resourceComputeInstanceTemplateDelete,
Importer: &schema.ResourceImporter{
State: resourceComputeInstanceTemplateImportState,
Expand Down Expand Up @@ -375,6 +376,7 @@ Google Cloud KMS.`,
"metadata_fingerprint": {
Type: schema.TypeString,
Computed: true,
ForceNew: true,
Description: `The unique fingerprint of the metadata.`,
},
"network_performance_config": {
Expand Down Expand Up @@ -449,6 +451,7 @@ Google Cloud KMS.`,
"name": {
Type: schema.TypeString,
Computed: true,
ForceNew: true,
Description: `The name of the network_interface.`,
},
"nic_type": {
Expand Down Expand Up @@ -483,6 +486,7 @@ Google Cloud KMS.`,
"public_ptr_domain_name": {
Type: schema.TypeString,
Computed: true,
ForceNew: true,
Description: `The DNS domain name for the public PTR record.The DNS domain name for the public PTR record.`,
},
},
Expand Down Expand Up @@ -517,47 +521,55 @@ Google Cloud KMS.`,
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ValidateFunc: validation.StringInSlice([]string{"IPV4_ONLY", "IPV4_IPV6", ""}, false),
Description: `The stack type for this network interface to identify whether the IPv6 feature is enabled or not. If not specified, IPV4_ONLY will be used.`,
},

"ipv6_access_type": {
Type: schema.TypeString,
Computed: true,
ForceNew: true,
Description: `One of EXTERNAL, INTERNAL to indicate whether the IP can be accessed from the Internet. This field is always inherited from its subnetwork.`,
},

"ipv6_access_config": {
Type: schema.TypeList,
Optional: true,
ForceNew: true,
Description: `An array of IPv6 access configurations for this interface. Currently, only one IPv6 access config, DIRECT_IPV6, is supported. If there is no ipv6AccessConfig specified, then this instance will have no external IPv6 Internet access.`,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"network_tier": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Description: `The service-level to be provided for IPv6 traffic when the subnet has an external subnet. Only PREMIUM tier is valid for IPv6`,
},
// Possibly configurable- this was added so we don't break if it's inadvertently set
// (assuming the same ass access config)
"public_ptr_domain_name": {
Type: schema.TypeString,
Computed: true,
ForceNew: true,
Description: `The domain name to be used when creating DNSv6 records for the external IPv6 ranges.`,
},
"external_ipv6": {
Type: schema.TypeString,
Computed: true,
ForceNew: true,
Description: `The first IPv6 address of the external IPv6 range associated with this instance, prefix length is stored in externalIpv6PrefixLength in ipv6AccessConfig. The field is output only, an IPv6 address from a subnetwork associated with the instance will be allocated dynamically.`,
},
"external_ipv6_prefix_length": {
Type: schema.TypeString,
Computed: true,
ForceNew: true,
Description: `The prefix length of the external IPv6 range.`,
},
"name": {
Type: schema.TypeString,
Computed: true,
ForceNew: true,
Description: `The name of this access configuration.`,
},
},
Expand All @@ -567,12 +579,14 @@ Google Cloud KMS.`,
Type: schema.TypeInt,
Optional: true,
Computed: true,
ForceNew: true,
Description: `The prefix length of the primary internal IPv6 range.`,
},
"ipv6_address": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
DiffSuppressFunc: ipv6RepresentationDiffSuppress,
Description: `An IPv6 internal network address for this network interface. If not specified, Google Cloud will automatically assign an internal IPv6 address from the instance's subnetwork.`,
},
Expand Down Expand Up @@ -650,6 +664,7 @@ Google Cloud KMS.`,
"min_node_cpus": {
Type: schema.TypeInt,
Optional: true,
ForceNew: true,
AtLeastOneOf: schedulingInstTemplateKeys,
Description: `Minimum number of cpus for the instance.`,
},
Expand Down Expand Up @@ -687,7 +702,7 @@ Must be from 0 to 315,576,000,000 inclusive.`,
"nanos": {
Type: schema.TypeInt,
Optional: true,
ForceNew: true,
ForceNew: true,
Description: `Span of time that's a fraction of a second at nanosecond
resolution. Durations less than one second are represented
with a 0 seconds field and a positive nanos field. Must
Expand All @@ -706,6 +721,7 @@ be from 0 to 999,999,999 inclusive.`,
"local_ssd_recovery_timeout" : {
Type: schema.TypeList,
Optional: true,
ForceNew: true,
Description: `Specifies the maximum amount of time a Local Ssd Vm should wait while
recovery of the Local Ssd state is attempted. Its value should be in
between 0 and 168 hours with hour granularity and the default value being 1
Expand Down Expand Up @@ -739,12 +755,14 @@ be from 0 to 999,999,999 inclusive.`,
"self_link": {
Type: schema.TypeString,
Computed: true,
ForceNew: true,
Description: `The URI of the created resource.`,
},

"self_link_unique": {
Type: schema.TypeString,
Computed: true,
ForceNew: true,
Description: `A special URI of the created resource that uniquely identifies this instance template.`,
},

Expand Down Expand Up @@ -914,13 +932,13 @@ be from 0 to 999,999,999 inclusive.`,
"tags_fingerprint": {
Type: schema.TypeString,
Computed: true,
ForceNew: true,
Description: `The unique fingerprint of the tags.`,
},

"labels": {
Type: schema.TypeMap,
Optional: true,
ForceNew: true,
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
Description: `A set of key/value label pairs to assign to instances created from this template.
Expand Down Expand Up @@ -1368,6 +1386,11 @@ func resourceComputeInstanceTemplateCreate(d *schema.ResourceData, meta interfac
return resourceComputeInstanceTemplateRead(d, meta)
}

func resourceComputeInstanceTemplateUpdate(d *schema.ResourceData, meta interface{}) error {
// Only the field "labels" and "terraform_labels" is mutable
return resourceComputeInstanceTemplateRead(d, meta)
}

type diskCharacteristics struct {
mode string
diskType string
Expand Down
Loading

0 comments on commit 4cd06bd

Please sign in to comment.