feat: support for nested write-only arguments + write-only arguments for `google_monitoring_notification_channel` #15538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open

ramonvermeulen wants to merge 18 commits into GoogleCloudPlatform:main from ramonvermeulen:f/24327-write-only-pointer-constraint-groups

+285 −64

mmv1/api/type.go

-Original file line number
+Diff line change
@@ Expand Up / @@ -854,6 +854,29 @@ func (t Type) WriteOnlyProperties() []*Type { @@
     	return props
     }
+    // AllUniqueNestedProperties Returns all unique nested properties (regular and write-only), preserving order and sorted by name.
+    func (t Type) AllUniqueNestedProperties() []*Type {
+    	seen := make(map[string]bool)
+    	var result []*Type
+    	for _, p := range t.NestedProperties() {
+    		key := strings.Join(p.Lineage(), "|")
+    		if !seen[key] {
+    			result = append(result, p)
+    			seen[key] = true
+    		}
+    	}
+    	for _, p := range t.WriteOnlyProperties() {
+    		key := strings.Join(p.Lineage(), "|")
+    		if !seen[key] {
+    			result = append(result, p)
+    			seen[key] = true
+    		}
+    	}
+    	return result
+    }
     func (t Type) Removed() bool {
     	return t.RemovedMessage != ""
     }
@@ Expand Down @@

mmv1/products/monitoring/NotificationChannel.yaml

-Original file line number
+Diff line change
@@ Expand Up / @@ -94,6 +94,7 @@ properties: @@
           the sensitive_labels block, but cannot be configured in both places.
       - name: 'sensitiveLabels'
         type: NestedObject
+        ignore_read: true
         description: |
           Different notification type behaviors are configured primarily using the the `labels` field on this
           resource. This block contains the labels which contain secrets or passwords so that they can be marked
@@ Expand All / @@ -102,13 +103,14 @@ properties: @@
           Credentials may not be specified in both locations and will cause an error. Changing from one location
           to a different credential configuration in the config will require an apply to update state.
-        url_param_only: true
         properties:
           - name: 'authToken'
             type: String
+            api_name: 'auth_token'
             description: |
               An authorization token for a notification channel. Channel types that support this field include: slack
             sensitive: true
+            write_only: true
             exactly_one_of:
               - 'sensitive_labels.0.auth_token'
               - 'sensitive_labels.0.password'
@@ Expand All / @@ -118,15 +120,18 @@ properties: @@
             description: |
               An password for a notification channel. Channel types that support this field include: webhook_basicauth
             sensitive: true
+            write_only: true
             exactly_one_of:
               - 'sensitive_labels.0.auth_token'
               - 'sensitive_labels.0.password'
               - 'sensitive_labels.0.service_key'
           - name: 'serviceKey'
             type: String
+            api_name: 'service_key'
             description: |
               An servicekey token for a notification channel. Channel types that support this field include: pagerduty
             sensitive: true
+            write_only: true
             exactly_one_of:
               - 'sensitive_labels.0.auth_token'
               - 'sensitive_labels.0.password'
@@ Expand Down @@

mmv1/templates/terraform/constants/monitoring_notification_channel.go.tmpl

-Original file line number
+Diff line change
@@ -1,13 +1,16 @@
     var sensitiveLabels = []string{"auth_token", "service_key", "password"}
+    var writeOnlySensitiveLabels = []string{"auth_token_wo_version", "service_key_wo_version", "password_wo_version"}
     func sensitiveLabelCustomizeDiff(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
-    	for _, sl := range sensitiveLabels {
-    		mapLabel := diff.Get("labels." + sl).(string)
-    		authLabel := diff.Get("sensitive_labels.0." + sl).(string)
-    		if mapLabel != "" && authLabel != "" {
-    			return fmt.Errorf("Sensitive label [%s] cannot be set in both `labels` and the `sensitive_labels` block.", sl)
-    		}
-    	}
-    	return nil
-    }
+    	  for _, sl := range sensitiveLabels {
+            l := strings.TrimSuffix(sl, "_wo")
+            l = strings.TrimSuffix(l, "_wo_version")
+            val := diff.Get("labels." + l).(string)
+            sensitiveVal := diff.Get("sensitive_labels.0." + sl).(string)
+            if val != "" && sensitiveVal != "" {
+                return fmt.Errorf("Sensitive label %q cannot be set at the same time as label %q.", sl, l)
+            }
+        }
+        return nil
+    }

