Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 14 additions & 1 deletion packages/dd-trace/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

const fs = require('fs')
const os = require('os')
const uuid = require('crypto-randomuuid')
const URL = require('url').URL
const log = require('./log')
const pkg = require('./pkg')
const coalesce = require('koalas')
const tagger = require('./tagger')
const { isTrue, isFalse } = require('./util')
const uuid = require('crypto-randomuuid')
const { GIT_REPOSITORY_URL, GIT_COMMIT_SHA } = require('./plugins/util/tags')
const { getGitMetadataFromGitProperties } = require('./git_properties')

const fromEntries = Object.fromEntries || (entries =>
entries.reduce((obj, [k, v]) => Object.assign(obj, { [k]: v }), {}))
Expand Down Expand Up @@ -539,6 +540,18 @@ ken|consumer_?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?)
process.env.DD_GIT_COMMIT_SHA,
this.tags[GIT_COMMIT_SHA]
)
if (!this.repositoryUrl || !this.commitSHA) {
Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker for this PR, but this is starting to be a lot of code in config.js which should probably be moved out at some point.

const DD_GIT_PROPERTIES_FILE = coalesce(
process.env.DD_GIT_PROPERTIES_FILE,
`${process.cwd()}/git.properties`
)
const gitPropertiesString = maybeFile(DD_GIT_PROPERTIES_FILE)
if (gitPropertiesString) {
const { commitSHA, repositoryUrl } = getGitMetadataFromGitProperties(gitPropertiesString)
this.commitSHA = this.commitSHA || commitSHA
this.repositoryUrl = this.repositoryUrl || repositoryUrl
}
}
}

