terramate-io / terramate

Terramate CLI is an open-source Infrastructure as Code (IaC) Orchestration and Code Generation tool for Terraform, OpenTofu and Terragrunt.
https://terramate.io
Mozilla Public License 2.0
3.27k stars 91 forks source link

[BUG] Labeled globals does not support quoted strings #1849

Open donovanrost opened 2 months ago

donovanrost commented 2 months ago

Describe the bug For resource tagging, we use a format like subdomain.company.com/key = value. At the top level stack we define:

globals {
  default_tags = {
      "meta.company.com/manager" = "terraform"
      "terramate.company.com/stack" = terramate.stack.id
  }

This works.

In a child stack, we would like add additional default_tags where the tags can follow the same format.

If I add

globals "default_tags" {
  test = "tag"
}

and run terramate debug show globals I will see the expected merged default_tags

default_tags = { "meta.company.com/manager" = "terraform" "terramate.company.com/stack" = "885edb70-3210-4680-b8d2-0145b493a710" test = "tag" }

If I wrap test in quotes like "test", I'm given the following error

/root/projects/rewrite-exploration/stacks/_devops_development/us-gov-east-1/stack.tm.hcl:20,3-4: HCL syntax error: loading from /root/projects/rewrite-exploration/stacks: loading from /root/projects/rewrite-exploration/stacks/_devops_development: loading from /root/projects/rewrite-exploration/stacks/_devops_development/us-gov-east-1: Argument names must not be quoted.

In this simple example, it of course is not necessary to wrap test in quotes. However, if the tag key is more complex like meta.company.com/example then I get

An argument or block definition is required here. To set an argument, use the equals sign "=" to introduce the argument value.

So quotes are necessary

Environment (please complete the following information):

Additional context Add any other context about the problem here.

i4ki commented 2 months ago

Hi @donovanrost

Unfortunately, not all objects can be extended by labeled globals because of a design issue.

The problem is that HCL attribute names must be an identifier. What you are trying to do is not a valid HCL syntax.

If you do:

globals {
  default_tags = {
      "meta.company.com/manager" = "terraform"
      "terramate.company.com/stack" = terramate.stack.id
  }
}

then the attribute is default_tags which is a valid HCL identifier. The "meta.company.com/manager" is an object key of the HCL Object expression.

If you do:

globals "default_tags" {
  test = "tag"
}

then it works because test is a valid identifier. If you quote "test" then it's not valid HCL syntax anymore.

We are aware of this limitation and we iterated ideas on some alternative syntaxes but it's being hard to find a good solution. One possible solution:

globals "default_tags" {
  set {
    name = "other.company.com/manager"
    value = "terraform"
  }
}

This set block requires a name and value attribute and then we bypass the syntax limitation but then it means there are two ways of setting global values. What do you think?

donovanrost commented 2 months ago

Thank you so much for the detailed explanation.

globals "default_tags" {
  set {
    name = "other.company.com/manager"
    value = "terraform"
  }
}

Would actually work great. From my point of view, it's better to have two ways of doing something if it allows you to avoid complicated dict merging by hand

We are aware of this limitation and we iterated ideas on some alternative syntaxes but it's being hard to find a good solution.

Is this set syntax implemented already, or are you searching for user feedback.

i4ki commented 2 months ago

Is this set syntax implemented already, or are you searching for user feedback.

Yes I was only asking for your feedback. Great that you liked it. I will raise this topic with the other engineers, so maybe we can prioritise this or a similar solution soon. cc @mariux @soerenmartius

donovanrost commented 1 month ago

In the interim, do you have any suggestions on how to best approach this?

i4ki commented 1 month ago

AFAICS there's no good alternative.

If changing the naming scheme of the keys are not possible, then what you can do is make the global.default_tags a set once value in the root directory and tm_merge() it with a per-stack defined global.

Hopefully we will soon have the solution implemented.