From fc55be3f907760828deea53b2c8d23de3628acce Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Mon, 9 Oct 2023 23:03:42 +0530 Subject: [PATCH] fix (jkube-kit) : Don't pass Invalid port in host headers for Helm OCI push Passing host:port seems to work when host url contains a valid port. However, this becomes host:-1 for some host that doesn't contain port in URL string (like r.example.com). Add case for these kind of hosts so that we pass correct host in Host HTTP headers. Signed-off-by: Rohan Kumar --- CHANGELOG.md | 1 + .../resource/helm/oci/OCIRegistryClient.java | 2 +- .../helm/oci/OCIRegistryEndpoint.java | 10 ++++++++++ .../helm/oci/OCIRegistryEndpointTest.java | 20 +++++++++++++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34a1836d93..6120028399 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Usage: * Fix #2399: Helm no longer generates default function; broadens support for different value types * Fix #2400: Helm supports complex values in `values.yaml` fragments (such as annotations or arrays) * Fix #2414: OpenShift Gradle Plugin picks up `jkube.build.pushSecret` property +* Fix #2417: Don't pass Invalid port in host headers for Helm OCI push _**Note**_: - Container Images generated using jkube opinionated defaults no longer contain full timestamp in `org.label-schema.build-date` label. The label contains the build date in the format `yyyy-MM-dd`. diff --git a/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/oci/OCIRegistryClient.java b/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/oci/OCIRegistryClient.java index c6c3807267..65fdc5bbfe 100644 --- a/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/oci/OCIRegistryClient.java +++ b/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/oci/OCIRegistryClient.java @@ -125,7 +125,7 @@ private OCIManifestLayer uploadBlob(String uploadUrl, CountingInputStream blobSt private HttpRequest.Builder newRequest() { return httpClient.newHttpRequestBuilder() - .header("Host", ociRegistryEndpoint.getBaseUrl().getHost() + ":" + ociRegistryEndpoint.getBaseUrl().getPort()) + .header("Host", ociRegistryEndpoint.getOCIRegistryHost()) .header("User-Agent", USER_AGENT); } diff --git a/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/oci/OCIRegistryEndpoint.java b/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/oci/OCIRegistryEndpoint.java index 486b752607..19c05bd0a3 100644 --- a/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/oci/OCIRegistryEndpoint.java +++ b/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/oci/OCIRegistryEndpoint.java @@ -22,6 +22,8 @@ import java.util.HashMap; import java.util.Map; +import static org.apache.commons.lang3.StringUtils.EMPTY; + public class OCIRegistryEndpoint { private static final Map PROTOCOL_MAPPER = new HashMap<>(); @@ -53,6 +55,14 @@ public String getManifestUrl(Chart chart) { return String.format("%s/%s/manifests/%s", apiV2Url, chart.getName(), chart.getVersion()); } + public String getOCIRegistryHost() { + final StringBuilder hostBuilder = new StringBuilder(baseUrl.getHost()); + if (baseUrl.getPort() > 0 && baseUrl.getPort() != 80 && baseUrl.getPort() != 443) { + hostBuilder.append(":").append(baseUrl.getPort()); + } + return hostBuilder.toString(); + } + public URI getBaseUrl() { return baseUrl; } diff --git a/jkube-kit/resource/helm/src/test/java/org/eclipse/jkube/kit/resource/helm/oci/OCIRegistryEndpointTest.java b/jkube-kit/resource/helm/src/test/java/org/eclipse/jkube/kit/resource/helm/oci/OCIRegistryEndpointTest.java index d57506727a..18ac70e5c9 100644 --- a/jkube-kit/resource/helm/src/test/java/org/eclipse/jkube/kit/resource/helm/oci/OCIRegistryEndpointTest.java +++ b/jkube-kit/resource/helm/src/test/java/org/eclipse/jkube/kit/resource/helm/oci/OCIRegistryEndpointTest.java @@ -17,6 +17,8 @@ import org.eclipse.jkube.kit.resource.helm.HelmRepository; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import java.net.URI; @@ -81,4 +83,22 @@ void getBlobUrl_whenInvoked_shouldReturnBlobUrl() { assertThat(result) .isEqualTo("https://r.example.com/v2/myuser/test-chart/blobs/sha256:7ed393daf1ffc94803c08ffcbecb798fa58e786bebffbab02da5458f68d0ecb0"); } + + @ParameterizedTest(name = "getOCIRegistryHost with url {0} should return {1}") + @CsvSource({ + "http://localhost:5000/myuser,localhost:5000", + "https://r.example.com/myuser,r.example.com", + "https://r.example.com:443/myuser,r.example.com", + "http://r.example.com:80/myuser,r.example.com" + }) + void getOCIRegistryHost_whenBaseUrlContainsHostAndPort_thenReturnHostWithPort(String url, String expectedHost) { + // Given + this.registryEndpoint = new OCIRegistryEndpoint(HelmRepository.builder().url(url).build()); + + // When + String host = registryEndpoint.getOCIRegistryHost(); + + // Then + assertThat(host).isEqualTo(expectedHost); + } }