Skip to content

Improve mandatory git field checks #1506

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.

Sign up for GitHub

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

Merged
dastrong merged 1 commit into from
Dec 19, 2024
Merged

Improve mandatory git field checks #1506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions src/commands/sarif/upload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {toBoolean} from '../../helpers/env'
import {enableFips} from '../../helpers/fips'
import {SpanTags} from '../../helpers/interfaces'
import {retryRequest} from '../../helpers/retry'
import {GIT_SHA, getSpanTags, REQUIRED_GIT_TAGS} from '../../helpers/tags'
import {GIT_SHA, getSpanTags, getMissingRequiredGitTags} from '../../helpers/tags'
import {buildPath} from '../../helpers/utils'
import * as validation from '../../helpers/validation'

Expand Down Expand Up @@ -100,14 +100,7 @@ export class UploadSarifReportCommand extends Command {
const spanTags = await getSpanTags(this.config, this.tags, !this.noCiTags)

// Gather any missing mandatory git fields to display to the user
const missingGitFields = Object.entries(spanTags).reduce((acc: string[], [tag, value]) => {
if (REQUIRED_GIT_TAGS[tag] && !value) {
acc.push(tag)
}

return acc
}, [])

const missingGitFields = getMissingRequiredGitTags(spanTags)
if (missingGitFields.length > 0) {
this.context.stdout.write(renderMissingTags(missingGitFields))

Expand Down
11 changes: 2 additions & 9 deletions src/commands/sbom/upload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {Command, Option} from 'clipanion'
import {FIPS_ENV_VAR, FIPS_IGNORE_ERROR_ENV_VAR} from '../../constants'
import {toBoolean} from '../../helpers/env'
import {enableFips} from '../../helpers/fips'
import {GIT_SHA, getSpanTags, GIT_REPOSITORY_URL, REQUIRED_GIT_TAGS} from '../../helpers/tags'
import {GIT_SHA, GIT_REPOSITORY_URL, getSpanTags, getMissingRequiredGitTags} from '../../helpers/tags'

import {renderMissingTags} from '../sarif/renderer'

Expand Down Expand Up @@ -98,14 +98,7 @@ export class UploadSbomCommand extends Command {
const tags = await getSpanTags(this.config, this.tags, !this.noCiTags)

// Gather any missing mandatory git fields to display to the user
const missingGitFields = Object.entries(tags).reduce((acc: string[], [tag, value]) => {
if (REQUIRED_GIT_TAGS[tag] && !value) {
acc.push(tag)
}

return acc
}, [])

const missingGitFields = getMissingRequiredGitTags(tags)
if (missingGitFields.length > 0) {
this.context.stdout.write(renderMissingTags(missingGitFields))

Expand Down
183 changes: 182 additions & 1 deletion src/helpers/__tests__/tags.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,24 @@
import {BaseContext} from 'clipanion'
import simpleGit from 'simple-git'

import {SpanTags} from '../interfaces'
import {parseTags, parseMetrics, getSpanTags, parseTagsFile, parseMeasuresFile} from '../tags'
import {
parseTags,
parseMetrics,
getSpanTags,
parseTagsFile,
parseMeasuresFile,
getMissingRequiredGitTags,
GIT_REPOSITORY_URL,
GIT_BRANCH,
GIT_SHA,
GIT_COMMIT_AUTHOR_EMAIL,
GIT_COMMIT_AUTHOR_NAME,
GIT_COMMIT_COMMITTER_EMAIL,
GIT_COMMIT_COMMITTER_NAME,
} from '../tags'

jest.mock('simple-git')

const fixturesPath = './src/helpers/__tests__/tags-fixtures'
const createMockContext = (): BaseContext => {
Expand Down Expand Up @@ -171,3 +188,167 @@ describe('getSpanTags', () => {
})
})
})

describe('sarif and sbom upload required git tags', () => {
// throwError will be used to simulate an error being thrown from simple-git
// commands and is used to condense the tests code.
const throwError = () => {
throw new Error()
}

// Reset all env vars before each test
beforeEach(() => {
// User defined env vars
process.env.DD_GIT_BRANCH = ''
process.env.DD_GIT_COMMIT_SHA = ''
process.env.DD_GIT_COMMIT_AUTHOR_EMAIL = ''
process.env.DD_GIT_COMMIT_AUTHOR_NAME = ''
process.env.DD_GIT_COMMIT_COMMITTER_EMAIL = ''
process.env.DD_GIT_COMMIT_COMMITTER_NAME = ''
// CI defined env vars - needed for tests to pass in the CI
process.env.GITHUB_SHA = ''
process.env.GITHUB_HEAD_REF = ''
process.env.GITHUB_REF = ''
})

test('should be valid as we have all required git fields', async () => {
;(simpleGit as jest.Mock).mockImplementation(() => ({
branch: () => ({current: 'main'}),
listRemote: async (git: any): Promise<string> => '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'
},
}))
const spanTags: SpanTags = await getSpanTags(
{
apiKey: undefined,
env: undefined,
envVarTags: undefined,
},
[],
true
)
const missingTags = getMissingRequiredGitTags(spanTags)
expect(missingTags).toHaveLength(0)
})

test('should be valid when all fields are specified using env vars', async () => {
;(simpleGit as jest.Mock).mockImplementation(() => ({
branch: throwError,
listRemote: throwError,
revparse: throwError,
show: throwError,
}))

process.env.DD_GIT_REPOSITORY_URL = 'https://www.github.com/datadog/safe-repository'
process.env.DD_GIT_BRANCH = 'main'
process.env.DD_GIT_COMMIT_SHA = 'commitSHA'
process.env.DD_GIT_COMMIT_AUTHOR_EMAIL = 'authorEmail'
process.env.DD_GIT_COMMIT_AUTHOR_NAME = 'authorName'
process.env.DD_GIT_COMMIT_COMMITTER_EMAIL = 'committerEmail'
process.env.DD_GIT_COMMIT_COMMITTER_NAME = 'committerName'

const spanTags: SpanTags = await getSpanTags(
{
apiKey: undefined,
env: undefined,
envVarTags: undefined,
},
[],
true
)
const missingTags = getMissingRequiredGitTags(spanTags)
expect(missingTags).toHaveLength(0)
})

test('should not be valid when missing an env var', async () => {
;(simpleGit as jest.Mock).mockImplementation(() => ({
branch: throwError,
listRemote: throwError,
revparse: throwError,
show: throwError,
}))

process.env.DD_GIT_REPOSITORY_URL = 'https://www.github.com/datadog/safe-repository'
process.env.DD_GIT_BRANCH = 'main'
// missing DD_GIT_COMMIT_SHA
process.env.DD_GIT_COMMIT_AUTHOR_EMAIL = 'authorEmail'
process.env.DD_GIT_COMMIT_AUTHOR_NAME = 'authorName'
process.env.DD_GIT_COMMIT_COMMITTER_EMAIL = 'committerEmail'
process.env.DD_GIT_COMMIT_COMMITTER_NAME = 'committerName'

const spanTags: SpanTags = await getSpanTags(
{
apiKey: undefined,
env: undefined,
envVarTags: undefined,
},
[],
true
)
const missingTags = getMissingRequiredGitTags(spanTags)
expect(missingTags).toHaveLength(1)
expect(missingTags).toContain(GIT_SHA)
})

test('should be invalid when no git metadata is there (no .git)', async () => {
;(simpleGit as jest.Mock).mockImplementation(() => ({
branch: throwError,
listRemote: throwError,
revparse: throwError,
show: throwError,
}))
const spanTags: SpanTags = await getSpanTags(
{
apiKey: undefined,
env: undefined,
envVarTags: undefined,
},
[],
true
)
const missingTags = getMissingRequiredGitTags(spanTags)
expect(missingTags).toHaveLength(6)
expect(missingTags).toContain(GIT_BRANCH)
expect(missingTags).toContain(GIT_SHA)
expect(missingTags).toContain(GIT_COMMIT_AUTHOR_EMAIL)
expect(missingTags).toContain(GIT_COMMIT_AUTHOR_NAME)
expect(missingTags).toContain(GIT_COMMIT_COMMITTER_EMAIL)
expect(missingTags).toContain(GIT_COMMIT_COMMITTER_NAME)
})

test('should be invalid when an env var overrides a value retrieved from git', async () => {
;(simpleGit as jest.Mock).mockImplementation(() => ({
branch: () => ({current: 'main'}),
listRemote: async (git: any): Promise<string> => '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'
},
}))

process.env.DD_GIT_BRANCH = ' '

const spanTags: SpanTags = await getSpanTags(
{
apiKey: undefined,
env: undefined,
envVarTags: undefined,
},
[],
true
)
const missingTags = getMissingRequiredGitTags(spanTags)
expect(missingTags).toHaveLength(1)
expect(missingTags).toContain(GIT_BRANCH)
})
})
35 changes: 26 additions & 9 deletions src/helpers/tags.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {isFile} from '../commands/junit/utils'
import {getCISpanTags} from './ci'
import {DatadogCiConfig} from './config'
import {getGitMetadata} from './git/format-git-span-data'
import {SpanTags} from './interfaces'
import {SpanTag, SpanTags} from './interfaces'
import {getUserGitSpanTags} from './user-provided-git'

