Skip to content

Commit

Permalink
Improve default container name handling
Browse files Browse the repository at this point in the history 
Signed-off-by: Paolo Di Tommaso <[email protected]>
  • Loading branch information
@pditommaso
pditommaso committed Apr 29, 2024
1 parent d605891 commit 5325f2b
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 83 deletions.
15 changes: 2 additions & 13 deletions src/main/groovy/io/seqera/wave/controller/ContainerController.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,17 @@ import io.seqera.wave.tower.User
import io.seqera.wave.tower.auth.JwtAuthStore
import io.seqera.wave.util.DataTimeUtils
import io.seqera.wave.util.LongRndKey
import io.seqera.wave.util.StringUtils
import jakarta.inject.Inject
import jakarta.inject.Named
import static io.micronaut.http.HttpHeaders.WWW_AUTHENTICATE
import static io.seqera.wave.WaveDefault.TOWER
import static io.seqera.wave.service.builder.BuildFormat.DOCKER
import static io.seqera.wave.service.builder.BuildFormat.SINGULARITY
import static io.seqera.wave.util.ContainerHelper.checkContainerSpec
import static io.seqera.wave.util.ContainerHelper.makeContainerId
import static io.seqera.wave.util.ContainerHelper.condaFileFromRequest
import static io.seqera.wave.util.ContainerHelper.containerFileFromPackages
import static io.seqera.wave.util.ContainerHelper.decodeBase64OrFail
import static io.seqera.wave.util.ContainerHelper.makeContainerId
import static io.seqera.wave.util.ContainerHelper.makeResponseV1
import static io.seqera.wave.util.ContainerHelper.makeResponseV2
import static io.seqera.wave.util.ContainerHelper.makeTargetImage
Expand Down Expand Up @@ -247,16 +246,6 @@ class ContainerController {
}
}

protected String publicRepo(SubmitContainerTokenRequest req) {
if( !buildConfig.defaultPublicRepository )
return null
if( buildConfig.defaultPublicRepository.contains('/') )
return buildConfig.defaultPublicRepository
return !req.nameStrategy || req.nameStrategy==ImageNameStrategy.imageSuffix
? StringUtils.pathConcat(buildConfig.defaultPublicRepository, 'library')
: StringUtils.pathConcat(buildConfig.defaultPublicRepository, 'library/build')
}

