Repository: EnMasseProject/enmasse
Pull request #2192 (2019-01-11 10:07:19)

Fix up a few things in KubeUtil
This includes:

* Using compiled patterns
* Added unit tests
* Private constructor for utility class
* Make constant actually constant


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 21a06c991..9e2f32e8d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,27 @@
+## 0.26.0
+* Support for switching address space plans
+* Support for switching address plans for `anycast`, `multicast`, and `queue` address types in standard address space
+* Readiness on initial configuration for routers in standard address space. This ensures that routers can only be connected to once they have receive initial configuration, and will allow a HA configuration (> 1) routers to upgrade without downtime for the service.
+* Router configuration now enforces authenticatePeer: yes, requiring clients to perform SASL handshake
+* Allow querying all EnMasse resources cluster wide
+* Allow listing connections by container id by clicking on link in Addresses pane in the console
+* Update all API versions from v1alpha1 to v1beta1. v1alpha1 is now deprecated for all EnMasse custom resources and will get removed.
+* Bugfix: allow address full policy to be set for brokers in standard address space
+* Allow clients to authenticate as service account users by supplying @@serviceaccount@@ as the username and the token as the password.
+
+## 0.25.0
+* Prometheus metrics and alerts for address health are now exposed
+* Pooled broker deployments now use N deployments with 1 broker instead of 1 deployment with N brokers. This is to handle a resize bug in previous versions. When upgrading, take care to drain all queues first to avoid message loss.
+* Improved command line output of addresses and address spaces with more tabular information about the resource(s)
+* Allow configuring storageClass used for broker volumes
+* Add support for serviceaccount authentication type for users
+* Allow resizing volumes if configured (and underlying provider supports it)
+* Add support for nodeAffinity preferred labels for operators and messaging infra
+* Add support for configuring network network policies for address space infrastructure
+* Add support for changing address space plans
+* Split broker image into a 'broker' and 'broker-plugin' image run as initContainer image
+* Bugfixes in address handling in standard-controller
+
 ## 0.24.0 (...)
 * Support automatic upgrades of EnMasse based on infrastructure configuration versions
 * Update default set of address space plans and address plans to better show their use
diff --git a/api-common/pom.xml b/api-common/pom.xml
index 6b4ec0550..cd038d20b 100644
--- a/api-common/pom.xml
+++ b/api-common/pom.xml
@@ -97,6 +97,7 @@
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-api</artifactId>
+      <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>org.junit.jupiter</groupId>
@@ -111,7 +112,7 @@
     <dependency>
       <groupId>org.junit.platform</groupId>
       <artifactId>junit-platform-launcher</artifactId>
-      <scope>compile</scope>
+      <scope>test</scope>
     </dependency>
   </dependencies>
 </project>
diff --git a/api-model/pom.xml b/api-model/pom.xml
index 388efc216..9f474be8a 100644
--- a/api-model/pom.xml
+++ b/api-model/pom.xml
@@ -36,6 +36,11 @@
       <artifactId>junit-jupiter-engine</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-params</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.junit.platform</groupId>
       <artifactId>junit-platform-launcher</artifactId>
diff --git a/api-model/src/main/java/io/enmasse/address/model/KubeUtil.java b/api-model/src/main/java/io/enmasse/address/model/KubeUtil.java
index 7b860952f..4c31f59c5 100644
--- a/api-model/src/main/java/io/enmasse/address/model/KubeUtil.java
+++ b/api-model/src/main/java/io/enmasse/address/model/KubeUtil.java
@@ -4,15 +4,20 @@
  */
 package io.enmasse.address.model;
 
+import java.util.regex.Pattern;
+
 import io.enmasse.config.AnnotationKeys;
 
 /**
  * Various static utilities that don't belong in a specific place
  */
