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

Detect & exclude unsupported Terraform versions #361

Merged
merged 1 commit into from
Apr 20, 2021
Merged

Conversation

sundowndev
Copy link
Contributor

@sundowndev sundowndev commented Mar 23, 2021

Q A
πŸ› Bug fix? yes
πŸš€ New feature? no
⚠ Deprecations? yes
❌ BC Break no
πŸ”— Related issues #340
❓ Documentation yes

Description

The goal of this patch is to warn the user about the state they provided in case it's from an unsupported version of Terraform.

It is expected that running driftctl against a terraform state generated by an unsupported version should not be possible and be reported clearly to the user.

In the case of multiple Terraform versions inside a set of tfstates, they should be ignored so at least some execution can happen. This is probably to combine with the future state exclusion feature.

Implementation

I used go-version by hashicorp to perform the constraint check on Terraform version state. After parsing the state, the version is checked against the constraint (>= 0.11). If the constraint is satisfied, the scan can continue as usual. If it doesn't, a warning is displayed to tell the user this particular state was generated using an unsupported version of Terraform. The scan continue and the ignored state appears as drifted with 0% coverage.

The version constraints are stored in a variable called UnsupportedVersionConstraints :

var (
	UnsupportedVersionConstraints = []string{"<0.11.0", ">=0.13, <0.14"}
)

Example :

$ driftctl scan --from tfstate://terraform1.tfstate 
WARNING: terraform1.tfstate was generated using Terraform 0.10.8 which is currently not supported by driftctl
Please read documentation at https://docs.driftctl.com/0.7.0/limitations
Scanning resources: ⣟ (36)
Found unmanaged resources:
  aws_ebs_volume:
    - vol-01bd44304179ba354
    - vol-00fe1a5a8af4c6ef5
  aws_s3_bucket:
    - 09721498761983
    - 09721498761982
    - thisisatestbucket1298
    - thisisatestbucket1398
  aws_instance:
    - i-0ca7afc4ff96d6c4b
Found 7 resource(s)
 - 0% coverage
 - 0 covered by IaC
 - 7 not covered by IaC
 - 0 deleted on cloud provider
 - 0/0 drifted from IaC
exit status 1

About constraints

I still need to figure out which version of Terraform is supported or not. But as stated in #340, we won't support versions below 0.11. So for now, the unsupported constraints are <0.11. This patch assume we can have multiple constraints in the future, so it's possible to add a version constraint at any moment :

  • <0.11, =0.13.2 Exclude versions below 0.11 and 0.13.2
  • =0.9, =0.10 Exclude versions 0.9.0 and 0.10.0
  • <0.11, >=0.13, <0.14 Exclude versions below 0.11 and all 0.13

EDIT: I tested with TF v0.11, v0.14.8, & 0.14.9 and it does not appear to have incompatible behavior. May be we need more tests to ensure we fully support versions >=0.11.

Known limitations

Although it's simple to add a constraint at any time, it's important to note the limitation about this feature. If we excludes versions below 0.11, the constraint will looks like this : <0.11. But we'll not be able to include any version below 0.11. Also, if we decide to exclude a specific minor version, it's possible to do so using this syntax : >=0.13, <0.14

@sundowndev sundowndev added the kind/enhancement New feature or improvement label Mar 23, 2021
@sundowndev sundowndev self-assigned this Mar 23, 2021
@codecov
Copy link

codecov bot commented Mar 23, 2021

Codecov Report

Merging #361 (9084d4e) into v0.8 (1924900) will increase coverage by 0.01%.
The diff coverage is 78.26%.

Impacted file tree graph

@@            Coverage Diff             @@
##             v0.8     #361      +/-   ##
==========================================
+ Coverage   70.73%   70.74%   +0.01%     
==========================================
  Files         284      285       +1     
  Lines        6396     6417      +21     
==========================================
+ Hits         4524     4540      +16     
- Misses       1504     1508       +4     
- Partials      368      369       +1     
Impacted Files Coverage Ξ”
pkg/iac/terraform/state/terraform_state_reader.go 68.27% <58.33%> (-1.36%) ⬇️
pkg/iac/terraform/state/versions.go 100.00% <100.00%> (ΓΈ)

@sundowndev sundowndev marked this pull request as ready for review March 25, 2021 16:09
@sundowndev sundowndev requested a review from a team as a code owner March 25, 2021 16:09
moadibfr
moadibfr previously approved these changes Mar 29, 2021
@sundowndev sundowndev added this to Review in driftctl via automation Apr 8, 2021
@sundowndev sundowndev added this to the v0.8.0 milestone Apr 8, 2021
Copy link
Contributor

@wbeuil wbeuil left a comment

Choose a reason for hiding this comment

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

Good work, I have 2 small comments

const TerraformStateReaderSupplier = "tfstate"
const (
TerraformStateReaderSupplier = "tfstate"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to do that ? Or it is the linter that did this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's just me, I'll revert this change


func (u *UnsupportedVersionError) Error() string {
return fmt.Sprintf("%s was generated using Terraform %s which is currently not supported by driftctl\n"+
"Please read documentation at https://docs.driftctl.com/0.7.0/limitations", u.StateFile, u.Version)
Copy link
Contributor

Choose a reason for hiding this comment

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

We're gonna switch the link to the deep link one https://docs.driftctl.com/limitations

wbeuil
wbeuil previously approved these changes Apr 12, 2021
Copy link
Contributor

@wbeuil wbeuil left a comment

Choose a reason for hiding this comment

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

LGTM

@eliecharra
Copy link
Contributor

To rebase on v0.8 and change target branch

@sundowndev sundowndev changed the base branch from main to v0.8 April 16, 2021 14:09
@sundowndev sundowndev merged commit 055a0ca into v0.8 Apr 20, 2021
driftctl automation moved this from Review to Done Apr 20, 2021
@sundowndev sundowndev deleted the feat/unsupportedTF branch April 20, 2021 12:43
@sundowndev sundowndev moved this from Review to Done in driftctl May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or improvement
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants