cnauroth commented on code in PR #5092:
URL: https://github.com/apache/hadoop/pull/5092#discussion_r1009574652
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/TestDockerContainerRuntime.java:
##########
@@ -2033,19 +2033,16 @@ 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",
Review Comment:
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"
```
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DockerLinuxContainerRuntime.java:
##########
@@ -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]{32,}$");
Review Comment:
This allows "sha256:" followed by anywhere from 32 to an infinite number of
characters. The maximum length of a SHA-256 is 64 characters. I believe Docker
allows referencing a shortened SHA-256 as small as 12 characters, though I'm
having trouble finding documentation to back that up.
Maybe we should go with either `{32,64}` or `{12,64}`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
