chore: add foundation for automated Acceptance Testing (#71)

* fix provider configuration init * use provider values from env * add precheck * prefix * hook into ci/cd * yup * fix key name * lint * oops * words
PrefectHQ · Oct 25, 2023 · 7b5b68d · 7b5b68d
1 parent 6822a11
commit 7b5b68d
Show file tree

Hide file tree

Showing 8 changed files with 166 additions and 70 deletions.
diff --git a/.github/workflows/provider-build.yaml b/.github/workflows/provider-build.yaml
@@ -39,6 +39,29 @@ jobs:
       - name: Unit Test
         run: make test
 
+  testacc:
+    permissions:
+      contents: read
+    name: Acceptance Tests
+    environment: Acceptance Tests
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions/setup-go@v4
+        with:
+          go-version-file: 'go.mod'
+      - uses: hashicorp/setup-terraform@v2
+        with:
+          terraform_version: '1.5.*'
+          terraform_wrapper: false
+      - name: Install gotestsum
+        run: go install gotest.tools/gotestsum@latest
+      - run: make testacc
+        env:
+          PREFECT_API_URL: ${{ secrets.ACC_TEST_PREFECT_API_URL }}
+          PREFECT_API_KEY: ${{ secrets.ACC_TEST_PREFECT_API_KEY }}
+          PREFECT_CLOUD_ACCOUNT_ID: ${{ secrets.ACC_TEST_PREFECT_CLOUD_ACCOUNT_ID }}
+
   lint:
     permissions:
       contents: read

diff --git a/Makefile b/Makefile
@@ -31,9 +31,15 @@ lint:
 	golangci-lint run
 .PHONY: lint
 
-install: clean build 
+install: clean build
 	echo "@TODO Placeholder install - move built provider to ~.terraform.d/plugins/"
 
 test:
 	gotestsum --max-fails=50 ./...
 .PHONY: test
+
+# NOTE: Acceptance Tests create real infrastructure
+# against a dedicated testing account
+testacc:
+	TF_ACC=1 make test
+.PHONY: testacc
diff --git a/README.md b/README.md
@@ -20,19 +20,19 @@ Terraform provider for [Prefect 2](https://github.com/PrefectHQ/prefect) and [Pr
 * Workspace (`prefect_workspace`)
 
 ## Deployment:
-The "examples" folder makes use of this local provider.   
+The "examples" folder makes use of this local provider.
 By following the instructions below you can get it deployed to a target Prefect Cloud 2.0 account.
 
 ### 1. Create Prefect 2.0 Service Account
-The creation of a [service account](https://docs.prefect.io/ui/service-accounts/#create-a-service-account) generates a key that we will use to authenticate the terraform provider to Prefect Cloud. 
+The creation of a [service account](https://docs.prefect.io/ui/service-accounts/#create-a-service-account) generates a key that we will use to authenticate the terraform provider to Prefect Cloud.
 The advantage of a service account is that it is not tied to a user, but directly to the account.
 
 ### 2. Configure environment variables
 In this step we export 3 environment variables:
 
-PREFECT_API_URL = the Prefect Cloud API endpoint  
-PREFECT_API_KEY = the authentication key (generated as part of the previous step)  
-PREFECT_ACCOUNT_ID = the account id (by clicking on your organisation you'll see this in the URL)  
+PREFECT_API_URL = the Prefect Cloud API endpoint
+PREFECT_API_KEY = the authentication key (generated as part of the previous step)
+PREFECT_ACCOUNT_ID = the account id (by clicking on your organisation you'll see this in the URL)
 
 Run this on your terminal (replace placeholders):
 
@@ -44,8 +44,8 @@ export PREFECT_ACCOUNT_ID="<YOUR-ACCOUNT-ID>"
 ### 3. Build the provider
 This builds the providers's binary and move it to the Terraform plugins directory (usually under `~/.terraform.d/plugins/`)
 
-Before building the provider make sure you've set the correct CPU architecture of your machine.  
-E.g: for MAC M1, use `darwin_arm64`, for MAC Intel use `darwin_amd64`.  
+Before building the provider make sure you've set the correct CPU architecture of your machine.
+E.g: for MAC M1, use `darwin_arm64`, for MAC Intel use `darwin_amd64`.
 ```
 export CPU_ARCHITECTURE="darwin_arm64"
 ```
@@ -62,7 +62,7 @@ terraform init
 terraform apply
 ```
 
-In case of a success output, go to Prefect Cloud and find the workspace named `terraform-workspace`   
+In case of a success output, go to Prefect Cloud and find the workspace named `terraform-workspace`
 
 ### 5. Tear down the example infrastructure
 While the capability to _destroy_ work queues and blocks is in place, you won't be able to completely destroy the stack because the API endpoint to destroy a workspace hasn't been made available. For now you'll need to go to the Prefect Cloud UI, click on the workspace `terraform-workspace` > workspace settings > (hamburger button on the top right) > delete.
@@ -76,11 +76,11 @@ rm -rf examples/.terraform
 ```
 
 ## Notes & Improvements:
-* This is far from complete.   
-* There are no code _tests_ in place at present (adding them will very likely lead to changes to make the code more robust).  
-* The provider's implementation of `blocks` has been done generically. For a more granular infrastructure state control, consider changing the BlockDocument's `data` field to the target block type e.g: `s3`, `kubernetes job`. This would generate more work and duplication though.   
-* The GO Prefect 2.0 API (`prefect_api`) should be moved out of the provider, into its own project.  
-* Should authentication be handled with temporary tokens?  
+* This is far from complete.
+* There are no code _tests_ in place at present (adding them will very likely lead to changes to make the code more robust).
+* The provider's implementation of `blocks` has been done generically. For a more granular infrastructure state control, consider changing the BlockDocument's `data` field to the target block type e.g: `s3`, `kubernetes job`. This would generate more work and duplication though.
+* The GO Prefect 2.0 API (`prefect_api`) should be moved out of the provider, into its own project.
+* Should authentication be handled with temporary tokens?
 
 ## Development
 
@@ -116,4 +116,20 @@ provider_installation {
 }
 ```
 
+### Testing
+
+`make` commands are configured to invoke unit and acceptance tests
+
+```shell
+make test
+```
+
+In order to run the suite of Acceptance tests:
+
+```shell
+make testacc
+```
+
+**Note:** Acceptance tests create real Prefect Cloud resources, and require a Prefect Cloud account when running locally
+
 See [Development Overrides for Provider Developers](https://developer.hashicorp.com/terraform/cli/config/config-file#development-overrides-for-provider-developers) for details.
diff --git a/internal/provider/datasources/service_account_test.go b/internal/provider/datasources/service_account_test.go
@@ -14,13 +14,14 @@ import (
 func TestAccDatasource_service_account(t *testing.T) {
 	dataSourceName := "data.prefect_service_account.bot"
 	// generate random resource name
-	randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
+	randomName := testutils.TestAccPrefix + acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
 
 	resource.ParallelTest(t, resource.TestCase{
 		ProtoV6ProviderFactories: testutils.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { testutils.AccTestPreCheck(t) },
 		Steps: []resource.TestStep{
 			{
-				Config: testutils.ProviderConfig + fixtureAccServiceAccountDataSource(randomName),
+				Config: fixtureAccServiceAccountDataSource(randomName),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					// Check the prefect_service_account datasource
 					resource.TestCheckResourceAttr(dataSourceName, "name", randomName),

diff --git a/internal/provider/datasources/workspace_role_test.go b/internal/provider/datasources/workspace_role_test.go
@@ -20,9 +20,10 @@ func TestAccDatasource_workspace_role_defaults(t *testing.T) {
 
 	resource.ParallelTest(t, resource.TestCase{
 		ProtoV6ProviderFactories: testutils.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { testutils.AccTestPreCheck(t) },
 		Steps: []resource.TestStep{
 			{
-				Config: testutils.ProviderConfig + fixtureAccWorkspaceRoleDataSource(owner),
+				Config: fixtureAccWorkspaceRoleDataSource(owner),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr(dataSourceName, "name", owner),
 					resource.TestCheckResourceAttrSet(dataSourceName, "created"),
@@ -32,7 +33,7 @@ func TestAccDatasource_workspace_role_defaults(t *testing.T) {
 				),
 			},
 			{
-				Config: testutils.ProviderConfig + fixtureAccWorkspaceRoleDataSource(worker),
+				Config: fixtureAccWorkspaceRoleDataSource(worker),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr(dataSourceName, "name", worker),
 					resource.TestCheckResourceAttrSet(dataSourceName, "created"),
@@ -42,7 +43,7 @@ func TestAccDatasource_workspace_role_defaults(t *testing.T) {
 				),
 			},
 			{
-				Config: testutils.ProviderConfig + fixtureAccWorkspaceRoleDataSource(developer),
+				Config: fixtureAccWorkspaceRoleDataSource(developer),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr(dataSourceName, "name", developer),
 					resource.TestCheckResourceAttrSet(dataSourceName, "created"),
@@ -52,7 +53,7 @@ func TestAccDatasource_workspace_role_defaults(t *testing.T) {
 				),
 			},
 			{
-				Config: testutils.ProviderConfig + fixtureAccWorkspaceRoleDataSource(viewer),
+				Config: fixtureAccWorkspaceRoleDataSource(viewer),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr(dataSourceName, "name", viewer),
 					resource.TestCheckResourceAttrSet(dataSourceName, "created"),
@@ -62,7 +63,7 @@ func TestAccDatasource_workspace_role_defaults(t *testing.T) {
 				),
 			},
 			{
-				Config: testutils.ProviderConfig + fixtureAccWorkspaceRoleDataSource(runner),
+				Config: fixtureAccWorkspaceRoleDataSource(runner),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr(dataSourceName, "name", runner),
 					resource.TestCheckResourceAttrSet(dataSourceName, "created"),

diff --git a/internal/provider/provider.go b/internal/provider/provider.go
@@ -5,7 +5,9 @@ import (
 	"fmt"
 	"net/url"
 	"os"
+	"strings"
 
+	"github.com/google/uuid"
 	"github.com/hashicorp/terraform-plugin-framework/datasource"
 	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/provider"
@@ -69,7 +71,8 @@ func (p *PrefectProvider) Configure(ctx context.Context, req provider.ConfigureR
 		return
 	}
 
-	// Ensure that any values passed in to provider are known
+	// Ensure that all configuration values passed in to provider are known
+	// https://developer.hashicorp.com/terraform/plugin/framework/handling-data/terraform-concepts#unknown-values
 	if config.Endpoint.IsUnknown() {
 		resp.Diagnostics.AddAttributeError(
 			path.Root("endpoint"),
@@ -84,16 +87,7 @@ func (p *PrefectProvider) Configure(ctx context.Context, req provider.ConfigureR
 			path.Root("api_key"),
 			"Unknown Prefect API Key",
 			"The Prefect API Key is not known at configuration time. "+
-				"Potential resolutions: target apply the source of the value first, set the value statically in the configuration, set the PREFECT_API_URL environment variable, or remove the value.",
-		)
-	}
-
-	if config.APIKey.IsUnknown() {
-		resp.Diagnostics.AddAttributeError(
-			path.Root("api_key"),
-			"Unknown Prefect API Key",
-			"The Prefect API Key is not known at configuration time. "+
-				"Potential resolutions: target apply the source of the value first, set the value statically in the configuration, set the PREFECT_API_URL environment variable, or remove the value.",
+				"Potential resolutions: target apply the source of the value first, set the value statically in the configuration, set the PREFECT_API_KEY environment variable, or remove the value.",
 		)
 	}
 
@@ -119,25 +113,26 @@ func (p *PrefectProvider) Configure(ctx context.Context, req provider.ConfigureR
 		return
 	}
 
-	// Use provider values if supplied, otherwise fall back to environment variables
+	// Extract endpoint from configuration or environment variable.
+	// If the endpoint is not set, or the value is not a valid URL, emit an error.
 	var endpoint string
-	if !config.Endpoint.IsUnknown() && !config.Endpoint.IsNull() {
+	if !config.Endpoint.IsNull() {
 		endpoint = config.Endpoint.ValueString()
-	} else if u, ok := os.LookupEnv("PREFECT_API_URL"); ok {
-		endpoint = u
-	} else {
-		endpoint = "http://localhost:4200/api"
+	} else if apiURLEnvVar, ok := os.LookupEnv("PREFECT_API_URL"); ok {
+		endpoint = apiURLEnvVar
 	}
-
-	// Validate values (ensure that they are non-empty)
 	if endpoint == "" {
 		resp.Diagnostics.AddAttributeError(
 			path.Root("endpoint"),
 			"Missing Prefect API Endpoint",
 			"The Prefect API Endpoint is set to an empty value. "+
-				"Potential resolutions: set the endpoint attribute or PREFECT_API_URL environment variable to a non-empty value, or remove the value. "+fmt.Sprintf("endpoint %q unknown %t", endpoint, config.Endpoint.IsUnknown()),
+				"Potential resolutions: set the endpoint attribute or PREFECT_API_URL environment variable to a non-empty value, or remove the value.",
 		)
 	}
+	// Here, we'll ensure that the /api suffix is present on the endpoint
+	if !strings.HasSuffix(endpoint, "/api") {
+		endpoint = fmt.Sprintf("%s/api", endpoint)
+	}
 
 	endpointURL, err := url.Parse(endpoint)
 	if err != nil {
@@ -147,31 +142,68 @@ func (p *PrefectProvider) Configure(ctx context.Context, req provider.ConfigureR
 			fmt.Sprintf("The Prefect API Endpoint %q is not a valid URL: %s", endpoint, err),
 		)
 	}
+	isPrefectCloudEndpoint := endpointURL.Host == "api.prefect.cloud" || endpointURL.Host == "api.prefect.dev" || endpointURL.Host == "api.stg.prefect.dev"
 
+	// Extract the API Key from configuration or environment variable.
 	var apiKey string
-	if !config.APIKey.IsUnknown() {
+	if !config.APIKey.IsNull() {
 		apiKey = config.APIKey.ValueString()
-	} else if key, ok := os.LookupEnv("PREFECT_API_KEY"); ok {
-		apiKey = key
+	} else if apiKeyEnvVar, ok := os.LookupEnv("PREFECT_API_KEY"); ok {
+		apiKey = apiKeyEnvVar
 	}
 
-	// If API Key is unset, check that we're running against Prefect Cloud
-	if endpointURL.Host == "api.prefect.cloud" || endpointURL.Host == "api.prefect.dev" || endpointURL.Host == "api.stg.prefect.dev" {
-		if apiKey == "" {
+	// Extract the Account ID from configuration or environment variable.
+	// If the ID is set to an invalid UUID, emit an error.
+	var accountID uuid.UUID
+	if !config.AccountID.IsNull() {
+		accountID = config.AccountID.ValueUUID()
+	} else if accountIDEnvVar, ok := os.LookupEnv("PREFECT_CLOUD_ACCOUNT_ID"); ok {
+		accountID, err = uuid.Parse(accountIDEnvVar)
+		if err != nil {
 			resp.Diagnostics.AddAttributeWarning(
+				path.Root("account_id"),
+				"Invalid Prefect Account ID defined in PREFECT_CLOUD_ACCOUNT_ID ",
+				fmt.Sprintf("The PREFECT_CLOUD_ACCOUNT_ID value %q is not a valid UUID: %s", accountIDEnvVar, err),
+			)
+		}
+	}
+
+	// If the endpoint is pointed to Prefect Cloud, we will ensure
+	// that a valid API Key is passed.
+	// Additionally, we will warn if an Account ID is missing,
+	// as it's likely that this is a user misconfiguration.
+	if isPrefectCloudEndpoint {
+		if apiKey == "" {
+			resp.Diagnostics.AddAttributeError(
 				path.Root("api_key"),
 				"Missing Prefect API Key",
 				"The Prefect API Endpoint is configured to Prefect Cloud, however, the Prefect API Key is empty. "+
 					"Potential resolutions: set the endpoint attribute or PREFECT_API_URL environment variable to a Prefect server installation, set the PREFECT_API_KEY environment variable, or configure the api_key attribute.",
 			)
 		}
-	} else if apiKey != "" {
-		resp.Diagnostics.AddAttributeWarning(
-			path.Root("api_key"),
-			"Non-Empty Prefect API Key",
-			"The Prefect API Key is set, however, the Endpoint is set to a Prefect server installation. "+
-				"Potential resolutions: set the endpoint attribute or PREFECT_API_URL environment variable to a Prefect Cloud endpoint, unset the PREFECT_API_KEY environment variable, or remove the api_key attribute.",
-		)
+
+		if accountID == uuid.Nil {
+			resp.Diagnostics.AddAttributeWarning(
+				path.Root("account_id"),
+				"Missing Prefect Account ID",
+				"The Prefect API Endpoint is configured to Prefect Cloud, however, the Prefect Account ID is empty. "+
+					"Potential resolutions: set the PREFECT_CLOUD_ACCOUNT_ID environment variable, or configure the account_id attribute.",
+			)
+		}
+	}
+
+	// If the endpoint is pointed to a self-hosted Prefect Server installation,
+	// we will warn the practitioner if an API Key is set, as it's possible that
+	// this is a user misconfiguration.
+	if !isPrefectCloudEndpoint {
+		if apiKey != "" {
+			resp.Diagnostics.AddAttributeWarning(
+				path.Root("api_key"),
+				"Prefect API Key ",
+				"The Prefect API Key is set, however, the Endpoint is set to a Prefect server installation. "+
+					"Potential resolutions: set the endpoint attribute or PREFECT_API_URL environment variable to a Prefect Cloud endpoint, unset the PREFECT_API_KEY environment variable, or remove the api_key attribute.",
+			)
+		}
 	}
 
 	if resp.Diagnostics.HasError() {
@@ -181,7 +213,7 @@ func (p *PrefectProvider) Configure(ctx context.Context, req provider.ConfigureR
 	prefectClient, err := client.New(
 		client.WithEndpoint(endpoint),
 		client.WithAPIKey(apiKey),
-		client.WithDefaults(config.AccountID.ValueUUID(), config.WorkspaceID.ValueUUID()),
+		client.WithDefaults(accountID, config.WorkspaceID.ValueUUID()),
 	)
 	if err != nil {
 		resp.Diagnostics.AddError(

diff --git a/internal/provider/resources/workspace_role_test.go b/internal/provider/resources/workspace_role_test.go
@@ -12,13 +12,14 @@ import (
 //nolint:paralleltest // we use the resource.ParallelTest helper instead
 func TestAccResource_workspace_role(t *testing.T) {
 	resourceName := "prefect_workspace_role.role"
-	randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
+	randomName := testutils.TestAccPrefix + acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
 	resource.ParallelTest(t, resource.TestCase{
 		ProtoV6ProviderFactories: testutils.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { testutils.AccTestPreCheck(t) },
 		Steps: []resource.TestStep{
 			{
 				// Check creation + existence of the workspace role resource
-				Config: testutils.ProviderConfig + fixtureAccWorkspaceRoleResource(randomName),
+				Config: fixtureAccWorkspaceRoleResource(randomName),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceName, "name", randomName),
 					resource.TestCheckResourceAttr(resourceName, "description", fmt.Sprintf("%s description", randomName)),
@@ -29,7 +30,7 @@ func TestAccResource_workspace_role(t *testing.T) {
 			},
 			{
 				// Check updates for the workspace role resource
-				Config: testutils.ProviderConfig + fixtureAccWorkspaceRoleReesourceUpdated(randomName),
+				Config: fixtureAccWorkspaceRoleReesourceUpdated(randomName),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceName, "name", randomName),
 					resource.TestCheckResourceAttr(resourceName, "description", fmt.Sprintf("description for %s", randomName)),