Superfluous $$'s and quotes

[bedge]

Found a couple of cases where there's extra $'s and "'s being added.

Here's a sample input json:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "VisualEditor0",
            "Effect": "Allow",
            "Action": [
                "s3:PutObject",
                "s3:GetBucketLocation",
                "s3:GetObjectAcl",
                "s3:GetObject",
                "s3:ListBucket",
                "s3:ListBucketVersions",
                "s3:DeleteObject",
                "s3:DeleteObjectVersion"
            ],
            "Resource": [
                "arn:aws:s3:::${aws_s3_bucket.output.id}/*",
                "arn:aws:s3:::${aws_s3_bucket.output.id}"
            ]
        },
        {
            "Sid": "VisualEditor1",
            "Effect": "Allow",
            "Action": [
                "s3:GetBucketLocation",
                "s3:GetObjectAcl",
                "s3:GetObject",
                "s3:ListBucket",
                "s3:ListBucketVersions",
                "s3:DeleteObject",
                "s3:DeleteObjectVersion"
            ],
            "Resource": [
                "arn:aws:s3:::${aws_s3_bucket.upload.id}/*",
                "arn:aws:s3:::${aws_s3_bucket.upload.id}"
            ]
        },
        {
            "Effect": "Allow",
            "Action": [
              "kms:Encrypt",
              "kms:Decrypt",
              "kms:ReEncrypt*",
              "kms:GenerateDataKey*",
              "kms:DescribeKey"
            ],
            "Resource": [
              "${aws_kms_key.key.arn}"
            ]
        }
    ]
}

and the resulting aws_iam_policy_document:

data "aws_iam_policy_document" "policy" {
  statement {
    sid    = "VisualEditor0"
    effect = "Allow"

    resources = [
      "arn:aws:s3:::$${aws_s3_bucket.output.id}/*",
      "arn:aws:s3:::$${aws_s3_bucket.output.id}",
    ]

    actions = [
      "s3:PutObject",
      "s3:GetBucketLocation",
      "s3:GetObjectAcl",
      "s3:GetObject",
      "s3:ListBucket",
      "s3:ListBucketVersions",
      "s3:DeleteObject",
      "s3:DeleteObjectVersion",
    ]
  }

  statement {
    sid    = "VisualEditor1"
    effect = "Allow"

    resources = [
      "arn:aws:s3:::$${aws_s3_bucket.upload.id}/*",
      "arn:aws:s3:::$${aws_s3_bucket.upload.id}",
    ]

    actions = [
      "s3:GetBucketLocation",
      "s3:GetObjectAcl",
      "s3:GetObject",
      "s3:ListBucket",
      "s3:ListBucketVersions",
      "s3:DeleteObject",
      "s3:DeleteObjectVersion",
    ]
  }

  statement {
    sid       = ""
    effect    = "Allow"
    resources = ["$${aws_kms_key.key.arn}"]

    actions = [
      "kms:Encrypt",
      "kms:Decrypt",
      "kms:ReEncrypt*",
      "kms:GenerateDataKey*",
      "kms:DescribeKey",
    ]
  }
}

Here's the diff of manual edits needed to fix the output:

[I] ➜ diff tenant-bucket-access.hcl.orig tenant-bucket-access.hcl
7,8c7,8
<       "arn:aws:s3:::$${aws_s3_bucket.output.id}/*",
<       "arn:aws:s3:::$${aws_s3_bucket.output.id}",
---
>       "arn:aws:s3:::${aws_s3_bucket.output.id}/*",
>       "arn:aws:s3:::${aws_s3_bucket.output.id}",
28,29c28,29
<       "arn:aws:s3:::$${aws_s3_bucket.upload.id}/*",
<       "arn:aws:s3:::$${aws_s3_bucket.upload.id}",
---
>       "arn:aws:s3:::${aws_s3_bucket.upload.id}/*",
>       "arn:aws:s3:::${aws_s3_bucket.upload.id}",
46c46
<     resources = ["$${aws_kms_key.key.arn}"]
---
>     resources = [ aws_kms_key.key.arn ]

