Skip to content

Commit 0ba1d5e

Browse files
vandyliuvandyliu
authored
Fix deployment env vars not working if is_secret=True (#60)
1 parent e9d620a commit 0ba1d5e

File tree

7 files changed

+70
-20
lines changed

7 files changed

+70
-20
lines changed

‎.github/workflows/testacc.yml

+3-1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ jobs:
5757
- name: Determine if expensive tests should run
5858
if: github.event_name == 'pull_request'
5959
run: |
60+
git fetch --all
6061
echo "CHECKING FILES FOR SKIP LOGIC..."
6162
SKIP_TESTS="True"
6263
FILES_TO_CHECK=(
@@ -69,11 +70,12 @@ jobs:
6970
"internal/provider/resources/resource_deployment.go"
7071
)
7172
for file in "${FILES_TO_CHECK[@]}"; do
72-
if git diff --name-only ${{ github.base_ref }} ${{ github.head_ref }} | grep -q "$file"; then
73+
if git diff --name-only remotes/origin/${{ github.base_ref }} remotes/origin/${{ github.head_ref }} | grep -q "$file"; then
7374
SKIP_TESTS="False"
7475
break
7576
fi
7677
done
78+
echo "SKIP_CLUSTER_RESOURCE_TESTS=$SKIP_TESTS"
7779
echo "SKIP_CLUSTER_RESOURCE_TESTS=$SKIP_TESTS" >> $GITHUB_ENV
7880
- env:
7981
TF_ACC: "1"

‎docs/resources/deployment.md

+3
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,9 @@ Required:
158158

159159
- `is_secret` (Boolean) Whether Environment variable is a secret
160160
- `key` (String) Environment variable key
161+
162+
Optional:
163+
161164
- `value` (String, Sensitive) Environment variable value
162165

163166
Read-Only:

‎internal/provider/models/deployment.go

+31-5
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"context"
55
"time"
66

7+
"github.com/samber/lo"
8+
79
"github.com/astronomer/terraform-provider-astro/internal/clients/platform"
810
"github.com/astronomer/terraform-provider-astro/internal/provider/schemas"
911
"github.com/astronomer/terraform-provider-astro/internal/utils"
@@ -132,6 +134,7 @@ func (data *DeploymentResource) ReadFromResponse(
132134
ctx context.Context,
133135
deployment *platform.Deployment,
134136
originalAstroRuntimeVersion *string,
137+
requestEnvVars *[]platform.DeploymentEnvironmentVariableRequest,
135138
) diag.Diagnostics {
136139
// Read common fields
137140
data.Id = types.StringValue(deployment.Id)
@@ -180,7 +183,30 @@ func (data *DeploymentResource) ReadFromResponse(
180183
data.ImageTag = types.StringValue(deployment.ImageTag)
181184
data.ImageRepository = types.StringValue(deployment.ImageRepository)
182185
data.ImageVersion = types.StringPointerValue(deployment.ImageVersion)
183-
data.EnvironmentVariables, diags = utils.ObjectSet(ctx, deployment.EnvironmentVariables, schemas.DeploymentEnvironmentVariableAttributeTypes(), DeploymentEnvironmentVariableTypesObject)
186+
187+
// Environment variables are a special case
188+
// 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
189+
// We must use the request value and set it in the Terraform response to keep Terraform from emitting errors
190+
// Since the value is marked as sensitive, Terraform will not output the actual value in the plan/apply output
191+
envVars := *deployment.EnvironmentVariables
192+
if requestEnvVars != nil && deployment.EnvironmentVariables != nil {
193+
requestEnvVarsMap := lo.SliceToMap(*requestEnvVars, func(envVar platform.DeploymentEnvironmentVariableRequest) (string, platform.DeploymentEnvironmentVariable) {
194+
return envVar.Key, platform.DeploymentEnvironmentVariable{
195+
Key: envVar.Key,
196+
Value: envVar.Value,
197+
IsSecret: envVar.IsSecret,
198+
}
199+
})
200+
for i, envVar := range envVars {
201+
if envVar.IsSecret {
202+
if requestEnvVar, ok := requestEnvVarsMap[envVar.Key]; ok {
203+
// If the envVar has a secret value, update the value in the response
204+
envVars[i].Value = requestEnvVar.Value
205+
}
206+
}
207+
}
208+
}
209+
data.EnvironmentVariables, diags = utils.ObjectSet(ctx, &envVars, schemas.DeploymentEnvironmentVariableAttributeTypes(), DeploymentEnvironmentVariableTypesObject)
184210
if diags.HasError() {
185211
return diags
186212
}
@@ -210,7 +236,7 @@ func (data *DeploymentResource) ReadFromResponse(
210236
// Read hybrid deployment specific fields
211237
data.TaskPodNodePoolId = types.StringPointerValue(deployment.TaskPodNodePoolId)
212238

213-
// Read hosted deployment specific fields
239+
// Read hosted (standard and dedicated) deployment specific fields
214240
data.ResourceQuotaCpu = types.StringPointerValue(deployment.ResourceQuotaCpu)
215241
data.ResourceQuotaMemory = types.StringPointerValue(deployment.ResourceQuotaMemory)
216242
data.DefaultTaskPodCpu = types.StringPointerValue(deployment.DefaultTaskPodCpu)
@@ -277,12 +303,12 @@ func (data *DeploymentDataSource) ReadFromResponse(
277303
return diags
278304
}
279305
data.Executor = types.StringPointerValue((*string)(deployment.Executor))
306+
data.SchedulerCpu = types.StringValue(deployment.SchedulerCpu)
307+
data.SchedulerMemory = types.StringValue(deployment.SchedulerMemory)
280308
if deployment.SchedulerAu != nil {
281309
deploymentSchedulerAu := int64(*deployment.SchedulerAu)
282310
data.SchedulerAu = types.Int64Value(deploymentSchedulerAu)
283311
}
284-
data.SchedulerCpu = types.StringValue(deployment.SchedulerCpu)
285-
data.SchedulerMemory = types.StringValue(deployment.SchedulerMemory)
286312
data.SchedulerReplicas = types.Int64Value(int64(deployment.SchedulerReplicas))
287313
data.ImageTag = types.StringValue(deployment.ImageTag)
288314
data.ImageRepository = types.StringValue(deployment.ImageRepository)
@@ -317,7 +343,7 @@ func (data *DeploymentDataSource) ReadFromResponse(
317343
// Read hybrid deployment specific fields
318344
data.TaskPodNodePoolId = types.StringPointerValue(deployment.TaskPodNodePoolId)
319345

320-
// Read hosted deployment specific fields
346+
// Read hosted (standard and dedicated) deployment specific fields
321347
data.ResourceQuotaCpu = types.StringPointerValue(deployment.ResourceQuotaCpu)
322348
data.ResourceQuotaMemory = types.StringPointerValue(deployment.ResourceQuotaMemory)
323349
data.DefaultTaskPodCpu = types.StringPointerValue(deployment.DefaultTaskPodCpu)

‎internal/provider/resources/resource_cluster.go

+3
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ func (r *ClusterResource) Create(
228228
readyCluster, err := stateConf.WaitForStateContext(ctx)
229229
if err != nil {
230230
resp.Diagnostics.AddError("Cluster creation failed", err.Error())
231+
return
231232
}
232233

233234
diags = data.ReadFromResponse(ctx, readyCluster.(*platform.Cluster))
@@ -381,6 +382,7 @@ func (r *ClusterResource) Update(
381382
readyCluster, err := stateConf.WaitForStateContext(ctx)
382383
if err != nil {
383384
resp.Diagnostics.AddError("Cluster update failed", err.Error())
385+
return
384386
}
385387

386388
diags = data.ReadFromResponse(ctx, readyCluster.(*platform.Cluster))
@@ -451,6 +453,7 @@ func (r *ClusterResource) Delete(
451453
_, err = stateConf.WaitForStateContext(ctx)
452454
if err != nil {
453455
resp.Diagnostics.AddError("Cluster deletion failed", err.Error())
456+
return
454457
}
455458

456459
tflog.Trace(ctx, fmt.Sprintf("deleted a cluster resource: %v", data.Id.ValueString()))

‎internal/provider/resources/resource_deployment.go

+17-9
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ func (r *DeploymentResource) Create(
9393

9494
var diags diag.Diagnostics
9595
var createDeploymentRequest platform.CreateDeploymentRequest
96+
var envVars []platform.DeploymentEnvironmentVariableRequest
9697

9798
originalAstroRuntimeVersion := data.OriginalAstroRuntimeVersion.ValueString()
9899
if len(originalAstroRuntimeVersion) == 0 {
@@ -136,7 +137,7 @@ func (r *DeploymentResource) Create(
136137
}
137138

138139
// env vars
139-
envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
140+
envVars, diags = RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
140141
if diags.HasError() {
141142
resp.Diagnostics.Append(diags...)
142143
return
@@ -196,7 +197,7 @@ func (r *DeploymentResource) Create(
196197
}
197198

198199
// env vars
199-
envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
200+
envVars, diags = RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
200201
if diags.HasError() {
201202
resp.Diagnostics.Append(diags...)
202203
return
@@ -254,7 +255,7 @@ func (r *DeploymentResource) Create(
254255
}
255256

256257
// env vars
257-
envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
258+
envVars, diags = RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
258259
if diags.HasError() {
259260
resp.Diagnostics.Append(diags...)
260261
return
@@ -298,7 +299,7 @@ func (r *DeploymentResource) Create(
298299
return
299300
}
300301

301-
diags = data.ReadFromResponse(ctx, deployment.JSON200, data.OriginalAstroRuntimeVersion.ValueStringPointer())
302+
diags = data.ReadFromResponse(ctx, deployment.JSON200, data.OriginalAstroRuntimeVersion.ValueStringPointer(), &envVars)
302303
if diags.HasError() {
303304
resp.Diagnostics.Append(diags...)
304305
return
@@ -323,6 +324,12 @@ func (r *DeploymentResource) Read(
323324
return
324325
}
325326

327+
envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
328+
if diags.HasError() {
329+
resp.Diagnostics.Append(diags...)
330+
return
331+
}
332+
326333
// get request
327334
deployment, err := r.platformClient.GetDeploymentWithResponse(
328335
ctx,
@@ -349,7 +356,7 @@ func (r *DeploymentResource) Read(
349356
return
350357
}
351358

352-
diags := data.ReadFromResponse(ctx, deployment.JSON200, data.OriginalAstroRuntimeVersion.ValueStringPointer())
359+
diags = data.ReadFromResponse(ctx, deployment.JSON200, data.OriginalAstroRuntimeVersion.ValueStringPointer(), &envVars)
353360
if diags.HasError() {
354361
resp.Diagnostics.Append(diags...)
355362
return
@@ -377,6 +384,7 @@ func (r *DeploymentResource) Update(
377384
// update request
378385
var diags diag.Diagnostics
379386
var updateDeploymentRequest platform.UpdateDeploymentRequest
387+
var envVars []platform.DeploymentEnvironmentVariableRequest
380388

381389
switch data.Type.ValueString() {
382390
case string(platform.DeploymentTypeSTANDARD):
@@ -406,7 +414,7 @@ func (r *DeploymentResource) Update(
406414
}
407415

408416
// env vars
409-
envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
417+
envVars, diags = RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
410418
if diags.HasError() {
411419
resp.Diagnostics.Append(diags...)
412420
return
@@ -464,7 +472,7 @@ func (r *DeploymentResource) Update(
464472
}
465473

466474
// env vars
467-
envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
475+
envVars, diags = RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
468476
if diags.HasError() {
469477
resp.Diagnostics.Append(diags...)
470478
return
@@ -520,7 +528,7 @@ func (r *DeploymentResource) Update(
520528
}
521529

522530
// env vars
523-
envVars, diags := RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
531+
envVars, diags = RequestDeploymentEnvironmentVariables(ctx, data.EnvironmentVariables)
524532
if diags.HasError() {
525533
resp.Diagnostics.Append(diags...)
526534
return
@@ -565,7 +573,7 @@ func (r *DeploymentResource) Update(
565573
return
566574
}
567575

568-
diags = data.ReadFromResponse(ctx, deployment.JSON200, data.OriginalAstroRuntimeVersion.ValueStringPointer())
576+
diags = data.ReadFromResponse(ctx, deployment.JSON200, data.OriginalAstroRuntimeVersion.ValueStringPointer(), &envVars)
569577
if diags.HasError() {
570578
resp.Diagnostics.Append(diags...)
571579
return

‎internal/provider/resources/resource_deployment_test.go

+12-4
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) {
151151
resource.TestCheckNoResourceAttr(awsResourceVar, "worker_queues"),
152152
resource.TestCheckResourceAttr(awsResourceVar, "scheduler_size", "SMALL"),
153153
resource.TestCheckResourceAttrSet(awsResourceVar, "environment_variables.0.key"),
154+
resource.TestCheckResourceAttrSet(awsResourceVar, "environment_variables.1.key"),
154155
// Check via API that deployment exists
155156
testAccCheckDeploymentExistence(t, awsDeploymentName, true, true),
156157
),
@@ -222,12 +223,14 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) {
222223
CloudProvider: "AWS",
223224
Executor: "KUBERNETES",
224225
SchedulerSize: "SMALL",
225-
IncludeEnvironmentVariables: false,
226+
IncludeEnvironmentVariables: true,
226227
IsDevelopmentMode: false,
227228
}),
228229
Check: resource.ComposeTestCheckFunc(
229230
resource.TestCheckResourceAttr(awsResourceVar, "scheduler_size", "SMALL"),
230231
resource.TestCheckResourceAttr(awsResourceVar, "is_development_mode", "false"),
232+
resource.TestCheckResourceAttrSet(awsResourceVar, "environment_variables.0.key"),
233+
resource.TestCheckResourceAttrSet(awsResourceVar, "environment_variables.1.key"),
231234
// Check via API that deployment exists
232235
testAccCheckDeploymentExistence(t, awsDeploymentName, true, true),
233236
),
@@ -237,7 +240,7 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) {
237240
ResourceName: awsResourceVar,
238241
ImportState: true,
239242
ImportStateVerify: true,
240-
ImportStateVerifyIgnore: []string{"external_ips"},
243+
ImportStateVerifyIgnore: []string{"external_ips", "environment_variables.1.value"}, // environment_variables.1.value is a secret value
241244
},
242245
},
243246
})
@@ -279,7 +282,7 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) {
279282
ResourceName: azureCeleryResourceVar,
280283
ImportState: true,
281284
ImportStateVerify: true,
282-
ImportStateVerifyIgnore: []string{"external_ips", "oidc_issuer_url"},
285+
ImportStateVerifyIgnore: []string{"external_ips", "oidc_issuer_url", "environment_variables.1.value"}, // environment_variables.0.value is a secret value
283286
},
284287
},
285288
})
@@ -321,7 +324,7 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) {
321324
ResourceName: gcpKubernetesResourceVar,
322325
ImportState: true,
323326
ImportStateVerify: true,
324-
ImportStateVerifyIgnore: []string{"external_ips", "oidc_issuer_url"},
327+
ImportStateVerifyIgnore: []string{"external_ips", "oidc_issuer_url", "environment_variables.1.value"}, // environment_variables.0.value is a secret value
325328
},
326329
},
327330
})
@@ -401,6 +404,11 @@ func envVarsStr(includeEnvVars bool) string {
401404
key = "key1"
402405
value = "value1"
403406
is_secret = false
407+
},
408+
{
409+
key = "key2"
410+
value = "value2"
411+
is_secret = true
404412
}]`
405413
}
406414
return fmt.Sprintf("environment_variables = %v", environmentVariables)

‎internal/provider/schemas/deployment.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ func DeploymentEnvironmentVariableResourceAttributes() map[string]resourceSchema
593593
},
594594
"value": resourceSchema.StringAttribute{
595595
MarkdownDescription: "Environment variable value",
596-
Required: true,
596+
Optional: true,
597597
Sensitive: true,
598598
},
599599
"updated_at": resourceSchema.StringAttribute{

0 commit comments

Comments
 (0)