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

support import block for tf 1.5.0+ #1339

Merged
merged 2 commits into from
Jun 16, 2023

Conversation

nishigori
Copy link
Contributor

docs:

New import block was added from Terraform 1.5.0
I added type support in a form similar to moved block.

@CLAassistant
Copy link

CLAassistant commented Jun 13, 2023

CLA assistant check
All committers have signed the CLA.

Comment on lines 41 to 45
import {

}

Copy link
Member

Choose a reason for hiding this comment

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

I see that you've added this but none of the test assertions were modified. How can we test this change?

Copy link
Contributor Author

@nishigori nishigori Jun 15, 2023

Choose a reason for hiding this comment

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

Without the implementation code, the test will fail.
Here, as with the moved block, this means that the import block is not a syntax error.

Like:

# Case: Only changed test code
$ git diff origin/master
diff --git a/pkg/scanners/terraform/parser/parser_test.go b/pkg/scanners/terraform/parser/parser_test.go
index 8f693a0d..ebef5458 100644
--- a/pkg/scanners/terraform/parser/parser_test.go
+++ b/pkg/scanners/terraform/parser/parser_test.go
@@ -38,6 +38,10 @@ moved {

 }

+import {
+
+}
+
 resource "cats_cat" "mittens" {
        name = "mittens"
        special = true

$ make test
...
--- FAIL: Test_BasicParsing (0.00s)
    parser_test.go:68: test.tf:19,1-7: Unsupported block type; Blocks of type "import" are not expected here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@simar7 Do you mean you want more tests? (For example, for attributes in an import block)

Copy link
Member

Choose a reason for hiding this comment

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

@simar7 Do you mean you want more tests? (For example, for attributes in an import block)

Yes that's right. I meant that we should test this import block functionality by having attributes in the import block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added there testing from 4ca5e4f

@simar7
Copy link
Member

simar7 commented Jun 15, 2023

Just reading the release notes and it seems this feature isn't fully ready just yet?

This is an early version of the import block feature, for which we are actively seeking user feedback to shape future development. The import block currently does not support interpolation in the id field, which must be a string.

If that's the case, I propose we should wait until they have finalized the spec for it.

@nishigori
Copy link
Contributor Author

@simar7 Yes, it seems this feature isn't fully ready just yet.
My suggestion is to have defsec support just to say that the import block exists.
This is because tfsec is giving me an error and I want to suppress it ( https://github.com/aquasecurity/tfsec/issues/2070 )

@simar7 simar7 force-pushed the more-tf-type-import-block branch from 4ca5e4f to fa744b9 Compare June 16, 2023 21:30
@simar7
Copy link
Member

simar7 commented Jun 16, 2023

@simar7 Yes, it seems this feature isn't fully ready just yet.
My suggestion is to have defsec support just to say that the import block exists.
This is because tfsec is giving me an error and I want to suppress it ( aquasecurity/tfsec#2070 )

I see - that's a great point. Your suggestion makes sense to include this in. Thanks for the PR!

@simar7 simar7 self-requested a review June 16, 2023 21:54
@simar7
Copy link
Member

simar7 commented Jun 16, 2023

Associated issue for tracking aquasecurity/trivy#4657

@simar7 simar7 merged commit 269528c into aquasecurity:master Jun 16, 2023
@nishigori nishigori deleted the more-tf-type-import-block branch June 17, 2023 03:57
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.

3 participants