From 1d4e76f29e03b952011029c80a680923e04152f3 Mon Sep 17 00:00:00 2001 From: Riley Karson Date: Tue, 7 Apr 2020 13:03:02 -0700 Subject: [PATCH] Add support for multiple formats for pubsub refs in source repo (#3339) * Add support for multiple formats for pubsub refs in source repo (a TypeMap's key_value). * Add docs * Return string * Handle err, assert type * Assert more types * Use empty strings, not nils * Update overrides/terraform/property_override.rb * Pull utils function to source_repo_utils, make regex constant * fmt --- overrides/terraform/property_override.rb | 11 ++++++++++ products/sourcerepo/api.yaml | 2 +- products/sourcerepo/terraform.yaml | 7 ++++++ .../terraform/expand_property_method.erb | 9 +++++--- templates/terraform/schema_property.erb | 3 +++ third_party/terraform/utils/pubsub_utils.go | 4 +++- .../terraform/utils/source_repo_utils.go | 22 +++++++++++++++++++ third_party/terraform/utils/utils.go.erb | 4 ++++ 8 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 third_party/terraform/utils/source_repo_utils.go diff --git a/overrides/terraform/property_override.rb b/overrides/terraform/property_override.rb index 21c365638086..8b7567efba06 100644 --- a/overrides/terraform/property_override.rb +++ b/overrides/terraform/property_override.rb @@ -78,6 +78,13 @@ def self.attributes # Names of fields that should be included in the updateMask. :update_mask_fields, + # For a TypeMap, the expander function to call on the key. + # Defaults to expandString. + :key_expander, + + # For a TypeMap, the DSF to apply to the key. + :key_diff_suppress_func, + # ==================== # Schema Modifications # ==================== @@ -146,6 +153,10 @@ def validate check :unordered_list, type: :boolean, default: false check :schema_config_mode_attr, type: :boolean, default: false + # technically set as a default everywhere, but only maps will use this. + check :key_expander, type: String, default: 'expandString' + check :key_diff_suppress_func, type: String + check :diff_suppress_func, type: String check :state_func, type: String check :validation, type: Provider::Terraform::Validation diff --git a/products/sourcerepo/api.yaml b/products/sourcerepo/api.yaml index e4d9cf53f02a..af4dd6c3e664 100644 --- a/products/sourcerepo/api.yaml +++ b/products/sourcerepo/api.yaml @@ -85,4 +85,4 @@ objects: Email address of the service account used for publishing Cloud Pub/Sub messages. This service account needs to be in the same project as the PubsubConfig. When added, the caller needs to have iam.serviceAccounts.actAs permission on this service account. - If unspecified, it defaults to the compute engine default service account. \ No newline at end of file + If unspecified, it defaults to the compute engine default service account. diff --git a/products/sourcerepo/terraform.yaml b/products/sourcerepo/terraform.yaml index d7bf5832f449..688c25f64cb7 100644 --- a/products/sourcerepo/terraform.yaml +++ b/products/sourcerepo/terraform.yaml @@ -42,6 +42,13 @@ overrides: !ruby/object:Overrides::ResourceOverrides description: | Resource name of the repository, of the form `{{repo}}`. The repo name may contain slashes. eg, `name/with/slash` + pubsubConfigs: !ruby/object:Overrides::Terraform::PropertyOverride + key_diff_suppress_func: 'compareSelfLinkOrResourceName' + key_expander: 'expandSourceRepoRepositoryPubsubConfigsTopic' + key_description: | + A Cloud Pub/Sub topic in this repo's project. Values are of the form + `projects//topics/` or `` (where the topic will + be inferred). pubsubConfigs.serviceAccountEmail: !ruby/object:Overrides::Terraform::PropertyOverride default_from_api: true custom_code: !ruby/object:Provider::Terraform::CustomCode diff --git a/templates/terraform/expand_property_method.erb b/templates/terraform/expand_property_method.erb index 63cbee02bd7c..f1169861172d 100644 --- a/templates/terraform/expand_property_method.erb +++ b/templates/terraform/expand_property_method.erb @@ -40,7 +40,11 @@ func expand<%= prefix -%><%= titlelize_property(property) -%>(v interface{}, d T <% end -%> - m[original["<%= property.key_name -%>"].(string)] = transformed + transformed<%= property.key_name.camelize(:upper) -%>, err := <%= property.key_expander -%>(original["<%= Google::StringUtils.underscore(property.key_name) -%>"], d, config) + if err != nil { + return nil, err + } + m[transformed<%= property.key_name.camelize(:upper) -%>] = transformed } return m, nil } @@ -176,8 +180,7 @@ func expand<%= prefix -%><%= titlelize_property(property) -%>(v interface{}, d T <% property.nested_properties.each do |prop| -%> <%# Map is a map from {key -> object} in the API, but Terraform can't represent that so we treat the key as a property of the object in Terraform schema. %> -<% next if property.is_a?(Api::Type::Map) && prop.name == property.key_name -%> -<%= lines(build_expand_method(prefix + titlelize_property(property), prop, object), 1) -%> +<%= lines(build_expand_method(prefix + titlelize_property(property), prop, object), 1) -%> <% end -%> <% end -%> diff --git a/templates/terraform/schema_property.erb b/templates/terraform/schema_property.erb index 3179692754f4..689b910b2b0d 100644 --- a/templates/terraform/schema_property.erb +++ b/templates/terraform/schema_property.erb @@ -122,6 +122,9 @@ "<%= property.key_name -%>": { Type: schema.TypeString, Required: true, + <% if !property.diff_suppress_func.nil? -%> + DiffSuppressFunc: <%= property.key_diff_suppress_func %>, + <% end -%> <% if force_new?(property, object) -%> ForceNew: true, <% end -%> diff --git a/third_party/terraform/utils/pubsub_utils.go b/third_party/terraform/utils/pubsub_utils.go index 592fc57bc309..64d1137e4f21 100644 --- a/third_party/terraform/utils/pubsub_utils.go +++ b/third_party/terraform/utils/pubsub_utils.go @@ -5,6 +5,8 @@ import ( "regexp" ) +const PubsubTopicRegex = "projects\\/.*\\/topics\\/.*" + func getComputedSubscriptionName(project, subscription string) string { match, _ := regexp.MatchString("projects\\/.*\\/subscriptions\\/.*", subscription) if match { @@ -14,7 +16,7 @@ func getComputedSubscriptionName(project, subscription string) string { } func getComputedTopicName(project, topic string) string { - match, _ := regexp.MatchString("projects\\/.*\\/topics\\/.*", topic) + match, _ := regexp.MatchString(PubsubTopicRegex, topic) if match { return topic } diff --git a/third_party/terraform/utils/source_repo_utils.go b/third_party/terraform/utils/source_repo_utils.go new file mode 100644 index 000000000000..48229bb24715 --- /dev/null +++ b/third_party/terraform/utils/source_repo_utils.go @@ -0,0 +1,22 @@ +package google + +import "regexp" + +func expandSourceRepoRepositoryPubsubConfigsTopic(v interface{}, d TerraformResourceData, config *Config) (string, error) { + // short-circuit if the topic is a full uri so we don't need to getProject + ok, err := regexp.MatchString(PubsubTopicRegex, v.(string)) + if err != nil { + return "", err + } + + if ok { + return v.(string), nil + } + + project, err := getProject(d, config) + if err != nil { + return "", err + } + + return getComputedTopicName(project, v.(string)), err +} diff --git a/third_party/terraform/utils/utils.go.erb b/third_party/terraform/utils/utils.go.erb index 296df513878e..64a8358eaa68 100644 --- a/third_party/terraform/utils/utils.go.erb +++ b/third_party/terraform/utils/utils.go.erb @@ -423,3 +423,7 @@ func stringInSlice(arr []string, str string) bool { func migrateStateNoop(v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) { return is, nil } + +func expandString(v interface{}, d TerraformResourceData, config *Config) (string, error) { + return v.(string), nil +}