Incorrect diagnostic without default `type = "ssh"` in `connection` block

woodne commented 1 year ago

Extension Version

v2.28.0

VS Code Version

Version: 1.82.3 (Universal) Commit: fdb98833154679dbaa7af67a5a29fe19e55c2b73 Date: 2023-10-02T11:06:17.496Z (2 days ago) Electron: 25.8.1 ElectronBuildId: 24153832 Chromium: 114.0.5735.289 Node.js: 18.15.0 V8: 11.4.183.29-electron.0 OS: Darwin arm64 23.0.0

Operating System

macOS Sonoma 14.0

Terraform Version

Terraform v1.6.0 on darwin_arm64

Steps to Reproduce

Create a .tf file with the following contents:

provider "aws" {
  region = "us-west-2"
}

resource "aws_instance" "test" {
  ami           = "ami-0efcece6bed30fd98"
  instance_type = "m5.large"

  provisioner "remote-exec" {
    connection {
      host = self.private_ip

      port = 22

      bastion_host = "127.0.0.1"
    }

    inline = ["echo 'hello world'"]
  }
}

run terraform init

run terraform validate

Expected Behavior

terraform validate succeeds, and the vscode extension shows no errors

Actual Behavior

VS code extension is incorrectly reporting that bastion_host under the remote-exec provisioner connection block is unexpected.

But according to the latest HCL syntax it is indeed correct: https://developer.hashicorp.com/terraform/language/resources/provisioners/connection#argument-reference

Terraform Configuration

provider "aws" {
  region = "us-west-2"
}

resource "aws_instance" "test" {
  ami           = "ami-0efcece6bed30fd98"
  instance_type = "m5.large"

  provisioner "remote-exec" {
    connection {
      host = self.private_ip

      port = 22

      bastion_host = "127.0.0.1"
    }

    inline = ["echo 'hello world'"]
  }
}

Project Structure

No response

Gist

No response

Anything Else?

No response

Workarounds

No response

References

No response

Help Wanted

[ ] I'm interested in contributing a fix myself

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

brettcurtis commented 1 year ago

This extends out a bit further into tfvars and other places as well.

radeksimko commented 1 year ago

@woodne Thanks for the report. ~It looks like we are missing a number of fields in the connection block. The field in question was actually added ages ago (0.7.7) in https://github.com/hashicorp/terraform/commit/a7cbbbd25823bca2b4faa2b2786ee7afd6667e60~

I'll look into bringing the schema up to date for the block.

@brettcurtis May I ask you to raise a separate issue and attach a snippet of configuration, and ideally a screenshot? Thanks.

radeksimko commented 1 year ago

Actually it's a little more complicated. I didn't scroll far enough in that file to realise we do in fact track all those fields: https://github.com/hashicorp/terraform-schema/blob/7b256e6c65f9b53047526165ccec7a686b0f3861/internal/schema/0.12/connection_block.go#L126-L131

Workaround

There's a relatively straight-forward "workaround" which I'd even call good practice. In other words I would argue this makes configuration better regardless of whether it's flagged by the extension or not.

resource "aws_instance" "test" {
  ami           = "ami-0efcece6bed30fd98"
  instance_type = "m5.large"

  provisioner "remote-exec" {
    connection {
      type = "ssh"
      host = self.private_ip
      port = 22
      bastion_host = "127.0.0.1"
    }

    inline = ["echo 'hello world'"]
  }
}

^ adding type = "ssh" makes it more explicit and obvious to the reader that SSH is in use (rather than "somewhere documented default which happens to be SSH").

Fix

I still agree that raising error in the originally reported configuration is confusing/wrong and we should fix that.

Additionally, we are still missing a few other proxy-related fields introduced in 1.3 for the ssh communicator:

Screenshot 2023-10-05 at 09 12 49

and we somehow don't seem to be recognising the older target_platform field:

Screenshot 2023-10-05 at 09 04 46

(https://github.com/hashicorp/terraform-schema/blob/7b256e6c65f9b53047526165ccec7a686b0f3861/internal/schema/0.15/connection_block.go#L28-L37)

I will look into all three issues.

Future Feature

We can look into possibly raising warnings or something of that nature for those missing defaults like type to encourage best practices. I will file a follow-up issue to track that.

radeksimko commented 1 year ago

I managed to find solutions for all three issues I described above, and those PRs are now pending review:

I will post an update here when we have merged and released this.

sheppyj commented 1 year ago

My colleagues and I are experiencing the same issue as @brettcurtis where valid tfvars outside of the root of the configuration directory are reporting as incorrect syntax... i've created ; https://github.com/hashicorp/vscode-terraform/issues/1574

If that is of any help @radeksimko

brettcurtis commented 1 year ago

Another example with terraform_remote_state datasource if it's helpful:

kkirpichnikov commented 1 year ago

One more example from a .hcl file that is used for terragrunt

radeksimko commented 1 year ago

@kkirpichnikov We do not support non-Terraform HCL files and we never did (at least since v2). The Terraform extension intentionally avoids claiming *.hcl as it cannot provide any value to arbitrary HCL files - as demonstrated by the diagnostics on the screenshot. If you configured VS Code such that it treats all *.hcl as Terraform then I'd suggest to remove that settings.

There is HCL extension which can help with syntax highlighting which you can install and it shouldn't need any additional tweaking as it automatically claims *.hcl and that will work for Terragrunt and other HCL-based languages, like Packer, Nomad etc.

For anyone else experiencing incorrect diagnostics that aren't the same as in the initial post I would kindly ask you to create new issue and attach a screenshot and code to copy paste to help us reproduce.

kkirpichnikov commented 1 year ago

@radeksimko anyway, it was working for a very long time till the latest release.

voxmaster commented 1 year ago

.tfvars looks also broken

radeksimko commented 1 year ago

For anyone else experiencing incorrect diagnostics that aren't the same as in the initial post I would kindly ask you to create new issue and attach a screenshot and code to copy paste to help us reproduce instead of attaching more comments here.

Thanks!

radeksimko commented 1 year ago

A new version 2.28.1 was just released. This fixes all the three mentioned bugs in my comment above, including the originally reported bug by @woodne. The update should appear automatically in VS Code.

Thank you for the timing and level of detail in your report @woodne

In case you experience any different validation related bug, please do let us know through a new issue.

github-actions[bot] commented 11 months ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

hashicorp / vscode-terraform