chore: adjust ci to use license file instead of license key #445

toph-allen · 2025-09-30T20:57:41Z

Intent

CI now uses the license file instead of a license key. It's now no longer possible to use a license key via an environment variable — pass the license file path directly to build_test_env() instead. The license key secret has been removed from this repo.

Fixes #438

Approach

Added some diagnostics to utils-ci.R to print the logs and inspect of docker containers where Connect fails to become available.
Removed the license key path entirely from utils-ci.R. Functions there now only expect license files.
Updated relevant workflows to explicitly pass the license file path to build_test_env().
Some other cleanup, including making container name discovery automatic rather than hard-coded.

Checklist

~~Does this change update NEWS.md (referencing the connected issue if necessary)?~~
~~Does this change need documentation? Have you run devtools::document()?~~

jonkeane

We can (and should) cleanup the RSC_LICENSE stuff a bit more here.

We can do this as a follow on, but I think there's still significant complexity still around, for example, if license files are the way then we should remove the license key portions of places like

connectapi/R/utils-ci.R

Lines 6 to 29 in 365b2f0

    
           determine_license_env <- function(license) { 
        
             if (fs::file_exists(license) && fs::path_ext(license) == "lic") { 
        
               # Docker needs this to be an absolute path 
        
               license <- fs::path_abs(license) 
        
               cat_line("determine_license: looks like a license file") 
        
               return(list( 
        
                 type = "file", 
        
                 cmd_params = c( 
        
                   "-v", 
        
                   paste0(license, ":/var/lib/rstudio-connect/license.lic") 
        
                 ), 
        
                 env_params = c(RSC_LICENSE_FILE = license) 
        
               )) 
        
             } else { 
        
               cat_line("determine_license: looks like a license key") 
        
               return(list( 
        
                 type = "key", 
        
                 cmd_params = c(), 
        
                 env_params = c( 
        
                   RSC_LICENSE = license 
        
                 ) 
        
               )) 
        
             } 
        
           }

I suspect untangling that + deleting the paths that would go that direction might get us to a much much simpler place (and one where we would be able to more effectively notice funky things like accidentally using one or the other more easily!). If I'm following this code through, basically we are passing around (1) the contents of the license file and (2) an env var of a file on the runner that will ultimately "just" get mounted to /var/lib/rstudio-connect/license.lic (

connectapi/inst/ci/test-connect-lic.yml

Lines 12 to 13 in 365b2f0

    
             - "${RSC_LICENSE_FILE}:/var/lib/rstudio-connect/license.lic" 
        
           platform: linux/amd64

) right? We should be able to get to there much more directly than what I've found by tracing the instances of RSC_LICENSE(_FILE))

jonkeane · 2025-09-30T21:52:04Z

.github/workflows/integration-tests.yaml

    env:
      CONNECT_VERSION: ${{ matrix.version }}
      GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
-      RSC_LICENSE: ${{ secrets.RSC_LICENSE }}


This line could be RSC_LICENSE: /tmp/connect.lic

And then you wouldn't need to do line 58 below at all (which is extra good cause >> $GITHUB_ENV should be used sparingly). Though, if it is just hardcoded, maybe this should be hardcoded in the makefile instead?

But also, stepping back and thinking about this: should this be specified at all? Where does it get used, maybe we can get rid of it entirely?

Another option would be to add this(hardcoded in our CI yamls) as the connect_license argument to

connectapi/.github/workflows/integration-tests.yaml

Lines 61 to 62 in 365b2f0

connectapi:::build_test_env()

shell: Rscript {0}

Yeah, I'd like to keep the flexibility to specify a different license key path if running these functions locally. Passing it directly as an argument to connectapi:::build_test_env() is the route I went.

toph-allen · 2025-09-30T22:16:00Z

We can (and should) cleanup the RSC_LICENSE stuff a bit more here.

We can do this as a follow on, but I think there's still significant complexity still around, for example, if license files are the way then we should remove the license key portions of places like

Yeah, you're right. The current code retains a lot of complexity by retaining the ability to use a license file or a license key.

I can't think of a reason we wouldn't be able to use license files locally, so I'll go ahead and do that extra bit of cleanup now! Save our future selves the complexity.

