-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Changes from 4 commits
b641be1
8c8c92f
7c6f5c4
52b84ab
ba1f671
0c79c66
aa383fc
5232fb8
eb21d81
86207ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
/* | ||
* 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.Collections; | ||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
import java.util.stream.Stream; | ||
|
||
public class RegistryAliasGroup { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be a set? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 List<String> getAliasesGroup(String registry) { | ||
for (ImmutableList<String> aliasGroup : REGISTRY_ALIAS_GROUPS) { | ||
if (aliasGroup.contains(registry)) { | ||
// Found a group. Move the requested "registry" to the front before returning it. | ||
Stream<String> self = Stream.of(registry); | ||
Stream<String> withoutSelf = aliasGroup.stream().filter(alias -> !registry.equals(alias)); | ||
return Stream.concat(self, withoutSelf).collect(Collectors.toList()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps this could even return the stream itself. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just tried returning a stream, but it does not seem concise at the caller side. We can switch to Stream later if it seems fit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. For later, the caller could use something like:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, probably even do |
||
} | ||
} | ||
|
||
return Collections.singletonList(registry); | ||
} | ||
} |
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 java.util.List; | ||
import org.junit.Assert; | ||
import org.junit.Test; | ||
|
||
public class RegistryAliasGroupTest { | ||
|
||
@Test | ||
public void testGetAliasesGroup_noKnownAliases() { | ||
List<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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I think in this codebase, we prefer to use There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
---|---|---|
@@ -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" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file. | |
|
||
- `jib:buildTar` goal to build an image tarball at `target/jib-image.tar`, which can be loaded into docker using `docker load` ([#514](https://github.com/GoogleContainerTools/jib/issues/514)) | ||
|
||
- For Docker Hub, also tries registry aliases when getting a credential from `docker.config` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: probably just call it "the Docker config" - There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1, also adding this to |
||
|
||
### Changed | ||
|
||
### Fixed | ||
|
There was a problem hiding this comment.
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.