From 0359fa4c002ef620bc63ae829658dd3e9db37265 Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Thu, 6 Jun 2024 12:59:33 +0300 Subject: [PATCH 01/19] Added option to bypass automatic tagging by adding comment above specific resources. --- src/common/runner/runner.go | 1 + src/common/runner/runner_test.go | 17 ++++++- src/common/utils/utils.go | 6 +++ src/common/yaml/yaml_writer.go | 8 +++ src/common/yaml/yaml_writer_test.go | 32 ++++++++++++ src/terraform/structure/terraform_parser.go | 13 +++++ .../structure/terraform_parser_test.go | 50 +++++++++++++++++++ .../resources/skipComment/noSkip.yaml | 37 ++++++++++++++ .../resources/skipComment/skipAll.yaml | 38 ++++++++++++++ .../resources/skipComment/skipOne.yaml | 22 ++++++++ tests/integration/integration_test.go | 27 ++++++++++ tests/integration/skipComment/skipResource.tf | 24 +++++++++ .../integration/skipComment/skipResource.yaml | 25 ++++++++++ tests/terraform/skipComment/noSkip.tf | 15 ++++++ tests/terraform/skipComment/skipAll.tf | 16 ++++++ tests/terraform/skipComment/skipOne.tf | 15 ++++++ 16 files changed, 345 insertions(+), 1 deletion(-) create mode 100644 tests/cloudformation/resources/skipComment/noSkip.yaml create mode 100644 tests/cloudformation/resources/skipComment/skipAll.yaml create mode 100644 tests/cloudformation/resources/skipComment/skipOne.yaml create mode 100644 tests/integration/skipComment/skipResource.tf create mode 100644 tests/integration/skipComment/skipResource.yaml create mode 100644 tests/terraform/skipComment/noSkip.tf create mode 100644 tests/terraform/skipComment/skipAll.tf create mode 100644 tests/terraform/skipComment/skipOne.tf diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index e86937032..e2a4387f9 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -181,6 +181,7 @@ func (r *Runner) TagFile(file string) { logger.Info(fmt.Sprintf("Failed to parse file %v with parser %v", file, reflect.TypeOf(parser))) continue } + utils.AppendSkipedByCommentToRunnerSkippedResources(&r.skippedResources) isFileTaggable := false for _, block := range blocks { if r.isSkippedResourceType(block.GetResourceType()) { diff --git a/src/common/runner/runner_test.go b/src/common/runner/runner_test.go index 05fe706b1..9d8b768f6 100644 --- a/src/common/runner/runner_test.go +++ b/src/common/runner/runner_test.go @@ -2,13 +2,14 @@ package runner import ( "fmt" - "github.com/bridgecrewio/yor/src/common/tagging/tags" "os" "path/filepath" "strings" "testing" "time" + "github.com/bridgecrewio/yor/src/common/tagging/tags" + cloudformationStructure "github.com/bridgecrewio/yor/src/cloudformation/structure" "github.com/bridgecrewio/yor/src/common/clioptions" "github.com/bridgecrewio/yor/src/common/gitservice" @@ -221,6 +222,20 @@ func TestRunnerInternals(t *testing.T) { assert.NotContains(t, output, "aws_s3_bucket.test-bucket") }) + t.Run("Test skip resource by comment", func(t *testing.T) { + options := clioptions.TagOptions{ + Directory: "../../../tests/terraform/skipComment/skipOne.tf", + Parsers: []string{"Terraform"}, + } + runner := Runner{} + runner.Init(&options) + runner.parsers[0].ParseFile(options.Directory) + utils.AppendSkipedByCommentToRunnerSkippedResources(&runner.skippedResources) + result := make([]string, 0) + result = append(result, "aws_instance.example_instance") + assert.Equal(t, result, runner.skippedResources) + }) + t.Run("Test skip resource - cloudformation", func(t *testing.T) { runner := Runner{} rootDir := "../../../tests/cloudformation" diff --git a/src/common/utils/utils.go b/src/common/utils/utils.go index 81002a1be..46c3bcc7e 100644 --- a/src/common/utils/utils.go +++ b/src/common/utils/utils.go @@ -17,6 +17,12 @@ import ( // RemoveGcpInvalidChars Source of regex: https://cloud.google.com/compute/docs/labeling-resources var RemoveGcpInvalidChars = regexp.MustCompile(`[^\p{Ll}\p{Lo}\p{N}_-]`) +var SkipResourcesByComment = make([]string, 0) + +func AppendSkipedByCommentToRunnerSkippedResources(skippedResources *[]string) { + *skippedResources = append(*skippedResources, SkipResourcesByComment...) + SkipResourcesByComment = SkipResourcesByComment[:0] +} func InSlice[T comparable](elems []T, v T) bool { for _, s := range elems { diff --git a/src/common/yaml/yaml_writer.go b/src/common/yaml/yaml_writer.go index eac890360..7c8ce6d56 100644 --- a/src/common/yaml/yaml_writer.go +++ b/src/common/yaml/yaml_writer.go @@ -287,12 +287,20 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar for i, line := range fileLines { cleanContent := strings.TrimSpace(line) if strings.HasPrefix(cleanContent, resourcesStartToken+":") { + if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIPALL" { + utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, resourceNames...) + } readResources = true resourcesIndent = countLeadingSpaces(line) continue } if readResources { + if i > 0 { + if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIP" { + utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, strings.Trim(strings.TrimSpace(line), ":")) + } + } lineIndent := countLeadingSpaces(line) if lineIndent <= resourcesIndent && strings.TrimSpace(line) != "" && !strings.Contains(line, "#") { // No longer inside resources block, get the last line of the previous resource if exists diff --git a/src/common/yaml/yaml_writer_test.go b/src/common/yaml/yaml_writer_test.go index a83b7925c..53d74490d 100644 --- a/src/common/yaml/yaml_writer_test.go +++ b/src/common/yaml/yaml_writer_test.go @@ -11,6 +11,7 @@ import ( "github.com/bridgecrewio/yor/src/common/structure" "github.com/bridgecrewio/yor/src/common/tagging/simple" "github.com/bridgecrewio/yor/src/common/tagging/tags" + "github.com/bridgecrewio/yor/src/common/utils" "github.com/stretchr/testify/assert" ) @@ -241,3 +242,34 @@ func TestTagReplacement(t *testing.T) { assert.Equal(t, *res["attribute"], structure.Lines{Start: 40, End: 53}) }) } + +func TestYaml_ResorceSkipTagging(t *testing.T) { + t.Run("Test some resources with skip comment added to utils.SkipResourcesByComment", func(t *testing.T) { + filePath := "../../../tests/cloudformation/resources/SkipComment/skipOne.yaml" + resorseSkip := []string{"NewVolume"} + expectedResourceNames := []string{"NewVolume", "NewVolume2"} + MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Equal(t, utils.SkipResourcesByComment, resorseSkip) + assert.NotEqual(t, utils.SkipResourcesByComment, "NewVolume2") + defer resetSkipArr() + }) + t.Run("All resources with skip comment added to utils.SkipResourcesByComment", func(t *testing.T) { + filePath := "../../../tests/cloudformation/resources/SkipComment/skipAll.yaml" + resorseSkip := []string{"NewVolume", "NewVolume2"} + expectedResourceNames := []string{"NewVolume", "NewVolume2"} + MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Equal(t, utils.SkipResourcesByComment, resorseSkip) + defer resetSkipArr() + }) + t.Run("No resources with skip all comment in the file, utils.SkipResourcesByComment should be empty", func(t *testing.T) { + filePath := "../../../tests/cloudformation/resources/SkipComment/noSkip.yaml" + expectedResourceNames := []string{"NewVolume"} + MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Empty(t, utils.SkipResourcesByComment) + defer resetSkipArr() + }) +} + +func resetSkipArr() { + utils.SkipResourcesByComment = []string{} +} diff --git a/src/terraform/structure/terraform_parser.go b/src/terraform/structure/terraform_parser.go index e5f18dde7..665ffe9c0 100644 --- a/src/terraform/structure/terraform_parser.go +++ b/src/terraform/structure/terraform_parser.go @@ -132,6 +132,7 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, error) if err != nil { return nil, fmt.Errorf("failed to read file %s because %s", filePath, err) } + lines := strings.Split(string(src), "\n") // parse the file into hclwrite.File and hclsyntax.File to allow getting existing tags and lines hclFile, diagnostics := hclwrite.ParseConfig(src, filePath, hcl.InitialPos) @@ -151,6 +152,7 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, error) syntaxBlocks := hclSyntaxFile.Body.(*hclsyntax.Body).Blocks + skipAll := false rawBlocks := hclFile.Body().Blocks() parsedBlocks := make([]structure.IBlock, 0) for i, block := range rawBlocks { @@ -174,6 +176,17 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, error) } terraformBlock.Init(filePath, block) terraformBlock.AddHclSyntaxBlock(syntaxBlocks[i]) + line := terraformBlock.GetLines().Start + if line > 1 && line <= len(lines) { + lineAbove := lines[line-2] + if strings.ToUpper(strings.TrimSpace(lineAbove))== "#YOR:SKIPALL" { + skipAll = true + } + + if strings.ToUpper(strings.TrimSpace(lineAbove)) == "#YOR:SKIP" || skipAll { + utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, terraformBlock.GetResourceID()) + } + } parsedBlocks = append(parsedBlocks, terraformBlock) } diff --git a/src/terraform/structure/terraform_parser_test.go b/src/terraform/structure/terraform_parser_test.go index 4e0545922..06e69fa38 100644 --- a/src/terraform/structure/terraform_parser_test.go +++ b/src/terraform/structure/terraform_parser_test.go @@ -22,6 +22,52 @@ import ( "github.com/stretchr/testify/assert" ) +func TestTerraformParser_SkipResourceByComment(t *testing.T) { + t.Run("SkipAll comment added all resources to utils.SkipResourcesByComment", func(t *testing.T) { + // Initialize TerraformParser and parse file with all resources containing skip comment + p := &TerraformParser{} + p.Init("../../../tests/terraform/skipComment/", nil) + defer p.Close() + filePath := "../../../tests/terraform/skipComment/skipAll.tf" + _, err := p.ParseFile(filePath) + if err != nil { + t.Errorf("failed to read hcl file because %s", err) + } + exceptedSkipResources := []string{"aws_vpc.example_vpc", "aws_subnet.example_subnet", "aws_instance.example_instance"} + assert.Equal(t, exceptedSkipResources, utils.SkipResourcesByComment) + defer resetSkipArr() + }) + + t.Run("No resources with skip comment in the file, utils.SkipResourcesByComment should be empty", func(t *testing.T) { + // Initialize TerraformParser and parse file with no skip tags + p := &TerraformParser{} + p.Init("../../../tests/terraform/skipComment/", nil) + defer p.Close() + filePath := "../../../tests/terraform/skipComment/noSkip.tf" + _, err := p.ParseFile(filePath) + if err != nil { + t.Errorf("failed to read hcl file because %s", err) + } + assert.Empty(t, utils.SkipResourcesByComment) + defer resetSkipArr() + }) + + t.Run("One resource with skip comment, only that resource added to utils.SkipResourcesByComment", func(t *testing.T) { + // Initialize TerraformParser and parse file with one resource containing skip tag + p := &TerraformParser{} + p.Init("../../../tests/terraform/skipComment/", nil) + defer p.Close() + filePath := "../../../tests/terraform/skipComment/skipOne.tf" + _, err := p.ParseFile(filePath) + if err != nil { + t.Errorf("failed to read hcl file because %s", err) + } + exceptedSkipResources := []string{"aws_instance.example_instance"} + assert.Equal(t, exceptedSkipResources, utils.SkipResourcesByComment) + defer resetSkipArr() + }) +} + func TestTerraformParser_ParseFile(t *testing.T) { t.Run("parse aws eks file", func(t *testing.T) { p := &TerraformParser{} @@ -787,3 +833,7 @@ func compareTokenArrays(got []hclwrite.Tokens, want []hclwrite.Tokens) bool { return true } + +func resetSkipArr() { + utils.SkipResourcesByComment = []string{} +} diff --git a/tests/cloudformation/resources/skipComment/noSkip.yaml b/tests/cloudformation/resources/skipComment/noSkip.yaml new file mode 100644 index 000000000..78e26b489 --- /dev/null +++ b/tests/cloudformation/resources/skipComment/noSkip.yaml @@ -0,0 +1,37 @@ +AWSTemplateFormatVersion: '2010-09-09' +Description: Sample EBS Volume with EC2 instance template +Resources: +#YOR:SKIP + NewVolume: + Type: AWS::EC2::Volume + Properties: + Size: 100 + Encrypted: true + #Encrypted: false + Tags: + - Key: MyTag + Value: TagValue + - Key: Name + Value: !Ref EnvironmentName + AvailabilityZone: us-west-2a + DeletionPolicy: Snapshot + + NewVolume2: + Type: AWS::EC2::Volume + Properties: + Size: 100 + Encrypted: true + #Encrypted: false + Tags: + - Key: MyTag + Value: TagValue + - Key: Name + Value: !Ref EnvironmentName + AvailabilityZone: us-west-2a + DeletionPolicy: Snapshot + +Outputs: + VolumeId: + Value: !Ref NewVolume + Export: + Name: NewVolumeId \ No newline at end of file diff --git a/tests/cloudformation/resources/skipComment/skipAll.yaml b/tests/cloudformation/resources/skipComment/skipAll.yaml new file mode 100644 index 000000000..4e84696b0 --- /dev/null +++ b/tests/cloudformation/resources/skipComment/skipAll.yaml @@ -0,0 +1,38 @@ + +AWSTemplateFormatVersion: '2010-09-09' +Description: Sample EBS Volume with EC2 instance template +#yor:skipAll +Resources: + NewVolume: + Type: AWS::EC2::Volume + Properties: + Size: 100 + Encrypted: true + #Encrypted: false + Tags: + - Key: MyTag + Value: TagValue + - Key: Name + Value: !Ref EnvironmentName + AvailabilityZone: us-west-2a + DeletionPolicy: Snapshot + + NewVolume2: + Type: AWS::EC2::Volume + Properties: + Size: 100 + Encrypted: true + #Encrypted: false + Tags: + - Key: MyTag + Value: TagValue + - Key: Name + Value: !Ref EnvironmentName + AvailabilityZone: us-west-2a + DeletionPolicy: Snapshot + +Outputs: + VolumeId: + Value: !Ref NewVolume + Export: + Name: NewVolumeId diff --git a/tests/cloudformation/resources/skipComment/skipOne.yaml b/tests/cloudformation/resources/skipComment/skipOne.yaml new file mode 100644 index 000000000..f2b12c348 --- /dev/null +++ b/tests/cloudformation/resources/skipComment/skipOne.yaml @@ -0,0 +1,22 @@ +AWSTemplateFormatVersion: '2010-09-09' +Description: Sample EBS Volume with EC2 instance template +Resources: + NewVolume: + Type: AWS::EC2::Volume + Properties: + Size: 100 + Encrypted: true + #Encrypted: false + Tags: + - Key: MyTag + Value: TagValue + - Key: Name + Value: !Ref EnvironmentName + AvailabilityZone: us-west-2a + DeletionPolicy: Snapshot + +Outputs: + VolumeId: + Value: !Ref NewVolume + Export: + Name: NewVolumeId \ No newline at end of file diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 935ee04c1..c59be75dc 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -254,6 +254,33 @@ func TestRunResults(t *testing.T) { } } }) + + t.Run("Test tagging terraform and cloudFormation files and skip resource by comment", func(t *testing.T) { + yorRunner := runner.Runner{} + err := yorRunner.Init(&clioptions.TagOptions{ + Directory: ".\\skipComment", + TagGroups: getTagGroups(), + Parsers: []string{"Terraform", "CloudFormation"}, + }) + tfFileBytes, _ := os.ReadFile("skipComment\\skipResource.tf") + yamlFileBytes, _ := os.ReadFile("skipComment\\skipResource.yaml") + defer func() { + _ = os.WriteFile(".\\skipComment\\skipResource.tf", tfFileBytes, 0644) + _ = os.WriteFile(".\\skipComment\\skipResource.yaml", yamlFileBytes, 0644) + }() + failIfErr(t, err) + reportService, err := yorRunner.TagDirectory() + failIfErr(t, err) + + reportService.CreateReport() + report := reportService.GetReport() + + newTags := report.NewResourceTags + for _, newTag := range newTags { + assert.NotEqual(t, "aws_vpc.example_vpc", newTag.ResourceID) + assert.NotEqual(t, "NewVolume1", newTag.ResourceID) + } + }) } func TestTagUncommittedResults(t *testing.T) { diff --git a/tests/integration/skipComment/skipResource.tf b/tests/integration/skipComment/skipResource.tf new file mode 100644 index 000000000..26c2f2237 --- /dev/null +++ b/tests/integration/skipComment/skipResource.tf @@ -0,0 +1,24 @@ +#yor:skip +resource "aws_vpc" "example_vpc" { + cidr_block = "10.0.0.0/16" + tags = { + } +} + +resource "aws_subnet" "example_subnet" { + vpc_id = aws_vpc.example_vpc.id + cidr_block = "10.0.1.0/24" + availability_zone = "us-west-1a" + tags = { + yor_name = "example_subnet" + yor_trace = "74091a97-d11a-4500-a0c3-af942a0d8e00" + } +} + +resource "aws_instance" "example_instance" { + ami = "ami-0c55b159cbfafe1f0" + instance_type = "t2.micro" + subnet_id = aws_subnet.example_subnet.id + tags = { + } +} \ No newline at end of file diff --git a/tests/integration/skipComment/skipResource.yaml b/tests/integration/skipComment/skipResource.yaml new file mode 100644 index 000000000..f4148e308 --- /dev/null +++ b/tests/integration/skipComment/skipResource.yaml @@ -0,0 +1,25 @@ +AWSTemplateFormatVersion: '2010-09-09' +Description: Sample EBS Volume with EC2 instance template +Resources: + NewVolume: + Type: AWS::EC2::Volume + Properties: + Size: 100 + Encrypted: true + #Encrypted: false + AvailabilityZone: us-west-2a + Tags: + - Key: yor_trace + Value: d5e1032c-34e9-428d-8b17-4dff36d05e68 + - Key: yor_name + Value: NewVolume +#yor:skip + NewVolume1: + Type: AWS::EC2::Volume + Properties: + Size: 100 + Encrypted: true + #Encrypted: false + AvailabilityZone: us-west-2a + Tags: + DeletionPolicy: Snapshot \ No newline at end of file diff --git a/tests/terraform/skipComment/noSkip.tf b/tests/terraform/skipComment/noSkip.tf new file mode 100644 index 000000000..0e999b427 --- /dev/null +++ b/tests/terraform/skipComment/noSkip.tf @@ -0,0 +1,15 @@ +resource "aws_vpc" "example_vpc" { + cidr_block = "10.0.0.0/16" +} + +resource "aws_subnet" "example_subnet" { + vpc_id = aws_vpc.example_vpc.id + cidr_block = "10.0.1.0/24" + availability_zone = "us-west-1a" +} + +resource "aws_instance" "example_instance" { + ami = "ami-0c55b159cbfafe1f0" + instance_type = "t2.micro" + subnet_id = aws_subnet.example_subnet.id +} diff --git a/tests/terraform/skipComment/skipAll.tf b/tests/terraform/skipComment/skipAll.tf new file mode 100644 index 000000000..cf8a7f4e3 --- /dev/null +++ b/tests/terraform/skipComment/skipAll.tf @@ -0,0 +1,16 @@ +#YOR:SKIPALL +resource "aws_vpc" "example_vpc" { + cidr_block = "10.0.0.0/16" +} + +resource "aws_subnet" "example_subnet" { + vpc_id = aws_vpc.example_vpc.id + cidr_block = "10.0.1.0/24" + availability_zone = "us-west-1a" +} + +resource "aws_instance" "example_instance" { + ami = "ami-0c55b159cbfafe1f0" + instance_type = "t2.micro" + subnet_id = aws_subnet.example_subnet.id +} \ No newline at end of file diff --git a/tests/terraform/skipComment/skipOne.tf b/tests/terraform/skipComment/skipOne.tf new file mode 100644 index 000000000..619b9ed96 --- /dev/null +++ b/tests/terraform/skipComment/skipOne.tf @@ -0,0 +1,15 @@ +resource "aws_vpc" "example_vpc" { + cidr_block = "10.0.0.0/16" +} + +resource "aws_subnet" "example_subnet" { + vpc_id = aws_vpc.example_vpc.id + cidr_block = "10.0.1.0/24" + availability_zone = "us-west-1a" +} +#yor:Skip +resource "aws_instance" "example_instance" { + ami = "ami-0c55b159cbfafe1f0" + instance_type = "t2.micro" + subnet_id = aws_subnet.example_subnet.id +} \ No newline at end of file From 1d9dce72a8f636ec56ffea965add2ca12ec1c68a Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Thu, 6 Jun 2024 14:22:58 +0300 Subject: [PATCH 02/19] Fix of UT --- src/common/yaml/yaml_writer_test.go | 6 +++--- tests/cloudformation/resources/skipComment/noSkip.yaml | 1 - tests/cloudformation/resources/skipComment/skipOne.yaml | 1 + 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/yaml/yaml_writer_test.go b/src/common/yaml/yaml_writer_test.go index 53d74490d..7421369a5 100644 --- a/src/common/yaml/yaml_writer_test.go +++ b/src/common/yaml/yaml_writer_test.go @@ -245,7 +245,7 @@ func TestTagReplacement(t *testing.T) { func TestYaml_ResorceSkipTagging(t *testing.T) { t.Run("Test some resources with skip comment added to utils.SkipResourcesByComment", func(t *testing.T) { - filePath := "../../../tests/cloudformation/resources/SkipComment/skipOne.yaml" + filePath := "../../../tests/cloudformation/resources/skipComment/skipOne.yaml" resorseSkip := []string{"NewVolume"} expectedResourceNames := []string{"NewVolume", "NewVolume2"} MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") @@ -254,7 +254,7 @@ func TestYaml_ResorceSkipTagging(t *testing.T) { defer resetSkipArr() }) t.Run("All resources with skip comment added to utils.SkipResourcesByComment", func(t *testing.T) { - filePath := "../../../tests/cloudformation/resources/SkipComment/skipAll.yaml" + filePath := "../../../tests/cloudformation/resources/skipComment/skipAll.yaml" resorseSkip := []string{"NewVolume", "NewVolume2"} expectedResourceNames := []string{"NewVolume", "NewVolume2"} MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") @@ -262,7 +262,7 @@ func TestYaml_ResorceSkipTagging(t *testing.T) { defer resetSkipArr() }) t.Run("No resources with skip all comment in the file, utils.SkipResourcesByComment should be empty", func(t *testing.T) { - filePath := "../../../tests/cloudformation/resources/SkipComment/noSkip.yaml" + filePath := "../../../tests/cloudformation/resources/skipComment/noSkip.yaml" expectedResourceNames := []string{"NewVolume"} MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") assert.Empty(t, utils.SkipResourcesByComment) diff --git a/tests/cloudformation/resources/skipComment/noSkip.yaml b/tests/cloudformation/resources/skipComment/noSkip.yaml index 78e26b489..b56b83fc4 100644 --- a/tests/cloudformation/resources/skipComment/noSkip.yaml +++ b/tests/cloudformation/resources/skipComment/noSkip.yaml @@ -1,7 +1,6 @@ AWSTemplateFormatVersion: '2010-09-09' Description: Sample EBS Volume with EC2 instance template Resources: -#YOR:SKIP NewVolume: Type: AWS::EC2::Volume Properties: diff --git a/tests/cloudformation/resources/skipComment/skipOne.yaml b/tests/cloudformation/resources/skipComment/skipOne.yaml index f2b12c348..a8017e5a7 100644 --- a/tests/cloudformation/resources/skipComment/skipOne.yaml +++ b/tests/cloudformation/resources/skipComment/skipOne.yaml @@ -1,6 +1,7 @@ AWSTemplateFormatVersion: '2010-09-09' Description: Sample EBS Volume with EC2 instance template Resources: +#yor:skip NewVolume: Type: AWS::EC2::Volume Properties: From c44c9685d1004855e113514074a6d3ab07b2e20d Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Sun, 9 Jun 2024 13:58:30 +0300 Subject: [PATCH 03/19] Fix integration and go-lint tests --- src/common/runner/runner_test.go | 30 ++++++++++++--------- src/common/utils/utils.go | 5 ++++ src/terraform/structure/terraform_parser.go | 2 +- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/common/runner/runner_test.go b/src/common/runner/runner_test.go index 9d8b768f6..4cbb1173e 100644 --- a/src/common/runner/runner_test.go +++ b/src/common/runner/runner_test.go @@ -223,18 +223,24 @@ func TestRunnerInternals(t *testing.T) { }) t.Run("Test skip resource by comment", func(t *testing.T) { - options := clioptions.TagOptions{ - Directory: "../../../tests/terraform/skipComment/skipOne.tf", - Parsers: []string{"Terraform"}, - } - runner := Runner{} - runner.Init(&options) - runner.parsers[0].ParseFile(options.Directory) - utils.AppendSkipedByCommentToRunnerSkippedResources(&runner.skippedResources) - result := make([]string, 0) - result = append(result, "aws_instance.example_instance") - assert.Equal(t, result, runner.skippedResources) - }) + options := clioptions.TagOptions{ + Directory: "../../../tests/terraform/skipComment/skipOne.tf", + Parsers: []string{"Terraform"}, + } + runner := Runner{} + err := runner.Init(&options) + if err != nil { + t.Error(err) + } + tfBlocks, err := runner.parsers[0].ParseFile(options.Directory) + if err != nil { + t.Error(err) + } + assert.Equal(t, 3, len(tfBlocks)) + utils.AppendSkipedByCommentToRunnerSkippedResources(&runner.skippedResources) + result := []string{"aws_instance.example_instance"} + assert.Equal(t, result, runner.skippedResources) + }) t.Run("Test skip resource - cloudformation", func(t *testing.T) { runner := Runner{} diff --git a/src/common/utils/utils.go b/src/common/utils/utils.go index 46c3bcc7e..c0fb23185 100644 --- a/src/common/utils/utils.go +++ b/src/common/utils/utils.go @@ -9,6 +9,7 @@ import ( "regexp" "strings" "unicode" + "sync" "github.com/bridgecrewio/yor/src/common" "github.com/bridgecrewio/yor/src/common/logger" @@ -20,6 +21,10 @@ var RemoveGcpInvalidChars = regexp.MustCompile(`[^\p{Ll}\p{Lo}\p{N}_-]`) var SkipResourcesByComment = make([]string, 0) func AppendSkipedByCommentToRunnerSkippedResources(skippedResources *[]string) { + var mutex sync.Mutex + mutex.Lock() + defer mutex.Unlock() + *skippedResources = append(*skippedResources, SkipResourcesByComment...) SkipResourcesByComment = SkipResourcesByComment[:0] } diff --git a/src/terraform/structure/terraform_parser.go b/src/terraform/structure/terraform_parser.go index 665ffe9c0..e8879b8a8 100644 --- a/src/terraform/structure/terraform_parser.go +++ b/src/terraform/structure/terraform_parser.go @@ -179,7 +179,7 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, error) line := terraformBlock.GetLines().Start if line > 1 && line <= len(lines) { lineAbove := lines[line-2] - if strings.ToUpper(strings.TrimSpace(lineAbove))== "#YOR:SKIPALL" { + if strings.ToUpper(strings.TrimSpace(lineAbove)) == "#YOR:SKIPALL" { skipAll = true } From 1fe6d9f552cfd7bdc5b73391b5b4e00386758b08 Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Sun, 9 Jun 2024 14:45:42 +0300 Subject: [PATCH 04/19] Fix integration_test and go-lint tests 2 --- src/common/runner/runner_test.go | 34 ++++++++++----------- src/common/utils/utils.go | 2 +- src/terraform/structure/terraform_parser.go | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/common/runner/runner_test.go b/src/common/runner/runner_test.go index 4cbb1173e..71094f424 100644 --- a/src/common/runner/runner_test.go +++ b/src/common/runner/runner_test.go @@ -223,24 +223,24 @@ func TestRunnerInternals(t *testing.T) { }) t.Run("Test skip resource by comment", func(t *testing.T) { - options := clioptions.TagOptions{ - Directory: "../../../tests/terraform/skipComment/skipOne.tf", - Parsers: []string{"Terraform"}, - } - runner := Runner{} - err := runner.Init(&options) - if err != nil { - t.Error(err) - } - tfBlocks, err := runner.parsers[0].ParseFile(options.Directory) - if err != nil { - t.Error(err) - } - assert.Equal(t, 3, len(tfBlocks)) - utils.AppendSkipedByCommentToRunnerSkippedResources(&runner.skippedResources) + options := clioptions.TagOptions{ + Directory: "../../../tests/terraform/skipComment/skipOne.tf", + Parsers: []string{"Terraform"}, + } + runner := Runner{} + err := runner.Init(&options) + if err != nil { + t.Error(err) + } + tfBlocks, err := runner.parsers[0].ParseFile(options.Directory) + if err != nil { + t.Error(err) + } + assert.Equal(t, 3, len(tfBlocks)) + utils.AppendSkipedByCommentToRunnerSkippedResources(&runner.skippedResources) result := []string{"aws_instance.example_instance"} - assert.Equal(t, result, runner.skippedResources) - }) + assert.Equal(t, result, runner.skippedResources) + }) t.Run("Test skip resource - cloudformation", func(t *testing.T) { runner := Runner{} diff --git a/src/common/utils/utils.go b/src/common/utils/utils.go index c0fb23185..28c95a4d0 100644 --- a/src/common/utils/utils.go +++ b/src/common/utils/utils.go @@ -19,9 +19,9 @@ import ( // RemoveGcpInvalidChars Source of regex: https://cloud.google.com/compute/docs/labeling-resources var RemoveGcpInvalidChars = regexp.MustCompile(`[^\p{Ll}\p{Lo}\p{N}_-]`) var SkipResourcesByComment = make([]string, 0) +var mutex sync.Mutex func AppendSkipedByCommentToRunnerSkippedResources(skippedResources *[]string) { - var mutex sync.Mutex mutex.Lock() defer mutex.Unlock() diff --git a/src/terraform/structure/terraform_parser.go b/src/terraform/structure/terraform_parser.go index e8879b8a8..84f412445 100644 --- a/src/terraform/structure/terraform_parser.go +++ b/src/terraform/structure/terraform_parser.go @@ -182,7 +182,7 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, error) if strings.ToUpper(strings.TrimSpace(lineAbove)) == "#YOR:SKIPALL" { skipAll = true } - + if strings.ToUpper(strings.TrimSpace(lineAbove)) == "#YOR:SKIP" || skipAll { utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, terraformBlock.GetResourceID()) } From cb7e4a567559882a4931af5ec14cea9a3d83925b Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Sun, 9 Jun 2024 15:28:37 +0300 Subject: [PATCH 05/19] fix integration test3 --- src/common/utils/utils.go | 3 +-- src/common/yaml/yaml_writer.go | 6 ++++++ src/terraform/structure/terraform_parser.go | 3 +++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/common/utils/utils.go b/src/common/utils/utils.go index 28c95a4d0..d92f00969 100644 --- a/src/common/utils/utils.go +++ b/src/common/utils/utils.go @@ -23,10 +23,9 @@ var mutex sync.Mutex func AppendSkipedByCommentToRunnerSkippedResources(skippedResources *[]string) { mutex.Lock() - defer mutex.Unlock() - *skippedResources = append(*skippedResources, SkipResourcesByComment...) SkipResourcesByComment = SkipResourcesByComment[:0] + mutex.Unlock() } func InSlice[T comparable](elems []T, v T) bool { diff --git a/src/common/yaml/yaml_writer.go b/src/common/yaml/yaml_writer.go index 7c8ce6d56..aa6f57a12 100644 --- a/src/common/yaml/yaml_writer.go +++ b/src/common/yaml/yaml_writer.go @@ -7,6 +7,7 @@ import ( "regexp" "sort" "strings" + "sync" "github.com/bridgecrewio/yor/src/common/logger" "github.com/bridgecrewio/yor/src/common/structure" @@ -16,6 +17,7 @@ import ( ) const SingleIndent = " " +var mutex sync.Mutex func WriteYAMLFile(readFilePath string, blocks []structure.IBlock, writeFilePath string, tagsAttributeName string, resourcesStartToken string) error { // #nosec G304 @@ -288,7 +290,9 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar cleanContent := strings.TrimSpace(line) if strings.HasPrefix(cleanContent, resourcesStartToken+":") { if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIPALL" { + mutex.Lock() utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, resourceNames...) + mutex.Unlock() } readResources = true resourcesIndent = countLeadingSpaces(line) @@ -298,7 +302,9 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar if readResources { if i > 0 { if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIP" { + mutex.Lock() utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, strings.Trim(strings.TrimSpace(line), ":")) + mutex.Unlock() } } lineIndent := countLeadingSpaces(line) diff --git a/src/terraform/structure/terraform_parser.go b/src/terraform/structure/terraform_parser.go index 84f412445..eae513aca 100644 --- a/src/terraform/structure/terraform_parser.go +++ b/src/terraform/structure/terraform_parser.go @@ -25,6 +25,7 @@ import ( ) var ignoredDirs = []string{".git", ".DS_Store", ".idea", ".terraform"} +var mutex sync.Mutex var unsupportedTerraformBlocks = []string{ "aws_autoscaling_group", // This resource specifically supports tags with a different structure, see: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/autoscaling_group#tag-and-tags "aws_lb_listener", // This resource does not support tags, although docs state otherwise. @@ -184,7 +185,9 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, error) } if strings.ToUpper(strings.TrimSpace(lineAbove)) == "#YOR:SKIP" || skipAll { + mutex.Lock() utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, terraformBlock.GetResourceID()) + mutex.Unlock() } } parsedBlocks = append(parsedBlocks, terraformBlock) From 17234700216fae47a9faff52f09067a26ce151fb Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Mon, 10 Jun 2024 10:50:39 +0300 Subject: [PATCH 06/19] Fix integration_test and lint4 --- src/common/color_check.go | 55 ++++++++++++------------ src/common/reports/report_service.go | 38 ++++++++-------- src/common/reports/results_test.go | 2 +- src/common/tagging/external/tag_group.go | 2 +- src/common/utils/utils.go | 2 +- src/common/yaml/yaml_writer.go | 5 ++- tests/integration/integration_test.go | 7 ++- tests/utils/utils.go | 4 +- 8 files changed, 59 insertions(+), 56 deletions(-) diff --git a/src/common/color_check.go b/src/common/color_check.go index 5247adc18..ad2f36b62 100644 --- a/src/common/color_check.go +++ b/src/common/color_check.go @@ -1,34 +1,33 @@ package common type ColorStruct struct { - NoColor bool - Reset string - Green string - Yellow string - Blue string - Purple string + NoColor bool + Reset string + Green string + Yellow string + Blue string + Purple string } -func NoColorCheck (NoColorBool bool) *ColorStruct { - var colors ColorStruct - colors = ColorStruct{ - NoColor : true, - Reset : "", - Green : "", - Yellow : "", - Blue : "", - Purple : "", - } - if !NoColorBool { - colors = ColorStruct{ - NoColor : false, - Reset : "\033[0m", - Green : "\033[32m", - Yellow : "\033[33m", - Blue : "\033[34m", - Purple : "\033[35m", - } - } - return &colors +func NoColorCheck(NoColorBool bool) *ColorStruct { + var colors ColorStruct + colors = ColorStruct{ + NoColor: true, + Reset: "", + Green: "", + Yellow: "", + Blue: "", + Purple: "", + } + if !NoColorBool { + colors = ColorStruct{ + NoColor: false, + Reset: "\033[0m", + Green: "\033[32m", + Yellow: "\033[33m", + Blue: "\033[34m", + Purple: "\033[35m", + } + } + return &colors } - diff --git a/src/common/reports/report_service.go b/src/common/reports/report_service.go index b87236a6a..e66f800a3 100644 --- a/src/common/reports/report_service.go +++ b/src/common/reports/report_service.go @@ -149,16 +149,16 @@ func (r *ReportService) printUpdatedResourcesToStdout(colors *common.ColorStruct fmt.Print(colors.Green, fmt.Sprintf("Updated Resource Traces (%v):\n", r.report.Summary.UpdatedResources), colors.Reset) table := tablewriter.NewWriter(os.Stdout) table.SetHeader([]string{"File", "Resource", "Tag Key", "Old Value", "Updated Value", "Yor ID"}) - if !colors.NoColor { - table.SetColumnColor( - tablewriter.Colors{}, - tablewriter.Colors{}, - tablewriter.Colors{tablewriter.Bold}, - tablewriter.Colors{tablewriter.Normal, tablewriter.FgRedColor}, - tablewriter.Colors{tablewriter.Normal, tablewriter.FgGreenColor}, - tablewriter.Colors{}, - ) - } + if !colors.NoColor { + table.SetColumnColor( + tablewriter.Colors{}, + tablewriter.Colors{}, + tablewriter.Colors{tablewriter.Bold}, + tablewriter.Colors{tablewriter.Normal, tablewriter.FgRedColor}, + tablewriter.Colors{tablewriter.Normal, tablewriter.FgGreenColor}, + tablewriter.Colors{}, + ) + } table.SetRowLine(true) table.SetRowSeparator("-") @@ -176,15 +176,15 @@ func (r *ReportService) printNewResourcesToStdout(colors *common.ColorStruct) { table.SetHeader([]string{"File", "Resource", "Tag Key", "Tag Value", "Yor ID"}) table.SetRowLine(true) table.SetRowSeparator("-") - if !colors.NoColor { - table.SetColumnColor( - tablewriter.Colors{}, - tablewriter.Colors{}, - tablewriter.Colors{tablewriter.Bold}, - tablewriter.Colors{tablewriter.Normal, tablewriter.FgGreenColor}, - tablewriter.Colors{}, - ) - } + if !colors.NoColor { + table.SetColumnColor( + tablewriter.Colors{}, + tablewriter.Colors{}, + tablewriter.Colors{tablewriter.Bold}, + tablewriter.Colors{tablewriter.Normal, tablewriter.FgGreenColor}, + tablewriter.Colors{}, + ) + } for _, tr := range r.report.NewResourceTags { table.Append([]string{tr.File, tr.ResourceID, tr.TagKey, tr.UpdatedValue, tr.YorTraceID}) } diff --git a/src/common/reports/results_test.go b/src/common/reports/results_test.go index 000ae4a9a..f6cd502a4 100644 --- a/src/common/reports/results_test.go +++ b/src/common/reports/results_test.go @@ -120,7 +120,7 @@ func TestResultsGeneration(t *testing.T) { t.Run("Test CLI output structure", func(t *testing.T) { ReportServiceInst.CreateReport() - colors := common.NoColorCheck(false) + colors := common.NoColorCheck(false) output := utils.CaptureOutputColors(ReportServiceInst.PrintToStdout) lines := strings.Split(output, "\n") // Verify banner diff --git a/src/common/tagging/external/tag_group.go b/src/common/tagging/external/tag_group.go index e503882dd..2f21f8609 100644 --- a/src/common/tagging/external/tag_group.go +++ b/src/common/tagging/external/tag_group.go @@ -244,7 +244,7 @@ func (t *TagGroup) CalculateTagValue(block structure.IBlock, tag Tag) (tags.ITag } } if len(gitModifiersCounts) == 1 { - for k, _ := range gitModifiersCounts { + for k := range gitModifiersCounts { retTag.Value = evaluateTemplateVariable(k) break } diff --git a/src/common/utils/utils.go b/src/common/utils/utils.go index d92f00969..6661f0dfd 100644 --- a/src/common/utils/utils.go +++ b/src/common/utils/utils.go @@ -8,8 +8,8 @@ import ( "reflect" "regexp" "strings" - "unicode" "sync" + "unicode" "github.com/bridgecrewio/yor/src/common" "github.com/bridgecrewio/yor/src/common/logger" diff --git a/src/common/yaml/yaml_writer.go b/src/common/yaml/yaml_writer.go index aa6f57a12..9c97bb563 100644 --- a/src/common/yaml/yaml_writer.go +++ b/src/common/yaml/yaml_writer.go @@ -7,7 +7,7 @@ import ( "regexp" "sort" "strings" - "sync" + "sync" "github.com/bridgecrewio/yor/src/common/logger" "github.com/bridgecrewio/yor/src/common/structure" @@ -17,6 +17,7 @@ import ( ) const SingleIndent = " " + var mutex sync.Mutex func WriteYAMLFile(readFilePath string, blocks []structure.IBlock, writeFilePath string, tagsAttributeName string, resourcesStartToken string) error { @@ -302,7 +303,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar if readResources { if i > 0 { if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIP" { - mutex.Lock() + mutex.Lock() utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, strings.Trim(strings.TrimSpace(line), ":")) mutex.Unlock() } diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index c59be75dc..5bcd794c1 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -255,6 +255,9 @@ func TestRunResults(t *testing.T) { } }) +} + +func TestSkipResourcesByComment(t *testing.T) { t.Run("Test tagging terraform and cloudFormation files and skip resource by comment", func(t *testing.T) { yorRunner := runner.Runner{} err := yorRunner.Init(&clioptions.TagOptions{ @@ -262,8 +265,8 @@ func TestRunResults(t *testing.T) { TagGroups: getTagGroups(), Parsers: []string{"Terraform", "CloudFormation"}, }) - tfFileBytes, _ := os.ReadFile("skipComment\\skipResource.tf") - yamlFileBytes, _ := os.ReadFile("skipComment\\skipResource.yaml") + tfFileBytes, _ := os.ReadFile(".\\skipComment\\skipResource.tf") + yamlFileBytes, _ := os.ReadFile(".\\skipComment\\skipResource.yaml") defer func() { _ = os.WriteFile(".\\skipComment\\skipResource.tf", tfFileBytes, 0644) _ = os.WriteFile(".\\skipComment\\skipResource.yaml", yamlFileBytes, 0644) diff --git a/tests/utils/utils.go b/tests/utils/utils.go index 489c23c49..4ef37a5ba 100644 --- a/tests/utils/utils.go +++ b/tests/utils/utils.go @@ -6,7 +6,7 @@ import ( "log" "os" - "github.com/bridgecrewio/yor/src/common" + "github.com/bridgecrewio/yor/src/common" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" ) @@ -39,7 +39,7 @@ func CaptureOutput(f func()) string { } func CaptureOutputColors(f func(*common.ColorStruct)) string { - colors := common.NoColorCheck(false) + colors := common.NoColorCheck(false) reader, writer, err := os.Pipe() if err != nil { panic(err) From 3bc062df6411ded75c2477a28184dbea5986a52c Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Mon, 10 Jun 2024 13:25:56 +0300 Subject: [PATCH 07/19] "Another way to implement skipping resources tagging by comments" --- .../structure/cloudformation_parser.go | 20 +++--- .../structure/cloudformation_parser_test.go | 16 ++--- src/common/color_check.go | 4 +- src/common/json/json_writer.go | 7 +- src/common/parser.go | 2 +- src/common/runner/runner.go | 4 +- src/common/runner/runner_test.go | 7 +- src/common/tagging/external/tag_group.go | 4 +- src/common/utils/utils.go | 10 --- src/common/utils/utils_test.go | 4 +- src/common/yaml/yaml_writer.go | 21 +++--- src/common/yaml/yaml_writer_test.go | 34 ++++----- .../structure/serverless_block_test.go | 2 +- src/serverless/structure/serverless_parser.go | 17 ++--- .../structure/serverless_parser_test.go | 12 ++-- src/terraform/structure/terraform_parser.go | 27 ++++---- .../structure/terraform_parser_test.go | 69 +++++++++---------- tests/integration/integration_test.go | 6 +- 18 files changed, 121 insertions(+), 145 deletions(-) diff --git a/src/cloudformation/structure/cloudformation_parser.go b/src/cloudformation/structure/cloudformation_parser.go index 092286aed..9e28912d0 100644 --- a/src/cloudformation/structure/cloudformation_parser.go +++ b/src/cloudformation/structure/cloudformation_parser.go @@ -112,7 +112,8 @@ func goformationParse(file string) (*cloudformation.Template, error) { return template, err } -func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, error) { +func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, []string, error) { + skipResourcesByComment := make([]string, 0) goformationLock.Lock() template, err := goformationParse(filePath) goformationLock.Unlock() @@ -121,12 +122,12 @@ func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, e if err == nil { err = fmt.Errorf("failed to parse template %v", filePath) } - return nil, err + return nil, skipResourcesByComment, err } if template.Transform != nil { logger.Info(fmt.Sprintf("Skipping CFN template %s as SAM templates are not yet supported", filePath)) - return nil, nil + return nil, skipResourcesByComment, nil } resourceNames := make([]string, 0) @@ -138,13 +139,13 @@ func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, e var resourceNamesToLines map[string]*structure.Lines switch utils.GetFileFormat(filePath) { case common.YmlFileType.FileFormat, common.YamlFileType.FileFormat: - resourceNamesToLines = yaml.MapResourcesLineYAML(filePath, resourceNames, ResourcesStartToken) + resourceNamesToLines, skipResourcesByComment = yaml.MapResourcesLineYAML(filePath, resourceNames, ResourcesStartToken) case common.JSONFileType.FileFormat: var fileBracketsMapping map[int]json.BracketPair resourceNamesToLines, fileBracketsMapping = json.MapResourcesLineJSON(filePath, resourceNames) p.FileToBracketMapping.Store(filePath, fileBracketsMapping) default: - return nil, fmt.Errorf("unsupported file type %s", utils.GetFileFormat(filePath)) + return nil, skipResourcesByComment, fmt.Errorf("unsupported file type %s", utils.GetFileFormat(filePath)) } minResourceLine := math.MaxInt8 @@ -181,9 +182,9 @@ func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, e p.FileToResourcesLines.Store(filePath, structure.Lines{Start: minResourceLine, End: maxResourceLine}) - return parsedBlocks, nil + return parsedBlocks, skipResourcesByComment, nil } - return nil, err + return nil, skipResourcesByComment, err } func (p *CloudformationParser) extractTagsAndLines(filePath string, lines *structure.Lines, tagsValue reflect.Value) (structure.Lines, []tags.ITag) { @@ -195,8 +196,7 @@ func (p *CloudformationParser) extractTagsAndLines(filePath string, lines *struc func (p *CloudformationParser) GetExistingTags(tagsValue reflect.Value) []tags.ITag { existingTags := make([]goformationTags.Tag, 0) if tagsValue.Kind() == reflect.Slice { - ok := true - existingTags, ok = tagsValue.Interface().([]goformationTags.Tag) + existingTags, ok := tagsValue.Interface().([]goformationTags.Tag) if !ok { for i := 0; i < tagsValue.Len(); i++ { iTag := tagsValue.Index(i) @@ -237,7 +237,7 @@ func (p *CloudformationParser) WriteFile(readFilePath string, blocks []structure return err } - _, err = p.ParseFile(tempFile.Name()) + _, _, err = p.ParseFile(tempFile.Name()) if err != nil { return fmt.Errorf("editing file %v resulted in a malformed template, please open a github issue with the relevant details", readFilePath) } diff --git a/src/cloudformation/structure/cloudformation_parser_test.go b/src/cloudformation/structure/cloudformation_parser_test.go index 85324973e..c3d141ee5 100644 --- a/src/cloudformation/structure/cloudformation_parser_test.go +++ b/src/cloudformation/structure/cloudformation_parser_test.go @@ -19,7 +19,7 @@ func TestCloudformationParser_ParseFile(t *testing.T) { directory := "../../../tests/cloudformation/resources/ebs" cfnParser := CloudformationParser{} cfnParser.Init(directory, nil) - cfnBlocks, err := cfnParser.ParseFile(directory + "/ebs.yaml") + cfnBlocks, _, err := cfnParser.ParseFile(directory + "/ebs.yaml") if err != nil { t.Errorf("ParseFile() error = %v", err) return @@ -42,7 +42,7 @@ func TestCloudformationParser_ParseFile(t *testing.T) { directory := "../../../tests/cloudformation/resources/ebs" cfnParser := CloudformationParser{} cfnParser.Init(directory, nil) - cfnBlocks, err := cfnParser.ParseFile(directory + "/ebs.json") + cfnBlocks, _, err := cfnParser.ParseFile(directory + "/ebs.json") if err != nil { t.Errorf("ParseFile() error = %v", err) return @@ -66,7 +66,7 @@ func TestCloudformationParser_ParseFile(t *testing.T) { cfnParser := CloudformationParser{} cfnParser.Init(directory, nil) sourceFile := directory + "/base.template" - cfnBlocks, _ := cfnParser.ParseFile(sourceFile) + cfnBlocks, _, _ := cfnParser.ParseFile(sourceFile) rawFileLines, _ := cfnParser.FileToResourcesLines.Load(sourceFile) resourceLines := rawFileLines.(structure.Lines) @@ -81,7 +81,7 @@ func TestCloudformationParser_ParseFile(t *testing.T) { cfnParser := CloudformationParser{} cfnParser.Init(directory, nil) sourceFile := directory + "/cfn.yaml" - cfnBlocks, _ := cfnParser.ParseFile(sourceFile) + cfnBlocks, _, _ := cfnParser.ParseFile(sourceFile) assert.Equal(t, 3, len(cfnBlocks)) }) @@ -89,7 +89,7 @@ func TestCloudformationParser_ParseFile(t *testing.T) { directory := "../../../tests/cloudformation/resources/special_tags" cfnParser := CloudformationParser{} cfnParser.Init(directory, nil) - cfnBlocks, err := cfnParser.ParseFile(directory + "/cfn.yaml") + cfnBlocks, _, err := cfnParser.ParseFile(directory + "/cfn.yaml") if err != nil { t.Errorf("ParseFile() error = %v", err) return @@ -123,7 +123,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { expected := map[string]*structure.Lines{ "NewVolume": {Start: 3, End: 15}, } - actual := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) + actual, _ := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) compareLines(t, expected, actual) }) @@ -136,7 +136,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { "EC2LaunchTemplateResource0": {Start: 18, End: 23}, "EC2LaunchTemplateResource1": {Start: 24, End: 34}, } - actual := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) + actual, _ := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) compareLines(t, expected, actual) }) @@ -169,7 +169,7 @@ func writeCFNTestHelper(t *testing.T, directory string, testFileName string, fil tagGroup.SetTags(extraTags) tagGroup.InitTagGroup("", []string{}, []string{}) writeFilePath := directory + "/" + testFileName + "_tagged." + fileType - cfnBlocks, err := cfnParser.ParseFile(readFilePath) + cfnBlocks, _, err := cfnParser.ParseFile(readFilePath) for _, block := range cfnBlocks { err := tagGroup.CreateTagsForBlock(block) if err != nil { diff --git a/src/common/color_check.go b/src/common/color_check.go index ad2f36b62..0b809c605 100644 --- a/src/common/color_check.go +++ b/src/common/color_check.go @@ -9,7 +9,7 @@ type ColorStruct struct { Purple string } -func NoColorCheck(NoColorBool bool) *ColorStruct { +func NoColorCheck(noColorBool bool) *ColorStruct { var colors ColorStruct colors = ColorStruct{ NoColor: true, @@ -19,7 +19,7 @@ func NoColorCheck(NoColorBool bool) *ColorStruct { Blue: "", Purple: "", } - if !NoColorBool { + if !noColorBool { colors = ColorStruct{ NoColor: false, Reset: "\033[0m", diff --git a/src/common/json/json_writer.go b/src/common/json/json_writer.go index 11d7babef..4d58c8fb5 100644 --- a/src/common/json/json_writer.go +++ b/src/common/json/json_writer.go @@ -91,7 +91,8 @@ func AddTagsToResourceStr(fullOriginStr string, resourceBlock structure.IBlock, firstTagStr := tagsStr[firstTagIndex : firstTagIndex+strings.Index(tagsStr[firstTagIndex+1:], "\"")] tagEntryIndent := findIndent(tagsStr, '"', strings.Index(tagsStr[1:], "{")) // find the indent of the key and value entry compact := false - if strings.Contains(firstTagStr, "\n") { + switch { + case strings.Contains(firstTagStr, "\n"): // If the tag string has a newline, it means the indent needs to be re-evaluated. Example for this use case: // "Tags": [ // { @@ -102,10 +103,10 @@ func AddTagsToResourceStr(fullOriginStr string, resourceBlock structure.IBlock, indentDiff := len(tagEntryIndent) - len(tagBlockIndent) tagBlockIndent = tagBlockIndent[0 : len(tagBlockIndent)-indentDiff] tagEntryIndent = tagEntryIndent[0 : len(tagEntryIndent)-indentDiff] - } else if len(tagsLinesList) == 1 { + case len(tagsLinesList) == 1: // multi tags in one line compact = true - } else { + default: // Otherwise, need to take the indent of the "{" character. This case handles: // "Tags": [ // { "Key": "some-key", "Value": "some-val" } diff --git a/src/common/parser.go b/src/common/parser.go index b4982e296..7102263ad 100644 --- a/src/common/parser.go +++ b/src/common/parser.go @@ -6,7 +6,7 @@ type IParser interface { Init(rootDir string, args map[string]string) Name() string ValidFile(filePath string) bool - ParseFile(filePath string) ([]structure.IBlock, error) + ParseFile(filePath string) ([]structure.IBlock, []string, error) WriteFile(readFilePath string, blocks []structure.IBlock, writeFilePath string) error GetSkippedDirs() []string GetSupportedFileExtensions() []string diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index e2a4387f9..f32380240 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -176,12 +176,12 @@ func (r *Runner) TagFile(file string) { continue } logger.Info(fmt.Sprintf("Tagging %v\n", file)) - blocks, err := parser.ParseFile(file) + blocks, skipResourcesByComment, err := parser.ParseFile(file) if err != nil { logger.Info(fmt.Sprintf("Failed to parse file %v with parser %v", file, reflect.TypeOf(parser))) continue } - utils.AppendSkipedByCommentToRunnerSkippedResources(&r.skippedResources) + r.skippedResources = append(r.skippedResources, skipResourcesByComment...) isFileTaggable := false for _, block := range blocks { if r.isSkippedResourceType(block.GetResourceType()) { diff --git a/src/common/runner/runner_test.go b/src/common/runner/runner_test.go index 71094f424..982fb2631 100644 --- a/src/common/runner/runner_test.go +++ b/src/common/runner/runner_test.go @@ -232,12 +232,12 @@ func TestRunnerInternals(t *testing.T) { if err != nil { t.Error(err) } - tfBlocks, err := runner.parsers[0].ParseFile(options.Directory) + tfBlocks, skipResourcesByComment, err := runner.parsers[0].ParseFile(options.Directory) if err != nil { t.Error(err) } assert.Equal(t, 3, len(tfBlocks)) - utils.AppendSkipedByCommentToRunnerSkippedResources(&runner.skippedResources) + runner.skippedResources = append(runner.skippedResources, skipResourcesByComment...) result := []string{"aws_instance.example_instance"} assert.Equal(t, result, runner.skippedResources) }) @@ -307,6 +307,9 @@ func Test_YorNameTag(t *testing.T) { runner := Runner{} err := runner.Init(&options) + if err != nil { + t.Error(err) + } reportService, err := runner.TagDirectory() if err != nil { t.Error(err) diff --git a/src/common/tagging/external/tag_group.go b/src/common/tagging/external/tag_group.go index 2f21f8609..40adec6af 100644 --- a/src/common/tagging/external/tag_group.go +++ b/src/common/tagging/external/tag_group.go @@ -78,9 +78,9 @@ func (t Tag) SatisfyFilters(block structure.IBlock) bool { case "directory": prefixes := make([]string, 0) - switch filterValue.(type) { + switch filterValue := filterValue.(type) { case []interface{}: - for _, e := range filterValue.([]interface{}) { + for _, e := range filterValue { prefixes = append(prefixes, e.(string)) } case interface{}: diff --git a/src/common/utils/utils.go b/src/common/utils/utils.go index 6661f0dfd..81002a1be 100644 --- a/src/common/utils/utils.go +++ b/src/common/utils/utils.go @@ -8,7 +8,6 @@ import ( "reflect" "regexp" "strings" - "sync" "unicode" "github.com/bridgecrewio/yor/src/common" @@ -18,15 +17,6 @@ import ( // RemoveGcpInvalidChars Source of regex: https://cloud.google.com/compute/docs/labeling-resources var RemoveGcpInvalidChars = regexp.MustCompile(`[^\p{Ll}\p{Lo}\p{N}_-]`) -var SkipResourcesByComment = make([]string, 0) -var mutex sync.Mutex - -func AppendSkipedByCommentToRunnerSkippedResources(skippedResources *[]string) { - mutex.Lock() - *skippedResources = append(*skippedResources, SkipResourcesByComment...) - SkipResourcesByComment = SkipResourcesByComment[:0] - mutex.Unlock() -} func InSlice[T comparable](elems []T, v T) bool { for _, s := range elems { diff --git a/src/common/utils/utils_test.go b/src/common/utils/utils_test.go index e2a4de2c0..dade664b4 100644 --- a/src/common/utils/utils_test.go +++ b/src/common/utils/utils_test.go @@ -109,11 +109,11 @@ func TestInSlice(t *testing.T) { args: args[int]{slice: []int{1, 2, 3, 4}, elem: 5}, want: false, }, - //{ // not supported for generics + // { // not supported for generics // name: "different kinds", // args: args[int]{slice: []int{1, 2, 3, 4}, elem: "bana"}, // want: false, - //}, + // }, } for _, tt := range testsStr { diff --git a/src/common/yaml/yaml_writer.go b/src/common/yaml/yaml_writer.go index 9c97bb563..f4f45ac83 100644 --- a/src/common/yaml/yaml_writer.go +++ b/src/common/yaml/yaml_writer.go @@ -7,7 +7,6 @@ import ( "regexp" "sort" "strings" - "sync" "github.com/bridgecrewio/yor/src/common/logger" "github.com/bridgecrewio/yor/src/common/structure" @@ -18,8 +17,6 @@ import ( const SingleIndent = " " -var mutex sync.Mutex - func WriteYAMLFile(readFilePath string, blocks []structure.IBlock, writeFilePath string, tagsAttributeName string, resourcesStartToken string) error { // #nosec G304 // read file bytes @@ -269,8 +266,10 @@ func FindTagsLinesYAML(textLines []string, tagsAttributeName string) (structure. return tagsLines, tagsExist } -func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStartToken string) map[string]*structure.Lines { +func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStartToken string) (map[string]*structure.Lines, []string) { resourceToLines := make(map[string]*structure.Lines) + skipResourcesByComment := make([]string, 0) + for _, resourceName := range resourceNames { // initialize a map between resource name and its lines in file resourceToLines[resourceName] = &structure.Lines{Start: -1, End: -1} @@ -279,7 +278,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar file, err := os.ReadFile(filePath) if err != nil { logger.Warning(fmt.Sprintf("failed to read file %s", filePath)) - return nil + return nil, skipResourcesByComment } readResources := false @@ -291,9 +290,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar cleanContent := strings.TrimSpace(line) if strings.HasPrefix(cleanContent, resourcesStartToken+":") { if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIPALL" { - mutex.Lock() - utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, resourceNames...) - mutex.Unlock() + skipResourcesByComment = append(skipResourcesByComment, resourceNames...) } readResources = true resourcesIndent = countLeadingSpaces(line) @@ -303,9 +300,9 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar if readResources { if i > 0 { if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIP" { - mutex.Lock() - utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, strings.Trim(strings.TrimSpace(line), ":")) - mutex.Unlock() + + skipResourcesByComment = append(skipResourcesByComment, strings.Trim(strings.TrimSpace(line), ":")) + } } lineIndent := countLeadingSpaces(line) @@ -340,7 +337,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar // Handle last line of resource is last line of file resourceToLines[latestResourceName].End = findLastNonEmptyLine(fileLines, len(fileLines)-1) } - return resourceToLines + return resourceToLines, skipResourcesByComment } func countLeadingSpaces(line string) int { diff --git a/src/common/yaml/yaml_writer_test.go b/src/common/yaml/yaml_writer_test.go index 7421369a5..87147978a 100644 --- a/src/common/yaml/yaml_writer_test.go +++ b/src/common/yaml/yaml_writer_test.go @@ -11,7 +11,6 @@ import ( "github.com/bridgecrewio/yor/src/common/structure" "github.com/bridgecrewio/yor/src/common/tagging/simple" "github.com/bridgecrewio/yor/src/common/tagging/tags" - "github.com/bridgecrewio/yor/src/common/utils" "github.com/stretchr/testify/assert" ) @@ -229,13 +228,13 @@ func TestTagReplacement(t *testing.T) { }) t.Run("Test line computation with duplicate - CFN", func(t *testing.T) { - res := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_cfn.yaml", []string{"S3Bucket", "CloudFrontDistribution"}, "Resources") + res, _ := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_cfn.yaml", []string{"S3Bucket", "CloudFrontDistribution"}, "Resources") assert.Equal(t, *res["S3Bucket"], structure.Lines{Start: 14, End: 17}) assert.Equal(t, *res["CloudFrontDistribution"], structure.Lines{Start: 18, End: 60}) }) t.Run("Test line computation with duplicate - SLS", func(t *testing.T) { - res := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_sls.yaml", []string{"attribute", "zone", "customer", "apiVersion"}, "functions") + res, _ := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_sls.yaml", []string{"attribute", "zone", "customer", "apiVersion"}, "functions") assert.Equal(t, *res["apiVersion"], structure.Lines{Start: 7, End: 12}) assert.Equal(t, *res["customer"], structure.Lines{Start: 14, End: 24}) assert.Equal(t, *res["zone"], structure.Lines{Start: 26, End: 38}) @@ -243,33 +242,26 @@ func TestTagReplacement(t *testing.T) { }) } -func TestYaml_ResorceSkipTagging(t *testing.T) { - t.Run("Test some resources with skip comment added to utils.SkipResourcesByComment", func(t *testing.T) { +func TestYaml_SkipResourceByComment(t *testing.T) { + t.Run("Test some resources with skip comment added to skipResourcesByComment slice", func(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/skipOne.yaml" resorseSkip := []string{"NewVolume"} expectedResourceNames := []string{"NewVolume", "NewVolume2"} - MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") - assert.Equal(t, utils.SkipResourcesByComment, resorseSkip) - assert.NotEqual(t, utils.SkipResourcesByComment, "NewVolume2") - defer resetSkipArr() + _, skipResourcesByComment := MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Equal(t, skipResourcesByComment, resorseSkip) + assert.NotEqual(t, skipResourcesByComment, "NewVolume2") }) - t.Run("All resources with skip comment added to utils.SkipResourcesByComment", func(t *testing.T) { + t.Run("All resources with skip comment added to skipResourcesByComment slice", func(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/skipAll.yaml" resorseSkip := []string{"NewVolume", "NewVolume2"} expectedResourceNames := []string{"NewVolume", "NewVolume2"} - MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") - assert.Equal(t, utils.SkipResourcesByComment, resorseSkip) - defer resetSkipArr() + _, skipResourcesByComment := MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Equal(t, skipResourcesByComment, resorseSkip) }) - t.Run("No resources with skip all comment in the file, utils.SkipResourcesByComment should be empty", func(t *testing.T) { + t.Run("No resources with skip all comment in the file, skipResourcesByComment slice should be empty", func(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/noSkip.yaml" expectedResourceNames := []string{"NewVolume"} - MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") - assert.Empty(t, utils.SkipResourcesByComment) - defer resetSkipArr() + _, skipResourcesByComment := MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Empty(t, skipResourcesByComment) }) } - -func resetSkipArr() { - utils.SkipResourcesByComment = []string{} -} diff --git a/src/serverless/structure/serverless_block_test.go b/src/serverless/structure/serverless_block_test.go index f4c96a587..5e3b69447 100644 --- a/src/serverless/structure/serverless_block_test.go +++ b/src/serverless/structure/serverless_block_test.go @@ -54,7 +54,7 @@ func TestServerlessBlock_UpdateTags(t *testing.T) { &tags.Tag{Key: "TAG1_FUNC", Value: "Func1 Tag Value"}, &tags.Tag{Key: "TAG2_FUNC", Value: "Func2 Tag Value"}, &tags.Tag{Key: "yor_trace", Value: "yor_trace"}, &tags.Tag{Key: "git_last_modified_at", Value: "2"}, } absFilePath, _ := filepath.Abs(strings.Join([]string{parser.YamlParser.RootDir, "serverless.yml"}, "/")) - template, err := parser.ParseFile(absFilePath) + template, _, err := parser.ParseFile(absFilePath) if err != nil { t.Errorf("There was an error processing the cloudformation template: %s", err) } diff --git a/src/serverless/structure/serverless_parser.go b/src/serverless/structure/serverless_parser.go index d8cb7537f..e3cb6a67b 100644 --- a/src/serverless/structure/serverless_parser.go +++ b/src/serverless/structure/serverless_parser.go @@ -71,12 +71,13 @@ func (p *ServerlessParser) ValidFile(file string) bool { return true } -func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, error) { +func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, []string, error) { + skipResourcesByComment := make([]string, 0) parsedBlocks := make([]structure.IBlock, 0) fileFormat := utils.GetFileFormat(filePath) fileName := filepath.Base(filePath) if !(fileName == fmt.Sprintf("serverless.%s", fileFormat) || fileName == fmt.Sprintf("config.%s", fileFormat)) { - return nil, nil + return nil, skipResourcesByComment, nil } // #nosec G304 - file is from user template, err := serverlessParse(filePath) @@ -87,10 +88,10 @@ func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, error if err == nil { err = fmt.Errorf("failed to parse file %v", filePath) } - return nil, err + return nil, skipResourcesByComment, err } if template.Functions == nil && template.Resources.Resources == nil { - return parsedBlocks, nil + return parsedBlocks, skipResourcesByComment, nil } // cfnStackTagsResource := p.template.Provider.CFNTags @@ -101,9 +102,9 @@ func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, error } switch utils.GetFileFormat(filePath) { case common.YmlFileType.FileFormat, common.YamlFileType.FileFormat: - resourceNamesToLines = yamlUtils.MapResourcesLineYAML(filePath, resourceNames, FunctionsSectionName) + resourceNamesToLines, skipResourcesByComment = yamlUtils.MapResourcesLineYAML(filePath, resourceNames, FunctionsSectionName) default: - return nil, fmt.Errorf("unsupported file type %s", utils.GetFileFormat(filePath)) + return nil, skipResourcesByComment, fmt.Errorf("unsupported file type %s", utils.GetFileFormat(filePath)) } minResourceLine := math.MaxInt8 maxResourceLine := 0 @@ -141,7 +142,7 @@ func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, error p.YamlParser.FileToResourcesLines.Store(filePath, structure.Lines{Start: minResourceLine, End: maxResourceLine}) } - return parsedBlocks, nil + return parsedBlocks, skipResourcesByComment, nil } func (p *ServerlessParser) WriteFile(readFilePath string, blocks []structure.IBlock, writeFilePath string) error { @@ -160,7 +161,7 @@ func (p *ServerlessParser) WriteFile(readFilePath string, blocks []structure.IBl if err != nil { return err } - _, err = p.ParseFile(tempFile.Name()) + _, _, err = p.ParseFile(tempFile.Name()) if err != nil { return fmt.Errorf("editing file %v resulted in a malformed template, please open a github issue with the relevant details", readFilePath) } diff --git a/src/serverless/structure/serverless_parser_test.go b/src/serverless/structure/serverless_parser_test.go index 562878888..69f33cc0f 100644 --- a/src/serverless/structure/serverless_parser_test.go +++ b/src/serverless/structure/serverless_parser_test.go @@ -29,7 +29,7 @@ func TestServerlessParser_ParseFile(t *testing.T) { slsParser.Init(directory, nil) slsFilepath, _ := filepath.Abs(filepath.Join(slsParser.YamlParser.RootDir, "serverless.yml")) expectedSlsFilepath, _ := filepath.Abs(filepath.Join(slsParser.YamlParser.RootDir, "serverless_expected.yaml")) - slsBlocks, err := slsParser.ParseFile(slsFilepath) + slsBlocks, _, err := slsParser.ParseFile(slsFilepath) if err != nil { t.Errorf("ParseFile() error = %v", err) return @@ -94,7 +94,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { slsFilepath, _ := filepath.Abs(strings.Join([]string{directory, "serverless.yml"}, "/")) slsParser := ServerlessParser{} slsParser.Init(directory, nil) - slsBlocks, err := slsParser.ParseFile(slsFilepath) + slsBlocks, _, err := slsParser.ParseFile(slsFilepath) if err != nil { t.Errorf("ParseFile() error = %v", err) return @@ -120,7 +120,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { slsParser := ServerlessParser{} slsParser.Init(directory, nil) var func1Block, func2Block *ServerlessBlock - slsBlocks, err := slsParser.ParseFile(slsFilepath) + slsBlocks, _, err := slsParser.ParseFile(slsFilepath) for _, block := range slsBlocks { castedBlock := block.(*ServerlessBlock) if castedBlock.Name == "myFunction" { @@ -150,7 +150,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { slsParser := ServerlessParser{} slsParser.Init(directory, nil) var func1Block, func2Block *ServerlessBlock - slsBlocks, err := slsParser.ParseFile(slsFilepath) + slsBlocks, _, err := slsParser.ParseFile(slsFilepath) for _, block := range slsBlocks { castedBlock := block.(*ServerlessBlock) if castedBlock.Name == "myFunction" { @@ -179,7 +179,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { slsFilepath, _ := filepath.Abs(strings.Join([]string{directory, "file.yml"}, "/")) slsParser := ServerlessParser{} slsParser.Init(directory, nil) - parsedBlocks, _ := slsParser.ParseFile(slsFilepath) + parsedBlocks, _, _ := slsParser.ParseFile(slsFilepath) if parsedBlocks != nil { t.Fail() } @@ -200,7 +200,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { tagGroup.SetTags(extraTags) tagGroup.InitTagGroup("", []string{}, []string{}) writeFilePath := directory + "/serverless_tagged.yml" - slsBlocks, err := slsParser.ParseFile(readFilePath) + slsBlocks, _, err := slsParser.ParseFile(readFilePath) for _, block := range slsBlocks { err := tagGroup.CreateTagsForBlock(block) if err != nil { diff --git a/src/terraform/structure/terraform_parser.go b/src/terraform/structure/terraform_parser.go index eae513aca..033fbadf9 100644 --- a/src/terraform/structure/terraform_parser.go +++ b/src/terraform/structure/terraform_parser.go @@ -31,7 +31,7 @@ var unsupportedTerraformBlocks = []string{ "aws_lb_listener", // This resource does not support tags, although docs state otherwise. "aws_lb_listener_rule", // This resource does not support tags, although docs state otherwise. "aws_cloudwatch_log_destination", // This resource does not support tags, although docs state otherwise. - "google_monitoring_notification_channel", //This resource uses labels for other purposes. + "google_monitoring_notification_channel", // This resource uses labels for other purposes. "aws_secretsmanager_secret_rotation", // This resource does not support tags, although tfschema states otherwise. } @@ -126,12 +126,13 @@ func (p *TerraformParser) ValidFile(_ string) bool { return true } -func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, error) { +func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, []string, error) { // #nosec G304 // read file bytes + skipResourcesByComment := make([]string, 0) src, err := os.ReadFile(filePath) if err != nil { - return nil, fmt.Errorf("failed to read file %s because %s", filePath, err) + return nil, skipResourcesByComment, fmt.Errorf("failed to read file %s because %s", filePath, err) } lines := strings.Split(string(src), "\n") @@ -139,16 +140,16 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, error) hclFile, diagnostics := hclwrite.ParseConfig(src, filePath, hcl.InitialPos) if diagnostics != nil && diagnostics.HasErrors() { hclErrors := diagnostics.Errs() - return nil, fmt.Errorf("failed to parse hcl file %s because of errors %s", filePath, hclErrors) + return nil, skipResourcesByComment, fmt.Errorf("failed to parse hcl file %s because of errors %s", filePath, hclErrors) } hclSyntaxFile, diagnostics := hclsyntax.ParseConfig(src, filePath, hcl.InitialPos) if diagnostics != nil && diagnostics.HasErrors() { hclErrors := diagnostics.Errs() - return nil, fmt.Errorf("failed to parse hcl file %s because of errors %s", filePath, hclErrors) + return nil, skipResourcesByComment, fmt.Errorf("failed to parse hcl file %s because of errors %s", filePath, hclErrors) } if hclFile == nil || hclSyntaxFile == nil { - return nil, fmt.Errorf("failed to parse hcl file %s", filePath) + return nil, skipResourcesByComment, fmt.Errorf("failed to parse hcl file %s", filePath) } syntaxBlocks := hclSyntaxFile.Body.(*hclsyntax.Body).Blocks @@ -185,15 +186,13 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, error) } if strings.ToUpper(strings.TrimSpace(lineAbove)) == "#YOR:SKIP" || skipAll { - mutex.Lock() - utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, terraformBlock.GetResourceID()) - mutex.Unlock() + skipResourcesByComment = append(skipResourcesByComment, terraformBlock.GetResourceID()) } } parsedBlocks = append(parsedBlocks, terraformBlock) } - return parsedBlocks, nil + return parsedBlocks, skipResourcesByComment, nil } func (p *TerraformParser) WriteFile(readFilePath string, blocks []structure.IBlock, writeFilePath string) error { @@ -243,7 +242,7 @@ func (p *TerraformParser) WriteFile(readFilePath string, blocks []structure.IBlo return err } - _, err = p.ParseFile(tempFile.Name()) + _, _, err = p.ParseFile(tempFile.Name()) if err != nil { return fmt.Errorf("editing file %v resulted in malformed terraform, please open a github issue with the relevant details", readFilePath) } @@ -282,7 +281,7 @@ func (p *TerraformParser) modifyBlockTags(rawBlock *hclwrite.Block, parsedBlock tagsAttributeName := parsedBlock.(*TerraformBlock).TagsAttributeName tagsAttribute := rawBlock.Body().GetAttribute(tagsAttributeName) - //we don't add tags to data sources + // we don't add tags to data sources if rawBlock.Type() == "data" { return } @@ -582,7 +581,7 @@ func ExtractProviderFromModuleSrc(source string) string { } if isTerraformRegistryModule(source) { matches := utils.FindSubMatchByGroup(RegistryModuleRegex, source) - val, _ := matches["PROVIDER"] + val := matches["PROVIDER"] return val } withoutRef := strings.Split(source, "//")[0] @@ -615,7 +614,7 @@ func (p *TerraformParser) isModuleTaggable(fp string, moduleName string, moduleD files, _ := os.ReadDir(expectedModuleDir) for _, f := range files { if strings.HasSuffix(f.Name(), ".tf") { - blocks, _ := p.ParseFile(filepath.Join(expectedModuleDir, f.Name())) + blocks, _, _ := p.ParseFile(filepath.Join(expectedModuleDir, f.Name())) for _, b := range blocks { if b.(*TerraformBlock).HclSyntaxBlock.Type == VariableBlockType { for _, tagAtt := range tagAtts { diff --git a/src/terraform/structure/terraform_parser_test.go b/src/terraform/structure/terraform_parser_test.go index 06e69fa38..9f687e70d 100644 --- a/src/terraform/structure/terraform_parser_test.go +++ b/src/terraform/structure/terraform_parser_test.go @@ -23,48 +23,45 @@ import ( ) func TestTerraformParser_SkipResourceByComment(t *testing.T) { - t.Run("SkipAll comment added all resources to utils.SkipResourcesByComment", func(t *testing.T) { + t.Run("SkipAll comment added all resources to skipResourcesByComment slice", func(t *testing.T) { // Initialize TerraformParser and parse file with all resources containing skip comment p := &TerraformParser{} p.Init("../../../tests/terraform/skipComment/", nil) defer p.Close() filePath := "../../../tests/terraform/skipComment/skipAll.tf" - _, err := p.ParseFile(filePath) + _, skipResourcesByComment, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } exceptedSkipResources := []string{"aws_vpc.example_vpc", "aws_subnet.example_subnet", "aws_instance.example_instance"} - assert.Equal(t, exceptedSkipResources, utils.SkipResourcesByComment) - defer resetSkipArr() + assert.Equal(t, exceptedSkipResources, skipResourcesByComment) }) - t.Run("No resources with skip comment in the file, utils.SkipResourcesByComment should be empty", func(t *testing.T) { + t.Run("No resources with skip comment in the file, skipResourcesByComment slice should be empty", func(t *testing.T) { // Initialize TerraformParser and parse file with no skip tags p := &TerraformParser{} p.Init("../../../tests/terraform/skipComment/", nil) defer p.Close() filePath := "../../../tests/terraform/skipComment/noSkip.tf" - _, err := p.ParseFile(filePath) + _, skipResourcesByComment, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } - assert.Empty(t, utils.SkipResourcesByComment) - defer resetSkipArr() + assert.Empty(t, skipResourcesByComment) }) - t.Run("One resource with skip comment, only that resource added to utils.SkipResourcesByComment", func(t *testing.T) { + t.Run("One resource with skip comment, only that resource added to skipResourcesByComment slice", func(t *testing.T) { // Initialize TerraformParser and parse file with one resource containing skip tag p := &TerraformParser{} p.Init("../../../tests/terraform/skipComment/", nil) defer p.Close() filePath := "../../../tests/terraform/skipComment/skipOne.tf" - _, err := p.ParseFile(filePath) + _, skipResourcesByComment, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } exceptedSkipResources := []string{"aws_instance.example_instance"} - assert.Equal(t, exceptedSkipResources, utils.SkipResourcesByComment) - defer resetSkipArr() + assert.Equal(t, exceptedSkipResources, skipResourcesByComment) }) } @@ -91,7 +88,7 @@ func TestTerraformParser_ParseFile(t *testing.T) { "eks_subnet2": {Start: 55, End: 63}, "eks_cluster": {Start: 65, End: 78}, } - parsedBlocks, err := p.ParseFile(filePath) + parsedBlocks, _, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } @@ -143,7 +140,7 @@ func TestTerraformParser_ParseFile(t *testing.T) { "instance_null_tags": nil, } - parsedBlocks, err := p.ParseFile(filePath) + parsedBlocks, _, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } @@ -167,7 +164,7 @@ func TestTerraformParser_ParseFile(t *testing.T) { p.Init("../../../tests/terraform/resources", nil) defer p.Close() filePath := "../../../tests/terraform/resources/collision/main.tf" - parsedBlocks, err := p.ParseFile(filePath) + parsedBlocks, _, err := p.ParseFile(filePath) assert.Nil(t, parsedBlocks) assert.NotNil(t, err) }) @@ -177,7 +174,7 @@ func TestTerraformParser_ParseFile(t *testing.T) { p.Init("../../../tests/terraform/malformed_file_in_dir", nil) defer p.Close() filePath := "../../../tests/terraform/resources/malformed_file_in_dir/trail.tf" - parsedBlocks, err := p.ParseFile(filePath) + parsedBlocks, _, err := p.ParseFile(filePath) assert.Nil(t, parsedBlocks) assert.NotNil(t, err) }) @@ -226,7 +223,7 @@ func TestTerraformParser_Module(t *testing.T) { defer func() { _ = os.WriteFile(writeFilePath, writeFileBytes, 0644) }() - parsedBlocks, err := p.ParseFile(filePath) + parsedBlocks, _, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } @@ -245,7 +242,7 @@ func TestTerraformParser_Module(t *testing.T) { if err != nil { t.Error(err) } - parsedTaggedFileTags, err := p.ParseFile(writeFilePath) + parsedTaggedFileTags, _, err := p.ParseFile(writeFilePath) if err != nil { t.Error(err) } @@ -289,7 +286,7 @@ func TestTerraformParser_Module(t *testing.T) { defer func() { _ = os.WriteFile(writeFilePath, writeFileBytes, 0644) }() - parsedBlocks, err := p.ParseFile(filePath) + parsedBlocks, _, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } @@ -308,7 +305,7 @@ func TestTerraformParser_Module(t *testing.T) { if err != nil { t.Error(err) } - parsedTaggedFileTags, err := p.ParseFile(writeFilePath) + parsedTaggedFileTags, _, err := p.ParseFile(writeFilePath) if err != nil { t.Error(err) } @@ -345,7 +342,7 @@ func TestTerraformParser_Module(t *testing.T) { defer func() { _ = os.WriteFile(writeFilePath, writeFileBytes, 0644) }() - parsedBlocks, err := p.ParseFile(filePath) + parsedBlocks, _, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } @@ -362,7 +359,7 @@ func TestTerraformParser_Module(t *testing.T) { if err != nil { t.Error(err) } - parsedTaggedFileTags, err := p.ParseFile(writeFilePath) + parsedTaggedFileTags, _, err := p.ParseFile(writeFilePath) if err != nil { t.Error(err) } @@ -388,7 +385,7 @@ func TestTerraformParser_Module(t *testing.T) { p := &TerraformParser{} p.Init("../../../tests/terraform/mixed", nil) defer p.Close() - blocks, err := p.ParseFile("../../../tests/terraform/mixed/mixed.tf") + blocks, _, err := p.ParseFile("../../../tests/terraform/mixed/mixed.tf") if err != nil { t.Fail() } @@ -402,7 +399,7 @@ func TestTerraformParser_Module(t *testing.T) { defer p.Close() sourceFilePath := "../../../tests/terraform/data/main.tf" expectedFileName := "../../../tests/terraform/data/expected.txt" - blocks, err := p.ParseFile(sourceFilePath) + blocks, _, err := p.ParseFile(sourceFilePath) if err != nil { t.Fail() } @@ -429,7 +426,7 @@ func TestTerraformParser_Module(t *testing.T) { defer p.Close() sourceFilePath := "../../../tests/terraform/tags_with_comments/main.tf" expectedFileName := "../../../tests/terraform/tags_with_comments/expected.txt" - blocks, err := p.ParseFile(sourceFilePath) + blocks, _, err := p.ParseFile(sourceFilePath) if err != nil { t.Fail() } @@ -453,7 +450,7 @@ func TestTerraformParser_Module(t *testing.T) { p := &TerraformParser{} p.Init("../../../tests/terraform/supported", nil) defer p.Close() - blocks, err := p.ParseFile("../../../tests/terraform/supported/unsupported.tf") + blocks, _, err := p.ParseFile("../../../tests/terraform/supported/unsupported.tf") if err != nil { t.Fail() } @@ -466,7 +463,7 @@ func TestTerraformParser_Module(t *testing.T) { defer p.Close() sourceFilePath := "../../../tests/terraform/module/module_with_tags/main.tf" expectedFileName := "../../../tests/terraform/module/module_with_tags/expected.txt" - blocks, err := p.ParseFile(sourceFilePath) + blocks, _, err := p.ParseFile(sourceFilePath) if err != nil { t.Fail() } @@ -494,7 +491,7 @@ func TestTerraformParser_Module(t *testing.T) { p.Init("../../../tests/terraform/module/tfe_module", nil) defer p.Close() sourceFilePath := "../../../tests/terraform/module/tfe_module/main.tf" - blocks, err := p.ParseFile(sourceFilePath) + blocks, _, err := p.ParseFile(sourceFilePath) if err != nil { t.Fail() } @@ -511,7 +508,7 @@ func TestTerraformParser_Module(t *testing.T) { defer p.Close() sourceFilePath := "../../../tests/terraform/module/module/main.tf" expectedFileName := "../../../tests/terraform/module/module/expected.txt" - blocks, err := p.ParseFile(sourceFilePath) + blocks, _, err := p.ParseFile(sourceFilePath) if err != nil { t.Fail() } @@ -542,7 +539,7 @@ func TestTerraformParser_Module(t *testing.T) { filePath := "../../../tests/terraform/resources/attributescenarios/main.tf" resultFilePath := "../../../tests/terraform/resources/attributescenarios/main_result.tf" expectedFilePath := "../../../tests/terraform/resources/attributescenarios/expected.txt" - blocks, _ := p.ParseFile(filePath) + blocks, _, _ := p.ParseFile(filePath) assert.Equal(t, 8, len(blocks)) for _, block := range blocks { if block.IsBlockTaggable() { @@ -571,7 +568,7 @@ func TestTerraformParser_Module(t *testing.T) { expectedFiles := []string{"main.tf", "sub_local_module/main.tf", "sub_local_module/variables.tf"} for _, file := range expectedFiles { filePath := filepath.Join(directory, file) - fileBlocks, err := terraformParser.ParseFile(filePath) + fileBlocks, _, err := terraformParser.ParseFile(filePath) if err != nil { assert.Fail(t, fmt.Sprintf("Failed to parse file %v", filePath)) } @@ -591,7 +588,7 @@ func TestTerraformParser_Module(t *testing.T) { terraformParser := TerraformParser{} terraformParser.Init(directory, nil) defer terraformParser.Close() - blocks, _ := terraformParser.ParseFile(directory + "/main.tf") + blocks, _, _ := terraformParser.ParseFile(directory + "/main.tf") assert.Equal(t, 8, len(blocks)) for _, block := range blocks { assert.True(t, block.IsBlockTaggable(), fmt.Sprintf("Block %v should be taggable", block.GetResourceID())) @@ -614,7 +611,7 @@ func TestTerraformParser_Module(t *testing.T) { defer func() { _ = os.WriteFile(writeFilePath, writeFileBytes, 0644) }() - parsedBlocks, err := p.ParseFile(filePath) + parsedBlocks, _, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } @@ -631,7 +628,7 @@ func TestTerraformParser_Module(t *testing.T) { if err != nil { t.Error(err) } - parsedTaggedFileTags, err := p.ParseFile(writeFilePath) + parsedTaggedFileTags, _, err := p.ParseFile(writeFilePath) if err != nil { t.Error(err) } @@ -833,7 +830,3 @@ func compareTokenArrays(got []hclwrite.Tokens, want []hclwrite.Tokens) bool { return true } - -func resetSkipArr() { - utils.SkipResourcesByComment = []string{} -} diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 5bcd794c1..3c1dfdb79 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -305,7 +305,7 @@ func TestTagUncommittedResults(t *testing.T) { terraformParser.Init(terragoatAWSDirectory, nil) dbAppFile := path.Join(terragoatAWSDirectory, "db-app.tf") - blocks, err := terraformParser.ParseFile(dbAppFile) + blocks, _, err := terraformParser.ParseFile(dbAppFile) failIfErr(t, err) defaultInstanceBlock := blocks[0].(*terraformStructure.TerraformBlock) if defaultInstanceBlock.GetResourceID() != "aws_db_instance.default" { @@ -371,7 +371,7 @@ func TestTagUncommittedResults(t *testing.T) { terraformParser.Init(terragoatAWSDirectory, nil) dbAppFile := path.Join(terragoatAWSDirectory, "db-app.tf") - blocks, err := terraformParser.ParseFile(dbAppFile) + blocks, _, err := terraformParser.ParseFile(dbAppFile) failIfErr(t, err) defaultInstanceBlock := blocks[0].(*terraformStructure.TerraformBlock) if defaultInstanceBlock.GetResourceID() != "aws_db_instance.default" { @@ -422,7 +422,7 @@ func TestLocalModules(t *testing.T) { terraformParser := terraformStructure.TerraformParser{} terraformParser.Init(targetDirectory, nil) dbAppFile := path.Join(targetDirectory, "module.broker.tf") - blocks, _ := terraformParser.ParseFile(dbAppFile) + blocks, _, _ := terraformParser.ParseFile(dbAppFile) defaultInstanceBlock := blocks[0].(*terraformStructure.TerraformBlock) rawTags := defaultInstanceBlock.HclSyntaxBlock.Body.Attributes["common_tags"] From 84eae67b97710b76d5bdff6f7ed815750dd711bc Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Mon, 10 Jun 2024 14:35:08 +0300 Subject: [PATCH 08/19] Fix integration_test5 --- src/cloudformation/structure/cloudformation_parser.go | 3 ++- src/common/runner/runner.go | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cloudformation/structure/cloudformation_parser.go b/src/cloudformation/structure/cloudformation_parser.go index 9e28912d0..9db4694eb 100644 --- a/src/cloudformation/structure/cloudformation_parser.go +++ b/src/cloudformation/structure/cloudformation_parser.go @@ -196,7 +196,8 @@ func (p *CloudformationParser) extractTagsAndLines(filePath string, lines *struc func (p *CloudformationParser) GetExistingTags(tagsValue reflect.Value) []tags.ITag { existingTags := make([]goformationTags.Tag, 0) if tagsValue.Kind() == reflect.Slice { - existingTags, ok := tagsValue.Interface().([]goformationTags.Tag) + ok := true + existingTags, ok = tagsValue.Interface().([]goformationTags.Tag) if !ok { for i := 0; i < tagsValue.Len(); i++ { iTag := tagsValue.Index(i) diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index f32380240..7b8232ef8 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -181,6 +181,9 @@ func (r *Runner) TagFile(file string) { logger.Info(fmt.Sprintf("Failed to parse file %v with parser %v", file, reflect.TypeOf(parser))) continue } + if r.skippedResources == nil { + r.skippedResources = []string{} + } r.skippedResources = append(r.skippedResources, skipResourcesByComment...) isFileTaggable := false for _, block := range blocks { From 793c189af70d8bcf71aec8e16aaa0a01bdbaad6d Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Mon, 10 Jun 2024 14:59:02 +0300 Subject: [PATCH 09/19] Fix integration_test6 --- src/common/runner/runner.go | 3 +++ tests/integration/integration_test.go | 16 ++++++++-------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index 7b8232ef8..cf86677d7 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -170,6 +170,7 @@ func (r *Runner) isSkippedResource(resource string) bool { } func (r *Runner) TagFile(file string) { + var mutex sync.Mutex for _, parser := range r.parsers { if r.isFileSkipped(parser, file) { logger.Debug(fmt.Sprintf("%v parser Skipping %v", parser.Name(), file)) @@ -184,7 +185,9 @@ func (r *Runner) TagFile(file string) { if r.skippedResources == nil { r.skippedResources = []string{} } + mutex.Lock() r.skippedResources = append(r.skippedResources, skipResourcesByComment...) + mutex.Unlock() isFileTaggable := false for _, block := range blocks { if r.isSkippedResourceType(block.GetResourceType()) { diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 3c1dfdb79..9d32f612b 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -261,15 +261,15 @@ func TestSkipResourcesByComment(t *testing.T) { t.Run("Test tagging terraform and cloudFormation files and skip resource by comment", func(t *testing.T) { yorRunner := runner.Runner{} err := yorRunner.Init(&clioptions.TagOptions{ - Directory: ".\\skipComment", + Directory: "./skipComment", TagGroups: getTagGroups(), Parsers: []string{"Terraform", "CloudFormation"}, }) - tfFileBytes, _ := os.ReadFile(".\\skipComment\\skipResource.tf") - yamlFileBytes, _ := os.ReadFile(".\\skipComment\\skipResource.yaml") + tfFileBytes, _ := os.ReadFile("./skipComment/skipResource.tf") + yamlFileBytes, _ := os.ReadFile("./skipComment/skipResource.yaml") defer func() { - _ = os.WriteFile(".\\skipComment\\skipResource.tf", tfFileBytes, 0644) - _ = os.WriteFile(".\\skipComment\\skipResource.yaml", yamlFileBytes, 0644) + _ = os.WriteFile("./skipComment/skipResource.tf", tfFileBytes, 0644) + _ = os.WriteFile("./skipComment/skipResource.yaml", yamlFileBytes, 0644) }() failIfErr(t, err) reportService, err := yorRunner.TagDirectory() @@ -305,7 +305,7 @@ func TestTagUncommittedResults(t *testing.T) { terraformParser.Init(terragoatAWSDirectory, nil) dbAppFile := path.Join(terragoatAWSDirectory, "db-app.tf") - blocks, _, err := terraformParser.ParseFile(dbAppFile) + blocks,_, err := terraformParser.ParseFile(dbAppFile) failIfErr(t, err) defaultInstanceBlock := blocks[0].(*terraformStructure.TerraformBlock) if defaultInstanceBlock.GetResourceID() != "aws_db_instance.default" { @@ -371,7 +371,7 @@ func TestTagUncommittedResults(t *testing.T) { terraformParser.Init(terragoatAWSDirectory, nil) dbAppFile := path.Join(terragoatAWSDirectory, "db-app.tf") - blocks, _, err := terraformParser.ParseFile(dbAppFile) + blocks,_, err := terraformParser.ParseFile(dbAppFile) failIfErr(t, err) defaultInstanceBlock := blocks[0].(*terraformStructure.TerraformBlock) if defaultInstanceBlock.GetResourceID() != "aws_db_instance.default" { @@ -422,7 +422,7 @@ func TestLocalModules(t *testing.T) { terraformParser := terraformStructure.TerraformParser{} terraformParser.Init(targetDirectory, nil) dbAppFile := path.Join(targetDirectory, "module.broker.tf") - blocks, _, _ := terraformParser.ParseFile(dbAppFile) + blocks,_, _ := terraformParser.ParseFile(dbAppFile) defaultInstanceBlock := blocks[0].(*terraformStructure.TerraformBlock) rawTags := defaultInstanceBlock.HclSyntaxBlock.Body.Attributes["common_tags"] From 9f247b91ca3898d4054eb5d81541a476a2e41668 Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Mon, 10 Jun 2024 16:51:01 +0300 Subject: [PATCH 10/19] Fix integration_test6 --- src/common/runner/runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index cf86677d7..d8d583db6 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -182,10 +182,10 @@ func (r *Runner) TagFile(file string) { logger.Info(fmt.Sprintf("Failed to parse file %v with parser %v", file, reflect.TypeOf(parser))) continue } + mutex.Lock() if r.skippedResources == nil { r.skippedResources = []string{} } - mutex.Lock() r.skippedResources = append(r.skippedResources, skipResourcesByComment...) mutex.Unlock() isFileTaggable := false From 045f87e79304e70622afa1df062a16dc72f04b82 Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Mon, 10 Jun 2024 17:05:17 +0300 Subject: [PATCH 11/19] Fix integration_test7 --- src/common/runner/runner.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index d8d583db6..6ad6c2e60 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -43,6 +43,8 @@ type Runner struct { const WorkersNumEnvKey = "YOR_WORKER_NUM" +var mutex sync.Mutex + func (r *Runner) Init(commands *clioptions.TagOptions) error { dir := commands.Directory extraTags, extraTagGroups, err := loadExternalResources(commands.CustomTagging) @@ -170,7 +172,6 @@ func (r *Runner) isSkippedResource(resource string) bool { } func (r *Runner) TagFile(file string) { - var mutex sync.Mutex for _, parser := range r.parsers { if r.isFileSkipped(parser, file) { logger.Debug(fmt.Sprintf("%v parser Skipping %v", parser.Name(), file)) From 8c802729d25d7c522289168501537cf8c575a6a1 Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Sun, 16 Jun 2024 11:07:43 +0300 Subject: [PATCH 12/19] Another way to solve the issue --- .../structure/cloudformation_parser.go | 17 +++--- .../structure/cloudformation_parser_test.go | 16 +++--- src/common/parser.go | 2 +- src/common/runner/runner.go | 16 +++--- src/common/runner/runner_test.go | 21 ------- src/common/utils/utils.go | 1 + src/common/yaml/yaml_writer.go | 12 ++-- src/common/yaml/yaml_writer_test.go | 19 ++++--- .../structure/serverless_block_test.go | 2 +- src/serverless/structure/serverless_parser.go | 17 +++--- .../structure/serverless_parser_test.go | 12 ++-- src/terraform/structure/terraform_parser.go | 19 +++---- .../structure/terraform_parser_test.go | 56 +++++++++---------- tests/integration/integration_test.go | 6 +- 14 files changed, 96 insertions(+), 120 deletions(-) diff --git a/src/cloudformation/structure/cloudformation_parser.go b/src/cloudformation/structure/cloudformation_parser.go index 9db4694eb..04d0e7192 100644 --- a/src/cloudformation/structure/cloudformation_parser.go +++ b/src/cloudformation/structure/cloudformation_parser.go @@ -112,8 +112,7 @@ func goformationParse(file string) (*cloudformation.Template, error) { return template, err } -func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, []string, error) { - skipResourcesByComment := make([]string, 0) +func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock , error) { goformationLock.Lock() template, err := goformationParse(filePath) goformationLock.Unlock() @@ -122,12 +121,12 @@ func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, [ if err == nil { err = fmt.Errorf("failed to parse template %v", filePath) } - return nil, skipResourcesByComment, err + return nil, err } if template.Transform != nil { logger.Info(fmt.Sprintf("Skipping CFN template %s as SAM templates are not yet supported", filePath)) - return nil, skipResourcesByComment, nil + return nil , nil } resourceNames := make([]string, 0) @@ -139,13 +138,13 @@ func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, [ var resourceNamesToLines map[string]*structure.Lines switch utils.GetFileFormat(filePath) { case common.YmlFileType.FileFormat, common.YamlFileType.FileFormat: - resourceNamesToLines, skipResourcesByComment = yaml.MapResourcesLineYAML(filePath, resourceNames, ResourcesStartToken) + resourceNamesToLines = yaml.MapResourcesLineYAML(filePath, resourceNames, ResourcesStartToken) case common.JSONFileType.FileFormat: var fileBracketsMapping map[int]json.BracketPair resourceNamesToLines, fileBracketsMapping = json.MapResourcesLineJSON(filePath, resourceNames) p.FileToBracketMapping.Store(filePath, fileBracketsMapping) default: - return nil, skipResourcesByComment, fmt.Errorf("unsupported file type %s", utils.GetFileFormat(filePath)) + return nil , fmt.Errorf("unsupported file type %s", utils.GetFileFormat(filePath)) } minResourceLine := math.MaxInt8 @@ -182,9 +181,9 @@ func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, [ p.FileToResourcesLines.Store(filePath, structure.Lines{Start: minResourceLine, End: maxResourceLine}) - return parsedBlocks, skipResourcesByComment, nil + return parsedBlocks , nil } - return nil, skipResourcesByComment, err + return nil , err } func (p *CloudformationParser) extractTagsAndLines(filePath string, lines *structure.Lines, tagsValue reflect.Value) (structure.Lines, []tags.ITag) { @@ -238,7 +237,7 @@ func (p *CloudformationParser) WriteFile(readFilePath string, blocks []structure return err } - _, _, err = p.ParseFile(tempFile.Name()) + _, err = p.ParseFile(tempFile.Name()) if err != nil { return fmt.Errorf("editing file %v resulted in a malformed template, please open a github issue with the relevant details", readFilePath) } diff --git a/src/cloudformation/structure/cloudformation_parser_test.go b/src/cloudformation/structure/cloudformation_parser_test.go index c3d141ee5..85324973e 100644 --- a/src/cloudformation/structure/cloudformation_parser_test.go +++ b/src/cloudformation/structure/cloudformation_parser_test.go @@ -19,7 +19,7 @@ func TestCloudformationParser_ParseFile(t *testing.T) { directory := "../../../tests/cloudformation/resources/ebs" cfnParser := CloudformationParser{} cfnParser.Init(directory, nil) - cfnBlocks, _, err := cfnParser.ParseFile(directory + "/ebs.yaml") + cfnBlocks, err := cfnParser.ParseFile(directory + "/ebs.yaml") if err != nil { t.Errorf("ParseFile() error = %v", err) return @@ -42,7 +42,7 @@ func TestCloudformationParser_ParseFile(t *testing.T) { directory := "../../../tests/cloudformation/resources/ebs" cfnParser := CloudformationParser{} cfnParser.Init(directory, nil) - cfnBlocks, _, err := cfnParser.ParseFile(directory + "/ebs.json") + cfnBlocks, err := cfnParser.ParseFile(directory + "/ebs.json") if err != nil { t.Errorf("ParseFile() error = %v", err) return @@ -66,7 +66,7 @@ func TestCloudformationParser_ParseFile(t *testing.T) { cfnParser := CloudformationParser{} cfnParser.Init(directory, nil) sourceFile := directory + "/base.template" - cfnBlocks, _, _ := cfnParser.ParseFile(sourceFile) + cfnBlocks, _ := cfnParser.ParseFile(sourceFile) rawFileLines, _ := cfnParser.FileToResourcesLines.Load(sourceFile) resourceLines := rawFileLines.(structure.Lines) @@ -81,7 +81,7 @@ func TestCloudformationParser_ParseFile(t *testing.T) { cfnParser := CloudformationParser{} cfnParser.Init(directory, nil) sourceFile := directory + "/cfn.yaml" - cfnBlocks, _, _ := cfnParser.ParseFile(sourceFile) + cfnBlocks, _ := cfnParser.ParseFile(sourceFile) assert.Equal(t, 3, len(cfnBlocks)) }) @@ -89,7 +89,7 @@ func TestCloudformationParser_ParseFile(t *testing.T) { directory := "../../../tests/cloudformation/resources/special_tags" cfnParser := CloudformationParser{} cfnParser.Init(directory, nil) - cfnBlocks, _, err := cfnParser.ParseFile(directory + "/cfn.yaml") + cfnBlocks, err := cfnParser.ParseFile(directory + "/cfn.yaml") if err != nil { t.Errorf("ParseFile() error = %v", err) return @@ -123,7 +123,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { expected := map[string]*structure.Lines{ "NewVolume": {Start: 3, End: 15}, } - actual, _ := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) + actual := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) compareLines(t, expected, actual) }) @@ -136,7 +136,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { "EC2LaunchTemplateResource0": {Start: 18, End: 23}, "EC2LaunchTemplateResource1": {Start: 24, End: 34}, } - actual, _ := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) + actual := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) compareLines(t, expected, actual) }) @@ -169,7 +169,7 @@ func writeCFNTestHelper(t *testing.T, directory string, testFileName string, fil tagGroup.SetTags(extraTags) tagGroup.InitTagGroup("", []string{}, []string{}) writeFilePath := directory + "/" + testFileName + "_tagged." + fileType - cfnBlocks, _, err := cfnParser.ParseFile(readFilePath) + cfnBlocks, err := cfnParser.ParseFile(readFilePath) for _, block := range cfnBlocks { err := tagGroup.CreateTagsForBlock(block) if err != nil { diff --git a/src/common/parser.go b/src/common/parser.go index 7102263ad..339510335 100644 --- a/src/common/parser.go +++ b/src/common/parser.go @@ -6,7 +6,7 @@ type IParser interface { Init(rootDir string, args map[string]string) Name() string ValidFile(filePath string) bool - ParseFile(filePath string) ([]structure.IBlock, []string, error) + ParseFile(filePath string) ([]structure.IBlock , error) WriteFile(readFilePath string, blocks []structure.IBlock, writeFilePath string) error GetSkippedDirs() []string GetSupportedFileExtensions() []string diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index 6ad6c2e60..0ba3ff906 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -168,6 +168,12 @@ func (r *Runner) isSkippedResource(resource string) bool { return true } } + for _, skippedResource := range utils.SkipResourcesByComment { + if resource == skippedResource { + return true + } + } + return false } @@ -178,18 +184,12 @@ func (r *Runner) TagFile(file string) { continue } logger.Info(fmt.Sprintf("Tagging %v\n", file)) - blocks, skipResourcesByComment, err := parser.ParseFile(file) + blocks, err := parser.ParseFile(file) if err != nil { logger.Info(fmt.Sprintf("Failed to parse file %v with parser %v", file, reflect.TypeOf(parser))) continue } - mutex.Lock() - if r.skippedResources == nil { - r.skippedResources = []string{} - } - r.skippedResources = append(r.skippedResources, skipResourcesByComment...) - mutex.Unlock() - isFileTaggable := false + isFileTaggable := false for _, block := range blocks { if r.isSkippedResourceType(block.GetResourceType()) { continue diff --git a/src/common/runner/runner_test.go b/src/common/runner/runner_test.go index 982fb2631..55fc88946 100644 --- a/src/common/runner/runner_test.go +++ b/src/common/runner/runner_test.go @@ -221,27 +221,6 @@ func TestRunnerInternals(t *testing.T) { }) assert.NotContains(t, output, "aws_s3_bucket.test-bucket") }) - - t.Run("Test skip resource by comment", func(t *testing.T) { - options := clioptions.TagOptions{ - Directory: "../../../tests/terraform/skipComment/skipOne.tf", - Parsers: []string{"Terraform"}, - } - runner := Runner{} - err := runner.Init(&options) - if err != nil { - t.Error(err) - } - tfBlocks, skipResourcesByComment, err := runner.parsers[0].ParseFile(options.Directory) - if err != nil { - t.Error(err) - } - assert.Equal(t, 3, len(tfBlocks)) - runner.skippedResources = append(runner.skippedResources, skipResourcesByComment...) - result := []string{"aws_instance.example_instance"} - assert.Equal(t, result, runner.skippedResources) - }) - t.Run("Test skip resource - cloudformation", func(t *testing.T) { runner := Runner{} rootDir := "../../../tests/cloudformation" diff --git a/src/common/utils/utils.go b/src/common/utils/utils.go index 81002a1be..4cc3cc624 100644 --- a/src/common/utils/utils.go +++ b/src/common/utils/utils.go @@ -17,6 +17,7 @@ import ( // RemoveGcpInvalidChars Source of regex: https://cloud.google.com/compute/docs/labeling-resources var RemoveGcpInvalidChars = regexp.MustCompile(`[^\p{Ll}\p{Lo}\p{N}_-]`) +var SkipResourcesByComment = make([]string, 0) func InSlice[T comparable](elems []T, v T) bool { for _, s := range elems { diff --git a/src/common/yaml/yaml_writer.go b/src/common/yaml/yaml_writer.go index f4f45ac83..b52fd84f9 100644 --- a/src/common/yaml/yaml_writer.go +++ b/src/common/yaml/yaml_writer.go @@ -266,10 +266,8 @@ func FindTagsLinesYAML(textLines []string, tagsAttributeName string) (structure. return tagsLines, tagsExist } -func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStartToken string) (map[string]*structure.Lines, []string) { +func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStartToken string) (map[string]*structure.Lines) { resourceToLines := make(map[string]*structure.Lines) - skipResourcesByComment := make([]string, 0) - for _, resourceName := range resourceNames { // initialize a map between resource name and its lines in file resourceToLines[resourceName] = &structure.Lines{Start: -1, End: -1} @@ -278,7 +276,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar file, err := os.ReadFile(filePath) if err != nil { logger.Warning(fmt.Sprintf("failed to read file %s", filePath)) - return nil, skipResourcesByComment + return nil } readResources := false @@ -290,7 +288,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar cleanContent := strings.TrimSpace(line) if strings.HasPrefix(cleanContent, resourcesStartToken+":") { if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIPALL" { - skipResourcesByComment = append(skipResourcesByComment, resourceNames...) + utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, resourceNames...) } readResources = true resourcesIndent = countLeadingSpaces(line) @@ -301,7 +299,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar if i > 0 { if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIP" { - skipResourcesByComment = append(skipResourcesByComment, strings.Trim(strings.TrimSpace(line), ":")) + utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, strings.Trim(strings.TrimSpace(line), ":")) } } @@ -337,7 +335,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar // Handle last line of resource is last line of file resourceToLines[latestResourceName].End = findLastNonEmptyLine(fileLines, len(fileLines)-1) } - return resourceToLines, skipResourcesByComment + return resourceToLines } func countLeadingSpaces(line string) int { diff --git a/src/common/yaml/yaml_writer_test.go b/src/common/yaml/yaml_writer_test.go index 87147978a..1860fcb6d 100644 --- a/src/common/yaml/yaml_writer_test.go +++ b/src/common/yaml/yaml_writer_test.go @@ -11,6 +11,7 @@ import ( "github.com/bridgecrewio/yor/src/common/structure" "github.com/bridgecrewio/yor/src/common/tagging/simple" "github.com/bridgecrewio/yor/src/common/tagging/tags" + "github.com/bridgecrewio/yor/src/common/utils" "github.com/stretchr/testify/assert" ) @@ -228,13 +229,13 @@ func TestTagReplacement(t *testing.T) { }) t.Run("Test line computation with duplicate - CFN", func(t *testing.T) { - res, _ := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_cfn.yaml", []string{"S3Bucket", "CloudFrontDistribution"}, "Resources") + res := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_cfn.yaml", []string{"S3Bucket", "CloudFrontDistribution"}, "Resources") assert.Equal(t, *res["S3Bucket"], structure.Lines{Start: 14, End: 17}) assert.Equal(t, *res["CloudFrontDistribution"], structure.Lines{Start: 18, End: 60}) }) t.Run("Test line computation with duplicate - SLS", func(t *testing.T) { - res, _ := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_sls.yaml", []string{"attribute", "zone", "customer", "apiVersion"}, "functions") + res := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_sls.yaml", []string{"attribute", "zone", "customer", "apiVersion"}, "functions") assert.Equal(t, *res["apiVersion"], structure.Lines{Start: 7, End: 12}) assert.Equal(t, *res["customer"], structure.Lines{Start: 14, End: 24}) assert.Equal(t, *res["zone"], structure.Lines{Start: 26, End: 38}) @@ -247,21 +248,21 @@ func TestYaml_SkipResourceByComment(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/skipOne.yaml" resorseSkip := []string{"NewVolume"} expectedResourceNames := []string{"NewVolume", "NewVolume2"} - _, skipResourcesByComment := MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") - assert.Equal(t, skipResourcesByComment, resorseSkip) - assert.NotEqual(t, skipResourcesByComment, "NewVolume2") + _ = MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Equal(t, utils.SkipResourcesByComment, resorseSkip) + assert.NotEqual(t,utils.SkipResourcesByComment, "NewVolume2") }) t.Run("All resources with skip comment added to skipResourcesByComment slice", func(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/skipAll.yaml" resorseSkip := []string{"NewVolume", "NewVolume2"} expectedResourceNames := []string{"NewVolume", "NewVolume2"} - _, skipResourcesByComment := MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") - assert.Equal(t, skipResourcesByComment, resorseSkip) + _ = MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Equal(t, utils.SkipResourcesByComment, resorseSkip) }) t.Run("No resources with skip all comment in the file, skipResourcesByComment slice should be empty", func(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/noSkip.yaml" expectedResourceNames := []string{"NewVolume"} - _, skipResourcesByComment := MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") - assert.Empty(t, skipResourcesByComment) + _ = MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Empty(t, utils.SkipResourcesByComment) }) } diff --git a/src/serverless/structure/serverless_block_test.go b/src/serverless/structure/serverless_block_test.go index 5e3b69447..f4c96a587 100644 --- a/src/serverless/structure/serverless_block_test.go +++ b/src/serverless/structure/serverless_block_test.go @@ -54,7 +54,7 @@ func TestServerlessBlock_UpdateTags(t *testing.T) { &tags.Tag{Key: "TAG1_FUNC", Value: "Func1 Tag Value"}, &tags.Tag{Key: "TAG2_FUNC", Value: "Func2 Tag Value"}, &tags.Tag{Key: "yor_trace", Value: "yor_trace"}, &tags.Tag{Key: "git_last_modified_at", Value: "2"}, } absFilePath, _ := filepath.Abs(strings.Join([]string{parser.YamlParser.RootDir, "serverless.yml"}, "/")) - template, _, err := parser.ParseFile(absFilePath) + template, err := parser.ParseFile(absFilePath) if err != nil { t.Errorf("There was an error processing the cloudformation template: %s", err) } diff --git a/src/serverless/structure/serverless_parser.go b/src/serverless/structure/serverless_parser.go index e3cb6a67b..25caeed35 100644 --- a/src/serverless/structure/serverless_parser.go +++ b/src/serverless/structure/serverless_parser.go @@ -71,13 +71,12 @@ func (p *ServerlessParser) ValidFile(file string) bool { return true } -func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, []string, error) { - skipResourcesByComment := make([]string, 0) +func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock , error) { parsedBlocks := make([]structure.IBlock, 0) fileFormat := utils.GetFileFormat(filePath) fileName := filepath.Base(filePath) if !(fileName == fmt.Sprintf("serverless.%s", fileFormat) || fileName == fmt.Sprintf("config.%s", fileFormat)) { - return nil, skipResourcesByComment, nil + return nil, nil } // #nosec G304 - file is from user template, err := serverlessParse(filePath) @@ -88,10 +87,10 @@ func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, []str if err == nil { err = fmt.Errorf("failed to parse file %v", filePath) } - return nil, skipResourcesByComment, err + return nil, err } if template.Functions == nil && template.Resources.Resources == nil { - return parsedBlocks, skipResourcesByComment, nil + return parsedBlocks, nil } // cfnStackTagsResource := p.template.Provider.CFNTags @@ -102,9 +101,9 @@ func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, []str } switch utils.GetFileFormat(filePath) { case common.YmlFileType.FileFormat, common.YamlFileType.FileFormat: - resourceNamesToLines, skipResourcesByComment = yamlUtils.MapResourcesLineYAML(filePath, resourceNames, FunctionsSectionName) + resourceNamesToLines = yamlUtils.MapResourcesLineYAML(filePath, resourceNames, FunctionsSectionName) default: - return nil, skipResourcesByComment, fmt.Errorf("unsupported file type %s", utils.GetFileFormat(filePath)) + return nil, fmt.Errorf("unsupported file type %s", utils.GetFileFormat(filePath)) } minResourceLine := math.MaxInt8 maxResourceLine := 0 @@ -142,7 +141,7 @@ func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, []str p.YamlParser.FileToResourcesLines.Store(filePath, structure.Lines{Start: minResourceLine, End: maxResourceLine}) } - return parsedBlocks, skipResourcesByComment, nil + return parsedBlocks, nil } func (p *ServerlessParser) WriteFile(readFilePath string, blocks []structure.IBlock, writeFilePath string) error { @@ -161,7 +160,7 @@ func (p *ServerlessParser) WriteFile(readFilePath string, blocks []structure.IBl if err != nil { return err } - _, _, err = p.ParseFile(tempFile.Name()) + _, err = p.ParseFile(tempFile.Name()) if err != nil { return fmt.Errorf("editing file %v resulted in a malformed template, please open a github issue with the relevant details", readFilePath) } diff --git a/src/serverless/structure/serverless_parser_test.go b/src/serverless/structure/serverless_parser_test.go index 69f33cc0f..bda0224fe 100644 --- a/src/serverless/structure/serverless_parser_test.go +++ b/src/serverless/structure/serverless_parser_test.go @@ -29,7 +29,7 @@ func TestServerlessParser_ParseFile(t *testing.T) { slsParser.Init(directory, nil) slsFilepath, _ := filepath.Abs(filepath.Join(slsParser.YamlParser.RootDir, "serverless.yml")) expectedSlsFilepath, _ := filepath.Abs(filepath.Join(slsParser.YamlParser.RootDir, "serverless_expected.yaml")) - slsBlocks, _, err := slsParser.ParseFile(slsFilepath) + slsBlocks, err := slsParser.ParseFile(slsFilepath) if err != nil { t.Errorf("ParseFile() error = %v", err) return @@ -94,7 +94,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { slsFilepath, _ := filepath.Abs(strings.Join([]string{directory, "serverless.yml"}, "/")) slsParser := ServerlessParser{} slsParser.Init(directory, nil) - slsBlocks, _, err := slsParser.ParseFile(slsFilepath) + slsBlocks, err := slsParser.ParseFile(slsFilepath) if err != nil { t.Errorf("ParseFile() error = %v", err) return @@ -120,7 +120,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { slsParser := ServerlessParser{} slsParser.Init(directory, nil) var func1Block, func2Block *ServerlessBlock - slsBlocks, _, err := slsParser.ParseFile(slsFilepath) + slsBlocks, err := slsParser.ParseFile(slsFilepath) for _, block := range slsBlocks { castedBlock := block.(*ServerlessBlock) if castedBlock.Name == "myFunction" { @@ -150,7 +150,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { slsParser := ServerlessParser{} slsParser.Init(directory, nil) var func1Block, func2Block *ServerlessBlock - slsBlocks, _, err := slsParser.ParseFile(slsFilepath) + slsBlocks, err := slsParser.ParseFile(slsFilepath) for _, block := range slsBlocks { castedBlock := block.(*ServerlessBlock) if castedBlock.Name == "myFunction" { @@ -179,7 +179,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { slsFilepath, _ := filepath.Abs(strings.Join([]string{directory, "file.yml"}, "/")) slsParser := ServerlessParser{} slsParser.Init(directory, nil) - parsedBlocks, _, _ := slsParser.ParseFile(slsFilepath) + parsedBlocks, _ := slsParser.ParseFile(slsFilepath) if parsedBlocks != nil { t.Fail() } @@ -200,7 +200,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { tagGroup.SetTags(extraTags) tagGroup.InitTagGroup("", []string{}, []string{}) writeFilePath := directory + "/serverless_tagged.yml" - slsBlocks, _, err := slsParser.ParseFile(readFilePath) + slsBlocks , err := slsParser.ParseFile(readFilePath) for _, block := range slsBlocks { err := tagGroup.CreateTagsForBlock(block) if err != nil { diff --git a/src/terraform/structure/terraform_parser.go b/src/terraform/structure/terraform_parser.go index 033fbadf9..cca8c780d 100644 --- a/src/terraform/structure/terraform_parser.go +++ b/src/terraform/structure/terraform_parser.go @@ -126,13 +126,12 @@ func (p *TerraformParser) ValidFile(_ string) bool { return true } -func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, []string, error) { +func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock , error) { // #nosec G304 // read file bytes - skipResourcesByComment := make([]string, 0) src, err := os.ReadFile(filePath) if err != nil { - return nil, skipResourcesByComment, fmt.Errorf("failed to read file %s because %s", filePath, err) + return nil, fmt.Errorf("failed to read file %s because %s", filePath, err) } lines := strings.Split(string(src), "\n") @@ -140,16 +139,16 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, []stri hclFile, diagnostics := hclwrite.ParseConfig(src, filePath, hcl.InitialPos) if diagnostics != nil && diagnostics.HasErrors() { hclErrors := diagnostics.Errs() - return nil, skipResourcesByComment, fmt.Errorf("failed to parse hcl file %s because of errors %s", filePath, hclErrors) + return nil, fmt.Errorf("failed to parse hcl file %s because of errors %s", filePath, hclErrors) } hclSyntaxFile, diagnostics := hclsyntax.ParseConfig(src, filePath, hcl.InitialPos) if diagnostics != nil && diagnostics.HasErrors() { hclErrors := diagnostics.Errs() - return nil, skipResourcesByComment, fmt.Errorf("failed to parse hcl file %s because of errors %s", filePath, hclErrors) + return nil, fmt.Errorf("failed to parse hcl file %s because of errors %s", filePath, hclErrors) } if hclFile == nil || hclSyntaxFile == nil { - return nil, skipResourcesByComment, fmt.Errorf("failed to parse hcl file %s", filePath) + return nil , fmt.Errorf("failed to parse hcl file %s", filePath) } syntaxBlocks := hclSyntaxFile.Body.(*hclsyntax.Body).Blocks @@ -186,13 +185,13 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, []stri } if strings.ToUpper(strings.TrimSpace(lineAbove)) == "#YOR:SKIP" || skipAll { - skipResourcesByComment = append(skipResourcesByComment, terraformBlock.GetResourceID()) + utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, terraformBlock.GetResourceID()) } } parsedBlocks = append(parsedBlocks, terraformBlock) } - return parsedBlocks, skipResourcesByComment, nil + return parsedBlocks , nil } func (p *TerraformParser) WriteFile(readFilePath string, blocks []structure.IBlock, writeFilePath string) error { @@ -242,7 +241,7 @@ func (p *TerraformParser) WriteFile(readFilePath string, blocks []structure.IBlo return err } - _, _, err = p.ParseFile(tempFile.Name()) + _, err = p.ParseFile(tempFile.Name()) if err != nil { return fmt.Errorf("editing file %v resulted in malformed terraform, please open a github issue with the relevant details", readFilePath) } @@ -614,7 +613,7 @@ func (p *TerraformParser) isModuleTaggable(fp string, moduleName string, moduleD files, _ := os.ReadDir(expectedModuleDir) for _, f := range files { if strings.HasSuffix(f.Name(), ".tf") { - blocks, _, _ := p.ParseFile(filepath.Join(expectedModuleDir, f.Name())) + blocks, _ := p.ParseFile(filepath.Join(expectedModuleDir, f.Name())) for _, b := range blocks { if b.(*TerraformBlock).HclSyntaxBlock.Type == VariableBlockType { for _, tagAtt := range tagAtts { diff --git a/src/terraform/structure/terraform_parser_test.go b/src/terraform/structure/terraform_parser_test.go index 9f687e70d..3f987ead6 100644 --- a/src/terraform/structure/terraform_parser_test.go +++ b/src/terraform/structure/terraform_parser_test.go @@ -29,12 +29,12 @@ func TestTerraformParser_SkipResourceByComment(t *testing.T) { p.Init("../../../tests/terraform/skipComment/", nil) defer p.Close() filePath := "../../../tests/terraform/skipComment/skipAll.tf" - _, skipResourcesByComment, err := p.ParseFile(filePath) + _, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } exceptedSkipResources := []string{"aws_vpc.example_vpc", "aws_subnet.example_subnet", "aws_instance.example_instance"} - assert.Equal(t, exceptedSkipResources, skipResourcesByComment) + assert.Equal(t, exceptedSkipResources, utils.SkipResourcesByComment) }) t.Run("No resources with skip comment in the file, skipResourcesByComment slice should be empty", func(t *testing.T) { @@ -43,11 +43,11 @@ func TestTerraformParser_SkipResourceByComment(t *testing.T) { p.Init("../../../tests/terraform/skipComment/", nil) defer p.Close() filePath := "../../../tests/terraform/skipComment/noSkip.tf" - _, skipResourcesByComment, err := p.ParseFile(filePath) + _, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } - assert.Empty(t, skipResourcesByComment) + assert.Empty(t, utils.SkipResourcesByComment) }) t.Run("One resource with skip comment, only that resource added to skipResourcesByComment slice", func(t *testing.T) { @@ -56,12 +56,12 @@ func TestTerraformParser_SkipResourceByComment(t *testing.T) { p.Init("../../../tests/terraform/skipComment/", nil) defer p.Close() filePath := "../../../tests/terraform/skipComment/skipOne.tf" - _, skipResourcesByComment, err := p.ParseFile(filePath) + _, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } exceptedSkipResources := []string{"aws_instance.example_instance"} - assert.Equal(t, exceptedSkipResources, skipResourcesByComment) + assert.Equal(t, exceptedSkipResources, utils.SkipResourcesByComment) }) } @@ -88,7 +88,7 @@ func TestTerraformParser_ParseFile(t *testing.T) { "eks_subnet2": {Start: 55, End: 63}, "eks_cluster": {Start: 65, End: 78}, } - parsedBlocks, _, err := p.ParseFile(filePath) + parsedBlocks, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } @@ -140,7 +140,7 @@ func TestTerraformParser_ParseFile(t *testing.T) { "instance_null_tags": nil, } - parsedBlocks, _, err := p.ParseFile(filePath) + parsedBlocks, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } @@ -164,7 +164,7 @@ func TestTerraformParser_ParseFile(t *testing.T) { p.Init("../../../tests/terraform/resources", nil) defer p.Close() filePath := "../../../tests/terraform/resources/collision/main.tf" - parsedBlocks, _, err := p.ParseFile(filePath) + parsedBlocks, err := p.ParseFile(filePath) assert.Nil(t, parsedBlocks) assert.NotNil(t, err) }) @@ -174,7 +174,7 @@ func TestTerraformParser_ParseFile(t *testing.T) { p.Init("../../../tests/terraform/malformed_file_in_dir", nil) defer p.Close() filePath := "../../../tests/terraform/resources/malformed_file_in_dir/trail.tf" - parsedBlocks, _, err := p.ParseFile(filePath) + parsedBlocks, err := p.ParseFile(filePath) assert.Nil(t, parsedBlocks) assert.NotNil(t, err) }) @@ -223,7 +223,7 @@ func TestTerraformParser_Module(t *testing.T) { defer func() { _ = os.WriteFile(writeFilePath, writeFileBytes, 0644) }() - parsedBlocks, _, err := p.ParseFile(filePath) + parsedBlocks, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } @@ -242,7 +242,7 @@ func TestTerraformParser_Module(t *testing.T) { if err != nil { t.Error(err) } - parsedTaggedFileTags, _, err := p.ParseFile(writeFilePath) + parsedTaggedFileTags, err := p.ParseFile(writeFilePath) if err != nil { t.Error(err) } @@ -286,7 +286,7 @@ func TestTerraformParser_Module(t *testing.T) { defer func() { _ = os.WriteFile(writeFilePath, writeFileBytes, 0644) }() - parsedBlocks, _, err := p.ParseFile(filePath) + parsedBlocks, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } @@ -305,7 +305,7 @@ func TestTerraformParser_Module(t *testing.T) { if err != nil { t.Error(err) } - parsedTaggedFileTags, _, err := p.ParseFile(writeFilePath) + parsedTaggedFileTags, err := p.ParseFile(writeFilePath) if err != nil { t.Error(err) } @@ -342,7 +342,7 @@ func TestTerraformParser_Module(t *testing.T) { defer func() { _ = os.WriteFile(writeFilePath, writeFileBytes, 0644) }() - parsedBlocks, _, err := p.ParseFile(filePath) + parsedBlocks, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } @@ -359,7 +359,7 @@ func TestTerraformParser_Module(t *testing.T) { if err != nil { t.Error(err) } - parsedTaggedFileTags, _, err := p.ParseFile(writeFilePath) + parsedTaggedFileTags, err := p.ParseFile(writeFilePath) if err != nil { t.Error(err) } @@ -385,7 +385,7 @@ func TestTerraformParser_Module(t *testing.T) { p := &TerraformParser{} p.Init("../../../tests/terraform/mixed", nil) defer p.Close() - blocks, _, err := p.ParseFile("../../../tests/terraform/mixed/mixed.tf") + blocks, err := p.ParseFile("../../../tests/terraform/mixed/mixed.tf") if err != nil { t.Fail() } @@ -399,7 +399,7 @@ func TestTerraformParser_Module(t *testing.T) { defer p.Close() sourceFilePath := "../../../tests/terraform/data/main.tf" expectedFileName := "../../../tests/terraform/data/expected.txt" - blocks, _, err := p.ParseFile(sourceFilePath) + blocks, err := p.ParseFile(sourceFilePath) if err != nil { t.Fail() } @@ -426,7 +426,7 @@ func TestTerraformParser_Module(t *testing.T) { defer p.Close() sourceFilePath := "../../../tests/terraform/tags_with_comments/main.tf" expectedFileName := "../../../tests/terraform/tags_with_comments/expected.txt" - blocks, _, err := p.ParseFile(sourceFilePath) + blocks, err := p.ParseFile(sourceFilePath) if err != nil { t.Fail() } @@ -450,7 +450,7 @@ func TestTerraformParser_Module(t *testing.T) { p := &TerraformParser{} p.Init("../../../tests/terraform/supported", nil) defer p.Close() - blocks, _, err := p.ParseFile("../../../tests/terraform/supported/unsupported.tf") + blocks, err := p.ParseFile("../../../tests/terraform/supported/unsupported.tf") if err != nil { t.Fail() } @@ -463,7 +463,7 @@ func TestTerraformParser_Module(t *testing.T) { defer p.Close() sourceFilePath := "../../../tests/terraform/module/module_with_tags/main.tf" expectedFileName := "../../../tests/terraform/module/module_with_tags/expected.txt" - blocks, _, err := p.ParseFile(sourceFilePath) + blocks, err := p.ParseFile(sourceFilePath) if err != nil { t.Fail() } @@ -491,7 +491,7 @@ func TestTerraformParser_Module(t *testing.T) { p.Init("../../../tests/terraform/module/tfe_module", nil) defer p.Close() sourceFilePath := "../../../tests/terraform/module/tfe_module/main.tf" - blocks, _, err := p.ParseFile(sourceFilePath) + blocks, err := p.ParseFile(sourceFilePath) if err != nil { t.Fail() } @@ -508,7 +508,7 @@ func TestTerraformParser_Module(t *testing.T) { defer p.Close() sourceFilePath := "../../../tests/terraform/module/module/main.tf" expectedFileName := "../../../tests/terraform/module/module/expected.txt" - blocks, _, err := p.ParseFile(sourceFilePath) + blocks, err := p.ParseFile(sourceFilePath) if err != nil { t.Fail() } @@ -539,7 +539,7 @@ func TestTerraformParser_Module(t *testing.T) { filePath := "../../../tests/terraform/resources/attributescenarios/main.tf" resultFilePath := "../../../tests/terraform/resources/attributescenarios/main_result.tf" expectedFilePath := "../../../tests/terraform/resources/attributescenarios/expected.txt" - blocks, _, _ := p.ParseFile(filePath) + blocks, _ := p.ParseFile(filePath) assert.Equal(t, 8, len(blocks)) for _, block := range blocks { if block.IsBlockTaggable() { @@ -568,7 +568,7 @@ func TestTerraformParser_Module(t *testing.T) { expectedFiles := []string{"main.tf", "sub_local_module/main.tf", "sub_local_module/variables.tf"} for _, file := range expectedFiles { filePath := filepath.Join(directory, file) - fileBlocks, _, err := terraformParser.ParseFile(filePath) + fileBlocks, err := terraformParser.ParseFile(filePath) if err != nil { assert.Fail(t, fmt.Sprintf("Failed to parse file %v", filePath)) } @@ -588,7 +588,7 @@ func TestTerraformParser_Module(t *testing.T) { terraformParser := TerraformParser{} terraformParser.Init(directory, nil) defer terraformParser.Close() - blocks, _, _ := terraformParser.ParseFile(directory + "/main.tf") + blocks, _ := terraformParser.ParseFile(directory + "/main.tf") assert.Equal(t, 8, len(blocks)) for _, block := range blocks { assert.True(t, block.IsBlockTaggable(), fmt.Sprintf("Block %v should be taggable", block.GetResourceID())) @@ -611,7 +611,7 @@ func TestTerraformParser_Module(t *testing.T) { defer func() { _ = os.WriteFile(writeFilePath, writeFileBytes, 0644) }() - parsedBlocks, _, err := p.ParseFile(filePath) + parsedBlocks, err := p.ParseFile(filePath) if err != nil { t.Errorf("failed to read hcl file because %s", err) } @@ -628,7 +628,7 @@ func TestTerraformParser_Module(t *testing.T) { if err != nil { t.Error(err) } - parsedTaggedFileTags, _, err := p.ParseFile(writeFilePath) + parsedTaggedFileTags, err := p.ParseFile(writeFilePath) if err != nil { t.Error(err) } diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 9d32f612b..41f6d5b8d 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -305,7 +305,7 @@ func TestTagUncommittedResults(t *testing.T) { terraformParser.Init(terragoatAWSDirectory, nil) dbAppFile := path.Join(terragoatAWSDirectory, "db-app.tf") - blocks,_, err := terraformParser.ParseFile(dbAppFile) + blocks, err := terraformParser.ParseFile(dbAppFile) failIfErr(t, err) defaultInstanceBlock := blocks[0].(*terraformStructure.TerraformBlock) if defaultInstanceBlock.GetResourceID() != "aws_db_instance.default" { @@ -371,7 +371,7 @@ func TestTagUncommittedResults(t *testing.T) { terraformParser.Init(terragoatAWSDirectory, nil) dbAppFile := path.Join(terragoatAWSDirectory, "db-app.tf") - blocks,_, err := terraformParser.ParseFile(dbAppFile) + blocks, err := terraformParser.ParseFile(dbAppFile) failIfErr(t, err) defaultInstanceBlock := blocks[0].(*terraformStructure.TerraformBlock) if defaultInstanceBlock.GetResourceID() != "aws_db_instance.default" { @@ -422,7 +422,7 @@ func TestLocalModules(t *testing.T) { terraformParser := terraformStructure.TerraformParser{} terraformParser.Init(targetDirectory, nil) dbAppFile := path.Join(targetDirectory, "module.broker.tf") - blocks,_, _ := terraformParser.ParseFile(dbAppFile) + blocks, _ := terraformParser.ParseFile(dbAppFile) defaultInstanceBlock := blocks[0].(*terraformStructure.TerraformBlock) rawTags := defaultInstanceBlock.HclSyntaxBlock.Body.Attributes["common_tags"] From ca99cf1e36be7039643dd7a225da8991147605b2 Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Sun, 16 Jun 2024 14:36:30 +0300 Subject: [PATCH 13/19] Fix UT --- src/common/yaml/yaml_writer_test.go | 8 +++++++- src/terraform/structure/terraform_parser_test.go | 6 ++++++ .../resources/skipComment/skipOne.yaml | 15 +++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/common/yaml/yaml_writer_test.go b/src/common/yaml/yaml_writer_test.go index 1860fcb6d..e7031c138 100644 --- a/src/common/yaml/yaml_writer_test.go +++ b/src/common/yaml/yaml_writer_test.go @@ -247,10 +247,11 @@ func TestYaml_SkipResourceByComment(t *testing.T) { t.Run("Test some resources with skip comment added to skipResourcesByComment slice", func(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/skipOne.yaml" resorseSkip := []string{"NewVolume"} - expectedResourceNames := []string{"NewVolume", "NewVolume2"} + expectedResourceNames := []string{"NewVolume"} _ = MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") assert.Equal(t, utils.SkipResourcesByComment, resorseSkip) assert.NotEqual(t,utils.SkipResourcesByComment, "NewVolume2") + defer resetSkipArr() }) t.Run("All resources with skip comment added to skipResourcesByComment slice", func(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/skipAll.yaml" @@ -258,11 +259,16 @@ func TestYaml_SkipResourceByComment(t *testing.T) { expectedResourceNames := []string{"NewVolume", "NewVolume2"} _ = MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") assert.Equal(t, utils.SkipResourcesByComment, resorseSkip) + defer resetSkipArr() }) t.Run("No resources with skip all comment in the file, skipResourcesByComment slice should be empty", func(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/noSkip.yaml" expectedResourceNames := []string{"NewVolume"} _ = MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") assert.Empty(t, utils.SkipResourcesByComment) + defer resetSkipArr() }) } +func resetSkipArr() { + utils.SkipResourcesByComment = []string{} +} diff --git a/src/terraform/structure/terraform_parser_test.go b/src/terraform/structure/terraform_parser_test.go index 3f987ead6..6743647e0 100644 --- a/src/terraform/structure/terraform_parser_test.go +++ b/src/terraform/structure/terraform_parser_test.go @@ -35,6 +35,7 @@ func TestTerraformParser_SkipResourceByComment(t *testing.T) { } exceptedSkipResources := []string{"aws_vpc.example_vpc", "aws_subnet.example_subnet", "aws_instance.example_instance"} assert.Equal(t, exceptedSkipResources, utils.SkipResourcesByComment) + defer resetSkipArr() }) t.Run("No resources with skip comment in the file, skipResourcesByComment slice should be empty", func(t *testing.T) { @@ -48,6 +49,7 @@ func TestTerraformParser_SkipResourceByComment(t *testing.T) { t.Errorf("failed to read hcl file because %s", err) } assert.Empty(t, utils.SkipResourcesByComment) + defer resetSkipArr() }) t.Run("One resource with skip comment, only that resource added to skipResourcesByComment slice", func(t *testing.T) { @@ -62,6 +64,7 @@ func TestTerraformParser_SkipResourceByComment(t *testing.T) { } exceptedSkipResources := []string{"aws_instance.example_instance"} assert.Equal(t, exceptedSkipResources, utils.SkipResourcesByComment) + defer resetSkipArr() }) } @@ -830,3 +833,6 @@ func compareTokenArrays(got []hclwrite.Tokens, want []hclwrite.Tokens) bool { return true } +func resetSkipArr() { + utils.SkipResourcesByComment = []string{} + } diff --git a/tests/cloudformation/resources/skipComment/skipOne.yaml b/tests/cloudformation/resources/skipComment/skipOne.yaml index a8017e5a7..789df3ca2 100644 --- a/tests/cloudformation/resources/skipComment/skipOne.yaml +++ b/tests/cloudformation/resources/skipComment/skipOne.yaml @@ -15,6 +15,21 @@ Resources: Value: !Ref EnvironmentName AvailabilityZone: us-west-2a DeletionPolicy: Snapshot + + NewVolume2: + Type: AWS::EC2::Volume + Properties: + Size: 100 + Encrypted: true + #Encrypted: false + Tags: + - Key: MyTag + Value: TagValue + - Key: Name + Value: !Ref EnvironmentName + AvailabilityZone: us-west-2a + DeletionPolicy: Snapshot + Outputs: VolumeId: From 56fd2e8fa4b8297ad458c108e6cf2f7993c10b5d Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Mon, 17 Jun 2024 13:05:47 +0300 Subject: [PATCH 14/19] readme file and fixing code review --- README.md | 48 +++++++++++++++++++ .../structure/cloudformation_parser.go | 10 ++-- src/common/parser.go | 2 +- src/common/runner/runner.go | 4 +- src/common/yaml/yaml_writer.go | 2 +- src/common/yaml/yaml_writer_test.go | 2 +- src/serverless/structure/serverless_parser.go | 2 +- .../structure/serverless_parser_test.go | 2 +- src/terraform/structure/terraform_parser.go | 7 ++- .../structure/terraform_parser_test.go | 2 +- 10 files changed, 63 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 4dcae5c4d..5276be3f4 100644 --- a/README.md +++ b/README.md @@ -225,7 +225,55 @@ yor list-tags yor list-tags --tag-groups git ``` +## Supporting comment format +To prevent resource from being tagged, apply the following comment pattern above the resource, currently supported only in Terraform and CloudFormation files. +## Example + +# skip specific resource - #yor:skip +```sh +## for terraform files +#yor:Skip +resource "aws_instance" "example_instance" { + ami = "ami-0c55b159cbfafe1f0" + instance_type = "t2.micro" + subnet_id = aws_subnet.example_subnet.id } + +## for cloudformation files +#yor:skip + ExampleInt: + Type: AWS::Lambda::Function + Properties: + Description: An example template +``` + +# skip all rsources in the page - #yor:skipAll +```sh +## for terraform files +#yor:skipAll +resource "aws_vpc" "example_vpc" { + cidr_block = "10.0.0.0/16" } + +resource "aws_subnet" "example_subnet" { + vpc_id = aws_vpc.example_vpc.id + cidr_block = "10.0.1.0/24" + availability_zone = "us-west-1a" } + +## for cloudformation files +#yor:skipAll +Resources: + NewVolume: + Type: AWS::EC2::Volume + Properties: + Size: 100 + + NewVolume2: + Type: AWS::EC2::Volume + Tags: + - Key: MyTag + Value: TagValue + - Key: Name +``` ### What is Yor trace? yor_trace is a magical tag creating a unique identifier for an IaC resource code block. diff --git a/src/cloudformation/structure/cloudformation_parser.go b/src/cloudformation/structure/cloudformation_parser.go index 04d0e7192..092286aed 100644 --- a/src/cloudformation/structure/cloudformation_parser.go +++ b/src/cloudformation/structure/cloudformation_parser.go @@ -112,7 +112,7 @@ func goformationParse(file string) (*cloudformation.Template, error) { return template, err } -func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock , error) { +func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, error) { goformationLock.Lock() template, err := goformationParse(filePath) goformationLock.Unlock() @@ -126,7 +126,7 @@ func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock , if template.Transform != nil { logger.Info(fmt.Sprintf("Skipping CFN template %s as SAM templates are not yet supported", filePath)) - return nil , nil + return nil, nil } resourceNames := make([]string, 0) @@ -144,7 +144,7 @@ func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock , resourceNamesToLines, fileBracketsMapping = json.MapResourcesLineJSON(filePath, resourceNames) p.FileToBracketMapping.Store(filePath, fileBracketsMapping) default: - return nil , fmt.Errorf("unsupported file type %s", utils.GetFileFormat(filePath)) + return nil, fmt.Errorf("unsupported file type %s", utils.GetFileFormat(filePath)) } minResourceLine := math.MaxInt8 @@ -181,9 +181,9 @@ func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock , p.FileToResourcesLines.Store(filePath, structure.Lines{Start: minResourceLine, End: maxResourceLine}) - return parsedBlocks , nil + return parsedBlocks, nil } - return nil , err + return nil, err } func (p *CloudformationParser) extractTagsAndLines(filePath string, lines *structure.Lines, tagsValue reflect.Value) (structure.Lines, []tags.ITag) { diff --git a/src/common/parser.go b/src/common/parser.go index 339510335..b4982e296 100644 --- a/src/common/parser.go +++ b/src/common/parser.go @@ -6,7 +6,7 @@ type IParser interface { Init(rootDir string, args map[string]string) Name() string ValidFile(filePath string) bool - ParseFile(filePath string) ([]structure.IBlock , error) + ParseFile(filePath string) ([]structure.IBlock, error) WriteFile(readFilePath string, blocks []structure.IBlock, writeFilePath string) error GetSkippedDirs() []string GetSupportedFileExtensions() []string diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index 0ba3ff906..17c2d28f7 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -43,8 +43,6 @@ type Runner struct { const WorkersNumEnvKey = "YOR_WORKER_NUM" -var mutex sync.Mutex - func (r *Runner) Init(commands *clioptions.TagOptions) error { dir := commands.Directory extraTags, extraTagGroups, err := loadExternalResources(commands.CustomTagging) @@ -189,7 +187,7 @@ func (r *Runner) TagFile(file string) { logger.Info(fmt.Sprintf("Failed to parse file %v with parser %v", file, reflect.TypeOf(parser))) continue } - isFileTaggable := false + isFileTaggable := false for _, block := range blocks { if r.isSkippedResourceType(block.GetResourceType()) { continue diff --git a/src/common/yaml/yaml_writer.go b/src/common/yaml/yaml_writer.go index b52fd84f9..b3005b3c3 100644 --- a/src/common/yaml/yaml_writer.go +++ b/src/common/yaml/yaml_writer.go @@ -266,7 +266,7 @@ func FindTagsLinesYAML(textLines []string, tagsAttributeName string) (structure. return tagsLines, tagsExist } -func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStartToken string) (map[string]*structure.Lines) { +func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStartToken string) map[string]*structure.Lines { resourceToLines := make(map[string]*structure.Lines) for _, resourceName := range resourceNames { // initialize a map between resource name and its lines in file diff --git a/src/common/yaml/yaml_writer_test.go b/src/common/yaml/yaml_writer_test.go index e7031c138..2e8c7d365 100644 --- a/src/common/yaml/yaml_writer_test.go +++ b/src/common/yaml/yaml_writer_test.go @@ -250,7 +250,7 @@ func TestYaml_SkipResourceByComment(t *testing.T) { expectedResourceNames := []string{"NewVolume"} _ = MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") assert.Equal(t, utils.SkipResourcesByComment, resorseSkip) - assert.NotEqual(t,utils.SkipResourcesByComment, "NewVolume2") + assert.NotEqual(t, utils.SkipResourcesByComment, "NewVolume2") defer resetSkipArr() }) t.Run("All resources with skip comment added to skipResourcesByComment slice", func(t *testing.T) { diff --git a/src/serverless/structure/serverless_parser.go b/src/serverless/structure/serverless_parser.go index 25caeed35..d8cb7537f 100644 --- a/src/serverless/structure/serverless_parser.go +++ b/src/serverless/structure/serverless_parser.go @@ -71,7 +71,7 @@ func (p *ServerlessParser) ValidFile(file string) bool { return true } -func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock , error) { +func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, error) { parsedBlocks := make([]structure.IBlock, 0) fileFormat := utils.GetFileFormat(filePath) fileName := filepath.Base(filePath) diff --git a/src/serverless/structure/serverless_parser_test.go b/src/serverless/structure/serverless_parser_test.go index bda0224fe..562878888 100644 --- a/src/serverless/structure/serverless_parser_test.go +++ b/src/serverless/structure/serverless_parser_test.go @@ -200,7 +200,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { tagGroup.SetTags(extraTags) tagGroup.InitTagGroup("", []string{}, []string{}) writeFilePath := directory + "/serverless_tagged.yml" - slsBlocks , err := slsParser.ParseFile(readFilePath) + slsBlocks, err := slsParser.ParseFile(readFilePath) for _, block := range slsBlocks { err := tagGroup.CreateTagsForBlock(block) if err != nil { diff --git a/src/terraform/structure/terraform_parser.go b/src/terraform/structure/terraform_parser.go index cca8c780d..2cd870b36 100644 --- a/src/terraform/structure/terraform_parser.go +++ b/src/terraform/structure/terraform_parser.go @@ -25,7 +25,6 @@ import ( ) var ignoredDirs = []string{".git", ".DS_Store", ".idea", ".terraform"} -var mutex sync.Mutex var unsupportedTerraformBlocks = []string{ "aws_autoscaling_group", // This resource specifically supports tags with a different structure, see: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/autoscaling_group#tag-and-tags "aws_lb_listener", // This resource does not support tags, although docs state otherwise. @@ -126,7 +125,7 @@ func (p *TerraformParser) ValidFile(_ string) bool { return true } -func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock , error) { +func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, error) { // #nosec G304 // read file bytes src, err := os.ReadFile(filePath) @@ -148,7 +147,7 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock , error } if hclFile == nil || hclSyntaxFile == nil { - return nil , fmt.Errorf("failed to parse hcl file %s", filePath) + return nil, fmt.Errorf("failed to parse hcl file %s", filePath) } syntaxBlocks := hclSyntaxFile.Body.(*hclsyntax.Body).Blocks @@ -191,7 +190,7 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock , error parsedBlocks = append(parsedBlocks, terraformBlock) } - return parsedBlocks , nil + return parsedBlocks, nil } func (p *TerraformParser) WriteFile(readFilePath string, blocks []structure.IBlock, writeFilePath string) error { diff --git a/src/terraform/structure/terraform_parser_test.go b/src/terraform/structure/terraform_parser_test.go index 6743647e0..be7c07040 100644 --- a/src/terraform/structure/terraform_parser_test.go +++ b/src/terraform/structure/terraform_parser_test.go @@ -835,4 +835,4 @@ func compareTokenArrays(got []hclwrite.Tokens, want []hclwrite.Tokens) bool { } func resetSkipArr() { utils.SkipResourcesByComment = []string{} - } +} From 6969b5417d60356012d5f64e5ab0225d0f4328cb Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Thu, 20 Jun 2024 10:12:26 +0300 Subject: [PATCH 15/19] Feature skip resource by comment --- .../structure/cloudformation_parser.go | 9 ++++++- .../structure/cloudformation_parser_test.go | 4 +-- src/common/parser.go | 1 + src/common/runner/runner.go | 7 ++--- src/common/utils/utils.go | 1 - src/common/yaml/yaml_writer.go | 11 ++++---- src/common/yaml/yaml_writer_test.go | 26 +++++++------------ src/serverless/structure/serverless_parser.go | 8 +++++- src/terraform/structure/terraform_parser.go | 6 ++++- .../structure/terraform_parser_test.go | 13 +++------- 10 files changed, 47 insertions(+), 39 deletions(-) diff --git a/src/cloudformation/structure/cloudformation_parser.go b/src/cloudformation/structure/cloudformation_parser.go index 092286aed..f2022563b 100644 --- a/src/cloudformation/structure/cloudformation_parser.go +++ b/src/cloudformation/structure/cloudformation_parser.go @@ -29,6 +29,7 @@ import ( type CloudformationParser struct { *types.YamlParser *types.JSONParser + skippedByCommentList []string } const TagsAttributeName = "Tags" @@ -113,6 +114,7 @@ func goformationParse(file string) (*cloudformation.Template, error) { } func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, error) { + skipResourcesByComment := make([]string, 0) goformationLock.Lock() template, err := goformationParse(filePath) goformationLock.Unlock() @@ -138,7 +140,8 @@ func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, e var resourceNamesToLines map[string]*structure.Lines switch utils.GetFileFormat(filePath) { case common.YmlFileType.FileFormat, common.YamlFileType.FileFormat: - resourceNamesToLines = yaml.MapResourcesLineYAML(filePath, resourceNames, ResourcesStartToken) + resourceNamesToLines,skipResourcesByComment = yaml.MapResourcesLineYAML(filePath, resourceNames, ResourcesStartToken) + p.skippedByCommentList = append(p.skippedByCommentList, skipResourcesByComment...) case common.JSONFileType.FileFormat: var fileBracketsMapping map[int]json.BracketPair resourceNamesToLines, fileBracketsMapping = json.MapResourcesLineJSON(filePath, resourceNames) @@ -293,3 +296,7 @@ func (p *CloudformationParser) getTagsLines(filePath string, resourceLinesRange return structure.Lines{Start: -1, End: -1} } } +func (p *CloudformationParser) GetSkipResourcesByComment() []string { + return p.skippedByCommentList +} + diff --git a/src/cloudformation/structure/cloudformation_parser_test.go b/src/cloudformation/structure/cloudformation_parser_test.go index 85324973e..2cade4425 100644 --- a/src/cloudformation/structure/cloudformation_parser_test.go +++ b/src/cloudformation/structure/cloudformation_parser_test.go @@ -123,7 +123,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { expected := map[string]*structure.Lines{ "NewVolume": {Start: 3, End: 15}, } - actual := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) + actual,_:= yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) compareLines(t, expected, actual) }) @@ -136,7 +136,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { "EC2LaunchTemplateResource0": {Start: 18, End: 23}, "EC2LaunchTemplateResource1": {Start: 24, End: 34}, } - actual := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) + actual,_ := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) compareLines(t, expected, actual) }) diff --git a/src/common/parser.go b/src/common/parser.go index b4982e296..74abec6e2 100644 --- a/src/common/parser.go +++ b/src/common/parser.go @@ -10,5 +10,6 @@ type IParser interface { WriteFile(readFilePath string, blocks []structure.IBlock, writeFilePath string) error GetSkippedDirs() []string GetSupportedFileExtensions() []string + GetSkipResourcesByComment() []string Close() } diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index 17c2d28f7..0fb049080 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -160,13 +160,13 @@ func (r *Runner) isSkippedResourceType(resourceType string) bool { return false } -func (r *Runner) isSkippedResource(resource string) bool { +func (r *Runner) isSkippedResource(resource string,skipResource [] string) bool { for _, skippedResource := range r.skippedResources { if resource == skippedResource { return true } } - for _, skippedResource := range utils.SkipResourcesByComment { + for _, skippedResource := range skipResource { if resource == skippedResource { return true } @@ -192,7 +192,8 @@ func (r *Runner) TagFile(file string) { if r.isSkippedResourceType(block.GetResourceType()) { continue } - if r.isSkippedResource(block.GetResourceID()) { + skipResourcesByComment := parser.GetSkipResourcesByComment() + if r.isSkippedResource(block.GetResourceID(),skipResourcesByComment) { continue } if block.IsBlockTaggable() { diff --git a/src/common/utils/utils.go b/src/common/utils/utils.go index 4cc3cc624..81002a1be 100644 --- a/src/common/utils/utils.go +++ b/src/common/utils/utils.go @@ -17,7 +17,6 @@ import ( // RemoveGcpInvalidChars Source of regex: https://cloud.google.com/compute/docs/labeling-resources var RemoveGcpInvalidChars = regexp.MustCompile(`[^\p{Ll}\p{Lo}\p{N}_-]`) -var SkipResourcesByComment = make([]string, 0) func InSlice[T comparable](elems []T, v T) bool { for _, s := range elems { diff --git a/src/common/yaml/yaml_writer.go b/src/common/yaml/yaml_writer.go index b3005b3c3..396a0f5b9 100644 --- a/src/common/yaml/yaml_writer.go +++ b/src/common/yaml/yaml_writer.go @@ -266,8 +266,9 @@ func FindTagsLinesYAML(textLines []string, tagsAttributeName string) (structure. return tagsLines, tagsExist } -func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStartToken string) map[string]*structure.Lines { +func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStartToken string) (map[string]*structure.Lines ,[]string ){ resourceToLines := make(map[string]*structure.Lines) + skipResourcesByComment := make([]string, 0) for _, resourceName := range resourceNames { // initialize a map between resource name and its lines in file resourceToLines[resourceName] = &structure.Lines{Start: -1, End: -1} @@ -276,7 +277,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar file, err := os.ReadFile(filePath) if err != nil { logger.Warning(fmt.Sprintf("failed to read file %s", filePath)) - return nil + return nil,skipResourcesByComment } readResources := false @@ -288,7 +289,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar cleanContent := strings.TrimSpace(line) if strings.HasPrefix(cleanContent, resourcesStartToken+":") { if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIPALL" { - utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, resourceNames...) + skipResourcesByComment = append(skipResourcesByComment, resourceNames...) } readResources = true resourcesIndent = countLeadingSpaces(line) @@ -299,7 +300,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar if i > 0 { if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIP" { - utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, strings.Trim(strings.TrimSpace(line), ":")) + skipResourcesByComment = append(skipResourcesByComment, strings.Trim(strings.TrimSpace(line), ":")) } } @@ -335,7 +336,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar // Handle last line of resource is last line of file resourceToLines[latestResourceName].End = findLastNonEmptyLine(fileLines, len(fileLines)-1) } - return resourceToLines + return resourceToLines,skipResourcesByComment } func countLeadingSpaces(line string) int { diff --git a/src/common/yaml/yaml_writer_test.go b/src/common/yaml/yaml_writer_test.go index 2e8c7d365..25a6f2760 100644 --- a/src/common/yaml/yaml_writer_test.go +++ b/src/common/yaml/yaml_writer_test.go @@ -11,7 +11,6 @@ import ( "github.com/bridgecrewio/yor/src/common/structure" "github.com/bridgecrewio/yor/src/common/tagging/simple" "github.com/bridgecrewio/yor/src/common/tagging/tags" - "github.com/bridgecrewio/yor/src/common/utils" "github.com/stretchr/testify/assert" ) @@ -229,13 +228,13 @@ func TestTagReplacement(t *testing.T) { }) t.Run("Test line computation with duplicate - CFN", func(t *testing.T) { - res := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_cfn.yaml", []string{"S3Bucket", "CloudFrontDistribution"}, "Resources") + res,_ := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_cfn.yaml", []string{"S3Bucket", "CloudFrontDistribution"}, "Resources") assert.Equal(t, *res["S3Bucket"], structure.Lines{Start: 14, End: 17}) assert.Equal(t, *res["CloudFrontDistribution"], structure.Lines{Start: 18, End: 60}) }) t.Run("Test line computation with duplicate - SLS", func(t *testing.T) { - res := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_sls.yaml", []string{"attribute", "zone", "customer", "apiVersion"}, "functions") + res,_ := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_sls.yaml", []string{"attribute", "zone", "customer", "apiVersion"}, "functions") assert.Equal(t, *res["apiVersion"], structure.Lines{Start: 7, End: 12}) assert.Equal(t, *res["customer"], structure.Lines{Start: 14, End: 24}) assert.Equal(t, *res["zone"], structure.Lines{Start: 26, End: 38}) @@ -248,27 +247,22 @@ func TestYaml_SkipResourceByComment(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/skipOne.yaml" resorseSkip := []string{"NewVolume"} expectedResourceNames := []string{"NewVolume"} - _ = MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") - assert.Equal(t, utils.SkipResourcesByComment, resorseSkip) - assert.NotEqual(t, utils.SkipResourcesByComment, "NewVolume2") - defer resetSkipArr() + _,skipResourceByComment:= MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Equal(t, skipResourceByComment, resorseSkip) + assert.NotEqual(t,skipResourceByComment, "NewVolume2") }) t.Run("All resources with skip comment added to skipResourcesByComment slice", func(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/skipAll.yaml" resorseSkip := []string{"NewVolume", "NewVolume2"} expectedResourceNames := []string{"NewVolume", "NewVolume2"} - _ = MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") - assert.Equal(t, utils.SkipResourcesByComment, resorseSkip) - defer resetSkipArr() + _,skipResourceByComment := MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Equal(t,skipResourceByComment, resorseSkip) }) t.Run("No resources with skip all comment in the file, skipResourcesByComment slice should be empty", func(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/noSkip.yaml" expectedResourceNames := []string{"NewVolume"} - _ = MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") - assert.Empty(t, utils.SkipResourcesByComment) - defer resetSkipArr() + _ ,skipResourceByComment:= MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Empty(t, skipResourceByComment) }) } -func resetSkipArr() { - utils.SkipResourcesByComment = []string{} -} + diff --git a/src/serverless/structure/serverless_parser.go b/src/serverless/structure/serverless_parser.go index d8cb7537f..8032b7014 100644 --- a/src/serverless/structure/serverless_parser.go +++ b/src/serverless/structure/serverless_parser.go @@ -25,6 +25,7 @@ const FunctionType = "function" type ServerlessParser struct { YamlParser types.YamlParser + skippedByCommentList []string } var slsParseLock sync.Mutex @@ -72,6 +73,7 @@ func (p *ServerlessParser) ValidFile(file string) bool { } func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, error) { + skipResourcesByComment := make([]string, 0) parsedBlocks := make([]structure.IBlock, 0) fileFormat := utils.GetFileFormat(filePath) fileName := filepath.Base(filePath) @@ -101,7 +103,8 @@ func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, error } switch utils.GetFileFormat(filePath) { case common.YmlFileType.FileFormat, common.YamlFileType.FileFormat: - resourceNamesToLines = yamlUtils.MapResourcesLineYAML(filePath, resourceNames, FunctionsSectionName) + resourceNamesToLines,skipResourcesByComment= yamlUtils.MapResourcesLineYAML(filePath, resourceNames, FunctionsSectionName) + p.skippedByCommentList = append(p.skippedByCommentList, skipResourcesByComment...) default: return nil, fmt.Errorf("unsupported file type %s", utils.GetFileFormat(filePath)) } @@ -143,6 +146,9 @@ func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, error } return parsedBlocks, nil } +func (p *ServerlessParser) GetSkipResourcesByComment() []string { + return p.skippedByCommentList +} func (p *ServerlessParser) WriteFile(readFilePath string, blocks []structure.IBlock, writeFilePath string) error { for _, block := range blocks { diff --git a/src/terraform/structure/terraform_parser.go b/src/terraform/structure/terraform_parser.go index 2cd870b36..64e014817 100644 --- a/src/terraform/structure/terraform_parser.go +++ b/src/terraform/structure/terraform_parser.go @@ -48,6 +48,7 @@ type TerraformParser struct { moduleInstallDir string downloadedPaths []string tfClientLock sync.Mutex + skippedByCommentList []string } func (p *TerraformParser) Name() string { @@ -184,7 +185,7 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, error) } if strings.ToUpper(strings.TrimSpace(lineAbove)) == "#YOR:SKIP" || skipAll { - utils.SkipResourcesByComment = append(utils.SkipResourcesByComment, terraformBlock.GetResourceID()) + p.skippedByCommentList = append(p.skippedByCommentList, terraformBlock.GetResourceID()) } } parsedBlocks = append(parsedBlocks, terraformBlock) @@ -192,6 +193,9 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, error) return parsedBlocks, nil } +func (p *TerraformParser) GetSkipResourcesByComment() []string { + return p.skippedByCommentList +} func (p *TerraformParser) WriteFile(readFilePath string, blocks []structure.IBlock, writeFilePath string) error { // #nosec G304 diff --git a/src/terraform/structure/terraform_parser_test.go b/src/terraform/structure/terraform_parser_test.go index be7c07040..64d152e33 100644 --- a/src/terraform/structure/terraform_parser_test.go +++ b/src/terraform/structure/terraform_parser_test.go @@ -34,8 +34,7 @@ func TestTerraformParser_SkipResourceByComment(t *testing.T) { t.Errorf("failed to read hcl file because %s", err) } exceptedSkipResources := []string{"aws_vpc.example_vpc", "aws_subnet.example_subnet", "aws_instance.example_instance"} - assert.Equal(t, exceptedSkipResources, utils.SkipResourcesByComment) - defer resetSkipArr() + assert.Equal(t, exceptedSkipResources, p.GetSkipResourcesByComment()) }) t.Run("No resources with skip comment in the file, skipResourcesByComment slice should be empty", func(t *testing.T) { @@ -48,8 +47,7 @@ func TestTerraformParser_SkipResourceByComment(t *testing.T) { if err != nil { t.Errorf("failed to read hcl file because %s", err) } - assert.Empty(t, utils.SkipResourcesByComment) - defer resetSkipArr() + assert.Empty(t, p.GetSkipResourcesByComment()) }) t.Run("One resource with skip comment, only that resource added to skipResourcesByComment slice", func(t *testing.T) { @@ -63,8 +61,7 @@ func TestTerraformParser_SkipResourceByComment(t *testing.T) { t.Errorf("failed to read hcl file because %s", err) } exceptedSkipResources := []string{"aws_instance.example_instance"} - assert.Equal(t, exceptedSkipResources, utils.SkipResourcesByComment) - defer resetSkipArr() + assert.Equal(t, exceptedSkipResources, p.GetSkipResourcesByComment()) }) } @@ -833,6 +830,4 @@ func compareTokenArrays(got []hclwrite.Tokens, want []hclwrite.Tokens) bool { return true } -func resetSkipArr() { - utils.SkipResourcesByComment = []string{} -} + From b4e8d93b8b98d29ef90f5605a5997a6a3e7b8f41 Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Sun, 23 Jun 2024 02:35:02 +0300 Subject: [PATCH 16/19] Fix the golint --- .../structure/cloudformation_parser.go | 3 +-- .../structure/cloudformation_parser_test.go | 4 ++-- src/common/runner/runner.go | 4 ++-- src/common/yaml/yaml_writer.go | 8 ++++---- src/common/yaml/yaml_writer_test.go | 15 +++++++-------- src/serverless/structure/serverless_parser.go | 4 ++-- src/terraform/structure/terraform_parser_test.go | 1 - 7 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/cloudformation/structure/cloudformation_parser.go b/src/cloudformation/structure/cloudformation_parser.go index f2022563b..37b89bb51 100644 --- a/src/cloudformation/structure/cloudformation_parser.go +++ b/src/cloudformation/structure/cloudformation_parser.go @@ -140,7 +140,7 @@ func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, e var resourceNamesToLines map[string]*structure.Lines switch utils.GetFileFormat(filePath) { case common.YmlFileType.FileFormat, common.YamlFileType.FileFormat: - resourceNamesToLines,skipResourcesByComment = yaml.MapResourcesLineYAML(filePath, resourceNames, ResourcesStartToken) + resourceNamesToLines, skipResourcesByComment = yaml.MapResourcesLineYAML(filePath, resourceNames, ResourcesStartToken) p.skippedByCommentList = append(p.skippedByCommentList, skipResourcesByComment...) case common.JSONFileType.FileFormat: var fileBracketsMapping map[int]json.BracketPair @@ -299,4 +299,3 @@ func (p *CloudformationParser) getTagsLines(filePath string, resourceLinesRange func (p *CloudformationParser) GetSkipResourcesByComment() []string { return p.skippedByCommentList } - diff --git a/src/cloudformation/structure/cloudformation_parser_test.go b/src/cloudformation/structure/cloudformation_parser_test.go index 2cade4425..435eb7392 100644 --- a/src/cloudformation/structure/cloudformation_parser_test.go +++ b/src/cloudformation/structure/cloudformation_parser_test.go @@ -123,7 +123,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { expected := map[string]*structure.Lines{ "NewVolume": {Start: 3, End: 15}, } - actual,_:= yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) + actual, _ := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) compareLines(t, expected, actual) }) @@ -136,7 +136,7 @@ func Test_mapResourcesLineYAML(t *testing.T) { "EC2LaunchTemplateResource0": {Start: 18, End: 23}, "EC2LaunchTemplateResource1": {Start: 24, End: 34}, } - actual,_ := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) + actual, _ := yaml.MapResourcesLineYAML(filePath, resourcesNames, ResourcesStartToken) compareLines(t, expected, actual) }) diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index 0fb049080..92611b0f0 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -160,7 +160,7 @@ func (r *Runner) isSkippedResourceType(resourceType string) bool { return false } -func (r *Runner) isSkippedResource(resource string,skipResource [] string) bool { +func (r *Runner) isSkippedResource(resource string, skipResource []string) bool { for _, skippedResource := range r.skippedResources { if resource == skippedResource { return true @@ -193,7 +193,7 @@ func (r *Runner) TagFile(file string) { continue } skipResourcesByComment := parser.GetSkipResourcesByComment() - if r.isSkippedResource(block.GetResourceID(),skipResourcesByComment) { + if r.isSkippedResource(block.GetResourceID(), skipResourcesByComment) { continue } if block.IsBlockTaggable() { diff --git a/src/common/yaml/yaml_writer.go b/src/common/yaml/yaml_writer.go index 396a0f5b9..17fee1db6 100644 --- a/src/common/yaml/yaml_writer.go +++ b/src/common/yaml/yaml_writer.go @@ -266,7 +266,7 @@ func FindTagsLinesYAML(textLines []string, tagsAttributeName string) (structure. return tagsLines, tagsExist } -func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStartToken string) (map[string]*structure.Lines ,[]string ){ +func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStartToken string) (map[string]*structure.Lines, []string) { resourceToLines := make(map[string]*structure.Lines) skipResourcesByComment := make([]string, 0) for _, resourceName := range resourceNames { @@ -277,7 +277,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar file, err := os.ReadFile(filePath) if err != nil { logger.Warning(fmt.Sprintf("failed to read file %s", filePath)) - return nil,skipResourcesByComment + return nil, skipResourcesByComment } readResources := false @@ -289,7 +289,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar cleanContent := strings.TrimSpace(line) if strings.HasPrefix(cleanContent, resourcesStartToken+":") { if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIPALL" { - skipResourcesByComment = append(skipResourcesByComment, resourceNames...) + skipResourcesByComment = append(skipResourcesByComment, resourceNames...) } readResources = true resourcesIndent = countLeadingSpaces(line) @@ -336,7 +336,7 @@ func MapResourcesLineYAML(filePath string, resourceNames []string, resourcesStar // Handle last line of resource is last line of file resourceToLines[latestResourceName].End = findLastNonEmptyLine(fileLines, len(fileLines)-1) } - return resourceToLines,skipResourcesByComment + return resourceToLines, skipResourcesByComment } func countLeadingSpaces(line string) int { diff --git a/src/common/yaml/yaml_writer_test.go b/src/common/yaml/yaml_writer_test.go index 25a6f2760..97cf5d53e 100644 --- a/src/common/yaml/yaml_writer_test.go +++ b/src/common/yaml/yaml_writer_test.go @@ -228,13 +228,13 @@ func TestTagReplacement(t *testing.T) { }) t.Run("Test line computation with duplicate - CFN", func(t *testing.T) { - res,_ := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_cfn.yaml", []string{"S3Bucket", "CloudFrontDistribution"}, "Resources") + res, _ := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_cfn.yaml", []string{"S3Bucket", "CloudFrontDistribution"}, "Resources") assert.Equal(t, *res["S3Bucket"], structure.Lines{Start: 14, End: 17}) assert.Equal(t, *res["CloudFrontDistribution"], structure.Lines{Start: 18, End: 60}) }) t.Run("Test line computation with duplicate - SLS", func(t *testing.T) { - res,_ := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_sls.yaml", []string{"attribute", "zone", "customer", "apiVersion"}, "functions") + res, _ := MapResourcesLineYAML("../../../tests/cloudformation/resources/duplicate_entries/duplicate_sls.yaml", []string{"attribute", "zone", "customer", "apiVersion"}, "functions") assert.Equal(t, *res["apiVersion"], structure.Lines{Start: 7, End: 12}) assert.Equal(t, *res["customer"], structure.Lines{Start: 14, End: 24}) assert.Equal(t, *res["zone"], structure.Lines{Start: 26, End: 38}) @@ -247,22 +247,21 @@ func TestYaml_SkipResourceByComment(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/skipOne.yaml" resorseSkip := []string{"NewVolume"} expectedResourceNames := []string{"NewVolume"} - _,skipResourceByComment:= MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + _, skipResourceByComment := MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") assert.Equal(t, skipResourceByComment, resorseSkip) - assert.NotEqual(t,skipResourceByComment, "NewVolume2") + assert.NotEqual(t, skipResourceByComment, "NewVolume2") }) t.Run("All resources with skip comment added to skipResourcesByComment slice", func(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/skipAll.yaml" resorseSkip := []string{"NewVolume", "NewVolume2"} expectedResourceNames := []string{"NewVolume", "NewVolume2"} - _,skipResourceByComment := MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") - assert.Equal(t,skipResourceByComment, resorseSkip) + _, skipResourceByComment := MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + assert.Equal(t, skipResourceByComment, resorseSkip) }) t.Run("No resources with skip all comment in the file, skipResourcesByComment slice should be empty", func(t *testing.T) { filePath := "../../../tests/cloudformation/resources/skipComment/noSkip.yaml" expectedResourceNames := []string{"NewVolume"} - _ ,skipResourceByComment:= MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") + _, skipResourceByComment := MapResourcesLineYAML(filePath, expectedResourceNames, "Resources") assert.Empty(t, skipResourceByComment) }) } - diff --git a/src/serverless/structure/serverless_parser.go b/src/serverless/structure/serverless_parser.go index 8032b7014..b967995bd 100644 --- a/src/serverless/structure/serverless_parser.go +++ b/src/serverless/structure/serverless_parser.go @@ -24,7 +24,7 @@ const FunctionsSectionName = "functions" const FunctionType = "function" type ServerlessParser struct { - YamlParser types.YamlParser + YamlParser types.YamlParser skippedByCommentList []string } @@ -103,7 +103,7 @@ func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, error } switch utils.GetFileFormat(filePath) { case common.YmlFileType.FileFormat, common.YamlFileType.FileFormat: - resourceNamesToLines,skipResourcesByComment= yamlUtils.MapResourcesLineYAML(filePath, resourceNames, FunctionsSectionName) + resourceNamesToLines, skipResourcesByComment = yamlUtils.MapResourcesLineYAML(filePath, resourceNames, FunctionsSectionName) p.skippedByCommentList = append(p.skippedByCommentList, skipResourcesByComment...) default: return nil, fmt.Errorf("unsupported file type %s", utils.GetFileFormat(filePath)) diff --git a/src/terraform/structure/terraform_parser_test.go b/src/terraform/structure/terraform_parser_test.go index 64d152e33..4c1fcf754 100644 --- a/src/terraform/structure/terraform_parser_test.go +++ b/src/terraform/structure/terraform_parser_test.go @@ -830,4 +830,3 @@ func compareTokenArrays(got []hclwrite.Tokens, want []hclwrite.Tokens) bool { return true } - From ccffeaf5c875c4e7fc5ebf24745abc04893e54f4 Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Sun, 30 Jun 2024 10:22:18 +0300 Subject: [PATCH 17/19] Fix golint --- src/cloudformation/structure/cloudformation_parser.go | 2 +- src/serverless/structure/serverless_parser.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cloudformation/structure/cloudformation_parser.go b/src/cloudformation/structure/cloudformation_parser.go index 37b89bb51..e3b337ef8 100644 --- a/src/cloudformation/structure/cloudformation_parser.go +++ b/src/cloudformation/structure/cloudformation_parser.go @@ -114,7 +114,7 @@ func goformationParse(file string) (*cloudformation.Template, error) { } func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, error) { - skipResourcesByComment := make([]string, 0) + var skipResourcesByComment []string goformationLock.Lock() template, err := goformationParse(filePath) goformationLock.Unlock() diff --git a/src/serverless/structure/serverless_parser.go b/src/serverless/structure/serverless_parser.go index b967995bd..1006128b7 100644 --- a/src/serverless/structure/serverless_parser.go +++ b/src/serverless/structure/serverless_parser.go @@ -73,7 +73,7 @@ func (p *ServerlessParser) ValidFile(file string) bool { } func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, error) { - skipResourcesByComment := make([]string, 0) + var skipResourcesByComment []string parsedBlocks := make([]structure.IBlock, 0) fileFormat := utils.GetFileFormat(filePath) fileName := filepath.Base(filePath) From 0a0216fbef72ebfe1545f7072a6aa7a77c39f660 Mon Sep 17 00:00:00 2001 From: Chana_Movshowich Date: Sun, 30 Jun 2024 11:12:54 +0300 Subject: [PATCH 18/19] Fix security --- src/terraform/structure/terraform_parser.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/terraform/structure/terraform_parser.go b/src/terraform/structure/terraform_parser.go index 64e014817..74072ac3c 100644 --- a/src/terraform/structure/terraform_parser.go +++ b/src/terraform/structure/terraform_parser.go @@ -360,7 +360,7 @@ func (p *TerraformParser) modifyBlockTags(rawBlock *hclwrite.Block, parsedBlock // => we should replace it! rawTagsTokens = newTagsTokens // checkov:skip=CKV_SECRET_6 false positive } else { - rawTagsTokens = InsertTokens(rawTagsTokens, newTagsTokens[2:len(newTagsTokens)-2]) + rawTagsTokens = InsertTokens(rawTagsTokens, newTagsTokens[2:len(newTagsTokens)-2]) // checkov:skip=CKV_SECRET_6 false positive } rawBlock.Body().SetAttributeRaw(tagsAttributeName, rawTagsTokens) return From d52654f240ec15ba2f5f9b89f9c94aa9c38b6572 Mon Sep 17 00:00:00 2001 From: ChanochShayner <57212002+ChanochShayner@users.noreply.github.com> Date: Sun, 30 Jun 2024 11:21:12 +0300 Subject: [PATCH 19/19] Update src/terraform/structure/terraform_parser.go --- src/terraform/structure/terraform_parser.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/terraform/structure/terraform_parser.go b/src/terraform/structure/terraform_parser.go index 74072ac3c..1700b7487 100644 --- a/src/terraform/structure/terraform_parser.go +++ b/src/terraform/structure/terraform_parser.go @@ -360,7 +360,7 @@ func (p *TerraformParser) modifyBlockTags(rawBlock *hclwrite.Block, parsedBlock // => we should replace it! rawTagsTokens = newTagsTokens // checkov:skip=CKV_SECRET_6 false positive } else { - rawTagsTokens = InsertTokens(rawTagsTokens, newTagsTokens[2:len(newTagsTokens)-2]) // checkov:skip=CKV_SECRET_6 false positive + rawTagsTokens = InsertTokens(rawTagsTokens, newTagsTokens[2:len(newTagsTokens)-2]) // checkov:skip=CKV_SECRET_80 false positive } rawBlock.Body().SetAttributeRaw(tagsAttributeName, rawTagsTokens) return