Merge pull request #10 from dbsystel/fix-provider-calculated-values

Fix errors with calculated provider attributes

Author: mmichaelb

redshift/config_data_api.go

@@ -24,12 +24,6 @@ func buildConnStrFromDataApiConfig(workgroupName, database, awsRegion string) st
 
 func getConfigFromDataApiResourceData(d *schema.ResourceData, database string) (*Config, error) {
 	workgroupName := d.Get("data_api.0.workgroup_name").(string)
-	if workgroupName == "" {
-		return nil, fmt.Errorf(`attribute "workgroup_name" is required in data_api configuration`)
-	}
 	region := d.Get("data_api.0.region").(string)
-	if region == "" {
-		return nil, fmt.Errorf(`attribute "region" is required in data_api configuration`)
-	}
 	return NewDataApiConfig(workgroupName, database, region, 1), nil
 }

redshift/config_pq_proxy.go

@@ -21,11 +21,11 @@ import (
 type temporaryCredentialsResolverFunc func(username string, d *schema.ResourceData) (string, string, error)
 
 func NewPqConfig(host, database, username, password string, port int, sslMode string, maxConns int) *Config {
-	connStr := buildConnStrFromPqConfig(host, database, username, password, port, sslMode, maxConns)
+	connStr := buildConnStrFromPqConfig(host, database, username, password, port, sslMode)
 	return NewConfig(proxyDriverName, connStr, database, maxConns)
 }
 
-func buildConnStrFromPqConfig(host, database, username, password string, port int, sslMode string, maxConns int) string {
+func buildConnStrFromPqConfig(host, database, username, password string, port int, sslMode string) string {
 	params := map[string]string{}
 
 	params["sslmode"] = sslMode
@@ -47,31 +47,14 @@ func buildConnStrFromPqConfig(host, database, username, password string, port in
 		strings.Join(paramsArray, "&"),
 	)
 }
-func getRequiredResourceDataValue[V int | string](d *schema.ResourceData, path string) (V, error) {
-	valueRaw, valuePresent := d.GetOk(path)
-	if !valuePresent {
-		var emptyValue V
-		return emptyValue, fmt.Errorf("attribute %q is required in pq configuration", path)
-	}
-	return valueRaw.(V), nil
-}
 
 func getConfigFromPqResourceData(d *schema.ResourceData, database string, maxConnections int, temporaryCredentialsResolver temporaryCredentialsResolverFunc) (*Config, error) {
 	var err error
-	var host, username, password, sslMode string
-	var port int
-	if host, err = getRequiredResourceDataValue[string](d, "host"); err != nil {
-		return nil, err
-	}
-	if username, err = getRequiredResourceDataValue[string](d, "username"); err != nil {
-		return nil, err
-	}
-	if port, err = getRequiredResourceDataValue[int](d, "port"); err != nil {
-		return nil, err
-	}
-	if sslMode, err = getRequiredResourceDataValue[string](d, "sslmode"); err != nil {
-		return nil, err
-	}
+	var password string
+	host := d.Get("host").(string)
+	username := d.Get("username").(string)
+	port := d.Get("port").(int)
+	sslMode := d.Get("sslmode").(string)
 	_, useTemporaryCredentials := d.GetOk("temporary_credentials")
 	if useTemporaryCredentials {
 		log.Println("[DEBUG] using temporary credentials authentication")
@@ -82,9 +65,7 @@ func getConfigFromPqResourceData(d *schema.ResourceData, database string, maxCon
 		log.Printf("[DEBUG] got temporary credentials with username %s\n", username)
 	} else {
 		log.Println("[DEBUG] using password authentication")
-		if password, err = getRequiredResourceDataValue[string](d, "password"); err != nil {
-			return nil, err
-		}
+		password = d.Get("password").(string)
 	}
 	return NewPqConfig(host, database, username, password, port, sslMode, maxConnections), nil
 }

redshift/provider.go

@@ -23,15 +23,14 @@ func Provider() *schema.Provider {
 				Type:          schema.TypeString,
 				Description:   "Name of Redshift server address to connect to.",
 				Optional:      true,
-				DefaultFunc:   schema.EnvDefaultFunc("REDSHIFT_HOST", ""),
+				DefaultFunc:   schema.EnvDefaultFunc("REDSHIFT_HOST", nil),
 				ConflictsWith: []string{"data_api"},
 			},
 			"username": {
-				Type:          schema.TypeString,
-				Optional:      true,
-				DefaultFunc:   schema.EnvDefaultFunc("REDSHIFT_USER", "root"),
-				Description:   "Redshift user name to connect as.",
-				ConflictsWith: []string{"data_api"},
+				Type:        schema.TypeString,
+				Optional:    true,
+				DefaultFunc: schema.EnvDefaultFunc("REDSHIFT_USER", "root"),
+				Description: "Redshift user name to connect as.",
 			},
 			"password": {
 				Type:        schema.TypeString,
@@ -41,15 +40,13 @@ func Provider() *schema.Provider {
 				Sensitive:   true,
 				ConflictsWith: []string{
 					"temporary_credentials",
-					"data_api",
 				},
 			},
 			"port": {
-				Type:          schema.TypeInt,
-				Description:   "The Redshift port number to connect to at the server host.",
-				Optional:      true,
-				DefaultFunc:   schema.EnvDefaultFunc("REDSHIFT_PORT", 5439),
-				ConflictsWith: []string{"data_api"},
+				Type:        schema.TypeInt,
+				Description: "The Redshift port number to connect to at the server host.",
+				Optional:    true,
+				DefaultFunc: schema.EnvDefaultFunc("REDSHIFT_PORT", 5439),
 			},
 			"sslmode": {
 				Type:        schema.TypeString,
@@ -62,7 +59,6 @@ func Provider() *schema.Provider {
 					"verify-ca",
 					"verify-full",
 				}, false),
-				ConflictsWith: []string{"data_api"},
 			},
 			"database": {
 				Type:        schema.TypeString,
@@ -71,12 +67,11 @@ func Provider() *schema.Provider {
 				DefaultFunc: schema.EnvDefaultFunc("REDSHIFT_DATABASE", "redshift"),
 			},
 			"max_connections": {
-				Type:          schema.TypeInt,
-				Optional:      true,
-				Default:       defaultProviderMaxOpenConnections,
-				Description:   "Maximum number of connections to establish to the database. Zero means unlimited.",
-				ValidateFunc:  validation.IntAtLeast(-1),
-				ConflictsWith: []string{"data_api"},
+				Type:         schema.TypeInt,
+				Optional:     true,
+				Default:      defaultProviderMaxOpenConnections,
+				Description:  "Maximum number of connections to establish to the database. Zero means unlimited.",
+				ValidateFunc: validation.IntAtLeast(-1),
 			},
 			"data_api": {
 				Type:        schema.TypeList,
@@ -85,12 +80,7 @@ func Provider() *schema.Provider {
 				MaxItems:    1,
 				ConflictsWith: []string{
 					"host",
-					"username",
 					"password",
-					"port",
-					"sslmode",
-					"max_connections",
-					"temporary_credentials",
 				},
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{

redshift/provider_test.go

@@ -5,11 +5,14 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"regexp"
 	"strings"
 	"testing"
 
 	"github.com/aws/aws-sdk-go-v2/config"
 	"github.com/aws/aws-sdk-go-v2/service/sts"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 	redshiftdatasqldriver "github.com/mmichaelb/redshift-data-sql-driver"
@@ -23,7 +26,8 @@ var (
 func init() {
 	testAccProvider = Provider()
 	testAccProviders = map[string]func() (*schema.Provider, error){
-		"redshift": func() (*schema.Provider, error) { return testAccProvider, nil },
+		"redshift":   func() (*schema.Provider, error) { return testAccProvider, nil },
+		"testvalues": getTestValuesProvider,
 	}
 }
 
@@ -191,21 +195,6 @@ func Test_getConfigFromResourceData(t *testing.T) {
 			},
 			false,
 		},
-		{
-			"Data API config missing region",
-			args{
-				d: schema.TestResourceDataRaw(t, Provider().Schema, map[string]interface{}{
-					"database": "some-database",
-					"data_api": []interface{}{
-						map[string]interface{}{
-							"workgroup_name": "some-workgroup",
-						},
-					},
-				}),
-			},
-			nil,
-			true,
-		},
 		{
 			"PQ config",
 			args{
@@ -251,19 +240,6 @@ func Test_getConfigFromResourceData(t *testing.T) {
 			},
 			false,
 		},
-		{
-			"PQ config - missing host",
-			args{
-				d: schema.TestResourceDataRaw(t, Provider().Schema, map[string]interface{}{
-					"username": "some-user",
-					"port":     4122,
-					"database": "some-database",
-					"sslmode":  "require",
-				}),
-			},
-			nil,
-			true,
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -291,6 +267,63 @@ func Test_getConfigFromResourceData(t *testing.T) {
 	}
 }
 
+func TestAccProviderCalculatedValues_HostConfig(t *testing.T) {
+	testHostValue := generateRandomObjectName("tf_acc_calc_val_host")
+	providerConfig := fmt.Sprintf(`
+provider "redshift" {
+  host     = testvalues_value.calculated_host.result
+  password = "somepassword"
+}
+
+resource "testvalues_value" "calculated_host" {
+  value = %[1]q
+}
+`, testHostValue)
+	expectedError := fmt.Sprintf(`dial tcp: lookup %s: no such host`, testHostValue)
+	// no such host error should occur, not a missing attribute error
+	testCalculatedProviderValues(t, providerConfig, expectedError)
+}
+
+func TestAccProviderCalculatedValues_RedshiftDataConfig(t *testing.T) {
+	testWorkgroupValue := generateRandomObjectName("tf_acc_calc_val_host")
+	providerConfig := fmt.Sprintf(`
+provider "redshift" {
+  database = "somedb"
+
+  data_api {
+    workgroup_name = testvalues_value.calculated_workgroup.result
+    region         = "us-west-2"
+  }
+}
+
+resource "testvalues_value" "calculated_workgroup" {
+  value = %[1]q
+}
+`, testWorkgroupValue)
+	// redshift endpoint doesn't exist in this region error should occur, not a missing attribute error
+	expectedError := "ValidationException: Redshift endpoint doesn't exist in this region."
+	testCalculatedProviderValues(t, providerConfig, expectedError)
+}
+
+func testCalculatedProviderValues(t *testing.T, providerConfig string, expectedError string) {
+	defer unsetAndSetEnvVars("REDSHIFT_DATABASE", "REDSHIFT_HOST", "REDSHIFT_USER", "REDSHIFT_PASSWORD", "REDSHIFT_DATA_API_SERVERLESS_WORKGROUP_NAME")()
+	testDbName := generateRandomObjectName("tf_acc_calc_val_db")
+	testDbConfig := testAccDataSourceRedshiftDatabaseConfigBasic(testDbName)
+	cfg := fmt.Sprintf(`
+%[1]s
+%[2]s
+`, providerConfig, testDbConfig)
+	resource.ParallelTest(t, resource.TestCase{
+		ProviderFactories: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config:      cfg,
+				ExpectError: regexp.MustCompile(expectedError),
+			},
+		},
+	})
+}
+
 func unsetAndSetEnvVars(envName ...string) func() {
 	envValues := make(map[string]string)
 	for _, env := range envName {
@@ -308,3 +341,56 @@ func unsetAndSetEnvVars(envName ...string) func() {
 		}
 	}
 }
+
+type testValuesProvider struct {
+	testValues map[string]interface{}
+}
+
+func (p *testValuesProvider) getProvider() *schema.Provider {
+	return &schema.Provider{
+		ResourcesMap: map[string]*schema.Resource{
+			"testvalues_value": {
+				CreateContext: func(ctx context.Context, data *schema.ResourceData, i interface{}) diag.Diagnostics {
+					value := data.Get("value").(string)
+					data.Set("result", value)
+					data.SetId(value)
+					p.testValues[value] = &struct{}{}
+					return nil
+				},
+				DeleteContext: func(ctx context.Context, data *schema.ResourceData, i interface{}) diag.Diagnostics {
+					value := data.Get("value").(string)
+					delete(p.testValues, value)
+					data.SetId("")
+					return nil
+				},
+				ReadContext: func(ctx context.Context, data *schema.ResourceData, i interface{}) diag.Diagnostics {
+					value := data.Get("value").(string)
+
+					if _, ok := p.testValues[value]; !ok {
+						data.SetId("")
+						return nil
+					} else {
+						data.SetId(value)
+						data.Set("result", value)
+					}
+					return nil
+				},
+				Schema: map[string]*schema.Schema{
+					"value": {
+						Type:     schema.TypeString,
+						Required: true,
+						ForceNew: true,
+					},
+					"result": {
+						Type:     schema.TypeString,
+						Computed: true,
+					},
+				},
+			},
+		},
+	}
+}
+
+func getTestValuesProvider() (*schema.Provider, error) {
+	return (&testValuesProvider{testValues: make(map[string]interface{})}).getProvider(), nil
+}