toph-allen · 2025-09-30T22:54:39Z

.github/CONTRIBUTING.md

 This has some additional requirements.

- You need a valid Connect license key or file. Put the contents of the license key, or the path to the license file, in the `RSC_LICENSE` environment variable.
+- You need a valid Connect license file (`.lic` file). Place it in the root of the repository as `connect-license.lic`.


.gitignore already covers *.lic :)

R/utils-ci.R

karawoo · 2025-09-30T23:13:06Z

R/utils-ci.R

-  wait_for_connect_ready(hosts[1])
-  wait_for_connect_ready(hosts[2])
+  wait_for_connect_ready(hosts[1], container_name = "ci-connect-1")
+  wait_for_connect_ready(hosts[2], container_name = "ci-connect-2")


what if we return the name from compose_find_hosts (or a separate function) so we don't have to hard code these?

Co-authored-by: Kara Woo <[email protected]>

jonkeane

Pulling this thread even more (sorry, this is scope creeping, but I keep wondering "but why?") Do we need to be setting RSC_LICENSE* at all (in the Makefile, but then also in utils-ci.R line 42)?

If I'm reading https://github.com/rstudio/rstudio-docker-products/blob/edd6d68702e27f1e25d9239784ea7e90142b8e63/connect/startup.sh#L31-L45 correctly (which I think is where that stuff is used), we don't need to so long as we've got a file in /var/lib/rstudio-connect/*.lic Or maybe that chmod 0600 is important for us?

jonkeane · 2025-10-01T00:44:41Z

inst/ci/test-connect.yml

-version: '2.3'
-services:
-
-  connect:
-    hostname: connect
-    image: ghcr.io/rstudio/rstudio-connect:${CONNECT_VERSION}
-    scale: 2
-    ports:
-      - 3939
-    privileged: true
-    environment:
-      - RSC_LICENSE
-      - RSC_HASTE_ENABLED=true
-    platform: linux/amd64
-    # if you need to customize Connect config until we have env vars
-    # volumes:
-    #  - ../../tmp.gcfg:/etc/rstudio-connect/rstudio-connect.gcfg


I totally believe that this file is superfluous, but would you mind mentioning what it used to be / if something else superseded it?

Oh, I think I see now, this is all the connect where it was using license keys, yeah? Do we need to remove these (and maybe other?) references from the makefile too then?

connectapi/Makefile

Lines 49 to 58 in 365b2f0

connect-up:

NETWORK=${NETWORK} \

RSC_LICENSE=$(RSC_LICENSE) \

CONNECT_VERSION=$(CONNECT_VERSION) \

docker compose -f inst/ci/test-connect.yml -f .github/local/make-network.yml up -d

connect-down:

NETWORK=${NETWORK} \

docker compose -f inst/ci/test-connect.yml -f .github/local/make-network.yml down

Yes — this one was the "license key" compose file.

toph-allen · 2025-10-01T01:01:50Z

Pulling this thread even more (sorry, this is scope creeping, but I keep wondering "but why?") Do we need to be setting RSC_LICENSE* at all (in the Makefile, but then also in utils-ci.R line 42)?

I think it's fine to keep pulling at it here. I probably just missed the Makefile instances. :)

toph-allen · 2025-10-01T15:04:29Z

Pulling this thread even more (sorry, this is scope creeping, but I keep wondering "but why?") Do we need to be setting RSC_LICENSE* at all (in the Makefile, but then also in utils-ci.R line 42)?

