scottwinkler / terraform-provider-shell

Terraform provider for executing shell commands and saving output to state file
Mozilla Public License 2.0
278 stars 61 forks source link

Objects have changed outside of Terraform after first apply #103

Open alkoclick opened 2 years ago

alkoclick commented 2 years ago

Hello,

First of all, congratulations for this awesome project! I'm using Terraform for my dotfiles repo and this provider allows me to nicely interact with local system resources like apt,homebrew and gpg and manage them in a stateful way.

Issue

I'm seeing an issue when immediately after creating new resources where Terraform reports them as "changed outside of Terraform". If I run terraform apply -refresh-only as suggested, the issue is resolved and this planned change no longer appears.

Versions:

∴ terraform --version
Terraform v1.0.11
on linux_amd64
+ provider registry.terraform.io/scottwinkler/shell v1.7.10

Console output:

Terraform detected the following changes made outside of Terraform since the last "terraform apply":

  # module.keys.shell_script.gpg_key[0] has been changed
  ~ resource "shell_script" "gpg_key" {
        id                    = "c6pq403skthoir27iphg"
      + read_error            = ""
        # (5 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Unless you have made equivalent changes to your configuration, or ignored the relevant attributes using ignore_changes, the following plan may include actions to undo or respond to these
changes.

─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

No changes. Your infrastructure matches the configuration.

Your configuration already matches the changes detected above. If you'd like to update the Terraform state to match, create and apply a refresh-only plan:
  terraform apply -refresh-only

In the state:

...
"read_error": null,
...

Interpretation

I looked up where this is defined and it's in resource_shell_script.go, line 87.

I am not very familiar with the Terraform provider schema behaviors, so I looked up the docs for computed and optional. From what I understand the read_error key should be initialized to "", and stay as "" for as long as there is no issue. However, I believe that due to the Optional: True parameter, it gets initialized to null instead.

Recommendation

I am not by any means an expert here, but I wanted to have an initial attempt at a solution. Based on the above, I think the problem should be solvable by changing the schema for read_error to be "computed" instead of "optional". That should be in line with its stated purpose, and (if I understand correctly) will allow it to be initialized as "" rather than null, which means that the issue will be resolved.

alkoclick commented 2 years ago

Replicatable example:

locals {
  apt_packages = [
    "fonts-firacode",
    "xclip",
  ]
  cmd_print_version = jsonencode({
    # Including the : character in the output somehow messes up the map and it thinks it's always changing
    "\"version\"" = "\"$(apt-cache policy $PACKAGE | grep --color=never Installed | sed 's/Installed: //')\""
  })
}

resource "shell_script" "apt_package" {
  for_each = toset(local.apt_packages)

  lifecycle_commands {
    # Sudo here will actually work normally during execution, if you have a pass you will be asked for it from STDIN
    create = format("sudo apt-get install -y --no-install-recommends $PACKAGE; echo %s", local.cmd_print_version)
    read   = format("echo %s", local.cmd_print_version)
    update = format("sudo apt-get install -y --no-install-recommends --reinstall $PACKAGE; echo %s", local.cmd_print_version)
    delete = "sudo apt-get remove -y $PACKAGE"
  }

  environment = {
    PACKAGE = each.value
  }
}

With export TF_LOG=trace:

2021-12-10T20:13:47.120+0100 [WARN]  Provider "registry.terraform.io/scottwinkler/shell" produced an unexpected new value for module.apt.shell_script.apt_package["xclip"] during refresh.
      - .read_error: was null, but now cty.StringVal("")
mkielar commented 2 years ago

Having the same with running python scripts with terraform 1.1.5 and shell 1.17.10, except there's no diff presented at all:

# module.hello-world-service.module.vg_route_alb["tools"].shell_script.gateway_route has changed
  ~ resource "shell_script" "gateway_route" {
        id                = "c985ev9ss20n6qk4vce0"
        # (4 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }
nskalis commented 1 year ago

@mkielar I encountered the same issue. Would you be so kind to advise if you managed to work around it?