Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include purl info in the event #5092

Merged
merged 12 commits into from
Oct 2, 2024
Merged

Include purl info in the event #5092

merged 12 commits into from
Oct 2, 2024

Conversation

D074360
Copy link
Member

@D074360 D074360 commented Sep 20, 2024

Changes

As part of build coordinates purl information is added by reading the cyclonedx BOM.

  • Tests
  • Documentation

@D074360 D074360 marked this pull request as ready for review October 1, 2024 11:39
@D074360 D074360 requested a review from a team as a code owner October 1, 2024 11:39
@D074360
Copy link
Member Author

D074360 commented Oct 1, 2024

/it

1 similar comment
@D074360
Copy link
Member Author

D074360 commented Oct 1, 2024

/it

Comment on lines 212 to 237
func getPurlForThePomAndDeleteIndividualBom(pomFilePath string) string {
bomPath := filepath.Join(filepath.Dir(pomFilePath) + "/target/" + mvnBomFilename + ".xml")
if exists, _ := piperutils.FileExists(bomPath); exists {
bom, err := piperutils.GetBom(bomPath)
if err != nil {
log.Entry().Warnf("failed to get bom file %s: %v", bomPath, err)
return ""
}

log.Entry().Debugf("Found purl: %s for the bomPath: %s", bom.Metadata.Component.Purl, bomPath)
purl := bom.Metadata.Component.Purl

// Check if the BOM is an aggregated BOM
if !isAggregatedBOM(bom) {
// Delete the individual BOM file
err = os.Remove(bomPath)
if err != nil {
log.Entry().Warnf("failed to delete bom file %s: %v", bomPath, err)
}
}

return purl
}
log.Entry().Debugf("bom file doesn't exist and hence no pURL info: %v", bomPath)
return ""
}
Copy link
Member

Choose a reason for hiding this comment

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

This code could be rewritten in a more 'Go way'.

func getPurlForThePomAndDeleteIndividualBom(pomFilePath string) string {
    bomPath := filepath.Join(filepath.Dir(pomFilePath), "target", mvnBomFilename+".xml")
    exists, _ := piperutils.FileExists(bomPath)
    if !exists {
        log.Entry().Debugf("bom file doesn't exist and hence no pURL info: %v", bomPath)
        return ""
    }

    bom, err := piperutils.GetBom(bomPath)
    if err != nil {
        log.Entry().Warnf("failed to get bom file %s: %v", bomPath, err)
        return ""
    }

    log.Entry().Debugf("Found purl: %s for the bomPath: %s", bom.Metadata.Component.Purl, bomPath)
    purl := bom.Metadata.Component.Purl

    if !isAggregatedBOM(bom) {
        if err := os.Remove(bomPath); err != nil {
            log.Entry().Warnf("failed to delete bom file %s: %v", bomPath, err)
        }
    }

    return purl
}   

This will reduce cognitive load by getting rid of nested if-else statements

@@ -225,6 +226,20 @@ func (exec *Execute) publish(packageJSON, registry, username, password string, p
return nil
}

func getPurl(packageJSON string) string {
expectedBomFilePath := filepath.Join(filepath.Dir(packageJSON) + "/" + npmBomFilename)
if exists, _ := CredentialUtils.FileExists(expectedBomFilePath); exists {
Copy link
Member

Choose a reason for hiding this comment

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

same here

return Bom{}, err
}
defer xmlFile.Close()
byteValue, _ := io.ReadAll(xmlFile)
Copy link
Member

Choose a reason for hiding this comment

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

ignoring errors is not recommended

@D074360
Copy link
Member Author

D074360 commented Oct 1, 2024

@Googlom I have addressed your comments, please have a look again. Could you please also trigger it test run. Thanks

Copy link

sonarcloud bot commented Oct 2, 2024

@Googlom
Copy link
Member

Googlom commented Oct 2, 2024

/it-go

@Googlom Googlom self-requested a review October 2, 2024 07:29
@D074360 D074360 enabled auto-merge (squash) October 2, 2024 07:30
@D074360 D074360 merged commit 5230c3d into master Oct 2, 2024
12 checks passed
@D074360 D074360 deleted the add-purl-info branch October 2, 2024 07:34
D074360 added a commit that referenced this pull request Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants