From 159048571e0a43ac0a331dc02da0e9358e011c82 Mon Sep 17 00:00:00 2001 From: Cameron Thornton Date: Fri, 27 Sep 2024 14:58:18 -0500 Subject: [PATCH 1/5] Convert from Ruby to Go fix unit test expecting hardcoded values fix ordering scratch disks in the state Remove hardcoded values that are handled by the API --- .../compute/resource_compute_instance_template.go.tmpl | 7 ------- ...ompute_region_instance_template_internal_test.go.tmpl | 9 +++++---- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance_template.go.tmpl b/mmv1/third_party/terraform/services/compute/resource_compute_instance_template.go.tmpl index 9363286c6b0b..56b7e06d1866 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance_template.go.tmpl +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance_template.go.tmpl @@ -1207,9 +1207,6 @@ func buildDisks(d *schema.ResourceData, config *transport_tpg.Config) ([]*comput // Build the disk var disk compute.AttachedDisk - disk.Type = "PERSISTENT" - disk.Mode = "READ_WRITE" - disk.Interface = "SCSI" disk.Boot = i == 0 disk.AutoDelete = d.Get(prefix + ".auto_delete").(bool) @@ -1620,10 +1617,6 @@ func reorderDisks(configDisks []interface{}, apiDisks []map[string]interface{}) disksByDeviceName[v.(string)] = i } else if v := disk["type"]; v.(string) == "SCRATCH" { iface := disk["interface"].(string) - if iface == "" { - // apply-time default - iface = "SCSI" - } scratchDisksByInterface[iface] = append(scratchDisksByInterface[iface], i) } else if v := disk["source"]; v.(string) != "" { attachedDisksBySource[v.(string)] = i diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_template_internal_test.go.tmpl b/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_template_internal_test.go.tmpl index a643e4719836..bb235b036588 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_template_internal_test.go.tmpl +++ b/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_template_internal_test.go.tmpl @@ -22,8 +22,9 @@ func TestComputeRegionInstanceTemplate_reorderDisks(t *testing.T) { cDeviceName := map[string]interface{}{ "device_name": "disk-1", } - cScratch := map[string]interface{}{ + cScratchScsi := map[string]interface{}{ "type": "SCRATCH", + "interface": "SCSI", } cSource := map[string]interface{}{ "source": "disk-source", @@ -81,7 +82,7 @@ func TestComputeRegionInstanceTemplate_reorderDisks(t *testing.T) { aBoot, aScratchNvme, aSource, aScratchScsi, aFallThrough, aDeviceName, }, ConfigDisks: []interface{}{ - cBoot, cFallThrough, cDeviceName, cScratch, cSource, cScratchNvme, + cBoot, cFallThrough, cDeviceName, cScratchScsi, cSource, cScratchNvme, }, ExpectedResult: []map[string]interface{}{ aBoot, aFallThrough, aDeviceName, aScratchScsi, aSource, aScratchNvme, @@ -92,7 +93,7 @@ func TestComputeRegionInstanceTemplate_reorderDisks(t *testing.T) { aBoot, aNoMatch, aScratchNvme, aScratchScsi, aFallThrough, aDeviceName, }, ConfigDisks: []interface{}{ - cBoot, cFallThrough, cDeviceName, cScratch, cSource, cScratchNvme, + cBoot, cFallThrough, cDeviceName, cScratchScsi, cSource, cScratchNvme, }, ExpectedResult: []map[string]interface{}{ aBoot, aFallThrough, aDeviceName, aScratchScsi, aScratchNvme, aNoMatch, @@ -103,7 +104,7 @@ func TestComputeRegionInstanceTemplate_reorderDisks(t *testing.T) { aBoot, aScratchNvme, aFallThrough, aSource, aScratchScsi, aFallThrough2, aDeviceName, }, ConfigDisks: []interface{}{ - cBoot, cFallThrough, cDeviceName, cScratch, cFallThrough, cSource, cScratchNvme, + cBoot, cFallThrough, cDeviceName, cScratchScsi, cFallThrough, cSource, cScratchNvme, }, ExpectedResult: []map[string]interface{}{ aBoot, aFallThrough, aDeviceName, aScratchScsi, aFallThrough2, aSource, aScratchNvme, From d986fec29649c43333e6fcfe626e4ec6045a7b0b Mon Sep 17 00:00:00 2001 From: Karol Gorczyk Date: Tue, 1 Oct 2024 09:30:27 +0200 Subject: [PATCH 2/5] add RFC1035 name validation to instance --- .../services/compute/resource_compute_instance.go.tmpl | 2 ++ mmv1/third_party/terraform/verify/validation.go | 10 +++++----- mmv1/third_party/terraform/verify/validation_test.go | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.tmpl b/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.tmpl index 299363f66c60..5801da70e7dd 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.tmpl +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.tmpl @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/terraform-provider-google/google/tpgresource" transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport" + "github.com/hashicorp/terraform-provider-google/google/verify" {{ if eq $.TargetVersionName `ga` }} "google.golang.org/api/compute/v1" @@ -396,6 +397,7 @@ func ResourceComputeInstance() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, + ValidateFunc: verify.ValidateRFC1035Name(1, 63), Description: `The name of the instance. One of name or self_link must be provided.`, }, diff --git a/mmv1/third_party/terraform/verify/validation.go b/mmv1/third_party/terraform/verify/validation.go index 2e9da45a5cf2..92ce82acf1a1 100644 --- a/mmv1/third_party/terraform/verify/validation.go +++ b/mmv1/third_party/terraform/verify/validation.go @@ -22,7 +22,7 @@ const ( SubnetworkLinkRegex = "projects/(" + ProjectRegex + ")/regions/(" + RegionRegex + ")/subnetworks/(" + SubnetworkRegex + ")$" - RFC1035NameTemplate = "[a-z](?:[-a-z0-9]{%d,%d}[a-z0-9])" + RFC1035NameTemplate = "[a-z]([-a-z0-9]{%v,%v}[a-z0-9])?" CloudIoTIdRegex = "^[a-zA-Z][-a-zA-Z0-9._+~%]{2,254}$" // Format of default Compute service accounts created by Google @@ -194,10 +194,10 @@ func ValidateRFC3339Time(v interface{}, k string) (warnings []string, errors []e } func ValidateRFC1035Name(min, max int) schema.SchemaValidateFunc { - if min < 2 || max < min { + if min < 1 || max < min { return func(i interface{}, k string) (s []string, errors []error) { - if min < 2 { - errors = append(errors, fmt.Errorf("min must be at least 2. Got: %d", min)) + if min < 1 { + errors = append(errors, fmt.Errorf("min must be at least 1. Got: %d", min)) } if max < min { errors = append(errors, fmt.Errorf("max must greater than min. Got [%d, %d]", min, max)) @@ -206,7 +206,7 @@ func ValidateRFC1035Name(min, max int) schema.SchemaValidateFunc { } } - return ValidateRegexp(fmt.Sprintf("^"+RFC1035NameTemplate+"$", min-2, max-2)) + return ValidateRegexp(fmt.Sprintf("^"+RFC1035NameTemplate+"$", min-1, max-2)) } func ValidateIpCidrRange(v interface{}, k string) (warnings []string, errors []error) { diff --git a/mmv1/third_party/terraform/verify/validation_test.go b/mmv1/third_party/terraform/verify/validation_test.go index b03fac489df7..c4a35627ab4e 100644 --- a/mmv1/third_party/terraform/verify/validation_test.go +++ b/mmv1/third_party/terraform/verify/validation_test.go @@ -174,7 +174,7 @@ func TestValidateRFC1035Name(t *testing.T) { {TestName: "cannot end with a dash", Min: 6, Max: 10, Value: "invalid-", ExpectError: true}, {TestName: "too short", Min: 6, Max: 10, Value: "short", ExpectError: true}, {TestName: "too long", Min: 6, Max: 10, Value: "toolooooong", ExpectError: true}, - {TestName: "min too small", Min: 1, Max: 10, Value: "", ExpectError: true}, + {TestName: "min too small", Min: 0, Max: 10, Value: "", ExpectError: true}, {TestName: "min < max", Min: 6, Max: 5, Value: "", ExpectError: true}, } From db610943dded942d020b40dad16e701e23f6ed76 Mon Sep 17 00:00:00 2001 From: Karol Gorczyk Date: Wed, 2 Oct 2024 09:24:10 +0200 Subject: [PATCH 3/5] small fix --- mmv1/third_party/terraform/verify/validation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/verify/validation.go b/mmv1/third_party/terraform/verify/validation.go index 92ce82acf1a1..5747d5bac5fa 100644 --- a/mmv1/third_party/terraform/verify/validation.go +++ b/mmv1/third_party/terraform/verify/validation.go @@ -22,7 +22,7 @@ const ( SubnetworkLinkRegex = "projects/(" + ProjectRegex + ")/regions/(" + RegionRegex + ")/subnetworks/(" + SubnetworkRegex + ")$" - RFC1035NameTemplate = "[a-z]([-a-z0-9]{%v,%v}[a-z0-9])?" + RFC1035NameTemplate = "[a-z]([-a-z0-9]{%d,%d}[a-z0-9])?" CloudIoTIdRegex = "^[a-zA-Z][-a-zA-Z0-9._+~%]{2,254}$" // Format of default Compute service accounts created by Google From 823b3e7d31739619c5641a269750f0c8762a8d02 Mon Sep 17 00:00:00 2001 From: Karol Gorczyk Date: Fri, 11 Oct 2024 10:51:08 +0200 Subject: [PATCH 4/5] add new constraints on length. and test cases --- .../terraform/verify/validation.go | 35 +++++++++++-------- .../terraform/verify/validation_test.go | 6 ++++ 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/mmv1/third_party/terraform/verify/validation.go b/mmv1/third_party/terraform/verify/validation.go index 5747d5bac5fa..b3a0152e7436 100644 --- a/mmv1/third_party/terraform/verify/validation.go +++ b/mmv1/third_party/terraform/verify/validation.go @@ -22,7 +22,7 @@ const ( SubnetworkLinkRegex = "projects/(" + ProjectRegex + ")/regions/(" + RegionRegex + ")/subnetworks/(" + SubnetworkRegex + ")$" - RFC1035NameTemplate = "[a-z]([-a-z0-9]{%d,%d}[a-z0-9])?" + RFC1035NameTemplate = "[a-z]([-a-z0-9]%v[a-z0-9])?" CloudIoTIdRegex = "^[a-zA-Z][-a-zA-Z0-9._+~%]{2,254}$" // Format of default Compute service accounts created by Google @@ -41,7 +41,7 @@ var ( // The first and last characters have different restrictions, than // the middle characters. The middle characters length must be between // 4 and 28 since the first and last character are excluded. - ServiceAccountNameRegex = fmt.Sprintf(RFC1035NameTemplate, 4, 28) + ServiceAccountNameRegex = fmt.Sprintf(RFC1035NameTemplate, "{4,28}") ServiceAccountLinkRegexPrefix = "projects/" + ProjectRegexWildCard + "/serviceAccounts/" PossibleServiceAccountNames = []string{ @@ -54,7 +54,7 @@ var ( ServiceAccountKeyNameRegex = ServiceAccountLinkRegexPrefix + "(.+)/keys/(.+)" // Format of service accounts created through the API - CreatedServiceAccountNameRegex = fmt.Sprintf(RFC1035NameTemplate, 4, 28) + "@" + ProjectNameInDNSFormRegex + "\\.iam\\.gserviceaccount\\.com$" + CreatedServiceAccountNameRegex = fmt.Sprintf(RFC1035NameTemplate, "{4,28}") + "@" + ProjectNameInDNSFormRegex + "\\.iam\\.gserviceaccount\\.com$" // Format of service-created service account // examples are: @@ -194,19 +194,26 @@ func ValidateRFC3339Time(v interface{}, k string) (warnings []string, errors []e } func ValidateRFC1035Name(min, max int) schema.SchemaValidateFunc { - if min < 1 || max < min { - return func(i interface{}, k string) (s []string, errors []error) { - if min < 1 { - errors = append(errors, fmt.Errorf("min must be at least 1. Got: %d", min)) - } - if max < min { - errors = append(errors, fmt.Errorf("max must greater than min. Got [%d, %d]", min, max)) - } - return + return func(i interface{}, k string) (s []string, errors []error) { + value := i.(string) + re := fmt.Sprintf("^"+RFC1035NameTemplate+"$", "*") + if min < 1 { + errors = append(errors, fmt.Errorf("min must be at least 1. Got: %d", min)) + } + if max < min { + errors = append(errors, fmt.Errorf("max must greater than min. Got [%d, %d]", min, max)) + } + + if len(value) < min || len(value) > max { + errors = append(errors, fmt.Errorf("%q (%q) must be between %d and %d characters long", k, value, min, max)) + } + + if !regexp.MustCompile(re).MatchString(value) { + errors = append(errors, fmt.Errorf("%q (%q) must match regex %q", k, value, re)) } - } - return ValidateRegexp(fmt.Sprintf("^"+RFC1035NameTemplate+"$", min-1, max-2)) + return + } } func ValidateIpCidrRange(v interface{}, k string) (warnings []string, errors []error) { diff --git a/mmv1/third_party/terraform/verify/validation_test.go b/mmv1/third_party/terraform/verify/validation_test.go index c4a35627ab4e..19555861fb64 100644 --- a/mmv1/third_party/terraform/verify/validation_test.go +++ b/mmv1/third_party/terraform/verify/validation_test.go @@ -170,12 +170,18 @@ func TestValidateRFC1035Name(t *testing.T) { {TestName: "valid lower bound", Min: 12, Max: 30, Value: "a-valid-name"}, {TestName: "valid upper bound", Min: 6, Max: 12, Value: "a-valid-name"}, {TestName: "valid with numbers", Min: 6, Max: 30, Value: "valid000-name"}, + {TestName: "valid shortest", Min: 1, Max: 63, Value: "a"}, + {TestName: "valid longest", Min: 1, Max: 63, Value: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, {TestName: "must start with a letter", Min: 6, Max: 10, Value: "0invalid", ExpectError: true}, {TestName: "cannot end with a dash", Min: 6, Max: 10, Value: "invalid-", ExpectError: true}, {TestName: "too short", Min: 6, Max: 10, Value: "short", ExpectError: true}, {TestName: "too long", Min: 6, Max: 10, Value: "toolooooong", ExpectError: true}, {TestName: "min too small", Min: 0, Max: 10, Value: "", ExpectError: true}, {TestName: "min < max", Min: 6, Max: 5, Value: "", ExpectError: true}, + {TestName: "min < max", Min: 6, Max: 5, Value: "", ExpectError: true}, + {TestName: "invalid smallest possible w/ higher limit", Min: 2, Max: 63, Value: "a", ExpectError: true}, + {TestName: "invalid smallest possible hyphen", Min: 1, Max: 1, Value: "-", ExpectError: true}, + {TestName: "invalid smallest possible ends with hyphen", Min: 2, Max: 63, Value: "a-", ExpectError: true}, } for _, c := range cases { From 48136153dca8ac62798f2730053e96b5787588d1 Mon Sep 17 00:00:00 2001 From: Karol Gorczyk Date: Fri, 11 Oct 2024 10:59:50 +0200 Subject: [PATCH 5/5] fix rebase errors --- .../compute/resource_compute_instance_template.go.tmpl | 7 +++++++ ...ompute_region_instance_template_internal_test.go.tmpl | 9 ++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance_template.go.tmpl b/mmv1/third_party/terraform/services/compute/resource_compute_instance_template.go.tmpl index 56b7e06d1866..9363286c6b0b 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance_template.go.tmpl +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance_template.go.tmpl @@ -1207,6 +1207,9 @@ func buildDisks(d *schema.ResourceData, config *transport_tpg.Config) ([]*comput // Build the disk var disk compute.AttachedDisk + disk.Type = "PERSISTENT" + disk.Mode = "READ_WRITE" + disk.Interface = "SCSI" disk.Boot = i == 0 disk.AutoDelete = d.Get(prefix + ".auto_delete").(bool) @@ -1617,6 +1620,10 @@ func reorderDisks(configDisks []interface{}, apiDisks []map[string]interface{}) disksByDeviceName[v.(string)] = i } else if v := disk["type"]; v.(string) == "SCRATCH" { iface := disk["interface"].(string) + if iface == "" { + // apply-time default + iface = "SCSI" + } scratchDisksByInterface[iface] = append(scratchDisksByInterface[iface], i) } else if v := disk["source"]; v.(string) != "" { attachedDisksBySource[v.(string)] = i diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_template_internal_test.go.tmpl b/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_template_internal_test.go.tmpl index bb235b036588..a643e4719836 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_template_internal_test.go.tmpl +++ b/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_template_internal_test.go.tmpl @@ -22,9 +22,8 @@ func TestComputeRegionInstanceTemplate_reorderDisks(t *testing.T) { cDeviceName := map[string]interface{}{ "device_name": "disk-1", } - cScratchScsi := map[string]interface{}{ + cScratch := map[string]interface{}{ "type": "SCRATCH", - "interface": "SCSI", } cSource := map[string]interface{}{ "source": "disk-source", @@ -82,7 +81,7 @@ func TestComputeRegionInstanceTemplate_reorderDisks(t *testing.T) { aBoot, aScratchNvme, aSource, aScratchScsi, aFallThrough, aDeviceName, }, ConfigDisks: []interface{}{ - cBoot, cFallThrough, cDeviceName, cScratchScsi, cSource, cScratchNvme, + cBoot, cFallThrough, cDeviceName, cScratch, cSource, cScratchNvme, }, ExpectedResult: []map[string]interface{}{ aBoot, aFallThrough, aDeviceName, aScratchScsi, aSource, aScratchNvme, @@ -93,7 +92,7 @@ func TestComputeRegionInstanceTemplate_reorderDisks(t *testing.T) { aBoot, aNoMatch, aScratchNvme, aScratchScsi, aFallThrough, aDeviceName, }, ConfigDisks: []interface{}{ - cBoot, cFallThrough, cDeviceName, cScratchScsi, cSource, cScratchNvme, + cBoot, cFallThrough, cDeviceName, cScratch, cSource, cScratchNvme, }, ExpectedResult: []map[string]interface{}{ aBoot, aFallThrough, aDeviceName, aScratchScsi, aScratchNvme, aNoMatch, @@ -104,7 +103,7 @@ func TestComputeRegionInstanceTemplate_reorderDisks(t *testing.T) { aBoot, aScratchNvme, aFallThrough, aSource, aScratchScsi, aFallThrough2, aDeviceName, }, ConfigDisks: []interface{}{ - cBoot, cFallThrough, cDeviceName, cScratchScsi, cFallThrough, cSource, cScratchNvme, + cBoot, cFallThrough, cDeviceName, cScratch, cFallThrough, cSource, cScratchNvme, }, ExpectedResult: []map[string]interface{}{ aBoot, aFallThrough, aDeviceName, aScratchScsi, aFallThrough2, aSource, aScratchNvme,