Closed mchoma closed 3 weeks ago
@dosu what do you think?
To handle image strings like mcr.microsoft.com/mssql/rhel/server:2022-CU13-rhel-9.1
in Image.java
, you can modify the from
method to accommodate cases with more than two '/' characters. Here is the updated code:
public static Image from(String imageUrl) {
final String[] slashTokens = imageUrl.split("/");
final String repoTag;
final String registry;
final String user;
final String repo;
final String tag;
switch (slashTokens.length) {
case 1:
registry = "";
user = "";
repoTag = slashTokens[0];
break;
case 2:
registry = "";
user = slashTokens[0];
repoTag = slashTokens[1];
break;
case 3:
registry = slashTokens[0];
user = slashTokens[1];
repoTag = slashTokens[2];
break;
default:
// Handle cases with more than two '/' characters
registry = slashTokens[0];
user = String.join("/", Arrays.copyOfRange(slashTokens, 1, slashTokens.length - 1));
repoTag = slashTokens[slashTokens.length - 1];
break;
}
final String[] tokens = repoTag.split(":");
switch (tokens.length) {
case 1:
repo = tokens[0];
tag = "";
break;
case 2:
repo = tokens[0];
tag = tokens[1];
break;
default:
throw new IllegalArgumentException("repoTag '" + repoTag + "' should have zero or one ':' character");
}
return new Image(registry, user, repo, tag);
}
This modification ensures that if there are more than two '/' characters, the method correctly assigns the registry and user, and then extracts the repoTag
from the last segment. This approach is similar to the logic used in the ImageStreamBuilder
class from the xtf
project, which also handles image URLs with varying numbers of '/' characters [1][2].
To continue talking to Dosu, mention @dosu.
@dosu any objection with linked pull request? Do you think it address original issue? Are linked tests correctly testing change?
The linked pull request addresses the original issue by adding a check to ensure that image names do not contain more than two '/' characters. The specific change is:
String[] parts = imageUrl.split("/");
if (parts.length > 3) {
throw new IllegalArgumentException("image '" + imageUrl + "' should have one or two '/' characters");
}
This code ensures that only valid image URLs are processed by throwing an IllegalArgumentException
if the imageUrl
contains more than two '/' characters [1].
Regarding the tests, I couldn't find specific information about the unit tests covering the Image.from
method in the provided data. It would be prudent to review the tests to ensure they cover various scenarios, including edge cases like image URLs with more than two '/' characters.
To continue talking to Dosu, mention @dosu.
@dosu No that check is not implemented in https://github.com/xtf-cz/xtf/pull/577/
The pull request #577 does address the original issue by modifying the parsing logic to handle image URLs with more than two slashes. This change allows the image mcr.microsoft.com/mssql/rhel/server:2022-CU13-rhel-9.1
to be parsed correctly.
Regarding the tests, the pull request includes updates to ensure that the new parsing logic is covered. It would be beneficial to review these tests to confirm they adequately cover various scenarios, including edge cases with different numbers of slashes in the image URLs. If the tests are comprehensive and pass successfully, the changes should be good to go.
To continue talking to Dosu, mention @dosu.
XTF does not allow for me to use image like
mcr.microsoft.com/mssql/rhel/server:2022-CU13-rhel-9.1
We need
case 4+:
section here?https://github.com/xtf-cz/xtf/blob/1daa976b2e32c0bc9fcdfda1ae947d37836c15fa/core/src/main/java/cz/xtf/core/image/Image.java#L54-L71
So we will end up with something like this