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.

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

Ensure that ParsedDockerfile supports platform args #2780

Merged
merged 3 commits into from May 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -21,7 +21,7 @@
@Slf4j
public class ParsedDockerfile {

private static final Pattern FROM_LINE_PATTERN = Pattern.compile("FROM ([^\\s]+).*");
private static final Pattern FROM_LINE_PATTERN = Pattern.compile("FROM (?<arg>--[^\\s]+\\s)*(?<image>[^\\s]+).*", Pattern.CASE_INSENSITIVE);

private final Path dockerFilePath;

Expand Down Expand Up @@ -57,7 +57,7 @@ private void parse(List<String> lines) {
dependencyImageNames = lines.stream()
.map(FROM_LINE_PATTERN::matcher)
.filter(Matcher::matches)
.map(matcher -> matcher.group(1))
.map(matcher -> matcher.group("image"))
.collect(Collectors.toSet());

if (!dependencyImageNames.isEmpty()) {
Expand Down
Expand Up @@ -16,6 +16,12 @@ public void doesSimpleParsing() {
assertEquals("extracts a single image name", Sets.newHashSet("someimage"), parsedDockerfile.getDependencyImageNames());
}

@Test
public void isCaseInsensitive() {
final ParsedDockerfile parsedDockerfile = new ParsedDockerfile(asList("from someimage", "RUN something"));
assertEquals("extracts a single image name", Sets.newHashSet("someimage"), parsedDockerfile.getDependencyImageNames());
}

@Test
public void handlesTags() {
final ParsedDockerfile parsedDockerfile = new ParsedDockerfile(asList("FROM someimage:tag", "RUN something"));
Expand All @@ -40,6 +46,18 @@ public void ignoringBuildStageNames() {
assertEquals("ignores build stage names and allows multiple images to be extracted", Sets.newHashSet("someimage", "nextimage"), parsedDockerfile.getDependencyImageNames());
}

@Test
public void ignoringPlatformArgs() {
final ParsedDockerfile parsedDockerfile = new ParsedDockerfile(asList("FROM --platform=linux/amd64 someimage", "RUN something"));
Copy link
Member

Choose a reason for hiding this comment

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

since the parsing seem to support multiple flags, perhaps we should add this (2 or more flags) case too

Copy link
Member Author

Choose a reason for hiding this comment

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

Platform seems to be the only arg that is supported by docker at present, but sure, seems like a good idea.

assertEquals("ignores platform args", Sets.newHashSet("someimage"), parsedDockerfile.getDependencyImageNames());
}

@Test
public void ignoringExtraPlatformArgs() {
final ParsedDockerfile parsedDockerfile = new ParsedDockerfile(asList("FROM --platform=linux/amd64 --somethingelse=value someimage", "RUN something"));
assertEquals("ignores platform args", Sets.newHashSet("someimage"), parsedDockerfile.getDependencyImageNames());
}

@Test
public void handlesGracefullyIfNoFromLine() {
final ParsedDockerfile parsedDockerfile = new ParsedDockerfile(asList("RUN something", "# is this even a valid Dockerfile?"));
Expand Down