more and better tests

Author: juan-fernandez

packages/dd-trace/src/git_properties.js

@@ -2,12 +2,30 @@ 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] : null,
-    repositoryUrl: repositoryUrlMatch ? repositoryUrlMatch[1] : null
+    commitSHA: commitSHAMatch ? commitSHAMatch[1] : undefined,
+    repositoryUrl: parsedUrl
   }
 }
 

packages/dd-trace/test/config.spec.js

@@ -1061,14 +1061,64 @@ 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
+      process.env.DD_TAGS = ddTags
     })
-    it('reads git.properties', () => {
+    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', '[email protected]:DataDog/dd-trace-js.git')
+      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')
     })
   })
 })

packages/dd-trace/test/fixtures/config/git.properties.credentials

@@ -0,0 +1,3 @@
+git.commit.sha=4e7da8069bcf5ffc8023603b95653e2dc99d1c7d
+git.repository_url=https://username:[email protected]/datadog/dd-trace-js
+[email protected]

packages/dd-trace/test/git_properties.spec.js

@@ -3,7 +3,7 @@ require('./setup/tap')
 const { getGitMetadataFromGitProperties } = require('../src/git_properties')
 
 describe('git_properties', () => {
-  describe('getGitMetadataFromGitProperties', () => {
+  context('getGitMetadataFromGitProperties', () => {
     it('reads commit SHA and repository URL', () => {
       const { commitSHA, repositoryUrl } = getGitMetadataFromGitProperties(`
 git.commit.sha=4e7da8069bcf5ffc8023603b95653e2dc99d1c7d
@@ -12,5 +12,38 @@ [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)
+    })
   })
 })