From ae876ddd1b5f2820bda992e33f2b9203d41d4785 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 07:23:33 +0000 Subject: [PATCH 1/6] Use Seqera Platform default workspace when none set locally `nextflow launch` and the `-with-tower` monitoring path previously fell back to the user's Personal workspace whenever no workspace was configured locally. Seqera Platform now exposes a per-user (and system) default workspace as the top-level `defaultWorkspaceId` field of the `GET /user-info` response. Read that field and use it as the fallback when no workspace is set via config (`tower.workspaceId`), environment (`TOWER_WORKSPACE_ID`), or the `-workspace` flag. Local/CLI settings still take precedence, and Platform-driven runs (with `TOWER_WORKFLOW_ID` set) are unaffected. The lookup is null-safe, so Platform versions predating the field degrade cleanly to the Personal workspace. - TowerClient.getDefaultWorkspaceId() reads the new field defensively - LaunchCommandImpl.resolveWorkspaceId() falls back to it before returning null - TowerFactory.create() applies it to the monitoring observer - `nextflow auth status` surfaces the Platform default workspace - docs and tests updated Assisted-by: Claude Code Signed-off-by: Claude --- docs/reference/cli/auth.mdx | 2 +- docs/reference/cli/launch.mdx | 2 +- docs/reference/config/tower.mdx | 4 +- .../io/seqera/tower/plugin/TowerClient.groovy | 22 ++++++++++ .../seqera/tower/plugin/TowerFactory.groovy | 15 ++++++- .../tower/plugin/auth/AuthCommandImpl.groovy | 26 +++++++++-- .../plugin/launch/LaunchCommandImpl.groovy | 8 ++++ .../tower/plugin/TowerClientTest.groovy | 44 +++++++++++++++++++ .../tower/plugin/TowerFactoryTest.groovy | 41 ++++++++++++++++- .../plugin/auth/AuthCommandImplTest.groovy | 32 ++++++++++++++ .../launch/LaunchCommandImplTest.groovy | 35 ++++++++++++++- 11 files changed, 221 insertions(+), 10 deletions(-) diff --git a/docs/reference/cli/auth.mdx b/docs/reference/cli/auth.mdx index 713ee9cb7d..ea8aaaed99 100644 --- a/docs/reference/cli/auth.mdx +++ b/docs/reference/cli/auth.mdx @@ -43,7 +43,7 @@ Remove Seqera authentication and revoke the Seqera Cloud access token, if applic ##### `config` -Set the Seqera primary compute environment, monitoring, and workspace. +Set the Seqera primary compute environment, monitoring, and workspace. If you do not set a default workspace here, Nextflow uses the default workspace configured in your Seqera Platform account (if any), otherwise your personal workspace. ##### `status` diff --git a/docs/reference/cli/launch.mdx b/docs/reference/cli/launch.mdx index a6ccf73e73..93b160ef86 100644 --- a/docs/reference/cli/launch.mdx +++ b/docs/reference/cli/launch.mdx @@ -81,7 +81,7 @@ The directory where intermediate result files are stored. ##### `-workspace` -The Seqera Platform workspace name. +The Seqera Platform workspace name. If not specified and no workspace is configured locally (via `tower.workspaceId` or `TOWER_WORKSPACE_ID`), the run uses your default workspace configured in Seqera Platform, or your personal workspace if no default is set. ##### `-workspace-secret` diff --git a/docs/reference/config/tower.mdx b/docs/reference/config/tower.mdx index c70ed3242b..bbd4498a44 100644 --- a/docs/reference/config/tower.mdx +++ b/docs/reference/config/tower.mdx @@ -39,6 +39,8 @@ The HTTP read timeout for Seqera Platform API requests (default: `'60s'`). ##### `tower.workspaceId` -The workspace ID in Seqera Platform in which to save the run (default: the launching user's personal workspace). +The workspace ID in Seqera Platform in which to save the run (default: the user's default workspace configured in Seqera Platform, or the personal workspace if no default is set). The workspace ID can also be specified using the environment variable `TOWER_WORKSPACE_ID` (config file has priority over the environment variable). + +When no workspace is specified via the config file or environment variable, Nextflow uses the default workspace configured in your Seqera Platform account (if any); otherwise it falls back to your personal workspace. diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy index b7e8828b17..49c26b395a 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy @@ -493,6 +493,28 @@ class TowerClient { return json.user as Map } + /** + * Return the user's server-side default workspace ID, if any. + * + * Seqera Platform exposes a per-user default workspace (falling back to a + * global system default) as the top-level {@code defaultWorkspaceId} field of + * the {@code GET /user-info} response. The value is access-validated by + * Platform and is {@code null} when no default is set, when the stored default + * is no longer accessible, or when the Platform version predates the feature. + * + * @return the default workspace ID, or {@code null} if none is available + */ + Long getDefaultWorkspaceId() { + try { + final json = apiGet("/user-info") + return json.defaultWorkspaceId as Long + } + catch( Exception e ) { + log.debug "Unable to resolve Seqera Platform default workspace: ${e.message}" + return null + } + } + /** * Calls the Seqera Platform to retrieve the workflow information. * diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy index a0b2f41959..ae0e0fedde 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy @@ -53,14 +53,27 @@ class TowerFactory implements TraceObserverFactoryV2 { // during session init and gets swallowed silently by the launcher checkAccessToken(config) final result = new ArrayList(1) + // resolve the workspace: local/CLI settings win; when none is set and this is + // not a Platform-driven run, fall back to the user's Platform default workspace + String workspaceId = config.workspaceId + if( !workspaceId && !env.get('TOWER_WORKFLOW_ID') ) + workspaceId = resolveDefaultWorkspaceId(session, env)?.toString() // create the tower observer - result.add( new TowerObserver(session, client(session, env), config.workspaceId, env)) + result.add( new TowerObserver(session, client(session, env), workspaceId, env)) // create the logs checkpoint if( session.cloudCachePath ) result.add( new LogsCheckpoint() ) return result } + /** + * Resolve the user's server-side default workspace from Seqera Platform. + * Extracted as a seam so it can be stubbed in tests without hitting the network. + */ + protected Long resolveDefaultWorkspaceId(Session session, Map env) { + return client(session, env).getDefaultWorkspaceId() + } + @Memoized static TowerClient client(Session session, Map env) { final opts = session.config.tower as Map ?: Collections.emptyMap() diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy index e0ec788138..65fdf2c6eb 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy @@ -842,6 +842,9 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { // Default workspace - use PlatformHelper final String workspaceId = PlatformHelper.getWorkspaceId(towerConfig, SysEnv.get()) final workspaceInfo = getConfigValue(config, 'tower.workspaceId', 'TOWER_WORKSPACE_ID') + // the workspace effectively used for runs: the local/CLI value if set, + // otherwise the Seqera Platform server-side default workspace (if any) + String effectiveWorkspaceId = workspaceId if( workspaceId ) { // Try to get workspace name and roles from API if we have a token def workspaceDetails = null @@ -864,7 +867,24 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { } } else { if( accessToken ) { - status.table.add(['Default workspace', 'None (Personal workspace)', 'default']) + // No local/CLI workspace set — check for a Seqera Platform default workspace + final httpClient = createTowerClient(endpoint, accessToken) + final platformDefaultId = httpClient.getDefaultWorkspaceId()?.toString() + if( platformDefaultId ) { + effectiveWorkspaceId = platformDefaultId + final userInfo = httpClient.getUserInfo() + final workspaceDetails = httpClient.getUserWorkspaceDetails(userInfo.id as String, platformDefaultId) + if( workspaceDetails ) { + status.workspaceRowIndex = status.table.size() + status.table.add(['Default workspace', platformDefaultId, 'platform']) + status.workspaceInfo = workspaceDetails + status.workspaceRoles = workspaceDetails.roles as List + } else { + status.table.add(['Default workspace', platformDefaultId, 'platform']) + } + } else { + status.table.add(['Default workspace', 'None (Personal workspace)', 'default']) + } } } @@ -874,9 +894,9 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { final httpClient = createTowerClient(endpoint, accessToken) try { if( config['tower.computeEnvId'] ) { - computeEnv = getComputeEnvironment(httpClient, config['tower.computeEnvId'] as String, workspaceId) + computeEnv = getComputeEnvironment(httpClient, config['tower.computeEnvId'] as String, effectiveWorkspaceId) } else { - final computeEnvs = listComputeEnvironments(httpClient, workspaceId) + final computeEnvs = listComputeEnvironments(httpClient, effectiveWorkspaceId) computeEnv = computeEnvs.find { ((Map) it).primary == true } as Map } } catch( Exception e ) { diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy index ac3d906e0f..fc16ac6087 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy @@ -993,6 +993,14 @@ class LaunchCommandImpl extends BaseCommandImpl implements CmdLaunch.LaunchComma return (matchingWorkspace as Map).workspaceId as Long } + // No local/CLI workspace set — fall back to the Seqera Platform server-side + // default workspace, if the user (or the system) has one configured + final defaultWorkspaceId = createTowerClient(apiEndpoint, accessToken).getDefaultWorkspaceId() + if (defaultWorkspaceId) { + log.debug "Using Seqera Platform default workspace ID: ${defaultWorkspaceId}" + return defaultWorkspaceId + } + return null } diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy index f4b3554e70..4f25691acd 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy @@ -254,6 +254,50 @@ class TowerClientTest extends Specification { wireMock.stop() } + def 'should resolve the default workspace id from user-info' () { + given: + def wireMock = new WireMockServer(0) + wireMock.start() + and: + TowerConfig config = Mock(TowerConfig) { + getHttpReadTimeout() >> Duration.of('5 s') + getHttpConnectTimeout() >> Duration.of('5 s') + getEndpoint() >> wireMock.baseUrl() + getAccessToken() >> 'token' + } + TowerClient client = new TowerClient(config) + + when: 'the user-info response carries a top-level defaultWorkspaceId' + wireMock.stubFor( + WireMock.get(WireMock.urlPathEqualTo('/user-info')) + .willReturn(WireMock.aResponse().withStatus(200) + .withHeader('Content-Type', 'application/json') + .withBody('{"user":{"id":1,"userName":"me"},"needConsent":false,"defaultWorkspaceId":42}'))) + then: + client.getDefaultWorkspaceId() == 42L + + when: 'no defaultWorkspaceId is present in the response' + wireMock.resetAll() + wireMock.stubFor( + WireMock.get(WireMock.urlPathEqualTo('/user-info')) + .willReturn(WireMock.aResponse().withStatus(200) + .withHeader('Content-Type', 'application/json') + .withBody('{"user":{"id":1,"userName":"me"},"needConsent":false}'))) + then: + client.getDefaultWorkspaceId() == null + + when: 'the endpoint returns an error' + wireMock.resetAll() + wireMock.stubFor( + WireMock.get(WireMock.urlPathEqualTo('/user-info')) + .willReturn(WireMock.aResponse().withStatus(500).withBody('boom'))) + then: 'the failure is swallowed and null is returned' + client.getDefaultWorkspaceId() == null + + cleanup: + wireMock.stop() + } + def 'should build URL without query params'() { given: def client = new TowerClient() diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy index e6ea438e5e..dc265b23a0 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy @@ -29,7 +29,10 @@ class TowerFactoryTest extends Specification { def 'should create a tower observer' () { given: - def factory = new TowerFactory(env: [TOWER_ACCESS_TOKEN: '123']) + // stub the Platform default-workspace lookup so no live API call is made when + // no workspace is configured locally + def factory = Spy(new TowerFactory(env: [TOWER_ACCESS_TOKEN: '123'])) + factory.resolveDefaultWorkspaceId(_, _) >> null when: def session = Mock(Session) { getConfig() >> [tower: [enabled: true]] } @@ -44,6 +47,42 @@ class TowerFactoryTest extends Specification { observer.@client.endpoint == 'http://foo.com/api' } + def 'should use the Platform default workspace when none is configured locally' () { + given: + def factory = Spy(new TowerFactory(env: [TOWER_ACCESS_TOKEN: 'xyz'])) + def session = Mock(Session) { getConfig() >> [tower: [enabled: true, accessToken: 'xyz']] } + + when: + def observer = (TowerObserver) factory.create(session)[0] + then: 'the Platform default workspace is resolved and used' + 1 * factory.resolveDefaultWorkspaceId(_, _) >> 300L + observer.getWorkspaceId() == '300' + } + + def 'should not use the Platform default when a workspace is configured locally' () { + given: + def factory = Spy(new TowerFactory(env: [:])) + def session = Mock(Session) { getConfig() >> [tower: [enabled: true, workspaceId: '200', accessToken: 'xyz']] } + + when: + def observer = (TowerObserver) factory.create(session)[0] + then: 'the local workspace wins and no default lookup is performed' + 0 * factory.resolveDefaultWorkspaceId(_, _) + observer.getWorkspaceId() == '200' + } + + def 'should not use the Platform default for a Platform-driven run' () { + given: + def factory = Spy(new TowerFactory(env: [TOWER_WORKSPACE_ID: '100', TOWER_WORKFLOW_ID: '111222333', TOWER_ACCESS_TOKEN: 'xyz'])) + def session = Mock(Session) { getConfig() >> [tower: [enabled: true, accessToken: 'xyz']] } + + when: + def observer = (TowerObserver) factory.create(session)[0] + then: 'the env workspace is used and no default lookup is performed' + 0 * factory.resolveDefaultWorkspaceId(_, _) + observer.getWorkspaceId() == '100' + } + @Unroll def 'should fail when enabled but no access token is provided' () { given: diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/auth/AuthCommandImplTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/auth/AuthCommandImplTest.groovy index 5e78b31fce..773dfc92d1 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/auth/AuthCommandImplTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/auth/AuthCommandImplTest.groovy @@ -897,6 +897,38 @@ param2 = 'value2'""" status.workspaceInfo == null } + def 'should show the Platform default workspace when none configured locally'() { + given: + def config = ['tower.accessToken': 'test-token'] + def client = Mock(TowerClient) { + getUserInfo() >> [userName: 'testuser', id: '123'] + getDefaultWorkspaceId() >> 777L + getUserWorkspaceDetails(_, _) >> [ + orgName: 'TestOrg', + workspaceName: 'DefaultWorkspace', + workspaceFullName: 'test-org/default-workspace' + ] + } + def cmd = Spy(new AuthCommandImpl()) + cmd.createTowerClient(_,_) >> client + cmd.checkApiConnection(_) >> true + cmd.listComputeEnvironments(_, _) >> [] + SysEnv.push([:]) // isolate from real environment variables + + when: + def status = cmd.collectStatus(config) + + then: + status.table[4][0] == 'Default workspace' + status.table[4][1].contains('777') + status.table[4][2] == 'platform' + status.workspaceInfo != null + status.workspaceInfo.workspaceName == 'DefaultWorkspace' + + cleanup: + SysEnv.pop() + } + def 'should collect status from environment variables'() { given: def client = Mock(TowerClient){ diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy index b47fa8fc8c..88a6858a6e 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy @@ -761,9 +761,11 @@ class LaunchCommandImplTest extends Specification { ex.message.contains('Workspace \'nonexistent\' not found') } - def 'should return null when no workspace specified'() { + def 'should return null when no workspace specified and no Platform default'() { given: - def cmd = new LaunchCommandImpl() + def client = Mock(TowerClient) { getDefaultWorkspaceId() >> null } + def cmd = Spy(new LaunchCommandImpl()) + cmd.createTowerClient(_, _) >> client def config = [:] when: @@ -773,6 +775,35 @@ class LaunchCommandImplTest extends Specification { workspaceId == null } + def 'should use the Platform default workspace when none specified locally'() { + given: + def client = Mock(TowerClient) { getDefaultWorkspaceId() >> 999L } + def cmd = Spy(new LaunchCommandImpl()) + cmd.createTowerClient(_, _) >> client + def config = [:] + + when: + def workspaceId = cmd.resolveWorkspaceId(config, null, 'token', 'endpoint') + + then: 'the Platform default workspace is used' + workspaceId == 999L + } + + def 'should prefer the config workspace over the Platform default'() { + given: + def client = Mock(TowerClient) + def cmd = Spy(new LaunchCommandImpl()) + cmd.createTowerClient(_, _) >> client + def config = ['tower.workspaceId': 12345L] + + when: + def workspaceId = cmd.resolveWorkspaceId(config, null, 'token', 'endpoint') + + then: 'the local config wins and the Platform default is never queried' + 0 * client.getDefaultWorkspaceId() + workspaceId == 12345L + } + // ===== Launch Result Tests ===== def 'should extract launch result with workflow details'() { From 3252f91fa7e1557ff4c4c339aa4e0d06612ddaff Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 07:45:40 +0000 Subject: [PATCH 2/6] Consolidate workspace resolution into a single policy Follow-up cleanup on the Platform default-workspace change. The fallback was implemented independently in three places, which had already produced divergent behaviour, and the /user-info response was fetched more than once per command. - Add PlatformHelper.getEffectiveWorkspaceId(), the single definition of "a local setting wins, otherwise use the Platform default". The Platform lookup is passed in as a supplier so PlatformHelper stays free of I/O and of any dependency on the API client (nf-wave has no TowerClient). - Add PlatformHelper.isPlatformRun() and route the four getters plus the new call site through it, so the TOWER_WORKFLOW_ID rule has one definition. - Route TowerFactory, LaunchCommandImpl and AuthCommandImpl through the shared resolver. This also fixes `nextflow launch` ignoring TOWER_WORKSPACE_ID, which the docs already claimed was honoured. - Memoize the GET /user-info fetch in TowerClient so getUserInfo() and getDefaultWorkspaceId() share one round-trip: `nextflow auth status` drops from 3 identical /user-info calls to 1. - Collapse the duplicated status-rendering branch in AuthCommandImpl and reuse the already-created client for the compute-env lookup. - Document the new `platform` source value and align the TowerConfig workspaceId description with the docs. Assisted-by: Claude Code Signed-off-by: Claude --- docs/reference/config/tower.mdx | 4 +- .../main/groovy/nextflow/cli/CmdAuth.groovy | 2 +- .../nextflow/platform/PlatformHelper.groovy | 43 +++++++++++-- .../platform/PlatformHelperTest.groovy | 36 +++++++++++ .../tower/plugin/BaseCommandImpl.groovy | 10 +++ .../io/seqera/tower/plugin/TowerClient.groovy | 22 +++++-- .../io/seqera/tower/plugin/TowerConfig.groovy | 2 +- .../seqera/tower/plugin/TowerFactory.groovy | 19 +++--- .../tower/plugin/auth/AuthCommandImpl.groovy | 62 ++++++------------- .../plugin/launch/LaunchCommandImpl.groovy | 27 ++++---- .../tower/plugin/TowerClientTest.groovy | 61 +++++++++++------- .../tower/plugin/TowerFactoryTest.groovy | 8 +-- .../launch/LaunchCommandImplTest.groovy | 48 +++++--------- 13 files changed, 206 insertions(+), 138 deletions(-) diff --git a/docs/reference/config/tower.mdx b/docs/reference/config/tower.mdx index bbd4498a44..f64a790bec 100644 --- a/docs/reference/config/tower.mdx +++ b/docs/reference/config/tower.mdx @@ -41,6 +41,4 @@ The HTTP read timeout for Seqera Platform API requests (default: `'60s'`). The workspace ID in Seqera Platform in which to save the run (default: the user's default workspace configured in Seqera Platform, or the personal workspace if no default is set). -The workspace ID can also be specified using the environment variable `TOWER_WORKSPACE_ID` (config file has priority over the environment variable). - -When no workspace is specified via the config file or environment variable, Nextflow uses the default workspace configured in your Seqera Platform account (if any); otherwise it falls back to your personal workspace. +The workspace ID can also be specified using the environment variable `TOWER_WORKSPACE_ID` (config file has priority over the environment variable). When neither is set, Nextflow uses the default workspace configured in your Seqera Platform account, if any. diff --git a/modules/nextflow/src/main/groovy/nextflow/cli/CmdAuth.groovy b/modules/nextflow/src/main/groovy/nextflow/cli/CmdAuth.groovy index cf324b19f5..719e524819 100644 --- a/modules/nextflow/src/main/groovy/nextflow/cli/CmdAuth.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/cli/CmdAuth.groovy @@ -449,7 +449,7 @@ class CmdAuth extends CmdBase implements UsageAware { * * Default workspace * Configured workspace ID and name - * nextflow config, env var $TOWER_WORKSPACE_ID, default (Personal) + * nextflow config, env var $TOWER_WORKSPACE_ID, platform (account default), default (Personal) * * * Primary compute env diff --git a/modules/nextflow/src/main/groovy/nextflow/platform/PlatformHelper.groovy b/modules/nextflow/src/main/groovy/nextflow/platform/PlatformHelper.groovy index c0e46e45c3..459380967b 100644 --- a/modules/nextflow/src/main/groovy/nextflow/platform/PlatformHelper.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/platform/PlatformHelper.groovy @@ -16,6 +16,8 @@ package nextflow.platform +import java.util.function.Supplier + import groovy.transform.CompileStatic import nextflow.Global import nextflow.Session @@ -29,6 +31,39 @@ import nextflow.SysEnv @CompileStatic class PlatformHelper { + /** + * A run made by Seqera Platform is signalled by the {@code TOWER_WORKFLOW_ID} + * environment variable. In that case the settings must be taken from the + * environment only, because Platform has already decided them for this run. + * + * @param env the applicable environment variables + * @return {@code true} when the current run was launched by Platform + */ + static boolean isPlatformRun(Map env) { + return env.get('TOWER_WORKFLOW_ID') as boolean + } + + /** + * Resolve the workspace effectively used for a run: the locally configured + * value wins, otherwise fall back to the user's server-side default workspace + * in Seqera Platform. + * + * The Platform lookup is supplied by the caller so that this class stays free + * of any I/O and of any dependency on the Platform API client. + * + * @param opts the configuration options for Platform (e.g. `session.config.navigate('tower')`) + * @param env the applicable environment variables + * @param platformDefault supplies the Platform default workspace ID, queried only when needed + * @return the workspace ID to use, or null to use the personal workspace + */ + static String getEffectiveWorkspaceId(Map opts, Map env, Supplier platformDefault) { + final local = getWorkspaceId(opts, env) + // a local setting always wins; for a Platform-driven run the environment is authoritative + if( local || isPlatformRun(env) ) + return local + return platformDefault?.get()?.toString() + } + /** * Get the configured Platform API endpoint: if the endpoint is not provided in the configuration, we fallback to the * environment variable `TOWER_API_ENDPOINT`. If neither is provided, we fallback to the default endpoint. @@ -114,7 +149,7 @@ class PlatformHelper { * @return the Platform access token */ static String getAccessToken(Map opts, Map env) { - final token = env.get('TOWER_WORKFLOW_ID') + final token = isPlatformRun(env) ? env.get('TOWER_ACCESS_TOKEN') : opts.containsKey('accessToken') ? opts.accessToken as String : env.get('TOWER_ACCESS_TOKEN') return token @@ -130,7 +165,7 @@ class PlatformHelper { * @return the Platform refresh token */ static String getRefreshToken(Map opts, Map env) { - final token = env.get('TOWER_WORKFLOW_ID') + final token = isPlatformRun(env) ? env.get('TOWER_REFRESH_TOKEN') : opts.containsKey('refreshToken') ? opts.refreshToken as String : env.get('TOWER_REFRESH_TOKEN') return token @@ -146,7 +181,7 @@ class PlatformHelper { */ static String getWorkspaceId(Map opts, Map env) { try { - final workspaceId = env.get('TOWER_WORKFLOW_ID') + final workspaceId = isPlatformRun(env) ? env.get('TOWER_WORKSPACE_ID') : opts.workspaceId as Long ?: env.get('TOWER_WORKSPACE_ID') as Long return workspaceId @@ -167,7 +202,7 @@ class PlatformHelper { * @return the Platform compute environment ID, or null */ static String getComputeEnvId(Map opts, Map env) { - final computeEnvId = env.get('TOWER_WORKFLOW_ID') + final computeEnvId = isPlatformRun(env) ? env.get('TOWER_COMPUTE_ENV_ID') : opts.containsKey('computeEnvId') ? opts.computeEnvId as String : env.get('TOWER_COMPUTE_ENV_ID') return computeEnvId diff --git a/modules/nextflow/src/test/groovy/nextflow/platform/PlatformHelperTest.groovy b/modules/nextflow/src/test/groovy/nextflow/platform/PlatformHelperTest.groovy index 7b8d792147..026d2c3cf9 100644 --- a/modules/nextflow/src/test/groovy/nextflow/platform/PlatformHelperTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/platform/PlatformHelperTest.groovy @@ -18,6 +18,7 @@ package nextflow.platform import nextflow.SysEnv import spock.lang.Specification +import spock.lang.Unroll /** * Test PlatformHelper functionality @@ -123,4 +124,39 @@ class PlatformHelperTest extends Specification { expect: PlatformHelper.getComputeEnvId([:], [:]) == null } + + def 'should detect a Platform-driven run'() { + expect: + !PlatformHelper.isPlatformRun([:]) + !PlatformHelper.isPlatformRun([TOWER_WORKSPACE_ID: '100']) + PlatformHelper.isPlatformRun([TOWER_WORKFLOW_ID: 'wf-1']) + } + + @Unroll + def 'should resolve effective workspace id from #SOURCE'() { + expect: + PlatformHelper.getEffectiveWorkspaceId(OPTS, ENV, () -> DEFAULT) == EXPECTED + + where: + SOURCE | OPTS | ENV | DEFAULT | EXPECTED + 'config' | [workspaceId: '200']| [:] | 999L | '200' + 'env var' | [:] | [TOWER_WORKSPACE_ID: '100'] | 999L | '100' + 'platform default' | [:] | [:] | 999L | '999' + 'nothing set' | [:] | [:] | null | null + // a Platform-driven run is authoritative: never override with the account default + 'platform run' | [:] | [TOWER_WORKFLOW_ID: 'wf-1'] | 999L | null + 'platform run env' | [:] | [TOWER_WORKFLOW_ID: 'wf-1', TOWER_WORKSPACE_ID: '100'] | 999L | '100' + } + + def 'should not query the platform default when a local workspace is set'() { + given: + def queried = false + + when: + final result = PlatformHelper.getEffectiveWorkspaceId([workspaceId: '200'], [:], () -> { queried = true; 999L }) + + then: + result == '200' + !queried + } } diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy index 6c1fe0841b..a57f1e6dc8 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy @@ -55,6 +55,16 @@ class BaseCommandImpl { return new ConfigBuilder().build(configFile.exists() ? [ configFile ] : []).flatten() } + /** + * Extract the `tower` scope options from a flattened config map, stripping the + * `tower.` prefix so the result can be passed to {@code PlatformHelper}. + */ + protected Map towerOpts(Map config) { + return config + .findAll { it.key.toString().startsWith('tower.') } + .collectEntries { k, v -> [(k.toString().substring(6)): v] } + } + protected List listUserWorkspaces(TowerClient client, String userId) { return client.listUserWorkspacesAndOrgs(userId).findAll { ((Map) it).workspaceId != null } } diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy index 49c26b395a..31bd9879eb 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy @@ -24,6 +24,7 @@ import groovy.json.JsonGenerator import groovy.json.JsonOutput import groovy.json.JsonSlurper import groovy.transform.CompileStatic +import groovy.transform.Memoized import groovy.transform.TupleConstructor import groovy.util.logging.Slf4j import io.seqera.http.HxClient @@ -485,12 +486,23 @@ class TowerClient { return response.message ? new JsonSlurper().parseText(response.message) as Map : [:] } + /** + * Fetch the full {@code GET /user-info} payload, which carries both the user + * object and the account-level settings such as {@code defaultWorkspaceId}. + * + * Memoized so that the several accessors reading different fields of the same + * response share a single HTTP round-trip per client instance. + */ + @Memoized + protected Map describeUser() { + return apiGet("/user-info") + } + /** * @return current user info (id, userName, etc.) from GET /user-info */ Map getUserInfo() { - final json = apiGet("/user-info") - return json.user as Map + return describeUser().user as Map } /** @@ -502,12 +514,14 @@ class TowerClient { * Platform and is {@code null} when no default is set, when the stored default * is no longer accessible, or when the Platform version predates the feature. * + * Unlike the other accessors on this endpoint, failures are swallowed: resolving + * the default workspace is a best-effort convenience and must never abort a run. + * * @return the default workspace ID, or {@code null} if none is available */ Long getDefaultWorkspaceId() { try { - final json = apiGet("/user-info") - return json.defaultWorkspaceId as Long + return describeUser().defaultWorkspaceId as Long } catch( Exception e ) { log.debug "Unable to resolve Seqera Platform default workspace: ${e.message}" diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerConfig.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerConfig.groovy index 706add76ae..f13b372c01 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerConfig.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerConfig.groovy @@ -80,7 +80,7 @@ class TowerConfig implements ConfigScope { @ConfigOption @Description(""" - The workspace ID in Seqera Platform in which to save the run (default: the launching user's personal workspace). + The workspace ID in Seqera Platform in which to save the run (default: the user's default workspace in Seqera Platform, or the launching user's personal workspace if no default is set). """) final String workspaceId diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy index ae0e0fedde..0c470102c6 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy @@ -25,6 +25,7 @@ import nextflow.SysEnv import nextflow.exception.AbortOperationException import nextflow.file.http.XAuthProvider import nextflow.file.http.XAuthRegistry +import nextflow.platform.PlatformHelper import nextflow.trace.TraceObserverFactoryV2 import nextflow.trace.TraceObserverV2 import nextflow.util.Duration @@ -53,13 +54,13 @@ class TowerFactory implements TraceObserverFactoryV2 { // during session init and gets swallowed silently by the launcher checkAccessToken(config) final result = new ArrayList(1) - // resolve the workspace: local/CLI settings win; when none is set and this is - // not a Platform-driven run, fall back to the user's Platform default workspace - String workspaceId = config.workspaceId - if( !workspaceId && !env.get('TOWER_WORKFLOW_ID') ) - workspaceId = resolveDefaultWorkspaceId(session, env)?.toString() + final client = client(session, env) + // resolve the workspace: a local setting wins, otherwise fall back to the + // user's default workspace configured in Seqera Platform + final opts = session.config.tower as Map ?: Collections.emptyMap() + final workspaceId = PlatformHelper.getEffectiveWorkspaceId(opts, env, () -> defaultWorkspaceId(client)) // create the tower observer - result.add( new TowerObserver(session, client(session, env), workspaceId, env)) + result.add( new TowerObserver(session, client, workspaceId, env)) // create the logs checkpoint if( session.cloudCachePath ) result.add( new LogsCheckpoint() ) @@ -67,11 +68,11 @@ class TowerFactory implements TraceObserverFactoryV2 { } /** - * Resolve the user's server-side default workspace from Seqera Platform. + * Query the user's server-side default workspace from Seqera Platform. * Extracted as a seam so it can be stubbed in tests without hitting the network. */ - protected Long resolveDefaultWorkspaceId(Session session, Map env) { - return client(session, env).getDefaultWorkspaceId() + protected Long defaultWorkspaceId(TowerClient client) { + return client.getDefaultWorkspaceId() } @Memoized diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy index 65fdf2c6eb..a940366712 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy @@ -802,8 +802,7 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { final status = new ConfigStatus([], null, null, null) // Extract tower config and strip prefix for PlatformHelper - final towerConfig = config.findAll { it.key.toString().startsWith('tower.') } - .collectEntries { k, v -> [(k.toString().substring(6)): v] } + final towerConfig = towerOpts(config) // API endpoint - use PlatformHelper final String endpoint = PlatformHelper.getEndpoint(towerConfig, SysEnv.get()) @@ -839,59 +838,36 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { final enabledValue = enabledInfo.value?.toString()?.toLowerCase() in ['true', '1', 'yes'] ? 'Yes' : 'No' status.table.add(['Workflow monitoring', enabledValue, (enabledInfo.source ?: 'default') as String]) - // Default workspace - use PlatformHelper - final String workspaceId = PlatformHelper.getWorkspaceId(towerConfig, SysEnv.get()) + // Default workspace: the local setting if any, otherwise the Seqera Platform + // default workspace -- i.e. the workspace a run would actually use + final String localWorkspaceId = PlatformHelper.getWorkspaceId(towerConfig, SysEnv.get()) final workspaceInfo = getConfigValue(config, 'tower.workspaceId', 'TOWER_WORKSPACE_ID') - // the workspace effectively used for runs: the local/CLI value if set, - // otherwise the Seqera Platform server-side default workspace (if any) - String effectiveWorkspaceId = workspaceId - if( workspaceId ) { - // Try to get workspace name and roles from API if we have a token - def workspaceDetails = null - if( accessToken ) { - final httpClient = createTowerClient(endpoint, accessToken) - final userInfo = httpClient.getUserInfo() - workspaceDetails = httpClient.getUserWorkspaceDetails(userInfo.id as String, workspaceId) - } + final httpClient = accessToken ? createTowerClient(endpoint, accessToken) : null + final String effectiveWorkspaceId = PlatformHelper.getEffectiveWorkspaceId( + towerConfig, SysEnv.get(), () -> httpClient?.getDefaultWorkspaceId() ) + if( effectiveWorkspaceId ) { + // report where the value came from: the local config/env, or Platform itself + final source = localWorkspaceId ? workspaceInfo.source as String : 'platform' + // Try to get workspace name and roles from API if we have a token + final workspaceDetails = httpClient?.getUserWorkspaceDetails(httpClient.getUserInfo().id as String, effectiveWorkspaceId) + // Add workspace ID row and remember its index + status.workspaceRowIndex = status.table.size() + status.table.add(['Default workspace', effectiveWorkspaceId, source]) if( workspaceDetails ) { - // Add workspace ID row and remember its index - status.workspaceRowIndex = status.table.size() - status.table.add(['Default workspace', workspaceId, workspaceInfo.source as String]) // Store workspace details for display after this row (outside table structure) // roles are included in workspaceDetails status.workspaceInfo = workspaceDetails status.workspaceRoles = workspaceDetails.roles as List - } else { - status.table.add(['Default workspace', workspaceId, workspaceInfo.source as String]) - } - } else { - if( accessToken ) { - // No local/CLI workspace set — check for a Seqera Platform default workspace - final httpClient = createTowerClient(endpoint, accessToken) - final platformDefaultId = httpClient.getDefaultWorkspaceId()?.toString() - if( platformDefaultId ) { - effectiveWorkspaceId = platformDefaultId - final userInfo = httpClient.getUserInfo() - final workspaceDetails = httpClient.getUserWorkspaceDetails(userInfo.id as String, platformDefaultId) - if( workspaceDetails ) { - status.workspaceRowIndex = status.table.size() - status.table.add(['Default workspace', platformDefaultId, 'platform']) - status.workspaceInfo = workspaceDetails - status.workspaceRoles = workspaceDetails.roles as List - } else { - status.table.add(['Default workspace', platformDefaultId, 'platform']) - } - } else { - status.table.add(['Default workspace', 'None (Personal workspace)', 'default']) - } } } + else if( accessToken ) { + status.table.add(['Default workspace', 'None (Personal workspace)', 'default']) + } // Compute environment and work directory def computeEnv = null - if( accessToken ) { - final httpClient = createTowerClient(endpoint, accessToken) + if( httpClient ) { try { if( config['tower.computeEnvId'] ) { computeEnv = getComputeEnvironment(httpClient, config['tower.computeEnvId'] as String, effectiveWorkspaceId) diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy index fc16ac6087..86332598f2 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy @@ -27,7 +27,9 @@ import io.seqera.tower.plugin.BaseCommandImpl import io.seqera.tower.plugin.TowerClient import io.seqera.tower.plugin.exception.ForbiddenException import nextflow.BuildInfo +import nextflow.SysEnv import nextflow.cli.CmdLaunch +import nextflow.platform.PlatformHelper import nextflow.util.ColorUtil import nextflow.exception.AbortOperationException import nextflow.file.FileHelper @@ -967,13 +969,8 @@ class LaunchCommandImpl extends BaseCommandImpl implements CmdLaunch.LaunchComma // ===== Workspace & User Helper Methods ===== protected Long resolveWorkspaceId(Map config, String workspaceName, String accessToken, String apiEndpoint) { - // First check config for workspace ID - final configWorkspaceId = config['tower.workspaceId'] - if (configWorkspaceId) { - return configWorkspaceId as Long - } - - // If workspace name provided, look it up + // The `-workspace` flag is the most explicit choice, so it wins over the + // config file and the environment: look it up by name if (workspaceName) { final httpClient = createTowerClient(apiEndpoint, accessToken) final userInfo = httpClient.getUserInfo() as Map @@ -993,15 +990,13 @@ class LaunchCommandImpl extends BaseCommandImpl implements CmdLaunch.LaunchComma return (matchingWorkspace as Map).workspaceId as Long } - // No local/CLI workspace set — fall back to the Seqera Platform server-side - // default workspace, if the user (or the system) has one configured - final defaultWorkspaceId = createTowerClient(apiEndpoint, accessToken).getDefaultWorkspaceId() - if (defaultWorkspaceId) { - log.debug "Using Seqera Platform default workspace ID: ${defaultWorkspaceId}" - return defaultWorkspaceId - } - - return null + // Otherwise apply the standard precedence: `tower.workspaceId` config, then the + // TOWER_WORKSPACE_ID environment variable, then the Seqera Platform default workspace. + // The client is created lazily so no API call is made when a local setting applies + final workspaceId = PlatformHelper.getEffectiveWorkspaceId( + towerOpts(config), SysEnv.get(), () -> createTowerClient(apiEndpoint, accessToken).getDefaultWorkspaceId() ) + log.debug "Resolved workspace ID: ${workspaceId ?: 'none (personal workspace)'}" + return workspaceId ? workspaceId as Long : null } protected Map findComputeEnv(TowerClient client, String computeEnvName, Long workspaceId) { diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy index 4f25691acd..60c91e06c2 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy @@ -25,6 +25,7 @@ import io.seqera.http.HxClient import nextflow.exception.AbortRunException import nextflow.util.Duration import spock.lang.Specification +import spock.lang.Unroll /** * * @author Paolo Di Tommaso @@ -254,10 +255,16 @@ class TowerClientTest extends Specification { wireMock.stop() } - def 'should resolve the default workspace id from user-info' () { + @Unroll + def 'should resolve the default workspace id when #SCENARIO' () { given: def wireMock = new WireMockServer(0) wireMock.start() + wireMock.stubFor( + WireMock.get(WireMock.urlPathEqualTo('/user-info')) + .willReturn(WireMock.aResponse().withStatus(STATUS) + .withHeader('Content-Type', 'application/json') + .withBody(BODY))) and: TowerConfig config = Mock(TowerConfig) { getHttpReadTimeout() >> Duration.of('5 s') @@ -267,32 +274,44 @@ class TowerClientTest extends Specification { } TowerClient client = new TowerClient(config) - when: 'the user-info response carries a top-level defaultWorkspaceId' - wireMock.stubFor( - WireMock.get(WireMock.urlPathEqualTo('/user-info')) - .willReturn(WireMock.aResponse().withStatus(200) - .withHeader('Content-Type', 'application/json') - .withBody('{"user":{"id":1,"userName":"me"},"needConsent":false,"defaultWorkspaceId":42}'))) - then: - client.getDefaultWorkspaceId() == 42L + expect: + client.getDefaultWorkspaceId() == EXPECTED + + cleanup: + wireMock.stop() + + where: + SCENARIO | STATUS | BODY | EXPECTED + 'the field is present' | 200 | '{"user":{"id":1},"needConsent":false,"defaultWorkspaceId":42}' | 42L + 'the field is absent' | 200 | '{"user":{"id":1},"needConsent":false}' | null + 'the field is null' | 200 | '{"user":{"id":1},"needConsent":false,"defaultWorkspaceId":null}' | null + 'the endpoint returns an error'| 500 | 'boom' | null + } - when: 'no defaultWorkspaceId is present in the response' - wireMock.resetAll() + def 'should fetch user-info only once per client' () { + given: + def wireMock = new WireMockServer(0) + wireMock.start() wireMock.stubFor( WireMock.get(WireMock.urlPathEqualTo('/user-info')) .willReturn(WireMock.aResponse().withStatus(200) .withHeader('Content-Type', 'application/json') - .withBody('{"user":{"id":1,"userName":"me"},"needConsent":false}'))) - then: - client.getDefaultWorkspaceId() == null + .withBody('{"user":{"id":1,"userName":"me"},"defaultWorkspaceId":42}'))) + and: + TowerConfig config = Mock(TowerConfig) { + getHttpReadTimeout() >> Duration.of('5 s') + getHttpConnectTimeout() >> Duration.of('5 s') + getEndpoint() >> wireMock.baseUrl() + getAccessToken() >> 'token' + } + TowerClient client = new TowerClient(config) - when: 'the endpoint returns an error' - wireMock.resetAll() - wireMock.stubFor( - WireMock.get(WireMock.urlPathEqualTo('/user-info')) - .willReturn(WireMock.aResponse().withStatus(500).withBody('boom'))) - then: 'the failure is swallowed and null is returned' - client.getDefaultWorkspaceId() == null + when: 'both accessors of the same endpoint are used' + client.getUserInfo() + client.getDefaultWorkspaceId() + + then: 'the response is shared, not re-fetched' + wireMock.verify(1, WireMock.getRequestedFor(WireMock.urlPathEqualTo('/user-info'))) cleanup: wireMock.stop() diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy index dc265b23a0..33db2f93d1 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy @@ -32,7 +32,7 @@ class TowerFactoryTest extends Specification { // stub the Platform default-workspace lookup so no live API call is made when // no workspace is configured locally def factory = Spy(new TowerFactory(env: [TOWER_ACCESS_TOKEN: '123'])) - factory.resolveDefaultWorkspaceId(_, _) >> null + factory.defaultWorkspaceId(_) >> null when: def session = Mock(Session) { getConfig() >> [tower: [enabled: true]] } @@ -55,7 +55,7 @@ class TowerFactoryTest extends Specification { when: def observer = (TowerObserver) factory.create(session)[0] then: 'the Platform default workspace is resolved and used' - 1 * factory.resolveDefaultWorkspaceId(_, _) >> 300L + 1 * factory.defaultWorkspaceId(_) >> 300L observer.getWorkspaceId() == '300' } @@ -67,7 +67,7 @@ class TowerFactoryTest extends Specification { when: def observer = (TowerObserver) factory.create(session)[0] then: 'the local workspace wins and no default lookup is performed' - 0 * factory.resolveDefaultWorkspaceId(_, _) + 0 * factory.defaultWorkspaceId(_) observer.getWorkspaceId() == '200' } @@ -79,7 +79,7 @@ class TowerFactoryTest extends Specification { when: def observer = (TowerObserver) factory.create(session)[0] then: 'the env workspace is used and no default lookup is performed' - 0 * factory.resolveDefaultWorkspaceId(_, _) + 0 * factory.defaultWorkspaceId(_) observer.getWorkspaceId() == '100' } diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy index 88a6858a6e..30f4b854b9 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy @@ -18,11 +18,13 @@ package io.seqera.tower.plugin.launch import io.seqera.http.HxClient import io.seqera.tower.plugin.TowerClient +import nextflow.SysEnv import nextflow.cli.CmdLaunch import nextflow.exception.AbortOperationException import org.junit.Rule import spock.lang.Specification import spock.lang.TempDir +import spock.lang.Unroll import test.OutputCapture import java.nio.file.Files @@ -761,47 +763,29 @@ class LaunchCommandImplTest extends Specification { ex.message.contains('Workspace \'nonexistent\' not found') } - def 'should return null when no workspace specified and no Platform default'() { + @Unroll + def 'should resolve workspace id from #SOURCE when no name is given'() { given: - def client = Mock(TowerClient) { getDefaultWorkspaceId() >> null } + def client = Mock(TowerClient) { getDefaultWorkspaceId() >> PLATFORM_DEFAULT } def cmd = Spy(new LaunchCommandImpl()) cmd.createTowerClient(_, _) >> client - def config = [:] + SysEnv.push(ENV) when: - def workspaceId = cmd.resolveWorkspaceId(config, null, 'token', 'endpoint') + def workspaceId = cmd.resolveWorkspaceId(CONFIG, null, 'token', 'endpoint') then: - workspaceId == null - } + workspaceId == EXPECTED - def 'should use the Platform default workspace when none specified locally'() { - given: - def client = Mock(TowerClient) { getDefaultWorkspaceId() >> 999L } - def cmd = Spy(new LaunchCommandImpl()) - cmd.createTowerClient(_, _) >> client - def config = [:] + cleanup: + SysEnv.pop() - when: - def workspaceId = cmd.resolveWorkspaceId(config, null, 'token', 'endpoint') - - then: 'the Platform default workspace is used' - workspaceId == 999L - } - - def 'should prefer the config workspace over the Platform default'() { - given: - def client = Mock(TowerClient) - def cmd = Spy(new LaunchCommandImpl()) - cmd.createTowerClient(_, _) >> client - def config = ['tower.workspaceId': 12345L] - - when: - def workspaceId = cmd.resolveWorkspaceId(config, null, 'token', 'endpoint') - - then: 'the local config wins and the Platform default is never queried' - 0 * client.getDefaultWorkspaceId() - workspaceId == 12345L + where: + SOURCE | CONFIG | ENV | PLATFORM_DEFAULT | EXPECTED + 'config' | ['tower.workspaceId': 12345L] | [:] | 999L | 12345L + 'env var' | [:] | [TOWER_WORKSPACE_ID: '5000'] | 999L | 5000L + 'platform default' | [:] | [:] | 999L | 999L + 'none' | [:] | [:] | null | null } // ===== Launch Result Tests ===== From b0e7f4346503f2a57287ab465f2cef6ed8d5af29 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 08:29:17 +0000 Subject: [PATCH 3/6] Publish resolved workspace so all subsystems agree Second cleanup pass on the Platform default-workspace change. The resolved workspace was handed only to TowerObserver, so three subsystems that scope themselves to the workspace still resolved it independently and saw the personal workspace while the run was reported into the Platform default: the Fusion licence request (a hard abort), Wave registry credentials (private pulls failing as unauthorized) and the Seqera executor's run creation. This hit exactly the target user: an org member with no local tower.workspaceId. TowerFactory now publishes the resolved value into session.config.tower. workspaceId, following the existing WaveFactory precedent of a trace observer factory writing resolved facts back into the session config. All four consumers already read through PlatformHelper.getWorkspaceId(session.config.tower, env), so they are fixed without any edit at the consumers. The lookup also ran with the run-critical retry policy (10 attempts, up to 90s apart) on the session-init path, so an unreachable endpoint could stall the start of a run for minutes before falling back to the personal workspace anyway. It now uses a dedicated client bounded to a single attempt and a 10s timeout, matching its best-effort contract. Note a user-visible precedence change: `-workspace ` now takes priority over the `tower.workspaceId` config setting, where previously the config won. An explicit CLI flag beating a config file is the conventional order; it is now covered by a test and the full precedence is documented. Also in this pass: - getDefaultWorkspaceId returns String, matching every other workspace-id accessor and removing a Long/String round-trip - towerOpts moves to PlatformHelper so the two remaining hand-rolled copies of the `tower.` prefix stripping can use it, including the one in modules/nextflow - `nextflow auth config` no longer labels the no-selection case "Personal workspace" when an account default exists, and uses the isPlatformRun seam - memoize listUserWorkspacesAndOrgs: `launch -workspace NAME` fetched it twice - drop a redundant local, an inert branch and a stray null-guard in auth status Assisted-by: Claude Code Signed-off-by: Claude --- docs/reference/cli/launch.mdx | 10 +- .../main/groovy/nextflow/cli/CmdAuth.groovy | 3 +- .../nextflow/platform/PlatformHelper.groovy | 17 ++- .../platform/PlatformHelperTest.groovy | 12 +- .../tower/plugin/BaseCommandImpl.groovy | 5 +- .../io/seqera/tower/plugin/TowerClient.groovy | 10 +- .../seqera/tower/plugin/TowerFactory.groovy | 51 +++++++- .../tower/plugin/auth/AuthCommandImpl.groovy | 82 ++++++++----- .../tower/plugin/TowerClientTest.groovy | 2 +- .../tower/plugin/TowerFactoryTest.groovy | 110 +++++++----------- .../launch/LaunchCommandImplTest.groovy | 32 +++-- 11 files changed, 204 insertions(+), 130 deletions(-) diff --git a/docs/reference/cli/launch.mdx b/docs/reference/cli/launch.mdx index 93b160ef86..c50441ecc3 100644 --- a/docs/reference/cli/launch.mdx +++ b/docs/reference/cli/launch.mdx @@ -81,7 +81,15 @@ The directory where intermediate result files are stored. ##### `-workspace` -The Seqera Platform workspace name. If not specified and no workspace is configured locally (via `tower.workspaceId` or `TOWER_WORKSPACE_ID`), the run uses your default workspace configured in Seqera Platform, or your personal workspace if no default is set. +The Seqera Platform workspace name. + +The workspace is resolved in the following order: + +1. This `-workspace` option +2. The `tower.workspaceId` configuration setting +3. The `TOWER_WORKSPACE_ID` environment variable +4. The default workspace configured in your Seqera Platform account +5. Your personal workspace ##### `-workspace-secret` diff --git a/modules/nextflow/src/main/groovy/nextflow/cli/CmdAuth.groovy b/modules/nextflow/src/main/groovy/nextflow/cli/CmdAuth.groovy index 719e524819..08b5f86afa 100644 --- a/modules/nextflow/src/main/groovy/nextflow/cli/CmdAuth.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/cli/CmdAuth.groovy @@ -285,8 +285,7 @@ class CmdAuth extends CmdBase implements UsageAware { // Read config to get the actual resolved endpoint value final builder = new ConfigCmdAdapter().setHomeDir(Const.APP_HOME_DIR).setCurrentDir(Const.APP_HOME_DIR) final config = builder.buildConfigObject().flatten() - final towerConfig = config.findAll { it.key.toString().startsWith('tower.') } - .collectEntries { k, v -> [(k.toString().substring(6)): v] } + final towerConfig = PlatformHelper.towerOpts(config) def defaultEndpoint = PlatformHelper.getEndpoint(towerConfig, SysEnv.get()) result << 'Authenticate with Seqera Platform' diff --git a/modules/nextflow/src/main/groovy/nextflow/platform/PlatformHelper.groovy b/modules/nextflow/src/main/groovy/nextflow/platform/PlatformHelper.groovy index 459380967b..93651c6a57 100644 --- a/modules/nextflow/src/main/groovy/nextflow/platform/PlatformHelper.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/platform/PlatformHelper.groovy @@ -31,6 +31,19 @@ import nextflow.SysEnv @CompileStatic class PlatformHelper { + /** + * Extract the `tower` scope options from a flattened config map, stripping the + * `tower.` prefix so the result can be passed to the accessors in this class. + * + * @param flatConfig a flattened config map, e.g. `['tower.endpoint': '...']` + * @return the `tower` options keyed without the prefix, e.g. `[endpoint: '...']` + */ + static Map towerOpts(Map flatConfig) { + return flatConfig + .findAll { it.key.toString().startsWith('tower.') } + .collectEntries { k, v -> [(k.toString().substring('tower.'.length())): v] } + } + /** * A run made by Seqera Platform is signalled by the {@code TOWER_WORKFLOW_ID} * environment variable. In that case the settings must be taken from the @@ -56,12 +69,12 @@ class PlatformHelper { * @param platformDefault supplies the Platform default workspace ID, queried only when needed * @return the workspace ID to use, or null to use the personal workspace */ - static String getEffectiveWorkspaceId(Map opts, Map env, Supplier platformDefault) { + static String getEffectiveWorkspaceId(Map opts, Map env, Supplier platformDefault) { final local = getWorkspaceId(opts, env) // a local setting always wins; for a Platform-driven run the environment is authoritative if( local || isPlatformRun(env) ) return local - return platformDefault?.get()?.toString() + return platformDefault.get() } /** diff --git a/modules/nextflow/src/test/groovy/nextflow/platform/PlatformHelperTest.groovy b/modules/nextflow/src/test/groovy/nextflow/platform/PlatformHelperTest.groovy index 026d2c3cf9..fddfef6627 100644 --- a/modules/nextflow/src/test/groovy/nextflow/platform/PlatformHelperTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/platform/PlatformHelperTest.groovy @@ -139,13 +139,13 @@ class PlatformHelperTest extends Specification { where: SOURCE | OPTS | ENV | DEFAULT | EXPECTED - 'config' | [workspaceId: '200']| [:] | 999L | '200' - 'env var' | [:] | [TOWER_WORKSPACE_ID: '100'] | 999L | '100' - 'platform default' | [:] | [:] | 999L | '999' + 'config' | [workspaceId: '200']| [:] | '999' | '200' + 'env var' | [:] | [TOWER_WORKSPACE_ID: '100'] | '999' | '100' + 'platform default' | [:] | [:] | '999' | '999' 'nothing set' | [:] | [:] | null | null // a Platform-driven run is authoritative: never override with the account default - 'platform run' | [:] | [TOWER_WORKFLOW_ID: 'wf-1'] | 999L | null - 'platform run env' | [:] | [TOWER_WORKFLOW_ID: 'wf-1', TOWER_WORKSPACE_ID: '100'] | 999L | '100' + 'platform run' | [:] | [TOWER_WORKFLOW_ID: 'wf-1'] | '999' | null + 'platform run env' | [:] | [TOWER_WORKFLOW_ID: 'wf-1', TOWER_WORKSPACE_ID: '100'] | '999' | '100' } def 'should not query the platform default when a local workspace is set'() { @@ -153,7 +153,7 @@ class PlatformHelperTest extends Specification { def queried = false when: - final result = PlatformHelper.getEffectiveWorkspaceId([workspaceId: '200'], [:], () -> { queried = true; 999L }) + final result = PlatformHelper.getEffectiveWorkspaceId([workspaceId: '200'], [:], () -> { queried = true; '999' }) then: result == '200' diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy index a57f1e6dc8..3e04679a42 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy @@ -22,6 +22,7 @@ import groovy.util.logging.Slf4j import nextflow.Const import nextflow.SysEnv import nextflow.config.ConfigBuilder +import nextflow.platform.PlatformHelper import nextflow.util.Duration @Slf4j @@ -60,9 +61,7 @@ class BaseCommandImpl { * `tower.` prefix so the result can be passed to {@code PlatformHelper}. */ protected Map towerOpts(Map config) { - return config - .findAll { it.key.toString().startsWith('tower.') } - .collectEntries { k, v -> [(k.toString().substring(6)): v] } + return PlatformHelper.towerOpts(config) } protected List listUserWorkspaces(TowerClient client, String userId) { diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy index 31bd9879eb..7644b5cfad 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy @@ -519,9 +519,9 @@ class TowerClient { * * @return the default workspace ID, or {@code null} if none is available */ - Long getDefaultWorkspaceId() { + String getDefaultWorkspaceId() { try { - return describeUser().defaultWorkspaceId as Long + return describeUser().defaultWorkspaceId?.toString() } catch( Exception e ) { log.debug "Unable to resolve Seqera Platform default workspace: ${e.message}" @@ -542,6 +542,12 @@ class TowerClient { } + /** + * Memoized for the same reason as {@link #describeUser()}: the workspaces a user + * belongs to do not change within a single command, and several call sites need + * the list to map a workspace name to its ID and back again. + */ + @Memoized List listUserWorkspacesAndOrgs(String userId) { final json = apiGet("/user/${userId}/workspaces") return json.orgsAndWorkspaces as List diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy index 0c470102c6..adf4d79934 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy @@ -38,6 +38,14 @@ import nextflow.util.Duration @CompileStatic class TowerFactory implements TraceObserverFactoryV2 { + /** + * Retry budget and timeout for the best-effort default-workspace lookup performed + * during session init -- deliberately much tighter than the telemetry retry policy. + */ + private static final int LOOKUP_MAX_ATTEMPTS = 1 + + private static final Duration LOOKUP_TIMEOUT = Duration.of('10s') + private Map env TowerFactory(){ @@ -46,7 +54,8 @@ class TowerFactory implements TraceObserverFactoryV2 { @Override Collection create(Session session) { - final config = new TowerConfig(session.config.tower as Map ?: Collections.emptyMap(), env) + final opts = session.config.tower as Map ?: Collections.emptyMap() + final config = new TowerConfig(opts, env) if( !isEnabled(session, config, env) ) return Collections.emptyList() // make sure the access token is available before the client is created, otherwise the @@ -57,8 +66,14 @@ class TowerFactory implements TraceObserverFactoryV2 { final client = client(session, env) // resolve the workspace: a local setting wins, otherwise fall back to the // user's default workspace configured in Seqera Platform - final opts = session.config.tower as Map ?: Collections.emptyMap() - final workspaceId = PlatformHelper.getEffectiveWorkspaceId(opts, env, () -> defaultWorkspaceId(client)) + final workspaceId = PlatformHelper.getEffectiveWorkspaceId(opts, env, () -> defaultWorkspaceId(opts)) + // publish the resolved value back into the session config: this is the single point + // where the workspace becomes known, and other subsystems that scope themselves to + // the workspace -- Wave (registry credentials), the Fusion licence, the Seqera + // executor -- read it back via PlatformHelper.getWorkspaceId(session.config.tower, env). + // Without this they would resolve to the personal workspace while the run is + // reported into the Platform default one. + publishWorkspaceId(session, workspaceId) // create the tower observer result.add( new TowerObserver(session, client, workspaceId, env)) // create the logs checkpoint @@ -69,10 +84,36 @@ class TowerFactory implements TraceObserverFactoryV2 { /** * Query the user's server-side default workspace from Seqera Platform. + * + * This runs during session initialization, before the pipeline script is even parsed, + * so it uses a dedicated client with a bounded retry budget and short timeouts: the + * lookup is a best-effort convenience that falls back to the personal workspace, and + * it must never be able to stall the start of a run for minutes when Platform is slow + * or unreachable. The shared client's retry policy is sized for the telemetry stream, + * which is a different trade-off. + * * Extracted as a seam so it can be stubbed in tests without hitting the network. */ - protected Long defaultWorkspaceId(TowerClient client) { - return client.getDefaultWorkspaceId() + protected String defaultWorkspaceId(Map opts) { + final boundedOpts = new HashMap(opts) + boundedOpts.retryPolicy = [maxAttempts: LOOKUP_MAX_ATTEMPTS] + boundedOpts.httpConnectTimeout = LOOKUP_TIMEOUT + boundedOpts.httpReadTimeout = LOOKUP_TIMEOUT + return new TowerClient(new TowerConfig(boundedOpts, env)).getDefaultWorkspaceId() + } + + /** + * Store the resolved workspace ID in the session config so that every subsystem + * scoping itself to the workspace observes the same value. Nothing is written when + * the workspace is unset, so the personal-workspace behaviour is left untouched. + */ + protected void publishWorkspaceId(Session session, String workspaceId) { + if( !workspaceId ) + return + final config = session.config + if( config.tower == null ) + config.tower = new HashMap(1) + (config.tower as Map).workspaceId = workspaceId } @Memoized diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy index a940366712..e4e419f52e 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy @@ -282,8 +282,7 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { final authConfig = readAuthFile() final existingToken = authConfig['tower.accessToken'] // Extract tower config for PlatformHelper (strip 'tower.' prefix) - final towerConfig = authConfig.findAll { it.key.toString().startsWith('tower.') } - .collectEntries { k, v -> [(k.toString().substring(6)): v] } + final towerConfig = towerOpts(authConfig) final apiUrl = PlatformHelper.getEndpoint(towerConfig, SysEnv.get()) if( !existingToken ) { @@ -503,9 +502,8 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { * @return Map containing 'changed' (boolean) and 'metadata' (workspace info) */ private Map configureWorkspace(TowerClient client, Map config, String userId) { - // Check if TOWER_WORKFLOW_ID environment variable is set - final envWorkspaceId = SysEnv.get('TOWER_WORKFLOW_ID') - if( envWorkspaceId ) { + // a Platform-driven run already has its workspace decided for it + if( PlatformHelper.isPlatformRun(SysEnv.get()) ) { println "\nDefault workspace: ${colorize('TOWER_WORKFLOW_ID environment variable is set', 'yellow')}" printColored(" Not prompting for default workspace configuration as environment variable takes precedence", "dim") return [changed: false, metadata: null] @@ -521,8 +519,11 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { // Show current workspace setting final currentWorkspaceId = config.get('tower.workspaceId') + // when no workspace is set locally the run falls back to the workspace configured + // as default in the Seqera Platform account, so label the options accordingly + final platformDefaultId = client.getDefaultWorkspaceId() - String currentSetting = getCurrentWorkspaceName(workspaces, config.get('tower.workspaceId')) + String currentSetting = getCurrentWorkspaceName(workspaces, currentWorkspaceId, platformDefaultId) println "\nDefault workspace. Current setting: ${colorize(currentSetting, 'cyan', true)}" printColored(" Workflow runs use this workspace by default", "dim") @@ -531,23 +532,45 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { // If threshold or fewer total options, show all at once if( workspaces.size() <= WORKSPACE_SELECTION_THRESHOLD ) { - return selectWorkspaceFromAll(config, workspaces, currentWorkspaceId) + return selectWorkspaceFromAll(config, workspaces, currentWorkspaceId, platformDefaultId) } else { // Two-stage selection: org first, then workspace - return selectWorkspaceByOrg(config, orgWorkspaces, currentWorkspaceId) + return selectWorkspaceByOrg(config, orgWorkspaces, currentWorkspaceId, platformDefaultId) } } - private String getCurrentWorkspaceName(List workspaces, currentWorkspaceId) { - final currentWorkspace = workspaces.find { ((Map) it).workspaceId.toString() == currentWorkspaceId?.toString() } as Map - return currentWorkspace ? "${currentWorkspace.orgName} / ${currentWorkspace.workspaceName}" : "None (Personal workspace)" + /** + * Describe the workspace runs currently use: the one selected locally if any, + * otherwise the Seqera Platform account default, otherwise the personal workspace. + */ + private String getCurrentWorkspaceName(List workspaces, currentWorkspaceId, String platformDefaultId = null) { + final effectiveId = currentWorkspaceId ?: platformDefaultId + final currentWorkspace = workspaces.find { ((Map) it).workspaceId.toString() == effectiveId?.toString() } as Map + if( !currentWorkspace ) + return "None (Personal workspace)" + final suffix = currentWorkspaceId ? '' : ' [Seqera Platform default]' + return "${currentWorkspace.orgName} / ${currentWorkspace.workspaceName}${suffix}" } - private Map selectWorkspaceFromAll(Map config, List workspaces, final currentWorkspaceId) { + /** + * Label for the "no workspace selected locally" option. Removing the local setting no + * longer implies the personal workspace: when the account has a default workspace in + * Seqera Platform, that is what runs will use. + */ + private String noSelectionLabel(List workspaces, String platformDefaultId) { + if( !platformDefaultId ) + return 'None (Personal workspace)' + final ws = workspaces.find { ((Map) it).workspaceId.toString() == platformDefaultId } as Map + final name = ws ? "${ws.orgName} / ${ws.workspaceName}" : platformDefaultId + return "None (use Seqera Platform default: ${name})" + } + + private Map selectWorkspaceFromAll(Map config, List workspaces, final currentWorkspaceId, String platformDefaultId = null) { println "\nAvailable workspaces:" - final isPersonalWorkspace = !currentWorkspaceId - final currentIndicator = isPersonalWorkspace ? colorize(' (current)', 'bold') : '' - println " 0. ${colorize('None (Personal workspace)', 'cyan', true)} ${colorize('[no organization]', 'dim', true)}${currentIndicator}" + final noSelection = !currentWorkspaceId + final currentIndicator = noSelection ? colorize(' (current)', 'bold') : '' + final noSelectionSuffix = platformDefaultId ? '' : " ${colorize('[no organization]', 'dim', true)}" + println " 0. ${colorize(noSelectionLabel(workspaces, platformDefaultId), 'cyan', true)}${noSelectionSuffix}${currentIndicator}" // Sort workspaces by org name, then workspace name final sortedWorkspaces = workspaces.sort { Map a, Map b -> @@ -564,7 +587,7 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { } // Show current workspace and prepare prompt - final currentWorkspaceName = getCurrentWorkspaceName(sortedWorkspaces, currentWorkspaceId) + final currentWorkspaceName = getCurrentWorkspaceName(sortedWorkspaces, currentWorkspaceId, platformDefaultId) println("\n${colorize('Leave blank to keep current setting', 'bold')} (${colorize(currentWorkspaceName, 'cyan')}),") final selection = promptForNumber(colorize("or select workspace (0-${sortedWorkspaces.size()}): ", 'bold', true), 0, sortedWorkspaces.size(), true) @@ -591,13 +614,13 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { } } - private Map selectWorkspaceByOrg(Map config, Map orgWorkspaces, final currentWorkspaceId) { + private Map selectWorkspaceByOrg(Map config, Map orgWorkspaces, final currentWorkspaceId, String platformDefaultId = null) { // Get current workspace info for prompts final allWorkspaces = [] as List orgWorkspaces.values().each { workspaceList -> allWorkspaces.addAll(workspaceList as List) } - final currentWorkspaceDisplay = getCurrentWorkspaceName(allWorkspaces, currentWorkspaceId) + final currentWorkspaceDisplay = getCurrentWorkspaceName(allWorkspaces, currentWorkspaceId, platformDefaultId) // First, select organization final orgs = orgWorkspaces.keySet().toList().sort { (it as String).toLowerCase() } @@ -607,7 +630,9 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { println "\nAvailable organizations:" orgs.eachWithIndex { orgName, index -> - final displayName = orgName == 'Personal' ? 'None [Personal workspace]' : orgName + final displayName = orgName == 'Personal' + ? noSelectionLabel(allWorkspaces, platformDefaultId) + : orgName println " ${index + 1}. ${colorize(displayName as String, 'cyan', true)}" } println("\n${colorize('Leave blank to keep current setting', 'bold')} (${colorize(currentWorkspaceDisplay, 'cyan')}),") @@ -840,26 +865,25 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { // Default workspace: the local setting if any, otherwise the Seqera Platform // default workspace -- i.e. the workspace a run would actually use - final String localWorkspaceId = PlatformHelper.getWorkspaceId(towerConfig, SysEnv.get()) final workspaceInfo = getConfigValue(config, 'tower.workspaceId', 'TOWER_WORKSPACE_ID') final httpClient = accessToken ? createTowerClient(endpoint, accessToken) : null final String effectiveWorkspaceId = PlatformHelper.getEffectiveWorkspaceId( towerConfig, SysEnv.get(), () -> httpClient?.getDefaultWorkspaceId() ) if( effectiveWorkspaceId ) { - // report where the value came from: the local config/env, or Platform itself - final source = localWorkspaceId ? workspaceInfo.source as String : 'platform' + // `workspaceInfo.source` is null exactly when neither the config nor the env + // var is set, which is when the value can only have come from Platform + final source = (workspaceInfo.source ?: 'platform') as String // Try to get workspace name and roles from API if we have a token - final workspaceDetails = httpClient?.getUserWorkspaceDetails(httpClient.getUserInfo().id as String, effectiveWorkspaceId) + final userId = httpClient?.getUserInfo()?.id as String + final workspaceDetails = httpClient?.getUserWorkspaceDetails(userId, effectiveWorkspaceId) // Add workspace ID row and remember its index status.workspaceRowIndex = status.table.size() status.table.add(['Default workspace', effectiveWorkspaceId, source]) - if( workspaceDetails ) { - // Store workspace details for display after this row (outside table structure) - // roles are included in workspaceDetails - status.workspaceInfo = workspaceDetails - status.workspaceRoles = workspaceDetails.roles as List - } + // Store workspace details for display after this row (outside table structure); + // printStatus() already treats a null workspaceInfo as "no details to show" + status.workspaceInfo = workspaceDetails + status.workspaceRoles = workspaceDetails?.roles as List } else if( accessToken ) { status.table.add(['Default workspace', 'None (Personal workspace)', 'default']) diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy index 60c91e06c2..ae4c659a6c 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy @@ -282,7 +282,7 @@ class TowerClientTest extends Specification { where: SCENARIO | STATUS | BODY | EXPECTED - 'the field is present' | 200 | '{"user":{"id":1},"needConsent":false,"defaultWorkspaceId":42}' | 42L + 'the field is present' | 200 | '{"user":{"id":1},"needConsent":false,"defaultWorkspaceId":42}' | '42' 'the field is absent' | 200 | '{"user":{"id":1},"needConsent":false}' | null 'the field is null' | 200 | '{"user":{"id":1},"needConsent":false,"defaultWorkspaceId":null}' | null 'the endpoint returns an error'| 500 | 'boom' | null diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy index 33db2f93d1..e1fddee6ce 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy @@ -18,6 +18,7 @@ package io.seqera.tower.plugin import nextflow.Session import nextflow.exception.AbortOperationException +import nextflow.platform.PlatformHelper import spock.lang.Specification import spock.lang.Unroll @@ -32,7 +33,7 @@ class TowerFactoryTest extends Specification { // stub the Platform default-workspace lookup so no live API call is made when // no workspace is configured locally def factory = Spy(new TowerFactory(env: [TOWER_ACCESS_TOKEN: '123'])) - factory.defaultWorkspaceId(_) >> null + factory.defaultWorkspaceId(_ as Map) >> null when: def session = Mock(Session) { getConfig() >> [tower: [enabled: true]] } @@ -50,37 +51,40 @@ class TowerFactoryTest extends Specification { def 'should use the Platform default workspace when none is configured locally' () { given: def factory = Spy(new TowerFactory(env: [TOWER_ACCESS_TOKEN: 'xyz'])) - def session = Mock(Session) { getConfig() >> [tower: [enabled: true, accessToken: 'xyz']] } + def config = [tower: [enabled: true, accessToken: 'xyz']] + def session = Mock(Session) { getConfig() >> config } when: def observer = (TowerObserver) factory.create(session)[0] + then: 'the Platform default workspace is resolved and used' - 1 * factory.defaultWorkspaceId(_) >> 300L + 1 * factory.defaultWorkspaceId(_ as Map) >> '300' observer.getWorkspaceId() == '300' - } - def 'should not use the Platform default when a workspace is configured locally' () { - given: - def factory = Spy(new TowerFactory(env: [:])) - def session = Mock(Session) { getConfig() >> [tower: [enabled: true, workspaceId: '200', accessToken: 'xyz']] } + and: 'the resolved value is published into the session config' + config.tower.workspaceId == '300' - when: - def observer = (TowerObserver) factory.create(session)[0] - then: 'the local workspace wins and no default lookup is performed' - 0 * factory.defaultWorkspaceId(_) - observer.getWorkspaceId() == '200' + // this is what keeps Wave (registry credentials), the Fusion licence and the Seqera + // executor scoped to the same workspace the run is reported into + and: 'a consumer resolving the workspace afterwards observes the same value' + PlatformHelper.getWorkspaceId(config.tower as Map, [:]) == '300' + new TowerConfig(config.tower as Map, [:]).workspaceId == '300' } - def 'should not use the Platform default for a Platform-driven run' () { + def 'should not publish anything when resolving to the personal workspace' () { given: - def factory = Spy(new TowerFactory(env: [TOWER_WORKSPACE_ID: '100', TOWER_WORKFLOW_ID: '111222333', TOWER_ACCESS_TOKEN: 'xyz'])) - def session = Mock(Session) { getConfig() >> [tower: [enabled: true, accessToken: 'xyz']] } + def factory = Spy(new TowerFactory(env: [TOWER_ACCESS_TOKEN: 'xyz'])) + def config = [tower: [enabled: true, accessToken: 'xyz']] + def session = Mock(Session) { getConfig() >> config } when: def observer = (TowerObserver) factory.create(session)[0] - then: 'the env workspace is used and no default lookup is performed' - 0 * factory.defaultWorkspaceId(_) - observer.getWorkspaceId() == '100' + + then: + 1 * factory.defaultWorkspaceId(_ as Map) >> null + observer.getWorkspaceId() == null + and: 'no workspace key is invented' + config.tower.workspaceId == null } @Unroll @@ -115,59 +119,29 @@ class TowerFactoryTest extends Specification { result == [] } - def 'should create with workspace id'() { - // - // the workspace id is taken from the env - // - when: - def session = Mock(Session) { getConfig() >> [tower: [enabled: true, accessToken: 'xyz']] } - def factory = new TowerFactory(env: [TOWER_WORKSPACE_ID: '100']) - def observer = (TowerObserver) factory.create(session)[0] - then: - observer.getWorkspaceId() == '100' - - // - // the workspace id is taken from the config - // - when: - session = Mock(Session) { getConfig() >> [tower: [enabled: true, workspaceId: '200', accessToken: 'xyz']] } - factory = new TowerFactory(env: [:]) - observer = (TowerObserver) factory.create(session)[0] - then: - observer.getWorkspaceId() == '200' + @Unroll + def 'should create with workspace id from #SOURCE'() { + given: + def factory = Spy(new TowerFactory(env: ENV)) + def session = Mock(Session) { getConfig() >> [tower: CONFIG] } - // - // the workspace id is set both in the config and the env - // the config has the priority - // when: - session = Mock(Session) { getConfig() >> [tower: [enabled: true, workspaceId: '200', accessToken: 'xyz']] } - factory = new TowerFactory(env: [TOWER_WORKSPACE_ID: '100']) - observer = (TowerObserver) factory.create(session)[0] - then: - observer.getWorkspaceId() == '200' + def observer = (TowerObserver) factory.create(session)[0] - // - // when TOWER_WORKFLOW_ID is set is a tower launch - // then the workspace id is only taken from the env - // - when: - session = Mock(Session) { getConfig() >> [tower: [enabled: true, workspaceId: '200', accessToken: 'xyz']] } - factory = new TowerFactory(env: [TOWER_WORKSPACE_ID: '100', TOWER_WORKFLOW_ID: '111222333', TOWER_ACCESS_TOKEN: 'xyz']) - observer = (TowerObserver) factory.create(session)[0] - then: - observer.getWorkspaceId() == '100' + then: 'a local setting is available, so the Platform default is never queried' + 0 * factory.defaultWorkspaceId(_ as Map) + observer.getWorkspaceId() == EXPECTED - // - // when enabled is false but `TOWER_WORKFLOW_ID` is provided - // then the observer should be created - // - when: - session = Mock(Session) { getConfig() >> [tower: [enabled: false]]} - factory = new TowerFactory(env: [TOWER_WORKSPACE_ID: '100', TOWER_WORKFLOW_ID: '111222333', TOWER_ACCESS_TOKEN: 'xyz']) - observer = (TowerObserver) factory.create(session)[0] - then: - observer.getWorkspaceId() == '100' + where: + SOURCE | ENV | CONFIG | EXPECTED + 'the env' | [TOWER_WORKSPACE_ID: '100'] | [enabled: true, accessToken: 'xyz'] | '100' + 'the config' | [:] | [enabled: true, workspaceId: '200', accessToken: 'xyz'] | '200' + // the config has priority over the env + 'the config over the env' | [TOWER_WORKSPACE_ID: '100'] | [enabled: true, workspaceId: '200', accessToken: 'xyz'] | '200' + // a Platform-driven run is authoritative: the workspace comes from the env only + 'a Platform-driven run' | [TOWER_WORKSPACE_ID: '100', TOWER_WORKFLOW_ID: '111222333', TOWER_ACCESS_TOKEN: 'xyz'] | [enabled: true, workspaceId: '200', accessToken: 'xyz'] | '100' + // the observer is created even when `enabled` is false, because TOWER_WORKFLOW_ID is set + 'a disabled Platform run' | [TOWER_WORKSPACE_ID: '100', TOWER_WORKFLOW_ID: '111222333', TOWER_ACCESS_TOKEN: 'xyz'] | [enabled: false] | '100' } @Unroll diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy index 30f4b854b9..d01aae0735 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy @@ -712,16 +712,26 @@ class LaunchCommandImplTest extends Specification { // ===== Workspace Resolution Tests ===== - def 'should use workspace ID from config'() { - given: - def cmd = new LaunchCommandImpl() - def config = ['tower.workspaceId': 12345L] + def 'should prefer the -workspace name over the config and the environment'() { + given: 'a workspace set in the config AND in the environment' + def workspaces = [ + [workspaceId: 111, workspaceName: 'ws1'], + [workspaceId: 222, workspaceName: 'ws2'] + ] + def client = Mock(TowerClient) { getUserInfo() >> [id: 'user-123'] } + def cmd = Spy(new LaunchCommandImpl()) + cmd.createTowerClient(_, _) >> client + cmd.listUserWorkspaces(_, _) >> workspaces + SysEnv.push([TOWER_WORKSPACE_ID: '5000']) - when: - def workspaceId = cmd.resolveWorkspaceId(config, null, 'token', 'endpoint') + when: 'the -workspace flag names a different workspace' + def workspaceId = cmd.resolveWorkspaceId(['tower.workspaceId': 12345L], 'ws2', 'token', 'endpoint') - then: - workspaceId == 12345L + then: 'the explicit flag wins over both' + workspaceId == 222 + + cleanup: + SysEnv.pop() } def 'should lookup workspace by name'() { @@ -782,9 +792,9 @@ class LaunchCommandImplTest extends Specification { where: SOURCE | CONFIG | ENV | PLATFORM_DEFAULT | EXPECTED - 'config' | ['tower.workspaceId': 12345L] | [:] | 999L | 12345L - 'env var' | [:] | [TOWER_WORKSPACE_ID: '5000'] | 999L | 5000L - 'platform default' | [:] | [:] | 999L | 999L + 'config' | ['tower.workspaceId': 12345L] | [:] | '999' | 12345L + 'env var' | [:] | [TOWER_WORKSPACE_ID: '5000'] | '999' | 5000L + 'platform default' | [:] | [:] | '999' | 999L 'none' | [:] | [:] | null | null } From cfdd0151f5f12e0dffe056ac380c7ea794e3ccb1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 10:54:48 +0000 Subject: [PATCH 4/6] Only publish the workspace when it came from Platform Third cleanup pass on the Platform default-workspace change. The previous commit published the resolved workspace into the session config unconditionally, so it also rewrote values the user had set: on a Platform-driven run with `tower.workspaceId` in the config and TOWER_WORKSPACE_ID in the environment it replaced the former with the latter, and with only the env var set it invented a config key nobody wrote. That was harmless only because PlatformHelper.getWorkspaceId ignores the config on a Platform-driven run -- the safety came from a rule enforced somewhere else rather than from this code. The publish is now guarded on there being no local setting at all, which is the only case the mechanism exists for, and it logs the workspace it picked so a run landing in an unconfigured workspace is no longer unexplained. The bounded lookup added in the previous commit had no test: every test stubbed the seam itself, so renaming a timeout field in TowerConfig would have silently restored the 10-attempt, ~3-minute retry policy on the session-init path with a green build. The bounding now lives in TowerConfig.forLookup(), which is pure and directly tested, and is shared by all three sites that make this lookup -- `nextflow launch` and `nextflow auth status` were still using the unbounded policy for the same best-effort call. Also fixes a pre-existing trap that this change depends on: TowerRetryPolicy resolved its defaults with the elvis operator, and 0 is falsy in Groovy, so `maxAttempts = 0` ("do not retry") silently became 10 attempts and `jitter = 0` became 0.25. Values are now resolved with explicit null checks, and a maxAttempts that would never run the operation is rejected rather than silently replaced. Smaller items: reuse PlatformHelper.towerOpts for the last hand-rolled copy of the prefix stripping; note at the Wave, Fusion and Seqera-executor read sites that the workspace may have been resolved during session init; make the "nothing was published" assertion able to fail; drop a test wholly subsumed by another and a stub whose arity never matched. Assisted-by: Claude Code Signed-off-by: Claude --- .../io/seqera/executor/SeqeraExecutor.groovy | 2 + .../tower/plugin/BaseCommandImpl.groovy | 11 +++++ .../io/seqera/tower/plugin/TowerClient.groovy | 2 +- .../io/seqera/tower/plugin/TowerConfig.groovy | 31 ++++++++++++ .../seqera/tower/plugin/TowerFactory.groovy | 48 +++++++++---------- .../tower/plugin/TowerFusionToken.groovy | 2 + .../tower/plugin/TowerRetryPolicy.groovy | 31 +++++++++--- .../tower/plugin/auth/AuthCommandImpl.groovy | 23 +++++---- .../plugin/launch/LaunchCommandImpl.groovy | 5 +- .../tower/plugin/TowerConfigTest.groovy | 34 +++++++++++++ .../tower/plugin/TowerFactoryTest.groovy | 19 +++++++- .../tower/plugin/TowerRetryPolicyTest.groovy | 36 ++++++++++++++ .../plugin/auth/AuthCommandImplTest.groovy | 4 +- .../launch/LaunchCommandImplTest.groovy | 25 ++-------- .../io/seqera/wave/plugin/WaveClient.groovy | 2 + 15 files changed, 204 insertions(+), 71 deletions(-) diff --git a/plugins/nf-seqera/src/main/io/seqera/executor/SeqeraExecutor.groovy b/plugins/nf-seqera/src/main/io/seqera/executor/SeqeraExecutor.groovy index 240252c366..8ed0dd1123 100644 --- a/plugins/nf-seqera/src/main/io/seqera/executor/SeqeraExecutor.groovy +++ b/plugins/nf-seqera/src/main/io/seqera/executor/SeqeraExecutor.groovy @@ -126,6 +126,8 @@ class SeqeraExecutor extends Executor implements ExtensionPoint { final workflowId = session.workflowMetadata?.platform?.workflowId this.workflowId = workflowId final workflowUrl = session.workflowMetadata?.platform?.workflowUrl + // note: the workspace may have been resolved from the Seqera Platform account default + // and written into `session.config.tower` by TowerFactory during session init final workspaceId = PlatformHelper.getWorkspaceId(towerConfig, SysEnv.get()) as Long final computeEnvId = PlatformHelper.getComputeEnvId(towerConfig, SysEnv.get()) ?: seqeraConfig.computeEnvId diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy index 3e04679a42..e28371fd5e 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy @@ -42,6 +42,17 @@ class BaseCommandImpl { return new TowerClient( new TowerConfig( [accessToken: accessToken, endpoint: apiUrl, httpConnectTimeout: Duration.of(API_TIMEOUT_MS)], SysEnv.get())) } + /** + * Client for a best-effort lookup whose failure is not fatal, bounded so that a slow + * or unreachable Platform cannot make an interactive command hang for minutes. + * + * @see TowerConfig#forLookup + */ + @Memoized + protected TowerClient createLookupClient(String apiUrl, String accessToken) { + return new TowerClient( TowerConfig.forLookup([accessToken: accessToken, endpoint: apiUrl], SysEnv.get()) ) + } + /** * Convert API endpoint to web URL * e.g., https://api.cloud.seqera.io -> https://cloud.seqera.io diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy index 7644b5cfad..9a93200adf 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy @@ -544,7 +544,7 @@ class TowerClient { /** * Memoized for the same reason as {@link #describeUser()}: the workspaces a user - * belongs to do not change within a single command, and several call sites need + * belongs to do not change for the life of the client, and several call sites need * the list to map a workspace name to its ID and back again. */ @Memoized diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerConfig.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerConfig.groovy index f13b372c01..86332cf636 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerConfig.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerConfig.groovy @@ -40,6 +40,13 @@ class TowerConfig implements ConfigScope { static final Duration DEFAULT_READ_TIMEOUT = Duration.of('60s') + /** + * Timeout applied to a best-effort lookup. Note this is a *per-phase* budget: it is + * used for both the connect and the read phase, so the worst case for one request is + * roughly twice this value. + */ + static final Duration LOOKUP_PHASE_TIMEOUT = Duration.of('10s') + @ConfigOption @Description(""" The unique access token for your Seqera Platform account. @@ -84,6 +91,30 @@ class TowerConfig implements ConfigScope { """) final String workspaceId + /** + * Build a config for a best-effort lookup: one that improves the experience when it + * succeeds but must never delay or abort the operation when Platform is slow or + * unreachable, because the caller has a working fallback. + * + * The default retry policy is sized for the telemetry stream -- 10 attempts with + * exponential backoff, up to roughly three minutes -- which is the wrong trade-off + * for such a lookup, especially on the session-init path where it delays the start of + * every run. This bounds it to a single attempt with short timeouts. + * + * The caller's options are copied, so the map passed in is never modified. + * + * @param opts the `tower` scope options, keyed without the prefix + * @param env the applicable environment variables + * @return a {@link TowerConfig} identical to the caller's except for retries and timeouts + */ + static TowerConfig forLookup(Map opts, Map env) { + final bounded = new HashMap(opts) + bounded.retryPolicy = [maxAttempts: 1] + bounded.httpConnectTimeout = LOOKUP_PHASE_TIMEOUT + bounded.httpReadTimeout = LOOKUP_PHASE_TIMEOUT + return new TowerConfig(bounded, env) + } + /* required by extension point -- do not remove */ TowerConfig() {} diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy index adf4d79934..30f91c3b86 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy @@ -38,14 +38,6 @@ import nextflow.util.Duration @CompileStatic class TowerFactory implements TraceObserverFactoryV2 { - /** - * Retry budget and timeout for the best-effort default-workspace lookup performed - * during session init -- deliberately much tighter than the telemetry retry policy. - */ - private static final int LOOKUP_MAX_ATTEMPTS = 1 - - private static final Duration LOOKUP_TIMEOUT = Duration.of('10s') - private Map env TowerFactory(){ @@ -66,14 +58,16 @@ class TowerFactory implements TraceObserverFactoryV2 { final client = client(session, env) // resolve the workspace: a local setting wins, otherwise fall back to the // user's default workspace configured in Seqera Platform + final local = PlatformHelper.getWorkspaceId(opts, env) final workspaceId = PlatformHelper.getEffectiveWorkspaceId(opts, env, () -> defaultWorkspaceId(opts)) - // publish the resolved value back into the session config: this is the single point - // where the workspace becomes known, and other subsystems that scope themselves to - // the workspace -- Wave (registry credentials), the Fusion licence, the Seqera - // executor -- read it back via PlatformHelper.getWorkspaceId(session.config.tower, env). - // Without this they would resolve to the personal workspace while the run is - // reported into the Platform default one. - publishWorkspaceId(session, workspaceId) + // when -- and only when -- the workspace came from the Platform account default, + // publish it back into the session config. Other subsystems that scope themselves + // to the workspace -- Wave (registry credentials), the Fusion licence, the Seqera + // executor -- read it via PlatformHelper.getWorkspaceId(session.config.tower, env) + // and would otherwise resolve to the personal workspace while the run is reported + // into the Platform default one. Anything the user set is left exactly as it is. + if( !local && workspaceId ) + publishDefaultWorkspaceId(session, workspaceId) // create the tower observer result.add( new TowerObserver(session, client, workspaceId, env)) // create the logs checkpoint @@ -95,25 +89,27 @@ class TowerFactory implements TraceObserverFactoryV2 { * Extracted as a seam so it can be stubbed in tests without hitting the network. */ protected String defaultWorkspaceId(Map opts) { - final boundedOpts = new HashMap(opts) - boundedOpts.retryPolicy = [maxAttempts: LOOKUP_MAX_ATTEMPTS] - boundedOpts.httpConnectTimeout = LOOKUP_TIMEOUT - boundedOpts.httpReadTimeout = LOOKUP_TIMEOUT - return new TowerClient(new TowerConfig(boundedOpts, env)).getDefaultWorkspaceId() + return new TowerClient(TowerConfig.forLookup(opts, env)).getDefaultWorkspaceId() } /** - * Store the resolved workspace ID in the session config so that every subsystem - * scoping itself to the workspace observes the same value. Nothing is written when - * the workspace is unset, so the personal-workspace behaviour is left untouched. + * Store the workspace resolved from the Seqera Platform account default in the session + * config, so that every subsystem scoping itself to the workspace observes it. + * + * Only ever called when the user set no workspace of their own, so this never + * overwrites a configured value. + * + * Note this relies on {@code session.config} being a plain map: it is normalized from + * the parsed {@code ConfigObject} by {@code ConfigCmdAdapter.normalize0} and converted + * in the {@link Session} constructor, so an absent key really does read as null here. */ - protected void publishWorkspaceId(Session session, String workspaceId) { - if( !workspaceId ) - return + protected void publishDefaultWorkspaceId(Session session, String workspaceId) { final config = session.config if( config.tower == null ) config.tower = new HashMap(1) (config.tower as Map).workspaceId = workspaceId + // tell the user why their run is landing in a workspace they did not configure + log.info "Using default workspace configured in your Seqera Platform account: $workspaceId" } @Memoized diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFusionToken.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFusionToken.groovy index dae4d5ea07..ff3de22c7f 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFusionToken.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFusionToken.groovy @@ -85,6 +85,8 @@ class TowerFusionToken implements FusionToken { this.accessToken = PlatformHelper.getAccessToken(config, env) this.refreshToken = PlatformHelper.getRefreshToken(config, env) this.workflowId = env.get('TOWER_WORKFLOW_ID') + // note: the workspace may have been resolved from the Seqera Platform account default + // and written into `session.config.tower` by TowerFactory during session init this.workspaceId = PlatformHelper.getWorkspaceId(config, env) this.client = TowerFactory.client() } diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerRetryPolicy.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerRetryPolicy.groovy index 83112a65b3..da5a352ed3 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerRetryPolicy.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerRetryPolicy.groovy @@ -64,7 +64,7 @@ class TowerRetryPolicy implements Retryable.Config, ConfigScope { @ConfigOption @Description(""" - Maximum number of retry attempts for Tower operations (default: `10`). + Maximum number of attempts for Tower operations, including the initial one (default: `10`). Use `-1` for no limit. """) int maxAttempts @@ -81,10 +81,29 @@ class TowerRetryPolicy implements Retryable.Config, ConfigScope { double multiplier TowerRetryPolicy(Map opts, Map legacy=Map.of()) { - this.delay = opts.delay as Duration ?: legacy.backOffDelay as Duration ?: RetryConfig.DEFAULT_DELAY - this.maxDelay = opts.maxDelay as Duration ?: RetryConfig.DEFAULT_MAX_DELAY - this.maxAttempts = opts.maxAttempts as Integer ?: legacy.maxRetries as Integer ?: DEFAULT_MAX_ATTEMPTS - this.jitter = opts.jitter as Double ?: RetryConfig.DEFAULT_JITTER - this.multiplier = opts.multiplier as Double ?: legacy.backOffBase as Double ?: RetryConfig.DEFAULT_MULTIPLIER + // note: use explicit null checks rather than the elvis operator, because 0 and 0.0 + // are falsy in Groovy -- `jitter = 0` or `maxAttempts = 0` are meaningful settings + // and must not be silently replaced by the defaults + this.delay = firstOf(opts.delay, legacy.backOffDelay, RetryConfig.DEFAULT_DELAY) as Duration + this.maxDelay = firstOf(opts.maxDelay, RetryConfig.DEFAULT_MAX_DELAY) as Duration + this.maxAttempts = firstOf(opts.maxAttempts, legacy.maxRetries, DEFAULT_MAX_ATTEMPTS) as Integer + this.jitter = firstOf(opts.jitter, RetryConfig.DEFAULT_JITTER) as Double + this.multiplier = firstOf(opts.multiplier, legacy.backOffBase, RetryConfig.DEFAULT_MULTIPLIER) as Double + // `maxAttempts` counts the initial attempt, so anything below 1 would mean "never + // run the operation at all"; the underlying retry library rejects 0 outright. + // -1 is the library's documented value for "retry indefinitely" and is allowed. + if( maxAttempts == 0 || maxAttempts < -1 ) + throw new IllegalArgumentException("Invalid Seqera Platform retry policy: 'maxAttempts' must be 1 or greater, or -1 for no limit -- offending value: $maxAttempts") + } + + /** + * Return the first non-null value, so that a legitimate falsy setting such as + * `0` or `0.0` is honoured instead of falling through to the default. + */ + private static Object firstOf(Object... values) { + for( Object it : values ) + if( it != null ) + return it + return null } } diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy index e4e419f52e..fc74715f9b 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy @@ -846,9 +846,11 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { final tokenInfo = getConfigValue(config, 'tower.accessToken', 'TOWER_ACCESS_TOKEN') final String tokenSource = tokenInfo.source ?: 'not set' - if( accessToken ) { + final httpClient = accessToken ? createTowerClient(endpoint, accessToken) : null + + if( httpClient ) { try { - final userInfo = createTowerClient(endpoint, accessToken).getUserInfo() + final userInfo = httpClient.getUserInfo() final currentUser = userInfo.userName as String status.table.add(['Authentication', "${colorize('✔ OK', 'green')} (user: $currentUser)".toString(), tokenSource]) } catch( Exception e ) { @@ -866,13 +868,13 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { // Default workspace: the local setting if any, otherwise the Seqera Platform // default workspace -- i.e. the workspace a run would actually use final workspaceInfo = getConfigValue(config, 'tower.workspaceId', 'TOWER_WORKSPACE_ID') - final httpClient = accessToken ? createTowerClient(endpoint, accessToken) : null final String effectiveWorkspaceId = PlatformHelper.getEffectiveWorkspaceId( - towerConfig, SysEnv.get(), () -> httpClient?.getDefaultWorkspaceId() ) + towerConfig, SysEnv.get(), + () -> accessToken ? createLookupClient(endpoint, accessToken).getDefaultWorkspaceId() : null ) if( effectiveWorkspaceId ) { - // `workspaceInfo.source` is null exactly when neither the config nor the env - // var is set, which is when the value can only have come from Platform + // when neither the config nor the env var is set there is no local source to + // report, so the value can only have come from the Platform account default final source = (workspaceInfo.source ?: 'platform') as String // Try to get workspace name and roles from API if we have a token final userId = httpClient?.getUserInfo()?.id as String @@ -1077,18 +1079,15 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { Files.createDirectories(configFile.parent) } - // Write tower config to seqera-auth.config file - final towerConfig = config.findAll { key, value -> - key.toString().startsWith('tower.') - } + // Write tower config to seqera-auth.config file, keyed without the `tower.` prefix + final towerConfig = towerOpts(config) final authConfigText = new StringBuilder() authConfigText.append("// Seqera Platform configuration\n") authConfigText.append("tower {\n") for (entry in towerConfig) { - final key = entry.key final value = entry.value - final configKey = key.toString().substring(6) // Remove "tower." prefix + final configKey = entry.key.toString() if (value instanceof String) { def line = " ${configKey} = '${value}'" diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy index 86332598f2..0689fe6918 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy @@ -992,9 +992,10 @@ class LaunchCommandImpl extends BaseCommandImpl implements CmdLaunch.LaunchComma // Otherwise apply the standard precedence: `tower.workspaceId` config, then the // TOWER_WORKSPACE_ID environment variable, then the Seqera Platform default workspace. - // The client is created lazily so no API call is made when a local setting applies + // The client is created lazily so no API call is made when a local setting applies, + // and it is bounded so a slow Platform cannot hang the command final workspaceId = PlatformHelper.getEffectiveWorkspaceId( - towerOpts(config), SysEnv.get(), () -> createTowerClient(apiEndpoint, accessToken).getDefaultWorkspaceId() ) + towerOpts(config), SysEnv.get(), () -> createLookupClient(apiEndpoint, accessToken).getDefaultWorkspaceId() ) log.debug "Resolved workspace ID: ${workspaceId ?: 'none (personal workspace)'}" return workspaceId ? workspaceId as Long : null } diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerConfigTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerConfigTest.groovy index d7f0de5d2b..7c91f615a8 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerConfigTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerConfigTest.groovy @@ -71,4 +71,38 @@ class TowerConfigTest extends Specification { config.httpConnectTimeout == Duration.of('5s') config.httpReadTimeout == Duration.of('2m') } + + def 'should bound retries and timeouts for a best-effort lookup'() { + given: 'options carrying the telemetry retry policy and long timeouts' + def opts = [ + accessToken : 'xyz', + endpoint : 'http://foo.com', + httpConnectTimeout: Duration.of('60s'), + httpReadTimeout : Duration.of('60s') ] + + when: + def config = TowerConfig.forLookup(opts, [:]) + + then: 'the lookup gets a single attempt and short timeouts' + config.retryPolicy.maxAttempts == 1 + config.httpConnectTimeout == TowerConfig.LOOKUP_PHASE_TIMEOUT + config.httpReadTimeout == TowerConfig.LOOKUP_PHASE_TIMEOUT + + and: 'everything else is carried over unchanged' + config.accessToken == 'xyz' + config.endpoint == 'http://foo.com' + + and: 'the caller options are not modified' + opts.httpConnectTimeout == Duration.of('60s') + opts.httpReadTimeout == Duration.of('60s') + !opts.containsKey('retryPolicy') + } + + def 'should bound a lookup even when the config asks for many retries'() { + when: 'the user configured an aggressive retry policy for telemetry' + def config = TowerConfig.forLookup([accessToken: 'xyz', retryPolicy: [maxAttempts: 10]], [:]) + + then: 'the lookup is still bounded to one attempt' + config.retryPolicy.maxAttempts == 1 + } } diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy index e1fddee6ce..53f06d793f 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy @@ -84,7 +84,24 @@ class TowerFactoryTest extends Specification { 1 * factory.defaultWorkspaceId(_ as Map) >> null observer.getWorkspaceId() == null and: 'no workspace key is invented' - config.tower.workspaceId == null + !(config.tower as Map).containsKey('workspaceId') + } + + def 'should not publish over a workspace the user configured' () { + given: + def factory = Spy(new TowerFactory(env: [TOWER_WORKSPACE_ID: '100', TOWER_ACCESS_TOKEN: 'xyz'])) + def config = [tower: [enabled: true, workspaceId: '200', accessToken: 'xyz']] + def session = Mock(Session) { getConfig() >> config } + + when: + def observer = (TowerObserver) factory.create(session)[0] + + then: 'the local setting is used and the Platform default is not queried' + 0 * factory.defaultWorkspaceId(_ as Map) + observer.getWorkspaceId() == '200' + + and: 'the config the user wrote is left exactly as it was' + (config.tower as Map).workspaceId == '200' } @Unroll diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerRetryPolicyTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerRetryPolicyTest.groovy index 5ecaf58eb0..36b44a624a 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerRetryPolicyTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerRetryPolicyTest.groovy @@ -70,4 +70,40 @@ class TowerRetryPolicyTest extends Specification { policy.maxDelay == RetryConfig.DEFAULT_MAX_DELAY policy.jitter == RetryConfig.DEFAULT_JITTER } + + def 'should honour a zero jitter instead of falling back to the default'() { + when: 'jitter is explicitly disabled -- 0.0 is falsy in Groovy' + def policy = new TowerRetryPolicy([jitter: 0]) + + then: + policy.jitter == 0d + } + + def 'should honour a single attempt'() { + when: + def policy = new TowerRetryPolicy([maxAttempts: 1]) + + then: + policy.maxAttempts == 1 + } + + def 'should allow an unlimited retry policy'() { + when: + def policy = new TowerRetryPolicy([maxAttempts: -1]) + + then: + policy.maxAttempts == -1 + } + + def 'should reject a maxAttempts value that would never run the operation'() { + when: 'a user writes 0 meaning "do not retry" -- previously this silently became 10' + new TowerRetryPolicy([maxAttempts: VALUE]) + + then: + def e = thrown(IllegalArgumentException) + e.message.contains('maxAttempts') + + where: + VALUE << [0, -2] + } } diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/auth/AuthCommandImplTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/auth/AuthCommandImplTest.groovy index 773dfc92d1..30031b65ca 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/auth/AuthCommandImplTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/auth/AuthCommandImplTest.groovy @@ -902,7 +902,7 @@ param2 = 'value2'""" def config = ['tower.accessToken': 'test-token'] def client = Mock(TowerClient) { getUserInfo() >> [userName: 'testuser', id: '123'] - getDefaultWorkspaceId() >> 777L + getDefaultWorkspaceId() >> '777' getUserWorkspaceDetails(_, _) >> [ orgName: 'TestOrg', workspaceName: 'DefaultWorkspace', @@ -911,6 +911,8 @@ param2 = 'value2'""" } def cmd = Spy(new AuthCommandImpl()) cmd.createTowerClient(_,_) >> client + // the Platform default lookup uses a bounded client + cmd.createLookupClient(_,_) >> client cmd.checkApiConnection(_) >> true cmd.listComputeEnvironments(_, _) >> [] SysEnv.push([:]) // isolate from real environment variables diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy index d01aae0735..a7317a5701 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/launch/LaunchCommandImplTest.groovy @@ -734,27 +734,6 @@ class LaunchCommandImplTest extends Specification { SysEnv.pop() } - def 'should lookup workspace by name'() { - given: - def config = [:] - def workspaces = [ - [workspaceId: 111, workspaceName: 'ws1'], - [workspaceId: 222, workspaceName: 'ws2'] - ] - def client = Mock(TowerClient) { - getUserInfo() >> [id: 'user-123'] - } - def cmd = Spy(new LaunchCommandImpl()) - cmd.createTowerClient(_,_) >> client - cmd.listUserWorkspaces(_, _) >> workspaces - - when: - def workspaceId = cmd.resolveWorkspaceId(config, 'ws2', 'token', 'endpoint') - - then: - workspaceId == 222 - } - def 'should throw error when workspace not found by name'() { given: def config = [:] @@ -763,7 +742,7 @@ class LaunchCommandImplTest extends Specification { } def cmd = Spy(new LaunchCommandImpl()) cmd.createTowerClient(_,_) >> client - cmd.listUserWorkspaces(_, _, _) >> [] + cmd.listUserWorkspaces(_, _) >> [] when: cmd.resolveWorkspaceId(config, 'nonexistent', 'token', 'endpoint') @@ -779,6 +758,8 @@ class LaunchCommandImplTest extends Specification { def client = Mock(TowerClient) { getDefaultWorkspaceId() >> PLATFORM_DEFAULT } def cmd = Spy(new LaunchCommandImpl()) cmd.createTowerClient(_, _) >> client + // the Platform default lookup uses a bounded client + cmd.createLookupClient(_, _) >> client SysEnv.push(ENV) when: diff --git a/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveClient.groovy b/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveClient.groovy index 7118591744..64baf81247 100644 --- a/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveClient.groovy +++ b/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveClient.groovy @@ -122,6 +122,8 @@ class WaveClient { this.session = session this.config = new WaveConfig(session.config.wave as Map ?: Collections.emptyMap(), SysEnv.get()) this.fusion = new FusionConfig(session.config.fusion as Map ?: Collections.emptyMap(), SysEnv.get()) + // note: the workspace may have been resolved from the Seqera Platform account default + // and written into `session.config.tower` by TowerFactory during session init this.tower = new TowerConfig(session.config.tower as Map ?: Collections.emptyMap(), SysEnv.get()) this.awsFargate = WaveFactory.isAwsBatchFargateMode(session.config) this.s5cmdConfigUrl = session.config.navigate('wave.s5cmdConfigUrl') as URL From bc1ec017d5fdcb3fc54f2e1c438e6760835c39e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 11:13:17 +0000 Subject: [PATCH 5/6] Bound the workspace lookup only where it helps Fourth cleanup pass on the Platform default-workspace change. The previous commit routed the CLI commands through a separate bounded client for the default-workspace lookup. That turns out to cost without buying anything: `nextflow launch` and `nextflow auth status` both make other calls through the shared client in the same flow, and those keep the default 10-attempt retry policy, so the command can still hang for minutes regardless. Meanwhile the second client has its own `describeUser()` memo and its own connection pool, so each command paid an extra `GET /user-info` and an extra TLS handshake. Both CLI sites go back to the shared client, and the bounded config is kept for `TowerFactory` alone, where the lookup runs unattended during session init and there is no earlier unbounded call. An invalid `tower.retryPolicy.maxAttempts` now warns and falls back to a single attempt instead of aborting. Throwing would have broken configs that work today -- `maxAttempts = 0` is currently ignored and silently means 10 -- and the default-workspace feature never depended on that validation, since the bounded config passes an explicit 1. Note the same falsy-elvis trap exists in six sibling retry classes (nf-wave, nf-seqera, nf-k8s, nf-google, nf-azure and the SRA datasource). They are left alone here: routing them all through a shared helper is its own change. Also: document the session-config coupling once on the javadoc of PlatformHelper.getWorkspaceId -- the function all the affected consumers call -- instead of three copies of a comment that the previous commit had already made stale; drop the BaseCommandImpl.towerOpts pass-through in favour of the PlatformHelper static; correct the lookup timeout doc, which overstated the worst case; remove a dead import and collapse two duplicated retry tests. Assisted-by: Claude Code Signed-off-by: Claude --- .../nextflow/platform/PlatformHelper.groovy | 8 +++++ .../io/seqera/executor/SeqeraExecutor.groovy | 4 +-- .../tower/plugin/BaseCommandImpl.groovy | 20 ------------ .../io/seqera/tower/plugin/TowerConfig.groovy | 12 +++---- .../seqera/tower/plugin/TowerFactory.groovy | 1 - .../tower/plugin/TowerFusionToken.groovy | 4 +-- .../tower/plugin/TowerRetryPolicy.groovy | 16 +++++++--- .../tower/plugin/auth/AuthCommandImpl.groovy | 9 +++--- .../plugin/launch/LaunchCommandImpl.groovy | 4 +-- .../tower/plugin/TowerConfigTest.groovy | 4 +-- .../tower/plugin/TowerRetryPolicyTest.groovy | 32 ++++++++----------- .../io/seqera/wave/plugin/WaveClient.groovy | 4 +-- 12 files changed, 52 insertions(+), 66 deletions(-) diff --git a/modules/nextflow/src/main/groovy/nextflow/platform/PlatformHelper.groovy b/modules/nextflow/src/main/groovy/nextflow/platform/PlatformHelper.groovy index 93651c6a57..a4cb30d0e6 100644 --- a/modules/nextflow/src/main/groovy/nextflow/platform/PlatformHelper.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/platform/PlatformHelper.groovy @@ -188,6 +188,14 @@ class PlatformHelper { * Return the Platform Workspace ID: if `TOWER_WORKFLOW_ID` is provided in the environment, it means we are running * in a Platform-made run and we should ONLY retrieve the workspace ID from the environment. Otherwise, check the * configuration or fallback to the environment. If no workspace ID is found, return null. + * + * Note for callers passing the `tower` scope of a live session config: when the user set no + * workspace of their own, the workspace configured as default in their Seqera Platform + * account is resolved during session init and written into that map, so this can return a + * value that appears nowhere in the user's configuration files. That is deliberate -- it is + * what keeps the run, Wave, the Fusion licence and the Seqera executor in the same + * workspace. See {@code TowerFactory.publishDefaultWorkspaceId}. + * * @param opts * @param env * @return diff --git a/plugins/nf-seqera/src/main/io/seqera/executor/SeqeraExecutor.groovy b/plugins/nf-seqera/src/main/io/seqera/executor/SeqeraExecutor.groovy index 8ed0dd1123..08a9a15b73 100644 --- a/plugins/nf-seqera/src/main/io/seqera/executor/SeqeraExecutor.groovy +++ b/plugins/nf-seqera/src/main/io/seqera/executor/SeqeraExecutor.groovy @@ -126,8 +126,8 @@ class SeqeraExecutor extends Executor implements ExtensionPoint { final workflowId = session.workflowMetadata?.platform?.workflowId this.workflowId = workflowId final workflowUrl = session.workflowMetadata?.platform?.workflowUrl - // note: the workspace may have been resolved from the Seqera Platform account default - // and written into `session.config.tower` by TowerFactory during session init + // note: the workspace may have been resolved during session init -- see the + // PlatformHelper.getWorkspaceId javadoc final workspaceId = PlatformHelper.getWorkspaceId(towerConfig, SysEnv.get()) as Long final computeEnvId = PlatformHelper.getComputeEnvId(towerConfig, SysEnv.get()) ?: seqeraConfig.computeEnvId diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy index e28371fd5e..6c1fe0841b 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/BaseCommandImpl.groovy @@ -22,7 +22,6 @@ import groovy.util.logging.Slf4j import nextflow.Const import nextflow.SysEnv import nextflow.config.ConfigBuilder -import nextflow.platform.PlatformHelper import nextflow.util.Duration @Slf4j @@ -42,17 +41,6 @@ class BaseCommandImpl { return new TowerClient( new TowerConfig( [accessToken: accessToken, endpoint: apiUrl, httpConnectTimeout: Duration.of(API_TIMEOUT_MS)], SysEnv.get())) } - /** - * Client for a best-effort lookup whose failure is not fatal, bounded so that a slow - * or unreachable Platform cannot make an interactive command hang for minutes. - * - * @see TowerConfig#forLookup - */ - @Memoized - protected TowerClient createLookupClient(String apiUrl, String accessToken) { - return new TowerClient( TowerConfig.forLookup([accessToken: accessToken, endpoint: apiUrl], SysEnv.get()) ) - } - /** * Convert API endpoint to web URL * e.g., https://api.cloud.seqera.io -> https://cloud.seqera.io @@ -67,14 +55,6 @@ class BaseCommandImpl { return new ConfigBuilder().build(configFile.exists() ? [ configFile ] : []).flatten() } - /** - * Extract the `tower` scope options from a flattened config map, stripping the - * `tower.` prefix so the result can be passed to {@code PlatformHelper}. - */ - protected Map towerOpts(Map config) { - return PlatformHelper.towerOpts(config) - } - protected List listUserWorkspaces(TowerClient client, String userId) { return client.listUserWorkspacesAndOrgs(userId).findAll { ((Map) it).workspaceId != null } } diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerConfig.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerConfig.groovy index 86332cf636..b474ce4f70 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerConfig.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerConfig.groovy @@ -41,11 +41,11 @@ class TowerConfig implements ConfigScope { static final Duration DEFAULT_READ_TIMEOUT = Duration.of('60s') /** - * Timeout applied to a best-effort lookup. Note this is a *per-phase* budget: it is - * used for both the connect and the read phase, so the worst case for one request is - * roughly twice this value. + * Timeout applied to a best-effort lookup. It is used for both the connect and the read + * budget; since the read timeout is applied as an overall request timeout covering + * connect, TLS and read, one bounded request cannot take much longer than this. */ - static final Duration LOOKUP_PHASE_TIMEOUT = Duration.of('10s') + static final Duration LOOKUP_TIMEOUT = Duration.of('10s') @ConfigOption @Description(""" @@ -110,8 +110,8 @@ class TowerConfig implements ConfigScope { static TowerConfig forLookup(Map opts, Map env) { final bounded = new HashMap(opts) bounded.retryPolicy = [maxAttempts: 1] - bounded.httpConnectTimeout = LOOKUP_PHASE_TIMEOUT - bounded.httpReadTimeout = LOOKUP_PHASE_TIMEOUT + bounded.httpConnectTimeout = LOOKUP_TIMEOUT + bounded.httpReadTimeout = LOOKUP_TIMEOUT return new TowerConfig(bounded, env) } diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy index 30f91c3b86..e3eff01a3a 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy @@ -28,7 +28,6 @@ import nextflow.file.http.XAuthRegistry import nextflow.platform.PlatformHelper import nextflow.trace.TraceObserverFactoryV2 import nextflow.trace.TraceObserverV2 -import nextflow.util.Duration /** * Create and register the Tower observer instance * diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFusionToken.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFusionToken.groovy index ff3de22c7f..f400259ca0 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFusionToken.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFusionToken.groovy @@ -85,8 +85,8 @@ class TowerFusionToken implements FusionToken { this.accessToken = PlatformHelper.getAccessToken(config, env) this.refreshToken = PlatformHelper.getRefreshToken(config, env) this.workflowId = env.get('TOWER_WORKFLOW_ID') - // note: the workspace may have been resolved from the Seqera Platform account default - // and written into `session.config.tower` by TowerFactory during session init + // note: the workspace may have been resolved during session init -- see the + // PlatformHelper.getWorkspaceId javadoc this.workspaceId = PlatformHelper.getWorkspaceId(config, env) this.client = TowerFactory.client() } diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerRetryPolicy.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerRetryPolicy.groovy index da5a352ed3..6010da77e6 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerRetryPolicy.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerRetryPolicy.groovy @@ -17,6 +17,7 @@ package io.seqera.tower.plugin +import groovy.util.logging.Slf4j import io.seqera.util.retry.Retryable import nextflow.config.spec.ConfigOption import nextflow.config.spec.ConfigScope @@ -40,6 +41,7 @@ import nextflow.util.RetryConfig * * @author Paolo Di Tommaso */ +@Slf4j class TowerRetryPolicy implements Retryable.Config, ConfigScope { /** @@ -89,11 +91,15 @@ class TowerRetryPolicy implements Retryable.Config, ConfigScope { this.maxAttempts = firstOf(opts.maxAttempts, legacy.maxRetries, DEFAULT_MAX_ATTEMPTS) as Integer this.jitter = firstOf(opts.jitter, RetryConfig.DEFAULT_JITTER) as Double this.multiplier = firstOf(opts.multiplier, legacy.backOffBase, RetryConfig.DEFAULT_MULTIPLIER) as Double - // `maxAttempts` counts the initial attempt, so anything below 1 would mean "never - // run the operation at all"; the underlying retry library rejects 0 outright. - // -1 is the library's documented value for "retry indefinitely" and is allowed. - if( maxAttempts == 0 || maxAttempts < -1 ) - throw new IllegalArgumentException("Invalid Seqera Platform retry policy: 'maxAttempts' must be 1 or greater, or -1 for no limit -- offending value: $maxAttempts") + // `maxAttempts` counts the initial attempt, and -1 is the retry library's value for + // "retry indefinitely". Anything else below 1 would mean "never run the operation at + // all" and is rejected by the library, so warn and fall back to a single attempt -- + // which is what `0` was almost certainly meant to express -- rather than abort a run + // over a setting that used to be silently ignored. + if( maxAttempts < 1 && maxAttempts != -1 ) { + log.warn "Invalid value for config option 'tower.retryPolicy.maxAttempts' -- offending value: $maxAttempts -- using 1 (no retries) instead" + this.maxAttempts = 1 + } } /** diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy index fc74715f9b..c9ac8cc31e 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/auth/AuthCommandImpl.groovy @@ -282,7 +282,7 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { final authConfig = readAuthFile() final existingToken = authConfig['tower.accessToken'] // Extract tower config for PlatformHelper (strip 'tower.' prefix) - final towerConfig = towerOpts(authConfig) + final towerConfig = PlatformHelper.towerOpts(authConfig) final apiUrl = PlatformHelper.getEndpoint(towerConfig, SysEnv.get()) if( !existingToken ) { @@ -827,7 +827,7 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { final status = new ConfigStatus([], null, null, null) // Extract tower config and strip prefix for PlatformHelper - final towerConfig = towerOpts(config) + final towerConfig = PlatformHelper.towerOpts(config) // API endpoint - use PlatformHelper final String endpoint = PlatformHelper.getEndpoint(towerConfig, SysEnv.get()) @@ -869,8 +869,7 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { // default workspace -- i.e. the workspace a run would actually use final workspaceInfo = getConfigValue(config, 'tower.workspaceId', 'TOWER_WORKSPACE_ID') final String effectiveWorkspaceId = PlatformHelper.getEffectiveWorkspaceId( - towerConfig, SysEnv.get(), - () -> accessToken ? createLookupClient(endpoint, accessToken).getDefaultWorkspaceId() : null ) + towerConfig, SysEnv.get(), () -> httpClient?.getDefaultWorkspaceId() ) if( effectiveWorkspaceId ) { // when neither the config nor the env var is set there is no local source to @@ -1080,7 +1079,7 @@ class AuthCommandImpl extends BaseCommandImpl implements CmdAuth.AuthCommand { } // Write tower config to seqera-auth.config file, keyed without the `tower.` prefix - final towerConfig = towerOpts(config) + final towerConfig = PlatformHelper.towerOpts(config) final authConfigText = new StringBuilder() authConfigText.append("// Seqera Platform configuration\n") diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy index 0689fe6918..2c8f528368 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy @@ -993,9 +993,9 @@ class LaunchCommandImpl extends BaseCommandImpl implements CmdLaunch.LaunchComma // Otherwise apply the standard precedence: `tower.workspaceId` config, then the // TOWER_WORKSPACE_ID environment variable, then the Seqera Platform default workspace. // The client is created lazily so no API call is made when a local setting applies, - // and it is bounded so a slow Platform cannot hang the command + // and it is the command's shared client so the `/user-info` response is fetched once final workspaceId = PlatformHelper.getEffectiveWorkspaceId( - towerOpts(config), SysEnv.get(), () -> createLookupClient(apiEndpoint, accessToken).getDefaultWorkspaceId() ) + PlatformHelper.towerOpts(config), SysEnv.get(), () -> createTowerClient(apiEndpoint, accessToken).getDefaultWorkspaceId() ) log.debug "Resolved workspace ID: ${workspaceId ?: 'none (personal workspace)'}" return workspaceId ? workspaceId as Long : null } diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerConfigTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerConfigTest.groovy index 7c91f615a8..9a10d43163 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerConfigTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerConfigTest.groovy @@ -85,8 +85,8 @@ class TowerConfigTest extends Specification { then: 'the lookup gets a single attempt and short timeouts' config.retryPolicy.maxAttempts == 1 - config.httpConnectTimeout == TowerConfig.LOOKUP_PHASE_TIMEOUT - config.httpReadTimeout == TowerConfig.LOOKUP_PHASE_TIMEOUT + config.httpConnectTimeout == TowerConfig.LOOKUP_TIMEOUT + config.httpReadTimeout == TowerConfig.LOOKUP_TIMEOUT and: 'everything else is carried over unchanged' config.accessToken == 'xyz' diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerRetryPolicyTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerRetryPolicyTest.groovy index 36b44a624a..0dea19ea67 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerRetryPolicyTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerRetryPolicyTest.groovy @@ -19,6 +19,7 @@ package io.seqera.tower.plugin import nextflow.util.Duration import nextflow.util.RetryConfig import spock.lang.Specification +import spock.lang.Unroll /** * Unit tests for TowerRetryPolicy @@ -58,7 +59,7 @@ class TowerRetryPolicyTest extends Specification { policy.multiplier == 1.5d } - def 'should use provided values when specified'() { + def 'should use provided values from the legacy options'() { when: def policy = new TowerRetryPolicy([:], [backOffDelay: 500, maxRetries: 100, backOffBase: 5]) @@ -79,29 +80,22 @@ class TowerRetryPolicyTest extends Specification { policy.jitter == 0d } - def 'should honour a single attempt'() { - when: - def policy = new TowerRetryPolicy([maxAttempts: 1]) - - then: - policy.maxAttempts == 1 - } - - def 'should allow an unlimited retry policy'() { - when: - def policy = new TowerRetryPolicy([maxAttempts: -1]) + @Unroll + def 'should honour a valid maxAttempts of #VALUE'() { + expect: '1 means no retries and -1 means retry indefinitely' + new TowerRetryPolicy([maxAttempts: VALUE]).maxAttempts == VALUE - then: - policy.maxAttempts == -1 + where: + VALUE << [1, -1] } - def 'should reject a maxAttempts value that would never run the operation'() { + @Unroll + def 'should fall back to a single attempt for an unusable maxAttempts of #VALUE'() { when: 'a user writes 0 meaning "do not retry" -- previously this silently became 10' - new TowerRetryPolicy([maxAttempts: VALUE]) + def policy = new TowerRetryPolicy([maxAttempts: VALUE]) - then: - def e = thrown(IllegalArgumentException) - e.message.contains('maxAttempts') + then: 'the run is not aborted, but the setting is not silently ignored either' + policy.maxAttempts == 1 where: VALUE << [0, -2] diff --git a/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveClient.groovy b/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveClient.groovy index 64baf81247..4651f97da2 100644 --- a/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveClient.groovy +++ b/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveClient.groovy @@ -122,8 +122,8 @@ class WaveClient { this.session = session this.config = new WaveConfig(session.config.wave as Map ?: Collections.emptyMap(), SysEnv.get()) this.fusion = new FusionConfig(session.config.fusion as Map ?: Collections.emptyMap(), SysEnv.get()) - // note: the workspace may have been resolved from the Seqera Platform account default - // and written into `session.config.tower` by TowerFactory during session init + // note: the workspace may have been resolved during session init -- see the + // PlatformHelper.getWorkspaceId javadoc this.tower = new TowerConfig(session.config.tower as Map ?: Collections.emptyMap(), SysEnv.get()) this.awsFargate = WaveFactory.isAwsBatchFargateMode(session.config) this.s5cmdConfigUrl = session.config.navigate('wave.s5cmdConfigUrl') as URL From f87ac7250f8b440017485148c377ebf6c6d4d801 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 08:45:01 +0000 Subject: [PATCH 6/6] Explain workspace permission failures and name workspaces in logs When a run is refused by Seqera Platform because the token cannot launch in the target workspace, Nextflow dumped the raw HTTP response. That reads as an unexpected transport error when it is in fact a common, well understood condition with a clear remedy. Every workspace reference was also a bare 14-digit ID that nobody can map to a workspace by eye. Diagnostics only -- no behavioural change. The abort on a refused workspace is still correct; it is now self-explanatory. TowerClient.workspaceAccessError explains a 403/404 on a workspace-scoped call, using the user's workspace list to separate the two causes: a workspace the account can see exists, so a refusal means the role is insufficient; one it cannot see means the ID is not usable by this account. The message also says whether the workspace came from the Platform account default, because that decides the fix, and links to the roles documentation. When the visibility lookup itself fails, no cause is claimed and the raw response is reported as before -- likewise for a 401, which says nothing about the workspace. traceProgress now shares the same error-message construction rather than duplicating it. TowerClient.workspaceLabel renders "123 [org / workspace]" for the sites that show an ID to a human. It is best-effort: it never throws and degrades to the bare ID, which is itself a signal that the workspace is not visible. It is applied where the data is already fetched or the message warrants the lookup -- the account-default log line and the new errors -- and deliberately not on debug lines that would otherwise pay for API calls unconditionally. Assisted-by: Claude Code Signed-off-by: Claude --- .../io/seqera/tower/plugin/TowerClient.groovy | 129 ++++++++++++--- .../seqera/tower/plugin/TowerFactory.groovy | 31 +++- .../plugin/launch/LaunchCommandImpl.groovy | 11 +- .../tower/plugin/TowerClientTest.groovy | 153 ++++++++++++++++++ .../tower/plugin/TowerFactoryTest.groovy | 3 + 5 files changed, 297 insertions(+), 30 deletions(-) diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy index 9a93200adf..b455881151 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerClient.groovy @@ -49,6 +49,9 @@ class TowerClient { static final public String DEF_ENDPOINT_URL = 'https://api.cloud.seqera.io' + /** Seqera Platform documentation for workspace roles and permissions */ + static final private String WORKSPACE_ROLES_URL = 'https://docs.seqera.io/platform-cloud/orgs-and-teams/roles' + static private final String TOKEN_PREFIX = '@token:' @TupleConstructor @@ -121,11 +124,11 @@ class TowerClient { } Map traceCreate(Map req, String workspaceId){ - return sendAndProcessRequest( getUrlTraceCreate(workspaceId), req, 'POST') + return sendAndProcessRequest( getUrlTraceCreate(workspaceId), req, 'POST', workspaceId) } Map traceBegin(Map req, String workspaceId, String workflowId){ - return sendAndProcessRequest( getUrlTraceBegin(workspaceId, workflowId), req, 'PUT') + return sendAndProcessRequest( getUrlTraceBegin(workspaceId, workflowId), req, 'PUT', workspaceId) } void traceComplete(Map req, String workspaceId, String workflowId) { @@ -143,15 +146,8 @@ class TowerClient { void traceProgress(Map req, String workspaceId, String workflowId) { final url = getUrlTraceProgress( workspaceId, workflowId ) final resp = sendHttpMessage(url, req, 'PUT') - if( resp.error ) { - final message = """\ - Unexpected HTTP response - - endpoint : $url - - status code : $resp.code - - response msg: $resp.message - """.stripIndent(true) - throw new AbortRunException(message) - } + if( resp.error ) + throw new AbortRunException(errorMessage(url, resp, workspaceId)) } /** @@ -166,20 +162,83 @@ class TowerClient { return sendHttpMessage(url, req, 'PATCH') } - protected Map sendAndProcessRequest(String url, Map req, String method){ + protected Map sendAndProcessRequest(String url, Map req, String method, String workspaceId=null){ final resp = sendHttpMessage(url, req, method) - if( resp.error ) { - final message = """\ - Unexpected HTTP response - - endpoint : $url - - status code : $resp.code - - response msg: $resp.message - """.stripIndent(true) - throw new AbortRunException(message) - } + if( resp.error ) + throw new AbortRunException(errorMessage(url, resp, workspaceId)) return parseTowerResponse(resp) } + /** + * Describe a failed request: a workspace-scoped call refused with an auth status is a + * common, well understood condition, so explain it rather than dumping the HTTP + * response. Everything else falls back to the raw response. + */ + private String errorMessage(String url, Response resp, String workspaceId) { + final explained = workspaceId ? workspaceAccessError(workspaceId, resp.code) : null + return explained ?: """\ + Unexpected HTTP response + - endpoint : $url + - status code : $resp.code + - response msg: $resp.message + """.stripIndent(true) + } + + /** + * Explain why a workspace-scoped request was refused, when the reason can be + * determined with confidence, and say how to resolve it. + * + * Whether the account can see the workspace at all separates the two cases: a + * workspace that is listed for the user exists and is visible, so a refusal means + * the role is insufficient; one that is not listed means the ID does not identify a + * workspace this account can use. A 401 is deliberately not handled here -- the token + * itself is rejected, so nothing about the workspace can be established. + * + * @return the explanation, or null when the status is not one we can attribute + */ + protected String workspaceAccessError(String workspaceId, int statusCode) { + if( statusCode != 403 && statusCode != 404 ) + return null + // if the visibility lookup itself fails we cannot attribute the refusal with + // confidence, so say nothing and let the caller report the raw response + final List workspaces + try { + workspaces = listUserWorkspacesAndOrgs(getUserInfo()?.id as String) + } + catch( Exception e ) { + log.debug "Unable to determine access to Seqera Platform workspace ${workspaceId}: ${e.message}" + return null + } + final visible = workspaces?.any { it.workspaceId?.toString() == workspaceId } + final label = workspaceLabel(workspaceId) + // explain where the workspace came from, because that determines the remedy + final isAccountDefault = workspaceId == getDefaultWorkspaceId() + final origin = isAccountDefault + ? "\nThis is the default workspace configured in your Seqera Platform account, which is used when no workspace is set locally." + : '' + + if( visible ) + return """\ + Cannot run in Seqera Platform workspace ${label}: your access token does not have permission to launch runs there.${origin} + To resolve this, either: + - ask an admin of that workspace to grant your user the 'launch' role or higher; or + ${isAccountDefault ? '- change the default workspace in your Seqera Platform user settings; or' : '- choose a workspace you have access to; or'} + - set `tower.workspaceId` (or TOWER_WORKSPACE_ID) to a workspace you can use. + + See ${WORKSPACE_ROLES_URL} + """.stripIndent(true) + + return """\ + Seqera Platform workspace ${label} is not available to your account: it either does not exist or your access token cannot see it.${origin} + To resolve this, either: + - check the workspace ID is correct; or + - ask an admin of that workspace to grant your user access; or + - set `tower.workspaceId` (or TOWER_WORKSPACE_ID) to a workspace you can use. + + See ${WORKSPACE_ROLES_URL} + """.stripIndent(true) + } + protected String getUrlTraceCreate(String workspaceId) { def result = this.endpoint + '/trace/create' if( workspaceId ) @@ -565,6 +624,34 @@ class TowerClient { throw new Exception("Seqera API error: HTTP ${code} for ${url}${resp.message ? ' - ' + resp.message :''}") } + /** + * Describe a workspace for humans, pairing its numeric ID with its name so that + * log messages and errors are readable, e.g. {@code 12345 [my-org / my-workspace]}. + * + * Best-effort by design: it degrades to the bare ID and never throws, so it is safe + * to use while building an error message. A workspace the account cannot see has no + * name to show -- which is itself a signal that the ID is wrong or inaccessible. + * + * Both underlying calls are memoized, so this is effectively free after first use. + * + * @param workspaceId Id of the workspace, may be null + * @return the ID annotated with the org and workspace name when they are known + */ + String workspaceLabel(String workspaceId) { + if( !workspaceId ) + return null + try { + final details = getUserWorkspaceDetails(getUserInfo()?.id as String, workspaceId) + return details + ? "${workspaceId} [${details.orgName} / ${details.workspaceName}]".toString() + : workspaceId + } + catch( Exception e ) { + log.debug "Unable to resolve the name of Seqera Platform workspace ${workspaceId}: ${e.message}" + return workspaceId + } + } + /** * Calls the Seqera Platform to retrieve the user's workspaces information * and select the one matching with the workspace Id. diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy index e3eff01a3a..b3b0b2a91a 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/TowerFactory.groovy @@ -39,6 +39,9 @@ class TowerFactory implements TraceObserverFactoryV2 { private Map env + /** bounded-retry client for the pre-run Platform lookups; created on demand and reused */ + private TowerClient lookupClient + TowerFactory(){ env = SysEnv.get() } @@ -66,7 +69,7 @@ class TowerFactory implements TraceObserverFactoryV2 { // and would otherwise resolve to the personal workspace while the run is reported // into the Platform default one. Anything the user set is left exactly as it is. if( !local && workspaceId ) - publishDefaultWorkspaceId(session, workspaceId) + publishDefaultWorkspaceId(session, workspaceId, opts) // create the tower observer result.add( new TowerObserver(session, client, workspaceId, env)) // create the logs checkpoint @@ -88,7 +91,24 @@ class TowerFactory implements TraceObserverFactoryV2 { * Extracted as a seam so it can be stubbed in tests without hitting the network. */ protected String defaultWorkspaceId(Map opts) { - return new TowerClient(TowerConfig.forLookup(opts, env)).getDefaultWorkspaceId() + return lookupClient(opts).getDefaultWorkspaceId() + } + + /** + * Name the workspace for the log line in {@link #publishDefaultWorkspaceId}. Shares the + * lookup client with {@link #defaultWorkspaceId}, so the {@code /user-info} response it + * already fetched is reused and only the workspace list is requested. + * + * Extracted as a seam so it can be stubbed in tests without hitting the network. + */ + protected String workspaceLabel(Map opts, String workspaceId) { + return lookupClient(opts).workspaceLabel(workspaceId) + } + + private TowerClient lookupClient(Map opts) { + if( lookupClient == null ) + lookupClient = new TowerClient(TowerConfig.forLookup(opts, env)) + return lookupClient } /** @@ -102,13 +122,14 @@ class TowerFactory implements TraceObserverFactoryV2 { * the parsed {@code ConfigObject} by {@code ConfigCmdAdapter.normalize0} and converted * in the {@link Session} constructor, so an absent key really does read as null here. */ - protected void publishDefaultWorkspaceId(Session session, String workspaceId) { + protected void publishDefaultWorkspaceId(Session session, String workspaceId, Map opts) { final config = session.config if( config.tower == null ) config.tower = new HashMap(1) (config.tower as Map).workspaceId = workspaceId - // tell the user why their run is landing in a workspace they did not configure - log.info "Using default workspace configured in your Seqera Platform account: $workspaceId" + // tell the user why their run is landing in a workspace they did not configure, + // naming it so the numeric ID alone does not have to be looked up + log.info "Using default workspace configured in your Seqera Platform account: ${workspaceLabel(opts, workspaceId)}" } @Memoized diff --git a/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy b/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy index 534d61a668..c23bc9d7a6 100644 --- a/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy +++ b/plugins/nf-tower/src/main/io/seqera/tower/plugin/launch/LaunchCommandImpl.groovy @@ -168,7 +168,7 @@ class LaunchCommandImpl extends BaseCommandImpl implements LaunchCommand { final wsDetails = httpClient.getUserWorkspaceDetails(userId, workspaceId.toString()) orgName = wsDetails?.orgName as String workspaceName = wsDetails?.workspaceName as String - log.debug "Using workspace '${workspaceName}' (ID: ${workspaceId})" + log.debug "Using workspace ${httpClient.workspaceLabel(workspaceId.toString())}" } else { log.debug "Using personal workspace for user: ${userName}" } @@ -993,8 +993,9 @@ class LaunchCommandImpl extends BaseCommandImpl implements LaunchCommand { // Otherwise apply the standard precedence: `tower.workspaceId` config, then the // TOWER_WORKSPACE_ID environment variable, then the Seqera Platform default workspace. - // The client is created lazily so no API call is made when a local setting applies, - // and it is the command's shared client so the `/user-info` response is fetched once + // The client is created lazily so that no API call is made when a local setting applies. + // The ID is not named here: `createTowerClient` returns a fresh client, so resolving the + // name would mean building one -- and two extra API calls -- just to log a line. final workspaceId = PlatformHelper.getEffectiveWorkspaceId( PlatformHelper.towerOpts(config), SysEnv.get(), () -> createTowerClient(apiEndpoint, accessToken).getDefaultWorkspaceId() ) log.debug "Resolved workspace ID: ${workspaceId ?: 'none (personal workspace)'}" @@ -1004,7 +1005,9 @@ class LaunchCommandImpl extends BaseCommandImpl implements LaunchCommand { protected Map findComputeEnv(TowerClient client, String computeEnvName, Long workspaceId) { final computeEnvs = listComputeEnvironments(client, workspaceId ? workspaceId.toString() : null) - log.debug "Looking for ${computeEnvName ? "compute environment with name: ${computeEnvName}" : "primary compute environment"} ${workspaceId ? "in workspace ID ${workspaceId}" : "in personal workspace"}" + // guarded: naming the workspace costs a lookup, and GString arguments are built eagerly + if( log.isDebugEnabled() ) + log.debug "Looking for ${computeEnvName ? "compute environment with name: ${computeEnvName}" : "primary compute environment"} ${workspaceId ? "in workspace ${client.workspaceLabel(workspaceId.toString())}" : "in personal workspace"}" for (item in computeEnvs) { final computeEnv = item as Map diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy index ae4c659a6c..4b70c941a2 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerClientTest.groovy @@ -317,6 +317,159 @@ class TowerClientTest extends Specification { wireMock.stop() } + /** A client backed by WireMock, with `/user-info` and the user's workspace list stubbed */ + private TowerClient stubClient(WireMockServer wireMock, String defaultWorkspaceId, List workspaces) { + wireMock.stubFor( + WireMock.get(WireMock.urlPathEqualTo('/user-info')) + .willReturn(WireMock.aResponse().withStatus(200) + .withHeader('Content-Type', 'application/json') + .withBody("""{"user":{"id":1,"userName":"me"},"defaultWorkspaceId":${defaultWorkspaceId ?: 'null'}}"""))) + final entries = workspaces.collect { """{"orgName":"acme","workspaceId":${it},"workspaceName":"ws-${it}"}""" } + wireMock.stubFor( + WireMock.get(WireMock.urlPathEqualTo('/user/1/workspaces')) + .willReturn(WireMock.aResponse().withStatus(200) + .withHeader('Content-Type', 'application/json') + .withBody("""{"orgsAndWorkspaces":[${entries.join(',')}]}"""))) + TowerConfig config = Mock(TowerConfig) { + getHttpReadTimeout() >> Duration.of('5 s') + getHttpConnectTimeout() >> Duration.of('5 s') + getEndpoint() >> wireMock.baseUrl() + getAccessToken() >> 'token' + } + return new TowerClient(config) + } + + def 'should label a workspace with its org and name' () { + given: + def wireMock = new WireMockServer(0) + wireMock.start() + def client = stubClient(wireMock, '42', ['42']) + + expect: 'a workspace the account can see is named' + client.workspaceLabel('42') == '42 [acme / ws-42]' + + and: 'one it cannot see degrades to the bare id rather than failing' + client.workspaceLabel('999') == '999' + + and: 'a null id is passed through' + client.workspaceLabel(null) == null + + cleanup: + wireMock.stop() + } + + def 'should not throw when the workspace name cannot be resolved' () { + given: + def wireMock = new WireMockServer(0) + wireMock.start() + // note: 404 rather than 5xx -- server errors are retried, which would make this + // test walk the full retry ladder + wireMock.stubFor( + WireMock.get(WireMock.anyUrl()) + .willReturn(WireMock.aResponse().withStatus(404).withBody('nope'))) + and: + TowerConfig config = Mock(TowerConfig) { + getHttpReadTimeout() >> Duration.of('5 s') + getHttpConnectTimeout() >> Duration.of('5 s') + getEndpoint() >> wireMock.baseUrl() + getAccessToken() >> 'token' + } + def client = new TowerClient(config) + + expect: 'labelling is best-effort -- it must be safe to use while building an error' + client.workspaceLabel('42') == '42' + + cleanup: + wireMock.stop() + } + + def 'should explain that the role is insufficient for a visible workspace' () { + given: + def wireMock = new WireMockServer(0) + wireMock.start() + def client = stubClient(wireMock, '42', ['42']) + + when: 'the workspace is visible to the account but the call is refused' + def msg = client.workspaceAccessError('42', 403) + + then: 'the cause is stated, not guessed at' + msg.contains('does not have permission to launch runs') + and: 'the workspace is named, not just numbered' + msg.contains('42 [acme / ws-42]') + and: 'the remedy and the docs are given' + msg.contains("'launch' role") + msg.contains('tower.workspaceId') + msg.contains('https://docs.seqera.io/platform-cloud/orgs-and-teams/roles') + and: 'it says where the workspace came from, because that decides the fix' + msg.contains('default workspace configured in your Seqera Platform account') + + cleanup: + wireMock.stop() + } + + def 'should explain that a workspace is not available to the account' () { + given: + def wireMock = new WireMockServer(0) + wireMock.start() + def client = stubClient(wireMock, '42', ['42']) + + when: 'the workspace is not one the account can see' + def msg = client.workspaceAccessError('999', STATUS) + + then: 'the message does not claim it is a permissions problem' + msg.contains('is not available to your account') + !msg.contains("'launch' role") + and: 'and does not claim it is the account default, because it is not' + !msg.contains('default workspace configured in your Seqera Platform account') + + cleanup: + wireMock.stop() + + where: + STATUS << [403, 404] + } + + def 'should not attribute a cause for statuses it cannot explain' () { + given: + def wireMock = new WireMockServer(0) + wireMock.start() + def client = stubClient(wireMock, '42', ['42']) + + expect: 'a server-side failure falls back to the generic HTTP report' + client.workspaceAccessError('42', 500) == null + + and: 'so does a rejected token -- it says nothing about the workspace' + client.workspaceAccessError('42', 401) == null + + cleanup: + wireMock.stop() + } + + def 'should not attribute a cause when the visibility lookup itself fails' () { + given: + def wireMock = new WireMockServer(0) + wireMock.start() + // note: 404 rather than 5xx -- server errors are retried, which would make this + // test walk the full retry ladder + wireMock.stubFor( + WireMock.get(WireMock.anyUrl()) + .willReturn(WireMock.aResponse().withStatus(404).withBody('nope'))) + and: + TowerConfig config = Mock(TowerConfig) { + getHttpReadTimeout() >> Duration.of('5 s') + getHttpConnectTimeout() >> Duration.of('5 s') + getEndpoint() >> wireMock.baseUrl() + getAccessToken() >> 'token' + } + def client = new TowerClient(config) + + expect: 'without the workspace list the two causes are indistinguishable, so neither is claimed' + client.workspaceAccessError('42', 403) == null + + cleanup: + wireMock.stop() + } + def 'should build URL without query params'() { given: def client = new TowerClient() diff --git a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy index 53f06d793f..96881b7624 100644 --- a/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy +++ b/plugins/nf-tower/src/test/io/seqera/tower/plugin/TowerFactoryTest.groovy @@ -61,6 +61,9 @@ class TowerFactoryTest extends Specification { 1 * factory.defaultWorkspaceId(_ as Map) >> '300' observer.getWorkspaceId() == '300' + and: 'the workspace is named in the log line announcing it' + 1 * factory.workspaceLabel(_ as Map, '300') >> '300 [acme / ws-300]' + and: 'the resolved value is published into the session config' config.tower.workspaceId == '300'