[flosell]

Hi @bedge, thanks for the report!

I'm guessing you are using iam-policy-json-to-terraform to transform a policy JSON document that's already part of your terraform code, including some terraform interpolations.

It's a valid use-case and something the tool absolutely should be able to support - however, we'll likely need to make the tool a bit more smart to do this well. I'll try to look into this and if it's more or less straightforward might have a fix for this soon. PRs are obviously also very much appreciated!

Some analysis where the current behaviour comes from:

• escaping $ signs: dollar signs are deliberately escaped to support policy variables (e.g. ${aws:username}). I'm guessing we could make this smarter by trying to detect if something is a policy variable or a terraform interpolation
• superflous interpolations (e.g. ["$${aws_kms_key.key.arn}"]): currently, this tools simply transforms JSON syntax into the equivalent HCL syntax - it's not aware of more detailed terraform expressions (esp. the newer syntax introduced in 0.12). Switching from the current third-party hcl-encoder to the hashicorp hcl library might help here. A workaround might be to run the generated code through terraform 0.12upgrade (terraform fmt doesn't seem to do the trick), however that might be more trouble than it's worth. I'm not aware of any other autoformatter that might fix this, anyone who does, please pitch in here.

[bedge]

Hi @flosell, yes, you are entirely correct. Makes complete sense given that I'm misusing the utility.

Here's a fe wmore use cases I can think of -
In-place re-write of existing tf files to migrate the heredoc json to the new preferred aws_iam_policy_document format. Have to fudge the names for the aws_iam_policy_document elements, although a generated name would be hard to complain about.

Or, as a vsc or intellij plugin where you could highlight the heredoc in the editor, give it a name (for the aws_iam_policy_document) and have it do the transformation in-place in the editor.

Regardless, there's nothing else like this, and even with the manual tweaks, it's a big time saver and deserves more widespread adoption.
The heredoc json format is less than optimal as suppresses any form of IDE edit-time syntax checking.

Your effort is very much appreciated.

[flosell]

Hi @bedge thanks for the feedback! I've improved the dollar-sign escaping a bit so it doesn't escape terraform interpolations anymore. That's now released as version 1.5.0 so if you have some time, give it a try and let me know if it improves the situation.

For some of the other cases you mentioned:

• superflous interpolations (["${aws_kms_key.key.arn}"] vs [ aws_kms_key.key.arn ]): I've looked into it a bit but so far, no solution that wouldn't go heavily down the road of building a terraform autoformatter (which I feel is out of scope for this tool, I would have hoped terraform fmt would do something like this) - I've created a separate issue Remove superflous quotes when dealing with inline terraform expressions #14 for this to document the discussion

• rewriting heredoc JSON in an entire file - this certainly sounds like an interesting idea that I might pursue in the future - given that those aren't normal JSON documents might make it a bit harder to do, however that might also be worth doing to improve support for this use-case. I collected a couple of issues in this milestone

• as for editor/IDE support, I'd be very much in favour of that, however is probably better placed in the existing plugins (if I can help simplifying the interface between them, happy to). HashiCorp seems to be investing into the Visual Studio Code Plugin and the Language Server so maybe there's a place for a PR there. My personal favourite, the IntelliJ plugin is unfortunately closed source these days but might still be worth opening a ticket there.

[bedge]

Hi @flosell,
Agreed, re-implementing something handled by the terraform's 0.12upgrade command seems wasteful. Maybe that's usable as a shim to cleanup the output?

Thanks for the link on the hashicorp VCS plugin info. I had read previously that they had little interest in owning it. I was also sharding my loyalties between vsc and intellij as their TF plugin is 0.12 compatible today.

Fantastic to see the continued interest in these small (albeit less so as time goes on) but vital tools.

[flosell]

Thanks again for the feedback! I'll close this issue here, in favour of the more specific issues on the topic.