BuildRequest makeBuildRequest(SubmitContainerTokenRequest req, PlatformId identity, String ip) {
if( !req.containerFile )
throw new BadRequestException("Missing dockerfile content")
Expand All @@ -270,7 +259,7 @@ class ContainerController {
final spackContent = spackFileFromRequest(req)
final format = req.formatSingularity() ? SINGULARITY : DOCKER
final platform = ContainerPlatform.of(req.containerPlatform)
final buildRepository = req.buildRepository ?: (req.freeze && publicRepo(req) ? publicRepo(req) : buildConfig.defaultBuildRepository)
final buildRepository = req.buildRepository ?: (req.freeze && buildConfig.defaultPublicRepository ? buildConfig.defaultPublicRepository : buildConfig.defaultBuildRepository)
final cacheRepository = req.cacheRepository ?: buildConfig.defaultCacheRepository
final configJson = dockerAuthService.credentialsConfigJson(containerSpec, buildRepository, cacheRepository, identity)
final containerConfig = req.freeze ? req.containerConfig : null
Expand Down
17 changes: 15 additions & 2 deletions src/main/groovy/io/seqera/wave/util/ContainerHelper.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ class ContainerHelper {
static String makeTargetImage(BuildFormat format, String repo, String id, @Nullable String condaFile, @Nullable String spackFile, @Nullable ImageNameStrategy nameStrategy) {
assert id, "Argument 'id' cannot be null or empty"
assert repo, "Argument 'repo' cannot be null or empty"
assert repo.contains('/'), "Argument 'repo' is not a valid container repository name"
assert format, "Argument 'format' cannot be null"

NameVersionPair tools
Expand Down Expand Up @@ -319,7 +318,7 @@ class ContainerHelper {
throw new BadRequestException("Unsupported image naming strategy: '${nameStrategy}'")
}

format==SINGULARITY ? "oras://${repo}:${tag}" : "${repo}:${tag}"
format==SINGULARITY ? "oras://${normaliseRepo(repo)}:${tag}" : "${normaliseRepo(repo)}:${tag}"
}

static protected String normalise0(String tag, int maxLength, String pattern) {
Expand Down Expand Up @@ -358,6 +357,20 @@ class ContainerHelper {
value ? normalise0(value.toLowerCase(), maxLength, /[^a-z0-9_.\-\/]/) : null
}

static protected String normaliseRepo(String value) {
if( !value )
return value
final parts = value.tokenize('/')
if( parts.size()>2 )
return value
if( parts.size()==1 ) {
return parts[0] + '/library/build'
}
else {
return parts[0] + '/library/' + parts[1]
}
}

static String makeContainerId(String containerFile, String condaFile, String spackFile, ContainerPlatform platform, String repository, BuildContext buildContext) {
final attrs = new LinkedHashMap<String,String>(10)
attrs.containerFile = containerFile
Expand Down
45 changes: 7 additions & 38 deletions src/test/groovy/io/seqera/wave/controller/ContainerControllerTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package io.seqera.wave.controller

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

import java.time.Instant
import java.time.temporal.ChronoUnit
Expand All @@ -32,7 +31,6 @@ import io.micronaut.http.client.annotation.Client
import io.micronaut.http.server.util.HttpClientAddressResolver
import io.micronaut.test.extensions.spock.annotation.MicronautTest
import io.seqera.wave.api.ContainerConfig
import io.seqera.wave.api.ImageNameStrategy
import io.seqera.wave.api.PackagesSpec
import io.seqera.wave.api.SubmitContainerTokenRequest
import io.seqera.wave.api.SubmitContainerTokenResponse
Expand Down Expand Up @@ -195,7 +193,7 @@ class ContainerControllerTest extends Specification {
and:
data.containerFile == DOCKER
data.identity.userId == 100
data.containerImage == 'wave/build:be9ee6ac1eeff4b5'
data.containerImage == 'wave/library/build:be9ee6ac1eeff4b5'
data.containerConfig == cfg
data.platform.toString() == 'linux/arm64'
}
Expand Down Expand Up @@ -224,7 +222,7 @@ class ContainerControllerTest extends Specification {
and:
data.containerFile == DOCKER
data.identity.userId == 100
data.containerImage == 'wave/build:be9ee6ac1eeff4b5'
data.containerImage == 'wave/library/build:be9ee6ac1eeff4b5'
data.containerConfig == cfg
data.platform.toString() == 'linux/arm64'
}
Expand All @@ -240,7 +238,7 @@ class ContainerControllerTest extends Specification {
then:
build.containerId =~ /7efaa2ed59c58a16/
build.containerFile == 'FROM foo'
build.targetImage == 'wave/build:7efaa2ed59c58a16'
build.targetImage == 'wave/library/build:7efaa2ed59c58a16'
build.platform == ContainerPlatform.of('amd64')

when:
Expand All @@ -249,7 +247,7 @@ class ContainerControllerTest extends Specification {
then:
build.containerId =~ /7efaa2ed59c58a16/
build.containerFile == 'FROM foo'
build.targetImage == 'wave/build:7efaa2ed59c58a16'
build.targetImage == 'wave/library/build:7efaa2ed59c58a16'
build.platform == ContainerPlatform.of('amd64')

// using 'arm' platform changes the id
Expand All @@ -259,7 +257,7 @@ class ContainerControllerTest extends Specification {
then:
build.containerId =~ /be9ee6ac1eeff4b5/
build.containerFile == 'FROM foo'
build.targetImage == 'wave/build:be9ee6ac1eeff4b5'
build.targetImage == 'wave/library/build:be9ee6ac1eeff4b5'
build.platform == ContainerPlatform.of('arm64')

when:
Expand All @@ -269,7 +267,7 @@ class ContainerControllerTest extends Specification {
build.containerId =~ /c6dac2e544419f71/
build.containerFile == 'FROM foo'
build.condaFile == 'some::conda-recipe'
build.targetImage == 'wave/build:c6dac2e544419f71'
build.targetImage == 'wave/library/build:c6dac2e544419f71'
build.platform == ContainerPlatform.of('arm64')

when:
Expand All @@ -281,7 +279,7 @@ class ContainerControllerTest extends Specification {
build.containerFile.startsWith('# Builder image\n')
build.condaFile == null
build.spackFile == 'some::spack-recipe'
build.targetImage == 'wave/build:b7d730d274d1e057'
build.targetImage == 'wave/library/build:b7d730d274d1e057'
build.platform == ContainerPlatform.of('arm64')
}

Expand Down Expand Up @@ -559,33 +557,4 @@ class ContainerControllerTest extends Specification {
e.message == "Attribute `packages` is not allowed"
}
@Unroll
def 'should return default public repo' () {
given:
def controller = new ContainerController(
inclusionService: Mock(ContainerInclusionService),
allowAnonymous: false,
buildConfig: new BuildConfig(defaultPublicRepository: REPO)
)
expect:
controller.publicRepo(new SubmitContainerTokenRequest(nameStrategy: STRATEGY ? ImageNameStrategy.valueOf(STRATEGY) : null)) == EXPECTED
where:
REPO | STRATEGY | EXPECTED
null | 'none' | null
null | null | null
null | 'tagPrefix' | null
null | 'imageSuffix' | null
and:
'foo.com' | null | 'foo.com/library'
'foo.com' | 'none' | 'foo.com/library/build'
'foo.com' | 'tagPrefix' | 'foo.com/library/build'
'foo.com' | 'imageSuffix' | 'foo.com/library'
and:
'foo.com/bar' | null | 'foo.com/bar'
'foo.com/bar' | 'none' | 'foo.com/bar'
'foo.com/bar' | 'tagPrefix' | 'foo.com/bar'
'foo.com/bar' | 'imageSuffix' | 'foo.com/bar'
}
}
8 changes: 4 additions & 4 deletions src/test/groovy/io/seqera/wave/service/builder/BuildRequestTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class BuildRequestTest extends Specification {

then:
req.containerId == '181ec22b26ae6d04'
req.targetImage == "docker.io/wave:${req.containerId}"
req.targetImage == "docker.io/library/wave:${req.containerId}"
req.containerFile == CONTENT
req.identity == USER
req.configJson == '{"config":"json"}'
Expand Down Expand Up @@ -122,7 +122,7 @@ class BuildRequestTest extends Specification {
)
then:
req.containerId == '8026e3a63b5c863f'
req.targetImage == 'docker.io/wave:samtools-1.0--8026e3a63b5c863f'
req.targetImage == 'docker.io/library/wave:samtools-1.0--8026e3a63b5c863f'
req.condaFile == CONDA_RECIPE
req.spackFile == null
and:
Expand Down Expand Up @@ -157,7 +157,7 @@ class BuildRequestTest extends Specification {
)
then:
req.containerId == '8726782b1d9bb8fb'
req.targetImage == 'docker.io/wave:bwa-0.7.15--8726782b1d9bb8fb'
req.targetImage == 'docker.io/library/wave:bwa-0.7.15--8726782b1d9bb8fb'
req.spackFile == SPACK_RECIPE
req.condaFile == null
and:
Expand Down Expand Up @@ -201,7 +201,7 @@ class BuildRequestTest extends Specification {
)
then:
req.containerId == 'd78ba9cb01188668'
req.targetImage == "oras://docker.io/wave:${req.containerId}"
req.targetImage == "oras://docker.io/library/wave:${req.containerId}"
req.containerFile == CONTENT
req.identity == USER
req.configJson == '{"config":"json"}'
Expand Down
76 changes: 50 additions & 26 deletions src/test/groovy/io/seqera/wave/util/ContainerHelperTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,21 @@ class ContainerHelperTest extends Specification {
'._-xyz._-' | 'xyz'
}

@Unroll
def 'should normalise repo' () {
expect:
ContainerHelper.normaliseRepo(REPO) == EXPECTED
where:
REPO | EXPECTED
null | null
'docker.io/a/b' | 'docker.io/a/b'
'docker.io/a/b/c' | 'docker.io/a/b/c'
'docker.io/foo' | 'docker.io/library/foo'
'docker.io/foo/' | 'docker.io/library/foo'
'docker.io' | 'docker.io/library/build'
'docker.io/' | 'docker.io/library/build'
}

def 'should make request target' () {
expect:
ContainerHelper.makeTargetImage(BuildFormat.DOCKER, 'quay.io/org/name', '12345', null, null, null)
Expand Down Expand Up @@ -606,42 +621,51 @@ class ContainerHelperTest extends Specification {

where:
FORMAT | REPO | ID | CONDA | SPACK | STRATEGY | EXPECTED
'DOCKER' | 'foo.com/build' | '123' | null | null | null | 'foo.com/build:123'
'DOCKER' | 'foo.com/build' | '123' | null | null | 'none' | 'foo.com/build:123'
'DOCKER' | 'foo.com/build' | '123' | null | null | 'tagPrefix' | 'foo.com/build:123'
'DOCKER' | 'foo.com/build' | '123' | null | null | 'imageSuffix' | 'foo.com/build:123'
'DOCKER' | 'foo.com/a/b' | '123' | null | null | null | 'foo.com/a/b:123'
'DOCKER' | 'foo.com/a/b' | '123' | null | null | 'none' | 'foo.com/a/b:123'
'DOCKER' | 'foo.com/a/b' | '123' | null | null | 'tagPrefix' | 'foo.com/a/b:123'
'DOCKER' | 'foo.com/a/b' | '123' | null | null | 'imageSuffix' | 'foo.com/a/b:123'
'DOCKER' | 'foo.com' | '123' | null | null | 'imageSuffix' | 'foo.com/library/build:123'
and:
'SINGULARITY' | 'foo.com/build' | '123' | null | null | null | 'oras://foo.com/build:123'
'SINGULARITY' | 'foo.com/build' | '123' | null | null | 'none' | 'oras://foo.com/build:123'
'SINGULARITY' | 'foo.com/build' | '123' | null | null | 'tagPrefix' | 'oras://foo.com/build:123'
'SINGULARITY' | 'foo.com/build' | '123' | null | null | 'imageSuffix' | 'oras://foo.com/build:123'
'SINGULARITY' | 'foo.com/a/b' | '123' | null | null | null | 'oras://foo.com/a/b:123'
'SINGULARITY' | 'foo.com/a/b' | '123' | null | null | 'none' | 'oras://foo.com/a/b:123'
'SINGULARITY' | 'foo.com/a/b' | '123' | null | null | 'tagPrefix' | 'oras://foo.com/a/b:123'
'SINGULARITY' | 'foo.com/a/b' | '123' | null | null | 'imageSuffix' | 'oras://foo.com/a/b:123'
'SINGULARITY' | 'foo.com' | '123' | null | null | 'imageSuffix' | 'oras://foo.com/library/build:123'
and:
'DOCKER' | 'foo.com/build' | '123' | CONDA1| null | null | 'foo.com/build:samtools-1.0--123'
'DOCKER' | 'foo.com/build' | '123' | CONDA1| null | 'none' | 'foo.com/build:123'
'DOCKER' | 'foo.com/build' | '123' | CONDA1| null | 'tagPrefix' | 'foo.com/build:samtools-1.0--123'
'DOCKER' | 'foo.com/a/b' | '123' | CONDA1| null | null | 'foo.com/a/b:samtools-1.0--123'
'DOCKER' | 'foo.com/a/b' | '123' | CONDA1| null | 'none' | 'foo.com/a/b:123'
'DOCKER' | 'foo.com/a/b' | '123' | CONDA1| null | 'tagPrefix' | 'foo.com/a/b:samtools-1.0--123'
'DOCKER' | 'foo.com/a/b' | '123' | CONDA1| null | 'imageSuffix' | 'foo.com/a/b/samtools:1.0--123'
'DOCKER' | 'foo.com' | '123' | CONDA1| null | 'imageSuffix' | 'foo.com/library/samtools:1.0--123'
'DOCKER' | 'foo.com/library' | '123' | CONDA1| null | 'imageSuffix' | 'foo.com/library/samtools:1.0--123'
'DOCKER' | 'foo.com/build' | '123' | CONDA1| null | 'imageSuffix' | 'foo.com/build/samtools:1.0--123'

and:
'SINGULARITY' | 'foo.com/build' | '123' | CONDA1| null | null | 'oras://foo.com/build:samtools-1.0--123'
'SINGULARITY' | 'foo.com/build' | '123' | CONDA1| null | 'none' | 'oras://foo.com/build:123'
'SINGULARITY' | 'foo.com/build' | '123' | CONDA1| null | 'tagPrefix' | 'oras://foo.com/build:samtools-1.0--123'
'SINGULARITY' | 'foo.com/a/b' | '123' | CONDA1| null | null | 'oras://foo.com/a/b:samtools-1.0--123'
'SINGULARITY' | 'foo.com/a/b' | '123' | CONDA1| null | 'none' | 'oras://foo.com/a/b:123'
'SINGULARITY' | 'foo.com/a/b' | '123' | CONDA1| null | 'tagPrefix' | 'oras://foo.com/a/b:samtools-1.0--123'
'SINGULARITY' | 'foo.com/a/b' | '123' | CONDA1| null | 'imageSuffix' | 'oras://foo.com/a/b/samtools:1.0--123'
'SINGULARITY' | 'foo.com/library' | '123' | CONDA1| null | 'imageSuffix' | 'oras://foo.com/library/samtools:1.0--123'
'SINGULARITY' | 'foo.com/build' | '123' | CONDA1| null | 'imageSuffix' | 'oras://foo.com/build/samtools:1.0--123'

and:
'DOCKER' | 'foo.com/build' | '123' | CONDA2| null | null | 'foo.com/build:samtools-1.0_bamtools-2.0_multiqc-1.15--123'
'DOCKER' | 'foo.com/build' | '123' | CONDA2| null | 'none' | 'foo.com/build:123'
'DOCKER' | 'foo.com/build' | '123' | CONDA2| null | 'tagPrefix' | 'foo.com/build:samtools-1.0_bamtools-2.0_multiqc-1.15--123'
'DOCKER' | 'foo.com/build' | '123' | CONDA2| null | 'imageSuffix' | 'foo.com/build/samtools_bamtools_multiqc:123'
'DOCKER' | 'foo.com/a/b' | '123' | CONDA2| null | null | 'foo.com/a/b:samtools-1.0_bamtools-2.0_multiqc-1.15--123'
'DOCKER' | 'foo.com/a/b' | '123' | CONDA2| null | 'none' | 'foo.com/a/b:123'
'DOCKER' | 'foo.com/a/b' | '123' | CONDA2| null | 'tagPrefix' | 'foo.com/a/b:samtools-1.0_bamtools-2.0_multiqc-1.15--123'
'DOCKER' | 'foo.com/a/b' | '123' | CONDA2| null | 'imageSuffix' | 'foo.com/a/b/samtools_bamtools_multiqc:123'

and:
'DOCKER' | 'foo.com/build' | '123' | null | SPACK1| null | 'foo.com/build:bwa-0.7.15--123'
'DOCKER' | 'foo.com/build' | '123' | null | SPACK1| 'none' | 'foo.com/build:123'
'DOCKER' | 'foo.com/build' | '123' | null | SPACK1| 'tagPrefix' | 'foo.com/build:bwa-0.7.15--123'
'DOCKER' | 'foo.com/build' | '123' | null | SPACK1| 'imageSuffix' | 'foo.com/build/bwa:0.7.15--123'
'DOCKER' | 'foo.com/a/b' | '123' | null | SPACK1| null | 'foo.com/a/b:bwa-0.7.15--123'
'DOCKER' | 'foo.com/a/b' | '123' | null | SPACK1| 'none' | 'foo.com/a/b:123'
'DOCKER' | 'foo.com/a/b' | '123' | null | SPACK1| 'tagPrefix' | 'foo.com/a/b:bwa-0.7.15--123'
'DOCKER' | 'foo.com/a/b' | '123' | null | SPACK1| 'imageSuffix' | 'foo.com/a/b/bwa:0.7.15--123'

and:
'DOCKER' | 'foo.com/build' | '123' | null | SPACK2| null | 'foo.com/build:bwa-0.7.15_salmon-1.1.1--123'
'DOCKER' | 'foo.com/build' | '123' | null | SPACK2| 'none' | 'foo.com/build:123'
'DOCKER' | 'foo.com/build' | '123' | null | SPACK2| 'tagPrefix' | 'foo.com/build:bwa-0.7.15_salmon-1.1.1--123'
'DOCKER' | 'foo.com/build' | '123' | null | SPACK2| 'imageSuffix' | 'foo.com/build/bwa_salmon:123'
'DOCKER' | 'foo.com/a/b' | '123' | null | SPACK2| null | 'foo.com/a/b:bwa-0.7.15_salmon-1.1.1--123'
'DOCKER' | 'foo.com/a/b' | '123' | null | SPACK2| 'none' | 'foo.com/a/b:123'
'DOCKER' | 'foo.com/a/b' | '123' | null | SPACK2| 'tagPrefix' | 'foo.com/a/b:bwa-0.7.15_salmon-1.1.1--123'
'DOCKER' | 'foo.com/a/b' | '123' | null | SPACK2| 'imageSuffix' | 'foo.com/a/b/bwa_salmon:123'
}

def 'should validate containerfile' () {
Expand Down

0 comments on commit 5325f2b

Please sign in to comment.