chore: error messaging for missing workspaceID + improved work pool i…

…mport parameter (#119)

* chore: clearer error messaging around missing workspaceID

* wow

* Generate Terraform Docs

* add import state test for work pools

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

docs/resources/work_pool.md

@@ -74,6 +74,9 @@ resource "prefect_work_pool" "example" {
 Import is supported using the following syntax:
 
 ```shell
-# Prefect Work Pools can be imported using the work pool's name
-terraform import prefect_work_pool.example name-of-work-pool
+# Prefect Work Pools can be imported using the format `workspace_id,name`
+terraform import prefect_work_pool.example 00000000-0000-0000-0000-000000000000,kubernetes-work-pool
+
+# You can also import by name only if you have a workspace_id set in your provider
+terraform import prefect_work_pool.example kubernetes-work-pool
 ```

examples/resources/prefect_work_pool/import.sh

@@ -1,2 +1,5 @@
-# Prefect Work Pools can be imported using the work pool's name
-terraform import prefect_work_pool.example name-of-work-pool
+# Prefect Work Pools can be imported using the format `workspace_id,name`
+terraform import prefect_work_pool.example 00000000-0000-0000-0000-000000000000,kubernetes-work-pool
+
+# You can also import by name only if you have a workspace_id set in your provider
+terraform import prefect_work_pool.example kubernetes-work-pool

internal/client/client.go

@@ -88,7 +88,7 @@ func WithAPIKey(apiKey string) Option {
 func WithDefaults(accountID uuid.UUID, workspaceID uuid.UUID) Option {
 	return func(client *Client) error {
 		if accountID == uuid.Nil && workspaceID != uuid.Nil {
-			return fmt.Errorf("accountID and workspaceID are inconsistent: accountID is nil and workspaceID is %q", workspaceID)
+			return fmt.Errorf("an accountID must be set if a workspaceID is set: accountID is %q and workspaceID is %q", accountID, workspaceID)
 		}
 
 		client.defaultAccountID = accountID

internal/client/variables.go

@@ -26,22 +26,17 @@ type VariablesClient struct {
 //
 //nolint:ireturn // required to support PrefectClient mocking
 func (c *Client) Variables(accountID uuid.UUID, workspaceID uuid.UUID) (api.VariablesClient, error) {
-	if accountID != uuid.Nil && workspaceID == uuid.Nil {
-		return nil, fmt.Errorf("accountID and workspaceID are inconsistent: accountID is %q and workspaceID is nil", accountID)
-	}
-
 	if accountID == uuid.Nil {
 		accountID = c.defaultAccountID
 	}
-
-	if accountID != uuid.Nil && workspaceID == uuid.Nil {
-		if c.defaultWorkspaceID == uuid.Nil {
-			return nil, fmt.Errorf("accountID and workspaceID are inconsistent: accountID is %q and supplied/default workspaceID are both nil", accountID)
-		}
-
+	if workspaceID == uuid.Nil {
 		workspaceID = c.defaultWorkspaceID
 	}
 
+	if accountID == uuid.Nil || workspaceID == uuid.Nil {
+		return nil, fmt.Errorf("both accountID and workspaceID must be set: accountID is %q and workspaceID is %q", accountID, workspaceID)
+	}
+
 	return &VariablesClient{
 		hc:          c.hc,
 		apiKey:      c.apiKey,

internal/client/work_pools.go

@@ -25,22 +25,17 @@ type WorkPoolsClient struct {
 //
 //nolint:ireturn // required to support PrefectClient mocking
 func (c *Client) WorkPools(accountID uuid.UUID, workspaceID uuid.UUID) (api.WorkPoolsClient, error) {
-	if accountID != uuid.Nil && workspaceID == uuid.Nil {
-		return nil, fmt.Errorf("accountID and workspaceID are inconsistent: accountID is %q and workspaceID is nil", accountID)
-	}
-
 	if accountID == uuid.Nil {
 		accountID = c.defaultAccountID
 	}
-
-	if accountID != uuid.Nil && workspaceID == uuid.Nil {
-		if c.defaultWorkspaceID == uuid.Nil {
-			return nil, fmt.Errorf("accountID and workspaceID are inconsistent: accountID is %q and supplied/default workspaceID are both nil", accountID)
-		}
-
+	if workspaceID == uuid.Nil {
 		workspaceID = c.defaultWorkspaceID
 	}
 
+	if accountID == uuid.Nil || workspaceID == uuid.Nil {
+		return nil, fmt.Errorf("both accountID and workspaceID must be set: accountID is %q and workspaceID is %q", accountID, workspaceID)
+	}
+
 	return &WorkPoolsClient{
 		hc:          c.hc,
 		apiKey:      c.apiKey,

internal/provider/resources/work_pool.go

@@ -430,5 +430,39 @@ func (r *WorkPoolResource) Delete(ctx context.Context, req resource.DeleteReques
 
 // ImportState imports the resource into Terraform state.
 func (r *WorkPoolResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) {
-	resource.ImportStatePassthroughID(ctx, path.Root("name"), req, resp)
+	// we'll allow input values in the form of:
+	// - "workspace_id,name"
+	// - "name"
+	maxInputCount := 2
+	inputParts := strings.Split(req.ID, ",")
+
+	// eg. "foo,bar,baz"
+	if len(inputParts) > maxInputCount {
+		resp.Diagnostics.AddError(
+			"Unexpected Import Identifier",
+			fmt.Sprintf("Expected a maximum of 2 import identifiers, in the form of `workspace_id,name`. Got %q", req.ID),
+		)
+
+		return
+	}
+
+	// eg. ",foo" or "foo,"
+	if len(inputParts) == maxInputCount && (inputParts[0] == "" || inputParts[1] == "") {
+		resp.Diagnostics.AddError(
+			"Unexpected Import Identifier",
+			fmt.Sprintf("Expected non-empty import identifiers, in the form of `workspace_id,name`. Got %q", req.ID),
+		)
+
+		return
+	}
+
+	if len(inputParts) == maxInputCount {
+		workspaceID := inputParts[0]
+		name := inputParts[1]
+
+		resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("workspace_id"), workspaceID)...)
+		resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("name"), name)...)
+	} else {
+		resource.ImportStatePassthroughID(ctx, path.Root("name"), req, resp)
+	}
 }

internal/provider/resources/work_pool_test.go

@@ -85,6 +85,13 @@ func TestAccResource_work_pool(t *testing.T) {
 					testAccCheckWorkPoolValues(&workPool, &api.WorkPool{Name: randomName2, Type: poolType2, IsPaused: false}),
 				),
 			},
+			// Import State checks - import by workspace_id,name (dynamic)
+			{
+				ImportState:       true,
+				ResourceName:      resourceName,
+				ImportStateIdFunc: getWorkPoolImportStateID(resourceName, workspaceDatsourceName),
+				ImportStateVerify: true,
+			},
 		},
 	})
 }
@@ -173,3 +180,21 @@ func testAccCheckIDsNotEqual(resourceName string, fetchedWorkPool *api.WorkPool)
 		return nil
 	}
 }
+
+func getWorkPoolImportStateID(workPoolResourceName string, workspaceDatsourceName string) resource.ImportStateIdFunc {
+	return func(state *terraform.State) (string, error) {
+		workspaceDatsource, exists := state.RootModule().Resources[workspaceDatsourceName]
+		if !exists {
+			return "", fmt.Errorf("Resource not found in state: %s", workspaceDatsourceName)
+		}
+		workspaceID, _ := uuid.Parse(workspaceDatsource.Primary.ID)
+
+		workPoolResource, exists := state.RootModule().Resources[workPoolResourceName]
+		if !exists {
+			return "", fmt.Errorf("Resource not found in state: %s", workPoolResourceName)
+		}
+		workPoolName := workPoolResource.Primary.Attributes["name"]
+
+		return fmt.Sprintf("%s,%s", workspaceID, workPoolName), nil
+	}
+}