fix(docker): fix retrofit2 docker client which is replacing ? with %3F causing api failure

clouddriver-docker/clouddriver-docker.gradle

@@ -17,6 +17,7 @@ dependencies {
   implementation "io.spinnaker.fiat:fiat-core:$fiatVersion"
   implementation "io.spinnaker.kork:kork-credentials"
   implementation "io.spinnaker.kork:kork-retrofit"
+  implementation "io.spinnaker.kork:kork-retrofit2"
   implementation "io.spinnaker.kork:kork-exceptions"
   implementation "io.spinnaker.kork:kork-web"
 
@@ -36,6 +37,6 @@ dependencies {
   testImplementation "org.springframework.security:spring-security-test"
   testImplementation "com.github.tomakehurst:wiremock-jre8"
   testImplementation "io.spinnaker.kork:kork-core"
-  implementation "io.spinnaker.kork:kork-retrofit2"
-  implementation "io.spinnaker.kork:kork-web"
+  testImplementation "com.github.tomakehurst:wiremock-jre8-standalone"
+
 }

clouddriver-docker/src/test/java/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClientTest.java

@@ -0,0 +1,141 @@
+/*
+ * Copyright 2025 OpsMx, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.netflix.spinnaker.clouddriver.docker.registry.api.v2.client;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static org.junit.jupiter.api.Assertions.assertIterableEquals;
+import static org.mockito.ArgumentMatchers.anyString;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
+import com.netflix.spinnaker.clouddriver.docker.registry.api.v2.auth.DockerBearerToken;
+import com.netflix.spinnaker.clouddriver.docker.registry.api.v2.auth.DockerBearerTokenService;
+import com.netflix.spinnaker.config.DefaultServiceClientProvider;
+import com.netflix.spinnaker.config.okhttp3.DefaultOkHttpClientBuilderProvider;
+import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider;
+import com.netflix.spinnaker.kork.client.ServiceClientProvider;
+import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory;
+import com.netflix.spinnaker.kork.retrofit.Retrofit2ServiceFactory;
+import com.netflix.spinnaker.kork.retrofit.Retrofit2ServiceFactoryAutoConfiguration;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
+import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties;
+import java.util.Arrays;
+import java.util.Map;
+import okhttp3.OkHttpClient;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.mockito.Mockito;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.mock.mockito.MockBean;
+import org.springframework.http.HttpStatus;
+import retrofit2.Retrofit;
+import retrofit2.converter.jackson.JacksonConverterFactory;
+
+@SpringBootTest(
+    classes = {
+      OkHttpClientConfigurationProperties.class,
+      Retrofit2ServiceFactory.class,
+      ServiceClientProvider.class,
+      OkHttpClientProvider.class,
+      OkHttpClient.class,
+      DefaultServiceClientProvider.class,
+      DefaultOkHttpClientBuilderProvider.class,
+      Retrofit2ServiceFactoryAutoConfiguration.class,
+      ObjectMapper.class
+    },
+    webEnvironment = SpringBootTest.WebEnvironment.NONE)
+public class DockerRegistryClientTest {
+
+  @RegisterExtension
+  static WireMockExtension wmDockerRegistry =
+      WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build();
+
+  static DockerRegistryClient.DockerRegistryService dockerRegistryService;
+  @MockBean DockerBearerTokenService dockerBearerTokenService;
+  static DockerRegistryClient dockerRegistryClient;
+  @Autowired ServiceClientProvider serviceClientProvider;
+  ObjectMapper objectMapper = new ObjectMapper();
+  Map<String, Object> tagsResponse;
+  String tagsResponseString;
+  String nextLink = "</v2/library/nginx/tags/list?last=1-alpine-slim&n=5>; rel=\"next\"";
+
+  @BeforeEach
+  public void init() throws JsonProcessingException {
+    tagsResponse =
+        Map.of(
+            "name",
+            "library/nginx",
+            "tags",
+            new String[] {"1", "1-alpine", "1-alpine-otel", "1-alpine-perl", "1-alpine-slim"});
+    tagsResponseString = objectMapper.writeValueAsString(tagsResponse);
+
+    DockerBearerToken bearerToken = new DockerBearerToken();
+    bearerToken.setToken("someToken");
+    bearerToken.setAccess_token("someToken");
+    Mockito.when(dockerBearerTokenService.getToken(anyString())).thenReturn(bearerToken);
+    dockerRegistryService =
+        buildService(DockerRegistryClient.DockerRegistryService.class, wmDockerRegistry.baseUrl());
+    dockerRegistryClient =
+        new DockerRegistryClient(
+            wmDockerRegistry.baseUrl(), 5, "", "", dockerRegistryService, dockerBearerTokenService);
+  }
+
+  private static <T> T buildService(Class<T> type, String baseUrl) {
+    return new Retrofit.Builder()
+        .baseUrl(baseUrl)
+        .client(new OkHttpClient())
+        .addCallAdapterFactory(ErrorHandlingExecutorCallAdapterFactory.getInstance())
+        .addConverterFactory(JacksonConverterFactory.create())
+        .build()
+        .create(type);
+  }
+
+  @Test
+  public void getTagsWithoutNextLink() {
+    wmDockerRegistry.stubFor(
+        WireMock.get(urlMatching("/v2/library/nginx/tags/list"))
+            .willReturn(
+                aResponse().withStatus(HttpStatus.OK.value()).withBody(tagsResponseString)));
+
+    DockerRegistryTags dockerRegistryTags = dockerRegistryClient.getTags("library/nginx");
+    String[] tags = (String[]) tagsResponse.get("tags");
+    assertIterableEquals(Arrays.asList(tags), dockerRegistryTags.getTags());
+  }
+
+  @Test
+  public void getTagsWithNextLink() {
+    wmDockerRegistry.stubFor(
+        WireMock.get(urlMatching("/v2/library/nginx/tags/list"))
+            .willReturn(
+                aResponse()
+                    .withStatus(HttpStatus.OK.value())
+                    .withHeader("link", nextLink)
+                    .withBody(tagsResponseString)));
+    // TODO: Fix the below error occurring due to retrofit2 replacing `?` with `%3F`
+    Assertions.assertThrows(
+        SpinnakerHttpException.class,
+        () -> dockerRegistryClient.getTags("library/nginx"),
+        "Status: 404, Method: GET, URL: http://<baseUrl>/v2/library/nginx/tags/list%3Flast=1-alpine-slim&n=5, Message: Not Found");
+  }
+}

clouddriver-docker/src/test/java/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryServiceTest.java

@@ -0,0 +1,112 @@
+/*
+ * Copyright 2025 OpsMx, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.netflix.spinnaker.clouddriver.docker.registry.api.v2.client;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import com.netflix.spinnaker.clouddriver.docker.registry.api.v2.auth.DockerBearerToken;
+import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory;
+import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall;
+import java.io.IOException;
+import java.time.Instant;
+import okhttp3.OkHttpClient;
+import okhttp3.ResponseBody;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import retrofit2.Call;
+import retrofit2.Retrofit;
+import retrofit2.converter.jackson.JacksonConverterFactory;
+import retrofit2.http.GET;
+import retrofit2.http.Header;
+import retrofit2.http.Headers;
+import retrofit2.http.Path;
+import retrofit2.http.Query;
+
+public class DockerRegistryServiceTest {
+
+  static DockerRegistryClient.DockerRegistryService dockerRegistryService;
+  static TokenService tokenService;
+  String service = "registry.docker.io";
+  String scope = "repository:library/nginx:pull";
+  String repository = "library/nginx";
+  String tagsPath = "v2/library/nginx/tags/list";
+  private Instant expiryTime = Instant.now();
+  static String bearerToken;
+
+  @BeforeAll
+  public static void setup() {
+    tokenService = buildService(TokenService.class, "https://auth.docker.io");
+    dockerRegistryService =
+        buildService(
+            DockerRegistryClient.DockerRegistryService.class, "https://registry-1.docker.io");
+  }
+
+  @Test
+  void getToken() {
+    DockerBearerToken token =
+        Retrofit2SyncCall.execute(tokenService.getToken("token", service, scope, "spinnaker"));
+    assertNotNull(token.getAccess_token());
+  }
+
+  @Test
+  void getTagsWithToken() throws IOException {
+    try (ResponseBody response =
+        Retrofit2SyncCall.execute(
+            dockerRegistryService.getTags(repository, getBearerToken(), "Spinnaker"))) {
+      assertNotNull(response.string());
+    }
+  }
+
+  @Test
+  void getTagsWithPathSupplied() throws IOException {
+    try (ResponseBody response =
+        Retrofit2SyncCall.execute(
+            dockerRegistryService.get(tagsPath, getBearerToken(), "Spinnaker"))) {
+      assertNotNull(response.string());
+    }
+  }
+
+  private String getBearerToken() {
+    if (bearerToken == null || Instant.now().isAfter(expiryTime)) {
+      DockerBearerToken token =
+          Retrofit2SyncCall.execute(tokenService.getToken("token", service, scope, "spinnaker"));
+      bearerToken = "Bearer " + token.getAccess_token();
+      this.expiryTime = Instant.now().plusSeconds(4 * 60); // 4 minutes
+    }
+    return bearerToken;
+  }
+
+  private static <T> T buildService(Class<T> type, String baseUrl) {
+    return new Retrofit.Builder()
+        .baseUrl(baseUrl)
+        .client(new OkHttpClient())
+        .addCallAdapterFactory(ErrorHandlingExecutorCallAdapterFactory.getInstance())
+        .addConverterFactory(JacksonConverterFactory.create())
+        .build()
+        .create(type);
+  }
+
+  private interface TokenService {
+    @GET("/{path}")
+    @Headers({"Docker-Distribution-API-Version: registry/2.0"})
+    Call<DockerBearerToken> getToken(
+        @Path(value = "path", encoded = true) String path,
+        @Query(value = "service") String service,
+        @Query(value = "scope") String scope,
+        @Header("User-Agent") String agent);
+  }
+}