cloudposse / terraform-aws-ecs-container-definition

Terraform module to generate well-formed JSON documents (container definitions) that are passed to the aws_ecs_task_definition Terraform resource
https://cloudposse.com/accelerate
Apache License 2.0
339 stars 244 forks source link

make outputs non-sensitive for terraform v0.15 #134

Closed njoerd114 closed 2 years ago

njoerd114 commented 3 years ago

what

references

caveats

njoerd114 commented 3 years ago

/test all

nitrocode commented 3 years ago

Will this prevent using older versions of terraform?

njoerd114 commented 3 years ago

Will this prevent using older versions of terraform?

the nonsensitive function is available from terraform >= v0.14

I updated the version constraints respectively, and I opened another PR that should address the current situation and help users identify the current module's incompatibility with v 0.15.0 : #135

Gowiem commented 3 years ago

/test all

Nuru commented 3 years ago

@osterman @aknysh Do we want to drop TF version 0.13 support to enable TF 0.15 support? I'm not comfortable with it, but I am not sure that the alternative (which I think would be getting rid of non-sensitive outputs) is better.

osterman commented 3 years ago

I am fine with it. Our interface will stabilize with TF 1.0, which will be released after 0.15

nitrocode commented 3 years ago

Will this prevent using older versions of terraform?

the nonsensitive function is available from terraform >= v0.14

I updated the version constraints respectively, and I opened another PR that should address the current situation and help users identify the current module's incompatibility with v 0.15.0 : #135

I don't see the nonsensitive function in terraform 0.14, in spite of the documentation.

EDIT: The docs were updated to state it's available in 0.15 and later on May 14 e27a927

Note: This function is only available in Terraform v0.15 and later.

$ cat not_working.tf
output "test_nonsensitive" {
  # this returns missing function in 0.14
  # this returns "Invalid value for "value" parameter: the given value is not sensitive, so this call is redundant." in 0.15
  value = nonsensitive("test")

  # this returns the correct output in both 0.14 and 0.15
  value = try(nonsensitive("test"), "test")
}
$ cat working.tf
output "test_nonsensitive" {
  # this returns the correct output in both 0.14 and 0.15
  value = try(nonsensitive("test"), "test")
}
$ tfenv use latest:0.14
$ terraform version
Terraform v0.14.11
$ terraform apply

Error: Call to unknown function

  on main.tf line 6, in output "test_nonsensitive":
   6:   value = nonsensitive("test")

There is no function named "nonsensitive".
$ tfenv use latest:0.15
$ terraform version
Terraform v0.15.3
$ terraform apply
╷
│ Error: Invalid function argument
│
│   on main.tf line 6, in output "test_nonsensitive":
│    6:   value = nonsensitive("test")
│
│ Invalid value for "value" parameter: the given value is not sensitive, so this call is redundant.

It appears that this change will only work for 0.15 and up. I'd be curious if the try() function would work instead for both 0.15 and previous versions.

output "json_map_encoded_list" {
  description = "JSON string encoded list of container definitions for use with other terraform resources such as aws_ecs_task_definition"
  value       = try("[${nonsensitive(local.json_map)}]", "[${local.json_map}]")
}

output "json_map_encoded" {
  description = "JSON string encoded container definitions for use with other terraform resources such as aws_ecs_task_definition"
  value       = try(nonsensitive(local.json_map), local.json_map)
}

output "json_map_object" {
  description = "JSON map encoded container definition"
  value       = try(nonsensitive(jsondecode(local.json_map)), jsondecode(local.json_map))
}
darpham commented 3 years ago

@osterman @aknysh Do we want to drop TF version 0.13 support to enable TF 0.15 support? I'm not comfortable with it, but I am not sure that the alternative (which I think would be getting rid of non-sensitive outputs) is better.

Can we just update documentation to tell users on < TF0.14 to use v0.56.0 release? Where do we draw the line in ensuring backwards compatibility by sacrificing new features/improvements

mergify[bot] commented 3 years ago

This pull request is now in conflict. Could you fix it @njoerd114? 🙏

noqqe commented 3 years ago

Any update on this?

nitrocode commented 3 years ago

What sensitive values are used in a task definition that cannot be replaced by using secrets / map_secrets and valueFrom an SSM (or ASM) arn ?

e.g.

  secrets = [
    {
      name      = "mysecretenvvar"
      valueFrom = "arn:aws:etc" # ssm arn
    }
  ]

or

  map_secrets = {
    mysecretenvvar = "arn:aws:etc" # ssm arn
  }
nitrocode commented 2 years ago

Please re-open if you have more information