From 2fb769969abb8ebbf1ff9002bbc9add3ab4dc568 Mon Sep 17 00:00:00 2001 From: Corneil du Plessis Date: Thu, 7 Nov 2024 13:28:20 +0200 Subject: [PATCH] Updated DropAuthorizationHeaderOnContainerTestManual with documentation on using for a manual test. Updated ContainerImageRestTemplateFactory with documentation on extra and improved formatting. Refactored CacheKey in ContainerImageRestTemplateFactory to be a Java Record and include extra to ensure correct behaviour for RestTemplate obtained. --- ...horizationHeaderOnContainerTestManual.java | 34 +++-- .../ContainerImageRestTemplateFactory.java | 136 ++++++++---------- 2 files changed, 82 insertions(+), 88 deletions(-) diff --git a/spring-cloud-dataflow-configuration-metadata/src/test/java/org/springframework/cloud/dataflow/container/registry/authorization/DropAuthorizationHeaderOnContainerTestManual.java b/spring-cloud-dataflow-configuration-metadata/src/test/java/org/springframework/cloud/dataflow/container/registry/authorization/DropAuthorizationHeaderOnContainerTestManual.java index ee1d49bb56..13e6aa32a5 100644 --- a/spring-cloud-dataflow-configuration-metadata/src/test/java/org/springframework/cloud/dataflow/container/registry/authorization/DropAuthorizationHeaderOnContainerTestManual.java +++ b/spring-cloud-dataflow-configuration-metadata/src/test/java/org/springframework/cloud/dataflow/container/registry/authorization/DropAuthorizationHeaderOnContainerTestManual.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2020 the original author or authors. + * Copyright 2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,11 +36,26 @@ import static org.assertj.core.api.Assertions.assertThat; /** - * @author Adam J. Weigold + * This test is aimed at performing a manual test against a deployed container registry; + * In order to invoke this test populate the fields of DropAuthorizationHeaderOnContainerTestManual.TestApplication + * named registryDomainName, registryUser, registrySecret and imageNameAndTag + * + * The image should be one built with spring-boot:build-image or paketo so that is has a label named 'org.springframework.boot.version' + * For docker hub use: + * registryDomainName="registry-1.docker.io", + * registryUser="docker user" + * registrySecret="docker access token" + * imageNameAndTag="springcloudstream/s3-sink-rabbit:5.0.0" + * * @author Corneil du Plessis */ public class DropAuthorizationHeaderOnContainerTestManual { + private static final String registryDomainName = "registry-1.docker.io"; + private static final String registryUser = ""; + private static final String registrySecret = ""; + private static final String imageNameAndTag = "springcloudstream/s3-sink-rabbit:5.0.0"; + private AnnotationConfigApplicationContext context; @AfterEach @@ -54,27 +69,26 @@ void clean() { @Test void testContainerImageLabels() { context = new AnnotationConfigApplicationContext(TestApplication.class); - ContainerRegistryProperties registryProperties = context.getBean(ContainerRegistryProperties.class); DefaultContainerImageMetadataResolver imageMetadataResolver = context.getBean(DefaultContainerImageMetadataResolver.class); - String imageNameAndTag = "springcloudstream/s3-sink-rabbit:5.0.0"; - Map imageLabels = imageMetadataResolver.getImageLabels(registryProperties.getDefaultRegistryHost() + "/" + imageNameAndTag); + Map imageLabels = imageMetadataResolver.getImageLabels(registryDomainName + "/" + imageNameAndTag); System.out.println("imageLabels:" + imageLabels.keySet()); - assertThat(imageLabels).containsKey("org.springframework.boot.spring-configuration-metadata.json"); + assertThat(imageLabels).containsKey("org.springframework.boot.version"); } @Import({ContainerRegistryAutoConfiguration.class, ApplicationConfigurationMetadataResolverAutoConfiguration.class}) @AutoConfigureWebClient static class TestApplication { + @Bean @Primary ContainerRegistryProperties containerRegistryProperties() { ContainerRegistryProperties properties = new ContainerRegistryProperties(); ContainerRegistryConfiguration registryConfiguration = new ContainerRegistryConfiguration(); - registryConfiguration.setRegistryHost("registry-1.docker.io"); + registryConfiguration.setRegistryHost(registryDomainName); registryConfiguration.setAuthorizationType(ContainerRegistryConfiguration.AuthorizationType.dockeroauth2); - registryConfiguration.setUser(""); - registryConfiguration.setSecret(""); - properties.setRegistryConfigurations(Collections.singletonMap("registry-1.docker.io", registryConfiguration)); + registryConfiguration.setUser(registryUser); + registryConfiguration.setSecret(registrySecret); + properties.setRegistryConfigurations(Collections.singletonMap(registryDomainName, registryConfiguration)); return properties; } diff --git a/spring-cloud-dataflow-container-registry/src/main/java/org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java b/spring-cloud-dataflow-container-registry/src/main/java/org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java index f62b4ec4de..a4c8c21813 100644 --- a/spring-cloud-dataflow-container-registry/src/main/java/org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java +++ b/spring-cloud-dataflow-container-registry/src/main/java/org/springframework/cloud/dataflow/container/registry/ContainerImageRestTemplateFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 the original author or authors. + * Copyright 2021-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,9 +18,11 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.Map; -import java.util.Objects; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; import javax.net.ssl.SSLException; @@ -41,19 +43,18 @@ import org.springframework.web.client.RestTemplate; /** - * On demand creates a cacheable {@link RestTemplate} instances for the purpose of the - * Container Registry access. Created RestTemplates can be configured to use Http Proxy - * and/or bypassing the SSL verification. + * On demand creates a cacheable {@link RestTemplate} instances for the purpose of the Container Registry access. + * Created RestTemplates can be configured to use Http Proxy and/or bypassing the SSL verification. * - * For configuring a Http Proxy in need to: 1. Add http proxy configuration using the - * spring.cloud.dataflow.container.httpProxy.* properties. 2. For every - * {@link ContainerRegistryConfiguration} that has to interact via the http proxy set the - * use-http-proxy flag to true. For example: - * spring.cloud.dataflow.container.registry-configurations[reg-name].use-http-proxy=ture + * For configuring a Http Proxy in need to: + * 1. Add http proxy configuration using the spring.cloud.dataflow.container.httpProxy.* properties. + * 2. For every {@link ContainerRegistryConfiguration} that has to interact via the http proxy set the use-http-proxy + * flag to true. For example: + * spring.cloud.dataflow.container.registry-configurations[reg-name].use-http-proxy=ture * - * Following example configures the default (e.g. DockerHub) registry to use the HTTP - * Proxy (my-proxy.test:8080) while the dockerhub-mirror and the private-snapshots - * registry configurations allow direct communication: + * Following example configures the default (e.g. DockerHub) registry to use the HTTP Proxy (my-proxy.test:8080) + * while the dockerhub-mirror and the private-snapshots registry configurations allow direct communication: + * * spring: * cloud: * dataflow: @@ -74,6 +75,7 @@ * * @author Christian Tzolov * @author Cheng Guan Poh + * @author Corneil du Plessis */ public class ContainerImageRestTemplateFactory { @@ -88,41 +90,16 @@ public class ContainerImageRestTemplateFactory { private final ContainerRegistryProperties properties; /** - * Depends on the disablesSslVerification and useHttpProxy a 4 different RestTemplate configurations might be + * Depends on the skipSslVerification and withHttpProxy and extra map with multiple configurations might be * used at the same time for interacting with different container registries. - * The cache map allows reusing the RestTemplates for given useHttpProxy and disablesSslVerification combination. + * The cache map allows reusing the RestTemplates for given withHttpProxy and skipSslVerification and extra map combination. */ private final ConcurrentHashMap restTemplateCache; /** - * Unique key for any useHttpProxy and disablesSslVerification combination. + * Unique key for any withHttpProxy and skipSslVerification combination. */ - private static class CacheKey { - private final boolean disablesSslVerification; - private final boolean useHttpProxy; - - public CacheKey(boolean disablesSslVerification, boolean useHttpProxy) { - this.disablesSslVerification = disablesSslVerification; - this.useHttpProxy = useHttpProxy; - } - - static CacheKey of(boolean disablesSslVerification, boolean useHttpProxy) { - return new CacheKey(disablesSslVerification, useHttpProxy); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - CacheKey cacheKey = (CacheKey) o; - return disablesSslVerification == cacheKey.disablesSslVerification && useHttpProxy == cacheKey.useHttpProxy; - } - - @Override - public int hashCode() { - return Objects.hash(disablesSslVerification, useHttpProxy); - } - } + record CacheKey(boolean skipSslVerification, boolean withHttpProxy, Map extra) {}; public ContainerImageRestTemplateFactory(RestTemplateBuilder restTemplateBuilder, ContainerRegistryProperties properties) { this.restTemplateBuilder = restTemplateBuilder; @@ -130,23 +107,30 @@ public ContainerImageRestTemplateFactory(RestTemplateBuilder restTemplateBuilder this.restTemplateCache = new ConcurrentHashMap(); } + /** + * Obtain a configured RestTemplate for interacting with container registry. + * @param skipSslVerification indicates we want to trust all certificates. + * @param withHttpProxy indicates we want to use configured proxy. + * @return A configured RestTemplate with the given ssl and proxy settings. + */ public RestTemplate getContainerRestTemplate(boolean skipSslVerification, boolean withHttpProxy) { return this.getContainerRestTemplate(skipSslVerification, withHttpProxy, Collections.emptyMap()); } + /** + * Obtain a configured RestTemplate for interacting with container registry. + * @param skipSslVerification indicates that we want to trust all certificates. + * @param withHttpProxy indicates we want to use the configure proxy host and port. + * @param extra by adding entry custom-registry=registry-domain we expect to remove Authorization headers. + * @return A configured RestTemplate with the given ssl and proxy and extra settings. + */ public RestTemplate getContainerRestTemplate(boolean skipSslVerification, boolean withHttpProxy, Map extra) { + var cacheKey = new CacheKey(skipSslVerification, withHttpProxy, new HashMap<>(extra)); try { - CacheKey cacheKey = CacheKey.of(skipSslVerification, withHttpProxy); - if (!this.restTemplateCache.containsKey(cacheKey)) { - RestTemplate restTemplate = createContainerRestTemplate(skipSslVerification, withHttpProxy, extra); - this.restTemplateCache.putIfAbsent(cacheKey, restTemplate); - } - return this.restTemplateCache.get(cacheKey); + return this.restTemplateCache.computeIfAbsent(cacheKey, (key) -> createContainerRestTemplate(key.skipSslVerification(), key.withHttpProxy(), key.extra())); } catch (Exception e) { - throw new ContainerRegistryException( - "Failed to create Container Image RestTemplate for disableSsl:" - + skipSslVerification + ", httpProxy:" + withHttpProxy, e); + throw new ContainerRegistryException("Failed to create Container Image RestTemplate for disableSsl:" + skipSslVerification + ", httpProxy:" + withHttpProxy, e); } } @@ -179,12 +163,6 @@ private RestTemplate createContainerRestTemplate(boolean skipSslVerification, bo * * Custom: * Custom Container Registry may have same type of issues as S3 so header needs to be dropped as well. - * - * @author Adam J. Weigold - * @author Janne Valkealahti - * @author Christian Tzolov - * @author Cheng Guan Poh - * @author Corneil du Plessis */ private HttpClient httpClientBuilder(boolean skipSslVerification, Map extra) { @@ -211,29 +189,27 @@ private HttpClient httpClientBuilder(boolean skipSslVerification, Map extra) { HttpMethod method = request.method(); - if(method.equals(HttpMethod.GET) || method.equals(HttpMethod.HEAD)) { - if (request.uri().contains(AMZ_CREDENTIAL)) { - return true; - } - if (request.uri().contains(AZURECR_URI_SUFFIX)) { - return request.requestHeaders() - .entries() - .stream() - .anyMatch(entry -> entry.getKey().equalsIgnoreCase(AUTHORIZATION_HEADER) - && entry.getValue().contains(BASIC_AUTH)); - } - return extra.containsKey(CUSTOM_REGISTRY) && request.uri().contains(extra.get(CUSTOM_REGISTRY)); + if(!method.equals(HttpMethod.GET) && !method.equals(HttpMethod.HEAD)) { + return false; } - return false; + if (request.uri().contains(AMZ_CREDENTIAL)) { + return true; + } + if (request.uri().contains(AZURECR_URI_SUFFIX)) { + return request.requestHeaders() + .entries() + .stream() + .anyMatch(entry -> entry.getKey().equalsIgnoreCase(AUTHORIZATION_HEADER) + && entry.getValue().contains(BASIC_AUTH)); + } + return extra.containsKey(CUSTOM_REGISTRY) && request.uri().contains(extra.get(CUSTOM_REGISTRY)); } private static void removeAuthorization(HttpHeaders headers) { - for(Map.Entry entry: headers.entries()) { - if(entry.getKey().equalsIgnoreCase(org.springframework.http.HttpHeaders.AUTHORIZATION)) { - headers.remove(entry.getKey()); - break; - } - } + Set authHeaders = headers.entries() + .stream() + .filter(entry -> entry.getKey().equalsIgnoreCase(AUTHORIZATION_HEADER)).map(Map.Entry::getKey).collect(Collectors.toSet()); + authHeaders.forEach(authHeader -> headers.remove(authHeader)); } private RestTemplate initRestTemplate(HttpClient client, boolean withHttpProxy, Map extra) { @@ -243,10 +219,13 @@ private RestTemplate initRestTemplate(HttpClient client, boolean withHttpProxy, if (!properties.getHttpProxy().isEnabled()) { throw new ContainerRegistryException("Registry Configuration uses a HttpProxy but non is configured!"); } - ProxyProvider.Builder builder = ProxyProvider.builder().type(ProxyProvider.Proxy.HTTP).host(properties.getHttpProxy().getHost()).port(properties.getHttpProxy().getPort()); + ProxyProvider.Builder builder = ProxyProvider.builder() + .type(ProxyProvider.Proxy.HTTP) + .host(properties.getHttpProxy().getHost()) + .port(properties.getHttpProxy().getPort()); client.proxy(typeSpec -> builder.build()); } - // TODO what do we do with extra? + ClientHttpRequestFactory customRequestFactory = new ReactorNettyClientRequestFactory(client); // DockerHub response's media-type is application/octet-stream although the content is in JSON. @@ -255,6 +234,7 @@ private RestTemplate initRestTemplate(HttpClient client, boolean withHttpProxy, // include application/octet-stream and text/plain. MappingJackson2HttpMessageConverter octetSupportJsonConverter = new MappingJackson2HttpMessageConverter(); ArrayList mediaTypeList = new ArrayList(octetSupportJsonConverter.getSupportedMediaTypes()); + mediaTypeList.add(MediaType.APPLICATION_JSON); mediaTypeList.add(MediaType.APPLICATION_OCTET_STREAM); mediaTypeList.add(MediaType.TEXT_PLAIN); octetSupportJsonConverter.setSupportedMediaTypes(mediaTypeList);