Simplify requesting short-lived Develocity access tokens (#259)

- Always fetch a token for every hostname in the access key
- Use any tokens that are successfully fetched
- Retain access key if no tokens can be fetched

sources/src/develocity/build-scan.ts

@@ -23,16 +23,7 @@ export async function setup(config: BuildScanConfig): Promise<void> {
     maybeExportVariableNotEmpty('GRADLE_PLUGIN_REPOSITORY_USERNAME', config.getGradlePluginRepositoryUsername())
     maybeExportVariableNotEmpty('GRADLE_PLUGIN_REPOSITORY_PASSWORD', config.getGradlePluginRepositoryPassword())
 
-    setupToken(
+    setupToken(config.getDevelocityAccessKey(), config.getDevelocityTokenExpiry())
-        config.getDevelocityAccessKey(),
-        config.getDevelocityTokenExpiry(),
-        getEnv('DEVELOCITY_ENFORCE_URL'),
-        getEnv('DEVELOCITY_URL')
-    )
-}
-
-function getEnv(variableName: string): string | undefined {
-    return process.env[variableName]
 }
 
 function maybeExportVariable(variableName: string, value: unknown): void {

sources/src/develocity/short-lived-token.ts

@@ -3,26 +3,21 @@ import * as core from '@actions/core'
 import {BuildScanConfig} from '../configuration'
 import {recordDeprecation} from '../deprecation-collector'
 
-export async function setupToken(
+export async function setupToken(develocityAccessKey: string, develocityTokenExpiry: string): Promise<void> {
-    develocityAccessKey: string,
-    develocityTokenExpiry: string,
-    enforceUrl: string | undefined,
-    develocityUrl: string | undefined
-): Promise<void> {
     if (develocityAccessKey) {
         try {
             core.debug('Fetching short-lived token...')
-            const tokens = await getToken(enforceUrl, develocityUrl, develocityAccessKey, develocityTokenExpiry)
+            const tokens = await getToken(develocityAccessKey, develocityTokenExpiry)
             if (tokens != null && !tokens.isEmpty()) {
                 core.debug(`Got token(s), setting the access key env vars`)
                 const token = tokens.raw()
                 core.setSecret(token)
                 exportAccessKeyEnvVars(token)
             } else {
-                handleMissingAccessTokenWithDeprecationWarning()
+                handleMissingAccessToken()
             }
         } catch (e) {
-            handleMissingAccessTokenWithDeprecationWarning()
+            handleMissingAccessToken()
             core.warning(`Failed to fetch short-lived token, reason: ${e}`)
         }
     }
@@ -34,60 +29,35 @@ function exportAccessKeyEnvVars(value: string): void {
     )
 }
 
-function handleMissingAccessTokenWithDeprecationWarning(): void {
+function handleMissingAccessToken(): void {
+    core.warning(`Failed to fetch short-lived token for Develocity`)
+
     if (process.env[BuildScanConfig.GradleEnterpriseAccessKeyEnvVar]) {
         // We do not clear the GRADLE_ENTERPRISE_ACCESS_KEY env var in v3, to let the users upgrade to DV 2024.1
         recordDeprecation(`The ${BuildScanConfig.GradleEnterpriseAccessKeyEnvVar} env var is deprecated`)
     }
     if (process.env[BuildScanConfig.DevelocityAccessKeyEnvVar]) {
-        core.warning(`Failed to fetch short-lived token, using Develocity Access key`)
+        core.warning(`The ${BuildScanConfig.DevelocityAccessKeyEnvVar} env var should be mapped to a short-lived token`)
     }
 }
 
-export async function getToken(
+export async function getToken(accessKey: string, expiry: string): Promise<DevelocityAccessCredentials | null> {
-    enforceUrl: string | undefined,
-    serverUrl: string | undefined,
-    accessKey: string,
-    expiry: string
-): Promise<DevelocityAccessCredentials | null> {
     const empty: Promise<DevelocityAccessCredentials | null> = new Promise(r => r(null))
     const develocityAccessKey = DevelocityAccessCredentials.parse(accessKey)
     const shortLivedTokenClient = new ShortLivedTokenClient()
 
-    async function promiseError(message: string): Promise<DevelocityAccessCredentials | null> {
-        return new Promise((resolve, reject) => reject(new Error(message)))
-    }
-
     if (develocityAccessKey == null) {
         return empty
     }
-    if (enforceUrl === 'true' || develocityAccessKey.isSingleKey()) {
-        if (!serverUrl) {
-            return promiseError('Develocity Server URL not configured')
-        }
-        const hostname = extractHostname(serverUrl)
-        if (hostname == null) {
-            return promiseError('Could not extract hostname from Develocity server URL')
-        }
-        const hostAccessKey = develocityAccessKey.forHostname(hostname)
-        if (!hostAccessKey) {
-            return promiseError(`Could not find corresponding key for hostname ${hostname}`)
-        }
-        try {
-            const token = await shortLivedTokenClient.fetchToken(serverUrl, hostAccessKey, expiry)
-            return DevelocityAccessCredentials.of([token])
-        } catch (e) {
-            return new Promise((resolve, reject) => reject(e))
-        }
-    }
-
     const tokens = new Array<HostnameAccessKey>()
     for (const k of develocityAccessKey.keys) {
         try {
+            core.info(`Requesting short-lived Develocity access token for ${k.hostname}`)
             const token = await shortLivedTokenClient.fetchToken(`https://${k.hostname}`, k, expiry)
             tokens.push(token)
         } catch (e) {
-            // Ignoring failed token, TODO: log this ?
+            // Ignore failure to obtain token
+            core.info(`Failed to obtain short-lived Develocity access token for ${k.hostname}: ${e}`)
         }
     }
     if (tokens.length > 0) {
@@ -96,17 +66,8 @@ export async function getToken(
     return empty
 }
 
-function extractHostname(serverUrl: string): string | null {
-    try {
-        const parsedUrl = new URL(serverUrl)
-        return parsedUrl.hostname
-    } catch (error) {
-        return null
-    }
-}
-
 class ShortLivedTokenClient {
-    httpc = new httpm.HttpClient('gradle/setup-gradle')
+    httpc = new httpm.HttpClient('gradle/actions/setup-gradle')
     maxRetries = 3
     retryInterval = 1000
 
@@ -187,14 +148,6 @@ export class DevelocityAccessCredentials {
         return this.keys.length === 0
     }
 
-    isSingleKey(): boolean {
-        return this.keys.length === 1
-    }
-
-    forHostname(hostname: string): HostnameAccessKey | undefined {
-        return this.keys.find(hostKey => hostKey.hostname === hostname)
-    }
-
     raw(): string {
         return this.keys
             .map(k => `${k.hostname}${DevelocityAccessCredentials.hostDelimiter}${k.key}`)

sources/test/jest/short-lived-token.test.ts

@@ -37,75 +37,40 @@ describe('short lived tokens', () => {
                 message: 'connect ECONNREFUSED 127.0.0.1:3333',
                 code: 'ECONNREFUSED'
             })
-        try {
+        await expect(getToken('localhost=key0', ''))
-            await getToken('true', 'http://localhost:3333', 'localhost=xyz;host1=key1', '')
+            .resolves
-            expect('should have thrown').toBeUndefined()
+            .toBeNull()
-        } catch (e) {
-            // @ts-ignore
-            expect(e.code).toBe('ECONNREFUSED')
-        }
     })
 
-    it('get short lived token fails when request fails', async () => {
+    it('get short lived token is null when request fails', async () => {
         nock('http://dev:3333')
             .post('/api/auth/token')
             .times(3)
             .reply(500, 'Internal error')
         expect.assertions(1)
-        await expect(getToken('true', 'http://dev:3333', 'dev=xyz;host1=key1', ''))
+        await expect(getToken('dev=xyz', ''))
-            .rejects
-            .toThrow('Develocity short lived token request failed http://dev:3333 with status code 500')
-    })
-
-    it('get short lived token fails when server url is not set', async () => {
-        expect.assertions(1)
-        await expect(getToken('true', undefined, 'localhost=xyz;host1=key1', ''))
-            .rejects
-            .toThrow('Develocity Server URL not configured')
-    })
-
-    it('get short lived token returns null when access key is empty', async () => {
-        expect.assertions(1)
-        await expect(getToken('true', 'http://dev:3333', '', ''))
             .resolves
             .toBeNull()
     })
 
-    it('get short lived token fails when host cannot be extracted from server url', async () => {
+    it('get short lived token returns null when access key is empty', async () => {
         expect.assertions(1)
-        await expect(getToken('true', 'not_a_url', 'localhost=xyz;host1=key1', ''))
+        await expect(getToken('', ''))
-            .rejects
+            .resolves
-            .toThrow('Could not extract hostname from Develocity server URL')
+            .toBeNull()
     })
 
-    it('get short lived token fails when access key does not contain corresponding host', async () => {
+    it('get short lived token succeeds when single key is set', async () => {
-        expect.assertions(1)
-        await expect(getToken('true', 'http://dev', 'host1=xyz;host2=key2', ''))
-            .rejects
-            .toThrow('Could not find corresponding key for hostname dev')
-    })
-
-    it('get short lived token succeeds when enforce url is true', async () => {
         nock('https://dev')
             .post('/api/auth/token')
             .reply(200, 'token')
         expect.assertions(1)
-        await expect(getToken('true', 'https://dev', 'dev=key1;host1=key2', ''))
+        await expect(getToken('dev=key1', ''))
             .resolves
             .toEqual({"keys": [{"hostname": "dev", "key": "token"}]})
     })
 
-    it('get short lived token succeeds when enforce url is false and single key is set', async () => {
+    it('get short lived token succeeds when multiple keys are set', async () => {
-        nock('https://dev')
-            .post('/api/auth/token')
-            .reply(200, 'token')
-        expect.assertions(1)
-        await expect(getToken('false', 'https://dev', 'dev=key1', ''))
-            .resolves
-            .toEqual({"keys": [{"hostname": "dev", "key": "token"}]})
-    })
-
-    it('get short lived token succeeds when enforce url is false and multiple keys are set', async () => {
         nock('https://dev')
             .post('/api/auth/token')
             .reply(200, 'token1')
@@ -113,12 +78,12 @@ describe('short lived tokens', () => {
             .post('/api/auth/token')
             .reply(200, 'token2')
         expect.assertions(1)
-        await expect(getToken('false', 'https://dev', 'dev=key1;prod=key2', ''))
+        await expect(getToken('dev=key1;prod=key2', ''))
             .resolves
             .toEqual({"keys": [{"hostname": "dev", "key": "token1"}, {"hostname": "prod", "key": "token2"}]})
     })
 
-    it('get short lived token succeeds when enforce url is false and multiple keys are set and one is failing', async () => {
+    it('get short lived token succeeds when multiple keys are set and one is failing', async () => {
         nock('https://dev')
             .post('/api/auth/token')
             .reply(200, 'token1')
@@ -130,8 +95,23 @@ describe('short lived tokens', () => {
             .post('/api/auth/token')
             .reply(200, 'token2')
         expect.assertions(1)
-        await expect(getToken('false', 'https://dev', 'dev=key1;bogus=key0;prod=key2', ''))
+        await expect(getToken('dev=key1;bogus=key0;prod=key2', ''))
             .resolves
             .toEqual({"keys": [{"hostname": "dev", "key": "token1"}, {"hostname": "prod", "key": "token2"}]})
     })
+
+    it('get short lived token is null when multiple keys are set and all are failing', async () => {
+        nock('https://dev')
+            .post('/api/auth/token')
+            .times(3)
+            .reply(500, 'Internal Error')
+        nock('https://bogus')
+            .post('/api/auth/token')
+            .times(3)
+            .reply(500, 'Internal Error')
+        expect.assertions(1)
+        await expect(getToken('dev=key1;bogus=key0', ''))
+            .resolves
+            .toBeNull()
+    })
 })