Merge pull request #20 from redpanda-data/remove-todo

[chore] remove TODOs and add cluster spec validators

redpanda/resources/acl/resource_acl.go

@@ -76,7 +76,6 @@ func (*ACL) Schema(_ context.Context, _ resource.SchemaRequest, response *resour
 func resourceACLSchema() schema.Schema {
 	return schema.Schema{
 		Attributes: map[string]schema.Attribute{
-			// TODO: needs String validators.
 			"resource_type": schema.StringAttribute{
 				Required:      true,
 				Description:   "The type of the resource (Topic, Group, etc...) this ACL shall target",

redpanda/resources/cluster/resource_cluster.go

@@ -176,8 +176,13 @@ func (c *Cluster) Create(ctx context.Context, req resource.CreateRequest, resp *
 	var model models.Cluster
 	resp.Diagnostics.Append(req.Plan.Get(ctx, &model)...)
 
+	clusterReq, err := GenerateClusterRequest(model)
+	if err != nil {
+		resp.Diagnostics.AddError("unable to parse CreateCluster request", err.Error())
+		return
+	}
 	op, err := c.CluClient.CreateCluster(ctx, &cloudv1beta1.CreateClusterRequest{
-		Cluster: GenerateClusterRequest(model),
+		Cluster: clusterReq,
 	})
 	if err != nil {
 		resp.Diagnostics.AddError("failed to create cluster", err.Error())
@@ -310,17 +315,25 @@ func (*Cluster) ImportState(ctx context.Context, req resource.ImportStateRequest
 }
 
 // GenerateClusterRequest was pulled out to enable unit testing
-func GenerateClusterRequest(model models.Cluster) *cloudv1beta1.Cluster {
+func GenerateClusterRequest(model models.Cluster) (*cloudv1beta1.Cluster, error) {
+	provider, err := utils.StringToCloudProvider(model.CloudProvider.ValueString())
+	if err != nil {
+		return nil, fmt.Errorf("unable to parse cloud provider: %v", err)
+	}
+	clusterType, err := utils.StringToClusterType(model.ClusterType.ValueString())
+	if err != nil {
+		return nil, fmt.Errorf("unable to parse cluster type: %v", err)
+	}
 	return &cloudv1beta1.Cluster{
 		Name:            model.Name.ValueString(),
 		ConnectionType:  utils.StringToConnectionType(model.ConnectionType.ValueString()),
-		CloudProvider:   utils.StringToCloudProvider(model.CloudProvider.ValueString()),
+		CloudProvider:   provider,
 		RedpandaVersion: model.RedpandaVersion.ValueString(),
 		ThroughputTier:  model.ThroughputTier.ValueString(),
 		Region:          model.Region.ValueString(),
 		Zones:           utils.TypeListToStringSlice(model.Zones),
 		NamespaceId:     model.NamespaceID.ValueString(),
 		NetworkId:       model.NetworkID.ValueString(),
-		Type:            utils.StringToClusterType(model.ClusterType.ValueString()),
-	}
+		Type:            clusterType,
+	}, nil
 }

redpanda/resources/cluster/resource_cluster_test.go

@@ -52,7 +52,7 @@ func TestGenerateClusterRequest(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := GenerateClusterRequest(tt.args.model); !reflect.DeepEqual(got, tt.want) {
+			if got, _ := GenerateClusterRequest(tt.args.model); !reflect.DeepEqual(got, tt.want) {
 				fmt.Println("got")
 				spew.Dump(got)
 				fmt.Println("want")

redpanda/resources/network/resource_network.go

@@ -161,7 +161,16 @@ func (n *Network) Create(ctx context.Context, request resource.CreateRequest, re
 	var model models.Network
 	response.Diagnostics.Append(request.Plan.Get(ctx, &model)...)
 
-	cloudProvider := utils.StringToCloudProvider(model.CloudProvider.ValueString())
+	cloudProvider, err := utils.StringToCloudProvider(model.CloudProvider.ValueString())
+	if err != nil {
+		response.Diagnostics.AddError("unsupported cloud provider", err.Error())
+		return
+	}
+	clusterType, err := utils.StringToClusterType(model.ClusterType.ValueString())
+	if err != nil {
+		response.Diagnostics.AddError("unsupported cluster type", err.Error())
+		return
+	}
 	// TODO add a check to the provider data here to see if region and cloud provider are set
 	// prefer the local value, but accept the provider value if local is
 	// unavailable if neither are set, fail
@@ -173,7 +182,7 @@ func (n *Network) Create(ctx context.Context, request resource.CreateRequest, re
 			Region:        model.Region.ValueString(),
 			CloudProvider: cloudProvider,
 			NamespaceId:   model.NamespaceID.ValueString(),
-			ClusterType:   utils.StringToClusterType(model.ClusterType.ValueString()),
+			ClusterType:   clusterType,
 		},
 	})
 	if err != nil {
@@ -186,7 +195,6 @@ func (n *Network) Create(ctx context.Context, request resource.CreateRequest, re
 		return
 	}
 
-	// TODO: accept user configuration for timeout
 	if err := utils.AreWeDoneYet(ctx, op, 15*time.Minute, n.OpsClient); err != nil {
 		response.Diagnostics.AddError("failed waiting for network creation", err.Error())
 		return
@@ -245,7 +253,6 @@ func (n *Network) Delete(ctx context.Context, request resource.DeleteRequest, re
 		response.Diagnostics.AddError("failed to delete network", err.Error())
 		return
 	}
-	// TODO allow configurable timeout
 	if err := utils.AreWeDoneYet(ctx, op, 15*time.Minute, n.OpsClient); err != nil {
 		response.Diagnostics.AddError("failed waiting for network deletion", err.Error())
 	}

redpanda/utils/utils.go

@@ -44,19 +44,16 @@ func IsNotFound(err error) bool {
 	return false
 }
 
-// TODO check more to see if the client handles this
-
 // StringToCloudProvider returns the cloudv1beta1's CloudProvider code based on
 // the input string.
-func StringToCloudProvider(p string) cloudv1beta1.CloudProvider {
+func StringToCloudProvider(p string) (cloudv1beta1.CloudProvider, error) {
 	switch strings.ToLower(p) {
 	case "aws":
-		return cloudv1beta1.CloudProvider_CLOUD_PROVIDER_AWS
+		return cloudv1beta1.CloudProvider_CLOUD_PROVIDER_AWS, nil
 	case "gcp":
-		return cloudv1beta1.CloudProvider_CLOUD_PROVIDER_GCP
+		return cloudv1beta1.CloudProvider_CLOUD_PROVIDER_GCP, nil
 	default:
-		return cloudv1beta1.CloudProvider_CLOUD_PROVIDER_UNSPECIFIED
-		// TODO should we error here?
+		return cloudv1beta1.CloudProvider_CLOUD_PROVIDER_UNSPECIFIED, fmt.Errorf("provider %q not supported", p)
 	}
 }
 
@@ -70,21 +67,19 @@ func CloudProviderToString(provider cloudv1beta1.CloudProvider) string {
 		return "gcp"
 	default:
 		return providerUnspecified
-		// TODO should we error here?
 	}
 }
 
 // StringToClusterType returns the cloudv1beta1's Cluster_Type code based on
 // the input string.
-func StringToClusterType(p string) cloudv1beta1.Cluster_Type {
+func StringToClusterType(p string) (cloudv1beta1.Cluster_Type, error) {
 	switch strings.ToLower(p) {
 	case "dedicated":
-		return cloudv1beta1.Cluster_TYPE_DEDICATED
+		return cloudv1beta1.Cluster_TYPE_DEDICATED, nil
 	case "cloud":
-		return cloudv1beta1.Cluster_TYPE_BYOC
+		return cloudv1beta1.Cluster_TYPE_BYOC, nil
 	default:
-		return cloudv1beta1.Cluster_TYPE_UNSPECIFIED
-		// TODO should we error here?
+		return cloudv1beta1.Cluster_TYPE_UNSPECIFIED, fmt.Errorf("cluster type %q not supported", p)
 	}
 }
 
@@ -98,7 +93,6 @@ func ClusterTypeToString(provider cloudv1beta1.Cluster_Type) string {
 		return "cloud"
 	default:
 		return providerUnspecified
-		// TODO should we error here?
 	}
 }