this.stats = {
Expand Down
32 changes: 32 additions & 0 deletions packages/dd-trace/src/git_properties.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
const commitSHARegex = /git\.commit\.sha=([a-f\d]{40})/
const repositoryUrlRegex = /git\.repository_url=([\w\d:@/.-]+)/

function getGitMetadataFromGitProperties (gitPropertiesString) {
if (!gitPropertiesString) {
return {}
}
const commitSHAMatch = gitPropertiesString.match(commitSHARegex)
const repositoryUrlMatch = gitPropertiesString.match(repositoryUrlRegex)

const repositoryUrl = repositoryUrlMatch ? repositoryUrlMatch[1] : undefined
let parsedUrl = repositoryUrl

if (repositoryUrl) {
try {
// repository URLs can contain username and password, so we want to filter those out
parsedUrl = new URL(repositoryUrl)
if (parsedUrl.password) {
parsedUrl = `${parsedUrl.origin}${parsedUrl.pathname}`
}
} catch (e) {
// if protocol isn't https, no password will be used
}
}

return {
commitSHA: commitSHAMatch ? commitSHAMatch[1] : undefined,
repositoryUrl: parsedUrl
}
}

module.exports = { getGitMetadataFromGitProperties }
72 changes: 71 additions & 1 deletion packages/dd-trace/test/config.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('Config', () => {
const BLOCKED_TEMPLATE_HTML = readFileSync(BLOCKED_TEMPLATE_HTML_PATH, { encoding: 'utf8' })
const BLOCKED_TEMPLATE_JSON_PATH = require.resolve('./fixtures/config/appsec-blocked-template.json')
const BLOCKED_TEMPLATE_JSON = readFileSync(BLOCKED_TEMPLATE_JSON_PATH, { encoding: 'utf8' })
const DD_GIT_PROPERTIES_FILE = require.resolve('./fixtures/config/git.properties')

beforeEach(() => {
pkg = {
Expand Down Expand Up @@ -918,7 +919,7 @@ describe('Config', () => {
}
})

expect(log.error).to.be.calledThrice
expect(log.error).to.be.callCount(4)
expect(log.error.firstCall).to.have.been.calledWithExactly(error)
expect(log.error.secondCall).to.have.been.calledWithExactly(error)
expect(log.error.thirdCall).to.have.been.calledWithExactly(error)
Expand Down Expand Up @@ -1058,4 +1059,73 @@ describe('Config', () => {
})
})
})

context('sci embedding', () => {
const DUMMY_COMMIT_SHA = 'b7b5dfa992008c77ab3f8a10eb8711e0092445b0'
const DUMMY_REPOSITORY_URL = '[email protected]:DataDog/dd-trace-js.git'
let ddTags
beforeEach(() => {
ddTags = process.env.DD_TAGS
})
afterEach(() => {
delete process.env.DD_GIT_PROPERTIES_FILE
delete process.env.DD_GIT_COMMIT_SHA
delete process.env.DD_GIT_REPOSITORY_URL
delete process.env.DD_TRACE_GIT_METADATA_ENABLED
process.env.DD_TAGS = ddTags
})
it('reads DD_GIT_* env vars', () => {
process.env.DD_GIT_COMMIT_SHA = DUMMY_COMMIT_SHA
process.env.DD_GIT_REPOSITORY_URL = DUMMY_REPOSITORY_URL
const config = new Config({})
expect(config).to.have.property('commitSHA', DUMMY_COMMIT_SHA)
expect(config).to.have.property('repositoryUrl', DUMMY_REPOSITORY_URL)
})
it('reads DD_TAGS env var', () => {
process.env.DD_TAGS = `git.commit.sha:${DUMMY_COMMIT_SHA},git.repository_url:${DUMMY_REPOSITORY_URL}`
process.env.DD_GIT_REPOSITORY_URL = DUMMY_REPOSITORY_URL
const config = new Config({})
expect(config).to.have.property('commitSHA', DUMMY_COMMIT_SHA)
expect(config).to.have.property('repositoryUrl', DUMMY_REPOSITORY_URL)
})
it('reads git.properties if it is available', () => {
process.env.DD_GIT_PROPERTIES_FILE = DD_GIT_PROPERTIES_FILE
const config = new Config({})
expect(config).to.have.property('commitSHA', '4e7da8069bcf5ffc8023603b95653e2dc99d1c7d')
expect(config).to.have.property('repositoryUrl', DUMMY_REPOSITORY_URL)
})
it('does not crash if git.properties is not available', () => {
process.env.DD_GIT_PROPERTIES_FILE = '/does/not/exist'
const config = new Config({})
expect(config).to.have.property('commitSHA', undefined)
expect(config).to.have.property('repositoryUrl', undefined)
})
it('does not read git.properties if env vars are passed', () => {
process.env.DD_GIT_PROPERTIES_FILE = DD_GIT_PROPERTIES_FILE
process.env.DD_GIT_COMMIT_SHA = DUMMY_COMMIT_SHA
process.env.DD_GIT_REPOSITORY_URL = 'https://github.com:env-var/dd-trace-js.git'
const config = new Config({})
expect(config).to.have.property('commitSHA', DUMMY_COMMIT_SHA)
expect(config).to.have.property('repositoryUrl', 'https://github.com:env-var/dd-trace-js.git')
})
it('still reads git.properties if one of the env vars is missing', () => {
process.env.DD_GIT_PROPERTIES_FILE = DD_GIT_PROPERTIES_FILE
process.env.DD_GIT_COMMIT_SHA = DUMMY_COMMIT_SHA
const config = new Config({})
expect(config).to.have.property('commitSHA', DUMMY_COMMIT_SHA)
expect(config).to.have.property('repositoryUrl', DUMMY_REPOSITORY_URL)
})
it('reads git.properties and filters out credentials', () => {
process.env.DD_GIT_PROPERTIES_FILE = require.resolve('./fixtures/config/git.properties.credentials')
const config = new Config({})
expect(config).to.have.property('commitSHA', '4e7da8069bcf5ffc8023603b95653e2dc99d1c7d')
expect(config).to.have.property('repositoryUrl', 'https://github.com/datadog/dd-trace-js')
})
it('does not read git metadata if DD_TRACE_GIT_METADATA_ENABLED is false', () => {
process.env.DD_TRACE_GIT_METADATA_ENABLED = 'false'
const config = new Config({})
expect(config).not.to.have.property('commitSHA')
expect(config).not.to.have.property('repositoryUrl')
})
})
})
2 changes: 2 additions & 0 deletions packages/dd-trace/test/fixtures/config/git.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
git.commit.sha=4e7da8069bcf5ffc8023603b95653e2dc99d1c7d
[email protected]:DataDog/dd-trace-js.git
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
git.commit.sha=4e7da8069bcf5ffc8023603b95653e2dc99d1c7d
git.repository_url=https://username:[email protected]/datadog/dd-trace-js
[email protected]
49 changes: 49 additions & 0 deletions packages/dd-trace/test/git_properties.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
require('./setup/tap')