export const CI_PIPELINE_URL = 'ci.pipeline.url'
Expand Down Expand Up @@ -205,14 +205,31 @@ export const parseMeasuresFile = (
/**
* These are required git tags for the following commands: sarif and sbom.
*/
export const REQUIRED_GIT_TAGS: Record<string, boolean> = {
[GIT_REPOSITORY_URL]: true,
[GIT_BRANCH]: true,
[GIT_SHA]: true,
[GIT_COMMIT_AUTHOR_EMAIL]: true,
[GIT_COMMIT_AUTHOR_NAME]: true,
[GIT_COMMIT_COMMITTER_EMAIL]: true,
[GIT_COMMIT_COMMITTER_NAME]: true,
export const REQUIRED_GIT_TAGS: SpanTag[] = [
GIT_REPOSITORY_URL,
GIT_BRANCH,
GIT_SHA,
GIT_COMMIT_AUTHOR_EMAIL,
GIT_COMMIT_AUTHOR_NAME,
GIT_COMMIT_COMMITTER_EMAIL,
GIT_COMMIT_COMMITTER_NAME,
]

/**
* A utility to determine which required git tags are missing.
* @param tags - the tags to check
* @returns an array of the missing required git tags (ex. ['git.repository_url', 'git.branch'])
*/
export const getMissingRequiredGitTags = (tags: SpanTags): string[] => {
const missingTags = REQUIRED_GIT_TAGS.reduce((acc: string[], tag: SpanTag) => {
if (!tags[tag] || (tags[tag] as string).trim() === '') {
acc.push(tag)
}

return acc
}, [])

return missingTags
}

/**
Expand Down