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.

Sign up for GitHub

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

Jump to bottom

[#572] Allow image url with more than 2 slashes to be parsed properly. #577

Merged
merged 1 commit into from
Sep 2, 2024
Merged

[#572] Allow image url with more than 2 slashes to be parsed properly. #577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion core/src/main/java/cz/xtf/core/image/Image.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cz.xtf.core.image;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import cz.xtf.core.config.XTFConfig;
Expand Down Expand Up @@ -70,7 +71,10 @@ public static Image from(String imageUrl) {
repoTag = slashTokens[2];
break;
default:
throw new IllegalArgumentException("image '" + imageUrl + "' should have one or two '/' characters");
registry = slashTokens[0];
user = slashTokens[1];
Copy link
Contributor

Choose a reason for hiding this comment

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

This is different to what dosu says :) My understanding is aligned with dosu implementation.

IIUC dosu implementation #572 (comment) will bring this result

registry = mcr.microsoft.com
user = mssql/rhel
repoTag = server:2022-CU13-rhel-9.1

whereas your implementation will bring

registry = mcr.microsoft.com
user = mssql
repoTag = rhel/server:2022-CU13-rhel-9.1

Copy link
Contributor

Choose a reason for hiding this comment

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

to me the latter makes more sense, though we are only bending image url (which can be almost anything) to what the class expects

Copy link
Contributor

@mchoma mchoma Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, you was faster than me wanted to write user = mssql makes more sense to me now while thinking about it.

Dosu just implemented what I requested in issue.

repoTag = String.join("/", Arrays.copyOfRange(slashTokens, 2, slashTokens.length));
break;
}

final String[] tokens = repoTag.split(":");
Expand Down
46 changes: 46 additions & 0 deletions core/src/test/java/cz/xtf/core/image/ImageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,50 @@ public void testNotResolvableImage() {
Assertions.assertThrows(UnknownImageException.class, () -> Image.resolve("blabla"),
"Expected UnknownImageException to be thrown for unknown image.");
}

@Test
public void testImageFromUrlOneToken() {
Image image = Image.from("nginx");
Assertions.assertTrue(image.getRegistry().isEmpty(), "Registry for image url: 'nginx' must be empty.");
Assertions.assertTrue(image.getUser().isEmpty(), "User for image url: 'nginx' must be empty.");
Assertions.assertEquals("nginx", image.getRepo(), "Wrong repository name.");
Assertions.assertTrue(image.getTag().isEmpty(), "Tag for image url: 'nginx' must be empty.");
}

@Test
public void testImageFromUrlTwoTokens() {
Image image = Image.from("user/nginx");
Assertions.assertTrue(image.getRegistry().isEmpty(), "Registry for image url: 'user/nginx' must be empty.");
Assertions.assertEquals("user", image.getUser(), "User for image url: 'user/nginx' is wrong.");
Assertions.assertEquals("nginx", image.getRepo(), "Wrong repository name.");
Assertions.assertTrue(image.getTag().isEmpty(), "Tag for image url: 'user/nginx' must be empty.");
}

@Test
public void testImageFromUrlThreeTokens() {
Image image = Image.from("quay.io/jaegertracing/all-in-one:1.56");
Assertions.assertEquals("quay.io", image.getRegistry(), "Wrong registry parsed from image url.");
Assertions.assertEquals("jaegertracing", image.getUser(), "Wrong user parsed from image url.");
Assertions.assertEquals("all-in-one", image.getRepo(), "Wrong repository name.");
Assertions.assertEquals("1.56", image.getTag(), "Wrong tag parsed from image url.");
}

@Test
public void testImageFromUrlFourTokens() {
Image image = Image.from("mcr.microsoft.com/mssql/rhel/server:2022-CU13-rhel-9.1");
Assertions.assertEquals("mcr.microsoft.com", image.getRegistry(), "Wrong registry parsed from image url.");
Assertions.assertEquals("mssql", image.getUser(), "Wrong user parsed from image url.");
Assertions.assertEquals("rhel/server", image.getRepo(), "Wrong repository name.");
Assertions.assertEquals("2022-CU13-rhel-9.1", image.getTag(), "Wrong tag parsed from image url.");
}

@Test
public void testImageFromUrlFourPlusTokens() {
Image image = Image.from("mycompany.registry.com/user/team-b/subteam-c/myservice/subservice/subsubservice:1.2.3");
Assertions.assertEquals("mycompany.registry.com", image.getRegistry(), "Wrong registry parsed from image url.");
Assertions.assertEquals("user", image.getUser(), "Wrong user parsed from image url.");
Assertions.assertEquals("team-b/subteam-c/myservice/subservice/subsubservice", image.getRepo(),
"Wrong repository name.");
Assertions.assertEquals("1.2.3", image.getTag(), "Wrong tag parsed from image url.");
}
}
Loading