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

[K9VULN-3872] Add --git-repository option to sbom/sarif upload #1570

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

piloulacdog
Copy link

@piloulacdog piloulacdog commented Feb 28, 2025

TL;DR

Adding --git-repository option support to datadog-ci sbom upload and datadog-ci sarif upload.

What

We have users complaining about CI integration of datadog-ci to generate SBOM for repository which do not report the correct git information.

And why?

Currently, it is not clear how the datadog-ci sbom upload command gets Git context. It currently does:

  1. Current process location
  2. CI environment variables (can be disabled with: --no-ci-tags option)
  3. Override environment variables (DD_GIT_* variables)

Currently users have two options to have the correct Git values when running the upload command from CI:

  1. disable ci automatic detection of env vars with--no-ci-tags
  2. use override DD_ variables.

The current workaround is to override DD_ variables

export DD_GIT_REPOSITORY_URL=$(git config --get remote.origin.url)
export DD_GIT_BRANCH=$(git rev-parse --abbrev-ref HEAD)
export DD_GIT_COMMIT_SHA=$COMMIT_SHA
...

it would be better if we could specify a repository while still read CI variables (not related to GIT info). New priority would be:

  1. Current process location
  2. CI environment variables (can be disabled with: --no-ci-tags option)
  3. Explicitly provided Git repository (through --git-repository option)
  4. Override environment variables (DD_GIT_* variables)

How?

We give user the ability to pass an extra parameter --git-repository which we will use to initialize the directory from which we read the git environment variables.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)

@piloulacdog piloulacdog added documentation Improvements or additions to documentation enhancement New feature or request labels Mar 3, 2025
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Mar 3, 2025

Datadog Report

Branch report: pierrelouis.lacorte/K9VULN-3872-add-git-repository-option
Commit report: 93e3332
Test service: datadog-ci-tests

✅ 0 Failed, 150 Passed, 0 Skipped, 45.67s Total duration (1m 36.88s time saved)

@piloulacdog piloulacdog force-pushed the pierrelouis.lacorte/K9VULN-3872-add-git-repository-option branch 4 times, most recently from 1a34b57 to 657fe87 Compare March 3, 2025 17:53
@piloulacdog piloulacdog marked this pull request as ready for review March 3, 2025 18:00
@piloulacdog piloulacdog requested review from a team as code owners March 3, 2025 18:00
@piloulacdog piloulacdog requested a review from tonyredondo March 3, 2025 18:00
@@ -0,0 +1,26 @@
[core]
Copy link
Author

Choose a reason for hiding this comment

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

I did create a duplicate "gitconfig" file in sbom/fixtures and sarif/fixtures as I couldn't settle on where to put it in common directory. And it would keep ensuring that those commands have independent test set.
But if you feel strongly another way, please let me know and I would move it.

@piloulacdog piloulacdog force-pushed the pierrelouis.lacorte/K9VULN-3872-add-git-repository-option branch from 657fe87 to 93e3332 Compare March 3, 2025 18:08
@@ -7,16 +7,20 @@ This command lets you upload SBOM files to the Datadog intake endpoint.

- CycloneDX 1.4
- CycloneDX 1.5
- CycloneDX 1.6
Copy link
Author

Choose a reason for hiding this comment

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

lucky documentation change, we do support 1.6 too

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@rtrieu rtrieu left a comment

Choose a reason for hiding this comment

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

A few non-breaking suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants