From 5a6df5b5c6ce552cc1606591027a2fd128a21b19 Mon Sep 17 00:00:00 2001 From: cshayner Date: Mon, 10 Jun 2024 21:17:22 +0300 Subject: [PATCH 1/5] test --- .../structure/cloudformation_parser.go | 17 ++++----- .../structure/cloudformation_parser_test.go | 12 +++---- src/common/parser.go | 2 +- src/common/runner/runner.go | 10 +++++- src/common/yaml/yaml_writer.go | 16 +++++++-- src/serverless/structure/serverless_parser.go | 17 ++++----- src/terraform/structure/terraform_parser.go | 30 +++++++++++----- tests/integration/integration_test.go | 35 +++++++++++++++++-- tests/integration/skipComment/skipResource.tf | 24 +++++++++++++ .../integration/skipComment/skipResource.yaml | 25 +++++++++++++ 10 files changed, 150 insertions(+), 38 deletions(-) create mode 100644 tests/integration/skipComment/skipResource.tf create mode 100644 tests/integration/skipComment/skipResource.yaml diff --git a/src/cloudformation/structure/cloudformation_parser.go b/src/cloudformation/structure/cloudformation_parser.go index 092286aed..9db4694eb 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) { @@ -237,7 +238,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..fd18b8529 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 @@ -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 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 e86937032..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) @@ -176,11 +178,17 @@ 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 } + mutex.Lock() + if r.skippedResources == nil { + r.skippedResources = []string{} + } + r.skippedResources = append(r.skippedResources, skipResourcesByComment...) + mutex.Unlock() isFileTaggable := false for _, block := range blocks { if r.isSkippedResourceType(block.GetResourceType()) { diff --git a/src/common/yaml/yaml_writer.go b/src/common/yaml/yaml_writer.go index eac890360..ab103814a 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 @@ -287,12 +288,21 @@ 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" { + skipResourcesByComment = append(skipResourcesByComment, resourceNames...) + } readResources = true resourcesIndent = countLeadingSpaces(line) continue } if readResources { + if i > 0 { + if strings.ToUpper(strings.TrimSpace(fileLines[i-1])) == "#YOR:SKIP" { + skipResourcesByComment = append(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 @@ -325,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 + return resourceToLines, skipResourcesByComment } func countLeadingSpaces(line string) int { diff --git a/src/serverless/structure/serverless_parser.go b/src/serverless/structure/serverless_parser.go index d8cb7537f..9cdbd7474 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, nil, 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/terraform/structure/terraform_parser.go b/src/terraform/structure/terraform_parser.go index e5f18dde7..def66348c 100644 --- a/src/terraform/structure/terraform_parser.go +++ b/src/terraform/structure/terraform_parser.go @@ -125,32 +125,35 @@ 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") // parse the file into hclwrite.File and hclsyntax.File to allow getting existing tags and lines 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 + skipAll := false rawBlocks := hclFile.Body().Blocks() parsedBlocks := make([]structure.IBlock, 0) for i, block := range rawBlocks { @@ -174,10 +177,21 @@ 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 { + 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 { @@ -227,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) } @@ -599,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/tests/integration/integration_test.go b/tests/integration/integration_test.go index 935ee04c1..1863f4b28 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -256,6 +256,35 @@ 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{ + 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) { t.Run("Test tagging twice no result second time", func(t *testing.T) { terragoatPath := utils.CloneRepo(utils.TerragoatURL, "063dc2db3bb036160ed39d3705508ee8293a27c8") @@ -275,7 +304,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" { @@ -341,7 +370,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" { @@ -392,7 +421,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"] 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..497c38fca --- /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 From 472c3ef8bacd27a8323f9edafdae25b0d4583fb7 Mon Sep 17 00:00:00 2001 From: cshayner Date: Mon, 10 Jun 2024 21:27:02 +0300 Subject: [PATCH 2/5] add skipResourcesMutex --- src/common/runner/runner.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index 6ad6c2e60..419b35a1e 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -43,7 +43,7 @@ type Runner struct { const WorkersNumEnvKey = "YOR_WORKER_NUM" -var mutex sync.Mutex +var skipResourcesMutex sync.Mutex func (r *Runner) Init(commands *clioptions.TagOptions) error { dir := commands.Directory @@ -163,11 +163,13 @@ func (r *Runner) isSkippedResourceType(resourceType string) bool { } func (r *Runner) isSkippedResource(resource string) bool { + skipResourcesMutex.Lock() for _, skippedResource := range r.skippedResources { if resource == skippedResource { return true } } + skipResourcesMutex.Unlock() return false } @@ -183,12 +185,12 @@ 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() + skipResourcesMutex.Lock() if r.skippedResources == nil { r.skippedResources = []string{} } r.skippedResources = append(r.skippedResources, skipResourcesByComment...) - mutex.Unlock() + skipResourcesMutex.Unlock() isFileTaggable := false for _, block := range blocks { if r.isSkippedResourceType(block.GetResourceType()) { From 7a1d9309a5da5011adc0b9310c3885dead755bc9 Mon Sep 17 00:00:00 2001 From: cshayner Date: Mon, 10 Jun 2024 21:49:35 +0300 Subject: [PATCH 3/5] add skipResourcesMutex --- src/common/runner/runner.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index 419b35a1e..a0f967d2d 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -44,6 +44,7 @@ type Runner struct { const WorkersNumEnvKey = "YOR_WORKER_NUM" var skipResourcesMutex sync.Mutex +var skipResourcesCheckMutex sync.Mutex func (r *Runner) Init(commands *clioptions.TagOptions) error { dir := commands.Directory @@ -162,14 +163,17 @@ func (r *Runner) isSkippedResourceType(resourceType string) bool { return false } -func (r *Runner) isSkippedResource(resource string) bool { - skipResourcesMutex.Lock() +func (r *Runner) isSkippedResource(resource string, skipResourcesByComment []string) bool { for _, skippedResource := range r.skippedResources { if resource == skippedResource { return true } } - skipResourcesMutex.Unlock() + for _, skippedResource := range skipResourcesByComment { + if resource == skippedResource { + return true + } + } return false } @@ -185,18 +189,18 @@ func (r *Runner) TagFile(file string) { logger.Info(fmt.Sprintf("Failed to parse file %v with parser %v", file, reflect.TypeOf(parser))) continue } - skipResourcesMutex.Lock() - if r.skippedResources == nil { - r.skippedResources = []string{} - } - r.skippedResources = append(r.skippedResources, skipResourcesByComment...) - skipResourcesMutex.Unlock() + //skipResourcesMutex.Lock() + //if r.skippedResources == nil { + // r.skippedResources = []string{} + //} + //r.skippedResources = append(r.skippedResources, skipResourcesByComment...) + //skipResourcesMutex.Unlock() isFileTaggable := false for _, block := range blocks { if r.isSkippedResourceType(block.GetResourceType()) { continue } - if r.isSkippedResource(block.GetResourceID()) { + if r.isSkippedResource(block.GetResourceID(), skipResourcesByComment) { continue } if block.IsBlockTaggable() { From 204d0c75622a680d917b005e94329695cab43a1e Mon Sep 17 00:00:00 2001 From: cshayner Date: Mon, 10 Jun 2024 21:55:53 +0300 Subject: [PATCH 4/5] add skipResourcesMutex --- src/common/runner/runner.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index a0f967d2d..6b8d84000 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -43,9 +43,6 @@ type Runner struct { const WorkersNumEnvKey = "YOR_WORKER_NUM" -var skipResourcesMutex sync.Mutex -var skipResourcesCheckMutex sync.Mutex - func (r *Runner) Init(commands *clioptions.TagOptions) error { dir := commands.Directory extraTags, extraTagGroups, err := loadExternalResources(commands.CustomTagging) @@ -189,12 +186,6 @@ func (r *Runner) TagFile(file string) { logger.Info(fmt.Sprintf("Failed to parse file %v with parser %v", file, reflect.TypeOf(parser))) continue } - //skipResourcesMutex.Lock() - //if r.skippedResources == nil { - // r.skippedResources = []string{} - //} - //r.skippedResources = append(r.skippedResources, skipResourcesByComment...) - //skipResourcesMutex.Unlock() isFileTaggable := false for _, block := range blocks { if r.isSkippedResourceType(block.GetResourceType()) { From 28395ff36f2c13a31f31e2480e5c7d996949c150 Mon Sep 17 00:00:00 2001 From: cshayner Date: Tue, 18 Jun 2024 18:10:53 +0300 Subject: [PATCH 5/5] add skipResourcesMutex --- .../structure/cloudformation_parser.go | 20 ++++++++++------ src/common/parser.go | 3 ++- src/common/runner/runner.go | 3 ++- src/serverless/structure/serverless_parser.go | 22 ++++++++++------- src/terraform/structure/terraform_parser.go | 24 +++++++++++-------- tests/integration/integration_test.go | 6 ++--- 6 files changed, 48 insertions(+), 30 deletions(-) diff --git a/src/cloudformation/structure/cloudformation_parser.go b/src/cloudformation/structure/cloudformation_parser.go index 9db4694eb..c20452a83 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" @@ -61,6 +62,10 @@ func (p *CloudformationParser) GetSupportedFileExtensions() []string { return []string{common.YamlFileType.Extension, common.YmlFileType.Extension, common.CFTFileType.Extension, common.JSONFileType.Extension} } +func (p *CloudformationParser) GetSkipResourcesByComment() []string { + return p.skippedByCommentList +} + // ValidFile Validate file has AWSTemplateFormatVersion func (p *CloudformationParser) ValidFile(filePath string) bool { // #nosec G304 @@ -112,7 +117,7 @@ func goformationParse(file string) (*cloudformation.Template, error) { return template, err } -func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, []string, error) { +func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, error) { skipResourcesByComment := make([]string, 0) goformationLock.Lock() template, err := goformationParse(filePath) @@ -122,12 +127,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) @@ -140,12 +145,13 @@ func (p *CloudformationParser) ParseFile(filePath string) ([]structure.IBlock, [ switch utils.GetFileFormat(filePath) { case common.YmlFileType.FileFormat, common.YamlFileType.FileFormat: 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) 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 +188,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 +244,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/common/parser.go b/src/common/parser.go index 7102263ad..74abec6e2 100644 --- a/src/common/parser.go +++ b/src/common/parser.go @@ -6,9 +6,10 @@ 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 + GetSkipResourcesByComment() []string Close() } diff --git a/src/common/runner/runner.go b/src/common/runner/runner.go index 6b8d84000..6d89b7385 100644 --- a/src/common/runner/runner.go +++ b/src/common/runner/runner.go @@ -181,7 +181,7 @@ 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 @@ -191,6 +191,7 @@ func (r *Runner) TagFile(file string) { if r.isSkippedResourceType(block.GetResourceType()) { continue } + skipResourcesByComment := parser.GetSkipResourcesByComment() if r.isSkippedResource(block.GetResourceID(), skipResourcesByComment) { continue } diff --git a/src/serverless/structure/serverless_parser.go b/src/serverless/structure/serverless_parser.go index 9cdbd7474..221c8db72 100644 --- a/src/serverless/structure/serverless_parser.go +++ b/src/serverless/structure/serverless_parser.go @@ -24,7 +24,8 @@ const FunctionsSectionName = "functions" const FunctionType = "function" type ServerlessParser struct { - YamlParser types.YamlParser + YamlParser types.YamlParser + skippedByCommentList []string } var slsParseLock sync.Mutex @@ -71,13 +72,17 @@ func (p *ServerlessParser) ValidFile(file string) bool { return true } -func (p *ServerlessParser) ParseFile(filePath string) ([]structure.IBlock, []string, error) { +func (p *ServerlessParser) GetSkipResourcesByComment() []string { + return p.skippedByCommentList +} + +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) if !(fileName == fmt.Sprintf("serverless.%s", fileFormat) || fileName == fmt.Sprintf("config.%s", fileFormat)) { - return nil, nil, nil + return nil, nil } // #nosec G304 - file is from user template, err := serverlessParse(filePath) @@ -88,10 +93,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 @@ -103,8 +108,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) + p.skippedByCommentList = append(p.skippedByCommentList, skipResourcesByComment...) 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 +148,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 +167,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/terraform/structure/terraform_parser.go b/src/terraform/structure/terraform_parser.go index def66348c..67297e874 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 { @@ -121,17 +122,20 @@ func (p *TerraformParser) GetSourceFiles(directory string) ([]string, error) { return files, nil } +func (p *TerraformParser) GetSkipResourcesByComment() []string { + return p.skippedByCommentList +} + 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") @@ -139,16 +143,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 @@ -185,13 +189,13 @@ func (p *TerraformParser) ParseFile(filePath string) ([]structure.IBlock, []stri } if strings.ToUpper(strings.TrimSpace(lineAbove)) == "#YOR:SKIP" || skipAll { - skipResourcesByComment = append(skipResourcesByComment, terraformBlock.GetResourceID()) + p.skippedByCommentList = append(p.skippedByCommentList, 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 { @@ -241,7 +245,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) } @@ -613,7 +617,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/tests/integration/integration_test.go b/tests/integration/integration_test.go index 1863f4b28..8e8432714 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -304,7 +304,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" { @@ -370,7 +370,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" { @@ -421,7 +421,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"]