Skip to content

Commit

Permalink
Fix #1929 Fixed parsing image name
Browse files Browse the repository at this point in the history 
Signed-off-by: balbusm <balbusm@gmail.com>
  • Loading branch information
@balbusm @manusa
balbusm authored and manusa committed Jul 6, 2023
1 parent 03d63b9 commit 163f75f
Show file tree
Hide file tree
Showing 5 changed files with 199 additions and 76 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Usage:

### 1.13.0 (2023-06-14)
* Fix #1478: Should detect and warn the user if creating ingress and ingress controller not available
* Fix #1929: Docker Image Name parsing fix
* Fix #2092: Support for `Chart.yaml` fragments
* Fix #2150: Bump Kubernetes Client to 6.6.0 (fixes issues when trace-logging OpenShift builds)
* Fix #2162: Bump Kubernetes Client to 6.6.1 (HttpClient with support for PUT + InputStream)
Expand Down Expand Up @@ -494,3 +495,4 @@ Only the set of documented features are available to users.

### 0.1.0 (2019-12-19)
* Initial release

38 changes: 11 additions & 27 deletions jkube-kit/config/image/src/main/java/org/eclipse/jkube/kit/config/image/ImageName.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,10 @@ private String joinTail(String[] parts) {
return builder.toString();
}

private boolean containsPeriodOrColon(String part) {
return containsPeriod(part) || containsColon(part);
}

