coinbase / terraform-landscape

Improve Terraform's plan output to be easier to read and understand
Apache License 2.0
1.58k stars 111 forks source link

ruby 2.7 emits warnings about terraform-landscape variable names #111

Closed handlerbot closed 3 years ago

handlerbot commented 4 years ago
> terraform show -no-color PLAN | landscape
(eval):293: warning: `_1' is reserved for numbered parameter; consider another name
(eval):305: warning: `_2' is reserved for numbered parameter; consider another name
(eval):313: warning: `_3' is reserved for numbered parameter; consider another name
(eval):340: warning: `_1' is reserved for numbered parameter; consider another name
(eval):352: warning: `_2' is reserved for numbered parameter; consider another name
[...]

https://blog.bigbinary.com/2020/03/03/ruby-2-7-introduces-numbered-parameters-as-default-block-parameters.html

sds commented 4 years ago

Hey @handlerbot, thanks for opening an issue.

Can you provide a redacted/simplified Terraform plan output that reproduces this issue so we can investigate? Thank you!

handlerbot commented 4 years ago

Hi @sds Sure, I can work on that soon.

sds commented 4 years ago

Still waiting on this @handlerbot, when you get a moment.

calve commented 4 years ago

I get the exact same error;

$ ~/Code/terraform011 plan > /tmp/plan
$ ~/.gem/ruby/2.7.0/bin/landscape < /tmp/plan 
(eval):293: warning: `_1' is reserved for numbered parameter; consider another name
(eval):305: warning: `_2' is reserved for numbered parameter; consider another name
(eval):313: warning: `_3' is reserved for numbered parameter; consider another name
(eval):340: warning: `_1' is reserved for numbered parameter; consider another name
(eval):352: warning: `_2' is reserved for numbered parameter; consider another name
~ module.db.module.db_instance.aws_db_instance.this
    backup_retention_period:   "7" => "35"

Plan: 0 to add, 1 to change, 0 to destroy.
$ cat /tmp/plan 
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

random_string.db_initial_password: Refreshing state... redacted
data.terraform_remote_state.prod_iam: Refreshing state...
data.terraform_remote_state.tools_sgs: Refreshing state...
data.terraform_remote_state.vpc: Refreshing state...
data.terraform_remote_state.sgs: Refreshing state...
data.terraform_remote_state.organization: Refreshing state...
data.null_data_source.sources[0]: Refreshing state...
data.null_data_source.sources[1]: Refreshing state...
data.null_data_source.sources: Refreshing state...
data.null_data_source.sources[1]: Refreshing state...
data.null_data_source.sources[0]: Refreshing state...
data.null_data_source.sources[2]: Refreshing state...
aws_security_group_rule.sgs-ingress: Refreshing state... (ID: sgrule-819837139)
aws_security_group_rule.sgs-egress: Refreshing state... (ID: sgrule-2307230748)
aws_security_group_rule.sgs-egress[1]: Refreshing state... (ID: sgrule-1839092727)
aws_security_group_rule.sgs-egress[0]: Refreshing state... (ID: sgrule-2543219078)
aws_security_group_rule.sgs-ingress-only: Refreshing state... (ID: sgrule-2399062927)
aws_security_group_rule.sgs-egress: Refreshing state... (ID: sgrule-3274778740)
data.aws_security_group.this: Refreshing state...
data.aws_iam_policy_document.db: Refreshing state...
data.aws_security_group.this: Refreshing state...
data.aws_security_group.this: Refreshing state...
aws_security_group_rule.sgs-ingress[0]: Refreshing state... (ID: sgrule-3264632796)
aws_security_group_rule.sgs-ingress[1]: Refreshing state... (ID: sgrule-2439793341)
aws_security_group_rule.sgs-ingress-only: Refreshing state... (ID: sgrule-897735583)
aws_security_group_rule.sgs-ingress: Refreshing state... (ID: sgrule-3076051546)
aws_kms_key.db: Refreshing state... (ID: b6274537-7ece-4db9-87d3-3a08679a047b)
aws_kms_alias.db: Refreshing state... (ID: alias/redacted/db/redacted)
aws_db_instance.this: Refreshing state... (ID: redacted)
null_resource.set_db_password: Refreshing state... (ID: 5725152425592639679)

------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  ~ module.db.module.db_instance.aws_db_instance.this
      backup_retention_period: "7" => "35"

Plan: 0 to add, 1 to change, 0 to destroy.

------------------------------------------------------------------------

Note: You didn't specify an "-out" parameter to save this plan, so Terraform
can't guarantee that exactly these actions will be performed if
"terraform apply" is subsequently run.

Releasing state lock. This may take a few moments...

I redacted a few values that should not be relevant here.

calve commented 4 years ago

As a workaround for future readers, you can set ruby warning verbosity to 0 and suppress the warning output

$ RUBYOPT="-W0" ~/.gem/ruby/2.7.0/bin/landscape < /tmp/plan
~ module.db.module.db_instance.aws_db_instance.this
    backup_retention_period:   "7" => "35"

Plan: 0 to add, 1 to change, 0 to destroy.

I do not know any way to specifically disable our warning.

Thanks for landscape, I greatly appreciate it.

SeekingMeaning commented 3 years ago

Hi, Homebrew maintainer here. We also get these messages but as syntax errors instead of warnings because we're updating Ruby to version 3.0.0.

% echo '+ some_resource_type.some_resource_name' | landscape
.../gems/treetop-1.6.10/lib/treetop/compiler/grammar_compiler.rb:50:in `class_eval': (eval):293: _1 is reserved for numbered parameter (SyntaxError)
(eval):305: _2 is reserved for numbered parameter
(eval):313: _3 is reserved for numbered parameter
(eval):340: _1 is reserved for numbered parameter
(eval):352: _2 is reserved for numbered parameter

With Ruby 2.7.2:

% echo '+ some_resource_type.some_resource_name' | landscape
(eval):293: warning: `_1' is reserved for numbered parameter; consider another name
(eval):305: warning: `_2' is reserved for numbered parameter; consider another name
(eval):313: warning: `_3' is reserved for numbered parameter; consider another name
(eval):340: warning: `_1' is reserved for numbered parameter; consider another name
(eval):352: warning: `_2' is reserved for numbered parameter; consider another name