Skip to content

Commit f787ce1

Browse files
fix(docker): resolve the issue of double encoding contained in the ECR's next link header causing the Retrofit2 client to fail due to error code 405 and Message: Method Not Allowed.
1 parent c780f97 commit f787ce1

File tree

3 files changed

+14
-32
lines changed

3 files changed

+14
-32
lines changed

clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/api/v2/client/DockerRegistryClient.groovy

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ class DockerRegistryClient {
243243
@Headers([
244244
"Docker-Distribution-API-Version: registry/2.0"
245245
])
246-
Call<ResponseBody> getTags(@Path(value="repository", encoded=true) String repository, @Header("Authorization") String token, @Header("User-Agent") String agent, @QueryMap Map<String, String> queryParams)
246+
Call<ResponseBody> getTags(@Path(value="repository", encoded=true) String repository, @Header("Authorization") String token, @Header("User-Agent") String agent, @QueryMap(encoded=true) Map<String, String> queryParams)
247247

248248

249249
@GET("/v2/{name}/manifests/{reference}")
@@ -263,14 +263,14 @@ class DockerRegistryClient {
263263
@Headers([
264264
"Docker-Distribution-API-Version: registry/2.0"
265265
])
266-
Call<ResponseBody> getCatalog(@Header("Authorization") String token, @Header("User-Agent") String agent, @QueryMap Map<String, String> queryParams)
266+
Call<ResponseBody> getCatalog(@Header("Authorization") String token, @Header("User-Agent") String agent, @QueryMap(encoded=true) Map<String, String> queryParams)
267267

268268

269269
@GET("/{path}")
270270
@Headers([
271271
"Docker-Distribution-API-Version: registry/2.0"
272272
])
273-
Call<ResponseBody> get(@Path(value="path", encoded=true) String path, @Header("Authorization") String token, @Header("User-Agent") String agent, @QueryMap Map<String, String> queryParams)
273+
Call<ResponseBody> get(@Path(value="path", encoded=true) String path, @Header("Authorization") String token, @Header("User-Agent") String agent, @QueryMap(encoded=true) Map<String, String> queryParams)
274274

275275

276276
@GET("/v2/")
@@ -394,7 +394,6 @@ class DockerRegistryClient {
394394
}
395395

396396
def headerValues = caseInsensitiveHeaders["link"]
397-
headers.values("link")
398397

399398
// We are at the end of the pagination.
400399
if (!headerValues || headerValues.size() == 0) {
@@ -453,6 +452,7 @@ class DockerRegistryClient {
453452
}
454453

455454
public DockerRegistryTags getTags(String repository, String path = null, Map<String, String> queryParams = [:]) {
455+
queryParams.computeIfAbsent("n", { paginateSize.toString() })
456456
def response = request({
457457
path ? Retrofit2SyncCall.executeCall(registryService.get(path, tokenService.basicAuthHeader, userAgent, queryParams)) :
458458
Retrofit2SyncCall.executeCall(registryService.getTags(repository, tokenService.basicAuthHeader, userAgent, queryParams))

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

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@
2020
import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
2121
import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
2222
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
23+
import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
2324
import static org.junit.jupiter.api.Assertions.assertEquals;
2425
import static org.junit.jupiter.api.Assertions.assertIterableEquals;
25-
import static org.junit.jupiter.api.Assertions.assertThrows;
2626
import static org.mockito.ArgumentMatchers.anyString;
2727

2828
import com.fasterxml.jackson.core.JsonProcessingException;
@@ -32,7 +32,6 @@
3232
import com.netflix.spinnaker.clouddriver.docker.registry.api.v2.auth.DockerBearerToken;
3333
import com.netflix.spinnaker.clouddriver.docker.registry.api.v2.auth.DockerBearerTokenService;
3434
import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory;
35-
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
3635
import java.util.Arrays;
3736
import java.util.Map;
3837
import okhttp3.OkHttpClient;
@@ -116,7 +115,7 @@ private static <T> T buildService(Class<T> type, String baseUrl) {
116115
@Test
117116
public void getTagsWithoutNextLink() {
118117
wmDockerRegistry.stubFor(
119-
WireMock.get(urlMatching("/v2/library/nginx/tags/list"))
118+
WireMock.get(urlMatching("/v2/library/nginx/tags/list\\?n=5"))
120119
.willReturn(
121120
aResponse().withStatus(HttpStatus.OK.value()).withBody(tagsResponseString)));
122121

@@ -128,7 +127,7 @@ public void getTagsWithoutNextLink() {
128127
@Test
129128
public void getTagsWithNextLink() {
130129
wmDockerRegistry.stubFor(
131-
WireMock.get(urlMatching("/v2/library/nginx/tags/list"))
130+
WireMock.get(urlMatching("/v2/library/nginx/tags/list\\?n=5"))
132131
.willReturn(
133132
aResponse()
134133
.withStatus(HttpStatus.OK.value())
@@ -147,7 +146,7 @@ public void getTagsWithNextLink() {
147146
"</v2/library/nginx/tags/list1>; rel=\"next\"")
148147
.withBody(tagsSecondResponseString)));
149148
wmDockerRegistry.stubFor(
150-
WireMock.get(urlMatching("/v2/library/nginx/tags/list1"))
149+
WireMock.get(urlMatching("/v2/library/nginx/tags/list1\\?n=5"))
151150
.willReturn(
152151
aResponse().withStatus(HttpStatus.OK.value()).withBody(tagsThirdResponseString)));
153152

@@ -190,12 +189,8 @@ public void getTagsWithNextLinkEncryptedAndEncoded() {
190189
String tagsListEndPointMinusQueryParams = "/v2/library/nginx/tags/list";
191190
String expectedEncodedParam = "Md1Woj%2FNOhjepFq7kPAr%2FEw%2FYxfcJoH9N52%2Blo3qAQ%3D%3D";
192191

193-
// notice %252F, %253D which are double encoded characters of /(or %2F) and =(or %3D)
194-
String doubleEncodedParam =
195-
"Md1Woj%252FNOhjepFq7kPAr%252FEw%252FYxfcJoH9N52%252Blo3qAQ%253D%253D";
196-
197192
wmDockerRegistry.stubFor(
198-
WireMock.get(urlMatching(tagsListEndPointMinusQueryParams))
193+
WireMock.get(urlMatching(tagsListEndPointMinusQueryParams + "\\?n=5"))
199194
.willReturn(
200195
aResponse()
201196
.withStatus(HttpStatus.OK.value())
@@ -211,26 +206,13 @@ public void getTagsWithNextLinkEncryptedAndEncoded() {
211206
.willReturn(
212207
aResponse().withStatus(HttpStatus.OK.value()).withBody(tagsSecondResponseString)));
213208

214-
wmDockerRegistry.stubFor(
215-
WireMock.get(
216-
urlMatching(
217-
tagsListEndPointMinusQueryParams + "\\?last=" + doubleEncodedParam + "&n=5"))
218-
.willReturn(aResponse().withStatus(405).withBody("Method Not Allowed")));
219-
220-
// TODO: Fix this issue of retrofit2 encoding the already encoded query parameter
221-
assertThrows(
222-
SpinnakerHttpException.class,
223-
() -> dockerRegistryClient.getTags("library/nginx"),
224-
"Status: 405, Method: GET, Message: Method Not Allowed");
209+
DockerRegistryTags dockerRegistryTags = dockerRegistryClient.getTags("library/nginx");
210+
assertThat(dockerRegistryTags.getTags()).hasSize(10);
225211

226-
wmDockerRegistry.verify(1, getRequestedFor(urlMatching(tagsListEndPointMinusQueryParams)));
227212
wmDockerRegistry.verify(
228-
1,
229-
getRequestedFor(
230-
urlMatching(
231-
tagsListEndPointMinusQueryParams + "\\?last=" + doubleEncodedParam + "&n=5")));
213+
1, getRequestedFor(urlMatching(tagsListEndPointMinusQueryParams + "\\?n=5")));
232214
wmDockerRegistry.verify(
233-
0,
215+
1,
234216
getRequestedFor(
235217
urlMatching(
236218
tagsListEndPointMinusQueryParams + "\\?last=" + expectedEncodedParam + "&n=5")));

clouddriver-docker/src/test/java/com/netflix/spinnaker/clouddriver/docker/registry/security/DockerRegistryNamedAccountCredentialsTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ private static OkHttpClient mockDockerOkClient(
110110
.build();
111111
when(mockTagListCall.execute()).thenReturn(tagListResponse);
112112
when(okHttpClient.newCall(
113-
argThat(r -> r.url().toString().equals("https://gcr.io/v2/myrepo/tags/list"))))
113+
argThat(r -> r.url().toString().equals("https://gcr.io/v2/myrepo/tags/list?n=100"))))
114114
.thenReturn(mockTagListCall);
115115

116116
doAnswer(

0 commit comments

Comments
 (0)