Skip to content

Conversation

Copy link
Collaborator

@nbagnard nbagnard left a comment

Choose a reason for hiding this comment

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

It's looking good.
I have only a few comments to help make our lives easier in the long run

.evg.yml Outdated
# sbom relevant variables
export SBOM_TOOL_DIR=sbom_generations
export SBOM_DIR=$ARTIFACTS_DIR/sboms
export SBOM_FINAL=$ARTIFACTS_DIR/sboms/mongo-jdbc-driver.cdx.json
Copy link
Collaborator

Choose a reason for hiding this comment

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

[nit] Let's call this SBOM_LITE from now on to match DEVPROD naming convention.

.evg.yml Outdated
- name: sbom
commands:
- func: "generate sbom"
- func: "add team name to sbom"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can merge this func with the previous one. It doesn't have to be so granular.
I also don't know how I feel with using func versus having a list of commands being part of a function.
It makes the func longer, but at the same time, having all piece be under the same function means that there it is clear that they have to happen in that order and that what the previous function produces is expected to exist for the next function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Originally, it was one function; however, it would stop executing after ./gradlew cyclonedxBom for some reason, so I made two separate functions. However, I can combine the two and just have two - command: shell.exec in one function.

export SBOM_TOOL_DIR=sbom_generations
export SBOM_DIR=$ARTIFACTS_DIR/sboms
export SBOM_FINAL=$ARTIFACTS_DIR/sboms/mongo-jdbc-driver.cdx.json
export SBOM_WITHOUT_TEAM_NAME=$ARTIFACTS_DIR/sboms/sbom_without_team_name.json
Copy link
Collaborator

Choose a reason for hiding this comment

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

Defining this variable isn't very useful since the Gradle script doesn't use this value.
Variables are great if you want to guarantee that names are the same everywhere and to make maintenance easier.

You can however using it with Gradle and that would be the perfect use of a variable. Although I think the name of the temp file doesn't have to be shared with all task and can simply be defined as part of the func.

I pushed an example on my local branch. I had it ready but never pushed it on my remote branch. Look at both the Evergreen script (line 466) and the gradle.properties file
master...nbagnard:mongo-jdbc-driver:ssdlc-poc#diff-73e7b844e5fa69d8104ed91b7c281ea7262bc60bd7be03d515048aff04478fe6R435

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I defined the output name and destination in the build.gradle file:

cyclonedxBom {
    // Boms destination directory. Defaults to 'build/reports'
    destination = file("artifacts/sboms")
    // The file name for the generated BOMs (before the file format suffix). Defaults to 'bom'
    outputName = "sbom_without_team_name"
}

These environmental variables were made to make the evergreen functions easier to read.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah, I didn't realize that the variables were case insensitive 👍
I think destination is not using the SBOM_DIR variable, could you add it and then think would be perfect!

@EthanHardyMongo EthanHardyMongo requested a review from nbagnard June 10, 2024 20:40
Copy link
Collaborator

@nbagnard nbagnard left a comment

Choose a reason for hiding this comment

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

LGTM!
Pending the last little destination variable to sync up between the evergreen script and gradle.properties

export SBOM_TOOL_DIR=sbom_generations
export SBOM_DIR=$ARTIFACTS_DIR/sboms
export SBOM_FINAL=$ARTIFACTS_DIR/sboms/mongo-jdbc-driver.cdx.json
export SBOM_WITHOUT_TEAM_NAME=$ARTIFACTS_DIR/sboms/sbom_without_team_name.json
Copy link
Collaborator

Choose a reason for hiding this comment

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

ah, I didn't realize that the variables were case insensitive 👍
I think destination is not using the SBOM_DIR variable, could you add it and then think would be perfect!

@EthanHardyMongo EthanHardyMongo added this pull request to the merge queue Jun 11, 2024
Merged via the queue into mongodb:master with commit 55092b1 Jun 11, 2024
@EthanHardyMongo EthanHardyMongo deleted the SQL-2091 branch June 11, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants