Extract shared client credentials header logic

cryostatio · Jan 25, 2021 · 8b06ec1 · 8b06ec1
1 parent d1e1c6d
commit 8b06ec1
Show file tree

Hide file tree

Showing 7 changed files with 151 additions and 79 deletions.
diff --git a/src/main/java/com/redhat/rhjmc/containerjfr/rules/PeriodicArchiver.java b/src/main/java/com/redhat/rhjmc/containerjfr/rules/PeriodicArchiver.java
@@ -43,19 +43,17 @@
 
 import java.io.IOException;
 import java.net.URI;
-import java.nio.charset.StandardCharsets;
 import java.util.ArrayDeque;
 import java.util.Queue;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
+import java.util.function.Function;
 
-import org.apache.commons.codec.binary.Base64;
 import org.apache.http.client.utils.URLEncodedUtils;
 
 import com.redhat.rhjmc.containerjfr.core.log.Logger;
 import com.redhat.rhjmc.containerjfr.core.net.Credentials;
-import com.redhat.rhjmc.containerjfr.net.web.http.AbstractAuthenticatedRequestHandler;
 import com.redhat.rhjmc.containerjfr.platform.ServiceRef;
 import com.redhat.rhjmc.containerjfr.util.HttpStatusCodeIdentifier;
 