const { getGitMetadataFromGitProperties } = require('../src/git_properties')

describe('git_properties', () => {
context('getGitMetadataFromGitProperties', () => {
it('reads commit SHA and repository URL', () => {
const { commitSHA, repositoryUrl } = getGitMetadataFromGitProperties(`
git.commit.sha=4e7da8069bcf5ffc8023603b95653e2dc99d1c7d
[email protected]:DataDog/dd-trace-js.git
`)
expect(commitSHA).to.equal('4e7da8069bcf5ffc8023603b95653e2dc99d1c7d')
expect(repositoryUrl).to.equal('[email protected]:DataDog/dd-trace-js.git')
})
it('filters out credentials', () => {
const { commitSHA, repositoryUrl } = getGitMetadataFromGitProperties(`
git.commit.sha=4e7da8069bcf5ffc8023603b95653e2dc99d1c7d
git.repository_url=https://username:[email protected]/datadog/dd-trace-js.git
`)
expect(commitSHA).to.equal('4e7da8069bcf5ffc8023603b95653e2dc99d1c7d')
expect(repositoryUrl).to.equal('https://github.com/datadog/dd-trace-js.git')
})
it('ignores other fields', () => {
const { commitSHA, repositoryUrl } = getGitMetadataFromGitProperties(`
git.commit.sha=4e7da8069bcf5ffc8023603b95653e2dc99d1c7d
[email protected]:DataDog/dd-trace-js.git
[email protected]
`)
expect(commitSHA).to.equal('4e7da8069bcf5ffc8023603b95653e2dc99d1c7d')
expect(repositoryUrl).to.equal('[email protected]:DataDog/dd-trace-js.git')
})
it('ignores badly formatted files', () => {
const { commitSHA, repositoryUrl } = getGitMetadataFromGitProperties(`
git.commit.sha=; rm -rf ;
git.repository_url=; rm -rf ;
`)
expect(commitSHA).to.equal(undefined)
expect(repositoryUrl).to.equal(undefined)
})
it('does not crash with empty files', () => {
const emptyStringResult = getGitMetadataFromGitProperties('')
expect(emptyStringResult.commitSHA).to.equal(undefined)
expect(emptyStringResult.repositoryUrl).to.equal(undefined)
const undefinedResult = getGitMetadataFromGitProperties(undefined)
expect(undefinedResult.commitSHA).to.equal(undefined)
expect(undefinedResult.repositoryUrl).to.equal(undefined)
})
})
})
2 changes: 1 addition & 1 deletion packages/dd-trace/test/serverless.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('Serverless', () => {

// trying to spawn with an invalid path will return a non-descriptive error, so we want to catch
// invalid paths and log our own error.
expect(logErrorSpy).to.have.been.calledOnceWith(
expect(logErrorSpy).to.have.been.calledWith(
'Serverless Mini Agent did not start. Could not find mini agent binary.'
)
existsSyncStub.returns(true)
Expand Down