Skip to content

Commit

Permalink
Support Azure Managed Identities in Fusion configuration logic (#5278)
Browse files Browse the repository at this point in the history
This commit extends the `AzFusionEnv` class so that it now takes into
account a potential Azure Managed Identity in Nextflow's configuration
file. It also fixes a configuration error (#5081) by which Nextflow complained
of a missing SAS token despite having a Managed Identity properly configured.

Signed-off-by: Alberto Miranda <[email protected]>
Signed-off-by: Paolo Di Tommaso <[email protected]>
Co-authored-by: Paolo Di Tommaso <[email protected]>
  • Loading branch information
alberto-miranda and pditommaso authored Sep 3, 2024
1 parent bf0cd32 commit a0bf8b4
Show file tree
Hide file tree
Showing 6 changed files with 147 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package nextflow.cloud.azure.config

import groovy.transform.CompileStatic
import nextflow.SysEnv
import nextflow.cloud.azure.nio.AzFileSystemProvider

/**
Expand All @@ -26,18 +27,15 @@ import nextflow.cloud.azure.nio.AzFileSystemProvider
@CompileStatic
class AzActiveDirectoryOpts {

private Map<String, String> sysEnv

String servicePrincipalId
String servicePrincipalSecret
String tenantId

AzActiveDirectoryOpts(Map config, Map<String, String> env = null) {
assert config != null
this.sysEnv = env == null ? new HashMap<String, String>(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<String, Object> getEnv() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -27,8 +27,6 @@ import groovy.transform.CompileStatic
@CompileStatic
class AzRegistryOpts {

private Map<String,String> sysEnv

String server
String userName
String password
Expand All @@ -37,12 +35,11 @@ class AzRegistryOpts {
this(Collections.emptyMap())
}

AzRegistryOpts(Map config, Map<String,String> env=null) {
AzRegistryOpts(Map config, Map<String,String> env=SysEnv.get()) {
assert config!=null
this.sysEnv = env==null ? new HashMap<String,String>(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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -28,20 +29,18 @@ import nextflow.util.Duration
@CompileStatic
class AzStorageOpts {

private Map<String,String> sysEnv
String accountKey
String accountName
String sasToken
Duration tokenDuration
Map<String,AzFileShareOpts> fileShares


AzStorageOpts(Map config, Map<String,String> env=null) {
AzStorageOpts(Map config, Map<String,String> env=SysEnv.get()) {
assert config!=null
this.sysEnv = env==null ? new HashMap<String,String>(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<String, Map>
: Collections.<String,Map> emptyMap())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,31 @@ class AzFusionEnv implements FusionEnv {

@Override
Map<String, String> getEnvironment(String scheme, FusionConfig config) {
if( scheme!='az' )
return Collections.<String,String>emptyMap()
if (scheme != 'az')
return Collections.<String, String> 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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <[email protected]>
*/
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)
}
}

0 comments on commit a0bf8b4

Please sign in to comment.