@@ -72,6 +70,7 @@ class PeriodicArchiver implements Runnable {
     private final WebClient webClient;
     private final String archiveRequestPath;
     private final String deleteRequestPath;
+    private final Function<Credentials, MultiMap> headersFactory;
     private final Logger logger;
 
     private final Queue<String> previousRecordings;
@@ -83,13 +82,15 @@ class PeriodicArchiver implements Runnable {
             WebClient webClient,
             String archiveRequestPath,
             String deleteRequestPath,
+            Function<Credentials, MultiMap> headersFactory,
             Logger logger) {
         this.webClient = webClient;
         this.archiveRequestPath = archiveRequestPath;
         this.deleteRequestPath = deleteRequestPath;
         this.serviceRef = serviceRef;
         this.credentials = credentials;
         this.rule = rule;
+        this.headersFactory = headersFactory;
         this.logger = logger;
 
         // FIXME this needs to be populated at startup by scanning the existing archived recordings,
@@ -132,24 +133,11 @@ void performArchival() throws InterruptedException, ExecutionException {
                                                 URLEncodedUtils.formatSegments(
                                                         rule.getRecordingName())))
                         .normalize();
-        MultiMap headers = MultiMap.caseInsensitiveMultiMap();
-        if (credentials != null) {
-            headers.add(
-                    AbstractAuthenticatedRequestHandler.JMX_AUTHORIZATION_HEADER,
-                    String.format(
-                            "Basic %s",
-                            Base64.encodeBase64String(
-                                    String.format(
-                                                    "%s:%s",
-                                                    credentials.getUsername(),
-                                                    credentials.getPassword())
-                                            .getBytes(StandardCharsets.UTF_8))));
-        }
 
         CompletableFuture<String> future = new CompletableFuture<>();
         this.webClient
                 .patch(path.toString())
-                .putHeaders(headers)
+                .putHeaders(headersFactory.apply(credentials))
                 .sendBuffer(
                         Buffer.buffer("save"),
                         ar -> {
@@ -178,25 +166,11 @@ Future<Boolean> pruneArchive(String recordingName) {
                                         ":recordingName",
                                         URLEncodedUtils.formatSegments(recordingName)))
                         .normalize();
-        // TODO refactor and extract this header creation
-        MultiMap headers = MultiMap.caseInsensitiveMultiMap();
-        if (credentials != null) {
-            headers.add(
-                    AbstractAuthenticatedRequestHandler.JMX_AUTHORIZATION_HEADER,
-                    String.format(
-                            "Basic %s",
-                            Base64.encodeBase64String(
-                                    String.format(
-                                                    "%s:%s",
-                                                    credentials.getUsername(),
-                                                    credentials.getPassword())
-                                            .getBytes(StandardCharsets.UTF_8))));
-        }
 
         CompletableFuture<Boolean> future = new CompletableFuture<>();
         this.webClient
                 .delete(path.toString())
-                .putHeaders(headers)
+                .putHeaders(headersFactory.apply(credentials))
                 .send(
                         ar -> {
                             if (ar.failed()) {

diff --git a/src/main/java/com/redhat/rhjmc/containerjfr/rules/PeriodicArchiverFactory.java b/src/main/java/com/redhat/rhjmc/containerjfr/rules/PeriodicArchiverFactory.java
@@ -41,28 +41,35 @@
  */
 package com.redhat.rhjmc.containerjfr.rules;
 
+import java.util.function.Function;
+
 import com.redhat.rhjmc.containerjfr.core.log.Logger;
 import com.redhat.rhjmc.containerjfr.core.net.Credentials;
 import com.redhat.rhjmc.containerjfr.net.web.http.api.v1.RecordingDeleteHandler;
 import com.redhat.rhjmc.containerjfr.net.web.http.api.v1.TargetRecordingPatchHandler;
 import com.redhat.rhjmc.containerjfr.platform.ServiceRef;
+
+import io.vertx.core.MultiMap;
 import io.vertx.ext.web.client.WebClient;
 
 class PeriodicArchiverFactory {
 
     private final WebClient webClient;
     private final String archiveRequestPath;
     private final String deleteRequestPath;
+    private final Function<Credentials, MultiMap> headersFactory;
     private final Logger logger;
 
     PeriodicArchiverFactory(
             WebClient webClient,
             TargetRecordingPatchHandler archiveHandler,
             RecordingDeleteHandler deleteHandler,
+            Function<Credentials, MultiMap> headersFactory,
             Logger logger) {
         this.webClient = webClient;
         this.archiveRequestPath = archiveHandler.path();
         this.deleteRequestPath = deleteHandler.path();
+        this.headersFactory = headersFactory;
         this.logger = logger;
     }
 
@@ -74,6 +81,7 @@ PeriodicArchiver create(ServiceRef serviceRef, Credentials credentials, Rule rul
                 webClient,
                 archiveRequestPath,
                 deleteRequestPath,
+                headersFactory,
                 logger);
     }
 }
diff --git a/src/main/java/com/redhat/rhjmc/containerjfr/rules/RuleProcessor.java b/src/main/java/com/redhat/rhjmc/containerjfr/rules/RuleProcessor.java
@@ -41,7 +41,6 @@
  */
 package com.redhat.rhjmc.containerjfr.rules;
 
-import java.nio.charset.StandardCharsets;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
@@ -50,17 +49,16 @@
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
+import java.util.function.Function;
 
 import javax.management.remote.JMXServiceURL;
 
-import org.apache.commons.codec.binary.Base64;
 import org.apache.http.client.utils.URLEncodedUtils;
 
 import com.redhat.rhjmc.containerjfr.configuration.CredentialsManager;
 import com.redhat.rhjmc.containerjfr.core.log.Logger;
 import com.redhat.rhjmc.containerjfr.core.net.Credentials;
 import com.redhat.rhjmc.containerjfr.core.net.discovery.JvmDiscoveryClient.EventKind;
-import com.redhat.rhjmc.containerjfr.net.web.http.AbstractAuthenticatedRequestHandler;
 import com.redhat.rhjmc.containerjfr.platform.PlatformClient;
 import com.redhat.rhjmc.containerjfr.platform.TargetDiscoveryEvent;
 import com.redhat.rhjmc.containerjfr.util.HttpStatusCodeIdentifier;
@@ -80,6 +78,7 @@ public class RuleProcessor implements Consumer<TargetDiscoveryEvent> {
     private final WebClient webClient;
     private final String postPath;
     private final PeriodicArchiverFactory periodicArchiverFactory;
+    private final Function<Credentials, MultiMap> headersFactory;
     private final Logger logger;
 
     private final Set<Future<?>> tasks;
@@ -92,6 +91,7 @@ public class RuleProcessor implements Consumer<TargetDiscoveryEvent> {
             WebClient webClient,
             String postPath,
             PeriodicArchiverFactory periodicArchiverFactory,
+            Function<Credentials, MultiMap> headersFactory,
             Logger logger) {
         this.platformClient = platformClient;
         this.registry = registry;
@@ -100,6 +100,7 @@ public class RuleProcessor implements Consumer<TargetDiscoveryEvent> {
         this.webClient = webClient;
         this.postPath = postPath;
         this.periodicArchiverFactory = periodicArchiverFactory;
+        this.headersFactory = headersFactory;
         this.logger = logger;
 
         this.tasks = new HashSet<>();
@@ -193,24 +194,11 @@ private Future<Boolean> startRuleRecording(
         String path =
                 postPath.replaceAll(
                         ":targetId", URLEncodedUtils.formatSegments(serviceUrl.toString()));
-        MultiMap headers = MultiMap.caseInsensitiveMultiMap();
-        if (credentials != null) {
-            headers.add(
-                    AbstractAuthenticatedRequestHandler.JMX_AUTHORIZATION_HEADER,
-                    String.format(
-                            "Basic %s",
-                            Base64.encodeBase64String(
-                                    String.format(
-                                                    "%s:%s",
-                                                    credentials.getUsername(),
-                                                    credentials.getPassword())
-                                            .getBytes(StandardCharsets.UTF_8))));
-        }
 
         CompletableFuture<Boolean> result = new CompletableFuture<>();
         this.webClient
                 .post(path)
-                .putHeaders(headers)
+                .putHeaders(headersFactory.apply(credentials))
                 .sendMultipartForm(
                         form,
                         ar -> {

diff --git a/src/main/java/com/redhat/rhjmc/containerjfr/rules/RulesModule.java b/src/main/java/com/redhat/rhjmc/containerjfr/rules/RulesModule.java
@@ -42,28 +42,34 @@
 package com.redhat.rhjmc.containerjfr.rules;
 
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.concurrent.Executors;
+import java.util.function.Function;
 
 import javax.inject.Named;
 import javax.inject.Singleton;
 
+import org.apache.commons.codec.binary.Base64;
 import com.google.gson.Gson;
 
 import com.redhat.rhjmc.containerjfr.configuration.ConfigurationModule;
 import com.redhat.rhjmc.containerjfr.configuration.CredentialsManager;
 import com.redhat.rhjmc.containerjfr.core.log.Logger;
+import com.redhat.rhjmc.containerjfr.core.net.Credentials;
 import com.redhat.rhjmc.containerjfr.core.sys.FileSystem;
 import com.redhat.rhjmc.containerjfr.net.HttpServer;
 import com.redhat.rhjmc.containerjfr.net.NetworkConfiguration;
+import com.redhat.rhjmc.containerjfr.net.web.http.AbstractAuthenticatedRequestHandler;
 import com.redhat.rhjmc.containerjfr.net.web.http.api.v1.RecordingDeleteHandler;
 import com.redhat.rhjmc.containerjfr.net.web.http.api.v1.TargetRecordingPatchHandler;
 import com.redhat.rhjmc.containerjfr.net.web.http.api.v1.TargetRecordingsPostHandler;
 import com.redhat.rhjmc.containerjfr.platform.PlatformClient;
 
 import dagger.Module;
 import dagger.Provides;
+import io.vertx.core.MultiMap;
 import io.vertx.core.Vertx;
 import io.vertx.ext.web.client.WebClient;
 import io.vertx.ext.web.client.WebClientOptions;
@@ -72,6 +78,7 @@
 public abstract class RulesModule {
     public static final String RULES_SUBDIRECTORY = "rules";
     public static final String RULES_WEB_CLIENT = "RULES_WEB_CLIENT";
+    public static final String RULES_HEADERS_FACTORY = "RULES_HEADERS_FACTORY";
 
     @Provides
     @Singleton
@@ -100,6 +107,7 @@ static RuleProcessor provideRuleProcessor(
             @Named(RULES_WEB_CLIENT) WebClient webClient,
             TargetRecordingsPostHandler postHandler,
             PeriodicArchiverFactory periodicArchiverFactory,
+            @Named(RULES_HEADERS_FACTORY) Function<Credentials, MultiMap> headersFactory,
             Logger logger) {
         return new RuleProcessor(
                 platformClient,
@@ -109,6 +117,7 @@ static RuleProcessor provideRuleProcessor(
                 webClient,
                 postHandler.path(),
                 periodicArchiverFactory,
+                headersFactory,
                 logger);
     }
 
@@ -118,8 +127,10 @@ static PeriodicArchiverFactory providePeriodicArchivedFactory(
             @Named(RULES_WEB_CLIENT) WebClient webClient,
             TargetRecordingPatchHandler archiveHandler,
             RecordingDeleteHandler deleteHandler,
+            @Named(RULES_HEADERS_FACTORY) Function<Credentials, MultiMap> headersFactory,
             Logger logger) {
-        return new PeriodicArchiverFactory(webClient, archiveHandler, deleteHandler, logger);
+        return new PeriodicArchiverFactory(
+                webClient, archiveHandler, deleteHandler, headersFactory, logger);
     }
 
     @Provides
@@ -136,4 +147,25 @@ static WebClient provideRulesWebClient(
                         .setVerifyHost(false);
         return WebClient.create(vertx, opts);
     }
+
+    @Provides
+    @Named(RULES_HEADERS_FACTORY)
+    static Function<Credentials, MultiMap> provideRulesHeadersFactory() {
+        return credentials -> {
+            MultiMap headers = MultiMap.caseInsensitiveMultiMap();
+            if (credentials != null) {
+                headers.add(
+                        AbstractAuthenticatedRequestHandler.JMX_AUTHORIZATION_HEADER,
+                        String.format(
+                                "Basic %s",
+                                Base64.encodeBase64String(
+                                        String.format(
+                                                        "%s:%s",
+                                                        credentials.getUsername(),
+                                                        credentials.getPassword())
+                                                .getBytes(StandardCharsets.UTF_8))));
+            }
+            return headers;
+        };
+    }
 }
diff --git a/src/test/java/com/redhat/rhjmc/containerjfr/rules/PeriodicArchiverTest.java b/src/test/java/com/redhat/rhjmc/containerjfr/rules/PeriodicArchiverTest.java
@@ -41,15 +41,9 @@
  */
 package com.redhat.rhjmc.containerjfr.rules;
 
-import java.util.Base64;
 
 import javax.management.remote.JMXServiceURL;
 
-import com.redhat.rhjmc.containerjfr.core.log.Logger;
-import com.redhat.rhjmc.containerjfr.core.net.Credentials;
-import com.redhat.rhjmc.containerjfr.net.web.http.AbstractAuthenticatedRequestHandler;
-import com.redhat.rhjmc.containerjfr.platform.ServiceRef;
-
 import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
 import org.junit.jupiter.api.Assertions;
@@ -63,6 +57,10 @@
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.mockito.stubbing.Answer;
 
+import com.redhat.rhjmc.containerjfr.core.log.Logger;
+import com.redhat.rhjmc.containerjfr.core.net.Credentials;
+import com.redhat.rhjmc.containerjfr.platform.ServiceRef;
+
 import io.vertx.core.AsyncResult;
 import io.vertx.core.Handler;
 import io.vertx.core.MultiMap;
@@ -92,6 +90,7 @@ class PeriodicArchiverTest {
     @Mock WebClient webClient;
     String archiveRequestPath = "/api/v1/targets/:targetId/recordings/:recordingName";
     String deleteRequestPath = "/api/v1/recordings/:recordingName";
+    @Mock MultiMap headers;
     @Mock Logger logger;
 
     @BeforeEach
@@ -105,6 +104,7 @@ void setup() throws Exception {
                         webClient,
                         archiveRequestPath,
                         deleteRequestPath,
+                        c -> headers,
                         logger);
     }
 
@@ -140,13 +140,8 @@ public Void answer(InvocationOnMock invocation) throws Throwable {
 
         ArgumentCaptor<MultiMap> headersCaptor = ArgumentCaptor.forClass(MultiMap.class);
         Mockito.verify(request).putHeaders(headersCaptor.capture());
-        MultiMap headers = headersCaptor.getValue();
-        MatcherAssert.assertThat(
-                headers.get(AbstractAuthenticatedRequestHandler.JMX_AUTHORIZATION_HEADER),
-                Matchers.equalTo(
-                        "Basic "
-                                + Base64.getEncoder()
-                                        .encodeToString("foouser:barpassword".getBytes())));
+        MultiMap capturedHeaders = headersCaptor.getValue();
+        MatcherAssert.assertThat(capturedHeaders, Matchers.sameInstance(headers));
 
         Mockito.verify(webClient)
                 .patch(
@@ -183,13 +178,8 @@ public Void answer(InvocationOnMock invocation) throws Throwable {
 
         ArgumentCaptor<MultiMap> headersCaptor = ArgumentCaptor.forClass(MultiMap.class);
         Mockito.verify(request).putHeaders(headersCaptor.capture());
-        MultiMap headers = headersCaptor.getValue();
-        MatcherAssert.assertThat(
-                headers.get(AbstractAuthenticatedRequestHandler.JMX_AUTHORIZATION_HEADER),
-                Matchers.equalTo(
-                        "Basic "
-                                + Base64.getEncoder()
-                                        .encodeToString("foouser:barpassword".getBytes())));
+        MultiMap capturedHeaders = headersCaptor.getValue();
+        MatcherAssert.assertThat(capturedHeaders, Matchers.sameInstance(headers));
 
         Mockito.verify(webClient).delete("/api/v1/recordings/auto_Test_Rule_1");
     }