Skip to content

Commit

Permalink
fix: move cluster_api_url parsing out of Terraform lifecycle path
Browse files Browse the repository at this point in the history
If a Cluster or ServerlessCluster returns a URL that can't be parsed,
it shouldn't be an error for that resource or taint the state.

This commit moves the parsing into cloud.SpawnConn instead, so an
unparseable URL will only affect downstream resources that try to use
it.
  • Loading branch information
ligfx authored and gene-redpanda committed Oct 21, 2024
1 parent bfcd753 commit 3856ba4
Show file tree
Hide file tree
Showing 13 changed files with 123 additions and 166 deletions.
6 changes: 2 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ require (
github.com/hashicorp/terraform-plugin-go v0.23.0
github.com/hashicorp/terraform-plugin-log v0.9.0
github.com/hashicorp/terraform-plugin-testing v1.10.0
github.com/redpanda-data/redpanda/src/go/rpk v0.0.0-20240715191109-e3ca3047d5b7
github.com/stretchr/testify v1.9.0
golang.org/x/time v0.6.0
google.golang.org/genproto/googleapis/rpc v0.0.0-20240711142825-46eb208f015d
Expand All @@ -33,12 +32,14 @@ require (
github.com/Masterminds/goutils v1.1.1 // indirect
github.com/Masterminds/semver/v3 v3.2.0 // indirect
github.com/Masterminds/sprig/v3 v3.2.3 // indirect
github.com/Microsoft/go-winio v0.6.2 // indirect
github.com/ProtonMail/go-crypto v1.1.0-alpha.2 // indirect
github.com/agext/levenshtein v1.2.2 // indirect
github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect
github.com/armon/go-radix v1.0.0 // indirect
github.com/bgentry/speakeasy v0.1.0 // indirect
github.com/bmatcuk/doublestar/v4 v4.6.1 // indirect
github.com/bufbuild/protocompile v0.13.0 // indirect
github.com/cloudflare/circl v1.3.7 // indirect
github.com/fatih/color v1.17.0 // indirect
github.com/golang/protobuf v1.5.4 // indirect
Expand Down Expand Up @@ -80,7 +81,6 @@ require (
github.com/posener/complete v1.2.3 // indirect
github.com/rivo/uniseg v0.4.7 // indirect
github.com/shopspring/decimal v1.3.1 // indirect
github.com/spf13/afero v1.11.0 // indirect
github.com/spf13/cast v1.5.0 // indirect
github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect
github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect
Expand All @@ -89,8 +89,6 @@ require (
github.com/yuin/goldmark-meta v1.1.0 // indirect
github.com/zclconf/go-cty v1.15.0 // indirect
go.abhg.dev/goldmark/frontmatter v0.2.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
go.uber.org/zap v1.27.0 // indirect
golang.org/x/crypto v0.26.0 // indirect
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 // indirect
golang.org/x/mod v0.19.0 // indirect
Expand Down
10 changes: 0 additions & 10 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,6 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
github.com/posener/complete v1.2.3 h1:NP0eAhjcjImqslEwo/1hq7gpajME0fTLTezBKDqfXqo=
github.com/posener/complete v1.2.3/go.mod h1:WZIdtGGp+qx0sLrYKtIRAruyNpv6hFCicSgv7Sy7s/s=
github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
github.com/redpanda-data/redpanda/src/go/rpk v0.0.0-20240715191109-e3ca3047d5b7 h1:eeQdVXqwYCh2WU/FEBz7JBJXWZsL5oStgGK8rWSFdho=
github.com/redpanda-data/redpanda/src/go/rpk v0.0.0-20240715191109-e3ca3047d5b7/go.mod h1:OS++uCOt5CjFIWARC7YpaA0gULdcd9oYmBfKf2K0Q9c=
github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ=
github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
Expand All @@ -230,8 +228,6 @@ github.com/shopspring/decimal v1.3.1/go.mod h1:DKyhrW/HYNuLGql+MJL6WCR6knT2jwCFR
github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
github.com/skeema/knownhosts v1.2.2 h1:Iug2P4fLmDw9f41PB6thxUkNUkJzB5i+1/exaj40L3A=
github.com/skeema/knownhosts v1.2.2/go.mod h1:xYbVRSPxqBZFrdmDyMmsOs+uX1UZC3nTN3ThzgDxUwo=
github.com/spf13/afero v1.11.0 h1:WJQKhtpdm3v2IzqG8VMqrr6Rf3UYpEF239Jy9wNepM8=
github.com/spf13/afero v1.11.0/go.mod h1:GH9Y3pIexgf1MTIWtNGyogA5MwRIDXGUr+hbWNoBjkY=
github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE=
github.com/spf13/cast v1.5.0 h1:rj3WzYc11XZaIZMPKmwP96zkFEnnAmV8s6XbB2aY32w=
github.com/spf13/cast v1.5.0/go.mod h1:SpXXQ5YoyJw6s3/6cMTQuxvgRl3PCJiyaX9p6b155UU=
Expand Down Expand Up @@ -270,14 +266,8 @@ go.abhg.dev/goldmark/frontmatter v0.2.0 h1:P8kPG0YkL12+aYk2yU3xHv4tcXzeVnN+gU0tJ
go.abhg.dev/goldmark/frontmatter v0.2.0/go.mod h1:XqrEkZuM57djk7zrlRUB02x8I5J0px76YjkOzhB4YlU=
go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A=
go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
go.uber.org/zap v1.18.1/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI=
go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8=
go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
Expand Down
28 changes: 23 additions & 5 deletions redpanda/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"fmt"
"io"
"net/http"
"regexp"
"strings"
"time"

Expand All @@ -47,22 +48,22 @@ type Endpoint struct {

var endpoints = map[string]Endpoint{
"dev": {
"api.dev.cloud.redpanda.com:443",
"https://api.dev.cloud.redpanda.com",
"https://dev-cloudv2.us.auth0.com/oauth/token",
"cloudv2-dev.redpanda.cloud",
},
"ign": {
"api.ign.cloud.redpanda.com:443",
"https://api.ign.cloud.redpanda.com",
"https://integration-cloudv2.us.auth0.com/oauth/token",
"cloudv2-ign.redpanda.cloud",
},
"pre": {
APIURL: "api.ppd.cloud.redpanda.com:443",
APIURL: "https://api.ppd.cloud.redpanda.com",
authURL: "https://preprod-cloudv2.us.auth0.com/oauth/token",
audience: "cloudv2-preprod.redpanda.cloud",
},
"prod": {
"api.redpanda.com:443",
"https://api.redpanda.com",
"https://auth.prd.cloud.redpanda.com/oauth/token",
"cloudv2-production.redpanda.cloud",
},
Expand Down Expand Up @@ -124,11 +125,28 @@ func RequestToken(ctx context.Context, endpoint *Endpoint, clientID, clientSecre

var rl = newRateLimiter(500)

var urlRegex = regexp.MustCompile(`^(?:https://)?([^/]+?(?::\d+)?)/?$`)

// parseHTTPSURLAsGrpc parse an HTTPS URL into a valid GRPC URL
func parseHTTPSURLAsGrpc(url string) (string, error) {
match := urlRegex.FindStringSubmatch(url)
if match == nil {
return "", fmt.Errorf("error converting url into grpc url: %v", url)
}
return match[1], nil
}

// SpawnConn returns a grpc connection to the given URL, it adds a bearer token
// to each request with the given 'authToken'.
func SpawnConn(url, authToken string) (*grpc.ClientConn, error) {
// we need a GRPC URL, but it's likely that we'll be given an HTTPS URL instead
grpcURL, err := parseHTTPSURLAsGrpc(url)
if err != nil {
return nil, err
}

return grpc.NewClient(
url,
grpcURL,
// Chain the interceptors using grpc_middleware.ChainUnaryClient
grpc.WithUnaryInterceptor(grpcmiddleware.ChainUnaryClient(
// Interceptor to add the Bearer token
Expand Down
86 changes: 86 additions & 0 deletions redpanda/cloud/cloud_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package cloud

import (
"testing"
)

func TestParseHTTPSURLAsGrpc(t *testing.T) {
testCases := []struct {
name string
url string
expected string
expectError bool
}{
{
name: "URL with incorrect scheme",
url: "http://example.com:8080",
expected: "",
expectError: true,
},
{
name: "URL with scheme, no port",
url: "https://example.com",
expected: "example.com",
expectError: false,
},
{
name: "URL without scheme, with port",
url: "example.com:9090",
expected: "example.com:9090",
expectError: false,
},
{
name: "URL without scheme, no port",
url: "example.com",
expected: "example.com",
expectError: false,
},
{
name: "IP address with port",
url: "192.168.1.1:8080",
expected: "192.168.1.1:8080",
expectError: false,
},
{
name: "IP address without port",
url: "192.168.1.1",
expected: "192.168.1.1",
expectError: false,
},
{
name: "URL with trailing slash",
url: "example.com/",
expected: "example.com",
expectError: false,
},
{
name: "URL with trailing path",
url: "example.com/api/v1",
expected: "",
expectError: true,
},
{
name: "Empty URL",
url: "",
expected: "",
expectError: true,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
result, err := parseHTTPSURLAsGrpc(tc.url)
if tc.expectError {
if err == nil {
t.Errorf("Expected an error, but got none")
}
} else {
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
if result != tc.expected {
t.Errorf("Expected %q, but got %q", tc.expected, result)
}
}
})
}
}
7 changes: 1 addition & 6 deletions redpanda/resources/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
controlplanev1beta2 "buf.build/gen/go/redpandadata/cloud/protocolbuffers/go/redpanda/api/controlplane/v1beta2"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
"github.com/redpanda-data/terraform-provider-redpanda/redpanda/models"
"github.com/redpanda-data/terraform-provider-redpanda/redpanda/utils"
)
Expand Down Expand Up @@ -238,11 +237,7 @@ func generateModel(ctx context.Context, cfg models.Cluster, cluster *controlplan
output.Zones = clusterZones

if cluster.GetDataplaneApi() != nil {
clusterURL, err := utils.SplitSchemeDefPort(cluster.DataplaneApi.Url, "443")
if err != nil {
return nil, fmt.Errorf("unable to parse Cluster API URL: %v", err)
}
output.ClusterAPIURL = basetypes.NewStringValue(clusterURL)
output.ClusterAPIURL = types.StringValue(cluster.DataplaneApi.Url)
}

rr, d := types.ListValueFrom(ctx, types.StringType, cluster.ReadReplicaClusterIds)
Expand Down
7 changes: 1 addition & 6 deletions redpanda/resources/cluster/data_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,6 @@ func (d *DataSourceCluster) Read(ctx context.Context, req datasource.ReadRequest
resp.Diagnostics.Append(dg...)
return
}
clusterURL, err := utils.SplitSchemeDefPort(cluster.DataplaneApi.Url, "443")
if err != nil {
resp.Diagnostics.AddError("unable to parse Cluster API URL", err.Error())
return
}
tags := make(map[string]attr.Value)
for k, v := range cluster.CloudProviderTags {
tags[k] = types.StringValue(v)
Expand Down Expand Up @@ -116,7 +111,7 @@ func (d *DataSourceCluster) Read(ctx context.Context, req datasource.ReadRequest
ResourceGroupID: types.StringValue(cluster.ResourceGroupId),
NetworkID: types.StringValue(cluster.NetworkId),
ID: types.StringValue(cluster.Id),
ClusterAPIURL: types.StringValue(clusterURL),
ClusterAPIURL: types.StringValue(cluster.DataplaneApi.Url),
ReadReplicaClusterIDs: rr,
}

Expand Down
10 changes: 5 additions & 5 deletions redpanda/resources/cluster/resource_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func TestGenerateModel(t *testing.T) {
ResourceGroupID: types.StringValue("rg-123"),
NetworkID: types.StringValue("net-456"),
ID: types.StringValue("cl-789"),
ClusterAPIURL: types.StringValue("test-cluster.rptest.io:443"),
ClusterAPIURL: types.StringValue("https://test-cluster.rptest.io:443"),
ReadReplicaClusterIDs: basetypes.NewListNull(types.StringType),
Zones: utils.TestingOnlyStringSliceToTypeList([]string{"us-west-2a", "us-west-2b"}),
AllowDeletion: types.BoolValue(false),
Expand Down Expand Up @@ -187,7 +187,7 @@ func TestGenerateModel(t *testing.T) {
ResourceGroupID: types.StringValue("rg-456"),
NetworkID: types.StringValue("net-789"),
ID: types.StringValue("cl-101"),
ClusterAPIURL: types.StringValue("gcp-private-cluster.rptest.io:443"),
ClusterAPIURL: types.StringValue("https://gcp-private-cluster.rptest.io:443"),
Zones: utils.TestingOnlyStringSliceToTypeList([]string{"us-central1-a", "us-central1-b", "us-central1-c"}),
AllowDeletion: types.BoolValue(true),
ReadReplicaClusterIDs: basetypes.NewListNull(types.StringType),
Expand Down Expand Up @@ -234,7 +234,7 @@ func TestGenerateModel(t *testing.T) {
ID: types.StringValue("cl-202"),
ResourceGroupID: types.StringValue("rg-789"),
NetworkID: types.StringValue("net-101"),
ClusterAPIURL: types.StringValue("aws-mtls-cluster.rptest.io:443"),
ClusterAPIURL: types.StringValue("https://aws-mtls-cluster.rptest.io:443"),
ReadReplicaClusterIDs: utils.TestingOnlyStringSliceToTypeList([]string{""}),
Zones: utils.TestingOnlyStringSliceToTypeList([]string{"eu-west-1a"}),
KafkaAPI: &models.KafkaAPI{
Expand Down Expand Up @@ -283,7 +283,7 @@ func TestGenerateModel(t *testing.T) {
CloudProvider: types.StringValue("gcp"),
ClusterType: types.StringValue("dedicated"),
ID: types.StringValue("cl-303"),
ClusterAPIURL: types.StringValue("gcp-aws-pl-cluster.rptest.io:443"),
ClusterAPIURL: types.StringValue("https://gcp-aws-pl-cluster.rptest.io:443"),
ThroughputTier: types.StringValue("t3"),
NetworkID: types.StringValue("net-303"),
ResourceGroupID: types.StringValue("123"),
Expand Down Expand Up @@ -346,7 +346,7 @@ func TestGenerateModel(t *testing.T) {
NetworkID: types.StringValue("net-404"),
ReadReplicaClusterIDs: utils.TestingOnlyStringSliceToTypeList([]string{""}),
Zones: utils.TestingOnlyStringSliceToTypeList([]string{"us-central1-a"}),
ClusterAPIURL: types.StringValue("aws-gcp-psc-cluster.rptest.io:443"),
ClusterAPIURL: types.StringValue("https://aws-gcp-psc-cluster.rptest.io:443"),
GcpPrivateServiceConnect: &models.GcpPrivateServiceConnect{
Enabled: types.BoolValue(true),
GlobalAccessEnabled: types.BoolValue(false),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,18 +76,13 @@ func (d *DataSourceServerlessCluster) Read(ctx context.Context, req datasource.R
resp.Diagnostics.AddError(fmt.Sprintf("failed to read serverless cluster %s", model.ID), err.Error())
return
}
clusterURL, err := utils.SplitSchemeDefPort(serverlessCluster.DataplaneApi.Url, "443")
if err != nil {
resp.Diagnostics.AddError("unable to parse Cluster API URL", err.Error())
return
}
// Mapping the fields from the serverless cluster to the Terraform state
resp.Diagnostics.Append(resp.State.Set(ctx, &models.ServerlessCluster{
Name: types.StringValue(serverlessCluster.Name),
ServerlessRegion: types.StringValue(serverlessCluster.ServerlessRegion),
ResourceGroupID: types.StringValue(serverlessCluster.ResourceGroupId),
ID: types.StringValue(serverlessCluster.Id),
ClusterAPIURL: types.StringValue(clusterURL),
ClusterAPIURL: types.StringValue(serverlessCluster.DataplaneApi.Url),
})...)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,17 +139,12 @@ func (c *ServerlessCluster) Create(ctx context.Context, req resource.CreateReque
resp.Diagnostics.AddError(fmt.Sprintf("successfully created the serverless cluster with ID %q, but failed to read the serverless cluster configuration: %v", op.GetResourceId(), err), err.Error())
return
}
clusterURL, err := utils.SplitSchemeDefPort(cluster.DataplaneApi.Url, "443")
if err != nil {
resp.Diagnostics.AddError("unable to parse Cluster API URL", err.Error())
return
}
resp.Diagnostics.Append(resp.State.Set(ctx, models.ServerlessCluster{
Name: types.StringValue(cluster.Name),
ServerlessRegion: types.StringValue(cluster.ServerlessRegion),
ResourceGroupID: types.StringValue(cluster.ResourceGroupId),
ID: types.StringValue(cluster.Id),
ClusterAPIURL: types.StringValue(clusterURL),
ClusterAPIURL: types.StringValue(cluster.DataplaneApi.Url),
})...)
}

Expand All @@ -175,18 +170,13 @@ func (c *ServerlessCluster) Read(ctx context.Context, req resource.ReadRequest,
resp.Diagnostics.AddWarning(fmt.Sprintf("serverless cluster %s is in state %s", cluster.Id, cluster.GetState()), "")
return
}
clusterURL, err := utils.SplitSchemeDefPort(cluster.DataplaneApi.Url, "443")
if err != nil {
resp.Diagnostics.AddError("unable to parse Cluster API URL", err.Error())
return
}

resp.Diagnostics.Append(resp.State.Set(ctx, models.ServerlessCluster{
Name: types.StringValue(cluster.Name),
ServerlessRegion: types.StringValue(cluster.ServerlessRegion),
ResourceGroupID: types.StringValue(cluster.ResourceGroupId),
ID: types.StringValue(cluster.Id),
ClusterAPIURL: types.StringValue(clusterURL),
ClusterAPIURL: types.StringValue(cluster.DataplaneApi.Url),
})...)
}

Expand Down
7 changes: 1 addition & 6 deletions redpanda/resources/topic/resource_topic.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,14 +303,9 @@ func (t *Topic) ImportState(ctx context.Context, req resource.ImportStateRequest
resp.Diagnostics.AddError(fmt.Sprintf("failed to find cluster with ID %q; make sure ADDR ID format is <topic_name>,<cluster_id>", clusterID), err.Error())
return
}
clusterURL, err := utils.SplitSchemeDefPort(cluster.DataplaneApi.Url, "443")
if err != nil {
resp.Diagnostics.AddError("unable to parse Cluster API URL", err.Error())
return
}
resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("name"), types.StringValue(topicName))...)
resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("id"), types.StringValue(topicName))...)
resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("cluster_api_url"), types.StringValue(clusterURL))...)
resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("cluster_api_url"), types.StringValue(cluster.DataplaneApi.Url))...)
}

func (t *Topic) createTopicClient(clusterURL string) error {
Expand Down
Loading

0 comments on commit 3856ba4

Please sign in to comment.