From 8540fe27ea9373016763c39f8cec11c9e86e2747 Mon Sep 17 00:00:00 2001 From: Sam Levenick Date: Wed, 6 Nov 2019 16:34:29 -0800 Subject: [PATCH] Update subnetwork to remove enable_flow_logs and depend on log_config instead (#2597) --- products/compute/api.yaml | 12 ----------- products/compute/terraform.yaml | 5 ++--- templates/terraform/constants/subnetwork.erb | 8 -------- .../subnetwork_log_config.go.erb | 19 +++++++++--------- .../extra_schema_entry/subnetwork.erb | 20 +++++++++++++++++++ .../tests/resource_compute_subnetwork_test.go | 9 ++------- 6 files changed, 33 insertions(+), 40 deletions(-) create mode 100644 templates/terraform/extra_schema_entry/subnetwork.erb diff --git a/products/compute/api.yaml b/products/compute/api.yaml index 597e3543a49c..42832938df98 100644 --- a/products/compute/api.yaml +++ b/products/compute/api.yaml @@ -9093,18 +9093,6 @@ objects: Only networks that are in the distributed mode can have subnetworks. input: true required: true - - !ruby/object:Api::Type::Boolean - name: 'enableFlowLogs' - description: | - Whether to enable flow logging for this subnetwork. - update_verb: :PATCH - update_url: projects/{{project}}/regions/{{region}}/subnetworks/{{name}} - update_id: 'enableFlowLogs' - fingerprint_name: 'fingerprint' - send_empty_value: true - deprecation_message: >- - This field is being removed in favor of log_config. If log_config is present, - flow logs are enabled. # TODO(rileykarson): Work with rambleraptor to remove this field from downstreams. - !ruby/object:Api::Type::Fingerprint name: 'fingerprint' diff --git a/products/compute/terraform.yaml b/products/compute/terraform.yaml index 89fc6e719f99..075cfb6fcbff 100644 --- a/products/compute/terraform.yaml +++ b/products/compute/terraform.yaml @@ -1680,10 +1680,8 @@ overrides: !ruby/object:Overrides::ResourceOverrides function: 'validateIpCidrRange' fingerprint: !ruby/object:Overrides::Terraform::PropertyOverride exclude: false - enableFlowLogs: !ruby/object:Overrides::Terraform::PropertyOverride - default_from_api: true logConfig: !ruby/object:Overrides::Terraform::PropertyOverride - default_from_api: true + send_empty_value: true custom_expand: 'templates/terraform/custom_expand/subnetwork_log_config.go.erb' custom_flatten: 'templates/terraform/custom_flatten/subnetwork_log_config.go.erb' ipCidrRange: !ruby/object:Overrides::Terraform::PropertyOverride @@ -1699,6 +1697,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides custom_code: !ruby/object:Provider::Terraform::CustomCode constants: templates/terraform/constants/subnetwork.erb resource_definition: templates/terraform/resource_definition/subnetwork.erb + extra_schema_entry: templates/terraform/extra_schema_entry/subnetwork.erb examples: - !ruby/object:Provider::Terraform::Examples name: "subnetwork_basic" diff --git a/templates/terraform/constants/subnetwork.erb b/templates/terraform/constants/subnetwork.erb index a54a182cae65..e698124da16c 100644 --- a/templates/terraform/constants/subnetwork.erb +++ b/templates/terraform/constants/subnetwork.erb @@ -17,11 +17,3 @@ func isShrinkageIpCidr(old, new, _ interface{}) bool { return true } - - -func splitSubnetID(id string) (region string, name string) { - parts := strings.Split(id, "/") - region = parts[0] - name = parts[1] - return -} diff --git a/templates/terraform/custom_expand/subnetwork_log_config.go.erb b/templates/terraform/custom_expand/subnetwork_log_config.go.erb index e1b4e3a63bc9..6a1ca64370b5 100644 --- a/templates/terraform/custom_expand/subnetwork_log_config.go.erb +++ b/templates/terraform/custom_expand/subnetwork_log_config.go.erb @@ -14,21 +14,20 @@ -%> func expand<%= prefix -%><%= titlelize_property(property) -%>(v interface{}, d TerraformResourceData, config *Config) (interface{}, error) { l := v.([]interface{}) + transformed := make(map[string]interface{}) if len(l) == 0 || l[0] == nil { - return nil, nil + // send enable = false to ensure logging is disabled if there is no config + transformed["enable"] = false + return transformed, nil } raw := l[0] original := raw.(map[string]interface{}) - v, ok := d.GetOkExists("enable_flow_logs") - - transformed := make(map[string]interface{}) - if !ok || v.(bool) { - transformed["enable"] = true - transformed["aggregationInterval"] = original["aggregation_interval"] - transformed["flowSampling"] = original["flow_sampling"] - transformed["metadata"] = original["metadata"] - } + // The log_config block is specified, so logging should be enabled + transformed["enable"] = true + transformed["aggregationInterval"] = original["aggregation_interval"] + transformed["flowSampling"] = original["flow_sampling"] + transformed["metadata"] = original["metadata"] return transformed, nil } diff --git a/templates/terraform/extra_schema_entry/subnetwork.erb b/templates/terraform/extra_schema_entry/subnetwork.erb new file mode 100644 index 000000000000..87ad70ed4de9 --- /dev/null +++ b/templates/terraform/extra_schema_entry/subnetwork.erb @@ -0,0 +1,20 @@ +<%# The license inside this block applies to this file. + # Copyright 2019 Google Inc. + # Licensed under the Apache License, Version 2.0 (the "License"); + # you may not use this file except in compliance with the License. + # You may obtain a copy of the License at + # + # http://www.apache.org/licenses/LICENSE-2.0 + # + # Unless required by applicable law or agreed to in writing, software + # distributed under the License is distributed on an "AS IS" BASIS, + # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + # See the License for the specific language governing permissions and + # limitations under the License. +-%> +"enable_flow_logs": { + Type: schema.TypeBool, + Computed: true, + Optional: true, + Removed: "This field is being removed in favor of log_config. If log_config is present, flow logs are enabled. Please remove this field", +}, \ No newline at end of file diff --git a/third_party/terraform/tests/resource_compute_subnetwork_test.go b/third_party/terraform/tests/resource_compute_subnetwork_test.go index 95db19864044..bf86bacec48a 100644 --- a/third_party/terraform/tests/resource_compute_subnetwork_test.go +++ b/third_party/terraform/tests/resource_compute_subnetwork_test.go @@ -318,8 +318,8 @@ func testAccCheckComputeSubnetworkExists(n string, subnetwork *compute.Subnetwor } config := testAccProvider.Meta().(*Config) - - region, subnet_name := splitSubnetID(rs.Primary.ID) + region := rs.Primary.Attributes["region"] + subnet_name := rs.Primary.Attributes["name"] found, err := config.clientCompute.Subnetworks.Get( config.Project, region, subnet_name).Do() @@ -549,7 +549,6 @@ resource "google_compute_subnetwork" "network-with-flow-logs" { ip_cidr_range = "10.0.0.0/16" region = "us-central1" network = "${google_compute_network.custom-test.self_link}" - enable_flow_logs = true log_config { aggregation_interval = "INTERVAL_5_SEC" flow_sampling = 0.5 @@ -571,7 +570,6 @@ resource "google_compute_subnetwork" "network-with-flow-logs" { ip_cidr_range = "10.0.0.0/16" region = "us-central1" network = "${google_compute_network.custom-test.self_link}" - enable_flow_logs = true log_config { aggregation_interval = "INTERVAL_30_SEC" flow_sampling = 0.8 @@ -593,7 +591,6 @@ resource "google_compute_subnetwork" "network-with-flow-logs" { ip_cidr_range = "10.0.0.0/16" region = "us-central1" network = "${google_compute_network.custom-test.self_link}" - enable_flow_logs = false } `, cnName, subnetworkName) } @@ -610,7 +607,6 @@ resource "google_compute_subnetwork" "network-with-flow-logs" { ip_cidr_range = "10.0.0.0/16" region = "us-central1" network = "${google_compute_network.custom-test.self_link}" - enable_flow_logs = true log_config { aggregation_interval = "INTERVAL_30_SEC" flow_sampling = 0.6 @@ -653,7 +649,6 @@ resource "google_compute_subnetwork" "network-with-flow-logs" { ip_cidr_range = "10.0.0.0/16" region = "us-central1" network = "${google_compute_network.custom-test.self_link}" - enable_flow_logs = true log_config { aggregation_interval = "INTERVAL_30_SEC" flow_sampling = 0.8