Skip to content

Fix interconnect immutability issue causing macsec failure #8203

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/11700.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note: enhancement
compute: added in-place update in `google_compute_interconnect` resource except for `remote_location ` and `requested_features` fields
```
107 changes: 96 additions & 11 deletions google-beta/services/compute/resource_compute_interconnect.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ lowercase letter, or digit, except the last character, which cannot be a dash.`,
"admin_enabled": {
Type: schema.TypeBool,
Optional: true,
ForceNew: true,
Description: `Administrative status of the interconnect. When this is set to true, the Interconnect is
functional and can carry traffic. When set to false, no packets can be carried over the
interconnect and no BGP routes are exchanged over it. By default, the status is set to true.`,
Expand All @@ -123,7 +122,6 @@ interconnect and no BGP routes are exchanged over it. By default, the status is
"description": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Description: `An optional description of this resource. Provide this property when you create the resource.`,
},
"labels": {
Expand All @@ -140,7 +138,6 @@ Please refer to the field 'effective_labels' for all of the labels present on th
"macsec": {
Type: schema.TypeList,
Optional: true,
ForceNew: true,
Description: `Configuration that enables Media Access Control security (MACsec) on the Cloud
Interconnect connection between Google and your on-premises router.`,
MaxItems: 1,
Expand All @@ -149,7 +146,6 @@ Interconnect connection between Google and your on-premises router.`,
"pre_shared_keys": {
Type: schema.TypeList,
Required: true,
ForceNew: true,
Description: `A keychain placeholder describing a set of named key objects along with their
start times. A MACsec CKN/CAK is generated for each key in the key chain.
Google router automatically picks the key with the most recent startTime when establishing
Expand All @@ -159,7 +155,6 @@ or re-establishing a MACsec secure link.`,
"name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: verify.ValidateRegexp(`^[a-z]([-a-z0-9]*[a-z0-9])?$`),
Description: `A name for this pre-shared key. The name must be 1-63 characters long, and
comply with RFC1035. Specifically, the name must be 1-63 characters long and match
Expand All @@ -170,7 +165,6 @@ or re-establishing a MACsec secure link.`,
"fail_open": {
Type: schema.TypeBool,
Optional: true,
ForceNew: true,
Description: `If set to true, the Interconnect connection is configured with a should-secure
MACsec security policy, that allows the Google router to fallback to cleartext
traffic if the MKA session cannot be established. By default, the Interconnect
Expand All @@ -180,7 +174,6 @@ if the MKA session cannot be established with your router.`,
"start_time": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Description: `A RFC3339 timestamp on or after which the key is valid. startTime can be in the
future. If the keychain has a single key, startTime can be omitted. If the keychain
has multiple keys, startTime is mandatory for each key. The start times of keys must
Expand All @@ -196,14 +189,12 @@ hours apart.`,
"macsec_enabled": {
Type: schema.TypeBool,
Optional: true,
ForceNew: true,
Description: `Enable or disable MACsec on this Interconnect connection.
MACsec enablement fails if the MACsec object is not specified.`,
},
"noc_contact_email": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Description: `Email address to contact the customer NOC for operations and maintenance notifications
regarding this Interconnect. If specified, this will be used for notifications in addition to
all other forms described, such as Cloud Monitoring logs alerting and Cloud Notifications.
Expand Down Expand Up @@ -275,7 +266,6 @@ Google to the customer in the LOA.`,
"effective_labels": {
Type: schema.TypeMap,
Computed: true,
ForceNew: true,
Description: `All of labels (key/value pairs) present on the resource in GCP, including the labels configured through Terraform, other clients and services.`,
Elem: &schema.Schema{Type: schema.TypeString},
},
Expand Down Expand Up @@ -721,7 +711,102 @@ func resourceComputeInterconnectRead(d *schema.ResourceData, meta interface{}) e
}

func resourceComputeInterconnectUpdate(d *schema.ResourceData, meta interface{}) error {
// Only the root field "labels" and "terraform_labels" are mutable
config := meta.(*transport_tpg.Config)
userAgent, err := tpgresource.GenerateUserAgentString(d, config.UserAgent)
if err != nil {
return err
}

billingProject := ""

project, err := tpgresource.GetProject(d, config)
if err != nil {
return fmt.Errorf("Error fetching project for Interconnect: %s", err)
}
billingProject = project

obj := make(map[string]interface{})
descriptionProp, err := expandComputeInterconnectDescription(d.Get("description"), d, config)
if err != nil {
return err
} else if v, ok := d.GetOkExists("description"); !tpgresource.IsEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, descriptionProp)) {
obj["description"] = descriptionProp
}
adminEnabledProp, err := expandComputeInterconnectAdminEnabled(d.Get("admin_enabled"), d, config)
if err != nil {
return err
} else if v, ok := d.GetOkExists("admin_enabled"); ok || !reflect.DeepEqual(v, adminEnabledProp) {
obj["adminEnabled"] = adminEnabledProp
}
nocContactEmailProp, err := expandComputeInterconnectNocContactEmail(d.Get("noc_contact_email"), d, config)
if err != nil {
return err
} else if v, ok := d.GetOkExists("noc_contact_email"); !tpgresource.IsEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, nocContactEmailProp)) {
obj["nocContactEmail"] = nocContactEmailProp
}
labelFingerprintProp, err := expandComputeInterconnectLabelFingerprint(d.Get("label_fingerprint"), d, config)
if err != nil {
return err
} else if v, ok := d.GetOkExists("label_fingerprint"); !tpgresource.IsEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, labelFingerprintProp)) {
obj["labelFingerprint"] = labelFingerprintProp
}
macsecProp, err := expandComputeInterconnectMacsec(d.Get("macsec"), d, config)
if err != nil {
return err
} else if v, ok := d.GetOkExists("macsec"); !tpgresource.IsEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, macsecProp)) {
obj["macsec"] = macsecProp
}
macsecEnabledProp, err := expandComputeInterconnectMacsecEnabled(d.Get("macsec_enabled"), d, config)
if err != nil {
return err
} else if v, ok := d.GetOkExists("macsec_enabled"); !tpgresource.IsEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, macsecEnabledProp)) {
obj["macsecEnabled"] = macsecEnabledProp
}
labelsProp, err := expandComputeInterconnectEffectiveLabels(d.Get("effective_labels"), d, config)
if err != nil {
return err
} else if v, ok := d.GetOkExists("effective_labels"); !tpgresource.IsEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, labelsProp)) {
obj["labels"] = labelsProp
}

url, err := tpgresource.ReplaceVars(d, config, "{{ComputeBasePath}}projects/{{project}}/global/interconnects/{{name}}")
if err != nil {
return err
}

log.Printf("[DEBUG] Updating Interconnect %q: %#v", d.Id(), obj)
headers := make(http.Header)

// err == nil indicates that the billing_project value was found
if bp, err := tpgresource.GetBillingProject(d, config); err == nil {
billingProject = bp
}

res, err := transport_tpg.SendRequest(transport_tpg.SendRequestOptions{
Config: config,
Method: "PATCH",
Project: billingProject,
RawURL: url,
UserAgent: userAgent,
Body: obj,
Timeout: d.Timeout(schema.TimeoutUpdate),
Headers: headers,
})

if err != nil {
return fmt.Errorf("Error updating Interconnect %q: %s", d.Id(), err)
} else {
log.Printf("[DEBUG] Finished updating Interconnect %q: %#v", d.Id(), res)
}

err = ComputeOperationWaitTime(
config, res, project, "Updating Interconnect", userAgent,
d.Timeout(schema.TimeoutUpdate))

if err != nil {
return err
}

return resourceComputeInterconnectRead(d, meta)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0
package compute_test

import (
"testing"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"

"github.com/hashicorp/terraform-provider-google-beta/google-beta/acctest"
)

func TestAccComputeInterconnect_computeInterconnectMacsecTest(t *testing.T) {
t.Parallel()

context := map[string]interface{}{
"random_suffix": acctest.RandString(t, 10),
}

acctest.VcrTest(t, resource.TestCase{
PreCheck: func() { acctest.AccTestPreCheck(t) },
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
CheckDestroy: testAccCheckComputeInterconnectDestroyProducer(t),
Steps: []resource.TestStep{
{
Config: testAccComputeInterconnect_computeInterconnectCreate(context),
},
{
ResourceName: "google_compute_interconnect.example-interconnect",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"labels", "location", "terraform_labels"},
},
{
Config: testAccComputeInterconnect_computeInterconnectEnableMacsec(context),
},
{
ResourceName: "google_compute_interconnect.example-interconnect",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"labels", "location", "terraform_labels"},
},
},
})
}

func testAccComputeInterconnect_computeInterconnectCreate(context map[string]interface{}) string {
return acctest.Nprintf(`
data "google_project" "project" {}

resource "google_compute_interconnect" "example-interconnect" {
name = "tf-test-example-interconnect%{random_suffix}"
customer_name = "internal_customer" # Special customer only available for Google testing.
interconnect_type = "DEDICATED"
link_type = "LINK_TYPE_ETHERNET_100G_LR"
location = "https://www.googleapis.com/compute/v1/projects/${data.google_project.project.name}/global/interconnectLocations/z2z-us-east4-zone1-lciadl-a" # Special location only available for Google testing.
requested_link_count = 1
admin_enabled = true
description = "example description"
macsec_enabled = false
noc_contact_email = "[email protected]"
requested_features = ["IF_MACSEC"]
}
`, context)
}

func testAccComputeInterconnect_computeInterconnectEnableMacsec(context map[string]interface{}) string {
return acctest.Nprintf(`
data "google_project" "project" {}

resource "google_compute_interconnect" "example-interconnect" {
name = "tf-test-example-interconnect%{random_suffix}"
customer_name = "internal_customer" # Special customer only available for Google testing.
interconnect_type = "DEDICATED"
link_type = "LINK_TYPE_ETHERNET_100G_LR"
location = "https://www.googleapis.com/compute/v1/projects/${data.google_project.project.name}/global/interconnectLocations/z2z-us-east4-zone1-lciadl-a" # Special location only available for Google testing.
requested_link_count = 1
admin_enabled = true
description = "example description"
macsec_enabled = true
noc_contact_email = "[email protected]"
requested_features = ["IF_MACSEC"]
macsec {
pre_shared_keys {
name = "test-key"
start_time = "2023-07-01T21:00:01.000Z"
}
}
}
`, context)
}