-public class KubeUtil {
-    private static int MAX_KUBE_NAME = 63 - 3; // max length of identifier - space for pod identifier
-    private static final String addressPattern = "[^a-z0-9\\-]";
-    private static final String usernamePattern = "[^a-z0-9\\-.@_]";
+public final class KubeUtil {
+    private static final int MAX_KUBE_NAME = 63 - 3; // max length of identifier - space for pod identifier
+    private static final Pattern addressPattern = Pattern.compile("[^a-z0-9\\-]");
+    private static final Pattern usernamePattern = Pattern.compile("[^a-z0-9\\-.@_]");
+
+    public KubeUtil() {
+    }
 
     public static String sanitizeName(String name) {
         return sanitizeWithPattern(name, addressPattern);
@@ -22,12 +27,15 @@ public class KubeUtil {
         return sanitizeWithPattern(name, usernamePattern);
     }
 
-    private static String sanitizeWithPattern(String value, String pattern) {
+    private static String sanitizeWithPattern(String value, Pattern pattern) {
         if (value == null) {
             return null;
         }
 
-        String clean = value.toLowerCase().replaceAll(pattern, "");
+        String clean = pattern
+                        .matcher(value.toLowerCase())
+                        .replaceAll("");
+
         if (clean.startsWith("-")) {
             clean = clean.replaceFirst("-", "1");
         }
@@ -39,6 +47,7 @@ public class KubeUtil {
         if (clean.endsWith("-")) {
             clean = clean.substring(0, clean.length() - 1) + "1";
         }
+
         return clean;
     }
 
@@ -88,7 +97,7 @@ public class KubeUtil {
             throw new IllegalArgumentException("Name length is longer than " + MAX_KUBE_NAME + " characters");
         }
 
-        if (name.matches(addressPattern)) {
+        if (addressPattern.matcher(name).matches()) {
             throw new IllegalArgumentException("Illegal characters found in " + name + ". Must not match " + addressPattern);
         }
     }
diff --git a/broker-plugin/plugin/src/main/resources/standard/colocated/broker.xml b/broker-plugin/plugin/src/main/resources/standard/colocated/broker.xml
index 4804012f1..ad30e3a0e 100644
--- a/broker-plugin/plugin/src/main/resources/standard/colocated/broker.xml
+++ b/broker-plugin/plugin/src/main/resources/standard/colocated/broker.xml
@@ -68,7 +68,7 @@ under the License.
       <global-max-size>${GLOBAL_MAX_SIZE}</global-max-size>
 
       <acceptors>
-         <acceptor name="artemis">tcp://0.0.0.0:61616?tcpSendBufferSize=1048576;tcpReceiveBufferSize=1048576</acceptor>
+         <acceptor name="artemis">tcp://0.0.0.0:61616?tcpSendBufferSize=1048576;tcpReceiveBufferSize=1048576;protocols=CORE</acceptor>
          <acceptor name="amqp">tcp://0.0.0.0:5673?protocols=AMQP;sslEnabled=true;keyStorePath=${KEYSTORE_PATH};keyStorePassword=enmasse;trustStorePath=${TRUSTSTORE_PATH};trustStorePassword=enmasse;verifyHost=false;needClientAuth=true</acceptor>
       </acceptors>
 
diff --git a/broker-plugin/plugin/src/main/resources/standard/sharded-queue/broker.xml b/broker-plugin/plugin/src/main/resources/standard/sharded-queue/broker.xml
index 644738610..3d64835af 100644
--- a/broker-plugin/plugin/src/main/resources/standard/sharded-queue/broker.xml
+++ b/broker-plugin/plugin/src/main/resources/standard/sharded-queue/broker.xml
@@ -70,7 +70,7 @@ under the License.
       <global-max-size>${GLOBAL_MAX_SIZE}</global-max-size>
 
       <acceptors>
-         <acceptor name="artemis">tcp://0.0.0.0:61616?tcpSendBufferSize=1048576;tcpReceiveBufferSize=1048576</acceptor>
+         <acceptor name="artemis">tcp://0.0.0.0:61616?tcpSendBufferSize=1048576;tcpReceiveBufferSize=1048576;protocols=CORE</acceptor>
          <acceptor name="amqp">tcp://0.0.0.0:5673?protocols=AMQP;sslEnabled=true;keyStorePath=${KEYSTORE_PATH};keyStorePassword=enmasse;trustStorePath=${TRUSTSTORE_PATH};trustStorePassword=enmasse;verifyHost=false;needClientAuth=true</acceptor>
       </acceptors>
 
diff --git a/broker-plugin/plugin/src/main/resources/standard/sharded-topic/broker.xml b/broker-plugin/plugin/src/main/resources/standard/sharded-topic/broker.xml
index dc85b1e27..258de8811 100644
--- a/broker-plugin/plugin/src/main/resources/standard/sharded-topic/broker.xml
+++ b/broker-plugin/plugin/src/main/resources/standard/sharded-topic/broker.xml
@@ -71,7 +71,7 @@ under the License.
       <global-max-size>${GLOBAL_MAX_SIZE}</global-max-size>
 
       <acceptors>
-         <acceptor name="artemis">tcp://0.0.0.0:61616?tcpSendBufferSize=1048576;tcpReceiveBufferSize=1048576</acceptor>
+         <acceptor name="artemis">tcp://0.0.0.0:61616?tcpSendBufferSize=1048576;tcpReceiveBufferSize=1048576;protocols=CORE</acceptor>
          <acceptor name="amqp">tcp://0.0.0.0:5673?protocols=AMQP;sslEnabled=true;keyStorePath=${KEYSTORE_PATH};keyStorePassword=enmasse;trustStorePath=${TRUSTSTORE_PATH};trustStorePassword=enmasse;verifyHost=false;needClientAuth=true</acceptor>
       </acceptors>
 
diff --git a/documentation/common/attributes.adoc b/documentation/common/attributes.adoc
index 84694f608..808aa6c96 100644
--- a/documentation/common/attributes.adoc
+++ b/documentation/common/attributes.adoc
@@ -4,7 +4,7 @@
 :doctype: book
 :experimental:
 :idprefix:
-:imagesdir: images
+:imagesdir: ../images
 :numbered:
 :sectanchors!:
 :sectnums:
diff --git a/documentation/modules/proc-logging-in-console.adoc b/documentation/modules/proc-logging-in-console.adoc
index 6457ae223..c03c2e663 100644
--- a/documentation/modules/proc-logging-in-console.adoc
+++ b/documentation/modules/proc-logging-in-console.adoc
@@ -13,5 +13,5 @@
 
 . Log in with your OpenShift user credentials. The {ProductName} console opens.
 
-image::../images/console-screenshot.png[{ProductName} console]
+image::{imagesdir}/console-screenshot.png[{ProductName} console]
 
diff --git a/keycloak-controller/pom.xml b/keycloak-controller/pom.xml
index ca841eb0a..d3f27eb58 100644
--- a/keycloak-controller/pom.xml
+++ b/keycloak-controller/pom.xml
@@ -46,6 +46,7 @@
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-api</artifactId>
+      <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>org.junit.jupiter</groupId>
@@ -60,7 +61,7 @@
     <dependency>
       <groupId>org.junit.platform</groupId>
       <artifactId>junit-platform-launcher</artifactId>
-      <scope>compile</scope>
+      <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>org.mockito</groupId>
diff --git a/mqtt-lwt/pom.xml b/mqtt-lwt/pom.xml
index c333ca09e..6bd169794 100644
--- a/mqtt-lwt/pom.xml
+++ b/mqtt-lwt/pom.xml
@@ -31,6 +31,7 @@
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-api</artifactId>
+      <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>org.junit.jupiter</groupId>
@@ -45,7 +46,7 @@
     <dependency>
       <groupId>org.junit.platform</groupId>
       <artifactId>junit-platform-launcher</artifactId>
-      <scope>compile</scope>
+      <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>io.vertx</groupId>
diff --git a/pom.xml b/pom.xml
index 9eb31a0a4..749e0ea36 100644
--- a/pom.xml
+++ b/pom.xml
@@ -534,6 +534,11 @@
                 <artifactId>junit-jupiter-engine</artifactId>
                 <version>${junit.jupiter.version}</version>
             </dependency>
+            <dependency>
+                <groupId>org.junit.jupiter</groupId>
+                <artifactId>junit-jupiter-params</artifactId>
+                <version>${junit.jupiter.version}</version>
+            </dependency>
             <dependency>
                 <groupId>org.junit.platform</groupId>
                 <artifactId>junit-platform-launcher</artifactId>
diff --git a/standard-controller/pom.xml b/standard-controller/pom.xml
index 5050eabe1..5646c3b48 100644
--- a/standard-controller/pom.xml
+++ b/standard-controller/pom.xml
@@ -66,6 +66,7 @@
     <dependency>
       <groupId>org.hamcrest</groupId>
       <artifactId>hamcrest-all</artifactId>
+      <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>org.junit.platform</groupId>


Test patch:
diff --git a/api-model/src/test/java/io/enmasse/address/model/v1/address/KubeUtilTest.java b/api-model/src/test/java/io/enmasse/address/model/v1/address/KubeUtilTest.java
index 0c1925299..b4e4187d8 100644
--- a/api-model/src/test/java/io/enmasse/address/model/v1/address/KubeUtilTest.java
+++ b/api-model/src/test/java/io/enmasse/address/model/v1/address/KubeUtilTest.java
@@ -5,13 +5,28 @@
 package io.enmasse.address.model.v1.address;
 
 import io.enmasse.address.model.KubeUtil;
+
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.params.provider.Arguments.of;
+
+import java.util.stream.Stream;
+
+import org.hamcrest.Matchers;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
+import static java.util.stream.Stream.concat;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 
 
 public class KubeUtilTest {
+
     @Test
     public void testLeaveSpaceForPodIdentifier() {
         String address = "receiver-round-robincli_rhearatherlongaddresswhichcanbeverylongblablabla";
@@ -20,4 +35,70 @@ public class KubeUtilTest {
         String id2 = KubeUtil.sanitizeName(id);
         assertThat(id, is(id2));
     }
+
+    @Test
+    public void testNull() {
+        assertNull(KubeUtil.sanitizeName(null));
+        assertNull(KubeUtil.sanitizeUserName(null));
+    }
+
+    protected static Stream<Arguments> commonNames() {
+        return Stream.of(
+                        of("foo", "foo"),
+                        of("-foo", "1foo"),
+                        of("foo-", "foo1"),
+                        of("-foo-", "1foo1"),
+                        of("--foo--", "1-foo-1"),
+                        of("--f-o-o--", "1-f-o-o-1"),
+
+                        of("foo#bar", "foobar"),
+                        of("#foo#bar", "foobar"),
+                        of("foo#bar#", "foobar"),
+                        of("#foo#bar#", "foobar"),
+
+                        of("foo##bar", "foobar"),
+                        of("foo#&&#bar", "foobar"),
+                        of("foo bar", "foobar"),
+
+                        of("###", ""),
+
+                        of("", "")
+
+                        );
+    }
+    
+    protected static Stream<Arguments> addressNames() {
+        return concat(
+                        commonNames(),
+                        Stream.of(
+                                        of("foo@bar", "foobar"),
+                                        of("foo.bar", "foobar")
+                                        ));
+    }
+
+    @ParameterizedTest
+    @MethodSource("addressNames")
+    public void testSanitizeAddressName(final String input, final String output) {
+        final String result = KubeUtil.sanitizeName(input);
+
+        assertThat(result.length(), Matchers.lessThanOrEqualTo(60));
+        assertEquals(output, result);
+
+    }
+
+    protected static Stream<Arguments> userNames() {
+        return concat(
+                        commonNames(),
+                        Stream.of(
+                                        of("foo@bar", "foo@bar"),
+                                        of("foo.bar", "foo.bar")
+                                        ));
+    }
+
+    @ParameterizedTest
+    @MethodSource("userNames")
+    public void testSanitizeUserName(final String input, final String output) {
+        assertEquals(output, KubeUtil.sanitizeUserName(input));
+    }
+
 }