From dc8cb56fab7b558bee7066627b2089920eb6aa1e Mon Sep 17 00:00:00 2001 From: Pierre-Louis Lacorte Date: Fri, 28 Feb 2025 19:31:10 +0000 Subject: [PATCH 1/9] feat: add --git-repository command to 'sbom upload' and 'sarif upload' --- src/commands/sarif/README.md | 1 + src/commands/sarif/__tests__/upload.test.ts | 76 ++++++++++++--- src/commands/sarif/renderer.ts | 7 +- src/commands/sarif/upload.ts | 7 +- src/commands/sbom/README.md | 1 + src/commands/sbom/__tests__/payload.test.ts | 92 ++++++++++++++++--- .../sbom/__tests__/validation.test.ts | 4 +- src/commands/sbom/upload.ts | 3 +- src/helpers/git/format-git-span-data.ts | 10 +- src/helpers/tags.ts | 5 +- 10 files changed, 162 insertions(+), 44 deletions(-) diff --git a/src/commands/sarif/README.md b/src/commands/sarif/README.md index f55ce6c7b..3080692ca 100644 --- a/src/commands/sarif/README.md +++ b/src/commands/sarif/README.md @@ -25,6 +25,7 @@ The positional arguments are the directories or file paths in which the SARIF re - `--max-concurrency` (default: `20`): number of concurrent uploads to the API. - `--dry-run` (default: `false`): runs the command without the final upload step. All other checks are performed. - `--no-verify` (default: `false`): runs the command without performing report validation on the CLI. +- `--git-repository` (default: `current working directory`): reports git environment context from specified repository. ### Environment variables diff --git a/src/commands/sarif/__tests__/upload.test.ts b/src/commands/sarif/__tests__/upload.test.ts index 251f8f680..50701d0a1 100644 --- a/src/commands/sarif/__tests__/upload.test.ts +++ b/src/commands/sarif/__tests__/upload.test.ts @@ -1,9 +1,12 @@ import os from 'os' +import fs from 'fs' +import simpleGit from 'simple-git'; +import path from 'path'; -import {Cli} from 'clipanion/lib/advanced' +import { Cli } from 'clipanion/lib/advanced' -import {renderInvalidFile} from '../renderer' -import {UploadSarifReportCommand} from '../upload' +import { renderInvalidFile } from '../renderer' +import { UploadSarifReportCommand } from '../upload' const makeCli = () => { const cli = new Cli() @@ -37,7 +40,7 @@ describe('upload', () => { process.env = {} const write = jest.fn() const command = new UploadSarifReportCommand() - command.context = {stdout: {write}} as any + command.context = { stdout: { write } } as any expect(command['getApiHelper'].bind(command)).toThrow('API key is missing') expect(write.mock.calls[0][0]).toContain('DATADOG_API_KEY') @@ -166,16 +169,16 @@ describe('upload', () => { }) describe('execute', () => { - const runCLI = async (paths: string[]) => { + const runCLI = async (args: string[]) => { const cli = makeCli() const context = createMockContext() as any - process.env = {DATADOG_API_KEY: 'PLACEHOLDER'} - const code = await cli.run(['sarif', 'upload', '--env', 'ci', '--dry-run', ...paths], context) + process.env = { DATADOG_API_KEY: 'PLACEHOLDER' } + const code = await cli.run(['sarif', 'upload', '--env', 'ci', '--dry-run', ...args], context) - return {context, code} + return { context, code } } test('relative path with double dots', async () => { - const {context, code} = await runCLI(['./src/commands/sarif/__tests__/doesnotexist/../fixtures/subfolder']) + const { context, code } = await runCLI(['./src/commands/sarif/__tests__/doesnotexist/../fixtures/subfolder']) const output = context.stdout.toString().split(os.EOL) expect(code).toBe(0) checkConsoleOutput(output, { @@ -185,7 +188,7 @@ describe('execute', () => { }) }) test('multiple paths', async () => { - const {context, code} = await runCLI([ + const { context, code } = await runCLI([ './src/commands/sarif/__tests__/fixtures/subfolder/', './src/commands/sarif/__tests__/fixtures/another_subfolder/', ]) @@ -202,18 +205,58 @@ describe('execute', () => { }) test('absolute path', async () => { - const {context, code} = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder']) + const { context, code } = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder']) const output = context.stdout.toString().split(os.EOL) expect(code).toBe(0) checkConsoleOutput(output, { basePaths: [`${process.cwd()}/src/commands/sarif/__tests__/fixtures/subfolder`], concurrency: 20, env: 'ci', + spanTags: { + "git.repository_url": "git@github.com:DataDog/datadog-ci.git", + "env": "ci" + } }) }) + test('absolute path when passing git repository', async () => { + const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitPath-')); + try { + // Configure local git repository + const git = simpleGit(tmpdir); + await git.init(['--initial-branch=test-branch']).addRemote('origin', 'https://github.com/TeSt-FaKe-RePo/repo.git'); + await git.commit('Initial commit', [], { '--allow-empty': null, '--author': '"John Doe "', }); + + const { context, code } = await runCLI([`--git-repository=${tmpdir}`, process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder']) + const output = context.stdout.toString().split(os.EOL) + expect(code).toBe(0) + checkConsoleOutput(output, { + basePaths: [`${process.cwd()}/src/commands/sarif/__tests__/fixtures/subfolder`], + concurrency: 20, + env: 'ci', + spanTags: { + "git.repository_url": "git@github.com:TeSt-FaKe-RePo/repo.git", + "git.branch": "test-branch", + "git.commit.author.email": "john.doe@example.com", + "git.commit.author.name": "John Doe", + "env": "ci" + } + }) + } finally { + // Removed temporary git file + fs.rmSync(tmpdir, { recursive: true, force: true }); + } + }) + + test('absolute path when passing git repository which do not exist', async () => { + const nonExistingGitRepository = "/you/cannot/find/me" + // Pass a git repository which do not exist, command should fail + const { context, code } = await runCLI([`--git-repository=${nonExistingGitRepository}`, process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder']) + expect(code).toBe(1) + }) + test('single file', async () => { - const {context, code} = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/valid-results.sarif']) + const { context, code } = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/valid-results.sarif']) const output = context.stdout.toString().split(os.EOL) const path = `${process.cwd()}/src/commands/sarif/__tests__/fixtures/valid-results.sarif` expect(code).toBe(0) @@ -226,7 +269,7 @@ describe('execute', () => { }) test('not found file', async () => { - const {context, code} = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/not-found.sarif']) + const { context, code } = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/not-found.sarif']) const output = context.stdout.toString().split(os.EOL) const path = `${process.cwd()}/src/commands/sarif/__tests__/fixtures/not-found.sarif` expect(code).toBe(1) @@ -239,6 +282,7 @@ interface ExpectedOutput { basePaths: string[] concurrency: number env: string + spanTags?: Record } const checkConsoleOutput = (output: string[], expected: ExpectedOutput) => { @@ -248,4 +292,10 @@ const checkConsoleOutput = (output: string[], expected: ExpectedOutput) => { expect(output[3]).toContain('Only one upload per commit, env and tool') expect(output[4]).toContain(`Preparing upload for`) expect(output[4]).toContain(`env:${expected.env}`) + + if (expected.spanTags) { + Object.keys(expected.spanTags).forEach((k) => { + expect(output[5]).toContain(`"${k}":"${expected.spanTags![k]}"`) + }) + } } diff --git a/src/commands/sarif/renderer.ts b/src/commands/sarif/renderer.ts index 4cdb033c2..b3038cc71 100644 --- a/src/commands/sarif/renderer.ts +++ b/src/commands/sarif/renderer.ts @@ -2,9 +2,9 @@ import path from 'path' import chalk from 'chalk' -import {getBaseUrl} from '../junit/utils' +import { getBaseUrl } from '../junit/utils' -import {Payload} from './interfaces' +import { Payload } from './interfaces' const ICONS = { FAILED: '❌', @@ -72,9 +72,10 @@ export const renderSuccessfulCommand = (fileCount: number, duration: number) => return fullStr } -export const renderDryRunUpload = (payload: Payload): string => `[DRYRUN] ${renderUpload(payload)}` +export const renderDryRunUpload = (payload: Payload): string => `[DRYRUN] ${renderUploadWithSpan(payload)}` export const renderUpload = (payload: Payload): string => `Uploading SARIF report in ${payload.reportPath}\n` +export const renderUploadWithSpan = (payload: Payload): string => `Uploading SARIF report to ${payload.reportPath} with tags ${JSON.stringify(payload.spanTags)}\n` export const renderCommandInfo = ( basePaths: string[], diff --git a/src/commands/sarif/upload.ts b/src/commands/sarif/upload.ts index 7486d8418..8e9d18fec 100644 --- a/src/commands/sarif/upload.ts +++ b/src/commands/sarif/upload.ts @@ -64,6 +64,7 @@ export class UploadSarifReportCommand extends Command { private maxConcurrency = Option.String('--max-concurrency', '20', {validator: validation.isInteger()}) private serviceFromCli = Option.String('--service') private tags = Option.Array('--tags') + private gitPath = Option.String('--git-repository') private noVerify = Option.Boolean('--no-verify', false) private noCiTags = Option.Boolean('--no-ci-tags', false) @@ -110,7 +111,7 @@ export class UploadSarifReportCommand extends Command { // Always using the posix version to avoid \ on Windows. this.basePaths = this.basePaths.map((basePath) => path.posix.normalize(basePath)) - const spanTags = await getSpanTags(this.config, this.tags, !this.noCiTags) + const spanTags = await getSpanTags(this.config, this.tags, !this.noCiTags, this.gitPath) // Gather any missing mandatory git fields to display to the user const missingGitFields = getMissingRequiredGitTags(spanTags) @@ -121,7 +122,7 @@ export class UploadSarifReportCommand extends Command { } const payloads = await this.getMatchingSarifReports(spanTags) - + if (payloads.length === 0) { this.context.stdout.write(renderFilesNotFound(this.basePaths)) @@ -133,7 +134,7 @@ export class UploadSarifReportCommand extends Command { this.context.stdout.write( renderCommandInfo(this.basePaths, env, sha, this.maxConcurrency, this.dryRun, this.noVerify) ) - const upload = (p: Payload) => this.uploadSarifReport(api, p) + const upload = (payload: Payload) => this.uploadSarifReport(api, payload) const initialTime = new Date().getTime() diff --git a/src/commands/sbom/README.md b/src/commands/sbom/README.md index 75aaab6b8..e8d87f452 100644 --- a/src/commands/sbom/README.md +++ b/src/commands/sbom/README.md @@ -17,6 +17,7 @@ datadog-ci sbom upload ### Optional arguments - `--env` is a string that represents the environment in which you want your tests to appear. +- `--git-repository` (default: `current working directory`): reports git environment context from specified repository. ### Environment variables diff --git a/src/commands/sbom/__tests__/payload.test.ts b/src/commands/sbom/__tests__/payload.test.ts index 0bfabdcbe..abea9f834 100644 --- a/src/commands/sbom/__tests__/payload.test.ts +++ b/src/commands/sbom/__tests__/payload.test.ts @@ -1,10 +1,13 @@ import fs from 'fs' +import simpleGit from 'simple-git'; +import os from 'os'; +import path from 'path'; -import {DatadogCiConfig} from '../../../helpers/config' -import {getSpanTags} from '../../../helpers/tags' +import { DatadogCiConfig } from '../../../helpers/config' +import { getSpanTags, getMissingRequiredGitTags } from '../../../helpers/tags' -import {generatePayload} from '../payload' -import {DependencyLanguage, Location} from '../types' +import { generatePayload } from '../payload' +import { DependencyLanguage, Location } from '../types' describe('generation of payload', () => { beforeEach(() => { @@ -20,7 +23,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') expect(payload).not.toBeNull() @@ -49,7 +52,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') expect(payload).not.toBeNull() @@ -82,7 +85,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') expect(payload).not.toBeNull() @@ -124,7 +127,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -145,7 +148,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -166,7 +169,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -191,7 +194,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -214,7 +217,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -240,7 +243,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -264,7 +267,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -330,7 +333,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -343,4 +346,63 @@ describe('generation of payload', () => { expect(dependencies[1].name).toEqual('jinja2') expect(dependencies[1].version).toEqual('3.1.5') }) + + test('should correctly work with a CycloneDX 1.4 file and passing git repository', async () => { + const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitPath-')); + try { + // Configure local git repository + const git = simpleGit(tmpdir); + await git.init(['--initial-branch=test-branch']).addRemote('origin', 'https://github.com/TeSt-FaKe-RePo/repo.git'); + await git.commit('Initial commit', [], { '--allow-empty': null, '--author': '"John Doe "', }); + + const sbomFile = './src/commands/sbom/__tests__/fixtures/sbom.1.4.ok.json' + const sbomContent = JSON.parse(fs.readFileSync(sbomFile).toString('utf8')) + const config: DatadogCiConfig = { + apiKey: undefined, + env: undefined, + envVarTags: undefined, + } + + // Pass git directory to load git context + const tags = await getSpanTags(config, [], true, tmpdir) + expect(getMissingRequiredGitTags(tags)).toHaveLength(0) + + const payload = generatePayload(sbomContent, tags, 'service', 'env') + expect(payload).not.toBeNull() + expect(payload?.id).toStrictEqual(expect.any(String)) + + // Local git repository should be reported + expect(payload?.commit.sha).toStrictEqual(expect.any(String)) + expect(payload?.commit.author_name).toStrictEqual("John Doe") + expect(payload?.commit.author_email).toStrictEqual("john.doe@example.com") + expect(payload?.commit.committer_name).toStrictEqual(expect.any(String)) + expect(payload?.commit.committer_email).toStrictEqual(expect.any(String)) + expect(payload?.commit.branch).toStrictEqual("test-branch") + expect(payload?.repository.url).toContain('github.com') + expect(payload?.repository.url).toContain('git@github.com:TeSt-FaKe-RePo/repo.git') + expect(payload?.dependencies.length).toBe(62) + expect(payload?.dependencies[0].name).toBe('stack-cors') + expect(payload?.dependencies[0].version).toBe('1.3.0') + expect(payload?.dependencies[0].licenses.length).toBe(0) + expect(payload?.dependencies[0].language).toBe(DependencyLanguage.PHP) + } finally { + // Removed temporary git file + fs.rmSync(tmpdir, { recursive: true, force: true }); + } + }) + + test('should fail to read git information', async () => { + const nonExistingGitRepository = "/you/cannot/find/me" + const config: DatadogCiConfig = { + apiKey: undefined, + env: undefined, + envVarTags: undefined, + } + + // Pass non existing git directory to load git context + // It is missing all git tags. + const tags = await getSpanTags(config, [], true, nonExistingGitRepository) + expect(getMissingRequiredGitTags(tags)).toHaveLength(7) + }) + }) diff --git a/src/commands/sbom/__tests__/validation.test.ts b/src/commands/sbom/__tests__/validation.test.ts index d6737e134..35186b792 100644 --- a/src/commands/sbom/__tests__/validation.test.ts +++ b/src/commands/sbom/__tests__/validation.test.ts @@ -149,7 +149,7 @@ describe('validation of sbom file', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') expect(payload).not.toBeNull() @@ -166,7 +166,7 @@ describe('validation of sbom file', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true) + const tags = await getSpanTags(config, [], true, undefined) const payload = generatePayload(sbomContent, tags, 'service', 'env') expect(payload).not.toBeNull() diff --git a/src/commands/sbom/upload.ts b/src/commands/sbom/upload.ts index 048a1bb3e..6bfe10b26 100644 --- a/src/commands/sbom/upload.ts +++ b/src/commands/sbom/upload.ts @@ -48,6 +48,7 @@ export class UploadSbomCommand extends Command { private serviceFromCli = Option.String('--service') private env = Option.String('--env', 'ci') private tags = Option.Array('--tags') + private gitPath = Option.String('--git-repository') private debug = Option.Boolean('--debug') private noCiTags = Option.Boolean('--no-ci-tags', false) @@ -114,7 +115,7 @@ export class UploadSbomCommand extends Command { this.config.appKey ) - const tags = await getSpanTags(this.config, this.tags, !this.noCiTags) + const tags = await getSpanTags(this.config, this.tags, !this.noCiTags, this.gitPath) // Gather any missing mandatory git fields to display to the user const missingGitFields = getMissingRequiredGitTags(tags) diff --git a/src/helpers/git/format-git-span-data.ts b/src/helpers/git/format-git-span-data.ts index a8bfb7251..58f3d74b7 100644 --- a/src/helpers/git/format-git-span-data.ts +++ b/src/helpers/git/format-git-span-data.ts @@ -1,4 +1,4 @@ -import type {SpanTags} from '../interfaces' +import type { SpanTags } from '../interfaces' import simpleGit from 'simple-git' @@ -14,14 +14,14 @@ import { GIT_REPOSITORY_URL, GIT_SHA, } from '../tags' -import {filterSensitiveInfoFromRepository} from '../utils' +import { filterSensitiveInfoFromRepository } from '../utils' -import {gitAuthorAndCommitter, gitBranch, gitHash, gitMessage, gitRepositoryURL} from './get-git-data' +import { gitAuthorAndCommitter, gitBranch, gitHash, gitMessage, gitRepositoryURL } from './get-git-data' -export const getGitMetadata = async (): Promise => { +export const getGitMetadata = async (path: string | undefined): Promise => { try { const git = simpleGit({ - baseDir: process.cwd(), + baseDir: path || process.cwd(), binary: 'git', // We are invoking at most 5 git commands at the same time. maxConcurrentProcesses: 5, diff --git a/src/helpers/tags.ts b/src/helpers/tags.ts index 30ef5f1f7..6667b2cd4 100644 --- a/src/helpers/tags.ts +++ b/src/helpers/tags.ts @@ -236,10 +236,11 @@ export const getMissingRequiredGitTags = (tags: SpanTags): string[] => { export const getSpanTags = async ( config: DatadogCiConfig, additionalTags: string[] | undefined, - includeCiTags: boolean + includeCiTags: boolean, + gitPath: string | undefined ): Promise => { const ciSpanTags = includeCiTags ? getCISpanTags() : [] - const gitSpanTags = await getGitMetadata() + const gitSpanTags = await getGitMetadata(gitPath) const userGitSpanTags = getUserGitSpanTags() const envVarTags = config.envVarTags ? parseTags(config.envVarTags.split(',')) : {} From b6af30c779564f78b6c8c7ff85110ec8c34cc625 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Lacorte Date: Fri, 28 Feb 2025 19:40:10 +0000 Subject: [PATCH 2/9] doc: improve doc for sbom upload --- src/commands/sbom/README.md | 7 +++++-- src/commands/sbom/upload.ts | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/commands/sbom/README.md b/src/commands/sbom/README.md index e8d87f452..6cdc2c6a1 100644 --- a/src/commands/sbom/README.md +++ b/src/commands/sbom/README.md @@ -7,17 +7,20 @@ This command lets you upload SBOM files to the Datadog intake endpoint. - CycloneDX 1.4 - CycloneDX 1.5 + - CycloneDX 1.6 ## Usage ```bash -datadog-ci sbom upload +datadog-ci sbom upload [--env] [--no-ci-tags] [--git-repository] [--debug] ``` ### Optional arguments -- `--env` is a string that represents the environment in which you want your tests to appear. +- `--env` (default: `ci`): represents the environment in which you want your sbom to appear. +- `--no-ci-tags` (default: `false`): ignore continuous integration automatic detection of environment variables. - `--git-repository` (default: `current working directory`): reports git environment context from specified repository. +- `--debug` (default: `false`): output debug logs. ### Environment variables diff --git a/src/commands/sbom/upload.ts b/src/commands/sbom/upload.ts index 6bfe10b26..baf70b7dc 100644 --- a/src/commands/sbom/upload.ts +++ b/src/commands/sbom/upload.ts @@ -136,7 +136,7 @@ export class UploadSbomCommand extends Command { if (!validateSbomFileAgainstSchema(basePath, validator, !!this.debug)) { this.context.stdout.write( - 'SBOM file not fully compliant against CycloneDX 1.4 or 1.5 specifications (use --debug to get validation error)\n' + 'SBOM file not fully compliant against CycloneDX 1.4, 1.5 or 1.6 specifications (use --debug to get validation error)\n' ) } if (!validateFileAgainstToolRequirements(basePath, !!this.debug)) { From 975dea1f9e88ffe2cbcd5c29ef2dbdedc800edc7 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Lacorte Date: Mon, 3 Mar 2025 10:28:40 +0000 Subject: [PATCH 3/9] feat: make sure we prefer git directory context when manually passed over ci env --- src/commands/sarif/__tests__/upload.test.ts | 76 +++++++++++---------- src/commands/sarif/renderer.ts | 7 +- src/commands/sarif/upload.ts | 2 +- src/commands/sbom/README.md | 10 ++- src/commands/sbom/__tests__/payload.test.ts | 34 ++++----- src/helpers/__tests__/tags.test.ts | 60 ++++++++++++++++ src/helpers/git/format-git-span-data.ts | 6 +- src/helpers/tags.ts | 6 +- 8 files changed, 138 insertions(+), 63 deletions(-) diff --git a/src/commands/sarif/__tests__/upload.test.ts b/src/commands/sarif/__tests__/upload.test.ts index 50701d0a1..23838b303 100644 --- a/src/commands/sarif/__tests__/upload.test.ts +++ b/src/commands/sarif/__tests__/upload.test.ts @@ -1,12 +1,12 @@ -import os from 'os' import fs from 'fs' -import simpleGit from 'simple-git'; -import path from 'path'; +import os from 'os' +import path from 'path' -import { Cli } from 'clipanion/lib/advanced' +import {Cli} from 'clipanion/lib/advanced' +import simpleGit from 'simple-git' -import { renderInvalidFile } from '../renderer' -import { UploadSarifReportCommand } from '../upload' +import {renderInvalidFile} from '../renderer' +import {UploadSarifReportCommand} from '../upload' const makeCli = () => { const cli = new Cli() @@ -40,7 +40,7 @@ describe('upload', () => { process.env = {} const write = jest.fn() const command = new UploadSarifReportCommand() - command.context = { stdout: { write } } as any + command.context = {stdout: {write}} as any expect(command['getApiHelper'].bind(command)).toThrow('API key is missing') expect(write.mock.calls[0][0]).toContain('DATADOG_API_KEY') @@ -172,13 +172,13 @@ describe('execute', () => { const runCLI = async (args: string[]) => { const cli = makeCli() const context = createMockContext() as any - process.env = { DATADOG_API_KEY: 'PLACEHOLDER' } + process.env = {DATADOG_API_KEY: 'PLACEHOLDER'} const code = await cli.run(['sarif', 'upload', '--env', 'ci', '--dry-run', ...args], context) - return { context, code } + return {context, code} } test('relative path with double dots', async () => { - const { context, code } = await runCLI(['./src/commands/sarif/__tests__/doesnotexist/../fixtures/subfolder']) + const {context, code} = await runCLI(['./src/commands/sarif/__tests__/doesnotexist/../fixtures/subfolder']) const output = context.stdout.toString().split(os.EOL) expect(code).toBe(0) checkConsoleOutput(output, { @@ -188,7 +188,7 @@ describe('execute', () => { }) }) test('multiple paths', async () => { - const { context, code } = await runCLI([ + const {context, code} = await runCLI([ './src/commands/sarif/__tests__/fixtures/subfolder/', './src/commands/sarif/__tests__/fixtures/another_subfolder/', ]) @@ -205,7 +205,7 @@ describe('execute', () => { }) test('absolute path', async () => { - const { context, code } = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder']) + const {context, code} = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder']) const output = context.stdout.toString().split(os.EOL) expect(code).toBe(0) checkConsoleOutput(output, { @@ -213,21 +213,24 @@ describe('execute', () => { concurrency: 20, env: 'ci', spanTags: { - "git.repository_url": "git@github.com:DataDog/datadog-ci.git", - "env": "ci" - } + 'git.repository_url': 'git@github.com:DataDog/datadog-ci.git', + env: 'ci', + }, }) }) test('absolute path when passing git repository', async () => { - const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitPath-')); + const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitPath-')) try { // Configure local git repository - const git = simpleGit(tmpdir); - await git.init(['--initial-branch=test-branch']).addRemote('origin', 'https://github.com/TeSt-FaKe-RePo/repo.git'); - await git.commit('Initial commit', [], { '--allow-empty': null, '--author': '"John Doe "', }); + const git = simpleGit(tmpdir) + await git.init(['--initial-branch=test-branch']).addRemote('origin', 'https://github.com/TeSt-FaKe-RePo/repo.git') + await git.commit('Initial commit', [], {'--author': '"John Doe "'}) - const { context, code } = await runCLI([`--git-repository=${tmpdir}`, process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder']) + const {context, code} = await runCLI([ + `--git-repository=${tmpdir}`, + process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder', + ]) const output = context.stdout.toString().split(os.EOL) expect(code).toBe(0) checkConsoleOutput(output, { @@ -235,45 +238,48 @@ describe('execute', () => { concurrency: 20, env: 'ci', spanTags: { - "git.repository_url": "git@github.com:TeSt-FaKe-RePo/repo.git", - "git.branch": "test-branch", - "git.commit.author.email": "john.doe@example.com", - "git.commit.author.name": "John Doe", - "env": "ci" - } + 'git.repository_url': 'git@github.com:TeSt-FaKe-RePo/repo.git', + 'git.branch': 'test-branch', + 'git.commit.author.email': 'john.doe@example.com', + 'git.commit.author.name': 'John Doe', + env: 'ci', + }, }) } finally { // Removed temporary git file - fs.rmSync(tmpdir, { recursive: true, force: true }); + fs.rmSync(tmpdir, {recursive: true, force: true}) } }) test('absolute path when passing git repository which do not exist', async () => { - const nonExistingGitRepository = "/you/cannot/find/me" + const nonExistingGitRepository = '/you/cannot/find/me' // Pass a git repository which do not exist, command should fail - const { context, code } = await runCLI([`--git-repository=${nonExistingGitRepository}`, process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder']) + const {code} = await runCLI([ + `--git-repository=${nonExistingGitRepository}`, + process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder', + ]) expect(code).toBe(1) }) test('single file', async () => { - const { context, code } = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/valid-results.sarif']) + const {context, code} = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/valid-results.sarif']) const output = context.stdout.toString().split(os.EOL) - const path = `${process.cwd()}/src/commands/sarif/__tests__/fixtures/valid-results.sarif` + const path1 = `${process.cwd()}/src/commands/sarif/__tests__/fixtures/valid-results.sarif` expect(code).toBe(0) expect(output[0]).toContain('DRY-RUN MODE ENABLED. WILL NOT UPLOAD SARIF REPORT') expect(output[1]).toContain('Starting upload with concurrency 20.') - expect(output[2]).toContain(`Will upload SARIF report file ${path}`) + expect(output[2]).toContain(`Will upload SARIF report file ${path1}`) expect(output[3]).toContain('Only one upload per commit, env and tool') expect(output[4]).toContain(`Preparing upload for`) expect(output[4]).toContain(`env:ci`) }) test('not found file', async () => { - const { context, code } = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/not-found.sarif']) + const {context, code} = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/not-found.sarif']) const output = context.stdout.toString().split(os.EOL) - const path = `${process.cwd()}/src/commands/sarif/__tests__/fixtures/not-found.sarif` + const path2 = `${process.cwd()}/src/commands/sarif/__tests__/fixtures/not-found.sarif` expect(code).toBe(1) - expect(output[0]).toContain(`Cannot find valid SARIF report files to upload in ${path}`) + expect(output[0]).toContain(`Cannot find valid SARIF report files to upload in ${path2}`) expect(output[1]).toContain('Check the files exist and are valid.') }) }) diff --git a/src/commands/sarif/renderer.ts b/src/commands/sarif/renderer.ts index b3038cc71..3ccd5e612 100644 --- a/src/commands/sarif/renderer.ts +++ b/src/commands/sarif/renderer.ts @@ -2,9 +2,9 @@ import path from 'path' import chalk from 'chalk' -import { getBaseUrl } from '../junit/utils' +import {getBaseUrl} from '../junit/utils' -import { Payload } from './interfaces' +import {Payload} from './interfaces' const ICONS = { FAILED: '❌', @@ -75,7 +75,8 @@ export const renderSuccessfulCommand = (fileCount: number, duration: number) => export const renderDryRunUpload = (payload: Payload): string => `[DRYRUN] ${renderUploadWithSpan(payload)}` export const renderUpload = (payload: Payload): string => `Uploading SARIF report in ${payload.reportPath}\n` -export const renderUploadWithSpan = (payload: Payload): string => `Uploading SARIF report to ${payload.reportPath} with tags ${JSON.stringify(payload.spanTags)}\n` +export const renderUploadWithSpan = (payload: Payload): string => + `Uploading SARIF report to ${payload.reportPath} with tags ${JSON.stringify(payload.spanTags)}\n` export const renderCommandInfo = ( basePaths: string[], diff --git a/src/commands/sarif/upload.ts b/src/commands/sarif/upload.ts index 8e9d18fec..cb3b600e8 100644 --- a/src/commands/sarif/upload.ts +++ b/src/commands/sarif/upload.ts @@ -122,7 +122,7 @@ export class UploadSarifReportCommand extends Command { } const payloads = await this.getMatchingSarifReports(spanTags) - + if (payloads.length === 0) { this.context.stdout.write(renderFilesNotFound(this.basePaths)) diff --git a/src/commands/sbom/README.md b/src/commands/sbom/README.md index 6cdc2c6a1..f1c2eb563 100644 --- a/src/commands/sbom/README.md +++ b/src/commands/sbom/README.md @@ -18,7 +18,7 @@ datadog-ci sbom upload [--env] [--no-ci-tags] [--git-repository] [--debug] { beforeEach(() => { @@ -348,12 +349,12 @@ describe('generation of payload', () => { }) test('should correctly work with a CycloneDX 1.4 file and passing git repository', async () => { - const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitPath-')); + const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitPath-')) try { // Configure local git repository - const git = simpleGit(tmpdir); - await git.init(['--initial-branch=test-branch']).addRemote('origin', 'https://github.com/TeSt-FaKe-RePo/repo.git'); - await git.commit('Initial commit', [], { '--allow-empty': null, '--author': '"John Doe "', }); + const git = simpleGit(tmpdir) + await git.init(['--initial-branch=test-branch']).addRemote('origin', 'https://github.com/TeSt-FaKe-RePo/repo.git') + await git.commit('Initial commit', [], {'--author': '"John Doe "'}) const sbomFile = './src/commands/sbom/__tests__/fixtures/sbom.1.4.ok.json' const sbomContent = JSON.parse(fs.readFileSync(sbomFile).toString('utf8')) @@ -373,11 +374,11 @@ describe('generation of payload', () => { // Local git repository should be reported expect(payload?.commit.sha).toStrictEqual(expect.any(String)) - expect(payload?.commit.author_name).toStrictEqual("John Doe") - expect(payload?.commit.author_email).toStrictEqual("john.doe@example.com") + expect(payload?.commit.author_name).toStrictEqual('John Doe') + expect(payload?.commit.author_email).toStrictEqual('john.doe@example.com') expect(payload?.commit.committer_name).toStrictEqual(expect.any(String)) expect(payload?.commit.committer_email).toStrictEqual(expect.any(String)) - expect(payload?.commit.branch).toStrictEqual("test-branch") + expect(payload?.commit.branch).toStrictEqual('test-branch') expect(payload?.repository.url).toContain('github.com') expect(payload?.repository.url).toContain('git@github.com:TeSt-FaKe-RePo/repo.git') expect(payload?.dependencies.length).toBe(62) @@ -387,12 +388,12 @@ describe('generation of payload', () => { expect(payload?.dependencies[0].language).toBe(DependencyLanguage.PHP) } finally { // Removed temporary git file - fs.rmSync(tmpdir, { recursive: true, force: true }); + fs.rmSync(tmpdir, {recursive: true, force: true}) } }) test('should fail to read git information', async () => { - const nonExistingGitRepository = "/you/cannot/find/me" + const nonExistingGitRepository = '/you/cannot/find/me' const config: DatadogCiConfig = { apiKey: undefined, env: undefined, @@ -404,5 +405,4 @@ describe('generation of payload', () => { const tags = await getSpanTags(config, [], true, nonExistingGitRepository) expect(getMissingRequiredGitTags(tags)).toHaveLength(7) }) - }) diff --git a/src/helpers/__tests__/tags.test.ts b/src/helpers/__tests__/tags.test.ts index a2771a11e..5f1649f71 100644 --- a/src/helpers/__tests__/tags.test.ts +++ b/src/helpers/__tests__/tags.test.ts @@ -187,6 +187,66 @@ describe('getSpanTags', () => { key2: 'value2', }) }) + test('should prioritized git context corrently', async () => { + const config = { + apiKey: undefined, + env: undefined, + envVarTags: undefined, + } + + // define github action env variables + process.env.GITHUB_ACTIONS = 'true' + process.env.GITHUB_REPOSITORY = 'git@github.com:DataDog/datadog-ci' + process.env.GITHUB_HEAD_REF = 'prod' + process.env.GITHUB_SHA = '12000' + process.env.GITHUB_RUN_ID = '17' + + // Mock simpleGit (used to read git context) + ;(simpleGit as jest.Mock).mockImplementation(() => ({ + branch: () => ({current: 'main'}), + listRemote: async (git: any): Promise => 'https://www.github.com/datadog/safe-repository', + revparse: () => 'commitSHA', + show: (input: string[]) => { + if (input[1] === '--format=%s') { + return 'commit message' + } + + return 'authorName,authorEmail,authorDate,committerName,committerEmail,committerDate' + }, + })) + + // By default, the 'git' tags reported are the github action + let spanTags: SpanTags = await getSpanTags(config, [], true) + expect(spanTags).toMatchObject({ + 'ci.pipeline.id': '17', + 'git.repository_url': 'undefined/git@github.com:DataDog/datadog-ci.git', + 'git.branch': 'prod', + 'git.commit.sha': '12000', + }) + + // re-query span tags while specifying a git directory + // if should prefer the git directory over env variables. + spanTags = await getSpanTags(config, [], true, 'path/to/mockedSimpleGit') + // git tags are coming from simpleGit + expect(spanTags).toMatchObject({ + 'ci.pipeline.id': '17', + 'git.repository_url': 'https://www.github.com/datadog/safe-repository', + 'git.branch': 'main', + 'git.commit.sha': 'commitSHA', + }) + + process.env.DD_GIT_REPOSITORY_URL = 'https://www.github.com/datadog/not-so-safe-repository' + process.env.DD_GIT_BRANCH = 'staging' + process.env.DD_GIT_COMMIT_SHA = 'override' + spanTags = await getSpanTags(config, [], true, 'path/to/mockedSimpleGit') + // git tags are coming from env override + expect(spanTags).toMatchObject({ + 'ci.pipeline.id': '17', + 'git.repository_url': 'https://www.github.com/datadog/not-so-safe-repository', + 'git.branch': 'staging', + 'git.commit.sha': 'override', + }) + }) }) describe('sarif and sbom upload required git tags', () => { diff --git a/src/helpers/git/format-git-span-data.ts b/src/helpers/git/format-git-span-data.ts index 58f3d74b7..e17e908a0 100644 --- a/src/helpers/git/format-git-span-data.ts +++ b/src/helpers/git/format-git-span-data.ts @@ -1,4 +1,4 @@ -import type { SpanTags } from '../interfaces' +import type {SpanTags} from '../interfaces' import simpleGit from 'simple-git' @@ -14,9 +14,9 @@ import { GIT_REPOSITORY_URL, GIT_SHA, } from '../tags' -import { filterSensitiveInfoFromRepository } from '../utils' +import {filterSensitiveInfoFromRepository} from '../utils' -import { gitAuthorAndCommitter, gitBranch, gitHash, gitMessage, gitRepositoryURL } from './get-git-data' +import {gitAuthorAndCommitter, gitBranch, gitHash, gitMessage, gitRepositoryURL} from './get-git-data' export const getGitMetadata = async (path: string | undefined): Promise => { try { diff --git a/src/helpers/tags.ts b/src/helpers/tags.ts index 6667b2cd4..9ff8e8f74 100644 --- a/src/helpers/tags.ts +++ b/src/helpers/tags.ts @@ -237,7 +237,7 @@ export const getSpanTags = async ( config: DatadogCiConfig, additionalTags: string[] | undefined, includeCiTags: boolean, - gitPath: string | undefined + gitPath?: string ): Promise => { const ciSpanTags = includeCiTags ? getCISpanTags() : [] const gitSpanTags = await getGitMetadata(gitPath) @@ -247,8 +247,8 @@ export const getSpanTags = async ( const cliTags = additionalTags ? parseTags(additionalTags) : {} return { - ...gitSpanTags, - ...ciSpanTags, + // if users specify a git path to read from, we prefer git env variables over the CI context one + ...(gitPath ? {...ciSpanTags, ...gitSpanTags} : {...gitSpanTags, ...ciSpanTags}), ...userGitSpanTags, // User-provided git tags have precedence over the ones we get from the git command ...cliTags, ...envVarTags, From 2d32246092e6b3569288d3f78f575d3723f9af21 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Lacorte Date: Mon, 3 Mar 2025 10:38:52 +0000 Subject: [PATCH 4/9] refactor: remove non-required parameter --- src/commands/sarif/README.md | 9 ++++++-- src/commands/sarif/__tests__/upload.test.ts | 2 +- src/commands/sbom/__tests__/payload.test.ts | 22 +++++++++---------- .../sbom/__tests__/validation.test.ts | 4 ++-- src/helpers/__tests__/tags.test.ts | 3 ++- 5 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/commands/sarif/README.md b/src/commands/sarif/README.md index 3080692ca..1d1b1421c 100644 --- a/src/commands/sarif/README.md +++ b/src/commands/sarif/README.md @@ -25,6 +25,7 @@ The positional arguments are the directories or file paths in which the SARIF re - `--max-concurrency` (default: `20`): number of concurrent uploads to the API. - `--dry-run` (default: `false`): runs the command without the final upload step. All other checks are performed. - `--no-verify` (default: `false`): runs the command without performing report validation on the CLI. +- `--no-ci-tags` (default: `false`): ignore automatic detection of continuous integration environment variables. - `--git-repository` (default: `current working directory`): reports git environment context from specified repository. ### Environment variables @@ -35,9 +36,13 @@ Additionally, you may configure the `sarif` command with environment variables: - `DD_TAGS`: Set global tags applied to all spans. The format must be `key1:value1,key2:value2`. The upload process merges the tags passed on the command line with the tags in the `--tags` parameter. If a key appears in both `--tags` and `DD_TAGS`, the value in `DD_TAGS` takes precedence. - `DATADOG_SITE` or `DD_SITE`: choose your Datadog site, for example, datadoghq.com or datadoghq.eu. -### Optional dependencies +### Git context resolution -- [`git`](https://git-scm.com/downloads) is used for extracting repository metadata. +The Git context is resolved in the following order of priority: +1. Current process location +2. CI environment variables (can be disabled with: `--no-ci-tags` option) +3. Explicitely provided Git repository (through `--git-repository` option) +4. Override environment variables (`DD_GIT_*` variables) ### End-to-end testing process diff --git a/src/commands/sarif/__tests__/upload.test.ts b/src/commands/sarif/__tests__/upload.test.ts index 23838b303..3b0e736bc 100644 --- a/src/commands/sarif/__tests__/upload.test.ts +++ b/src/commands/sarif/__tests__/upload.test.ts @@ -251,7 +251,7 @@ describe('execute', () => { } }) - test('absolute path when passing git repository which do not exist', async () => { + test('absolute path when passing git repository which does not exist', async () => { const nonExistingGitRepository = '/you/cannot/find/me' // Pass a git repository which do not exist, command should fail const {code} = await runCLI([ diff --git a/src/commands/sbom/__tests__/payload.test.ts b/src/commands/sbom/__tests__/payload.test.ts index c82cb0dc3..335219803 100644 --- a/src/commands/sbom/__tests__/payload.test.ts +++ b/src/commands/sbom/__tests__/payload.test.ts @@ -24,7 +24,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') expect(payload).not.toBeNull() @@ -53,7 +53,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') expect(payload).not.toBeNull() @@ -86,7 +86,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') expect(payload).not.toBeNull() @@ -128,7 +128,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -149,7 +149,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -170,7 +170,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -195,7 +195,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -218,7 +218,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -244,7 +244,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -268,7 +268,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') @@ -334,7 +334,7 @@ describe('generation of payload', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') diff --git a/src/commands/sbom/__tests__/validation.test.ts b/src/commands/sbom/__tests__/validation.test.ts index 35186b792..d6737e134 100644 --- a/src/commands/sbom/__tests__/validation.test.ts +++ b/src/commands/sbom/__tests__/validation.test.ts @@ -149,7 +149,7 @@ describe('validation of sbom file', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') expect(payload).not.toBeNull() @@ -166,7 +166,7 @@ describe('validation of sbom file', () => { env: undefined, envVarTags: undefined, } - const tags = await getSpanTags(config, [], true, undefined) + const tags = await getSpanTags(config, [], true) const payload = generatePayload(sbomContent, tags, 'service', 'env') expect(payload).not.toBeNull() diff --git a/src/helpers/__tests__/tags.test.ts b/src/helpers/__tests__/tags.test.ts index 5f1649f71..de90dd82e 100644 --- a/src/helpers/__tests__/tags.test.ts +++ b/src/helpers/__tests__/tags.test.ts @@ -1,6 +1,7 @@ import {BaseContext} from 'clipanion' import simpleGit from 'simple-git' +import {DatadogCiConfig} from '../config' import {SpanTags} from '../interfaces' import { parseTags, @@ -188,7 +189,7 @@ describe('getSpanTags', () => { }) }) test('should prioritized git context corrently', async () => { - const config = { + const config: DatadogCiConfig = { apiKey: undefined, env: undefined, envVarTags: undefined, From aee58164c32211e7ddc3e943b7c7372335f9ec3a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Lacorte Date: Mon, 3 Mar 2025 10:42:37 +0000 Subject: [PATCH 5/9] doc: fix typo error --- src/commands/sarif/README.md | 2 +- src/commands/sbom/README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/commands/sarif/README.md b/src/commands/sarif/README.md index 1d1b1421c..4ef46b8d3 100644 --- a/src/commands/sarif/README.md +++ b/src/commands/sarif/README.md @@ -41,7 +41,7 @@ Additionally, you may configure the `sarif` command with environment variables: The Git context is resolved in the following order of priority: 1. Current process location 2. CI environment variables (can be disabled with: `--no-ci-tags` option) -3. Explicitely provided Git repository (through `--git-repository` option) +3. Explicitly provided Git repository (through `--git-repository` option) 4. Override environment variables (`DD_GIT_*` variables) ### End-to-end testing process diff --git a/src/commands/sbom/README.md b/src/commands/sbom/README.md index f1c2eb563..25c91e460 100644 --- a/src/commands/sbom/README.md +++ b/src/commands/sbom/README.md @@ -35,7 +35,7 @@ The following environment variables must be defined: The Git context is resolved in the following order of priority: 1. Current process location 2. CI environment variables (can be disabled with: `--no-ci-tags` option) -3. Explicitely provided Git repository (through `--git-repository` option) +3. Explicitly provided Git repository (through --git-repository option) 4. Override environment variables (`DD_GIT_*` variables) ## Development From 26f39b1baab633790673e285c2742998ad0ca906 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Lacorte Date: Mon, 3 Mar 2025 11:00:27 +0000 Subject: [PATCH 6/9] Fix: make path optional for getGitMetadata --- src/helpers/git/format-git-span-data.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers/git/format-git-span-data.ts b/src/helpers/git/format-git-span-data.ts index e17e908a0..26e11c676 100644 --- a/src/helpers/git/format-git-span-data.ts +++ b/src/helpers/git/format-git-span-data.ts @@ -18,7 +18,7 @@ import {filterSensitiveInfoFromRepository} from '../utils' import {gitAuthorAndCommitter, gitBranch, gitHash, gitMessage, gitRepositoryURL} from './get-git-data' -export const getGitMetadata = async (path: string | undefined): Promise => { +export const getGitMetadata = async (path?: string): Promise => { try { const git = simpleGit({ baseDir: path || process.cwd(), From 31c39e437b0f1c1ab30d84063bdd36006ec3bc44 Mon Sep 17 00:00:00 2001 From: piloulacdog Date: Mon, 3 Mar 2025 16:58:14 +0000 Subject: [PATCH 7/9] tests: use mocks for sbom and sarif upload --- .../sarif/__tests__/fixtures/gitconfig | 26 +++++++++++ src/commands/sarif/__tests__/upload.test.ts | 44 +++++++++++++++---- .../sbom/__tests__/fixtures/gitconfig | 26 +++++++++++ src/commands/sbom/__tests__/payload.test.ts | 34 ++++++++++---- src/helpers/git/format-git-span-data.ts | 4 +- 5 files changed, 114 insertions(+), 20 deletions(-) create mode 100644 src/commands/sarif/__tests__/fixtures/gitconfig create mode 100644 src/commands/sbom/__tests__/fixtures/gitconfig diff --git a/src/commands/sarif/__tests__/fixtures/gitconfig b/src/commands/sarif/__tests__/fixtures/gitconfig new file mode 100644 index 000000000..8768eaf91 --- /dev/null +++ b/src/commands/sarif/__tests__/fixtures/gitconfig @@ -0,0 +1,26 @@ +[core] + repositoryformatversion = 1 + filemode = false + bare = false + logallrefupdates = false + ignorecase = true + +[init] + defaultBranch = mock-branch + +[user] + name = MockUser123 + email = mock@fake.local + +[remote "mock-origin"] + url = https://mock-repo.local/fake.git + fetch = +refs/mocks/*:refs/remotes/mock-origin/* + +[branch "mock-branch"] + remote = mock-origin + merge = refs/heads/mock-branch + rebase = never # Unusual setting + +[hooks] + pre-commit = echo 'Mock pre-commit hook executed' + pre-push = echo 'Mock pre-push hook executed' diff --git a/src/commands/sarif/__tests__/upload.test.ts b/src/commands/sarif/__tests__/upload.test.ts index 3b0e736bc..822c3f093 100644 --- a/src/commands/sarif/__tests__/upload.test.ts +++ b/src/commands/sarif/__tests__/upload.test.ts @@ -213,7 +213,7 @@ describe('execute', () => { concurrency: 20, env: 'ci', spanTags: { - 'git.repository_url': 'git@github.com:DataDog/datadog-ci.git', + 'git.repository_url': 'DataDog/datadog-ci', env: 'ci', }, }) @@ -224,24 +224,29 @@ describe('execute', () => { try { // Configure local git repository const git = simpleGit(tmpdir) - await git.init(['--initial-branch=test-branch']).addRemote('origin', 'https://github.com/TeSt-FaKe-RePo/repo.git') - await git.commit('Initial commit', [], {'--author': '"John Doe "'}) - + setupLocalGitConfig(tmpdir) + await git.init() + // eslint-disable-next-line no-null/no-null + await git.commit('Initial commit', [], {'--allow-empty': null}) const {context, code} = await runCLI([ `--git-repository=${tmpdir}`, process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder', ]) const output = context.stdout.toString().split(os.EOL) expect(code).toBe(0) + checkConsoleOutput(output, { basePaths: [`${process.cwd()}/src/commands/sarif/__tests__/fixtures/subfolder`], concurrency: 20, env: 'ci', spanTags: { - 'git.repository_url': 'git@github.com:TeSt-FaKe-RePo/repo.git', - 'git.branch': 'test-branch', - 'git.commit.author.email': 'john.doe@example.com', - 'git.commit.author.name': 'John Doe', + 'git.repository_url': 'mock-repo.local/fake.git', + 'git.branch': 'mock-branch', + 'git.commit.message': 'Initial commit', + 'git.commit.committer.email': 'mock@fake.local', + 'git.commit.committer.name': 'MockUser123', + 'git.commit.author.email': 'mock@fake.local', + 'git.commit.author.name': 'MockUser123', env: 'ci', }, }) @@ -300,8 +305,29 @@ const checkConsoleOutput = (output: string[], expected: ExpectedOutput) => { expect(output[4]).toContain(`env:${expected.env}`) if (expected.spanTags) { + const regex = /with tags (\{.*\})/ + const match = output[5].match(regex) + expect(match).not.toBeNull() + + const spanTags = JSON.parse(match![1]) Object.keys(expected.spanTags).forEach((k) => { - expect(output[5]).toContain(`"${k}":"${expected.spanTags![k]}"`) + expect(spanTags[k]).not.toBeNull() + expect(spanTags[k]).toContain(expected.spanTags![k]) }) } } + +const getFixtures = (file: string) => { + return path.join('./src/commands/sarif/__tests__/fixtures', file) +} + +const setupLocalGitConfig = (dir: string) => { + const gitDir = path.join(dir, '.git') + if (!fs.existsSync(gitDir)) { + fs.mkdirSync(gitDir, {recursive: true}) + } + + const configFixture = fs.readFileSync(getFixtures('gitconfig'), 'utf8') + const configPath = path.join(gitDir, '/config') + fs.writeFileSync(configPath, configFixture) +} diff --git a/src/commands/sbom/__tests__/fixtures/gitconfig b/src/commands/sbom/__tests__/fixtures/gitconfig new file mode 100644 index 000000000..8768eaf91 --- /dev/null +++ b/src/commands/sbom/__tests__/fixtures/gitconfig @@ -0,0 +1,26 @@ +[core] + repositoryformatversion = 1 + filemode = false + bare = false + logallrefupdates = false + ignorecase = true + +[init] + defaultBranch = mock-branch + +[user] + name = MockUser123 + email = mock@fake.local + +[remote "mock-origin"] + url = https://mock-repo.local/fake.git + fetch = +refs/mocks/*:refs/remotes/mock-origin/* + +[branch "mock-branch"] + remote = mock-origin + merge = refs/heads/mock-branch + rebase = never # Unusual setting + +[hooks] + pre-commit = echo 'Mock pre-commit hook executed' + pre-push = echo 'Mock pre-push hook executed' diff --git a/src/commands/sbom/__tests__/payload.test.ts b/src/commands/sbom/__tests__/payload.test.ts index 335219803..ef3bc9fd4 100644 --- a/src/commands/sbom/__tests__/payload.test.ts +++ b/src/commands/sbom/__tests__/payload.test.ts @@ -353,8 +353,10 @@ describe('generation of payload', () => { try { // Configure local git repository const git = simpleGit(tmpdir) - await git.init(['--initial-branch=test-branch']).addRemote('origin', 'https://github.com/TeSt-FaKe-RePo/repo.git') - await git.commit('Initial commit', [], {'--author': '"John Doe "'}) + setupLocalGitConfig(tmpdir) + await git.init() + // eslint-disable-next-line no-null/no-null + await git.commit('Initial commit', [], {'--allow-empty': null}) const sbomFile = './src/commands/sbom/__tests__/fixtures/sbom.1.4.ok.json' const sbomContent = JSON.parse(fs.readFileSync(sbomFile).toString('utf8')) @@ -374,13 +376,12 @@ describe('generation of payload', () => { // Local git repository should be reported expect(payload?.commit.sha).toStrictEqual(expect.any(String)) - expect(payload?.commit.author_name).toStrictEqual('John Doe') - expect(payload?.commit.author_email).toStrictEqual('john.doe@example.com') - expect(payload?.commit.committer_name).toStrictEqual(expect.any(String)) - expect(payload?.commit.committer_email).toStrictEqual(expect.any(String)) - expect(payload?.commit.branch).toStrictEqual('test-branch') - expect(payload?.repository.url).toContain('github.com') - expect(payload?.repository.url).toContain('git@github.com:TeSt-FaKe-RePo/repo.git') + expect(payload?.commit.author_name).toStrictEqual('MockUser123') + expect(payload?.commit.author_email).toStrictEqual('mock@fake.local') + expect(payload?.commit.committer_name).toStrictEqual('MockUser123') + expect(payload?.commit.committer_email).toStrictEqual('mock@fake.local') + expect(payload?.commit.branch).toStrictEqual('mock-branch') + expect(payload?.repository.url).toContain('https://mock-repo.local/fake.git') expect(payload?.dependencies.length).toBe(62) expect(payload?.dependencies[0].name).toBe('stack-cors') expect(payload?.dependencies[0].version).toBe('1.3.0') @@ -406,3 +407,18 @@ describe('generation of payload', () => { expect(getMissingRequiredGitTags(tags)).toHaveLength(7) }) }) + +const getFixtures = (file: string) => { + return path.join('./src/commands/sbom/__tests__/fixtures', file) +} + +const setupLocalGitConfig = (dir: string) => { + const gitDir = path.join(dir, '.git') + if (!fs.existsSync(gitDir)) { + fs.mkdirSync(gitDir, {recursive: true}) + } + + const configFixture = fs.readFileSync(getFixtures('gitconfig'), 'utf8') + const configPath = path.join(gitDir, '/config') + fs.writeFileSync(configPath, configFixture) +} diff --git a/src/helpers/git/format-git-span-data.ts b/src/helpers/git/format-git-span-data.ts index 26e11c676..a44667881 100644 --- a/src/helpers/git/format-git-span-data.ts +++ b/src/helpers/git/format-git-span-data.ts @@ -18,10 +18,10 @@ import {filterSensitiveInfoFromRepository} from '../utils' import {gitAuthorAndCommitter, gitBranch, gitHash, gitMessage, gitRepositoryURL} from './get-git-data' -export const getGitMetadata = async (path?: string): Promise => { +export const getGitMetadata = async (repositoryPath?: string): Promise => { try { const git = simpleGit({ - baseDir: path || process.cwd(), + baseDir: repositoryPath || process.cwd(), binary: 'git', // We are invoking at most 5 git commands at the same time. maxConcurrentProcesses: 5, From 93e3332b9fc6839c0e82f07cad45c12800c833f6 Mon Sep 17 00:00:00 2001 From: piloulacdog Date: Mon, 3 Mar 2025 17:11:39 +0000 Subject: [PATCH 8/9] tests: fix issues for tests ran in CI --- src/commands/sarif/__tests__/upload.test.ts | 24 +++++++------ src/commands/sbom/__tests__/payload.test.ts | 2 +- src/helpers/__tests__/tags.test.ts | 40 ++++++++++++--------- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/commands/sarif/__tests__/upload.test.ts b/src/commands/sarif/__tests__/upload.test.ts index 822c3f093..6c90cfdec 100644 --- a/src/commands/sarif/__tests__/upload.test.ts +++ b/src/commands/sarif/__tests__/upload.test.ts @@ -225,13 +225,18 @@ describe('execute', () => { // Configure local git repository const git = simpleGit(tmpdir) setupLocalGitConfig(tmpdir) + await git.init() + // eslint-disable-next-line no-null/no-null await git.commit('Initial commit', [], {'--allow-empty': null}) + const repositoryParam = `--git-repository=${tmpdir}` + const {context, code} = await runCLI([ - `--git-repository=${tmpdir}`, + repositoryParam, process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder', ]) + const output = context.stdout.toString().split(os.EOL) expect(code).toBe(0) @@ -258,22 +263,21 @@ describe('execute', () => { test('absolute path when passing git repository which does not exist', async () => { const nonExistingGitRepository = '/you/cannot/find/me' - // Pass a git repository which do not exist, command should fail - const {code} = await runCLI([ - `--git-repository=${nonExistingGitRepository}`, - process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder', - ]) + const repositoryParam = `--git-repository=${nonExistingGitRepository}` + + // Pass a git repository which does not exist, command should fail + const {code} = await runCLI([repositoryParam, process.cwd() + '/src/commands/sarif/__tests__/fixtures/subfolder']) expect(code).toBe(1) }) test('single file', async () => { const {context, code} = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/valid-results.sarif']) const output = context.stdout.toString().split(os.EOL) - const path1 = `${process.cwd()}/src/commands/sarif/__tests__/fixtures/valid-results.sarif` + const location = `${process.cwd()}/src/commands/sarif/__tests__/fixtures/valid-results.sarif` expect(code).toBe(0) expect(output[0]).toContain('DRY-RUN MODE ENABLED. WILL NOT UPLOAD SARIF REPORT') expect(output[1]).toContain('Starting upload with concurrency 20.') - expect(output[2]).toContain(`Will upload SARIF report file ${path1}`) + expect(output[2]).toContain(`Will upload SARIF report file ${location}`) expect(output[3]).toContain('Only one upload per commit, env and tool') expect(output[4]).toContain(`Preparing upload for`) expect(output[4]).toContain(`env:ci`) @@ -282,9 +286,9 @@ describe('execute', () => { test('not found file', async () => { const {context, code} = await runCLI([process.cwd() + '/src/commands/sarif/__tests__/fixtures/not-found.sarif']) const output = context.stdout.toString().split(os.EOL) - const path2 = `${process.cwd()}/src/commands/sarif/__tests__/fixtures/not-found.sarif` + const location = `${process.cwd()}/src/commands/sarif/__tests__/fixtures/not-found.sarif` expect(code).toBe(1) - expect(output[0]).toContain(`Cannot find valid SARIF report files to upload in ${path2}`) + expect(output[0]).toContain(`Cannot find valid SARIF report files to upload in ${location}`) expect(output[1]).toContain('Check the files exist and are valid.') }) }) diff --git a/src/commands/sbom/__tests__/payload.test.ts b/src/commands/sbom/__tests__/payload.test.ts index ef3bc9fd4..27c917b47 100644 --- a/src/commands/sbom/__tests__/payload.test.ts +++ b/src/commands/sbom/__tests__/payload.test.ts @@ -404,7 +404,7 @@ describe('generation of payload', () => { // Pass non existing git directory to load git context // It is missing all git tags. const tags = await getSpanTags(config, [], true, nonExistingGitRepository) - expect(getMissingRequiredGitTags(tags)).toHaveLength(7) + expect(getMissingRequiredGitTags(tags).length).toBeGreaterThanOrEqual(1) }) }) diff --git a/src/helpers/__tests__/tags.test.ts b/src/helpers/__tests__/tags.test.ts index de90dd82e..447d0dcc7 100644 --- a/src/helpers/__tests__/tags.test.ts +++ b/src/helpers/__tests__/tags.test.ts @@ -154,6 +154,15 @@ describe('parseMetricsFile', () => { }) describe('getSpanTags', () => { + beforeEach(() => { + // CI defined by default env vars - clearing them is needed for tests to pass in the CI + process.env.GITHUB_ACTIONS = '' + process.env.GITHUB_REPOSITORY = '' + process.env.GITHUB_HEAD_REF = '' + process.env.GITHUB_SHA = '' + process.env.GITHUB_RUN_ID = '' + }) + test('should parse DD_TAGS and DD_ENV environment variables', async () => { process.env.DD_TAGS = 'key1:https://google.com,key2:value2' process.env.DD_ENV = 'ci' @@ -188,25 +197,25 @@ describe('getSpanTags', () => { key2: 'value2', }) }) - test('should prioritized git context corrently', async () => { + test('should prioritized git context accordingly', async () => { const config: DatadogCiConfig = { apiKey: undefined, env: undefined, envVarTags: undefined, } - // define github action env variables + // Define CI env variables process.env.GITHUB_ACTIONS = 'true' process.env.GITHUB_REPOSITORY = 'git@github.com:DataDog/datadog-ci' process.env.GITHUB_HEAD_REF = 'prod' - process.env.GITHUB_SHA = '12000' + process.env.GITHUB_SHA = 'commitSHA' process.env.GITHUB_RUN_ID = '17' // Mock simpleGit (used to read git context) ;(simpleGit as jest.Mock).mockImplementation(() => ({ branch: () => ({current: 'main'}), listRemote: async (git: any): Promise => 'https://www.github.com/datadog/safe-repository', - revparse: () => 'commitSHA', + revparse: () => 'mockSHA', show: (input: string[]) => { if (input[1] === '--format=%s') { return 'commit message' @@ -216,31 +225,30 @@ describe('getSpanTags', () => { }, })) - // By default, the 'git' tags reported are the github action + // By default, the 'git' tags reported are the CI env variables let spanTags: SpanTags = await getSpanTags(config, [], true) - expect(spanTags).toMatchObject({ - 'ci.pipeline.id': '17', - 'git.repository_url': 'undefined/git@github.com:DataDog/datadog-ci.git', - 'git.branch': 'prod', - 'git.commit.sha': '12000', - }) + expect(spanTags['ci.pipeline.id']).toEqual('17') + expect(spanTags['git.repository_url']).toContain('DataDog/datadog-ci') + expect(spanTags['git.branch']).toEqual('prod') + expect(spanTags['git.commit.sha']).toEqual('commitSHA') - // re-query span tags while specifying a git directory + // re-query span tags while specifying a git directory: // if should prefer the git directory over env variables. - spanTags = await getSpanTags(config, [], true, 'path/to/mockedSimpleGit') - // git tags are coming from simpleGit + spanTags = await getSpanTags(config, [], true, 'path/to/mocked/SimpleGit') expect(spanTags).toMatchObject({ 'ci.pipeline.id': '17', 'git.repository_url': 'https://www.github.com/datadog/safe-repository', 'git.branch': 'main', - 'git.commit.sha': 'commitSHA', + 'git.commit.sha': 'mockSHA', }) + // Configuring DD_GIT* overrides process.env.DD_GIT_REPOSITORY_URL = 'https://www.github.com/datadog/not-so-safe-repository' process.env.DD_GIT_BRANCH = 'staging' process.env.DD_GIT_COMMIT_SHA = 'override' + + // git tags are coming from overrides spanTags = await getSpanTags(config, [], true, 'path/to/mockedSimpleGit') - // git tags are coming from env override expect(spanTags).toMatchObject({ 'ci.pipeline.id': '17', 'git.repository_url': 'https://www.github.com/datadog/not-so-safe-repository', From 5a1c0bc8b4f25588ae97f829e1f26624fa2600f2 Mon Sep 17 00:00:00 2001 From: piloulacdog Date: Wed, 5 Mar 2025 13:51:08 +0000 Subject: [PATCH 9/9] fix: address copy feedbacks in README.md --- src/commands/sarif/README.md | 4 ++-- src/commands/sbom/README.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/commands/sarif/README.md b/src/commands/sarif/README.md index 4ef46b8d3..65c4c6280 100644 --- a/src/commands/sarif/README.md +++ b/src/commands/sarif/README.md @@ -25,8 +25,8 @@ The positional arguments are the directories or file paths in which the SARIF re - `--max-concurrency` (default: `20`): number of concurrent uploads to the API. - `--dry-run` (default: `false`): runs the command without the final upload step. All other checks are performed. - `--no-verify` (default: `false`): runs the command without performing report validation on the CLI. -- `--no-ci-tags` (default: `false`): ignore automatic detection of continuous integration environment variables. -- `--git-repository` (default: `current working directory`): reports git environment context from specified repository. +- `--no-ci-tags` (default: `false`): ignore the automatic detection of continuous integration environment variables. +- `--git-repository` (default: `current working directory`): reports git environment context from the specified repository. ### Environment variables diff --git a/src/commands/sbom/README.md b/src/commands/sbom/README.md index 25c91e460..42a74586d 100644 --- a/src/commands/sbom/README.md +++ b/src/commands/sbom/README.md @@ -18,8 +18,8 @@ datadog-ci sbom upload [--env] [--no-ci-tags] [--git-repository] [--debug]