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

Hard to reach terraform_wrapper: false from CDKTF error #338

Closed
LumaKernel opened this issue Jul 26, 2023 · 6 comments
Closed

Hard to reach terraform_wrapper: false from CDKTF error #338

LumaKernel opened this issue Jul 26, 2023 · 6 comments

Comments

@LumaKernel
Copy link

LumaKernel commented Jul 26, 2023

Situation

  • Run cdktf get in GitHub Actions
    • It's working in local, but show errors like following.
> cdktf get

SyntaxError: Unexpected token c in JSON at position 1
    at JSON.parse (<anonymous>)
    at /home/runner/work/<REDACTED>/node_modules/cdktf-cli/bundle/bin/cmds/handlers.js:94:49074
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async KM (/home/runner/work/<REDACTED>/node_modules/cdktf-cli/bundle/bin/cmds/handlers.js:45:1221)
    at async c$ (/home/runner/work/<REDACTED>/node_modules/cdktf-cli/bundle/bin/cmds/handlers.js:94:48908)
    at async _$.generateTypescriptProvider (/home/runner/work/<REDACTED>/node_modules/cdktf-cli/bundle/bin/cmds/handlers.js:96:6837)
    at async _$.generateTypescript (/home/runner/work/<REDACTED>/node_modules/cdktf-cli/bundle/bin/cmds/handlers.js:96:8841)
    at async Promise.all (index 3)
    at async _$.generate (/home/runner/work/<REDACTED>/node_modules/cdktf-cli/bundle/bin/cmds/handlers.js:99:372)
    at async qDe (/home/runner/work/<REDACTED>/node_modules/cdktf-cli/bundle/bin/cmds/handlers.js:176:8163)
⠹ downloading and generating modules and providers...
SyntaxError: Unexpected token c in JSON at position 1
npm ERR! Lifecycle script `get` failed with error: 
npm ERR! Error: command failed 
npm ERR!   in workspace: <REDACTED> 
npm ERR!   at location: /home/runner/work/<REDACTED>

Here is no hint to know when it gets c character while it's parsing JSON.

I checked handlers.js:94:49074 and I figured out that the original code is this.

https://github.com/hashicorp/terraform-cdk/blob/49a7072948a115ef20ea5ebe62f8d04117de81c9/packages/%40cdktf/provider-generator/lib/get/generator/provider-schema.ts#L309-L313

    providerSchema = JSON.parse(
      await exec(terraformBinaryName, ["providers", "schema", "-json"], {
        cwd: outdir,
      })
    ) as ProviderSchema;

Then I tried terraform providers schema -json in GitHub Actions and get it's wrapping the CLI.

$ terraform providers schema -json
[command]/home/runner/work/_temp/e3701105-a0d4-426d-80e2-575b823dd917/terraform-bin providers schema -json
{"format_version":"1.0"}
::debug::Terraform exited with code 0.
::debug::stdout: {"format_version":"1.0"}%0A
::debug::stderr:
::debug::exitcode: 0
::set-output name=stdout::{"format_version":"1.0"}%0A
::set-output name=stderr::
::set-output name=exitcode::0

Now we get the character c is coming from [command]

Solution candidates

I have no right solution for this, but have ideas.

  • Defaulting to terraform_wrapper: false
    • simple
  • Having 2 binaries, one is terraform which is not wrapped, and one another terraform-wrapped which is wrapped
  • Make the ability to force disabling wrapping even if wrapped like with environment var like NO_GHA_TERRAFORM_WRAP=1, and setting this from CDKTF side to prevent this potential unrecognizable barrier for CDKTF starters.

I like third solution. How about that?

@bflad
Copy link
Member

bflad commented Jul 26, 2023

Hi @LumaKernel 👋 Thank you for reporting this and sorry you are running into trouble here. Could you please show us the full workflow configuration that led to this particular error?

There have been some internal discussions previously about removing the Terraform wrapper as the default (leaving this action only responsible for installation and, potentially, another action for running commands in the GitHub Actions context with setting outputs, etc).

@OJFord
Copy link
Contributor

OJFord commented Aug 18, 2023

Making it default-false at least sounds like an excellent idea - it's hard to discover that it's the cause even simply trying to use terraform output -json | jq.

@iselo
Copy link

iselo commented Sep 11, 2023

Hi there. Is any updates on this?
I've got the same error at GitHub Actions on calling toBeValidTerraform() with TypeScript.

const tempOutDir = Testing.fullSynth(terraformStack);
console.log(toBeValidTerraform(tempOutDir).message);
console.log(toPlanSuccessfully(tempOutDir).message);

terraform 1.5.7

  1. Ubuntu case (toPlan issue):
Node 18.17.1
Expected subject to be a valid terraform stack: SyntaxError: Unexpected token c in JSON at position 1.
Expected subject not to plan successfully

Node 20.6.1
subject to be a valid terraform stack: SyntaxError: Unexpected token 'c', "[command]/h"... is not valid JSON.
Expected subject not to plan successfully
  1. macOS case (toBeValid issue):
Node 18.17.1
Expected subject to be a valid terraform stack: SyntaxError: Unexpected token c in JSON at position 1.
Expected subject to plan successfully: Error: Command failed: terraform plan -input=false -lock=false .

Node 20.6.1
subject to be a valid terraform stack: SyntaxError: Unexpected token 'c', "[command]/U"... is not valid JSON.
Expected subject to plan successfully: Error: Command failed: terraform plan -input=false -lock=false .

It works locally macOS Terraform v1.5.7 on darwin_amd64.

@Danielku15
Copy link

This also leads for me to subsequent errors when using the CDK https://github.com/hashicorp/terraform-cdk-action

Was hunting down for hours this problem until I realized that this problem is there. I was able to workaround this problem with terraform_wrapper: false. I don't need the outputs.

OJFord added a commit to OJFord/setup-terraform that referenced this issue Oct 25, 2023
Presently using a command such as `terraform output -json | jq` does not
work with the wrapper enabled, as it is by default.

In order to consume terraform's output having set it up with this
Action, it is necessary either to disable the wrapper (`with:
terraform_wrapper: false`) or run it in its own Actions step with an
explicit `id` (e.g. `id: foo`) so that it can be referred to and consumed
(`${{steps.foo.outputs.stdout}}` et al.) in later steps.

