diff --git a/src/main/java/com/redhat/rhjmc/containerjfr/rules/PeriodicArchiver.java b/src/main/java/com/redhat/rhjmc/containerjfr/rules/PeriodicArchiver.java index 6bc2695ed4..ce9a9eb686 100644 --- 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 headersFactory; private final Logger logger; private final Queue previousRecordings; @@ -83,6 +82,7 @@ class PeriodicArchiver implements Runnable { WebClient webClient, String archiveRequestPath, String deleteRequestPath, + Function headersFactory, Logger logger) { this.webClient = webClient; this.archiveRequestPath = archiveRequestPath; @@ -90,6 +90,7 @@ class PeriodicArchiver implements Runnable { 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 future = new CompletableFuture<>(); this.webClient .patch(path.toString()) - .putHeaders(headers) + .putHeaders(headersFactory.apply(credentials)) .sendBuffer( Buffer.buffer("save"), ar -> { @@ -178,25 +166,11 @@ Future 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 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 index 7f0613259e..9de68b5915 100644 --- a/src/main/java/com/redhat/rhjmc/containerjfr/rules/PeriodicArchiverFactory.java +++ b/src/main/java/com/redhat/rhjmc/containerjfr/rules/PeriodicArchiverFactory.java @@ -41,11 +41,15 @@ */ 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 { @@ -53,16 +57,19 @@ class PeriodicArchiverFactory { private final WebClient webClient; private final String archiveRequestPath; private final String deleteRequestPath; + private final Function headersFactory; private final Logger logger; PeriodicArchiverFactory( WebClient webClient, TargetRecordingPatchHandler archiveHandler, RecordingDeleteHandler deleteHandler, + Function 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 index 26d3ddea87..0648279307 100644 --- 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 { private final WebClient webClient; private final String postPath; private final PeriodicArchiverFactory periodicArchiverFactory; + private final Function headersFactory; private final Logger logger; private final Set> tasks; @@ -92,6 +91,7 @@ public class RuleProcessor implements Consumer { WebClient webClient, String postPath, PeriodicArchiverFactory periodicArchiverFactory, + Function headersFactory, Logger logger) { this.platformClient = platformClient; this.registry = registry; @@ -100,6 +100,7 @@ public class RuleProcessor implements Consumer { 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 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 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 index 781975fb82..3aa6b63fc6 100644 --- a/src/main/java/com/redhat/rhjmc/containerjfr/rules/RulesModule.java +++ b/src/main/java/com/redhat/rhjmc/containerjfr/rules/RulesModule.java @@ -42,21 +42,26 @@ 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; @@ -64,6 +69,7 @@ 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 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 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 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 index d0b079c307..589787dab8 100644 --- 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 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 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"); } diff --git a/src/test/java/com/redhat/rhjmc/containerjfr/rules/RuleProcessorTest.java b/src/test/java/com/redhat/rhjmc/containerjfr/rules/RuleProcessorTest.java index 5a17c22cd9..c7b91d2a79 100644 --- a/src/test/java/com/redhat/rhjmc/containerjfr/rules/RuleProcessorTest.java +++ b/src/test/java/com/redhat/rhjmc/containerjfr/rules/RuleProcessorTest.java @@ -41,7 +41,6 @@ */ package com.redhat.rhjmc.containerjfr.rules; -import java.util.Base64; import java.util.HashSet; import java.util.Set; import java.util.concurrent.ScheduledExecutorService; @@ -65,7 +64,6 @@ 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.ServiceRef; import com.redhat.rhjmc.containerjfr.platform.TargetDiscoveryEvent; @@ -91,6 +89,7 @@ class RuleProcessorTest { @Mock WebClient webClient; String postPath = "/api/v1/targets/:targetId/recordings"; @Mock PeriodicArchiverFactory periodicArchiverFactory; + @Mock MultiMap headers; @Mock Logger logger; @BeforeEach @@ -104,6 +103,7 @@ void setup() { webClient, postPath, periodicArchiverFactory, + c -> headers, logger); } @@ -195,13 +195,8 @@ public Void answer(InvocationOnMock invocation) throws Throwable { ArgumentCaptor 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(scheduler).scheduleAtFixedRate(periodicArchiver, 67, 67, TimeUnit.SECONDS); } diff --git a/src/test/java/com/redhat/rhjmc/containerjfr/rules/RulesHeadersFactoryTest.java b/src/test/java/com/redhat/rhjmc/containerjfr/rules/RulesHeadersFactoryTest.java new file mode 100644 index 0000000000..2a7aca983f --- /dev/null +++ b/src/test/java/com/redhat/rhjmc/containerjfr/rules/RulesHeadersFactoryTest.java @@ -0,0 +1,85 @@ +/*- + * #%L + * Container JFR + * %% + * Copyright (C) 2020 Red Hat, Inc. + * %% + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or data + * (collectively the "Software"), free of charge and under any and all copyright + * rights in the Software, and any and all patent rights owned or freely + * licensable by each licensor hereunder covering either (i) the unmodified + * Software as contributed to or provided by such licensor, or (ii) the Larger + * Works (as defined below), to deal in both + * + * (a) the Software, and + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software (each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * The above copyright notice and either this complete permission notice or at + * a minimum a reference to the UPL must be included in all copies or + * substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * #L% + */ +package com.redhat.rhjmc.containerjfr.rules; + +import java.util.Map; +import java.util.function.Function; + +import org.apache.commons.codec.binary.Base64; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.junit.jupiter.api.Test; + +import com.redhat.rhjmc.containerjfr.core.net.Credentials; +import com.redhat.rhjmc.containerjfr.net.web.http.AbstractAuthenticatedRequestHandler; +import io.vertx.core.MultiMap; + +class RulesHeadersFactoryTest { + + Function factory = RulesModule.provideRulesHeadersFactory(); + + @Test + void testNoHeaderAddedIfCredentialsAreNull() { + MultiMap result = factory.apply(null); + MatcherAssert.assertThat(result, Matchers.emptyIterable()); + } + + @Test + void ensureCorrectHeaderWhenCredentialsProvided() { + Credentials credentials = new Credentials("foouser", "barpassword"); + MultiMap result = factory.apply(credentials); + MatcherAssert.assertThat(result.size(), Matchers.is(1)); + Map.Entry header = result.entries().get(0); + MatcherAssert.assertThat( + header.getKey(), + Matchers.equalTo(AbstractAuthenticatedRequestHandler.JMX_AUTHORIZATION_HEADER)); + MatcherAssert.assertThat( + header.getValue(), + Matchers.equalTo( + "Basic " + + Base64.encodeBase64String( + (credentials.getUsername() + + ":" + + credentials.getPassword()) + .getBytes()))); + } +}