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

Add check for image name length to not exceed 255 #2555

Closed
wants to merge 20 commits into from
Closed

Add check for image name length to not exceed 255 #2555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f52dfa3
Checks image name length
Devashishbasu Jan 15, 2024
f4d8573
uncommented test
Devashishbasu Jan 15, 2024
bceee52
Update ImageNameDistributionReferenceTest.java
Devashishbasu Jan 15, 2024
f4a0977
Update ImageName.java
Devashishbasu Jan 17, 2024
1593395
Update ImageName.java
Devashishbasu Jan 17, 2024
298e579
Update ImageName.java
Devashishbasu Jan 18, 2024
e91f186
Update ImageName.java
Devashishbasu Jan 18, 2024
325723f
Update ImageNameDistributionReferenceTest.java
Devashishbasu Jan 18, 2024
ad8f7a5
updating changes
Devashishbasu Jan 18, 2024
21ca153
Update ImageNameDistributionReferenceTest.java
Devashishbasu Jan 18, 2024
9e76a49
Update ImageName.java
Devashishbasu Jan 18, 2024
6d6aa4a
Update ImageNameDistributionReferenceTest.java
Devashishbasu Jan 18, 2024
d733469
Update ImageName.java
Devashishbasu Jan 18, 2024
7249365
updating changes
Devashishbasu Jan 18, 2024
d07beb7
Update ImageNameTest.java
Devashishbasu Jan 19, 2024
1b17182
Update ImageNameTest.java
Devashishbasu Jan 22, 2024
b28639c
Update ImageNameTest.java
Devashishbasu Jan 22, 2024
ccfb414
Update ImageNameTest.java
Devashishbasu Jan 24, 2024
30c2ab6
Update ImageName.java
Devashishbasu Jan 26, 2024
95a7bbc
Update ImageName.java
Devashishbasu Jan 29, 2024
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
9 changes: 9 additions & 0 deletions jkube-kit/config/image/src/main/java/org/eclipse/jkube/kit/config/image/ImageName.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public class ImageName {
// Digest
private String digest;

private static final int REPO_NAME_MAX_LENGTH = 255;
/**
* Create an image name
*
Expand Down Expand Up @@ -272,13 +273,16 @@ private void doValidate() {
// Strip off user from repository name
String user = inferUser();
String image = user != null ? repository.substring(user.length() + 1) : repository;

Object[] checks = new Object[] {
"registry", DOMAIN_REGEXP, registry,
"image", IMAGE_NAME_REGEXP, image,
"user", NAME_COMP_REGEXP, user,
"tag", TAG_REGEXP, tag,
"digest", DIGEST_REGEXP, digest

};

for (int i = 0; i < checks.length; i +=3) {
String value = (String) checks[i + 2];
Pattern checkPattern = (Pattern) checks[i + 1];
Expand All @@ -288,6 +292,11 @@ private void doValidate() {
checks[i], value, checkPattern.pattern()));
}
}

if (repository.length() > REPO_NAME_MAX_LENGTH) {
errors.add(String.format("Repository name must not be more than %d characters", REPO_NAME_MAX_LENGTH));
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add one more test in ImageNameTest for verifying exception with this exact message is thrown when too long image name is provided?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please add one more test in ImageNameTest for verifying exception with this exact message is thrown when too long image name is provided?

Ya sure

}

if (!errors.isEmpty()) {
StringBuilder buf = new StringBuilder();
buf.append(String.format("Given Docker name '%s' is invalid:%n", getFullName()));
Expand Down
2 changes: 1 addition & 1 deletion .../src/test/java/org/eclipse/jkube/kit/config/image/ImageNameDistributionReferenceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ void validNames(String name) {
"",
":justtag",
"@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
//"a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a:tag", // https://github.com/eclipse/jkube/issues/2542
"a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a:tag", // https://github.com/eclipse/jkube/issues/2542
//"repo@sha256:ffffffffffffffffffffffffffffffffff", // https://github.com/eclipse/jkube/issues/2543
"validname@invaliddigest:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"Uppercase:tag",
Expand Down
19 changes: 18 additions & 1 deletion jkube-kit/config/image/src/test/java/org/eclipse/jkube/kit/config/image/ImageNameTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* Red Hat, Inc. - initial API and implementation
*/
package org.eclipse.jkube.kit.config.image;

import org.apache.commons.lang3.StringUtils;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
Expand Down Expand Up @@ -124,6 +124,23 @@ private void verifyData(Object[] data) {
void testIllegalFormat() {
assertThrows(IllegalArgumentException.class, () -> new ImageName(""));
}
@Test
void shouldThrowExceptionOnTooLongImageName() {
// New test for too long repository name
String tooLongName = generateTooLongImageName();
assertThatIllegalArgumentException()
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I meant to add a new test; not modify existing test. Could you please move this code block into a separate test?

@Test 
void shouldThrowExceptionOnTooLongImageName() {

}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I meant to add a new test; not modify existing test. Could you please move this code block into a separate test?

@Test 
void shouldThrowExceptionOnTooLongImageName() {

}

Hii @rohanKanojia did all the changes you told me do, but getting a build failure

[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 03:38 min
[INFO] Finished at: 2024-01-22T14:44:48Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.1.2:test (default-test) on project jkube-kit-remote-dev: There are test failures.
[ERROR]
[ERROR] Please refer to /home/jenkins/agent/workspace/Build_Java8_PR-2555/jkube-kit/remote-dev/target/surefire-reports for the individual test results.
[ERROR] Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
[ERROR] -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.1.2:test (default-test) on project jkube-kit-remote-dev: There are test failures.

is it due to my changes? or can you suggest how can i fix this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I meant to add a new test; not modify existing test. Could you please move this code block into a separate test?

@Test 
void shouldThrowExceptionOnTooLongImageName() {

}

Hii @rohanKanojia did all the changes you told me do, but getting a build failure

[INFO] BUILD FAILURE [INFO] ------------------------------------------------------------------------ [INFO] Total time: 03:38 min [INFO] Finished at: 2024-01-22T14:44:48Z [INFO] ------------------------------------------------------------------------ [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.1.2:test (default-test) on project jkube-kit-remote-dev: There are test failures. [ERROR] [ERROR] Please refer to /home/jenkins/agent/workspace/Build_Java8_PR-2555/jkube-kit/remote-dev/target/surefire-reports for the individual test results. [ERROR] Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream. [ERROR] -> [Help 1] org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.1.2:test (default-test) on project jkube-kit-remote-dev: There are test failures.

is it due to my changes? or can you suggest how can i fix this?

test added by me is passing

Copy link
Member

Choose a reason for hiding this comment

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

@Devashishbasu : This failure seems unrelated to your changes. Probably it's a flaky test that needs to be fixed. Could you please try pushing again and see if you see the issue again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Devashishbasu : This failure seems unrelated to your changes. Probably it's a flaky test that needs to be fixed. Could you please try pushing again and see if you see the issue again?

Hi @rohanKanojia i've pushed the code again and seeing the same issue again...it is the same issue which you have opened #2576

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Devashishbasu : This failure seems unrelated to your changes. Probably it's a flaky test that needs to be fixed. Could you please try pushing again and see if you see the issue again?

Hii @rohanKanojia ...still the same issue persists

.as("Too long image name should fail")
.isThrownBy(() -> new ImageName(tooLongName))
.withMessageContaining("Repository name must not be more than 255 characters");

}

private String generateTooLongImageName() {
char repeatedChar = 'a';
int maxLength = 255 + 1;
String tooLongName = StringUtils.repeat(repeatedChar, maxLength);
return tooLongName;
}

@Test
void namesUsedByDockerTests() {
Expand Down
Loading