private boolean containsPeriod(String part) {
private boolean isRegistryPart(String part) {
return part.contains(".");
}

private boolean containsColon(String part) {
return part.contains(":");
}

/**
* Get the full name of this image, including the registry but without
* any tag (e.g. <code>privateregistry:fabric8io/java</code>)
Expand Down Expand Up @@ -281,26 +273,18 @@ private void parseComponentsBeforeTag(String rest) {
registry = null;
user = null;
repository = parts[0];
} else if (parts.length >= 2) {
if (containsPeriodOrColon(parts[0])) {
if (parts.length > 2) {
assignRegistryUserAndRepository(parts);
} else {
checkWhetherFirstElementIsUserOrRegistryAndAssign(parts);
}
} else if (parts.length == 2) {
if (isRegistryPart(parts[0])) {
assignRegistryAndRepository(parts);
} else {
registry = null;
user = parts[0];
repository = rest;
assignUserAndRepository(parts);
}
} else if (parts.length >= 3) {
if (isRegistryPart(parts[0])) {
assignRegistryUserAndRepository(parts);
} else {
assignUserAndRepository(parts);
}
}
}

private void checkWhetherFirstElementIsUserOrRegistryAndAssign(String[] parts) {
if (containsColon(parts[0])) {
assignRegistryAndRepository(parts);
} else {
assignUserAndRepository(parts);
}
}

Expand Down
203 changes: 162 additions & 41 deletions ...ig/image/src/test/java/org/eclipse/jkube/kit/config/image/ImageNameFullNameParseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,59 +13,180 @@
*/
package org.eclipse.jkube.kit.config.image;

import lombok.Builder;
import lombok.Value;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;

class ImageNameFullNameParseTest {

public static Stream<Object[]> data() {
public static Stream<Object[]> validData() {
return Stream.of(
new Object[][] {
{"Repo and Name", "eclipse/eclipse_jkube",
"eclipse/eclipse_jkube:latest", null, "eclipse", "eclipse/eclipse_jkube", "latest", null},
{"Repo and Name with special characters", "valid.name-with__separators/eclipse_jkube",
"valid.name-with__separators/eclipse_jkube:latest", null,
"valid.name-with__separators", "valid.name-with__separators/eclipse_jkube", "latest", null},
{"Repo, Name and Tag", "eclipse/eclipse_jkube:1.3.3.7",
"eclipse/eclipse_jkube:1.3.3.7", null, "eclipse", "eclipse/eclipse_jkube", "1.3.3.7", null},
{"Repo, Name and Tag with special characters", "valid.name-with__separators/eclipse_jkube:valid__tag-4.2",
"valid.name-with__separators/eclipse_jkube:valid__tag-4.2", null,
"valid.name-with__separators", "valid.name-with__separators/eclipse_jkube", "valid__tag-4.2", null},
{"Registry, Repo and Name", "docker.io/eclipse/eclipse_jkube",
"docker.io/eclipse/eclipse_jkube:latest", "docker.io", "eclipse", "eclipse/eclipse_jkube", "latest", null},
{"Registry, Repo and Name with special characters",
"long.registry.example.com:8080/valid.name--with__separators/eclipse_jkube",
"long.registry.example.com:8080/valid.name--with__separators/eclipse_jkube:latest",
"long.registry.example.com:8080",
"valid.name--with__separators", "valid.name--with__separators/eclipse_jkube", "latest", null},
{"Registry, Repo, Name and Tag", "docker.io/eclipse/eclipse_jkube:1.33.7",
"docker.io/eclipse/eclipse_jkube:1.33.7", "docker.io", "eclipse", "eclipse/eclipse_jkube", "1.33.7", null},
{"Registry, Repo, Name and Tag with special characters",
"long.registry.example.com:8080/valid.name--with__separators/eclipse_jkube:very--special__tag.2.A",
"long.registry.example.com:8080/valid.name--with__separators/eclipse_jkube:very--special__tag.2.A",
"long.registry.example.com:8080",
"valid.name--with__separators", "valid.name--with__separators/eclipse_jkube", "very--special__tag.2.A", null},
});
new Object[][] {
{
"Repo and Name", "eclipse/eclipse_jkube",
SimpleImageName.builder()
.fullName("eclipse/eclipse_jkube:latest")
.registry(null)
.user("eclipse")
.repository("eclipse/eclipse_jkube")
.tag("latest")
.digest(null)
.build()
},
{
"Repo, Name and Tag", "eclipse/eclipse_jkube:1.3.3.7",
SimpleImageName.builder()
.fullName("eclipse/eclipse_jkube:1.3.3.7")
.registry(null)
.user("eclipse")
.repository("eclipse/eclipse_jkube")
.tag("1.3.3.7")
.digest(null)
.build()
},
{
"Registry, Repo and Name", "docker.io/eclipse/eclipse_jkube",
SimpleImageName.builder()
.fullName("docker.io/eclipse/eclipse_jkube:latest")
.registry("docker.io")
.user("eclipse")
.repository("eclipse/eclipse_jkube")
.tag("latest")
.digest(null)
.build()
},
{ "Registry, Repo and Name with special characters",
"long.registry.example.com:8080/valid.name--with__separators/eclipse_jkube",
SimpleImageName.builder()
.fullName("long.registry.example.com:8080/valid.name--with__separators/eclipse_jkube:latest")
.registry("long.registry.example.com:8080")
.user("valid.name--with__separators")
.repository("valid.name--with__separators/eclipse_jkube")
.tag("latest")
.digest(null)
.build()
},
{ "Registry, Repo, Name and Tag", "docker.io/eclipse/eclipse_jkube:1.33.7",
SimpleImageName.builder()
.fullName("docker.io/eclipse/eclipse_jkube:1.33.7")
.registry("docker.io")
.user("eclipse")
.repository("eclipse/eclipse_jkube")
.tag("1.33.7")
.digest(null)
.build()
},
{
"Registry, Repo, Name and Tag with special characters",
"long.registry.example.com:8080/valid.name--with__separators/eclipse_jkube:very--special__tag.2.A",
SimpleImageName.builder()
.fullName(
"long.registry.example.com:8080/valid.name--with__separators/eclipse_jkube:very--special__tag.2.A")
.registry("long.registry.example.com:8080")
.user("valid.name--with__separators")
.repository("valid.name--with__separators/eclipse_jkube")
.tag("very--special__tag.2.A")
.digest(null)
.build()
},
{
"Repo only", "eclipse-temurin:11",
SimpleImageName.builder()
.fullName("eclipse-temurin:11")
.registry(null)
.user(null)
.repository("eclipse-temurin")
.tag("11")
.digest(null)
.build()
},
{
"User and repo", "user/my-repo_z:11",
SimpleImageName.builder()
.fullName("user/my-repo_z:11")
.registry(null)
.user("user")
.repository("user/my-repo_z")
.tag("11")
.digest(null)
.build()
},
{
"Registry and repo", "foo-bar-registry.jfrog.io/java:jre-17",
SimpleImageName.builder()
.fullName("foo-bar-registry.jfrog.io/java:jre-17")
.registry("foo-bar-registry.jfrog.io")
.user(null)
.repository("java")
.tag("jre-17")
.digest(null)
.build()
},
{
"JFrog repo", "user.jfrog.io/my-jkube/openjdk:jre-17",
SimpleImageName.builder()
.fullName("user.jfrog.io/my-jkube/openjdk:jre-17")
.registry("user.jfrog.io")
.user("my-jkube")
.repository("my-jkube/openjdk")
.tag("jre-17")
.digest(null)
.build()
},
});
}

@ParameterizedTest(name = "{0}")
@MethodSource("data")
void fullNameParse(String testName, String providedFullName, String expectedFullName, String registry, String user,
String repository, String tag, String digest) {
final ImageName result = new ImageName(providedFullName);
assertThat(result)
.extracting(
ImageName::getFullName,
ImageName::getRegistry,
ImageName::getUser,
ImageName::getRepository,
ImageName::getTag,
ImageName::getDigest
)
.containsExactly(expectedFullName, registry, user, repository, tag, digest);
@MethodSource("validData")
void shouldParseImageName(String testName, String providedImageName, SimpleImageName expectedImageName) {
ImageName imageName = new ImageName(providedImageName);
SimpleImageName currentImageName = SimpleImageName.toSimpleImageName(imageName);
assertThat(currentImageName).isEqualTo(expectedImageName);
}

public static Stream<Object[]> invalidData() {
return Stream.of(
new Object[][] {
{
"Repo and Name with special characters", "invalid.name-with__separators/eclipse_jkube"
},
{
"Repo, Name and Tag with special characters", "invalid.name-with__separators/eclipse_jkube:valid__tag-4.2"
},
});
}

@ParameterizedTest(name = "{0}")
@MethodSource("invalidData")
void shouldFailedParseImageName(String testName, String providedImageName) {
assertThatCode(() -> new ImageName(providedImageName))
.isInstanceOf(IllegalArgumentException.class);
}

@Builder
@Value
public static class SimpleImageName {
String fullName;
String registry;
String user;
String repository;
String tag;
String digest;

public static SimpleImageName toSimpleImageName(ImageName imageName) {
return new SimpleImageName(imageName.getFullName(),
imageName.getRegistry(),
imageName.getUser(),
imageName.getRepository(),
imageName.getTag(),
imageName.getDigest());
}

}
}
24 changes: 20 additions & 4 deletions jkube-kit/config/image/src/test/java/org/eclipse/jkube/kit/config/image/ImageNameTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,21 +154,37 @@ void namesUsedByDockerTests() {
}

@Test
void testImageNameWithUsernameHavingPeriods() {
void testImageNameWithUsernameHavingNumber() {
// Given
String name = "roman.gordill/customer-service-cache:latest";
String name = "romangordill10/customer-service-cache:latest";

// When
ImageName imageName = new ImageName(name);

// Then
assertThat(imageName).isNotNull();
assertThat(imageName.getUser()).isEqualTo("roman.gordill");
assertThat(imageName.getRepository()).isEqualTo("roman.gordill/customer-service-cache");
assertThat(imageName.getUser()).isEqualTo("romangordill10");
assertThat(imageName.getRepository()).isEqualTo("romangordill10/customer-service-cache");
assertThat(imageName.getTag()).isEqualTo("latest");
assertThat(imageName.getRegistry()).isNull();
}

@Test
void testImageNameWithRegistry() {
// Given
String name = "foo.com/customer-service-cache:latest";

// When
ImageName imageName = new ImageName(name);

// Then
assertThat(imageName).isNotNull();
assertThat(imageName.getUser()).isNull();
assertThat(imageName.getRepository()).isEqualTo("customer-service-cache");
assertThat(imageName.getTag()).isEqualTo("latest");
assertThat(imageName.getRegistry()).isEqualTo("foo.com");
}

@Test
void testImageNameWithNameContainingRegistryAndName() {
// Given
Expand Down
8 changes: 4 additions & 4 deletions ...richer/api/src/test/java/org/eclipse/jkube/kit/enricher/handler/ContainerHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,13 @@ void withImageNames_shouldReturnContainersWithConfiguredImages() {
}

@Test
@DisplayName("with user, image and tag with period in image user, should return configured image")
void withUserAndImageAndTagWithPeriodInImageUser_shouldReturnConfiguredImage() {
@DisplayName("with user, image and tag with dash in image user, should return configured image")
void withUserAndImageAndTagWithDashInImageUser_shouldReturnConfiguredImage() {
// Given
ContainerHandler containerHandler = createContainerHandler(project);
List<ImageConfiguration> imageConfigurations = new ArrayList<>();
imageConfigurations.add(ImageConfiguration.builder()
.name("roman.gordill/customer-service-cache:latest")
.name("roman-gordill/customer-service-cache:latest")
.registry("quay.io")
.build(BuildConfiguration.builder()
.from("quay.io/jkube/jkube-java:0.0.13")
Expand All @@ -222,7 +222,7 @@ void withUserAndImageAndTagWithPeriodInImageUser_shouldReturnConfiguredImage() {

// Then
assertThat(containers).singleElement()
.hasFieldOrPropertyWithValue("image", "quay.io/roman.gordill/customer-service-cache:latest");
.hasFieldOrPropertyWithValue("image", "quay.io/roman-gordill/customer-service-cache:latest");
}

@Test
Expand Down

0 comments on commit 163f75f

Please sign in to comment.