Skip to content

Commit

Permalink
Fix Azure system-assigned managed identity
Browse files Browse the repository at this point in the history
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
  • Loading branch information
pditommaso committed Jun 14, 2024
1 parent 306814e commit a639a17
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,28 @@
package nextflow.cloud.azure.config

import groovy.transform.CompileStatic
import groovy.transform.EqualsAndHashCode
import groovy.transform.ToString
import nextflow.cloud.azure.nio.AzFileSystemProvider

/**
* Model Azure managed identity config options
*
* @author Ben Sherman <bentshermann@gmail.com>
*/
@ToString(includePackage = false, includeNames = true)
@EqualsAndHashCode
@CompileStatic
class AzManagedIdentityOpts {

String clientId

Boolean system

String tenantId
boolean system

AzManagedIdentityOpts(Map config) {
assert config != null
this.clientId = config.clientId
this.system = config.system as Boolean
this.tenantId = config.tenantId
this.system = Boolean.parseBoolean(config.system as String)
}

Map<String, Object> getEnv() {
Expand All @@ -49,11 +50,11 @@ class AzManagedIdentityOpts {
boolean isConfigured() {
if( clientId && !system )
return true
if( !clientId && system && tenantId )
if( !clientId && system )
return true
if( !clientId && !system && !tenantId )
if( !clientId && !system )
return false
throw new IllegalArgumentException("Invalid Managed Identity configuration - Make sure the `clientId` or `system` is set in the nextflow config, as well as `tenantId`")
throw new IllegalArgumentException("Invalid Managed Identity configuration - Make sure the `clientId` or `system` is set in the nextflow config")
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import java.nio.file.spi.FileSystemProvider
import com.azure.storage.blob.BlobServiceClient
import com.azure.storage.blob.models.BlobStorageException
import groovy.transform.CompileStatic
import groovy.transform.Memoized
import groovy.util.logging.Slf4j
import nextflow.cloud.azure.batch.AzHelper
/**
Expand Down Expand Up @@ -203,7 +202,7 @@ class AzFileSystemProvider extends FileSystemProvider {
if( !accountName )
throw new IllegalArgumentException("Missing AZURE_STORAGE_ACCOUNT_NAME")

def client
BlobServiceClient client

if( managedIdentityUser || managedIdentitySystem ) {
client = createBlobServiceWithManagedIdentity(accountName, managedIdentityUser)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ class AzBatchServiceTest extends Specification {
}

@Unroll
def 'should create manage credentials token' () {
def 'should create user-assigned managed identity credentials token' () {
given:
def config = Mock(AzConfig)
def exec = Mock(AzBatchExecutor) {getConfig() >> new AzConfig(CONFIG) }
Expand All @@ -729,4 +729,5 @@ class AzBatchServiceTest extends Specification {
[:] | null
[managedIdentity: [clientId: 'client-123']] | 'client-123'
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* 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.config

import spock.lang.Specification
import spock.lang.Unroll

/**
*
* @author Paolo Di Tommaso <paolo.ditommaso@gmail.com>
*/
class AzManagedIdentityOptsTest extends Specification {

def 'should create manage identity opts' () {
when:
def opts = new AzManagedIdentityOpts([:])
then:
opts.clientId == null
!opts.system

when:
opts = new AzManagedIdentityOpts(clientId: 'foo', system: false)
then:
opts.clientId == 'foo'
!opts.system

when:
opts = new AzManagedIdentityOpts(system: true)
then:
opts.clientId == null
opts.system

when:
opts = new AzManagedIdentityOpts(system: 'false')
then:
opts.clientId == null
!opts.system

when:
opts = new AzManagedIdentityOpts(system: 'true')
then:
opts.clientId == null
opts.system
}

@Unroll
def 'should get env' () {
when:
def opts = new AzManagedIdentityOpts(OPTS)
then:
opts.getEnv() == ENV

where:
OPTS | ENV
[:] | [AZURE_MANAGED_IDENTITY_USER: null, AZURE_MANAGED_IDENTITY_SYSTEM: false]
[clientId:'foo'] | [AZURE_MANAGED_IDENTITY_USER: 'foo', AZURE_MANAGED_IDENTITY_SYSTEM: false]
[system:'true'] | [AZURE_MANAGED_IDENTITY_USER: null, AZURE_MANAGED_IDENTITY_SYSTEM: true]
}

def 'should check is valid' (){
expect:
!new AzManagedIdentityOpts([:]).isConfigured()
and:
new AzManagedIdentityOpts([clientId: 'foo']).isConfigured()
new AzManagedIdentityOpts([system: true]).isConfigured()

when:
new AzManagedIdentityOpts([clientId: 'foo',system: true]).isConfigured()
then:
thrown(IllegalArgumentException)
}

}

0 comments on commit a639a17

Please sign in to comment.