Add support for short-lived tokens by alextu · Pull Request #224 · gradle/actions

alextu · 2024-05-14T15:38:48Z

The setup-gradle action tries to get a short-lived access token given the passed Develocity access key.
This key can be passed either with the DEVELOCITY_ACCESS_KEY env var or better with a new input develocity-access-key.
If a token can be retrieved, then the DEVELOCITY_ACCESS_KEY env var will be set to the token. Otherwise the DEVELOCITY_ACCESS_KEY will be set to a blank string, to avoid a leak.

github-actions · 2024-05-14T15:40:41Z

Job Summary for Gradle

Demo adding Build Scan® comment to PR :: failing-build-with-comment-on-failure

Gradle Root Project	Requested Tasks	Gradle Version	Build Outcome	Build Scan®
kotlin-dsl	no-a-real-task	8.7	❌

github-actions · 2024-05-14T15:40:46Z

Job Summary for Gradle

Demo adding Build Scan® comment to PR :: successful-build-with-always-comment

Gradle Root Project	Requested Tasks	Gradle Version	Build Outcome	Build Scan®
kotlin-dsl	build	8.7	✅

github-actions · 2024-05-14T15:40:50Z

Job Summary for Gradle

Demo adding Build Scan® comment to PR :: successful-build-with-always-comment

Gradle Root Project	Requested Tasks	Gradle Version	Build Outcome	Build Scan®
kotlin-dsl	build	8.7	✅

github-actions · 2024-05-14T15:40:55Z

Job Summary for Gradle

Demo adding Build Scan® comment to PR :: failing-build-with-comment-on-failure

Gradle Root Project	Requested Tasks	Gradle Version	Build Outcome	Build Scan®
kotlin-dsl	no-a-real-task	8.7	❌

bigdaz

Thanks @alextu. I left quite a few comments and change requests.

bigdaz · 2024-05-15T13:12:12Z

@@ -0,0 +1,164 @@
+import * as httpm from 'typed-rest-client/HttpClient'


The "package" for this file should not be "shortlived-token", but something more general purpose. Let's use "develocity".

Also, for consistency we should name this short-lived-token.

bigdaz · 2024-05-15T13:16:39Z

@@ -1,7 +1,8 @@
 import * as core from '@actions/core'


This file can be moved into the same "package" as shortlived-token.ts.

bigdaz · 2024-05-15T13:22:37Z

+      working-directory: .github/workflow-samples/no-ge
+      run: gradle help
+    - name: Check access key does not leak
+      run: "[ \"${DEVELOCITY_ACCESS_KEY}\" != \"${{ secrets.DEVELOCITY_ACCESS_KEY }}\" ] || (echo 'DEVELOCITY_ACCESS_KEY has leaked!'; exit 1)"


We need to ensure that the ACCESS KEY isn't leaked to the console here.

bigdaz · 2024-05-15T13:26:02Z

+        script: |
+          core.setFailed('No Build Scan detected')
+
+  inject-develocity-short-lived-token-failed:


What real behaviour are we trying to test here?

if requesting a token fails, we want to avoid the passed access key as an env var to be used downstream (and potentially leaked)

bigdaz · 2024-05-15T13:26:53Z

+        script: |
+          core.setFailed('No Build Scan detected')
+
+  inject-develocity-short-lived-token-with-env:


This test already exists in integ-test-inject-develocity. Just need to add the token check.

bigdaz · 2024-05-15T13:28:15Z

@@ -0,0 +1,151 @@
+name: Test develocity injection


Instead of adding a separate integration test, let's merge this coverage into the existing inject-develocity test. So we'll be testing develocity injection, and verifying that a short-lived token is used.

bigdaz · 2024-05-15T13:31:32Z

+  develocity-token-expiry:
+    description: The Develocity short-lived access tokens expiry in hours. Default is 2 hours.
+    required: false
+


We need to add the same arguments to the dependency-submission action. These actions share many implementation details, including build-scan publishing.

bigdaz · 2024-05-15T13:33:58Z

+        DEVELOCITY_URL: https://develocity.your-server.com
+        DEVELOCITY_PLUGIN_VERSION: 3.17
+      with:
+        develocity-access-key: ${{ secrets.MY_DEVELOCITY_ACCESS_KEY }}


I don't think this will work. The access key needs to be provided to the setup-gradle Step.

oh right, my bad

bigdaz · 2024-05-15T13:41:13Z

+
+This access key will be used during the action execution to get a short-lived token and set it to the DEVELOCITY_ACCESS_KEY environment variable.
+
+### Short-lived access tokens


The feature to map develocityAccessKey to a short-lived token isn't directly related to Develocity injection. I think we should include develocityAccessKey in our examples for Develocity injection, but this feature will also come into play when the build is configured for Develocity (without injection).

I think we should also consider adding test coverage for this: ensure a sample build that is wired to publish to ge.solutions-team.gradle.com can use short-lived access tokens.

bigdaz · 2024-05-15T13:42:18Z

    }
+    const develocityAccesskeyEnvVar = `DEVELOCITY_ACCESS_KEY`
+    if (config.getDevelocityAccessKey()) {
+        try {


Let's move this logic to shortlived-token.ts.

bigdaz

LGTM

…ars (#225) Follow up of #224, we now attempt to set both old and new access key env variables to a short lived token. If a short-lived token cannot be obtained, then: - DEVELOCITY_ACCESS_KEY is set to an empty string, preventing this from being used. - GRADLE_ENTERPRISE_ACCESS_KEY is left intact, with a deprecation warning being issued.

The setup-gradle action tries to get a short-lived access token given the supplied Develocity access key. This key can be passed either with the `DEVELOCITY_ACCESS_KEY` env var or via the `develocity-access-key` input parameter. If a token can be retrieved, then the `DEVELOCITY_ACCESS_KEY` env var will be set to the token. Otherwise the `DEVELOCITY_ACCESS_KEY` will be set to a blank string, to avoid a leak. --------- Co-authored-by: daz <daz@gradle.com>

…ars (gradle#225) Follow up of gradle#224, we now attempt to set both old and new access key env variables to a short lived token. If a short-lived token cannot be obtained, then: - DEVELOCITY_ACCESS_KEY is set to an empty string, preventing this from being used. - GRADLE_ENTERPRISE_ACCESS_KEY is left intact, with a deprecation warning being issued.

alextu requested review from bigdaz and welandaz May 14, 2024 15:38

alextu commented May 14, 2024

View reviewed changes

Comment thread .github/workflows/ci-integ-test.yml Outdated

welandaz approved these changes May 15, 2024

View reviewed changes

bigdaz requested changes May 15, 2024

View reviewed changes

bigdaz approved these changes May 15, 2024

View reviewed changes

alextu and others added 10 commits May 15, 2024 16:33

Add support for short-lived tokens

d66d1cc

Move short-lived-token files to develocity package

1287a7c

Move integ tests

fdd144f

Uncomment integ test

c9ffde8

Add access key and token expiry to dependency submission action

c3c6e7d

Move the env plumbing code to short-lived-token.ts

4deb17d

Fix url in integ test

81071db

Remove integ test from list

9a00459

Fix doc snippet

d4bf142

Fix test for GE plugin 3.16.3

81f24ed

bigdaz force-pushed the atual/short-lived-tokens branch from 83c899f to 81f24ed Compare May 15, 2024 22:43

bigdaz merged commit 500e0ee into main May 15, 2024

bigdaz deleted the atual/short-lived-tokens branch May 15, 2024 22:49

alextu mentioned this pull request May 16, 2024

Set both DEVELOCITY_ACCESS_KEY and GRADLE_ENTERPRISE_ACCESS_KEY env vars #225

Merged

bigdaz modified the milestones: v3.4, v3.3.2, v3.4.0 Jun 13, 2024

		@@ -0,0 +1,164 @@
		import * as httpm from 'typed-rest-client/HttpClient'


		This access key will be used during the action execution to get a short-lived token and set it to the DEVELOCITY_ACCESS_KEY environment variable.

		### Short-lived access tokens

Uh oh!

Conversation

alextu commented May 14, 2024

Uh oh!

github-actions Bot commented May 14, 2024

Job Summary for Gradle

Demo adding Build Scan® comment to PR :: failing-build-with-comment-on-failure

Uh oh!

github-actions Bot commented May 14, 2024

Job Summary for Gradle

Demo adding Build Scan® comment to PR :: successful-build-with-always-comment

Uh oh!

github-actions Bot commented May 14, 2024

Job Summary for Gradle

Demo adding Build Scan® comment to PR :: successful-build-with-always-comment

Uh oh!

Uh oh!

github-actions Bot commented May 14, 2024

Job Summary for Gradle

Demo adding Build Scan® comment to PR :: failing-build-with-comment-on-failure

Uh oh!

bigdaz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bigdaz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants