Prevent container freeze without custom repo (#499)

Signed-off-by: Paolo Di Tommaso <[email protected]>

src/main/groovy/io/seqera/wave/controller/ContainerController.groovy

@@ -209,16 +209,23 @@ class ContainerController {
         if( !v2 && req.nameStrategy )
             throw new BadRequestException("Attribute `nameStrategy` is not allowed by legacy container endpoint")
 
+        // prevent the use of container file and freeze without a custom build repository
+        if( req.containerFile && req.freeze && !isCustomRepo0(req.buildRepository) && (!v2 || (v2 && !req.packages)))
+            throw new BadRequestException("Attribute `buildRepository` must be specified when using freeze mode [1]")
+
+        // prevent the use of container image and freeze without a custom build repository
+        if( req.containerImage && req.freeze && !isCustomRepo0(req.buildRepository) )
+            throw new BadRequestException("Attribute `buildRepository` must be specified when using freeze mode [2]")
+
+        if( v2 && req.packages && req.freeze && !isCustomRepo0(req.buildRepository) && !buildConfig.defaultPublicRepository )
+            throw new BadRequestException("Attribute `buildRepository` must be specified when using freeze mode [3]")
+
         if( v2 && req.packages ) {
             // generate the container file required to assemble the container
             final generated = containerFileFromPackages(req.packages, req.formatSingularity())
             req = req.copyWith(containerFile: generated.bytes.encodeBase64().toString())
         }
 
-        // prevent the use of dockerfile file without providing
-        if( req.containerFile && req.freeze && !isCustomRepo0(req.buildRepository) && (!v2 || (v2 && !req.packages)))
-            throw new BadRequestException("Attribute `buildRepository` must be specified when using freeze mode")
-
         final ip = addressResolver.resolve(httpRequest)
         final data = makeRequestData(req, identity, ip)
         final token = tokenService.computeToken(data)
@@ -277,8 +284,12 @@ class ContainerController {
         if( parts.size()>1 ) {
             return repo
         }
-        else
-            return repo + (strategy==ImageNameStrategy.imageSuffix ? '/library' : '/library/build')
+
+        else {
+            // consider strategy==null the same as 'imageSuffix' considering this is only going to be applied
+            // to community registry which only allows build with Packages spec
+            return repo + (strategy==null || strategy==ImageNameStrategy.imageSuffix ? '/library' : '/library/build')
+        }
     }
 
     BuildRequest makeBuildRequest(SubmitContainerTokenRequest req, PlatformId identity, String ip) {

src/main/groovy/io/seqera/wave/util/ContainerHelper.groovy

@@ -326,20 +326,20 @@ class ContainerHelper {
         def tag = id
         if( nameStrategy==null || nameStrategy==ImageNameStrategy.tagPrefix ) {
             if( condaFile && (tools=guessCondaRecipeName(condaFile,false)) ) {
-                tag = "${normaliseTag(tools.both().join('_'))}--${id}"
+                tag = "${normaliseTag(tools.qualifiedNames())}--${id}"
             }
             else if( spackFile && (tools=guessSpackRecipeName(spackFile,false)) ) {
-                tag = "${normaliseTag(tools.both().join('_'))}--${id}"
+                tag = "${normaliseTag(tools.qualifiedNames())}--${id}"
             }
         }
         else if( nameStrategy==ImageNameStrategy.imageSuffix )  {
             if( condaFile && (tools=guessCondaRecipeName(condaFile,true)) ) {
-                repo = StringUtils.pathConcat(repo, normaliseName(tools.names.join('_')))
+                repo = StringUtils.pathConcat(repo, normaliseName(tools.friendlyNames()))
                 if( tools.versions?.size()==1 && tools.versions[0] )
                     tag = "${normaliseTag(tools.versions[0])}--${id}"
             }
             else if( spackFile && (tools=guessSpackRecipeName(spackFile, true)) ) {
-                repo = StringUtils.pathConcat(repo, normaliseName(tools.names.join('_')))
+                repo = StringUtils.pathConcat(repo, normaliseName(tools.friendlyNames()))
                 if( tools.versions?.size()==1 && tools.versions[0] )
                     tag = "${normaliseTag(tools.versions[0])}--${id}"
             }

src/main/groovy/io/seqera/wave/util/NameVersionPair.groovy

@@ -30,10 +30,13 @@ import groovy.transform.CompileStatic
 @Canonical
 class NameVersionPair {
 
+    private static final String SUFFIX = 'pruned'
+    private static final int MAX = 5
+
     Collection<String> names
     Collection<String> versions
 
-    List<String> both() {
+    private List<String> both() {
         final result = new ArrayList()
         for( int i=0; i<names.size(); i++) {
             final v = versions?[i]
@@ -42,4 +45,22 @@ class NameVersionPair {
         return result
     }
 
+    String friendlyNames(String sep='_') {
+        if( !names )
+            return null
+        if( names.size()<=MAX )
+            return names.join(sep)
+        else
+            return new ArrayList<>(names)[0..MAX-2].join(sep) + sep + SUFFIX
+    }
+
+    String qualifiedNames(String sep='_') {
+        final ret = both()
+        if( !ret )
+            return null
+        if( ret.size()<=MAX )
+            return ret.join(sep)
+        else
+            return new ArrayList<>(ret)[0..MAX-2].join(sep) + sep + SUFFIX
+    }
 }

src/main/resources/application-dev.yml

@@ -3,7 +3,6 @@ wave:
   scan:
     enabled: true
   build:
-    public: 'quay.io/seqera/wave/containers'
     workspace: 'build-workspace'
     spack:
       cacheDirectory: 'spack-cache'

src/test/groovy/io/seqera/wave/controller/ContainerControllerTest.groovy

@@ -510,6 +510,34 @@ class ContainerControllerTest extends Specification {
         then:
         e = thrown(BadRequestException)
         e.message == "Attribute `packages` is not allowed"
+
+        when:
+        req = new SubmitContainerTokenRequest(containerFile: 'from foo', freeze: true)
+        controller.handleRequest(null, req, new PlatformId(new User(id: 100)), false)
+        then:
+        e = thrown(BadRequestException)
+        e.message == "Attribute `buildRepository` must be specified when using freeze mode [1]"
+
+        when:
+        req = new SubmitContainerTokenRequest(containerFile: 'from foo', freeze: true)
+        controller.handleRequest(null, req, new PlatformId(new User(id: 100)), true)
+        then:
+        e = thrown(BadRequestException)
+        e.message == "Attribute `buildRepository` must be specified when using freeze mode [1]"
+
+        when:
+        req = new SubmitContainerTokenRequest(containerImage: 'alpine', freeze: true)
+        controller.handleRequest(null, req, new PlatformId(new User(id: 100)), false)
+        then:
+        e = thrown(BadRequestException)
+        e.message == "Attribute `buildRepository` must be specified when using freeze mode [2]"
+
+        when:
+        req = new SubmitContainerTokenRequest(containerImage: 'alpine', freeze: true)
+        controller.handleRequest(null, req, new PlatformId(new User(id: 100)), true)
+        then:
+        e = thrown(BadRequestException)
+        e.message == "Attribute `buildRepository` must be specified when using freeze mode [2]"
     }
 
     @Unroll
@@ -530,7 +558,7 @@ class ContainerControllerTest extends Specification {
         'foo.com/alpha/beta'| ImageNameStrategy.imageSuffix | 'foo.com'     | 'foo.com/alpha/beta'
         'foo.com/alpha/beta'| ImageNameStrategy.tagPrefix   | 'foo.com'     | 'foo.com/alpha/beta'
         and:
-        'foo.com'           | null                          | 'foo.com'     | 'foo.com/library/build'
+        'foo.com'           | null                          | 'foo.com'     | 'foo.com/library'
         'foo.com'           | ImageNameStrategy.imageSuffix | 'foo.com'     | 'foo.com/library'
         'foo.com'           | ImageNameStrategy.tagPrefix   | 'foo.com'     | 'foo.com/library/build'
         and:

src/test/groovy/io/seqera/wave/util/ContainerHelperTest.groovy

@@ -601,6 +601,17 @@ class ContainerHelperTest extends Specification {
                     - multiqc=1.15
                 '''
 
+    @Shared def CONDA3 = '''\
+                dependencies:
+                    - samtools=1.0
+                    - bamtools=2.0
+                    - multiqc=1.15
+                    - bwa=1.2.3
+                    - xx
+                    - yy
+                    - zz
+                '''
+
     @Shared def PIP1 = '''\
                 channels:
                 - bioconda
@@ -670,6 +681,11 @@ class ContainerHelperTest extends Specification {
         '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'
         and:
+        'DOCKER'      | 'foo.com/build'   | '123'     | CONDA3| null  | null          | 'foo.com/build:samtools-1.0_bamtools-2.0_multiqc-1.15_bwa-1.2.3_pruned--123'
+        'DOCKER'      | 'foo.com/build'   | '123'     | CONDA3| null  | 'none'        | 'foo.com/build:123'
+        'DOCKER'      | 'foo.com/build'   | '123'     | CONDA3| null  | 'tagPrefix'   | 'foo.com/build:samtools-1.0_bamtools-2.0_multiqc-1.15_bwa-1.2.3_pruned--123'
+        'DOCKER'      | 'foo.com/build'   | '123'     | CONDA3| null  | 'imageSuffix' | 'foo.com/build/samtools_bamtools_multiqc_bwa_pruned:123'
+        and:
         'DOCKER'      | 'foo.com/build'   | '123'     | PIP1 | null | null          | 'foo.com/build:pip_pandas-2.2.2--123'
         'DOCKER'      | 'foo.com/build'   | '123'     | PIP1 | null | 'none'        | 'foo.com/build:123'
         'DOCKER'      | 'foo.com/build'   | '123'     | PIP1 | null | 'tagPrefix'   | 'foo.com/build:pip_pandas-2.2.2--123'

src/test/groovy/io/seqera/wave/util/NameVersionPairTest.groovy

@@ -30,14 +30,29 @@ class NameVersionPairTest extends Specification {
     @Unroll
     def 'should join both names and version' () {
         expect:
-        new NameVersionPair(NAMES,VERS).both() == EXPECTED
+        new NameVersionPair(NAMES,VERS).qualifiedNames() == EXPECTED
 
         where:
         NAMES                       | VERS                  | EXPECTED
-        ['foo']                     | ['1.0']               | ['foo-1.0']
-        ['alpha','delta', 'gamma']  | ['1.0',null,'3.0']    | ['alpha-1.0', 'delta', 'gamma-3.0']
-        ['alpha','delta', 'gamma']  | ['1.0']               | ['alpha-1.0', 'delta', 'gamma']
+        ['foo']                     | ['1.0']               | 'foo-1.0'
+        ['alpha','delta', 'gamma']  | ['1.0',null,'3.0']    | 'alpha-1.0_delta_gamma-3.0'
+        ['alpha','delta', 'gamma']  | ['1.0']                   | 'alpha-1.0_delta_gamma'
+        ['a','b','c','d','e']       | ['1','2','3','4','5']     | 'a-1_b-2_c-3_d-4_e-5'
+        ['a','b','c','d','e','f']   | ['1','2','3','4','5','6'] | 'a-1_b-2_c-3_d-4_pruned'
+    }
+
+    @Unroll
+    def 'should join names up to three' () {
+        expect:
+        new NameVersionPair(NAMES,VERS).friendlyNames() == EXPECTED
 
+        where:
+        NAMES                       | VERS                  | EXPECTED
+        ['foo']                     | ['1.0']               | 'foo'
+        ['alpha','delta', 'gamma']  | ['1.0',null,'3.0']    | 'alpha_delta_gamma'
+        ['alpha','delta', 'gamma']  | ['1.0']               | 'alpha_delta_gamma'
+        ['a','b','c','d','e']       | ['1','2','3','4','5'] | 'a_b_c_d_e'
+        ['a','b','c','d','e','f']   | ['1','2','3','4','5'] | 'a_b_c_d_pruned'
     }
 
 }