From a5563ab4a6c06a8ad0636d9e2353278ed8a5123e Mon Sep 17 00:00:00 2001 From: Carlo Ruiz Date: Fri, 7 Jun 2024 14:52:55 -0700 Subject: [PATCH] private_endpoint_services: fix backwards compat bug Previously, when reading this resource for AWS the clusters, the '.aws' remained unset, despite expectations otherwide. 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.