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

Regex Update for New Resources in Terraform 0.12 #15

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

bgreg1012
Copy link

Hi, I have a use-case in Terraform 0.12 where I'm adding new resources and I found that the reTfPlanLine regex wasn't capturing lines like: + token = "abC123ABc". To fix this I added another capture group in reTfPlanLine to match the scenario where -> ... may not exist, and the logic to handle the different size capture groups.

@osterman
Copy link
Contributor

@eversC just a heads up ☝️ any suggestions?

@eversC
Copy link
Contributor

eversC commented May 29, 2020

Taking a look

For now, though, the test seems to fail:

$ go test -v

=== RUN   TestProcessLine
--- FAIL: TestProcessLine (0.00s)
    main_test.go:73: Got       ~ result           = "************" ->  -> (known after apply) *known after apply), want       ~ result           = "************" -> (known after apply)
    main_test.go:73: Got  ~ id =               "**************************************************************************************" ->  -> (known after apply)*known after apply), want  ~ id =               "**************************************************************************************" -> (known after apply)
=== RUN   TestGetCurrentResource
--- PASS: TestGetCurrentResource (0.00s)
=== RUN   TestPlanStatus
--- PASS: TestPlanStatus (0.00s)
=== RUN   TestMaskValue
--- PASS: TestMaskValue (0.00s)
FAIL
exit status 1
FAIL	_/tfmask	0.415s

@bgreg1012
Copy link
Author

Tests are failing? 🤔 When I run locally on that branch they pass:

=== RUN   TestProcessLine
--- PASS: TestProcessLine (0.00s)
=== RUN   TestGetCurrentResource
--- PASS: TestGetCurrentResource (0.00s)
=== RUN   TestPlanStatus
--- PASS: TestPlanStatus (0.00s)
=== RUN   TestMaskValue
--- PASS: TestMaskValue (0.00s)
PASS
ok  	github.com/bgreg1012/tfmask	0.030s```

@eversC
Copy link
Contributor

eversC commented May 29, 2020

@bgreg1012 you're right, I was running it on your master branch, which didn't have the 2nd commit 🤦

we should put this all in the makefile, but if you run

mkdir release
go build
mv tfmask release
cd tests
make test

I did a diff of the output of that on your branch, compared to cloudposse/tfmask, and it looks like values of the "random_id" resource are now revealed

@osterman
Copy link
Contributor

osterman commented Jun 5, 2020

and it looks like values of the "random_id" resource are now revealed

@bgreg1012 were you able to take a look at this?

@osterman
Copy link
Contributor

osterman commented Jul 1, 2020

I can confirm with @eversC there's a regression

image

Please note that running make -C tests test requires human spot-checking =/ not really good tests, just more like a sanity check.

Copy link
Contributor

@osterman osterman left a comment

Choose a reason for hiding this comment

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

Please check tests in tests/ folder as they are leaking secrets

@ricoli
Copy link

ricoli commented Jul 10, 2020

Hi @bgreg1012 , thanks for opening this - any chance you could look into the tests as mentioned by @osterman?

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

Successfully merging this pull request may close these issues.

4 participants