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

Add tf 0 12 support #12

Merged
merged 6 commits into from
Apr 23, 2020
Merged

Add tf 0 12 support #12

merged 6 commits into from
Apr 23, 2020

Conversation

eversC
Copy link
Contributor

@eversC eversC commented Apr 22, 2020

Adds support for terraform v0.12

On the face of it this will look like a tonne of changes so I'll make an attempt at summarising:

  • refactored most regex processing logic out of the main() func into various dedicated functions. I tried to keep as much code as I could the same when copying over.
  • added struct called match which holds all of the regex group matches from retfplanline. This wasn't exactly necessary but felt cleaner holding it all in its own struct
  • added an expression struct, which holds regexes that differ between versions of terraform.
  • added a versionedExpressions map which maps terraform version to an expression
  • added a switch via env var TFENV so users can use tf 0.12 (defaults to tf 0.11)
  • regexes for tf 0.12 (was planning on trying to make the original regexes applicable to both versions but it became too cumbersome)
  • unit tests for both tf 0.11 & 0.12 in main_tests.go

In terms of testing, I've done a diff (see attached) of the outputs from running make test with tf 0.11 in the /tests directory, on the latest binary you've released Vs. a binary I've built from my branch. I think it's come out well; just a few differences where the id of the random_string.some_password is different (which is fine).
tf-011-old-vs-new-diff.txt

For what it's worth, I've also run make test with tf 0.12 (see attached output). This looks okay to me but might need some sanity checking.
tf012_test.txt

@osterman
Copy link
Contributor

@eversC this looks amazing! Thanks so much for biting the bullet and helping us upgrade this.

@osterman
Copy link
Contributor

@eversC this part looks suspect: should id there be masked?

image

@osterman
Copy link
Contributor

osterman commented Apr 22, 2020

fixes #6

@osterman osterman self-requested a review April 23, 2020 13:50
@eversC
Copy link
Contributor Author

eversC commented Apr 23, 2020

Hi @osterman 👋 , thanks for getting back to me so quickly.

@eversC this part looks suspect: should id there be masked?

image

That looks like a bit of an oversight on my part, as it does seem id should be masked. It looks like tfmask currently exhibits that behaviour too, though, unless I'm missing something?

@eversC
Copy link
Contributor Author

eversC commented Apr 23, 2020

I made an additional commit, 2633d0e, which I believe fixes the random_string.some_password.id exposure issue.

I just updated the tfmaskValuesRegex and tfmaskResourceRegex regexes to include "id" and "random_string" respectively.

@osterman
Copy link
Contributor

It looks like tfmask currently exhibits that behaviour too, though, unless I'm missing something?

Yea, not improbable =)

main.go Outdated Show resolved Hide resolved
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.

LGTM! Changed default version of terraform to 0.12. Will merge and cut a release. Thanks for this!

@osterman osterman merged commit 0c7da96 into cloudposse-archives:master Apr 23, 2020
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.

2 participants