Skip to content

Commit

Permalink
validate CI tags before sending them (#3310)
Browse files Browse the repository at this point in the history
* ciapp-3571: add url validator

* ciapp-3571: add url validator and change tests from ci-spec

* fix variables to follow camelcase

* Remove valid import

Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com>

* Remove unnecessary check

Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com>

* Remove unnecessary check for CI_PIPELINE_URL

Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com>

* Format typo

Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com>

* Use validateMetadata return directly

Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com>

* use new ci-spec JSONs

* fix spacing and sort imports

* add test to validateMetadata

* Update JSONs and refactor URL checking

* Update JSONs and refactor URL checking

* Add URL validation tests

* Apply suggestions from code review

Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com>

* Add tag to test suite

---------

Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com>
  • Loading branch information
2 people authored and Stephen Belanger committed Jul 11, 2023
1 parent 8639ffb commit 42c2433
Show file tree
Hide file tree
Showing 15 changed files with 644 additions and 574 deletions.
2 changes: 1 addition & 1 deletion packages/dd-trace/src/plugins/util/ci.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function filterSensitiveInfoFromRepository (repositoryUrl) {

return `${protocol}//${hostname}${pathname}`
} catch (e) {
return repositoryUrl
return ''
}
}

Expand Down
45 changes: 41 additions & 4 deletions packages/dd-trace/src/plugins/util/test.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
const path = require('path')
const fs = require('fs')
const { URL } = require('url')
const log = require('../../log')

const istanbul = require('istanbul-lib-coverage')
const ignore = require('ignore')

const { getGitMetadata } = require('./git')
const { getUserProviderGitMetadata } = require('./user-provided-git')
const { getUserProviderGitMetadata, validateGitRepositoryUrl, validateGitCommitSha } = require('./user-provided-git')
const { getCIMetadata } = require('./ci')
const { getRuntimeAndOSMetadata } = require('./env')
const {
Expand All @@ -16,7 +18,8 @@ const {
GIT_COMMIT_AUTHOR_EMAIL,
GIT_COMMIT_AUTHOR_NAME,
GIT_COMMIT_MESSAGE,
CI_WORKSPACE_PATH
CI_WORKSPACE_PATH,
CI_PIPELINE_URL
} = require('./tags')
const id = require('../../id')

Expand Down Expand Up @@ -104,7 +107,8 @@ module.exports = {
mergeCoverage,
fromCoverageMapToCoverage,
getTestLineStart,
getCallSites
getCallSites,
removeInvalidMetadata
}

// Returns pkg manager and its version, separated by '-', e.g. npm-8.15.0 or yarn-1.22.19
Expand All @@ -116,6 +120,39 @@ function getPkgManager () {
}
}

function validateUrl (url) {
try {
const urlObject = new URL(url)
return (urlObject.protocol === 'https:' || urlObject.protocol === 'http:')
} catch (e) {
return false
}
}

function removeInvalidMetadata (metadata) {
return Object.keys(metadata).reduce((filteredTags, tag) => {
if (tag === GIT_REPOSITORY_URL) {
if (!validateGitRepositoryUrl(metadata[GIT_REPOSITORY_URL])) {
log.error('DD_GIT_REPOSITORY_URL must be a valid URL')
return filteredTags
}
}
if (tag === GIT_COMMIT_SHA) {
if (!validateGitCommitSha(metadata[GIT_COMMIT_SHA])) {
log.error('DD_GIT_COMMIT_SHA must be a full-length git SHA')
return filteredTags
}
}
if (tag === CI_PIPELINE_URL) {
if (!validateUrl(metadata[CI_PIPELINE_URL])) {
return filteredTags
}
}
filteredTags[tag] = metadata[tag]
return filteredTags
}, {})
}

function getTestEnvironmentMetadata (testFramework, config) {
// TODO: eventually these will come from the tracer (generally available)
const ciMetadata = getCIMetadata()
Expand Down Expand Up @@ -155,7 +192,7 @@ function getTestEnvironmentMetadata (testFramework, config) {
if (config && config.service) {
metadata['service.name'] = config.service
}
return metadata
return removeInvalidMetadata(metadata)
}

function getTestParametersString (parametersByTestName, testName) {
Expand Down
23 changes: 1 addition & 22 deletions packages/dd-trace/src/plugins/util/user-provided-git.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const {
} = require('./tags')

const { normalizeRef } = require('./ci')
const log = require('../../log')
const { URL } = require('url')

function removeEmptyValues (tags) {
Expand Down Expand Up @@ -53,25 +52,6 @@ function validateGitCommitSha (gitCommitSha) {
return isValidSha1 || isValidSha256
}

function removeInvalidGitMetadata (metadata) {
return Object.keys(metadata).reduce((filteredTags, tag) => {
if (tag === GIT_REPOSITORY_URL) {
if (!validateGitRepositoryUrl(metadata[GIT_REPOSITORY_URL])) {
log.error('DD_GIT_REPOSITORY_URL must be a valid URL')
return filteredTags
}
}
if (tag === GIT_COMMIT_SHA) {
if (!validateGitCommitSha(metadata[GIT_COMMIT_SHA])) {
log.error('DD_GIT_COMMIT_SHA must be a full-length git SHA')
return filteredTags
}
}
filteredTags[tag] = metadata[tag]
return filteredTags
}, {})
}

function getUserProviderGitMetadata () {
const {
DD_GIT_COMMIT_SHA,
Expand All @@ -95,7 +75,7 @@ function getUserProviderGitMetadata () {
tag = normalizeRef(DD_GIT_BRANCH)
}

const metadata = removeEmptyValues({
return removeEmptyValues({
[GIT_COMMIT_SHA]: DD_GIT_COMMIT_SHA,
[GIT_BRANCH]: branch,
[GIT_REPOSITORY_URL]: filterSensitiveInfoFromRepository(DD_GIT_REPOSITORY_URL),
Expand All @@ -108,7 +88,6 @@ function getUserProviderGitMetadata () {
[GIT_COMMIT_AUTHOR_EMAIL]: DD_GIT_COMMIT_AUTHOR_EMAIL,
[GIT_COMMIT_AUTHOR_DATE]: DD_GIT_COMMIT_AUTHOR_DATE
})
return removeInvalidGitMetadata(metadata)
}

module.exports = { getUserProviderGitMetadata, validateGitRepositoryUrl, validateGitCommitSha }
Loading

0 comments on commit 42c2433

Please sign in to comment.