From 36196f336a5724fe4e4583b24519c81dbd343363 Mon Sep 17 00:00:00 2001 From: carloruiz Date: Fri, 7 Jun 2024 16:48:55 -0700 Subject: [PATCH] private_endpoint_services: fix backwards compat bug (#214) Previously, when reading this resource for AWS the clusters, the '.aws' remained unset, despite expectations otherwise. This commit fixes the bug to ensure that field remains set. --- CHANGELOG.md | 4 + .../private_endpoint_services_resource.go | 17 +-- ...private_endpoint_services_resource_test.go | 101 +++++++++++++++--- 3 files changed, 98 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45bdbc5b..b211f253 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.7.6] - 2024-06-10 + ## Fixed - Update docs for allowlist resource to clear up with cidr_mask is @@ -17,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added some example values for clarity in README +- Fix bug when reading `cockroach_private_endpoint_services.#.aws.service_name`. + ## [1.7.5] - 2024-06-06 ## Fixed diff --git a/internal/provider/private_endpoint_services_resource.go b/internal/provider/private_endpoint_services_resource.go index ecbdb1fe..85330956 100644 --- a/internal/provider/private_endpoint_services_resource.go +++ b/internal/provider/private_endpoint_services_resource.go @@ -252,13 +252,6 @@ func loadEndpointServicesIntoTerraformState( EndpointServiceId: types.StringValue(service.GetEndpointServiceId()), } - if services[i].CloudProvider.String() == "AWS" { - services[i].Aws = PrivateLinkServiceAWSDetail{ - ServiceName: types.StringValue(service.Aws.GetServiceName()), - ServiceId: types.StringValue(service.Aws.GetServiceId()), - } - } - traceAPICall("GetAvailabilityZoneIds") apiAZs := service.GetAvailabilityZoneIds() azs := make([]types.String, len(apiAZs)) @@ -266,7 +259,15 @@ func loadEndpointServicesIntoTerraformState( azs[j] = types.StringValue(az) } services[i].AvailabilityZoneIds = azs - services[i].Aws.AvailabilityZoneIds = azs + + // Set the .AWS field for backward compatibility. + if services[i].CloudProvider.ValueString() == "AWS" { + services[i].Aws = PrivateLinkServiceAWSDetail{ + ServiceName: types.StringValue(service.GetName()), + ServiceId: types.StringValue(service.GetEndpointServiceId()), + AvailabilityZoneIds: azs, + } + } } var diags diag.Diagnostics state.Services, diags = types.ListValueFrom( diff --git a/internal/provider/private_endpoint_services_resource_test.go b/internal/provider/private_endpoint_services_resource_test.go index ae9f8fed..a2a04fde 100644 --- a/internal/provider/private_endpoint_services_resource_test.go +++ b/internal/provider/private_endpoint_services_resource_test.go @@ -36,7 +36,7 @@ import ( func TestAccDedicatedPrivateEndpointServicesResource(t *testing.T) { t.Parallel() clusterName := fmt.Sprintf("%s-endpt-svc-%s", tfTestPrefix, GenerateRandomString(2)) - testPrivateEndpointServicesResource(t, clusterName, false /* useMock */, false /* isServerless */) + testPrivateEndpointServicesResource(t, clusterName, false /* useMock */, false /* isServerless */, client.CLOUDPROVIDERTYPE_GCP) } // TestAccServerlessPrivateEndpointServicesResource attempts to create, check, @@ -45,7 +45,7 @@ func TestAccDedicatedPrivateEndpointServicesResource(t *testing.T) { func TestAccServerlessPrivateEndpointServicesResource(t *testing.T) { t.Parallel() clusterName := fmt.Sprintf("%s-endpt-svc-%s", tfTestPrefix, GenerateRandomString(2)) - testPrivateEndpointServicesResource(t, clusterName, false /* useMock */, true /* isServerless */) + testPrivateEndpointServicesResource(t, clusterName, false /* useMock */, true /* isServerless */, client.CLOUDPROVIDERTYPE_AWS) } // TestIntegrationAllowlistEntryResource attempts to create, check, and destroy @@ -64,7 +64,7 @@ func TestIntegrationPrivateEndpointServicesResource(t *testing.T) { finalCluster client.Cluster }{ { - "dedicated cluster", + "dedicated GCP cluster", client.Cluster{ Name: clusterName, Id: uuid.Nil.String(), @@ -85,6 +85,30 @@ func TestIntegrationPrivateEndpointServicesResource(t *testing.T) { }, }, }, + + { + "dedicated AWS cluster", + client.Cluster{ + Name: clusterName, + Id: uuid.Nil.String(), + CloudProvider: "AWS", + Config: client.ClusterConfig{ + Dedicated: &client.DedicatedHardwareConfig{ + StorageGib: 15, + MachineType: "not verified", + NumVirtualCpus: 2, + }, + }, + State: "CREATED", + Regions: []client.Region{ + { + Name: "us-east-1", + NodeCount: 1, + }, + }, + }, + }, + { "serverless cluster", client.Cluster{ @@ -126,24 +150,32 @@ func TestIntegrationPrivateEndpointServicesResource(t *testing.T) { s.EXPECT().GetCluster(gomock.Any(), clusterID). Return(&cluster, &http.Response{Status: http.StatusText(http.StatusOK)}, nil). Times(3) + var regions []string - if isServerless { - // AWS - regions = []string{"us-east-1", "eu-central-1"} - } else { - // GCP - regions = []string{"us-east1"} + for _, r := range c.finalCluster.Regions { + regions = append(regions, r.Name) } + services := &client.PrivateEndpointServices{} for _, region := range regions { - services.Services = append(services.Services, client.PrivateEndpointService{ + svc := client.PrivateEndpointService{ RegionName: region, CloudProvider: c.finalCluster.CloudProvider, Status: client.PRIVATEENDPOINTSERVICESTATUSTYPE_AVAILABLE, Name: "finalService-name", EndpointServiceId: "finalService-id", - AvailabilityZoneIds: []string{}, - }) + AvailabilityZoneIds: []string{"az1", "az2", "az3"}, + } + + if c.finalCluster.CloudProvider == client.CLOUDPROVIDERTYPE_AWS { + svc.Aws = &client.AWSPrivateLinkServiceDetail{ + ServiceName: "finalService-name", + ServiceId: "finalService-id", + AvailabilityZoneIds: []string{"az1", "az2", "az3"}, + } + } + + services.Services = append(services.Services, svc) } if !isServerless { initialServices := &client.PrivateEndpointServices{} @@ -164,13 +196,14 @@ func TestIntegrationPrivateEndpointServicesResource(t *testing.T) { clusterName, true, /* useMock */ isServerless, + c.finalCluster.CloudProvider, ) }) } } func testPrivateEndpointServicesResource( - t *testing.T, clusterName string, useMock bool, isServerless bool, + t *testing.T, clusterName string, useMock bool, isServerless bool, cloud client.CloudProviderType, ) { resourceName := "cockroach_private_endpoint_services.services" var clusterResourceName string @@ -182,8 +215,13 @@ func testPrivateEndpointServicesResource( numExpectedServices = 2 } else { clusterResourceName = "cockroach_cluster.dedicated" - privateEndpointServicesResourceConfigFn = getTestPrivateEndpointServicesResourceConfigForDedicated numExpectedServices = 1 + switch cloud { + case client.CLOUDPROVIDERTYPE_AWS: + privateEndpointServicesResourceConfigFn = getTestPrivateEndpointServicesResourceConfigForDedicatedAWS + case client.CLOUDPROVIDERTYPE_GCP: + privateEndpointServicesResourceConfigFn = getTestPrivateEndpointServicesResourceConfigForDedicatedGCP + } } checks := []resource.TestCheckFunc{ @@ -191,9 +229,20 @@ func testPrivateEndpointServicesResource( resource.TestCheckResourceAttr(resourceName, "services.#", strconv.Itoa(numExpectedServices)), } for i := 0; i < numExpectedServices; i++ { + svc := fmt.Sprintf("services.%d", i) checks = append(checks, - resource.TestCheckResourceAttr(resourceName, fmt.Sprintf("services.%d.status", i), string(client.PRIVATEENDPOINTSERVICESTATUSTYPE_AVAILABLE)), + resource.TestCheckResourceAttr(resourceName, svc+".status", string(client.PRIVATEENDPOINTSERVICESTATUSTYPE_AVAILABLE)), ) + + if cloud == client.CLOUDPROVIDERTYPE_AWS { + checks = append(checks, []resource.TestCheckFunc{ + resource.TestCheckResourceAttrPair(resourceName, svc+".name", resourceName, svc+".aws.service_name"), + resource.TestCheckResourceAttrPair(resourceName, svc+".endpoint_service_id", resourceName, svc+".aws.service_id"), + resource.TestCheckResourceAttrPair(resourceName, svc+".availability_zone_ids", resourceName, svc+".aws.availability_zone_ids"), + }...) + } else { + checks = append(checks, resource.TestCheckNoResourceAttr(resourceName, svc+".aws.service_name")) + } } resource.Test(t, resource.TestCase{ @@ -214,7 +263,7 @@ func testPrivateEndpointServicesResource( }) } -func getTestPrivateEndpointServicesResourceConfigForDedicated(clusterName string) string { +func getTestPrivateEndpointServicesResourceConfigForDedicatedGCP(clusterName string) string { return fmt.Sprintf(` resource "cockroach_cluster" "dedicated" { name = "%s" @@ -234,6 +283,26 @@ resource "cockroach_private_endpoint_services" "services" { `, clusterName) } +func getTestPrivateEndpointServicesResourceConfigForDedicatedAWS(clusterName string) string { + return fmt.Sprintf(` +resource "cockroach_cluster" "dedicated" { + name = "%s" + cloud_provider = "AWS" + dedicated = { + storage_gib = 15 + num_virtual_cpus = 2 + } + regions = [{ + name: "us-east-1" + node_count: 1 + }] +} +resource "cockroach_private_endpoint_services" "services" { + cluster_id = cockroach_cluster.dedicated.id +} +`, clusterName) +} + func getTestPrivateEndpointServicesResourceConfigForServerless(clusterName string) string { // Use two regions here so we end up creating the cluster on the multi- // region host, which has PrivateLink enabled.