From b0c239d5bee354eedc8dd38ac1983367294a0f18 Mon Sep 17 00:00:00 2001 From: Vandy Liu <33995460+vandyliu@users.noreply.github.com> Date: Wed, 5 Jun 2024 18:05:37 -0700 Subject: [PATCH] Fix hybrid cluster workspace authorization tests and doc fixes/updates (#84) --- .github/workflows/testacc.yml | 8 ++-- README.md | 4 +- docs/resources/cluster.md | 2 +- docs/resources/deployment.md | 2 +- .../hybrid_cluster_workspace_authorization.md | 10 +++++ docs/resources/team_roles.md | 16 ++++++++ docs/resources/workspace.md | 2 +- examples/resources/astro_cluster/resource.tf | 2 +- .../resources/astro_deployment/resource.tf | 2 +- .../resource.tf | 10 +++++ .../resources/astro_team_roles/resource.tf | 16 ++++++++ .../resources/astro_workspace/resource.tf | 2 +- .../scenarios/workspace_per_environment.tf | 2 +- examples/scenarios/workspace_per_team.tf | 2 +- .../workspace_per_team_per_environment.tf | 2 +- internal/provider/provider_test_utils.go | 41 +++++++------------ .../resources/resource_deployment_test.go | 16 +++++--- ...id_cluster_workspace_authorization_test.go | 29 +++---------- 18 files changed, 97 insertions(+), 71 deletions(-) diff --git a/.github/workflows/testacc.yml b/.github/workflows/testacc.yml index fd64c1eb..f2fe7c65 100644 --- a/.github/workflows/testacc.yml +++ b/.github/workflows/testacc.yml @@ -4,8 +4,6 @@ name: Acceptance Tests # since creation of deployments and clusters are costly on: pull_request: - paths-ignore: - - 'README.md' push: branches: - main @@ -87,11 +85,11 @@ jobs: HOSTED_ORGANIZATION_API_TOKEN: ${{ secrets.DEV_HOSTED_ORGANIZATION_API_TOKEN }} HOSTED_ORGANIZATION_ID: clczfgzc3001o01kjlphpi6hv HYBRID_CLUSTER_ID: clnp86ly5000401ndagu20g81 + HYBRID_DRY_RUN_CLUSTER_ID: clx2amddm002p01lcni4zbpgf HYBRID_NODE_POOL_ID: clnp86ly5000301ndzfxz895w ASTRO_API_HOST: https://api.astronomer-dev.io SKIP_CLUSTER_RESOURCE_TESTS: ${{ env.SKIP_CLUSTER_RESOURCE_TESTS }} HOSTED_TEAM_ID: clwbclrc100bl01ozjj5s4jmq - HYBRID_WORKSPACE_IDS: cl70oe7cu445571iynrkthtybl,cl8wpve4993871i37qe1k152c TESTARGS: "-failfast" run: make testacc @@ -130,10 +128,10 @@ jobs: HOSTED_ORGANIZATION_API_TOKEN: ${{ secrets.STAGE_HOSTED_ORGANIZATION_API_TOKEN }} HOSTED_ORGANIZATION_ID: clczfo8a111cz0t140nck8ubn HYBRID_CLUSTER_ID: clqqongl40fmv01m96dvxh2nu + HYBRID_DRY_RUN_CLUSTER_ID: clx2b2iag0b6301nbg6ox1wd8 HYBRID_NODE_POOL_ID: clqqongl40fmu01m94pwp4kct ASTRO_API_HOST: https://api.astronomer-stage.io HOSTED_TEAM_ID: clwv0r0x7091n01l0t1fm4vxy - HYBRID_WORKSPACE_IDS: clwv06sva08vg01hovu1j7znw TESTARGS: "-failfast" run: make testacc @@ -172,9 +170,9 @@ jobs: HOSTED_ORGANIZATION_API_TOKEN: ${{ secrets.DEV_HOSTED_ORGANIZATION_API_TOKEN }} HOSTED_ORGANIZATION_ID: clczfgzc3001o01kjlphpi6hv HYBRID_CLUSTER_ID: clnp86ly5000401ndagu20g81 + HYBRID_DRY_RUN_CLUSTER_ID: clx2amddm002p01lcni4zbpgf HYBRID_NODE_POOL_ID: clnp86ly5000301ndzfxz895w ASTRO_API_HOST: https://api.astronomer-dev.io HOSTED_TEAM_ID: clwbclrc100bl01ozjj5s4jmq - HYBRID_WORKSPACE_IDS: cl70oe7cu445571iynrkthtybl,cl8wpve4993871i37qe1k152c TESTARGS: "-failfast" run: make testacc \ No newline at end of file diff --git a/README.md b/README.md index 3bb44326..3efeb32f 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ Then commit the changes to `go.mod` and `go.sum`. terraform { required_providers { astro = { - source = "registry.terraform.io/astronomer/astro" + source = "astronomer/astro" } } } @@ -82,7 +82,7 @@ provider_installation { terraform { required_providers { astro = { - source = "registry.terraform.io/astronomer/astro" + source = "astronomer/astro" } } } diff --git a/docs/resources/cluster.md b/docs/resources/cluster.md index 9f9c2f63..1e68b8f4 100644 --- a/docs/resources/cluster.md +++ b/docs/resources/cluster.md @@ -52,7 +52,7 @@ resource "astro_cluster" "gcp_example" { } // Import an existing cluster -import = { +import { id = "clozc036j01to01jrlgvuf98d" // ID of the existing cluster to = astro_cluster.imported_cluster } diff --git a/docs/resources/deployment.md b/docs/resources/deployment.md index 918013ce..5d5656ae 100644 --- a/docs/resources/deployment.md +++ b/docs/resources/deployment.md @@ -90,7 +90,7 @@ resource "astro_deployment" "hybrid" { } // Import an existing deployment -import = { +import { id = "clv17vgft000801kkydsws63x" // ID of the existing deployment to = astro_deployment.imported_deployment } diff --git a/docs/resources/hybrid_cluster_workspace_authorization.md b/docs/resources/hybrid_cluster_workspace_authorization.md index 3055a7fd..2afecccb 100644 --- a/docs/resources/hybrid_cluster_workspace_authorization.md +++ b/docs/resources/hybrid_cluster_workspace_authorization.md @@ -17,6 +17,16 @@ resource "astro_hybrid_cluster_workspace_authorization" "example" { cluster_id = "clk8h0fv1006801j8yysfybbt" workspace_ids = ["cl70oe7cu445571iynrkthtybl", "cl70oe7cu445571iynrkthacsd"] } + +// Import existing hybrid cluster workspace authorization +import { + id = "clk8h0fv1006801j8yysfybbt" // ID of the existing hybrid cluster + to = astro_hybrid_cluster_workspace_authorization.imported_cluster_workspace_authorization +} +resource "astro_hybrid_cluster_workspace_authorization" "imported_cluster_workspace_authorization" { + cluster_id = "clk8h0fv1006801j8yysfybbt" + workspace_ids = ["cl70oe7cu445571iynrkthtybl"] +} ``` diff --git a/docs/resources/team_roles.md b/docs/resources/team_roles.md index 9c8d502b..27779e06 100644 --- a/docs/resources/team_roles.md +++ b/docs/resources/team_roles.md @@ -64,6 +64,22 @@ resource "astro_team_roles" "all_roles" { } ] } + +// Import existing team roles +import { + id = "clnp86ly5000401ndaga21g81" // ID of the existing team + to = astro_team_roles.imported_team_roles +} +resource "astro_team_roles" "imported_team_roles" { + team_id = "clnp86ly5000401ndaga21g81" + organization_role = "ORGANIZATION_MEMBER" + workspace_roles = [ + { + workspace_id = "clwp86ly5000401ndaga21g85" + role = "WORKSPACE_OWNER" + } + ] +} ``` diff --git a/docs/resources/workspace.md b/docs/resources/workspace.md index ac2ea4c0..2ec1479f 100644 --- a/docs/resources/workspace.md +++ b/docs/resources/workspace.md @@ -20,7 +20,7 @@ resource "astro_workspace" "example" { } // Import an existing workspace -import = { +import { id = "clozc036j01to01jrlgvu798d" // ID of the existing workspace to = astro_workspace.imported_workspace } diff --git a/examples/resources/astro_cluster/resource.tf b/examples/resources/astro_cluster/resource.tf index 55ee6fc3..4a896f6d 100644 --- a/examples/resources/astro_cluster/resource.tf +++ b/examples/resources/astro_cluster/resource.tf @@ -37,7 +37,7 @@ resource "astro_cluster" "gcp_example" { } // Import an existing cluster -import = { +import { id = "clozc036j01to01jrlgvuf98d" // ID of the existing cluster to = astro_cluster.imported_cluster } diff --git a/examples/resources/astro_deployment/resource.tf b/examples/resources/astro_deployment/resource.tf index d613b350..b475c4c7 100644 --- a/examples/resources/astro_deployment/resource.tf +++ b/examples/resources/astro_deployment/resource.tf @@ -75,7 +75,7 @@ resource "astro_deployment" "hybrid" { } // Import an existing deployment -import = { +import { id = "clv17vgft000801kkydsws63x" // ID of the existing deployment to = astro_deployment.imported_deployment } diff --git a/examples/resources/astro_hybrid_cluster_workspace_authorization/resource.tf b/examples/resources/astro_hybrid_cluster_workspace_authorization/resource.tf index 6125f72c..292d4b95 100644 --- a/examples/resources/astro_hybrid_cluster_workspace_authorization/resource.tf +++ b/examples/resources/astro_hybrid_cluster_workspace_authorization/resource.tf @@ -1,4 +1,14 @@ resource "astro_hybrid_cluster_workspace_authorization" "example" { cluster_id = "clk8h0fv1006801j8yysfybbt" workspace_ids = ["cl70oe7cu445571iynrkthtybl", "cl70oe7cu445571iynrkthacsd"] +} + +// Import existing hybrid cluster workspace authorization +import { + id = "clk8h0fv1006801j8yysfybbt" // ID of the existing hybrid cluster + to = astro_hybrid_cluster_workspace_authorization.imported_cluster_workspace_authorization +} +resource "astro_hybrid_cluster_workspace_authorization" "imported_cluster_workspace_authorization" { + cluster_id = "clk8h0fv1006801j8yysfybbt" + workspace_ids = ["cl70oe7cu445571iynrkthtybl"] } \ No newline at end of file diff --git a/examples/resources/astro_team_roles/resource.tf b/examples/resources/astro_team_roles/resource.tf index 9165fae0..1ba95fd5 100644 --- a/examples/resources/astro_team_roles/resource.tf +++ b/examples/resources/astro_team_roles/resource.tf @@ -48,4 +48,20 @@ resource "astro_team_roles" "all_roles" { role = "my custom role" } ] +} + +// Import existing team roles +import { + id = "clnp86ly5000401ndaga21g81" // ID of the existing team + to = astro_team_roles.imported_team_roles +} +resource "astro_team_roles" "imported_team_roles" { + team_id = "clnp86ly5000401ndaga21g81" + organization_role = "ORGANIZATION_MEMBER" + workspace_roles = [ + { + workspace_id = "clwp86ly5000401ndaga21g85" + role = "WORKSPACE_OWNER" + } + ] } \ No newline at end of file diff --git a/examples/resources/astro_workspace/resource.tf b/examples/resources/astro_workspace/resource.tf index 0dae6df3..92733789 100644 --- a/examples/resources/astro_workspace/resource.tf +++ b/examples/resources/astro_workspace/resource.tf @@ -5,7 +5,7 @@ resource "astro_workspace" "example" { } // Import an existing workspace -import = { +import { id = "clozc036j01to01jrlgvu798d" // ID of the existing workspace to = astro_workspace.imported_workspace } diff --git a/examples/scenarios/workspace_per_environment.tf b/examples/scenarios/workspace_per_environment.tf index 8123d61f..0f5756c8 100644 --- a/examples/scenarios/workspace_per_environment.tf +++ b/examples/scenarios/workspace_per_environment.tf @@ -19,7 +19,7 @@ Prod Workspace terraform { required_providers { astro = { - source = "registry.terraform.io/astronomer/astro" + source = "astronomer/astro" } } } diff --git a/examples/scenarios/workspace_per_team.tf b/examples/scenarios/workspace_per_team.tf index b5d84bca..d95483e1 100644 --- a/examples/scenarios/workspace_per_team.tf +++ b/examples/scenarios/workspace_per_team.tf @@ -19,7 +19,7 @@ Team 3 terraform { required_providers { astro = { - source = "registry.terraform.io/astronomer/astro" + source = "astronomer/astro" } } } diff --git a/examples/scenarios/workspace_per_team_per_environment.tf b/examples/scenarios/workspace_per_team_per_environment.tf index fb437f20..98ef85f9 100644 --- a/examples/scenarios/workspace_per_team_per_environment.tf +++ b/examples/scenarios/workspace_per_team_per_environment.tf @@ -22,7 +22,7 @@ Team 3 workspace prod terraform { required_providers { astro = { - source = "registry.terraform.io/astronomer/astro" + source = "astronomer/astro" } } } diff --git a/internal/provider/provider_test_utils.go b/internal/provider/provider_test_utils.go index a4b2fcb4..89bb05e0 100644 --- a/internal/provider/provider_test_utils.go +++ b/internal/provider/provider_test_utils.go @@ -23,32 +23,21 @@ func TestAccPreCheck(t *testing.T) { // about the appropriate environment variables being set are common to see in a pre-check // function. var missingEnvVars []string - if hostedToken := os.Getenv("HOSTED_ORGANIZATION_API_TOKEN"); len(hostedToken) == 0 { - missingEnvVars = append(missingEnvVars, "HOSTED_ORGANIZATION_API_TOKEN") - } - if hostedOrgId := os.Getenv("HOSTED_ORGANIZATION_ID"); len(hostedOrgId) == 0 { - missingEnvVars = append(missingEnvVars, "HOSTED_ORGANIZATION_ID") - } - if hybridToken := os.Getenv("HYBRID_ORGANIZATION_API_TOKEN"); len(hybridToken) == 0 { - missingEnvVars = append(missingEnvVars, "HYBRID_ORGANIZATION_API_TOKEN") - } - if hybridOrgId := os.Getenv("HYBRID_ORGANIZATION_ID"); len(hybridOrgId) == 0 { - missingEnvVars = append(missingEnvVars, "HYBRID_ORGANIZATION_ID") - } - if hybridWorkspaceIds := os.Getenv("HYBRID_WORKSPACE_IDS"); len(hybridWorkspaceIds) == 0 { - missingEnvVars = append(missingEnvVars, "HYBRID_WORKSPACE_IDS") - } - if host := os.Getenv("ASTRO_API_HOST"); len(host) == 0 { - missingEnvVars = append(missingEnvVars, "ASTRO_API_HOST") - } - if hybridClusterId := os.Getenv("HYBRID_CLUSTER_ID"); len(hybridClusterId) == 0 { - missingEnvVars = append(missingEnvVars, "HYBRID_CLUSTER_ID") - } - if hybridNodePoolId := os.Getenv("HYBRID_NODE_POOL_ID"); len(hybridNodePoolId) == 0 { - missingEnvVars = append(missingEnvVars, "HYBRID_NODE_POOL_ID") - } - if hostedTeamId := os.Getenv("HOSTED_TEAM_ID"); len(hostedTeamId) == 0 { - missingEnvVars = append(missingEnvVars, "HOSTED_TEAM_ID") + envVars := []string{ + "HOSTED_ORGANIZATION_API_TOKEN", + "HOSTED_ORGANIZATION_ID", + "HYBRID_ORGANIZATION_API_TOKEN", + "HYBRID_ORGANIZATION_ID", + "HYBRID_DRY_RUN_CLUSTER_ID", + "ASTRO_API_HOST", + "HYBRID_CLUSTER_ID", + "HYBRID_NODE_POOL_ID", + "HOSTED_TEAM_ID", + } + for _, envVar := range envVars { + if val := os.Getenv(envVar); len(val) == 0 { + missingEnvVars = append(missingEnvVars, envVar) + } } if len(missingEnvVars) > 0 { t.Fatalf("Pre-check failed: %+v must be set for acceptance tests", strings.Join(missingEnvVars, ", ")) diff --git a/internal/provider/resources/resource_deployment_test.go b/internal/provider/resources/resource_deployment_test.go index 291b22e2..4ad0b256 100644 --- a/internal/provider/resources/resource_deployment_test.go +++ b/internal/provider/resources/resource_deployment_test.go @@ -500,7 +500,7 @@ func TestAcc_ResourceDeploymentStandardRemovedOutsideOfTerraform(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(standardDeploymentResource, "name", standardDeploymentName), resource.TestCheckResourceAttr(standardDeploymentResource, "description", utils.TestResourceDescription), - // Check via API that workspace exists + // Check via API that deployment exists testAccCheckDeploymentExistence(t, standardDeploymentName, true, true), ), }, @@ -516,7 +516,7 @@ func TestAcc_ResourceDeploymentStandardRemovedOutsideOfTerraform(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(standardDeploymentResource, "name", standardDeploymentName), resource.TestCheckResourceAttr(standardDeploymentResource, "description", utils.TestResourceDescription), - // Check via API that workspace exists + // Check via API that deployment exists testAccCheckDeploymentExistence(t, standardDeploymentName, true, true), ), }, @@ -575,8 +575,12 @@ func hybridDeployment(input hybridDeploymentInput) string { taskPodNodePoolIdStr = fmt.Sprintf(`task_pod_node_pool_id = "%v"`, input.NodePoolId) } - workspaceId := strings.Split(os.Getenv("HYBRID_WORKSPACE_IDS"), ",")[0] return fmt.Sprintf(` +resource "astro_workspace" "%v_workspace" { + name = "%s" + description = "%s" + cicd_enforced_default = true +} resource "astro_deployment" "%v" { name = "%s" description = "%s" @@ -588,13 +592,15 @@ resource "astro_deployment" "%v" { is_dag_deploy_enabled = true scheduler_au = %v scheduler_replicas = 1 - workspace_id = "%v" + workspace_id = astro_workspace.%v_workspace.id %v %v %v } `, - input.Name, input.Name, utils.TestResourceDescription, input.ClusterId, input.Executor, input.SchedulerAu, workspaceId, + input.Name, input.Name, utils.TestResourceDescription, + input.Name, input.Name, utils.TestResourceDescription, + input.ClusterId, input.Executor, input.SchedulerAu, input.Name, envVarsStr(input.IncludeEnvironmentVariables), wqStr, taskPodNodePoolIdStr) } diff --git a/internal/provider/resources/resource_hybrid_cluster_workspace_authorization_test.go b/internal/provider/resources/resource_hybrid_cluster_workspace_authorization_test.go index c73dd173..e689be12 100644 --- a/internal/provider/resources/resource_hybrid_cluster_workspace_authorization_test.go +++ b/internal/provider/resources/resource_hybrid_cluster_workspace_authorization_test.go @@ -3,7 +3,6 @@ package resources_test import ( "context" "fmt" - "strconv" "strings" "github.com/samber/lo" @@ -26,10 +25,8 @@ func TestAcc_ResourceHybridClusterWorkspaceAuthorization(t *testing.T) { workspaceName := fmt.Sprintf("%v_workspace", namePrefix) workspaceResourceVar := fmt.Sprintf("astro_workspace.%v", workspaceName) - hybridWorkspaceIdsStr := os.Getenv("HYBRID_WORKSPACE_IDS") - hybridWorkspaceIds := strings.Split(hybridWorkspaceIdsStr, ",") - clusterId := os.Getenv("HYBRID_CLUSTER_ID") + clusterId := os.Getenv("HYBRID_DRY_RUN_CLUSTER_ID") clusterWorkspaceAuth := fmt.Sprintf("%v_auth", namePrefix) resourceVar := fmt.Sprintf("astro_hybrid_cluster_workspace_authorization.%v", clusterWorkspaceAuth) @@ -41,35 +38,19 @@ func TestAcc_ResourceHybridClusterWorkspaceAuthorization(t *testing.T) { testAccCheckHybridClusterWorkspaceAuthorizationExistence(t, clusterWorkspaceAuth, false), ), Steps: []resource.TestStep{ - // Test with existing workspaces and one created through terraform + // Test with workspace created through terraform { Config: astronomerprovider.ProviderConfig(t, false) + workspace(workspaceName, workspaceName, utils.TestResourceDescription, false) + hybridClusterWorkspaceAuthorization(hybridClusterWorkspaceAuthorizationInput{ Name: clusterWorkspaceAuth, ClusterId: clusterId, - WorkspaceIds: append(hybridWorkspaceIds, fmt.Sprintf("%v.id", workspaceResourceVar)), + WorkspaceIds: []string{fmt.Sprintf("%v.id", workspaceResourceVar)}, }), Check: resource.ComposeTestCheckFunc( // Check hybrid cluster workspace authorization resource.TestCheckResourceAttr(resourceVar, "cluster_id", clusterId), - resource.TestCheckResourceAttr(resourceVar, "workspace_ids.#", strconv.Itoa(len(hybridWorkspaceIds)+1)), - - testAccCheckHybridClusterWorkspaceAuthorizationExistence(t, clusterWorkspaceAuth, true), - ), - }, - // Remove terraform created workspace from cluster workspace authorization - { - Config: astronomerprovider.ProviderConfig(t, false) + - hybridClusterWorkspaceAuthorization(hybridClusterWorkspaceAuthorizationInput{ - Name: clusterWorkspaceAuth, - ClusterId: clusterId, - WorkspaceIds: hybridWorkspaceIds, - }), - Check: resource.ComposeTestCheckFunc( - // Check hybrid cluster workspace authorization - resource.TestCheckResourceAttr(resourceVar, "cluster_id", clusterId), - resource.TestCheckResourceAttr(resourceVar, "workspace_ids.#", strconv.Itoa(len(hybridWorkspaceIds))), + resource.TestCheckResourceAttr(resourceVar, "workspace_ids.#", "1"), testAccCheckHybridClusterWorkspaceAuthorizationExistence(t, clusterWorkspaceAuth, true), ), @@ -133,7 +114,7 @@ func testAccCheckHybridClusterWorkspaceAuthorizationExistence(t *testing.T, name assert.NoError(t, err) organizationId := os.Getenv("HYBRID_ORGANIZATION_ID") - clusterId := os.Getenv("HYBRID_CLUSTER_ID") + clusterId := os.Getenv("HYBRID_DRY_RUN_CLUSTER_ID") ctx := context.Background() resp, err := client.GetClusterWithResponse(ctx, organizationId, clusterId)