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 101c22bf4e..d3e6bb3259 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 @@ -62,10 +62,4 @@ class AzStorageOpts { } return result } - - synchronized String getOrCreateSasToken() { - if( !sasToken ) - sasToken = AzHelper.generateAccountSas(accountName, accountKey, tokenDuration) - return sasToken - } } 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 9e3d90212a..4888636e4c 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 @@ -17,47 +17,72 @@ package nextflow.cloud.azure.fusion +import groovy.util.logging.Slf4j +import nextflow.Global +import nextflow.cloud.azure.batch.AzHelper import groovy.transform.CompileStatic import nextflow.cloud.azure.config.AzConfig import nextflow.fusion.FusionConfig import nextflow.fusion.FusionEnv import org.pf4j.Extension + /** * Implement environment provider for Azure specific variables - * + * * @author Paolo Di Tommaso */ @Extension @CompileStatic +@Slf4j class AzFusionEnv implements FusionEnv { @Override Map getEnvironment(String scheme, FusionConfig config) { - if (scheme != 'az') + if (scheme != 'az') { return Collections. emptyMap() + } final cfg = AzConfig.config final result = new LinkedHashMap(10) - if (!cfg.storage().accountName) + if (!cfg.storage().accountName) { throw new IllegalArgumentException("Missing Azure Storage account name") + } - if (cfg.storage().accountKey && cfg.storage().sasToken) + 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 + // TODO: In theory, generating an impromptu SAS token for authentication methods other than + // `azure.storage.sasToken` should not be necessary, because those methods should already allow sufficient + // access for normal operation. Nevertheless, #5287 heavily implies that failing to do so causes the Azure + // Storage plugin or Fusion to fail. In any case, it may be possible to remove this in the future. + result.AZURE_STORAGE_SAS_TOKEN = getOrCreateSasToken() - // 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 - } + return result + } + + /** + * Return the SAS token if it is defined in the configuration, otherwise generate one based on the requested + * authentication method. + */ + synchronized String getOrCreateSasToken() { + + final cfg = AzConfig.config - // If a SAS token is configured, instead, Fusion also requires the token value + // If a SAS token is already defined in the configuration, just return it if (cfg.storage().sasToken) { - result.AZURE_STORAGE_SAS_TOKEN = cfg.storage().getOrCreateSasToken() + return cfg.storage().sasToken } - return result + // For Active Directory and Managed Identity, we cannot generate an *account* SAS token, but we can generate + // a *container* SAS token for the work directory. + if (cfg.activeDirectory().isConfigured() || cfg.managedIdentity().isConfigured()) { + return AzHelper.generateContainerSasWithActiveDirectory(Global.session.workDir, cfg.storage().tokenDuration) + } + + // Shared Key authentication can use an account SAS token + return AzHelper.generateAccountSasWithAccountKey(Global.session.workDir, cfg.storage().tokenDuration) } } 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 index f1942abb88..79774e9a9a 100644 --- a/plugins/nf-azure/src/test/nextflow/cloud/azure/fusion/AzFusionEnvTest.groovy +++ b/plugins/nf-azure/src/test/nextflow/cloud/azure/fusion/AzFusionEnvTest.groovy @@ -21,6 +21,7 @@ import nextflow.Global import nextflow.Session import nextflow.SysEnv import nextflow.fusion.FusionConfig + import spock.lang.Specification /** @@ -46,22 +47,117 @@ class AzFusionEnvTest extends Specification { env == Collections.emptyMap() } - def 'should return env environment'() { + def 'should return env environment with SAS token config when accountKey is provided'() { given: + def NAME = 'myaccount' + def KEY = 'myaccountkey' Global.session = Mock(Session) { - getConfig() >> [azure: [storage: [accountName: 'x1']]] + getConfig() >> [azure: [storage: [accountName: NAME, accountKey: KEY]]] } - and: when: def config = Mock(FusionConfig) - def env = new AzFusionEnv().getEnvironment('az', config) + def fusionEnv = Spy(AzFusionEnv) + 1 * fusionEnv.getOrCreateSasToken() >> 'generatedSasToken' + def env = fusionEnv.getEnvironment('az', config) + + then: + env.AZURE_STORAGE_ACCOUNT == NAME + env.AZURE_STORAGE_SAS_TOKEN + env.size() == 2 + } + + def 'should return env environment with SAS token config when a Service Principal is provided'() { + given: + def NAME = 'myaccount' + def CLIENT_ID = 'myclientid' + def CLIENT_SECRET = 'myclientsecret' + def TENANT_ID = 'mytenantid' + Global.session = Mock(Session) { + getConfig() >> [ + azure: [ + activeDirectory: [ + servicePrincipalId: CLIENT_ID, + servicePrincipalSecret: CLIENT_SECRET, + tenantId: TENANT_ID + ], + storage: [ + accountName: NAME + ] + ] + ] + } + + when: + def config = Mock(FusionConfig) + def fusionEnv = Spy(AzFusionEnv) + 1 * fusionEnv.getOrCreateSasToken() >> 'generatedSasToken' + def env = fusionEnv.getEnvironment('az', config) + + then: + env.AZURE_STORAGE_ACCOUNT == NAME + env.AZURE_STORAGE_SAS_TOKEN == 'generatedSasToken' + env.size() == 2 + } + + def 'should return env environment with SAS token config when a user-assigned Managed Identity is provided'() { + given: + def NAME = 'myaccount' + def CLIENT_ID = 'myclientid' + Global.session = Mock(Session) { + getConfig() >> [ + azure: [ + managedIdentity: [ + clientId: CLIENT_ID, + ], + storage: [ + accountName: NAME + ] + ] + ] + } + + when: + def config = Mock(FusionConfig) + def fusionEnv = Spy(AzFusionEnv) + 1 * fusionEnv.getOrCreateSasToken() >> 'generatedSasToken' + def env = fusionEnv.getEnvironment('az', config) + then: - env == [AZURE_STORAGE_ACCOUNT: 'x1'] + env.AZURE_STORAGE_ACCOUNT == NAME + env.AZURE_STORAGE_SAS_TOKEN == 'generatedSasToken' + env.size() == 2 + } + def 'should return env environment with SAS token config when a system-assigned Managed Identity is provided'() { + given: + def NAME = 'myaccount' + Global.session = Mock(Session) { + getConfig() >> [ + azure: [ + managedIdentity: [ + system: true + ], + storage: [ + accountName: NAME + ] + ] + ] + } + + when: + def config = Mock(FusionConfig) + def fusionEnv = Spy(AzFusionEnv) + 1 * fusionEnv.getOrCreateSasToken() >> 'generatedSasToken' + def env = fusionEnv.getEnvironment('az', config) + + then: + env.AZURE_STORAGE_ACCOUNT == NAME + env.AZURE_STORAGE_SAS_TOKEN == 'generatedSasToken' + env.size() == 2 } - def 'should return env environment with SAS token config'() { + def 'should return env environment with SAS token config when a sasToken is provided'() { given: Global.session = Mock(Session) { getConfig() >> [azure: [storage: [accountName: 'x1', sasToken: 'y1']]] @@ -99,4 +195,5 @@ class AzFusionEnvTest extends Specification { then: thrown(IllegalArgumentException) } + }