From d976e8440409018d78b2385132279ee4a1850ff2 Mon Sep 17 00:00:00 2001 From: Zhenhua Li Date: Fri, 4 Aug 2023 11:38:28 -0700 Subject: [PATCH 1/5] Modify labels fields --- tpgtools/property.go | 35 ++++++++++++++++++++ tpgtools/resource.go | 51 +++++++++++++++++++++++++++++ tpgtools/templates/resource.go.tmpl | 36 ++++++++++++++++++++ 3 files changed, 122 insertions(+) diff --git a/tpgtools/property.go b/tpgtools/property.go index 0ee26f2af299..46f7553cb746 100644 --- a/tpgtools/property.go +++ b/tpgtools/property.go @@ -311,6 +311,10 @@ func (p Property) DefaultStateSetter() string { case SchemaTypeFloat: fallthrough case SchemaTypeMap: + if p.IsResourceLabels() || p.IsResourceAnnotations() { + return fmt.Sprintf("d.Set(%q, flatten%s%s(res.%s, d))", p.Name(), p.resource.PathType(), p.PackagePath(), p.PackageName) + } + return fmt.Sprintf("d.Set(%q, res.%s)", p.Name(), p.PackageName) case SchemaTypeList, SchemaTypeSet: if p.typ.Items != nil && ((p.typ.Items.Type == "string" && len(p.typ.Items.Enum) == 0) || p.typ.Items.Type == "integer") { @@ -440,6 +444,14 @@ func (p Property) Objects() (props []Property) { return props } +func (p Property) IsResourceLabels() bool { + return p.Name() == "labels" && p.parent == nil +} + +func (p Property) IsResourceAnnotations() bool { + return p.Name() == "annotations" && p.parent == nil +} + // collapsedProperties returns the input list of properties with nested objects collapsed if needed. func collapsedProperties(props []Property) (collapsed []Property) { for _, v := range props { @@ -879,6 +891,12 @@ func createPropertiesFromSchema(schema *openapi.Schema, typeFetcher *TypeFetcher } props = append(props, p) + + // Add the "effective_labels" property when the current property is top level "labels" or + // add the "effective_annotations" property when the current property is top level "annotations" + if p.IsResourceLabels() || p.IsResourceAnnotations() { + props = append(props, build_effective_field(p, resource, parent)) + } } // handle conflict fields @@ -901,3 +919,20 @@ func createPropertiesFromSchema(schema *openapi.Schema, typeFetcher *TypeFetcher return props, nil } + +func build_effective_field(p Property, resource *Resource, parent *Property) Property { + title := fmt.Sprintf("effective_%s", p.title) + description := fmt.Sprintf("All of %s (key/value pairs) present on the resource in GCP, including the %s configured through Terraform, other clients and services.", p.title, p.title) + stateSetter := fmt.Sprintf("d.Set(%q, res.%s)", title, p.PackageName) + + return Property{ + title: title, + Type: p.Type, + Description: description, + resource: resource, + parent: parent, + Optional: true, + Computed: true, + StateSetter: &stateSetter, + } +} diff --git a/tpgtools/resource.go b/tpgtools/resource.go index b65c2061f9f9..eef2f2985f33 100644 --- a/tpgtools/resource.go +++ b/tpgtools/resource.go @@ -381,6 +381,26 @@ func (r Resource) IDFunction() string { return "tpgresource.ReplaceVarsForId" } +// Check if the resource has the lables field for the resource +func (r Resource) HasLabels() bool { + for _, p := range r.Properties { + if p.IsResourceLabels() { + return true + } + } + return false +} + +// Check if the resource has the annotations field for the resource +func (r Resource) HasAnnotations() bool { + for _, p := range r.Properties { + if p.IsResourceAnnotations() { + return true + } + } + return false +} + // ResourceInput is a Resource along with additional generation metadata. type ResourceInput struct { Resource @@ -496,6 +516,13 @@ func createResource(schema *openapi.Schema, info *openapi.Info, typeFetcher *Typ } props, err := createPropertiesFromSchema(schema, typeFetcher, overrides, &res, nil, location) + + if res.TitleCaseFullName() == "DataplexLake" { + glog.Infof("[WARNING] Generating from resource %s %s", res.Name(), res.TitleCaseFullName()) + + glog.Infof("[WARNING] Generating from props %#v", props) + } + if err != nil { return nil, err } @@ -810,6 +837,18 @@ func (r *Resource) loadHandWrittenSamples() []Sample { sample.Name = &sampleName } sample.TestSlug = RenderedString(snakeToTitleCase(miscellaneousNameSnakeCase(sampleName)).titlecase() + "HandWritten") + + // The "labels" and "annotations" fields in the state are decided by the configuration. + // During importing, as the configuration is unavailableafter, the "labels" and "annotations" fields in the state will be empty. + // So add the "labels" and the "annotations" fields to the ImportStateVerifyIgnore list. + if r.HasLabels() { + sample.IgnoreRead = append(sample.IgnoreRead, "labels") + } + + if r.HasAnnotations() { + sample.IgnoreRead = append(sample.IgnoreRead, "annotations") + } + samples = append(samples, sample) } @@ -896,6 +935,18 @@ func (r *Resource) loadDCLSamples() []Sample { } sample.DependencyList = dependencies sample.TestSlug = RenderedString(sampleNameToTitleCase(*sample.Name).titlecase()) + + // The "labels" and "annotations" fields in the state are decided by the configuration. + // During importing, as the configuration is unavailableafter, the "labels" and "annotations" fields in the state will be empty. + // So add the "labels" and the "annotations" fields to the ImportStateVerifyIgnore list. + if r.HasLabels() { + sample.IgnoreRead = append(sample.IgnoreRead, "labels") + } + + if r.HasAnnotations() { + sample.IgnoreRead = append(sample.IgnoreRead, "annotations") + } + samples = append(samples, sample) } diff --git a/tpgtools/templates/resource.go.tmpl b/tpgtools/templates/resource.go.tmpl index 532dd4c4c75b..66c1b5f5f348 100644 --- a/tpgtools/templates/resource.go.tmpl +++ b/tpgtools/templates/resource.go.tmpl @@ -714,6 +714,42 @@ func flatten{{$.PathType}}{{$v.PackagePath}}(obj *{{$.Package}}.{{$v.ObjectType} } {{ end -}} +{{ if $.HasLabels }} +func flatten{{$.PathType}}Labels(v map[string]string, d *schema.ResourceData) interface{} { + if v == nil { + return nil + } + + transformed := make(map[string]interface{}) + if l, ok := d.Get("labels").(map[string]interface{}); ok { + for k, _ := range l { + transformed[k] = l[k] + } + } + + log.Printf("[DEBUG] flattenDataplexLakeLabels %#v", transformed) + return transformed +} +{{ end }} + +{{ if $.HasAnnotations }} +func flatten{{$.PathType}}Annotations(v map[string]string, d *schema.ResourceData) interface{} { + if v == nil { + return nil + } + + transformed := make(map[string]interface{}) + if l, ok := d.Get("annotations").(map[string]interface{}); ok { + for k, _ := range l { + transformed[k] = l[k] + } + } + + log.Printf("[DEBUG] flattenDataplexLakeAnnotations %#v", transformed) + return transformed +} +{{ end }} + {{ range $v := .EnumArrays -}} func flatten{{$.PathType}}{{$v.PackagePath}}Array(obj []{{$.Package}}.{{$v.ObjectType}}Enum) interface{} { if obj == nil { From a8b9c3291cfe84682e84d6c3fcd02cc54caaeea7 Mon Sep 17 00:00:00 2001 From: Zhenhua Li Date: Mon, 7 Aug 2023 10:05:50 -0700 Subject: [PATCH 2/5] effective_labels filed is not optional --- .../tests/resource_cloudbuild_worker_pool_test.go.erb | 6 ++++++ tpgtools/property.go | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/tests/resource_cloudbuild_worker_pool_test.go.erb b/mmv1/third_party/terraform/tests/resource_cloudbuild_worker_pool_test.go.erb index 8dc25191686e..acfef3e1978b 100644 --- a/mmv1/third_party/terraform/tests/resource_cloudbuild_worker_pool_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_cloudbuild_worker_pool_test.go.erb @@ -41,6 +41,7 @@ func TestAccCloudbuildWorkerPool_basic(t *testing.T) { { ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"annotations"}, ResourceName: "google_cloudbuild_worker_pool.pool", }, { @@ -79,6 +80,11 @@ resource "google_cloudbuild_worker_pool" "pool" { machine_type = "e2-standard-4" no_external_ip = false } + + annotations = { + env = "foo" + default_expiration_ms = 3600000 + } } `, context) } diff --git a/tpgtools/property.go b/tpgtools/property.go index 46f7553cb746..318715c3a48e 100644 --- a/tpgtools/property.go +++ b/tpgtools/property.go @@ -931,7 +931,7 @@ func build_effective_field(p Property, resource *Resource, parent *Property) Pro Description: description, resource: resource, parent: parent, - Optional: true, + Optional: false, Computed: true, StateSetter: &stateSetter, } From 25da0f7ede33aa15bff70873add47362c7251bd2 Mon Sep 17 00:00:00 2001 From: Zhenhua Li Date: Mon, 7 Aug 2023 13:41:11 -0700 Subject: [PATCH 3/5] Add tests for labels --- .../resource_dataproc_workflow_template_test.go.erb | 9 +++++++++ .../overrides/dataplex/samples/asset/basic_asset.tf.tmpl | 6 ++++++ tpgtools/property.go | 4 ++-- tpgtools/resource.go | 7 ------- tpgtools/templates/resource.go.tmpl | 2 -- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/mmv1/third_party/terraform/tests/resource_dataproc_workflow_template_test.go.erb b/mmv1/third_party/terraform/tests/resource_dataproc_workflow_template_test.go.erb index a277266a01a4..495e7d8e8f73 100644 --- a/mmv1/third_party/terraform/tests/resource_dataproc_workflow_template_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_dataproc_workflow_template_test.go.erb @@ -37,6 +37,10 @@ func TestAccDataprocWorkflowTemplate_basic(t *testing.T) { { ImportState: true, ImportStateVerify: true, + // The "labels" field in the state are decided by the configuration. + // During importing, as the configuration is unavailableafter, the "labels" field in the state will be empty. + // So add the "labels" to the ImportStateVerifyIgnore list. + ImportStateVerifyIgnore: []string{"labels"}, ResourceName: "google_dataproc_workflow_template.template", }, }, @@ -124,6 +128,11 @@ resource "google_dataproc_workflow_template" "template" { query_file_uri = "someuri" } } + + labels = { + env = "foo" + somekey = "somevalue" + } } `, context) } diff --git a/tpgtools/overrides/dataplex/samples/asset/basic_asset.tf.tmpl b/tpgtools/overrides/dataplex/samples/asset/basic_asset.tf.tmpl index 07c9141bb937..d3f18d1990dd 100644 --- a/tpgtools/overrides/dataplex/samples/asset/basic_asset.tf.tmpl +++ b/tpgtools/overrides/dataplex/samples/asset/basic_asset.tf.tmpl @@ -52,6 +52,12 @@ resource "google_dataplex_asset" "primary" { name = "projects/{{project}}/buckets/{{bucket}}" type = "STORAGE_BUCKET" } + + labels = { + env = "foo" + my-asset = "exists" + } + project = "{{project}}" depends_on = [ diff --git a/tpgtools/property.go b/tpgtools/property.go index 318715c3a48e..ae202738bc17 100644 --- a/tpgtools/property.go +++ b/tpgtools/property.go @@ -895,7 +895,7 @@ func createPropertiesFromSchema(schema *openapi.Schema, typeFetcher *TypeFetcher // Add the "effective_labels" property when the current property is top level "labels" or // add the "effective_annotations" property when the current property is top level "annotations" if p.IsResourceLabels() || p.IsResourceAnnotations() { - props = append(props, build_effective_field(p, resource, parent)) + props = append(props, build_effective_labels_field(p, resource, parent)) } } @@ -920,7 +920,7 @@ func createPropertiesFromSchema(schema *openapi.Schema, typeFetcher *TypeFetcher return props, nil } -func build_effective_field(p Property, resource *Resource, parent *Property) Property { +func build_effective_labels_field(p Property, resource *Resource, parent *Property) Property { title := fmt.Sprintf("effective_%s", p.title) description := fmt.Sprintf("All of %s (key/value pairs) present on the resource in GCP, including the %s configured through Terraform, other clients and services.", p.title, p.title) stateSetter := fmt.Sprintf("d.Set(%q, res.%s)", title, p.PackageName) diff --git a/tpgtools/resource.go b/tpgtools/resource.go index eef2f2985f33..1e6f6e7a1498 100644 --- a/tpgtools/resource.go +++ b/tpgtools/resource.go @@ -516,13 +516,6 @@ func createResource(schema *openapi.Schema, info *openapi.Info, typeFetcher *Typ } props, err := createPropertiesFromSchema(schema, typeFetcher, overrides, &res, nil, location) - - if res.TitleCaseFullName() == "DataplexLake" { - glog.Infof("[WARNING] Generating from resource %s %s", res.Name(), res.TitleCaseFullName()) - - glog.Infof("[WARNING] Generating from props %#v", props) - } - if err != nil { return nil, err } diff --git a/tpgtools/templates/resource.go.tmpl b/tpgtools/templates/resource.go.tmpl index 66c1b5f5f348..a3f10f2db10b 100644 --- a/tpgtools/templates/resource.go.tmpl +++ b/tpgtools/templates/resource.go.tmpl @@ -727,7 +727,6 @@ func flatten{{$.PathType}}Labels(v map[string]string, d *schema.ResourceData) in } } - log.Printf("[DEBUG] flattenDataplexLakeLabels %#v", transformed) return transformed } {{ end }} @@ -745,7 +744,6 @@ func flatten{{$.PathType}}Annotations(v map[string]string, d *schema.ResourceDat } } - log.Printf("[DEBUG] flattenDataplexLakeAnnotations %#v", transformed) return transformed } {{ end }} From f680c5cdc3fc17bb46a272de21bfb4d340b72a77 Mon Sep 17 00:00:00 2001 From: Zhenhua Li Date: Mon, 7 Aug 2023 14:34:18 -0700 Subject: [PATCH 4/5] Append notes for lables description --- tpgtools/property.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tpgtools/property.go b/tpgtools/property.go index ae202738bc17..a08b16d0087f 100644 --- a/tpgtools/property.go +++ b/tpgtools/property.go @@ -890,13 +890,17 @@ func createPropertiesFromSchema(schema *openapi.Schema, typeFetcher *TypeFetcher resource.ReusedTypes = resource.RegisterReusedType(p) } - props = append(props, p) - // Add the "effective_labels" property when the current property is top level "labels" or // add the "effective_annotations" property when the current property is top level "annotations" + if p.IsResourceLabels() || p.IsResourceAnnotations() { + note := `**Note**: This field is non-authoritative, and will only manage the labels present in your configuration. + Please refer to the field 'effective_labels' for all of the labels present on the resource.` + p.Description = fmt.Sprintf("%s\n\n%s", p.Description, note) props = append(props, build_effective_labels_field(p, resource, parent)) } + + props = append(props, p) } // handle conflict fields From 0916ff21fe798890bea526475657b5a8b4e32c78 Mon Sep 17 00:00:00 2001 From: Zhenhua Li Date: Mon, 7 Aug 2023 15:16:56 -0700 Subject: [PATCH 5/5] Fix the notes --- tpgtools/property.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tpgtools/property.go b/tpgtools/property.go index a08b16d0087f..0292c3124857 100644 --- a/tpgtools/property.go +++ b/tpgtools/property.go @@ -894,8 +894,8 @@ func createPropertiesFromSchema(schema *openapi.Schema, typeFetcher *TypeFetcher // add the "effective_annotations" property when the current property is top level "annotations" if p.IsResourceLabels() || p.IsResourceAnnotations() { - note := `**Note**: This field is non-authoritative, and will only manage the labels present in your configuration. - Please refer to the field 'effective_labels' for all of the labels present on the resource.` + note := "**Note**: This field is non-authoritative, and will only manage the labels present in your configuration. " + + "Please refer to the field `effective_labels` for all of the labels present on the resource." p.Description = fmt.Sprintf("%s\n\n%s", p.Description, note) props = append(props, build_effective_labels_field(p, resource, parent)) }