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

YARN-11364. Docker Container to accept docker Image name with sha256 digest #5092

Merged
merged 2 commits into from Nov 1, 2022
Merged

YARN-11364. Docker Container to accept docker Image name with sha256 digest #5092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a780f0
YARN-11364. Docker Container to accept docker Image name with sha256
Oct 29, 2022
8908375
Updated regex and test cases with negative cases
Nov 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 13 additions & 3 deletions ...p/yarn/server/nodemanager/containermanager/linux/runtime/DockerLinuxContainerRuntime.java
View file
Open in desktop
Expand Up @@ -208,6 +208,8 @@ public class DockerLinuxContainerRuntime extends OCIContainerRuntime {
private static final Pattern dockerImagePattern =
Pattern.compile(DOCKER_IMAGE_PATTERN);

private static final Pattern DOCKER_DIGEST_PATTERN = Pattern.compile("^sha256:[a-z0-9]{12,64}$");

private static final String DEFAULT_PROCFS = "/proc";

@InterfaceAudience.Private
Expand Down Expand Up @@ -1201,9 +1203,17 @@ public static void validateImageName(String imageName)
throw new ContainerExecutionException(
ENV_DOCKER_CONTAINER_IMAGE + " not set!");
}
if (!dockerImagePattern.matcher(imageName).matches()) {
throw new ContainerExecutionException("Image name '" + imageName
+ "' doesn't match docker image name pattern");
// check if digest is part of imageName, extract and validate it.
String digest = null;
if (imageName.contains("@sha256")) {
String[] digestParts = imageName.split("@");
digest = digestParts[1];
imageName = digestParts[0];
}
if (!dockerImagePattern.matcher(imageName).matches() || (digest != null
&& !DOCKER_DIGEST_PATTERN.matcher(digest).matches())) {
throw new ContainerExecutionException(
"Image name '" + imageName + "' doesn't match docker image name pattern");
}
}

Expand Down
34 changes: 21 additions & 13 deletions ...op/yarn/server/nodemanager/containermanager/linux/runtime/TestDockerContainerRuntime.java
View file
Open in desktop
Expand Up @@ -2033,19 +2033,27 @@ public static Configuration enableMockContainerExecutor(Configuration conf) {

@Test
public void testDockerImageNamePattern() throws Exception {
String[] validNames =
{ "ubuntu", "fedora/httpd:version1.0",
"fedora/httpd:version1.0.test",
"fedora/httpd:version1.0.TEST",
"myregistryhost:5000/ubuntu",
"myregistryhost:5000/fedora/httpd:version1.0",
"myregistryhost:5000/fedora/httpd:version1.0.test",
"myregistryhost:5000/fedora/httpd:version1.0.TEST"};

String[] invalidNames = { "Ubuntu", "ubuntu || fedora", "ubuntu#",
"myregistryhost:50AB0/ubuntu", "myregistry#host:50AB0/ubuntu",
":8080/ubuntu"
};
String[] validNames = {"ubuntu", "fedora/httpd:version1.0", "fedora/httpd:version1.0.test",
"fedora/httpd:version1.0.TEST", "myregistryhost:5000/ubuntu",
"myregistryhost:5000/fedora/httpd:version1.0",
"myregistryhost:5000/fedora/httpd:version1.0.test",
"myregistryhost:5000/fedora/httpd:version1.0.TEST",
"123456789123.dkr.ecr.us-east-1.amazonaws.com/emr-docker-examples:pyspark-example"
+ "@sha256:f1d4ae3f7261a72e98c6ebefe9985cf10a0ea5bd762585a43e0700ed99863807"};

String[] invalidNames = {"Ubuntu", "ubuntu || fedora", "ubuntu#", "myregistryhost:50AB0/ubuntu",
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest also adding new negative tests here to make sure DOCKER_DIGEST_PATTERN doesn't match inputs that we don't want it to match. For example:

// Invalid: contains "@sha256" but doesn't really contain a digest.
"123456789123.dkr.ecr.us-east-1.amazonaws.com/emr-docker-examples:pyspark-example@sha256"

// Invalid: digest is too short.
"123456789123.dkr.ecr.us-east-1.amazonaws.com/emr-docker-examples:pyspark-example@sha256:f1d4"

// Invalid: digest is too long (depending on if we take my code review feedback on the regex).
"123456789123.dkr.ecr.us-east-1.amazonaws.com/emr-docker-examples:pyspark-example@sha256:f1d4ae3f7261a72e98c6ebefe9985cf10a0ea5bd762585a43e0700ed99863807f"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, will add these negative test cases.

"myregistry#host:50AB0/ubuntu", ":8080/ubuntu",

// Invalid: contains "@sha256" but doesn't really contain a digest.
"123456789123.dkr.ecr.us-east-1.amazonaws.com/emr-docker-examples:pyspark-example@sha256",

// Invalid: digest is too short.
"123456789123.dkr.ecr.us-east-1.amazonaws.com/emr-docker-examples:pyspark-example"
+ "@sha256:f1d4",

// Invalid: digest is too long
"123456789123.dkr.ecr.us-east-1.amazonaws.com/emr-docker-examples:pyspark-example"
+ "@sha256:f1d4ae3f7261a72e98c6ebefe9985cf10a0ea5bd762585a43e0700ed99863807f"};

for (String name : validNames) {
DockerLinuxContainerRuntime.validateImageName(name);
Expand Down