Fix deployment env vars not working if is_secret=True (#60)

.github/workflows/testacc.yml

@@ -57,6 +57,7 @@ jobs:
       - name: Determine if expensive tests should run
         if: github.event_name == 'pull_request'
         run: |
+         git fetch --all
          echo "CHECKING FILES FOR SKIP LOGIC..."
          SKIP_TESTS="True"
          FILES_TO_CHECK=(
@@ -69,11 +70,12 @@ jobs:
            "internal/provider/resources/resource_deployment.go"
          )
          for file in "${FILES_TO_CHECK[@]}"; do
-           if git diff --name-only ${{ github.base_ref }} ${{ github.head_ref }} | grep -q "$file"; then
+           if git diff --name-only remotes/origin/${{ github.base_ref }} remotes/origin/${{ github.head_ref }} | grep -q "$file"; then
              SKIP_TESTS="False"
              break
            fi
          done
+         echo "SKIP_CLUSTER_RESOURCE_TESTS=$SKIP_TESTS"
          echo "SKIP_CLUSTER_RESOURCE_TESTS=$SKIP_TESTS" >> $GITHUB_ENV
       - env:
           TF_ACC: "1"

docs/resources/deployment.md

@@ -158,6 +158,9 @@ Required:
 
 - `is_secret` (Boolean) Whether Environment variable is a secret
 - `key` (String) Environment variable key
+
+Optional:
+
 - `value` (String, Sensitive) Environment variable value
 
 Read-Only:

internal/provider/models/deployment.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"time"
 
+	"github.com/samber/lo"
+
 	"github.com/astronomer/terraform-provider-astro/internal/clients/platform"
 	"github.com/astronomer/terraform-provider-astro/internal/provider/schemas"
 	"github.com/astronomer/terraform-provider-astro/internal/utils"
@@ -132,6 +134,7 @@ func (data *DeploymentResource) ReadFromResponse(
 	ctx context.Context,
 	deployment *platform.Deployment,
 	originalAstroRuntimeVersion *string,
+	requestEnvVars *[]platform.DeploymentEnvironmentVariableRequest,
 ) diag.Diagnostics {
 	// Read common fields
 	data.Id = types.StringValue(deployment.Id)
@@ -180,7 +183,30 @@ func (data *DeploymentResource) ReadFromResponse(
 	data.ImageTag = types.StringValue(deployment.ImageTag)
 	data.ImageRepository = types.StringValue(deployment.ImageRepository)
 	data.ImageVersion = types.StringPointerValue(deployment.ImageVersion)
-	data.EnvironmentVariables, diags = utils.ObjectSet(ctx, deployment.EnvironmentVariables, schemas.DeploymentEnvironmentVariableAttributeTypes(), DeploymentEnvironmentVariableTypesObject)
+
+	// Environment variables are a special case
+	// Since terraform wants to know the values of the secret values in the request at all times, and our API does not send back the secret values in the response
+	// We must use the request value and set it in the Terraform response to keep Terraform from emitting errors
+	// Since the value is marked as sensitive, Terraform will not output the actual value in the plan/apply output
+	envVars := *deployment.EnvironmentVariables
+	if requestEnvVars != nil && deployment.EnvironmentVariables != nil {
+		requestEnvVarsMap := lo.SliceToMap(*requestEnvVars, func(envVar platform.DeploymentEnvironmentVariableRequest) (string, platform.DeploymentEnvironmentVariable) {
+			return envVar.Key, platform.DeploymentEnvironmentVariable{
+				Key:      envVar.Key,
+				Value:    envVar.Value,
+				IsSecret: envVar.IsSecret,
+			}
+		})
+		for i, envVar := range envVars {
+			if envVar.IsSecret {
+				if requestEnvVar, ok := requestEnvVarsMap[envVar.Key]; ok {
+					// If the envVar has a secret value, update the value in the response
+					envVars[i].Value = requestEnvVar.Value
+				}
+			}
+		}
+	}
+	data.EnvironmentVariables, diags = utils.ObjectSet(ctx, &envVars, schemas.DeploymentEnvironmentVariableAttributeTypes(), DeploymentEnvironmentVariableTypesObject)
 	if diags.HasError() {
 		return diags
 	}
@@ -210,7 +236,7 @@ func (data *DeploymentResource) ReadFromResponse(
 	// Read hybrid deployment specific fields
 	data.TaskPodNodePoolId = types.StringPointerValue(deployment.TaskPodNodePoolId)
 
-	// Read hosted deployment specific fields
+	// Read hosted (standard and dedicated) deployment specific fields
 	data.ResourceQuotaCpu = types.StringPointerValue(deployment.ResourceQuotaCpu)
 	data.ResourceQuotaMemory = types.StringPointerValue(deployment.ResourceQuotaMemory)
 	data.DefaultTaskPodCpu = types.StringPointerValue(deployment.DefaultTaskPodCpu)
@@ -277,12 +303,12 @@ func (data *DeploymentDataSource) ReadFromResponse(
 		return diags
 	}
 	data.Executor = types.StringPointerValue((*string)(deployment.Executor))
+	data.SchedulerCpu = types.StringValue(deployment.SchedulerCpu)
+	data.SchedulerMemory = types.StringValue(deployment.SchedulerMemory)
 	if deployment.SchedulerAu != nil {
 		deploymentSchedulerAu := int64(*deployment.SchedulerAu)
 		data.SchedulerAu = types.Int64Value(deploymentSchedulerAu)
 	}
-	data.SchedulerCpu = types.StringValue(deployment.SchedulerCpu)
-	data.SchedulerMemory = types.StringValue(deployment.SchedulerMemory)
 	data.SchedulerReplicas = types.Int64Value(int64(deployment.SchedulerReplicas))
 	data.ImageTag = types.StringValue(deployment.ImageTag)
 	data.ImageRepository = types.StringValue(deployment.ImageRepository)
@@ -317,7 +343,7 @@ func (data *DeploymentDataSource) ReadFromResponse(
 	// Read hybrid deployment specific fields
 	data.TaskPodNodePoolId = types.StringPointerValue(deployment.TaskPodNodePoolId)
 
-	// Read hosted deployment specific fields
+	// Read hosted (standard and dedicated) deployment specific fields
 	data.ResourceQuotaCpu = types.StringPointerValue(deployment.ResourceQuotaCpu)
 	data.ResourceQuotaMemory = types.StringPointerValue(deployment.ResourceQuotaMemory)
 	data.DefaultTaskPodCpu = types.StringPointerValue(deployment.DefaultTaskPodCpu)

internal/provider/resources/resource_cluster.go

@@ -228,6 +228,7 @@ func (r *ClusterResource) Create(
 	readyCluster, err := stateConf.WaitForStateContext(ctx)
 	if err != nil {
 		resp.Diagnostics.AddError("Cluster creation failed", err.Error())
+		return
 	}
 
 	diags = data.ReadFromResponse(ctx, readyCluster.(*platform.Cluster))
@@ -381,6 +382,7 @@ func (r *ClusterResource) Update(
 	readyCluster, err := stateConf.WaitForStateContext(ctx)
 	if err != nil {
 		resp.Diagnostics.AddError("Cluster update failed", err.Error())
+		return
 	}
 
 	diags = data.ReadFromResponse(ctx, readyCluster.(*platform.Cluster))
@@ -451,6 +453,7 @@ func (r *ClusterResource) Delete(
 	_, err = stateConf.WaitForStateContext(ctx)
 	if err != nil {
 		resp.Diagnostics.AddError("Cluster deletion failed", err.Error())
+		return
 	}
 
 	tflog.Trace(ctx, fmt.Sprintf("deleted a cluster resource: %v", data.Id.ValueString()))

internal/provider/resources/resource_deployment.go

@@ -93,6 +93,7 @@ func (r *DeploymentResource) Create(
 
 	var diags diag.Diagnostics
 	var createDeploymentRequest platform.CreateDeploymentRequest
+	var envVars []platform.DeploymentEnvironmentVariableRequest
 
 	originalAstroRuntimeVersion := data.OriginalAstroRuntimeVersion.ValueString()
 	if len(originalAstroRuntimeVersion) == 0 {
@@ -136,7 +137,7 @@ func (r *DeploymentResource) Create(
 		}
 
 		// env vars
-		envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
+		envVars, diags = RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
 		if diags.HasError() {
 			resp.Diagnostics.Append(diags...)
 			return
@@ -196,7 +197,7 @@ func (r *DeploymentResource) Create(
 		}
 
 		// env vars
-		envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
+		envVars, diags = RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
 		if diags.HasError() {
 			resp.Diagnostics.Append(diags...)
 			return
@@ -254,7 +255,7 @@ func (r *DeploymentResource) Create(
 		}
 
 		// env vars
-		envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
+		envVars, diags = RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
 		if diags.HasError() {
 			resp.Diagnostics.Append(diags...)
 			return
@@ -298,7 +299,7 @@ func (r *DeploymentResource) Create(
 		return
 	}
 
-	diags = data.ReadFromResponse(ctx, deployment.JSON200, data.OriginalAstroRuntimeVersion.ValueStringPointer())
+	diags = data.ReadFromResponse(ctx, deployment.JSON200, data.OriginalAstroRuntimeVersion.ValueStringPointer(), &envVars)
 	if diags.HasError() {
 		resp.Diagnostics.Append(diags...)
 		return
@@ -323,6 +324,12 @@ func (r *DeploymentResource) Read(
 		return
 	}
 
+	envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
+	if diags.HasError() {
+		resp.Diagnostics.Append(diags...)
+		return
+	}
+
 	// get request
 	deployment, err := r.platformClient.GetDeploymentWithResponse(
 		ctx,
@@ -349,7 +356,7 @@ func (r *DeploymentResource) Read(
 		return
 	}
 
-	diags := data.ReadFromResponse(ctx, deployment.JSON200, data.OriginalAstroRuntimeVersion.ValueStringPointer())
+	diags = data.ReadFromResponse(ctx, deployment.JSON200, data.OriginalAstroRuntimeVersion.ValueStringPointer(), &envVars)
 	if diags.HasError() {
 		resp.Diagnostics.Append(diags...)
 		return
@@ -377,6 +384,7 @@ func (r *DeploymentResource) Update(
 	// update request
 	var diags diag.Diagnostics
 	var updateDeploymentRequest platform.UpdateDeploymentRequest
+	var envVars []platform.DeploymentEnvironmentVariableRequest
 
 	switch data.Type.ValueString() {
 	case string(platform.DeploymentTypeSTANDARD):
@@ -406,7 +414,7 @@ func (r *DeploymentResource) Update(
 		}
 
 		// env vars
-		envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
+		envVars, diags = RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
 		if diags.HasError() {
 			resp.Diagnostics.Append(diags...)
 			return
@@ -464,7 +472,7 @@ func (r *DeploymentResource) Update(
 		}
 
 		// env vars
-		envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
+		envVars, diags = RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
 		if diags.HasError() {
 			resp.Diagnostics.Append(diags...)
 			return
@@ -520,7 +528,7 @@ func (r *DeploymentResource) Update(
 		}
 
 		// env vars
-		envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
+		envVars, diags = RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
 		if diags.HasError() {
 			resp.Diagnostics.Append(diags...)
 			return
@@ -565,7 +573,7 @@ func (r *DeploymentResource) Update(
 		return
 	}
 
-	diags = data.ReadFromResponse(ctx, deployment.JSON200, data.OriginalAstroRuntimeVersion.ValueStringPointer())
+	diags = data.ReadFromResponse(ctx, deployment.JSON200, data.OriginalAstroRuntimeVersion.ValueStringPointer(), &envVars)
 	if diags.HasError() {
 		resp.Diagnostics.Append(diags...)
 		return

internal/provider/resources/resource_deployment_test.go

@@ -151,6 +151,7 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) {
 					resource.TestCheckNoResourceAttr(awsResourceVar, "worker_queues"),
 					resource.TestCheckResourceAttr(awsResourceVar, "scheduler_size", "SMALL"),
 					resource.TestCheckResourceAttrSet(awsResourceVar, "environment_variables.0.key"),
+					resource.TestCheckResourceAttrSet(awsResourceVar, "environment_variables.1.key"),
 					// Check via API that deployment exists
 					testAccCheckDeploymentExistence(t, awsDeploymentName, true, true),
 				),
@@ -222,12 +223,14 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) {
 					CloudProvider:               "AWS",
 					Executor:                    "KUBERNETES",
 					SchedulerSize:               "SMALL",
-					IncludeEnvironmentVariables: false,
+					IncludeEnvironmentVariables: true,
 					IsDevelopmentMode:           false,
 				}),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(awsResourceVar, "scheduler_size", "SMALL"),
 					resource.TestCheckResourceAttr(awsResourceVar, "is_development_mode", "false"),
+					resource.TestCheckResourceAttrSet(awsResourceVar, "environment_variables.0.key"),
+					resource.TestCheckResourceAttrSet(awsResourceVar, "environment_variables.1.key"),
 					// Check via API that deployment exists
 					testAccCheckDeploymentExistence(t, awsDeploymentName, true, true),
 				),
@@ -237,7 +240,7 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) {
 				ResourceName:            awsResourceVar,
 				ImportState:             true,
 				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"external_ips"},
+				ImportStateVerifyIgnore: []string{"external_ips", "environment_variables.1.value"}, // environment_variables.1.value is a secret value
 			},
 		},
 	})
@@ -279,7 +282,7 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) {
 				ResourceName:            azureCeleryResourceVar,
 				ImportState:             true,
 				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"external_ips", "oidc_issuer_url"},
+				ImportStateVerifyIgnore: []string{"external_ips", "oidc_issuer_url", "environment_variables.1.value"}, // environment_variables.0.value is a secret value
 			},
 		},
 	})
@@ -321,7 +324,7 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) {
 				ResourceName:            gcpKubernetesResourceVar,
 				ImportState:             true,
 				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"external_ips", "oidc_issuer_url"},
+				ImportStateVerifyIgnore: []string{"external_ips", "oidc_issuer_url", "environment_variables.1.value"}, // environment_variables.0.value is a secret value
 			},
 		},
 	})
@@ -401,6 +404,11 @@ func envVarsStr(includeEnvVars bool) string {
 		key = "key1"
 		value = "value1"
 		is_secret = false
+	},
+	{
+		key = "key2"
+		value = "value2"
+		is_secret = true
 	}]`
 	}
 	return fmt.Sprintf("environment_variables = %v", environmentVariables)

internal/provider/schemas/deployment.go

@@ -593,7 +593,7 @@ func DeploymentEnvironmentVariableResourceAttributes() map[string]resourceSchema
 		},
 		"value": resourceSchema.StringAttribute{
 			MarkdownDescription: "Environment variable value",
-			Required:            true,
+			Optional:            true,
 			Sensitive:           true,
 		},
 		"updated_at": resourceSchema.StringAttribute{