To my understanding, the RSC_LICENSE_FILE in utils-ci.R line 42 is set because test-connect-lic.yml uses that env var to mount the license file (line 12 of test-connect-lic.yml). (Just confirming we're on the same page!)

The references in the Makefile are the superfluous ones, and I'm removing them.

toph-allen · 2025-10-01T15:12:22Z

There seems to be a lot of cruft in the Makefile. For example, there were connect-up and connect-file-up targets, which don't seem to be used by anything in CI. Likewise .github/local/test-connect-ci.yml does not seem to be used in CI, it's just used by the test-env-up target in the Makefile. Meanwhile the GitHub Actions use utils-ci.R which uses inst/ci/test-connect-lic.yml.

I'm not going to expand the scope of this PR to deal with that, but I'll file an issue to track.

[edit] Tracked in #446.

jonkeane · 2025-10-01T16:06:31Z

To my understanding, the RSC_LICENSE_FILE in utils-ci.R line 42 is set because test-connect-lic.yml uses that env var to mount the license file (line 12 of test-connect-lic.yml). (Just confirming we're on the same page!)

Ah, yes ok we do use that envvar to pass that around there. There are other options we could take, but those are bigger, and ...

I'm not going to expand the scope of this PR to deal with that, but I'll file an issue to track.

💯 agree, let's ship this with as much cleaning as we can now and we will follow up (and we should! Soon! but not here)

toph-allen added 5 commits September 30, 2025 16:57

diagnostics

15f7576

use license file instead of license key

af293ed

remove some diagnostics

de23714

remove extra sleep

7f7539a

update wait_for_connect_ready

3387cb1

toph-allen changed the title ~~chore: fix ci~~ chore: adjust ci to use license file instead of license key Sep 30, 2025

toph-allen marked this pull request as ready for review September 30, 2025 21:42

toph-allen requested review from jonkeane and karawoo September 30, 2025 21:56

jonkeane requested changes Sep 30, 2025

View reviewed changes

remove license paths; only allow license files

8e691f7

toph-allen requested a review from jonkeane September 30, 2025 22:50

update CONTRIBUTING.md

83390b2

toph-allen commented Sep 30, 2025

View reviewed changes

karawoo reviewed Sep 30, 2025

View reviewed changes

toph-allen and others added 2 commits September 30, 2025 19:16

Update R/utils-ci.R

a8e4d01

Co-authored-by: Kara Woo <[email protected]>

make container name discovery automatic

14b5eee

toph-allen requested a review from karawoo September 30, 2025 23:31

fix linting

f38f9a0

jonkeane reviewed Oct 1, 2025

View reviewed changes

remove references to old license-key workflow

fcb805e

toph-allen requested a review from jonkeane October 1, 2025 15:12

jonkeane approved these changes Oct 1, 2025

View reviewed changes

toph-allen merged commit 8e8f85d into main Oct 1, 2025
21 checks passed

toph-allen deleted the toph/fix-ci branch October 1, 2025 16:41

	determine_license_env <- function(license) {
	if (fs::file_exists(license) && fs::path_ext(license) == "lic") {
	# Docker needs this to be an absolute path
	license <- fs::path_abs(license)
	cat_line("determine_license: looks like a license file")
	return(list(
	type = "file",
	cmd_params = c(
	"-v",
	paste0(license, ":/var/lib/rstudio-connect/license.lic")
	),
	env_params = c(RSC_LICENSE_FILE = license)
	))
	} else {
	cat_line("determine_license: looks like a license key")
	return(list(
	type = "key",
	cmd_params = c(),
	env_params = c(
	RSC_LICENSE = license
	)
	))
	}
	}

	- "${RSC_LICENSE_FILE}:/var/lib/rstudio-connect/license.lic"
	platform: linux/amd64

	connect-up:
	NETWORK=${NETWORK} \
	RSC_LICENSE=$(RSC_LICENSE) \
	CONNECT_VERSION=$(CONNECT_VERSION) \
	docker compose -f inst/ci/test-connect.yml -f .github/local/make-network.yml up -d

	connect-down:
	NETWORK=${NETWORK} \
	docker compose -f inst/ci/test-connect.yml -f .github/local/make-network.yml down

chore: adjust ci to use license file instead of license key #445

chore: adjust ci to use license file instead of license key #445

Uh oh!

Conversation

toph-allen commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Intent

Approach

Checklist

Uh oh!

jonkeane 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!

toph-allen commented Sep 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonkeane 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!

toph-allen commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

toph-allen commented Oct 1, 2025

Uh oh!

toph-allen commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonkeane commented Oct 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

toph-allen commented Sep 30, 2025 •

edited

Loading

toph-allen commented Oct 1, 2025 •

edited

Loading

toph-allen commented Oct 1, 2025 •

edited

Loading