Remove hardcoded default for url_base (#33)

This change removes the hardcoded default for url_base of
http://localhost:8080 and performs validation on the url_base value.

Makefile

@@ -198,7 +198,7 @@ coverage-report:
 .PHONY: integration-tests
 integration-tests: $(TERRAFORM_BIN) $(KUBECTL_BIN) ## Start test environment, run acceptance tests, generate coverage report, and teardown test environment
 	$(MAKE) deploy-test-helper
-	KUBECONFIG=$(OUTPUT_DIR)/kubeconfig.yml $(MAKE) testacc
+	KUBECONFIG=$(OUTPUT_DIR)/kubeconfig.yml NUODB_CP_URL_BASE=http://localhost:8080 $(MAKE) testacc
 	$(MAKE) coverage-report
 	$(MAKE) undeploy-test-helper
 

docs/index.md

@@ -3,12 +3,12 @@
 page_title: "nuodbaas Provider"
 subcategory: ""
 description: |-
-  The NuoDB DBaaS provider provides the ability to manage the projects and databases running under the NuoDB Control Plane.
+  The NuoDB DBaaS Provider manages NuoDB databases using the NuoDB Control Plane.
 ---
 
 # nuodbaas Provider
 
-The NuoDB DBaaS provider provides the ability to manage the projects and databases running under the NuoDB Control Plane.
+The NuoDB DBaaS Provider manages NuoDB databases using the NuoDB Control Plane.
 
 ## Example Usage
 

internal/provider/provider.go

@@ -7,6 +7,7 @@ package provider
 
 import (
 	"context"
+	"net/url"
 	"os"
 
 	nuodbaas_client "github.com/nuodb/terraform-provider-nuodbaas/internal/client"
@@ -41,37 +42,39 @@ type NuoDbaasProviderModel struct {
 	Timeouts   map[string]framework.OperationTimeouts `tfsdk:"timeouts" hcl:"timeouts" cty:"timeouts"`
 }
 
+const (
+	NUODB_CP_USER        = "NUODB_CP_USER"
+	NUODB_CP_PASSWORD    = "NUODB_CP_PASSWORD"
+	NUODB_CP_URL_BASE    = "NUODB_CP_URL_BASE"
+	NUODB_CP_SKIP_VERIFY = "NUODB_CP_SKIP_VERIFY"
+)
+
 func (pm *NuoDbaasProviderModel) GetUser() string {
 	if pm.User != nil {
 		return *pm.User
 	}
-	return os.Getenv("NUODB_CP_USER")
+	return os.Getenv(NUODB_CP_USER)
 }
 
 func (pm *NuoDbaasProviderModel) GetPassword() string {
 	if pm.Password != nil {
 		return *pm.Password
 	}
-	return os.Getenv("NUODB_CP_PASSWORD")
+	return os.Getenv(NUODB_CP_PASSWORD)
 }
 
 func (pm *NuoDbaasProviderModel) GetUrlBase() string {
 	if pm.UrlBase != nil {
 		return *pm.UrlBase
 	}
-	if ret := os.Getenv("NUODB_CP_URL_BASE"); ret != "" {
-		return ret
-	}
-	// TODO(asz6): Consider removing this hardcoded default, which is
-	// unlikely to be useful in a non-test environment
-	return "http://localhost:8080"
+	return os.Getenv(NUODB_CP_URL_BASE)
 }
 
 func (pm *NuoDbaasProviderModel) GetSkipVerify() bool {
 	if pm.SkipVerify != nil {
 		return *pm.SkipVerify
 	}
-	return os.Getenv("NUODB_CP_SKIP_VERIFY") == "true"
+	return os.Getenv(NUODB_CP_SKIP_VERIFY) == "true"
 }
 
 func (pm *NuoDbaasProviderModel) CreateClient() (*openapi.Client, error) {
@@ -85,27 +88,27 @@ func (p *NuoDbaasProvider) Metadata(ctx context.Context, req provider.MetadataRe
 
 func (p *NuoDbaasProvider) Schema(ctx context.Context, req provider.SchemaRequest, resp *provider.SchemaResponse) {
 	resp.Schema = schema.Schema{
-		Description: "The NuoDB DBaaS provider provides the ability to manage the projects and databases running under the NuoDB Control Plane.",
+		Description: "The NuoDB DBaaS Provider manages NuoDB databases using the NuoDB Control Plane.",
 		Attributes: map[string]schema.Attribute{
 			"user": schema.StringAttribute{
 				Description: "The name of the user in the format `<organization>/<user>`. " +
-					"If not specified, defaults to the value of the `NUODB_CP_USER` environment variable.",
+					"If not specified, defaults to the value of the `" + NUODB_CP_USER + "` environment variable.",
 				Optional: true,
 			},
 			"password": schema.StringAttribute{
 				Description: "The password for the user. " +
-					"If not specified, defaults to the value of the `NUODB_CP_PASSWORD` environment variable.",
+					"If not specified, defaults to the value of the `" + NUODB_CP_PASSWORD + "` environment variable.",
 				Optional:  true,
 				Sensitive: true,
 			},
 			"url_base": schema.StringAttribute{
 				Description: "The base URL for the server, including the protocol. " +
-					"If not specified, defaults to the value of the `NUODB_CP_URL_BASE` environment variable.",
+					"If not specified, defaults to the value of the `" + NUODB_CP_URL_BASE + "` environment variable.",
 				Optional: true,
 			},
 			"skip_verify": schema.BoolAttribute{
 				Description: "Whether to skip server certificate verification. " +
-					"If not specified, defaults to the value of the `NUODB_CP_SKIP_VERIFY` environment variable.",
+					"If not specified, defaults to the value of the `" + NUODB_CP_SKIP_VERIFY + "` environment variable.",
 				Optional: true,
 			},
 			"timeouts": schema.MapNestedAttribute{
@@ -141,10 +144,28 @@ func (p *NuoDbaasProvider) Configure(ctx context.Context, req provider.Configure
 		return
 	}
 
+	// Validate server URL
+	if config.GetUrlBase() == "" {
+		resp.Diagnostics.AddError("Invalid provider configuration", "Must specify url_base or the environment variable "+NUODB_CP_URL_BASE)
+	} else {
+		url, err := url.Parse(config.GetUrlBase())
+		// url.Parse() does not return error if scheme is missing, so
+		// check that explicitly
+		if err != nil {
+			resp.Diagnostics.AddAttributeError(path.Empty().AtName("url_base"), "Invalid provider configuration", err.Error())
+		} else if url.Scheme == "" {
+			resp.Diagnostics.AddAttributeError(path.Empty().AtName("url_base"), "Invalid provider configuration", "No scheme found in URL")
+		}
+	}
+
 	// Validate timeout configuration
 	timeouts, err := framework.ParseTimeouts(config.Timeouts, resourceTypes())
 	if err != nil {
 		resp.Diagnostics.AddAttributeError(path.Empty().AtName("timeouts"), "Invalid provider configuration", err.Error())
+	}
+
+	// Check that no errors occurred
+	if resp.Diagnostics.HasError() {
 		return
 	}
 

internal/provider_test/integration_test.go

@@ -819,7 +819,7 @@ func TestNegative(t *testing.T) {
 			vars.providerCfg = NuoDbaasProviderModel{}
 		}()
 
-		// Configure invalid credentials and verify that read fail
+		// Configure invalid credentials and verify that reads fail
 		vars.providerCfg.User = ptr("org/user")
 		vars.providerCfg.Password = ptr("badpassword")
 		tf.WriteConfigT(t, vars.builder.Build())
@@ -829,20 +829,19 @@ func TestNegative(t *testing.T) {
 		require.Contains(t, string(out), "Unable to read projects")
 		require.Contains(t, string(out), "Unable to read databases")
 
-		// Configure invalid URL and verify that reads fail
-		badUrl := "http://badhost/"
-		vars.providerCfg.UrlBase = &badUrl
+		// Configure unreachable URL and verify that reads fail
+		vars.providerCfg.UrlBase = ptr("http://unreachable/")
 		tf.WriteConfigT(t, vars.builder.Build())
 		out, err = tf.Apply()
 		require.Error(t, err)
 		require.Contains(t, string(out), "Unable to read project")
 		require.Contains(t, string(out), "Unable to read projects")
 		require.Contains(t, string(out), "Unable to read databases")
+		vars.providerCfg.UrlBase = nil
 
 		// Specify bad timeout values
 		noSuffix := "999"
 		negative := "-1s"
-		vars.providerCfg.UrlBase = nil
 		vars.providerCfg.Timeouts = map[string]framework.OperationTimeouts{
 			"badresource": {},
 			"database":    {Create: &noSuffix, Update: &negative},
@@ -854,6 +853,30 @@ func TestNegative(t *testing.T) {
 		require.Contains(t, string(out), `missing unit in duration "999"`)
 		require.Contains(t, string(out), "Timeout for database update is negative: -1s")
 		require.Contains(t, string(out), "Invalid resource type: badresource")
+		vars.providerCfg.Timeouts = nil
+
+		// Temporarily override environment variable NUODB_CP_URL_BASE
+		// so that URL is not specified at all
+		urlBase := os.Getenv(NUODB_CP_URL_BASE)
+		defer os.Setenv(NUODB_CP_URL_BASE, urlBase)
+		os.Unsetenv(NUODB_CP_URL_BASE)
+		out, err = tf.Apply()
+		require.Error(t, err)
+		require.Contains(t, string(out), "Must specify url_base or the environment variable "+NUODB_CP_URL_BASE)
+
+		// Specify an invalid URL (bad port)
+		vars.providerCfg.UrlBase = ptr("http://host:-80")
+		tf.WriteConfigT(t, vars.builder.Build())
+		out, err = tf.Apply()
+		require.Error(t, err)
+		require.Contains(t, string(out), "invalid port")
+
+		// Specify an invalid URL (no scheme)
+		vars.providerCfg.UrlBase = ptr("badurl")
+		tf.WriteConfigT(t, vars.builder.Build())
+		out, err = tf.Apply()
+		require.Error(t, err)
+		require.Contains(t, string(out), "No scheme found in URL")
 	})
 
 	// Specify an invalid attribute and run `terraform apply`. This should