Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix heredoc parsing in parseConfig #97

Merged

Conversation

syndicut
Copy link
Contributor

Relates terraform-linters/tflint#1029

Workaround added in #93 should also be added to parseConfig function

@wata727
Copy link
Member

wata727 commented Jan 22, 2021

This workaround works for expressions only. In the parseConfig, which handles the body, a newline is meaningful and cannot be added.

@syndicut
Copy link
Contributor Author

syndicut commented Jan 22, 2021

This workaround works for expressions only. In the parseConfig, which handles the body, a newline is meaningful and cannot be added.

Sad. So the only thing is to fix it in hcl library?

@wata727
Copy link
Member

wata727 commented Jan 23, 2021

In my understanding, there is no problem in parsing the body (doesn't need the workaround). What problems are you facing?

@syndicut
Copy link
Contributor Author

In my understanding, there is no problem in parsing the body (doesn't need the workaround). What problems are you facing?

I'm trying to load Config() from plugin and have something like this in terraform configs:

resource "some_resource" "foo" {
  field         = <<EOF
contents
EOF
}

It causes error "Unterminated template string; No closing marker was found for the string."

@wata727
Copy link
Member

wata727 commented Jan 24, 2021

Hmm, I can't reproduce the error with the above configuration. Could you share a minimal reproduction code?

@syndicut
Copy link
Contributor Author

Hmm, I can't reproduce the error with the above configuration. Could you share a minimal reproduction code?

  1. Checkout code from this branch (I added Config loading rule to testing plugin for simplicity):
    https://github.com/syndicut/tflint/tree/heredoc-bug
  2. Build and install tflint itself
make install
  1. Build and install plugin (for unknown reason I cannot reproduce this bug in e2e tests)
mkdir -p ~/.tflint.d/plugins
cd plugin/stub-generator/sources/testing
go build -o ~/.tflint.d/plugins/tflint-ruleset-testing
  1. Go to heredoc sample in integration testing
cd integrationtest/inspection/heredoc/
  1. Run tflint
tflint 
Failed to check ruleset. An error occurred:

Error: Failed to check `config_example` rule: template.tf:4,4-4: Unterminated template string; No closing marker was found for the string.

@syndicut
Copy link
Contributor Author

Some workaround I found so far:

resource "aws_instance" "foo" {
  instance_type = <<EOF
t2.micro
EOF
  name = "test"
}

This will not cause any errors

@wata727
Copy link
Member

wata727 commented Jan 24, 2021

Understood. Sure, this seems to be the same problem as hashicorp/hcl#441. When transferring blocks, the inside of the brackets is sent to the plugins, so the newline is lost in the pattern where heredoc is declared in the end.

To avoid this, I think it is necessary to add a newline only when the end of the block contents is heredoc, but it seems a little difficult. It may be possible to implement a workaround, but it may require changes to the essential mechanics, such as #89

@syndicut
Copy link
Contributor Author

Understood. Sure, this seems to be the same problem as hashicorp/hcl#441. When transferring blocks, the inside of the brackets is sent to the plugins, so the newline is lost in the pattern where heredoc is declared in the end.

To avoid this, I think it is necessary to add a newline only when the end of the block contents is heredoc, but it seems a little difficult. It may be possible to implement a workaround, but it may require changes to the essential mechanics, such as #89

Added some hackish and fragile way to guess if we need to add a newline based on diagnostics output

Copy link
Member

@wata727 wata727 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants