fix: Add missing change.actions values to planfile.ts

[isobelhooper]

https://developer.hashicorp.com/terraform/internals/json-format#change-representation has two entries in change.actions that aren't currently in the definition for planfileSchema: ['read'] and ['create', 'delete'].

I've done very little Typescript work, but It looks like these are pretty simple to add, so I've made this PR to include them.

Motivation

I'm looking for a currently-maintained Github action to comment with summaries of terraform plan results on my team's PRs, and this one looks really nicely done.

I tried out this action with a change to one of my team's Terraform definitions, and while the code coped fine with a plan without changes, it errored on the first change entry, which had an action of "read".

The error output from Zod

[
  {
    "code": "invalid_union",
    "unionErrors": [
      {
        "issues": [
          {
            "received": "read",
            "code": "invalid_literal",
            "expected": "no-op",
            "path": [
              "resource_changes",
              0,
              "change",
              "actions",
              0
            ],
            "message": "Invalid literal value, expected \"no-op\""
          }
        ],
        "name": "ZodError"
      },
      {
        "issues": [
          {
            "received": "read",
            "code": "invalid_literal",
            "expected": "create",
            "path": [
              "resource_changes",
              0,
              "change",
              "actions",
              0
            ],
            "message": "Invalid literal value, expected \"create\""
          }
        ],
        "name": "ZodError"
      },
      {
        "issues": [
          {
            "received": "read",
            "code": "invalid_literal",
            "expected": "delete",
            "path": [
              "resource_changes",
              0,
              "change",
              "actions",
              0
            ],
            "message": "Invalid literal value, expected \"delete\""
          }
        ],
        "name": "ZodError"
      },
      {
        "issues": [
          {
            "received": "read",
            "code": "invalid_literal",
            "expected": "update",
            "path": [
              "resource_changes",
              0,
              "change",
              "actions",
              0
            ],
            "message": "Invalid literal value, expected \"update\""
          }
        ],
        "name": "ZodError"
      },
      {
        "issues": [
          {
            "code": "too_small",
            "minimum": 2,
            "inclusive": true,
            "exact": false,
            "type": "array",
            "path": [
              "resource_changes",
              0,
              "change",
              "actions"
            ],
            "message": "Array must contain at least 2 element(s)"
          }
        ],
        "name": "ZodError"
      }
    ],
    "path": [
      "resource_changes",
      0,
      "change",
      "actions"
    ],
    "message": "Invalid input"
  }
]

Changes

Adding the two possible entries for changes.actions that are in the current JSON output change representation but not in here yet.

[borchero]

Thanks for your contribution! I have no objection to adding the read option (I didn't know this is an option that is actually returned during plan). Would you prefer displaying read resources in the PR comment or should they be excluded?

Regarding ["create", "delete"], I'm wondering (1) why would that ever happen during apply and (2) how to integrate this into the current representation in the PR. What does this do semantically, "re-delete"? 😄

[isobelhooper]

Thanks for your contribution! I have no objection to adding the read option (I didn't know this is an option that is actually returned during plan). Would you prefer displaying read resources in the PR comment or should they be excluded?

I think it's okay to not display read resources, since the action doesn't display them at the moment. They could perhaps be added in a later PR.

Looking at https://developer.hashicorp.com/terraform/internals/json-format#plan-representation and at the plan JSON I was testing with, there's a action_reason field while might be useful to include. In the read option that my plan JSON had, the action_reason was "read_because_dependency_pending" - so the read resource relies on a resource that the plan is changing, but is not itself changing. I'm not sure if it'd be helpful or confusing to include it in that case.

Regarding ["create", "delete"], I'm wondering (1) why would that ever happen during apply and (2) how to integrate this into the current representation in the PR. What does this do semantically, "re-delete"? 😄

I was surprised by this too! I think it's one way that a replacement can be represented - create a new thing, then delete the old thing - but it is very strange compared to the other way around. 😄

[isobelhooper]

Apologies, I noticed that CI had failed due to a syntax error where I added one of the action types - I've added the missing comma to planfile.ts.

[borchero]

Sorry for getting back to you so late here @isobelhooper!

Let's not display the read resources for the time being, we can do that in a follow-up PR. Regarding ["create", "delete"], I would still like to see an example of how this looks like in the terraform plan output to know whether they can just be added to the "replaced resources".

[borchero]

As far as CI is concerned:

• You'll need to run npm build to satisfy the pre-commit checks
• I'll need to look into the end-to-end tests. Permissions are somehow missing for the GitHub token associated with PRs of external contributors

[simonblake-mp]

Let's not display the read resources for the time being, we can do that in a follow-up PR. Regarding ["create", "delete"], I would still like to see an example of how this looks like in the terraform plan output to know whether they can just be added to the "replaced resources".

@borchero I've just had this (or something very like it) crop up, where terraform-plan-commit fails with zodErrors about invalid literals that looks identical to @isobelhooper 's error above. I've got both the terraform plan text output and the -out saved, if that's useful. The error, though, is somewhat hard to reproduce - if I write some code to create an S3 bucket with the usual S3 sub-resources, if I add an iam_policy_document that refers to the S3 bucket itself - something like

resource "aws_s3_bucket" "bucket_log" {
  bucket = "somebucket-log"

  object_lock_enabled = true
}

... other S3 sub resources redacted for brevity

resource "aws_s3_bucket_policy" "bucket_log" {
  bucket = aws_s3_bucket.bucket_log.id
  policy = data.aws_iam_policy_document.bucket_log.json
}

data "aws_iam_policy_document" "bucket_log" {
  statement {
    principals {
      type        = "Service"
      identifiers = ["logging.s3.amazonaws.com"]
    }

... other conditions redacted for brevity

    actions   = ["s3:PutObject"]
    resources = ["${aws_s3_bucket.bucket_log.arn}/*"]
  }
}

then this crashes terraform-plan-comment , but if i change the resources line in the policy to a "*" wildcard, then terraform-plan-comment works fine. Both plan identically (n to add, 0 to change, 0 to delete), unfortunately the wildcard isn't actually valid, so the apply failed to create the bucket policy. A second plan pass, with the resources line reverted to above and after the bucket was created also worked fine with terraform-plan-comment - so it feels like it's a transitory state in the planfile that's related to referencing something that doesn't yet exist. 🤷

[borchero]

Sorry this took forever! I think this should do it now, thanks for the PR @isobelhooper :)

[isobelhooper]

Wanted to say thank you again for accepting this PR, as it means it works with our Terraform state - we're now using it for our PRs and it's very useful!