Skip to content

Commit

Permalink
private_endpoint_services: fix backwards compat bug
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@carloruiz
carloruiz committed Jun 7, 2024
1 parent 325fbb5 commit a5563ab
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 24 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
17 changes: 9 additions & 8 deletions internal/provider/private_endpoint_services_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,21 +252,22 @@ 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))
for j, az := range apiAZs {
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(
Expand Down
101 changes: 85 additions & 16 deletions internal/provider/private_endpoint_services_resource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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(),
Expand All @@ -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{
Expand Down Expand Up @@ -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{}
Expand All @@ -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
Expand All @@ -182,18 +215,34 @@ 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{
resource.TestCheckResourceAttr(clusterResourceName, "name", clusterName),
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{
Expand All @@ -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"
Expand All @@ -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.
Expand Down

0 comments on commit a5563ab

Please sign in to comment.