Fix distribution GAV value validation (#20)

octopusden · Sep 29, 2023 · 0c9049a · 0c9049a
1 parent 43f9908
commit 0c9049a
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 33 deletions.
diff --git a/...rg.octopusden/octopus/escrow/configuration/validation/GroovySlurperConfigValidator.groovy b/...rg.octopusden/octopus/escrow/configuration/validation/GroovySlurperConfigValidator.groovy
@@ -23,7 +23,7 @@ class GroovySlurperConfigValidator {
     public static final String DEPENDENCIES = 'dependencies'
     public static final String SECURITY_GROUPS = 'securityGroups'
     public static final String SECURITY_GROUPS_READ = "read"
-    private static final String FILE_PATTERN = "(file:)(/|/{3})([^/\\\\0,]+(/)?)+"
+    private static final String FILE_PATTERN = "file:/.+"
     private static final String PROHIBITED_SYMBOLS = "\\\\\\s:|\\?\\*\"'<>\\+"
     private static final String GAV_PROHIBITED_SYMBOLS = "/$PROHIBITED_SYMBOLS"
     private static final String GAV_MAVEN_PATTERN = "[^$GAV_PROHIBITED_SYMBOLS]+(:[^$GAV_PROHIBITED_SYMBOLS]+){1,3}"
@@ -230,31 +230,22 @@ class GroovySlurperConfigValidator {
         }
     }
 
-    def validateDistributionSection(ConfigObject distributionSection, VersionNames versionNames,  String moduleName, String moduleConfigName) {
+    def validateDistributionSection(ConfigObject distributionSection, VersionNames versionNames, String moduleName, String moduleConfigName) {
         validateForUnknownAttributes(distributionSection, DISTRIBUTION, SUPPORTED_DISTRIBUTION_ATTRIBUTES, moduleName, moduleConfigName)
-        def numericFormatFactory = new NumericVersionFactory(versionNames)
+        def expressionContext = new EscrowExpressionContext("validation", "1.0", "distribution.zip", new NumericVersionFactory(versionNames))
         if (distributionSection.containsKey("GAV")) {
             try {
-                def gavValue = EscrowExpressionParser.getInstance().parseAndEvaluate(distributionSection.get("GAV") as String, EscrowExpressionContext.getValidationEscrowExpressionContext(numericFormatFactory))
-                try { //TODO: GAV_PATTERN should be used to verify whole gavValue
-                    DistributionUtilities.parseDistributionGAV(gavValue as String).forEach { distributionItem ->
-                        if (distributionItem instanceof MavenArtifactDistributionEntity) {
-                            if (!GAV_PATTERN.matcher(distributionItem.gav).matches()) {
-                                //TODO: MavenArtifactDistributionEntity should match GAV_MAVEN_PATTERN instead of GAV_PATTERN
-                                registerError("GAV '${distributionItem.gav}' must match pattern '$GAV_PATTERN'")
-                            }
-                        }
-                    } //TODO: FileDistributionEntity should match GAV_FILE_PATTERN
-                } catch (Exception parsingException) {
-                    registerError("Fail to parse GAV: " + parsingException)
+                def gavValue = EscrowExpressionParser.getInstance().parseAndEvaluate(distributionSection.get("GAV") as String, expressionContext)
+                if (!GAV_PATTERN.matcher(gavValue as String).matches()) {
+                    registerError("GAV '$gavValue' must match pattern '$GAV_PATTERN'")
                 }
             } catch (Exception exception) {
                 registerError("GAV expression is not valid: " + exception.getMessage())
             }
         }
         if (distributionSection.containsKey("DEB")) {
             try {
-                def debValue = EscrowExpressionParser.getInstance().parseAndEvaluate(distributionSection.get("DEB") as String, EscrowExpressionContext.getValidationEscrowExpressionContext(numericFormatFactory))
+                def debValue = EscrowExpressionParser.getInstance().parseAndEvaluate(distributionSection.get("DEB") as String, expressionContext)
                 if (!DEB_PATTERN.matcher(debValue as String).matches()) {
                     registerError("DEB '$debValue' must match pattern '$DEB_PATTERN'")
                 }
@@ -264,7 +255,7 @@ class GroovySlurperConfigValidator {
         }
         if (distributionSection.containsKey("RPM")) {
             try {
-                def rpmValue = EscrowExpressionParser.getInstance().parseAndEvaluate(distributionSection.get("RPM") as String, EscrowExpressionContext.getValidationEscrowExpressionContext(numericFormatFactory))
+                def rpmValue = EscrowExpressionParser.getInstance().parseAndEvaluate(distributionSection.get("RPM") as String, expressionContext)
                 if (!RPM_PATTERN.matcher(rpmValue as String).matches()) {
                     registerError("RPM '$rpmValue' must match pattern '$RPM_PATTERN'")
                 }

diff --git a/...esolver-core/src/main/java/org/octopusden/octopus/escrow/dto/EscrowExpressionContext.java b/...esolver-core/src/main/java/org/octopusden/octopus/escrow/dto/EscrowExpressionContext.java
@@ -7,7 +7,6 @@
 import java.util.function.Function;
 
 public class EscrowExpressionContext {
-    private static volatile EscrowExpressionContext VALIDATION_CONTEXT = null;
     private final Map<String, String> env = System.getenv();
     private final String fileName;
     private final String version;
@@ -27,19 +26,6 @@ public EscrowExpressionContext(String component, String version, String fileName
         this(component, version, fileName, componentVersion -> numericVersionFactory.create(componentVersion));
     }
 
-    public static EscrowExpressionContext getValidationEscrowExpressionContext(NumericVersionFactory numericVersionFactory) {
-        if (VALIDATION_CONTEXT != null) {
-            return VALIDATION_CONTEXT;
-        }
-        synchronized (EscrowExpressionContext.class) {
-            if (VALIDATION_CONTEXT == null) {
-                VALIDATION_CONTEXT = new EscrowExpressionContext("zenit", "1984", "gold-medal.zip", numericVersionFactory);
-            }
-        }
-        return VALIDATION_CONTEXT;
-    }
-
-
     public String getFileName() {
         return fileName;
     }

diff --git a/...ctopusden/octopus/escrow/configuration/validation/GroovySlurperConfigValidatorTest.groovy b/...ctopusden/octopus/escrow/configuration/validation/GroovySlurperConfigValidatorTest.groovy
@@ -9,7 +9,7 @@ class GroovySlurperConfigValidatorTest extends GroovyTestCase {
     void testGAVPattern() {
         assert GAV_PATTERN.matcher("org.octopusden.octopus.bcomponent:builder:war,org.octopusden.octopus.bcomponent:builder:jar").matches()
         assert GAV_PATTERN.matcher("org.octopusden.octopus.bcomponent:builder:war,org.octopusden.octopus.bcomponent:builder:jar,file:///dir/file").matches()
-        assert !GAV_PATTERN.matcher("org.octopusden.octopus.bcomponent:builder:war,org.octopusden.octopus.bcomponent:builder:jar,file://dir/file").matches()
+        assert !GAV_PATTERN.matcher("org.octopusden.octopus.bcomponent:builder:war,org.octopusden.octopus.bcomponent:builder:jar,file/dir/file").matches()
         assert GAV_PATTERN.matcher("groupId:artifactId:package:classifier,file:/dir/file").matches()
     }
 

diff --git a/.../test/groovy/org/octopusden/octopus/escrow/resolvers/EscrowConfigurationLoaderTest.groovy b/.../test/groovy/org/octopusden/octopus/escrow/resolvers/EscrowConfigurationLoaderTest.groovy
@@ -399,7 +399,7 @@ class EscrowConfigurationLoaderTest extends GroovyTestCase {
             loadConfiguration("invalid/invalidDistributionGAV.groovy")
             assert false: 'EscrowException should be thrown'
         } catch (EscrowConfigurationException e) {
-            assert e.message.contains("GAV 'org.octopusden.octopus.bcomponent:build/er:war' must match pattern")
+            assert e.message.contains("GAV 'org.octopusden.octopus.bcomponent:build/er:war,org.octopusden.octopus.bcomponent:builder:jar' must match pattern")
         }
     }