You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/10/31 15:46:38 UTC
[GitHub] [hadoop] cnauroth commented on a diff in pull request #5092: YARN-11364. Docker Container to accept docker Image name with sha256 digest
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org