This seems to be the result of much confusion (issues passim) and is not
at all easy (hashicorp#338) to debug/diagnose and come to the realisation that
it's due to the wrapper, or even that such a thing exists.

This commit aims to address the issue by passing through stdout &
stderr, so that they're available in Unix pipelines and variable
assignment etc. as expected within a single step, while still retaining
the wrapper's listening behaviour to provide them as Actions outputs.

Closes hashicorp#20, hashicorp#80, hashicorp#85, hashicorp#149, hashicorp#338, and probably more.
OJFord added a commit to OJFord/setup-terraform that referenced this issue Oct 25, 2023
Presently using a command such as `terraform output -json | jq` does not
work with the wrapper enabled, as it is by default.

In order to consume terraform's output having set it up with this
Action, it is necessary either to disable the wrapper (`with:
terraform_wrapper: false`) or run it in its own Actions step with an
explicit `id` (e.g. `id: foo`) so that it can be referred to and consumed
(`${{steps.foo.outputs.stdout}}` et al.) in later steps.

This seems to be the result of much confusion (issues passim) and is not
at all easy (hashicorp#338) to debug/diagnose and come to the realisation that
it's due to the wrapper, or even that such a thing exists.

This commit aims to address the issue by passing through stdout &
stderr, so that they're available in Unix pipelines and variable
assignment etc. as expected within a single step, while still retaining
the wrapper's listening behaviour to provide them as Actions outputs.

Closes hashicorp#20, hashicorp#80, hashicorp#85, hashicorp#149, hashicorp#338, and probably more.
OJFord added a commit to OJFord/setup-terraform that referenced this issue Oct 26, 2023
Presently using a command such as `terraform output -json | jq` does not
work with the wrapper enabled, as it is by default.

In order to consume terraform's output having set it up with this
Action, it is necessary either to disable the wrapper (`with:
terraform_wrapper: false`) or run it in its own Actions step with an
explicit `id` (e.g. `id: foo`) so that it can be referred to and consumed
(`${{steps.foo.outputs.stdout}}` et al.) in later steps.

This seems to be the result of much confusion (issues passim) and is not
at all easy (hashicorp#338) to debug/diagnose and come to the realisation that
it's due to the wrapper, or even that such a thing exists.

@austinvalle identified the issue as being due to the `@actions/exec`
package writing the spawned command to stdout (along with then its
actual stdout). This has previously been reported upstream in
actions/toolkit#649; I've proposed actions/toolkit#1573 to fix it.

This commit aims to address the issue for `setup-terraform` in the
meantime by silencing `@actions/exec` and then writing out to stdout &
stderr from the listener buffers, which it writes to without this
additional logging.

Closes hashicorp#20, hashicorp#80, hashicorp#85, hashicorp#149, hashicorp#338, and probably more.
OJFord added a commit to OJFord/setup-terraform that referenced this issue Oct 26, 2023
Presently using a command such as `terraform output -json | jq` does not
work with the wrapper enabled, as it is by default.

In order to consume terraform's output having set it up with this
Action, it is necessary either to disable the wrapper (`with:
terraform_wrapper: false`) or run it in its own Actions step with an
explicit `id` (e.g. `id: foo`) so that it can be referred to and consumed
(`${{steps.foo.outputs.stdout}}` et al.) in later steps.

This seems to be the result of much confusion (issues passim) and is not
at all easy (hashicorp#338) to debug/diagnose and come to the realisation that
it's due to the wrapper, or even that such a thing exists.

@austinvalle identified the issue as being due to the `@actions/exec`
package writing the spawned command to stdout (along with then its
actual stdout). This has previously been reported upstream in
actions/toolkit#649; I've proposed actions/toolkit#1573 to fix it.

This commit aims to address the issue for `setup-terraform` in the
meantime by silencing `@actions/exec` and then writing out to stdout &
stderr from the listener buffers, which it writes to without this
additional logging.

Closes hashicorp#20, hashicorp#80, hashicorp#85, hashicorp#149, hashicorp#338, and probably more.
austinvalle pushed a commit to OJFord/setup-terraform that referenced this issue Oct 27, 2023
Presently using a command such as `terraform output -json | jq` does not
work with the wrapper enabled, as it is by default.

In order to consume terraform's output having set it up with this
Action, it is necessary either to disable the wrapper (`with:
terraform_wrapper: false`) or run it in its own Actions step with an
explicit `id` (e.g. `id: foo`) so that it can be referred to and consumed
(`${{steps.foo.outputs.stdout}}` et al.) in later steps.

This seems to be the result of much confusion (issues passim) and is not
at all easy (hashicorp#338) to debug/diagnose and come to the realisation that
it's due to the wrapper, or even that such a thing exists.

@austinvalle identified the issue as being due to the `@actions/exec`
package writing the spawned command to stdout (along with then its
actual stdout). This has previously been reported upstream in
actions/toolkit#649; I've proposed actions/toolkit#1573 to fix it.

This commit aims to address the issue for `setup-terraform` in the
meantime by silencing `@actions/exec` and then writing out to stdout &
stderr from the listener buffers, which it writes to without this
additional logging.

Closes hashicorp#20, hashicorp#80, hashicorp#85, hashicorp#149, hashicorp#338, and probably more.
austinvalle added a commit that referenced this issue Oct 27, 2023
* Fix output malformed when wrapper enabled

Presently using a command such as `terraform output -json | jq` does not
work with the wrapper enabled, as it is by default.

In order to consume terraform's output having set it up with this
Action, it is necessary either to disable the wrapper (`with:
terraform_wrapper: false`) or run it in its own Actions step with an
explicit `id` (e.g. `id: foo`) so that it can be referred to and consumed
(`${{steps.foo.outputs.stdout}}` et al.) in later steps.

This seems to be the result of much confusion (issues passim) and is not
at all easy (#338) to debug/diagnose and come to the realisation that
it's due to the wrapper, or even that such a thing exists.

@austinvalle identified the issue as being due to the `@actions/exec`
package writing the spawned command to stdout (along with then its
actual stdout). This has previously been reported upstream in
actions/toolkit#649; I've proposed actions/toolkit#1573 to fix it.

This commit aims to address the issue for `setup-terraform` in the
meantime by silencing `@actions/exec` and then writing out to stdout &
stderr from the listener buffers, which it writes to without this
additional logging.

Closes #20, #80, #85, #149, #338, and probably more.

* add test for stdout with jq

* update test name

* remove debug lines and add changelog

* add additional note about the bug fix to wrapper

---------

Co-authored-by: Austin Valle <austinvalle@gmail.com>
@austinvalle
Copy link
Member

This issue will be fixed in an upcoming v3.0.0 release next week. Please see this comment for more info

Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants