Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 16 commits into from
May 16, 2024
4 changes: 3 additions & 1 deletion .github/workflows/testacc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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=(
Expand All @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions docs/resources/deployment.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
36 changes: 31 additions & 5 deletions internal/provider/models/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions internal/provider/resources/resource_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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()))
Expand Down
26 changes: 17 additions & 9 deletions internal/provider/resources/resource_deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
16 changes: 12 additions & 4 deletions internal/provider/resources/resource_deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
),
Expand Down Expand Up @@ -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),
),
Expand All @@ -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
},
},
})
Expand Down Expand Up @@ -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
},
},
})
Expand Down Expand Up @@ -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
},
},
})
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion internal/provider/schemas/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down