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 for loop option in tags block #556

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

EstyFridman1
Copy link

@EstyFridman1 EstyFridman1 commented Aug 4, 2024

User description

This feature introduces support for loops within Terraform configurations
fixes:#496


Generated description

Dear maintainer, below is a concise technical summary of the changes proposed in this PR:

Introduce a new feature in the TerraformParser class to support for loops within the tags block of Terraform configurations. This enhancement allows users to dynamically generate tags using a for loop, improving flexibility in tag management. The modifyBlockTags and parseTagAttribute functions have been updated to handle the new for loop syntax. Additionally, test cases have been added to ensure the correct parsing and handling of tags with for loops.

TopicDetails
For Loop Support Introduce support for for loops in the TerraformParser class to enhance tag management flexibility.
Modified files (3)
  • src/terraform/structure/terraform_parser.go
  • tests/terraform/forLoop/main.tf
  • src/terraform/structure/terraform_parser_test.go
Latest Contributors(2)
EmailCommitDate
57212002+ChanochShayne...lint-fixes-542July 25, 2024
150965779+chanaMovshow...Skip-Comment-Feature-530June 30, 2024
Dependency Updates Update dependencies in go.mod to ensure compatibility and leverage new features.
Modified files (1)
  • go.mod
Latest Contributors(2)
EmailCommitDate
47931006+RabeaZr@users...go-git-vulnerability-483March 07, 2024
[email protected]Extend-to-support-conf...April 30, 2023
This pull request is reviewed by Baz. Join @EstyFridman1 and the rest of your team on (Baz).

@EstyFridman1 EstyFridman1 marked this pull request as draft August 4, 2024 21:28
@EstyFridman1 EstyFridman1 marked this pull request as ready for review August 5, 2024 06:04
@EstyFridman1
Copy link
Author

@ChanochShayner can you review it please?
thanks

Copy link
Contributor

@ChanochShayner ChanochShayner left a comment

Choose a reason for hiding this comment

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

Very nice!
Please fix your lint errors.

@@ -799,7 +815,38 @@ func getUncloseBracketsCount(bracketsCounters map[hclsyntax.TokenType]int) int {
return sum
}

func (p *TerraformParser) parseTagAttribute(tokens hclwrite.Tokens) map[string]string {
func (p *TerraformParser) parseTagAttribute(tokens hclwrite.Tokens) (map[string]string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why change this func signature to return 2 args?

Copy link
Author

Choose a reason for hiding this comment

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

@ChanochShayner
the unmarshal function can return error so i think that need to add option to return error
but if you think that it unnecessary i can remove it:
do you want to replace this:
err := dethcl.Unmarshal([]byte(hclBytes), hclData)
to this:
dethcl.Unmarshal([]byte(hclBytes), hclData)

Copy link
Author

Choose a reason for hiding this comment

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

@@ -50,19 +50,31 @@ func TestTerraformParser_SkipResourceByComment(t *testing.T) {
assert.Empty(t, p.GetSkipResourcesByComment())
})

t.Run("One resource with skip comment, only that resource added to skipResourcesByComment slice", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove this test?

Copy link
Author

Choose a reason for hiding this comment

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

if tagsAttribute != nil {
tagsTokens := tagsAttribute.Expr().BuildTokens(hclwrite.Tokens{})
parsedTags, _ := parser.parseTagAttribute(tagsTokens)
if block.GetResourceName() == "bucket_var_tags" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we test more than just this resource?

Copy link
Author

Choose a reason for hiding this comment

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

@ChanochShayner
Yes we can test also another resource
but we need to compare it with another expected result

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me

@ChanochShayner
Copy link
Contributor

@EstyFridman1 you still have some lint issues.

Copy link

stale bot commented Sep 22, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants