Skip to content

Commit

Permalink
Labeled bucket import (#1946)
Browse files Browse the repository at this point in the history
fixes #1916

This patches the upstream handling of labels to import `labels` and
`pulumiLabels` as well as `effectiveLabels`. Note we handle
`defaultLabels` on the provider by removing them from the imported
`labels`.

This allows for storage buckets to be imported cleanly along with their
labels.

The upstream provider does not import these properties. [The TF GCP
provider
allows](https://www.hashicorp.com/blog/terraform-google-provider-adds-updates-to-default-labels)
for non-managed labels on resources. this PR changes this in our
provider - it will now import ALL labels and assume they are all managed
in pulumi by default. Note that we still allow manually editing the
inputs after importing.

There is no way to fix #1916
without this though - we either have to assume the labels are managed by
pulumi or none are.


Note that I adopted the import machinery from
pulumi/pulumi-aws#3859

I've opened #1959 as a
possible follow-up if we decide to fix this behaviour for other
resources. Implementation might be tricky.

---------

Co-authored-by: Ian Wahbe <[email protected]>
  • Loading branch information
VenelinMartinov and iwahbe authored Apr 26, 2024
1 parent dc171dd commit 527672e
Show file tree
Hide file tree
Showing 15 changed files with 226 additions and 34 deletions.
6 changes: 3 additions & 3 deletions patches/0001-Allow-disabling-the-partner-name.patch
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniel Bradley <[email protected]>
Date: Tue, 7 Mar 2023 11:09:59 +0000
Subject: [PATCH 01/10] Allow disabling the partner name
Subject: [PATCH] Allow disabling the partner name

Add options to set or disable partner name.

diff --git a/google-beta/provider/provider.go b/google-beta/provider/provider.go
index 78672828a..5c45a2e63 100644
index 8f009a843..1b361dceb 100644
--- a/google-beta/provider/provider.go
+++ b/google-beta/provider/provider.go
@@ -123,6 +123,19 @@ func Provider() *schema.Provider {
Expand All @@ -29,7 +29,7 @@ index 78672828a..5c45a2e63 100644
"request_reason": {
Type: schema.TypeString,
Optional: true,
@@ -865,6 +878,21 @@ func ProviderConfigure(ctx context.Context, d *schema.ResourceData, p *schema.Pr
@@ -870,6 +883,21 @@ func ProviderConfigure(ctx context.Context, d *schema.ResourceData, p *schema.Pr
UserAgent: p.UserAgent("terraform-provider-google-beta", version.ProviderVersion),
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniel Bradley <[email protected]>
Date: Tue, 7 Mar 2023 11:50:33 +0000
Subject: [PATCH 02/10] Add nil checks for sql database instance flattening
Subject: [PATCH] Add nil checks for sql database instance flattening


diff --git a/google-beta/services/sql/resource_sql_database_instance.go b/google-beta/services/sql/resource_sql_database_instance.go
index c86d0101c..ee1d7e584 100644
index 29ecf4b7e..1a2b26b21 100644
--- a/google-beta/services/sql/resource_sql_database_instance.go
+++ b/google-beta/services/sql/resource_sql_database_instance.go
@@ -2043,6 +2043,10 @@ func resourceSqlDatabaseInstanceImport(d *schema.ResourceData, meta interface{})
@@ -2054,6 +2054,10 @@ func resourceSqlDatabaseInstanceImport(d *schema.ResourceData, meta interface{})
}

func flattenSettings(settings *sqladmin.Settings, d *schema.ResourceData) []map[string]interface{} {
Expand All @@ -19,7 +19,7 @@ index c86d0101c..ee1d7e584 100644
data := map[string]interface{}{
"version": settings.SettingsVersion,
"tier": settings.Tier,
@@ -2130,6 +2134,10 @@ func flattenDataCacheConfig(d *sqladmin.DataCacheConfig) []map[string]interface{
@@ -2143,6 +2147,10 @@ func flattenDataCacheConfig(d *sqladmin.DataCacheConfig) []map[string]interface{
}

func flattenBackupConfiguration(backupConfiguration *sqladmin.BackupConfiguration) []map[string]interface{} {
Expand All @@ -30,7 +30,7 @@ index c86d0101c..ee1d7e584 100644
data := map[string]interface{}{
"binary_log_enabled": backupConfiguration.BinaryLogEnabled,
"enabled": backupConfiguration.Enabled,
@@ -2222,6 +2230,10 @@ func flattenDatabaseFlags(databaseFlags []*sqladmin.DatabaseFlags) []map[string]
@@ -2235,6 +2243,10 @@ func flattenDatabaseFlags(databaseFlags []*sqladmin.DatabaseFlags) []map[string]
}

func flattenIpConfiguration(ipConfiguration *sqladmin.IpConfiguration, d *schema.ResourceData) interface{} {
Expand All @@ -41,7 +41,7 @@ index c86d0101c..ee1d7e584 100644
data := map[string]interface{}{
"ipv4_enabled": ipConfiguration.Ipv4Enabled,
"private_network": ipConfiguration.PrivateNetwork,
@@ -2272,6 +2284,10 @@ func flattenAuthorizedNetworks(entries []*sqladmin.AclEntry) interface{} {
@@ -2285,6 +2297,10 @@ func flattenAuthorizedNetworks(entries []*sqladmin.AclEntry) interface{} {
}

func flattenLocationPreference(locationPreference *sqladmin.LocationPreference) interface{} {
Expand All @@ -52,7 +52,7 @@ index c86d0101c..ee1d7e584 100644
data := map[string]interface{}{
"follow_gae_application": locationPreference.FollowGaeApplication,
"zone": locationPreference.Zone,
@@ -2282,6 +2298,10 @@ func flattenLocationPreference(locationPreference *sqladmin.LocationPreference)
@@ -2295,6 +2311,10 @@ func flattenLocationPreference(locationPreference *sqladmin.LocationPreference)
}

func flattenMaintenanceWindow(maintenanceWindow *sqladmin.MaintenanceWindow) interface{} {
Expand All @@ -63,7 +63,7 @@ index c86d0101c..ee1d7e584 100644
data := map[string]interface{}{
"day": maintenanceWindow.Day,
"hour": maintenanceWindow.Hour,
@@ -2356,6 +2376,10 @@ func flattenServerCaCerts(caCerts []*sqladmin.SslCert) []map[string]interface{}
@@ -2369,6 +2389,10 @@ func flattenServerCaCerts(caCerts []*sqladmin.SslCert) []map[string]interface{}
}

func flattenInsightsConfig(insightsConfig *sqladmin.InsightsConfig) interface{} {
Expand Down
2 changes: 1 addition & 1 deletion patches/0003-rebase-bigquery_dataset.patch
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Guinevere Saenger <[email protected]>
Date: Wed, 4 Oct 2023 12:49:17 -0700
Subject: [PATCH 03/10] rebase bigquery_dataset
Subject: [PATCH] rebase bigquery_dataset


diff --git a/google-beta/services/bigquery/resource_bigquery_dataset.go b/google-beta/services/bigquery/resource_bigquery_dataset.go
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniel Bradley <[email protected]>
Date: Tue, 7 Mar 2023 11:57:44 +0000
Subject: [PATCH 04/10] Add flattening of self managed certificates
Subject: [PATCH] Add flattening of self managed certificates


diff --git a/google-beta/services/certificatemanager/resource_certificate_manager_certificate.go b/google-beta/services/certificatemanager/resource_certificate_manager_certificate.go
Expand Down
2 changes: 1 addition & 1 deletion patches/0005-website-docs-d-tweaks.patch
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniel Bradley <[email protected]>
Date: Tue, 7 Mar 2023 11:16:06 +0000
Subject: [PATCH 05/10] website/docs/d tweaks
Subject: [PATCH] website/docs/d tweaks


diff --git a/website/docs/d/cloud_run_service.html.markdown b/website/docs/d/cloud_run_service.html.markdown
Expand Down
24 changes: 12 additions & 12 deletions patches/0006-docs-patching.patch
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Venelin <[email protected]>
Date: Fri, 1 Mar 2024 17:59:26 +0000
Subject: [PATCH 06/10] docs patching
Subject: [PATCH] docs patching


diff --git a/website/docs/r/api_gateway_api.html.markdown b/website/docs/r/api_gateway_api.html.markdown
Expand Down Expand Up @@ -1596,7 +1596,7 @@ index 20ce807e7..1cf3cbdbb 100644
<div class = "oics-button" style="float: right; margin: 0 0 -15px">
<a href="https://console.cloud.google.com/cloudshell/open?cloudshell_git_repo=https%3A%2F%2Fgithub.com%2Fterraform-google-modules%2Fdocs-examples.git&cloudshell_working_dir=vpn_tunnel_basic&cloudshell_image=gcr.io%2Fcloudshell-images%2Fcloudshell%3Alatest&open_in_editor=main.tf&cloudshell_print=.%2Fmotd&cloudshell_tutorial=.%2Ftutorial.md" target="_blank">
diff --git a/website/docs/r/container_cluster.html.markdown b/website/docs/r/container_cluster.html.markdown
index 18b533a67..4c78869fe 100644
index 0ad936d66..5d0ca9c6e 100644
--- a/website/docs/r/container_cluster.html.markdown
+++ b/website/docs/r/container_cluster.html.markdown
@@ -13,15 +13,12 @@ To get more information about GKE clusters, see:
Expand Down Expand Up @@ -1687,7 +1687,7 @@ index 18b533a67..4c78869fe 100644
release channel, but will not unenroll it. Instead, use the `"UNSPECIFIED"`
channel. Structure is [documented below](#nested_release_channel).

@@ -847,8 +865,6 @@ gvnic {
@@ -850,8 +868,6 @@ gvnic {

* `guest_accelerator` - (Optional) List of the type and count of accelerator cards attached to the instance.
Structure [documented below](#nested_guest_accelerator).
Expand All @@ -1696,7 +1696,7 @@ index 18b533a67..4c78869fe 100644

* `image_type` - (Optional) The image type to use for this node. Note that changing the image type
will delete and recreate all nodes in the node pool.
@@ -869,7 +885,7 @@ gvnic {
@@ -872,7 +888,7 @@ gvnic {
* `metadata` - (Optional) The metadata key/value pairs assigned to instances in
the cluster. From GKE `1.12` onwards, `disable-legacy-endpoints` is set to
`true` by the API; if `metadata` is set but that default value is not
Expand All @@ -1705,7 +1705,7 @@ index 18b533a67..4c78869fe 100644
value in your config.

* `min_cpu_platform` - (Optional) Minimum CPU platform to be used by this instance.
@@ -894,10 +910,7 @@ gvnic {
@@ -897,10 +913,7 @@ gvnic {
See the [official documentation](https://cloud.google.com/kubernetes-engine/docs/concepts/spot-vms)
for more information. Defaults to false.

Expand All @@ -1717,7 +1717,7 @@ index 18b533a67..4c78869fe 100644

* `service_account` - (Optional) The service account to be used by the Node VMs.
If not specified, the "default" service account is used.
@@ -909,13 +922,14 @@ gvnic {
@@ -912,13 +925,14 @@ gvnic {

* `resource_manager_tags` - (Optional) A map of resource manager tag keys and values to be attached to the nodes for managing Compute Engine firewalls using Network Firewall Policies. Tags must be according to specifications found [here](https://cloud.google.com/vpc/docs/tags-firewalls-overview#specifications). A maximum of 5 tag key-value pairs can be specified. Existing tags will be replaced with new values. Tags must be in one of the following formats ([KEY]=[VALUE]) 1. `tagKeys/{tag_key_id}=tagValues/{tag_value_id}` 2. `{org_id}/{tag_key_name}={tag_value_name}` 3. `{project_id}/{tag_key_name}={tag_value_name}`.

Expand All @@ -1739,7 +1739,7 @@ index 18b533a67..4c78869fe 100644

* `workload_metadata_config` - (Optional) Metadata configuration to expose to workloads on the node pool.
Structure is [documented below](#nested_workload_metadata_config).
@@ -964,12 +978,20 @@ sole_tenant_config {
@@ -967,12 +981,20 @@ sole_tenant_config {

* `threads_per_core` - (Required) The number of threads per physical core. To disable simultaneous multithreading (SMT) set this to 1. If unset, the maximum number of threads supported per core by the underlying processor is assumed.

Expand All @@ -1761,7 +1761,7 @@ index 18b533a67..4c78869fe 100644
<a name="nested_ephemeral_storage_config"></a>The `ephemeral_storage_config` block supports:

* `local_ssd_count` (Required) - Number of local SSDs to use to back ephemeral storage. Uses NVMe interfaces. Each local SSD is 375 GB in size. If zero, it means to disable using local SSDs as ephemeral storage.
@@ -1119,7 +1141,7 @@ for more details. This field only applies to private clusters, when
@@ -1122,7 +1144,7 @@ for more details. This field only applies to private clusters, when
* `private_endpoint_subnetwork` - (Optional) Subnetwork in cluster's network where master's endpoint will be provisioned.

* `master_global_access_config` (Optional) - Controls cluster master global
Expand All @@ -1770,7 +1770,7 @@ index 18b533a67..4c78869fe 100644
not modify the previously-set value. Structure is [documented below](#nested_master_global_access_config).

In addition, the `private_cluster_config` allows access to the following read-only fields:
@@ -1220,9 +1242,9 @@ Enables monitoring and attestation of the boot integrity of the instance. The at
@@ -1223,9 +1245,9 @@ Enables monitoring and attestation of the boot integrity of the instance. The at

* `mode` (Required) How to expose the node metadata to the workload running on the node.
Accepted values are:
Expand Down Expand Up @@ -3573,7 +3573,7 @@ index 81e68fdc2..b73f3c417 100644

<a name="nested_encryption_config"></a>The `encryption_config` block supports:
diff --git a/website/docs/r/sql_database_instance.html.markdown b/website/docs/r/sql_database_instance.html.markdown
index fc057586e..219de4650 100644
index 3bdf7ad72..94e07d02c 100644
--- a/website/docs/r/sql_database_instance.html.markdown
+++ b/website/docs/r/sql_database_instance.html.markdown
@@ -10,12 +10,12 @@ Creates a new Google SQL Database Instance. For more information, see the [offic
Expand Down Expand Up @@ -3633,7 +3633,7 @@ index fc057586e..219de4650 100644
configuration is detailed below.

The `settings` block supports:
@@ -497,7 +495,7 @@ The optional `clone` block supports:
@@ -499,7 +497,7 @@ The optional `clone` block supports:
* `allocated_ip_range` - (Optional) The name of the allocated ip range for the private ip CloudSQL instance. For example: "google-managed-services-default". If set, the cloned instance ip will be created in the allocated range. The range name must comply with [RFC 1035](https://tools.ietf.org/html/rfc1035). Specifically, the name must be 1-63 characters long and match the regular expression [a-z]([-a-z0-9]*[a-z0-9])?.

The optional `restore_backup_context` block supports:
Expand All @@ -3642,7 +3642,7 @@ index fc057586e..219de4650 100644
block during resource creation/update will trigger the restore action after the resource is created/updated.

* `backup_run_id` - (Required) The ID of the backup run to restore from.
@@ -535,21 +533,13 @@ instance.
@@ -537,21 +535,13 @@ instance.

* A `PRIVATE` address is an address for an instance which has been configured to use private networking see: [Private IP](https://cloud.google.com/sql/docs/mysql/private-ip).

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Aaron Friel <[email protected]>
Date: Thu, 22 Jun 2023 09:45:36 -0700
Subject: [PATCH 07/10] Exclude scripts dir from provider go module
Subject: [PATCH] Exclude scripts dir from provider go module

This directory references a private Go module, which we cannot "go vet" or check.

Expand Down
6 changes: 3 additions & 3 deletions patches/0008-Remove-duplicative-resource-token.patch
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Aaron Friel <[email protected]>
Date: Thu, 22 Jun 2023 16:38:12 -0700
Subject: [PATCH 08/10] Remove duplicative resource token
Subject: [PATCH] Remove duplicative resource token


diff --git a/google-beta/provider/provider.go b/google-beta/provider/provider.go
index 5c45a2e63..db90403cc 100644
index 1b361dceb..97ffe2051 100644
--- a/google-beta/provider/provider.go
+++ b/google-beta/provider/provider.go
@@ -838,6 +838,15 @@ func Provider() *schema.Provider {
@@ -843,6 +843,15 @@ func Provider() *schema.Provider {

func DatasourceMap() map[string]*schema.Resource {
datasourceMap, _ := DatasourceMapWithErrors()
Expand Down
6 changes: 3 additions & 3 deletions patches/0009-Fix-794-with-an-unconditional-read.patch
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Aaron Friel <[email protected]>
Date: Thu, 7 Sep 2023 16:28:00 -0700
Subject: [PATCH 09/10] Fix #794 with an unconditional read.
Subject: [PATCH] Fix #794 with an unconditional read.


diff --git a/google-beta/services/sql/resource_sql_database_instance.go b/google-beta/services/sql/resource_sql_database_instance.go
index ee1d7e584..d75ceba31 100644
index 1a2b26b21..3a064ec61 100644
--- a/google-beta/services/sql/resource_sql_database_instance.go
+++ b/google-beta/services/sql/resource_sql_database_instance.go
@@ -1903,10 +1903,11 @@ func resourceSqlDatabaseInstanceUpdate(d *schema.ResourceData, meta interface{})
@@ -1909,10 +1909,11 @@ func resourceSqlDatabaseInstanceUpdate(d *schema.ResourceData, meta interface{})
if err != nil {
return err
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Guinevere Saenger <[email protected]>
Date: Thu, 16 Nov 2023 13:58:59 -0800
Subject: [PATCH 10/10] Remove pattern string from compute forwarding rules
Subject: [PATCH] Remove pattern string from compute forwarding rules


diff --git a/website/docs/r/compute_forwarding_rule.html.markdown b/website/docs/r/compute_forwarding_rule.html.markdown
Expand Down
58 changes: 58 additions & 0 deletions patches/0011-fix-label-imports.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Venelin <[email protected]>
Date: Wed, 24 Apr 2024 12:49:49 +0100
Subject: [PATCH] fix label imports


diff --git a/google-beta/services/storage/resource_storage_bucket.go b/google-beta/services/storage/resource_storage_bucket.go
index 3baddab29..b26de57d9 100644
--- a/google-beta/services/storage/resource_storage_bucket.go
+++ b/google-beta/services/storage/resource_storage_bucket.go
@@ -1778,10 +1778,10 @@ func setStorageBucket(d *schema.ResourceData, config *transport_tpg.Config, res
if err := d.Set("lifecycle_rule", flattenBucketLifecycle(d, res.Lifecycle)); err != nil {
return fmt.Errorf("Error setting lifecycle_rule: %s", err)
}
- if err := tpgresource.SetLabels(res.Labels, d, "labels"); err != nil {
+ if err := tpgresource.SetLabelsNoDefaults(res.Labels, d, "labels", config.DefaultLabels); err != nil {
return fmt.Errorf("Error setting labels: %s", err)
}
- if err := tpgresource.SetLabels(res.Labels, d, "terraform_labels"); err != nil {
+ if err := tpgresource.SetLabelsNoDefaults(res.Labels, d, "terraform_labels", config.DefaultLabels); err != nil {
return fmt.Errorf("Error setting terraform_labels: %s", err)
}
if err := d.Set("effective_labels", res.Labels); err != nil {
diff --git a/google-beta/tpgresource/labels.go b/google-beta/tpgresource/labels.go
index 42bd5647d..d02317610 100644
--- a/google-beta/tpgresource/labels.go
+++ b/google-beta/tpgresource/labels.go
@@ -31,6 +31,30 @@ func SetLabels(labels map[string]string, d *schema.ResourceData, lineage string)
return d.Set(lineage, transformed)
}

+// Like SetLabels but also takes defaultLabels and skips them
+// if we are setting the labels field.
+func SetLabelsNoDefaults(labels map[string]string, d *schema.ResourceData, lineage string, defaultLabels map[string]string) error {
+ transformed := make(map[string]interface{})
+
+ if v, ok := d.GetOk(lineage); ok {
+ // We are reading after an update, so populate just the user defined labels.
+ if labels != nil {
+ for k := range v.(map[string]interface{}) {
+ transformed[k] = labels[k]
+ }
+ }
+ } else {
+ // We are reading for an import, so populate all of the labels, except the skipped ones.
+ for k, v := range labels {
+ if _, ok := defaultLabels[k]; !ok || lineage != "labels" {
+ transformed[k] = v
+ }
+ }
+ }
+
+ return d.Set(lineage, transformed)
+}
+
// Sets the "labels" field and "terraform_labels" with the value of the field "effective_labels" for data sources.
// When reading data source, as the labels field is unavailable in the configuration of the data source,
// the "labels" field will be empty. With this funciton, the labels "field" will have all of labels in the resource.
20 changes: 20 additions & 0 deletions provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,23 @@ func providerServer(t *testing.T) pulumirpc.ResourceProviderServer {
require.NoError(t, err)
return p
}

func pulumiTest(t *testing.T, dir string, opts ...opttest.Option) *pulumitest.PulumiTest {
if testing.Short() {
t.Skipf("Skipping in testing.Short() mode, assuming this is a CI run without GCP creds")
}

cwd, err := os.Getwd()
require.NoError(t, err)

options := []opttest.Option{
opttest.LocalProviderPath(providerName, filepath.Join(cwd, "..", "bin")),
}
options = append(options, opts...)

test := pulumitest.NewPulumiTest(t, dir, options...)

googleProj := getProject()
test.SetConfig("gcp:config:project", googleProj)
return test
}
Loading

0 comments on commit 527672e

Please sign in to comment.