mmv1/templates/terraform/decoders/monitoring_notification_channel.go.tmpl

-Original file line number
+Diff line change
@@ Expand Up / @@ -10,17 +10,40 @@ @@
     	See the License for the specific language governing permissions and
     	limitations under the License.
     */ -}}
-    if labelmap, ok := res["labels"]; ok {
-    	labels := labelmap.(map[string]interface{})
-    	for _, sl := range sensitiveLabels {
-    		if _, apiOk := labels[sl]; apiOk {
-    			if _, exists := d.GetOkExists("sensitive_labels.0." + sl); exists {
-    				delete(labels, sl)
-    			} else {
-    				labels[sl] = d.Get("labels." + sl)
+    labels, ok := res["labels"].(map[string]interface{})
+    if !ok {
+    	return res, nil
+    }
+    sensitiveLabelsRaw := d.Get("sensitive_labels")
+    if sensitiveLabelsRaw != nil {
+    	if sensitiveLabelsArr, ok := sensitiveLabelsRaw.([]interface{}); ok && len(sensitiveLabelsArr) > 0 {
+    		if configuredSensitiveLabels, ok := sensitiveLabelsArr[0].(map[string]interface{}); ok && len(configuredSensitiveLabels) > 0 {
+    			sensitiveLabels := make(map[string]interface{})
+    			for configKey, configVal := range configuredSensitiveLabels {
+    				apiField := strings.TrimSuffix(configKey, "_wo")
+    				delete(labels, apiField)
+    				sensitiveLabels[configKey] = configVal
+    			}
+    			if len(sensitiveLabels) > 0 {
+    				res["sensitiveLabels"] = sensitiveLabels
     			}
+    			return res, nil
     		}
     	}
     }
-    return res, nil
+    // Replace obfuscated API values with configured values to prevent a diff
+    // Scenario where end-user uses "sensitive" labels directly in "labels" block
+    if configuredLabels, ok := d.Get("labels").(map[string]interface{}); ok {
+    	for _, field := range []string{"auth_token", "password", "service_key"} {
+    		if configVal, exists := configuredLabels[field]; exists && configVal != nil && configVal != "" {
+    			labels[field] = configVal
+    		}
+    	}
+    }
+    return res, nil

mmv1/templates/terraform/encoders/monitoring_notification_channel.go.tmpl

-Original file line number
+Diff line change
@@ Expand Up / @@ -20,10 +20,22 @@ labels = labelmap.(map[string]string) @@
     for _, sl := range sensitiveLabels {
     	if auth, _ := d.GetOkExists("sensitive_labels.0." + sl); auth != "" {
     		labels[sl] = auth.(string)
     	}
     }
+    for _, slwov := range writeOnlySensitiveLabels {
+      if v, _ := d.GetOkExists("sensitive_labels.0." + slwov); v != "" {
+        target := strings.TrimSuffix(slwov, "_version")
+        woProp := tpgresource.GetRawConfigAttributeAsString(d, "sensitive_labels.0." + target)
+        if woProp != "" {
+          labels[strings.TrimSuffix(slwov, "_wo_version")] = woProp
+        }
+      }
+    }
     obj["labels"] = labels
+    delete(obj, "sensitiveLabels")
-    return obj, nil
+    return obj, nil

mmv1/templates/terraform/expand_property_method.go.tmpl

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -26,9 +26,9 @@ func expand{{$.GetPrefix}}{{$.TitlelizeProperty}}(v interface{}, d tpgresource.T
  
        original := raw.(map[string]interface{})

        transformed := make(map[string]interface{})

          {{- range $prop := $.NestedProperties }}

            {{- if not (eq $prop.Name $prop.KeyName) }}

            {{- if not (or (eq $prop.Name $prop.KeyName) $prop.ClientSide) }}

        transformed{{$prop.TitlelizeProperty}}, err := expand{{$.GetPrefix}}{{$.TitlelizeProperty}}{{$prop.TitlelizeProperty}}(original["{{ underscore $prop.Name }}"], d, config)

        transformed{{$prop.TitlelizeProperty}}, err := expand{{$.GetPrefix}}{{$.TitlelizeProperty}}{{$prop.TitlelizeProperty}}({{if $prop.WriteOnly }}tpgresource.GetRawConfigAttributeAsString(d.(*schema.ResourceData), "{{ join $prop.Lineage ".0." }}"){{ else }}original["{{ underscore $prop.Name }}"]{{ end }}, d, config)

        if err != nil {

          return nil, err

              {{- if or ($prop.SendEmptyValue) (and $prop.TGCSendEmptyValue $.ResourceMetadata.IsTgcCompiler) }}

    @@ -65,8 +65,8 @@ func expand{{$.GetPrefix}}{{$.TitlelizeProperty}}(v interface{}, d tpgresource.T
  
    func expand{{$.GetPrefix}}{{$.TitlelizeProperty}}(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {

      transformed := make(map[string]interface{})

          {{- range $prop := $.NestedProperties }}

            {{- if not (and (hasPrefix $prop.Type "KeyValue") $prop.IgnoreWrite) }}

      transformed{{$prop.TitlelizeProperty}}, err := expand{{$.GetPrefix}}{{$.TitlelizeProperty}}{{$prop.TitlelizeProperty}}({{ if $prop.FlattenObject }}nil{{ else }}d.Get("{{ underscore $prop.Name }}"), d, config)

            {{- if not (or (and (hasPrefix $prop.Type "KeyValue") $prop.IgnoreWrite) $prop.ClientSide) }}

      transformed{{$prop.TitlelizeProperty}}, err := expand{{$.GetPrefix}}{{$.TitlelizeProperty}}{{$prop.TitlelizeProperty}}({{ if $prop.FlattenObject }}nil{{ else if $prop.WriteOnly }}tpgresource.GetRawConfigAttributeAsString(d.(*schema.ResourceData), "{{ join $prop.Lineage ".0." }}"){{ else }}d.Get("{{ underscore $prop.Name }}"), d, config)

      if err != nil {

        return nil, err

              {{- if or ($prop.SendEmptyValue) (and $prop.TGCSendEmptyValue $.ResourceMetadata.IsTgcCompiler) }}

    @@ -121,8 +121,8 @@ func expand{{$.GetPrefix}}{{$.TitlelizeProperty}}(v interface{}, d tpgresource.T
  
            {{- end }}{{/* if $.IsA "Array */}}

        transformed := make(map[string]interface{})

            {{ range $prop := $.NestedProperties }}

              {{- if not (and (hasPrefix $prop.Type "KeyValue") $prop.IgnoreWrite) }}

          transformed{{$prop.TitlelizeProperty}}, err := expand{{$.GetPrefix}}{{$.TitlelizeProperty}}{{$prop.TitlelizeProperty}}(original["{{ underscore $prop.Name }}"], d, config)

              {{- if not (or (and (hasPrefix $prop.Type "KeyValue") $prop.IgnoreWrite) $prop.ClientSide) }}

          transformed{{$prop.TitlelizeProperty}}, err := expand{{$.GetPrefix}}{{$.TitlelizeProperty}}{{$prop.TitlelizeProperty}}({{if $prop.WriteOnly }}tpgresource.GetRawConfigAttributeAsString(d.(*schema.ResourceData), "{{ join $prop.Lineage ".0." }}"){{ else }}original["{{ underscore $prop.Name }}"]{{ end }}, d, config)

          if err != nil {

            return nil, err

                {{- if or ($prop.SendEmptyValue) (and $prop.TGCSendEmptyValue $.ResourceMetadata.IsTgcCompiler) }}

mmv1/templates/terraform/flatten_property_method.go.tmpl

-Original file line number
+Diff line change
@@ Expand Up / @@ -13,7 +13,7 @@ @@
       See the License for the specific language governing permissions and
       limitations under the License. */ -}}
     {{- define "flattenPropertyMethod" }}
-    {{- if or $.WriteOnlyLegacy $.WriteOnly }}
+    {{- if and (or $.WriteOnlyLegacy $.WriteOnly) }}
     {{- else if and $.CustomFlatten (not $.ShouldIgnoreCustomFlatten) }}
         {{- customTemplate $ $.CustomFlatten false -}}
     {{- else -}}
@@ Expand Down @@

mmv1/templates/terraform/nested_property_documentation.html.markdown.tmpl

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -1,34 +1,32 @@
  
    {{ "" }}

    {{- if and $.FlattenObject (not $.WriteOnlyProperties) }}

      {{- range $np := $.NestedProperties }}

    {{- $nested := or $.NestedProperties $.WriteOnlyProperties }}

    {{- if and $.FlattenObject $nested }}

      {{- range $np := $.AllUniqueNestedProperties }}

    {{- trimTemplate "nested_property_documentation.html.markdown.tmpl" $np -}}

      {{- end -}}

    {{- else if and $.NestedProperties (not $.WriteOnlyProperties) }}

    {{- else if $nested }}

    <a name="nested_{{ join $.Lineage "_" }}"></a>The `{{ underscore $.Name }}` block {{ if $.Output }}contains{{ else }}supports{{ end }}:

    {{ "" }}

      {{- if $.IsA "Map" }}

    * `{{ underscore $.KeyName }}` - (Required) The identifier for this object. Format specified above.

    {{ "" }}

      {{- end -}}

      {{- if $.NestedProperties }}

        {{- range $np := $.NestedProperties }}

      {{- range $np := $.AllUniqueNestedProperties -}}

    {{- trimTemplate "property_documentation.html.markdown.tmpl" $np -}}

        {{- end -}}

      {{- end -}}

    {{ "" }}

        {{- $innerNested := false }}

        {{- range $np := $.NestedProperties }}

        {{- range $np := $.AllUniqueNestedProperties }}

          {{- if $np.NestedProperties }}

            {{- $innerNested = true }}

          {{- end }}

        {{- end }}

        {{- if $innerNested}}

    {{ "" }}

        {{- end }}

        {{- range $np := $.NestedProperties }}

        {{- range $np := $.AllUniqueNestedProperties }}

          {{- if $np.NestedProperties }}

    {{- trimTemplate "nested_property_documentation.html.markdown.tmpl" $np -}}

          {{- end -}}

        {{- end -}}

      {{- end -}}

    {{- end -}}

    {{ "" }}

mmv1/templates/terraform/nested_property_write_only_documentation.html.markdown.tmpl

This file was deleted.

mmv1/third_party/terraform/acctest/resource_inventory_test.go

-Original file line number
+Diff line change
@@ Expand Up / @@ -245,6 +245,10 @@ func TestValidateResourceMetadata(t *testing.T) { @@
     		"google_os_config_v2_policy_orchestrator_for_folder.orchestration_state.previous_iteration_state.error.details.type_url":       true,
     		"google_os_config_v2_policy_orchestrator_for_organization.orchestration_state.current_iteration_state.error.details.type_url":  true,
     		"google_os_config_v2_policy_orchestrator_for_organization.orchestration_state.previous_iteration_state.error.details.type_url": true,
+    		"google_monitoring_notification_channel.sensitive_labels.auth_token":                                                           true,
+    		"google_monitoring_notification_channel.sensitive_labels.auth_token_wo":                                                        true,
+    		"google_monitoring_notification_channel.sensitive_labels.service_key":                                                          true,
+    		"google_monitoring_notification_channel.sensitive_labels.service_key_wo":                                                       true,
     	}
     	// Validate yaml files
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support for nested write-only arguments + write-only arguments for `google_monitoring_notification_channel` #15538

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

feat: support for nested write-only arguments + write-only arguments for google_monitoring_notification_channel #15538

Are you sure you want to change the base?

feat: support for nested write-only arguments + write-only arguments for google_monitoring_notification_channel #15538

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

feat: support for nested write-only arguments + write-only arguments for `google_monitoring_notification_channel` #15538

feat: support for nested write-only arguments + write-only arguments for `google_monitoring_notification_channel` #15538