Added terraform v14 support besides v12. #470
Conversation
devang-gaur
requested review from
cesar-rodriguez,
kanchwala-yusuf and
williepaul
Codecov Report
@@ Coverage Diff @@
## master #470 +/- ##
==========================================
- Coverage 74.84% 73.48% -1.37%
==========================================
Files 88 93 +5
Lines 2051 2074 +23
==========================================
- Hits 1535 1524 -11
- Misses 382 418 +36
+ Partials 134 132 -2
|
devang-gaur
force-pushed
the
tf14besides12
branch
from
2a3e837
to
6770311
Compare
go.mod
Outdated
| @@ -1,31 +1,37 @@ | |||
| module github.com/accurics/terrascan | |||
|
|
|||
| go 1.15 | |||
| go 1.14 | |||
There was a problem hiding this comment.
You're moving to an earlier version of go?
| @@ -14,7 +14,7 @@ | |||
| limitations under the License. | |||
| */ | |||
|
|
|||
| package tfv12 | |||
| package commons | |||
There was a problem hiding this comment.
The package naming here may be misleading later on if breaking changes are introduced... These still depend on some HCL libs. If HCLv3 is added, or if we say need to support TF 0.11, I'm not sure this would fit. @kanchwala-yusuf --would it may make more sense to name this based on the HCL version supported? Does this portion of the code also depend on the tf version used?
There was a problem hiding this comment.
@williepaul why would be support tf 0.11 ? We'll support future versions right? And the way how golang doesn't allow to use multiple minor versions of a dependency simultaneously, I think we'll have to move with next future releases of terraform as well IMHO
There was a problem hiding this comment.
And this commons folder is inside iac-providers/terraform/. its not a general commons for all iac-providers.. So I think if we'll have to refactor things in future. It will not have impact outside of tf iac support
There was a problem hiding this comment.
@williepaul, let's keep it simple and straight forward and not try to future proof it. I think, commons package works, and it is within terraform package itself so shouldn't be harmful
There was a problem hiding this comment.
@dev-gaur @kanchwala-yusuf I strongly disagree with the precedent--as a scanning tool, we should aim to scan IaC of all different versions and types. Why bother having an option to specify different versions, if we don't actually support different versions? Think from a user/customer perspective, because that's most important here. If a tool I am using to scan my files all of a sudden drops support for all my 1000 v12 IaC files I already have created, what do you think I'm going to do? Upgrade all of my IaC to v14, or find a tool that works with v12? The latter would be my first choice. Enterprise users are especially concerned about how often support is dropped. Sure, you can argue they can stick with older versions of the tool, but that brings in a separate maintenance issue for us as well, because situations can arise where we need to patch older versions (such as usage of old libraries that may have been affected by a vulnerability). Additionally, updating policies becomes another issue.
When it comes to tf11, in the commercial product, we support tf11, we have some customers using tf11, so supporting tf11 may not be just an option--it may be a requirement.
As far as the naming goes, it's not that big of a deal, but my thoughts are just that when new (and incompatible) versions are released, the "commons" name will be misleading...
| @@ -43,7 +51,7 @@ func TestNewIacProvider(t *testing.T) { | |||
| { | |||
| name: "not supported iac type", | |||
| iacType: "not-supported", | |||
| iacVersion: terraformV12, | |||
There was a problem hiding this comment.
In our test code, rather than just changing all references from 12 to 14, is it possible to pass this as a parameter, then run it against multiple versions? While our code doesn't change, we just want to make sure our interaction with the library doesn't change when different versions are used either.
devang-gaur
force-pushed
the
tf14besides12
branch
from
6770311
to
2d1b86c
Compare
devang-gaur
force-pushed
the
tf14besides12
branch
from
2d1b86c
to
ac81360
Compare
devang-gaur
force-pushed
the
tf14besides12
branch
from
ac81360
to
b1c5360
Compare
docs/getting-started/usage.md
Outdated
| @@ -118,7 +118,7 @@ Flags: | |||
| -d, --iac-dir string path to a directory containing one or more IaC files (default ".") | |||
| -f, --iac-file string path to a single IaC file | |||
| -i, --iac-type string iac type (helm, k8s, kustomize, terraform) | |||
| --iac-version string iac version (helm: v3, k8s: v1, kustomize: v3, terraform: v12) | |||
| --iac-version string iac version (helm: v3, k8s: v1, kustomize: v3, terraform: v14) | |||
There was a problem hiding this comment.
We are still supporting v12 as well, we should specify that in the help as well
devang-gaur
force-pushed
the
tf14besides12
branch
from
ee97d5a
to
a359304
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
Fix: potential bug added in PR #470
PLEASE NOTE : Users will experience breaking changes while using Terraform 0.11 files, after this PR is merged.
Fixes #324