Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check other potential alises if Docker Hub #605

Merged
merged 10 commits into from
Jul 17, 2018
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright 2018 Google LLC. All rights reserved.
*
* 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.google.cloud.tools.jib.registry;

import com.google.common.collect.ImmutableList;
import java.util.ArrayDeque;

public class RegistryAliasGroup {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistent style, we include a javadoc for each class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistent style, we include a javadoc for each class


private static final ImmutableList<ImmutableList<String>> REGISTRY_ALIAS_GROUPS =
ImmutableList.of(
// Docker Hub alias group
ImmutableList.of("registry.hub.docker.com", "index.docker.io"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a set?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, no need to be a list.


/**
* Returns the list of registry aliases for the given {@code registry}, including {@code registry}
* as the first element.
*
* @param registry the registry for which the alias group is requested
* @return non-empty list of registries where {@code registry} is the first element
*/
public static ImmutableList<String> getAliasesGroup(String registry) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should return List to minimize exposing Guava classes in APIs. Right?

Copy link
Member Author

@chanseokoh chanseokoh Jul 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I didn't intend for this class to be part of the public API, but I guess there is no way to keep it private to the API while the class is declared public. I'll change it to List.

for (ImmutableList<String> aliasGroup : REGISTRY_ALIAS_GROUPS) {
if (aliasGroup.contains(registry)) {
// Found a group. Move the requested "registry" to the front before returning it.
ArrayDeque<String> requestedRegistryAtHead = new ArrayDeque<>(aliasGroup);
requestedRegistryAtHead.remove(registry);
requestedRegistryAtHead.addFirst(registry);
return ImmutableList.copyOf(requestedRegistryAtHead);
}
}

return ImmutableList.of(registry);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.cloud.tools.jib.http.Authorization;
import com.google.cloud.tools.jib.http.Authorizations;
import com.google.cloud.tools.jib.json.JsonTemplateMapper;
import com.google.cloud.tools.jib.registry.RegistryAliasGroup;
import com.google.cloud.tools.jib.registry.credentials.json.DockerConfigTemplate;
import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
Expand Down Expand Up @@ -86,6 +87,17 @@ public Authorization retrieve() throws IOException {
return null;
}

for (String registry : RegistryAliasGroup.getAliasesGroup(registry)) {
Authorization authorization = retrieve(dockerConfigTemplate, registry);
if (authorization != null) {
return authorization;
}
}
return null;
}

@Nullable
private Authorization retrieve(DockerConfigTemplate dockerConfigTemplate, String registry) {
// First, tries to find defined auth.
String auth = dockerConfigTemplate.getAuthFor(registry);
if (auth != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright 2018 Google LLC. All rights reserved.
*
* 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.google.cloud.tools.jib.registry;

import com.google.common.collect.ImmutableList;
import org.junit.Assert;
import org.junit.Test;

public class RegistryAliasGroupTest {

@Test
public void testGetAliasesGroup_noKnownAliases() {
ImmutableList<String> singleton = RegistryAliasGroup.getAliasesGroup("something.gcr.io");
Assert.assertEquals(1, singleton.size());
Assert.assertEquals("something.gcr.io", singleton.get(0));
}

@Test
public void testGetAliasesGroup_registryHubDockerCom() {
Assert.assertArrayEquals(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think in this codebase, we prefer to use List over raw arrays, such that this can be like Assert.assertEquals(theList, Arrays.asList(a, b, c).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is certainly better.

RegistryAliasGroup.getAliasesGroup("registry.hub.docker.com").toArray(new String[0]),
new String[] {"registry.hub.docker.com", "index.docker.io"});
}

@Test
public void testGetAliasesGroup_indexDockerIo() {
Assert.assertArrayEquals(
RegistryAliasGroup.getAliasesGroup("index.docker.io").toArray(new String[0]),
new String[] {"index.docker.io", "registry.hub.docker.com"});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,21 @@ public void testRetrieve_none() throws IOException {

Assert.assertNull(dockerConfigCredentialRetriever.retrieve());
}

@Test
public void testRetrieve_credentialFromAlias() throws IOException {
Mockito.when(mockDockerCredentialHelperFactory.withCredentialHelperSuffix(Mockito.anyString()))
.thenReturn(Mockito.mock(DockerCredentialHelper.class));
Mockito.when(
mockDockerCredentialHelperFactory.withCredentialHelperSuffix(
"index.docker.io credential helper"))
.thenReturn(mockDockerCredentialHelper);

DockerConfigCredentialRetriever dockerConfigCredentialRetriever =
new DockerConfigCredentialRetriever(
"registry.hub.docker.com", dockerConfigFile, mockDockerCredentialHelperFactory);

Authorization authorization = dockerConfigCredentialRetriever.retrieve();
Assert.assertEquals(mockAuthorization, authorization);
}
}
16 changes: 15 additions & 1 deletion jib-core/src/test/resources/json/dockerconfig.json
Original file line number Diff line number Diff line change
@@ -1 +1,15 @@
{"auths":{"some other registry":{"auth":"some other auth"},"some registry":{"auth":"some auth","password":"ignored"},"https://registry":{"auth":"token"},"just registry":{},"https://with.protocol":{}},"credsStore":"some credential store","credHelpers":{"another registry":"another credential helper","some registry":"some credential helper"}}
{
"auths":{
"some other registry":{"auth":"some other auth"},
"some registry":{"auth":"some auth","password":"ignored"},
"https://registry":{"auth":"token"},
"just registry":{},
"https://with.protocol":{}
},
"credsStore":"some credential store",
"credHelpers":{
"another registry":"another credential helper",
"some registry":"some credential helper",
"index.docker.io":"index.docker.io credential helper"
}
}