diff --git a/plugins/nf-azure/src/main/nextflow/cloud/azure/config/AzActiveDirectoryOpts.groovy b/plugins/nf-azure/src/main/nextflow/cloud/azure/config/AzActiveDirectoryOpts.groovy index 462c6c8203..bc05264806 100644 --- a/plugins/nf-azure/src/main/nextflow/cloud/azure/config/AzActiveDirectoryOpts.groovy +++ b/plugins/nf-azure/src/main/nextflow/cloud/azure/config/AzActiveDirectoryOpts.groovy @@ -16,6 +16,7 @@ package nextflow.cloud.azure.config import groovy.transform.CompileStatic +import nextflow.SysEnv import nextflow.cloud.azure.nio.AzFileSystemProvider /** @@ -26,18 +27,15 @@ import nextflow.cloud.azure.nio.AzFileSystemProvider @CompileStatic class AzActiveDirectoryOpts { - private Map sysEnv - String servicePrincipalId String servicePrincipalSecret String tenantId AzActiveDirectoryOpts(Map config, Map env = null) { assert config != null - this.sysEnv = env == null ? new HashMap(System.getenv()) : env - this.servicePrincipalId = config.servicePrincipalId ?: sysEnv.get('AZURE_CLIENT_ID') - this.servicePrincipalSecret = config.servicePrincipalSecret ?: sysEnv.get('AZURE_CLIENT_SECRET') - this.tenantId = config.tenantId ?: sysEnv.get('AZURE_TENANT_ID') + this.servicePrincipalId = config.servicePrincipalId ?: SysEnv.get('AZURE_CLIENT_ID') + this.servicePrincipalSecret = config.servicePrincipalSecret ?: SysEnv.get('AZURE_CLIENT_SECRET') + this.tenantId = config.tenantId ?: SysEnv.get('AZURE_TENANT_ID') } Map getEnv() { diff --git a/plugins/nf-azure/src/main/nextflow/cloud/azure/config/AzRegistryOpts.groovy b/plugins/nf-azure/src/main/nextflow/cloud/azure/config/AzRegistryOpts.groovy index 16bb25548b..386571cf2d 100644 --- a/plugins/nf-azure/src/main/nextflow/cloud/azure/config/AzRegistryOpts.groovy +++ b/plugins/nf-azure/src/main/nextflow/cloud/azure/config/AzRegistryOpts.groovy @@ -16,8 +16,8 @@ package nextflow.cloud.azure.config - import groovy.transform.CompileStatic +import nextflow.SysEnv /** * Model Azure Batch registry config settings from nextflow config file @@ -27,8 +27,6 @@ import groovy.transform.CompileStatic @CompileStatic class AzRegistryOpts { - private Map sysEnv - String server String userName String password @@ -37,12 +35,11 @@ class AzRegistryOpts { this(Collections.emptyMap()) } - AzRegistryOpts(Map config, Map env=null) { + AzRegistryOpts(Map config, Map env=SysEnv.get()) { assert config!=null - this.sysEnv = env==null ? new HashMap(System.getenv()) : env this.server = config.server ?: 'docker.io' - this.userName = config.userName ?: sysEnv.get('AZURE_REGISTRY_USER_NAME') - this.password = config.password ?: sysEnv.get('AZURE_REGISTRY_PASSWORD') + this.userName = config.userName ?: env.get('AZURE_REGISTRY_USER_NAME') + this.password = config.password ?: env.get('AZURE_REGISTRY_PASSWORD') } boolean isConfigured() { diff --git a/plugins/nf-azure/src/main/nextflow/cloud/azure/config/AzStorageOpts.groovy b/plugins/nf-azure/src/main/nextflow/cloud/azure/config/AzStorageOpts.groovy index 1bfecfb780..101c22bf4e 100644 --- a/plugins/nf-azure/src/main/nextflow/cloud/azure/config/AzStorageOpts.groovy +++ b/plugins/nf-azure/src/main/nextflow/cloud/azure/config/AzStorageOpts.groovy @@ -17,6 +17,7 @@ package nextflow.cloud.azure.config import groovy.transform.CompileStatic +import nextflow.SysEnv import nextflow.cloud.azure.batch.AzHelper import nextflow.cloud.azure.nio.AzFileSystemProvider import nextflow.util.Duration @@ -28,7 +29,6 @@ import nextflow.util.Duration @CompileStatic class AzStorageOpts { - private Map sysEnv String accountKey String accountName String sasToken @@ -36,12 +36,11 @@ class AzStorageOpts { Map fileShares - AzStorageOpts(Map config, Map env=null) { + AzStorageOpts(Map config, Map env=SysEnv.get()) { assert config!=null - this.sysEnv = env==null ? new HashMap(System.getenv()) : env - this.accountKey = config.accountKey ?: sysEnv.get('AZURE_STORAGE_ACCOUNT_KEY') - this.accountName = config.accountName ?: sysEnv.get('AZURE_STORAGE_ACCOUNT_NAME') - this.sasToken = config.sasToken ?: sysEnv.get('AZURE_STORAGE_SAS_TOKEN') + this.accountKey = config.accountKey ?: env.get('AZURE_STORAGE_ACCOUNT_KEY') + this.accountName = config.accountName ?: env.get('AZURE_STORAGE_ACCOUNT_NAME') + this.sasToken = config.sasToken ?: env.get('AZURE_STORAGE_SAS_TOKEN') this.tokenDuration = (config.tokenDuration as Duration) ?: Duration.of('48h') this.fileShares = parseFileShares(config.fileShares instanceof Map ? config.fileShares as Map : Collections. emptyMap()) diff --git a/plugins/nf-azure/src/main/nextflow/cloud/azure/fusion/AzFusionEnv.groovy b/plugins/nf-azure/src/main/nextflow/cloud/azure/fusion/AzFusionEnv.groovy index 188e411c29..9e3d90212a 100644 --- a/plugins/nf-azure/src/main/nextflow/cloud/azure/fusion/AzFusionEnv.groovy +++ b/plugins/nf-azure/src/main/nextflow/cloud/azure/fusion/AzFusionEnv.groovy @@ -33,18 +33,31 @@ class AzFusionEnv implements FusionEnv { @Override Map getEnvironment(String scheme, FusionConfig config) { - if( scheme!='az' ) - return Collections.emptyMap() + if (scheme != 'az') + return Collections. emptyMap() - final cfg = AzConfig.config.storage() + final cfg = AzConfig.config final result = new LinkedHashMap(10) - if( !cfg.accountName ) - throw new IllegalArgumentException("Missing Azure storage account name") - if( !cfg.sasToken && !cfg.accountKey ) - throw new IllegalArgumentException("Missing Azure storage SAS token") - - result.AZURE_STORAGE_ACCOUNT = cfg.accountName - result.AZURE_STORAGE_SAS_TOKEN = cfg.getOrCreateSasToken() + + if (!cfg.storage().accountName) + throw new IllegalArgumentException("Missing Azure Storage account name") + + if (cfg.storage().accountKey && cfg.storage().sasToken) + throw new IllegalArgumentException("Azure Storage Access key and SAS token detected. Only one is allowed") + + // the account name is always required + result.AZURE_STORAGE_ACCOUNT = cfg.storage().accountName + + // If a Managed Identity or Service Principal is configured, Fusion only needs to know the account name + if (cfg.managedIdentity().isConfigured() || cfg.activeDirectory().isConfigured()) { + return result + } + + // If a SAS token is configured, instead, Fusion also requires the token value + if (cfg.storage().sasToken) { + result.AZURE_STORAGE_SAS_TOKEN = cfg.storage().getOrCreateSasToken() + } + return result } } diff --git a/plugins/nf-azure/src/test/nextflow/cloud/azure/batch/AzBatchServiceTest.groovy b/plugins/nf-azure/src/test/nextflow/cloud/azure/batch/AzBatchServiceTest.groovy index 660291859a..eceeb644b2 100644 --- a/plugins/nf-azure/src/test/nextflow/cloud/azure/batch/AzBatchServiceTest.groovy +++ b/plugins/nf-azure/src/test/nextflow/cloud/azure/batch/AzBatchServiceTest.groovy @@ -10,6 +10,7 @@ import com.azure.identity.ManagedIdentityCredential import com.google.common.hash.HashCode import nextflow.Global import nextflow.Session +import nextflow.SysEnv import nextflow.cloud.azure.config.AzConfig import nextflow.cloud.azure.config.AzManagedIdentityOpts import nextflow.cloud.azure.config.AzPoolOpts @@ -31,6 +32,14 @@ class AzBatchServiceTest extends Specification { static long _1GB = 1024 * 1024 * 1024 + def setup() { + SysEnv.push([:]) // <-- clear the system host env + } + + def cleanup() { + SysEnv.pop() // <-- restore the system host env + } + def 'should make job id'() { given: def task = Mock(TaskRun) { diff --git a/plugins/nf-azure/src/test/nextflow/cloud/azure/fusion/AzFusionEnvTest.groovy b/plugins/nf-azure/src/test/nextflow/cloud/azure/fusion/AzFusionEnvTest.groovy new file mode 100644 index 0000000000..f1942abb88 --- /dev/null +++ b/plugins/nf-azure/src/test/nextflow/cloud/azure/fusion/AzFusionEnvTest.groovy @@ -0,0 +1,102 @@ +/* + * Copyright 2013-2024, Seqera Labs + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package nextflow.cloud.azure.fusion + +import nextflow.Global +import nextflow.Session +import nextflow.SysEnv +import nextflow.fusion.FusionConfig +import spock.lang.Specification + +/** + * + * @author Alberto Miranda + */ +class AzFusionEnvTest extends Specification { + + def setup() { + SysEnv.push([:]) // <-- clear the system host env + } + + def cleanup() { + SysEnv.pop() // <-- restore the system host env + } + + def 'should return empty env'() { + given: + def provider = new AzFusionEnv() + when: + def env = provider.getEnvironment('aws', Mock(FusionConfig)) + then: + env == Collections.emptyMap() + } + + def 'should return env environment'() { + given: + Global.session = Mock(Session) { + getConfig() >> [azure: [storage: [accountName: 'x1']]] + } + and: + + when: + def config = Mock(FusionConfig) + def env = new AzFusionEnv().getEnvironment('az', config) + then: + env == [AZURE_STORAGE_ACCOUNT: 'x1'] + + } + + def 'should return env environment with SAS token config'() { + given: + Global.session = Mock(Session) { + getConfig() >> [azure: [storage: [accountName: 'x1', sasToken: 'y1']]] + } + and: + + when: + def config = Mock(FusionConfig) + def env = new AzFusionEnv().getEnvironment('az', config) + then: + env == [AZURE_STORAGE_ACCOUNT: 'x1', AZURE_STORAGE_SAS_TOKEN: 'y1'] + + } + + def 'should throw an exception when missing Azure Storage account name'() { + given: + Global.session = Mock(Session) { + getConfig() >> [azure: [storage: [sasToken: 'y1']]] + } + when: + def config = Mock(FusionConfig) + def env = new AzFusionEnv().getEnvironment('az', Mock(FusionConfig)) + then: + thrown(IllegalArgumentException) + } + + def 'should throw an exception when both account key and SAS token are present'() { + given: + Global.session = Mock(Session) { + getConfig() >> [azure: [storage: [accountName: 'x1', accountKey: 'y1', sasToken: 'z1']]] + } + when: + def config = Mock(FusionConfig) + def env = new AzFusionEnv().getEnvironment('az', Mock(FusionConfig)) + then: + thrown(IllegalArgumentException) + } +}