diff --git a/.github/workflows/provider-build.yaml b/.github/workflows/provider-build.yaml index 15d4d27a..e3c53a43 100644 --- 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 index cbc06c77..e6d3ee40 100644 --- 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 index a1eba1f5..242511bd 100644 --- 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="" ### 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 index b24314b8..3347160f 100644 --- 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 index a1f6c17b..90d24884 100644 --- 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 index 8201448e..ae5e4b39 100644 --- 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 index 40116e71..e1320e95 100644 --- 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)), diff --git a/internal/testutils/provider.go b/internal/testutils/provider.go index d89b126b..de6fc8f7 100644 --- a/internal/testutils/provider.go +++ b/internal/testutils/provider.go @@ -1,26 +1,42 @@ package testutils import ( + "os" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/provider" "github.com/hashicorp/terraform-plugin-framework/providerserver" "github.com/hashicorp/terraform-plugin-go/tfprotov6" - "github.com/prefecthq/terraform-provider-prefect/internal/provider" + prefectProvider "github.com/prefecthq/terraform-provider-prefect/internal/provider" ) -const ( - // ProviderConfig can be imported into each acceptance test case - // to initialize a provider configuration to be used in each assertion. - // dynamically inject configurations once provider.go is fixed - // https://github.com/PrefectHQ/terraform-provider-prefect/issues/67 - ProviderConfig = ` -provider "prefect" {} -` -) +// TestAccPrefix is the prefix set for all resources created via acceptance testing, +// so that we can easily identify and clean them up in case of flakiness/failures. +const TestAccPrefix = "terraform_acc_" + +// TestAccProvider defines the actual Provider, which is used during acceptance testing. +// This is the same Provider that is used by the CLI, and is used by +// custom test functions, primarily to access the underlying HTTP client. +var TestAccProvider provider.Provider = prefectProvider.New() // TestAccProtoV6ProviderFactories are used to instantiate a provider during // acceptance testing. The factory function will be invoked for every Terraform // CLI command executed to create a provider server to which the CLI can // reattach. var TestAccProtoV6ProviderFactories = map[string]func() (tfprotov6.ProviderServer, error){ - "prefect": TestAccProvider, + "prefect": providerserver.NewProtocol6WithError(TestAccProvider), +} + +// AccTestPreCheck is a utility hook, which every test suite will call +// in order to verify if the necessary provider configurations are passed +// through the environment variables. +// https://developer.hashicorp.com/terraform/plugin/testing/acceptance-tests/testcase#precheck +func AccTestPreCheck(t *testing.T) { + t.Helper() + neededVars := []string{"PREFECT_API_URL", "PREFECT_API_KEY", "PREFECT_CLOUD_ACCOUNT_ID"} + for _, key := range neededVars { + if v := os.Getenv(key); v == "" { + t.Fatalf("%s must be set for acceptance tests", key) + } + } } -var TestAccProvider = providerserver.NewProtocol6WithError(provider.New())