From 72893a93d2ae67af6febced640d0cef62650d6c8 Mon Sep 17 00:00:00 2001 From: "pankaj.chugh" Date: Wed, 7 Dec 2022 01:54:15 +0530 Subject: [PATCH 1/6] Initial Commit to support https://github.com/salesforce/dockerfile-image-update/issues/236 --- .../dockerfileimageupdate/CommandLine.java | 2 +- .../model/ImageKeyValuePair.java | 198 ++++++++++++++ .../process/ForkableRepoValidator.java | 3 + .../utils/DockerfileGitHubUtil.java | 11 + .../model/ImageKeyValuePairTest.java | 242 ++++++++++++++++++ 5 files changed, 455 insertions(+), 1 deletion(-) create mode 100644 dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java create mode 100644 dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java index b98b521a..ea10bfdf 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java @@ -48,7 +48,7 @@ public static void main(String[] args) DockerfileGitHubUtil dockerfileGitHubUtil = initializeDockerfileGithubUtil(ns.get(Constants.GIT_API)); /* Execute given command. */ - ((ExecutableWithNamespace)runClass.newInstance()).execute(ns, dockerfileGitHubUtil); + ((ExecutableWithNamespace)runClass.getDeclaredConstructor().newInstance()).execute(ns, dockerfileGitHubUtil); } static ArgumentParser getArgumentParser() { diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java new file mode 100644 index 00000000..0db3269c --- /dev/null +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java @@ -0,0 +1,198 @@ +package com.salesforce.dockerfileimageupdate.model; + +import com.google.common.collect.ImmutableList; +import org.apache.commons.lang3.StringUtils; + +import java.util.Arrays; +import java.util.List; + +public class ImageKeyValuePair { + private static final String IMAGE = "image"; + private static final String INVALID_IMAGE_VALUE = "You have not provided a valid value for image key."; + /** + * The name of the base image + */ + private final String baseImageName; + /** + * The tag of the base image + */ + private final String tag; + /** + * As of writing, this could include {@code AS name} to run a multi-stage build + */ + private final List additionalParts; + /** + * Comment starting with # + */ + private final String comments; + + /** + * Accepts a FROM instruction line from a Dockerfile + * See {@code isImageKeyValuePair} to ensure you're passing a valid line in. + * + * @param imageKeyValuePair a FROM instruction line from a Dockerfile + */ + public ImageKeyValuePair(String imageKeyValuePair) { + if (!isImageKeyValuePair(imageKeyValuePair)) { + throw new IllegalArgumentException(INVALID_IMAGE_VALUE); + } + String lineWithoutComment = imageKeyValuePair; + int commentIndex = imageKeyValuePair.indexOf("#"); + if (commentIndex >= 0) { + comments = imageKeyValuePair.substring(commentIndex); + lineWithoutComment = imageKeyValuePair.substring(0, commentIndex); + } else { + comments = null; + } + // Remove "image:" from remaining string + String lineWithoutImageKey = lineWithoutComment.trim(). + replaceFirst(IMAGE, "").replaceFirst(":", ""). + trim(); + + String[] imageAndTag = lineWithoutImageKey.split(":"); + if (StringUtils.isNotEmpty(lineWithoutImageKey) && imageAndTag.length > 0) { + baseImageName = imageAndTag[0]; + if (imageAndTag.length > 1) { + tag = imageAndTag[1]; + } else { + tag = null; + } + additionalParts = ImmutableList.of(); + } else { + baseImageName = null; + tag = null; + additionalParts = ImmutableList.of(); + } + } + + /** + * Internal API to get a new ComposeImageValuePair from an existing object + * @param baseImageName baseImageName to add + * @param tag tag to add + * @param additionalParts additionalParts to add + * @param comments comments to add + */ + private ImageKeyValuePair(String baseImageName, String tag, List additionalParts, String comments) { + this.baseImageName = baseImageName; + this.tag = tag; + this.additionalParts = ImmutableList.copyOf(additionalParts); + this.comments = comments; + } + + /** + * Check if this {@code lineInFile} is a FROM instruction, + * it is referencing {@code imageName} as a base image, + * and the tag is not the same as {@code imageTag} (or there is no tag) + * @param lineInFile Line a code file + * @param imageName images name + * @param imageTag tag for imageName + * @return {@link Boolean} value isImageKeyValuePairWithThisImageAndOlderTag + */ + public static boolean isImageKeyValuePairWithThisImageAndOlderTag(String lineInFile, String imageName, String imageTag) { + if (ImageKeyValuePair.isImageKeyValuePair(lineInFile)) { + ImageKeyValuePair ImageKeyValuePair = new ImageKeyValuePair(lineInFile); + return ImageKeyValuePair.hasBaseImage(imageName) && ImageKeyValuePair.hasADifferentTag(imageTag); + } + return false; + } + + /** + * Get a new {@code ComposeImageValuePair} the same as this but with the {@code tag} set as {@code newTag} + * @param newTag the new image tag + * @return a new FROM with the new image tag + */ + public ImageKeyValuePair getImageKeyValuePairWithNewTag(String newTag) { + return new ImageKeyValuePair(baseImageName, newTag, additionalParts, comments); + } + + /** + * Determines whether the line is a FROM instruction line in a Dockerfile + * @param composeImageKeyValueLine a single line(key:value) from a Docker-compose.yaml + * @return the line is a FROM instruction line or not + */ + public static boolean isImageKeyValuePair(String composeImageKeyValueLine) { + if (StringUtils.isNotBlank(composeImageKeyValueLine)) { + return composeImageKeyValueLine.trim().startsWith(ImageKeyValuePair.IMAGE); + } + return false; + } + + /** + * @return a String representation of a FROM instruction line in Dockerfile. No new line at the end + */ + @Override + public String toString() { + StringBuilder stringBuilder = new StringBuilder(IMAGE); + stringBuilder.append(": "); + stringBuilder.append(baseImageName); + if (hasTag()) { + stringBuilder.append(String.format(":%s", tag.trim())); + } + for (String part : additionalParts) { + if (StringUtils.isNotBlank(part)) { + stringBuilder.append(String.format(" %s", part.trim())); + } + } + + if (hasComments()) { + stringBuilder.append(String.format(" %s", comments)); + } + + return stringBuilder.toString(); + } + + public String getBaseImageName() { + return baseImageName; + } + + /** + * Check to see if the {@code baseImageName} in this object is the {@code imageToFind} without + * the other details (e.g. registry) + * @param imageToFind the image name to search for + * @return is {@code baseImageName} the same as {@code imageToFind} without extra things like registry + */ + public boolean hasBaseImage(String imageToFind) { + return baseImageName != null && + imageToFind != null && + baseImageName.endsWith(imageToFind); + } + + /** + * @return whether the {@code ComposeImageValuePair} has a {@code tag} + */ + public boolean hasTag() { + return tag != null; + } + + /** + * Determines whether the {@code tag} and {@code expectedTag} are the same + * @param expectedTag the tag to compare against ComposeImageValuePair's {@code tag} + * @return {@code true} if the 2 tags are different + */ + public boolean hasADifferentTag(String expectedTag) { + if (tag == null && expectedTag == null) { + return false; + } + if (tag == null || expectedTag == null) { + return true; + } + return !tag.trim().equals(expectedTag.trim()); + } + + public String getTag() { + return tag; + } + + public List getAdditionalParts() { + return additionalParts; + } + + public boolean hasComments() { + return comments != null; + } + + public String getComments() { + return comments; + } + +} diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidator.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidator.java index e7f8655a..3618e55a 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidator.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidator.java @@ -2,6 +2,7 @@ import com.salesforce.dockerfileimageupdate.model.FromInstruction; import com.salesforce.dockerfileimageupdate.model.GitForkBranch; +import com.salesforce.dockerfileimageupdate.model.ImageKeyValuePair; import com.salesforce.dockerfileimageupdate.model.ShouldForkResult; import com.salesforce.dockerfileimageupdate.utils.DockerfileGitHubUtil; import org.kohsuke.github.GHContent; @@ -101,6 +102,8 @@ protected boolean hasNoChanges(GHContent content, GitForkBranch gitForkBranch) { String line; while ( (line = reader.readLine()) != null ) { if (FromInstruction.isFromInstructionWithThisImageAndOlderTag(line, + gitForkBranch.getImageName(), gitForkBranch.getImageTag()) && + ImageKeyValuePair.isImageKeyValuePairWithThisImageAndOlderTag(line, gitForkBranch.getImageName(), gitForkBranch.getImageTag())) { return false; } diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java index 8d4dbc5e..c6397666 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java @@ -93,7 +93,10 @@ public Optional>> findFilesWithImage( // Filename search appears to yield better / more results than language:Dockerfile // Root cause: linguist doesn't currently deal with prefixes of files: // https://github.com/github/linguist/issues/4566 + // This will work in OR mode i.e, either filename is Dockerfile or docker-compose search.filename("Dockerfile"); + search.filename("docker-compose"); + if (!orgsToIncludeOrExclude.isEmpty()) { StringBuilder includeOrExcludeOrgsQuery = new StringBuilder(); for (Map.Entry org : orgsToIncludeOrExclude.entrySet()){ @@ -334,6 +337,14 @@ protected boolean changeIfDockerfileBaseImageLine(String imageToFind, String tag modified = true; } outputLine = fromInstruction.toString(); + } else if (ImageKeyValuePair.isImageKeyValuePair(line)) { + ImageKeyValuePair imageKeyValuePair = new ImageKeyValuePair(line); + if (imageKeyValuePair.hasBaseImage(imageToFind) && + imageKeyValuePair.hasADifferentTag(tag)) { + imageKeyValuePair = imageKeyValuePair.getImageKeyValuePairWithNewTag(tag); + modified = true; + } + outputLine = imageKeyValuePair.toString(); } stringBuilder.append(outputLine).append("\n"); return modified; diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java new file mode 100644 index 00000000..f7162df5 --- /dev/null +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java @@ -0,0 +1,242 @@ +package com.salesforce.dockerfileimageupdate.model; + +import com.google.common.collect.ImmutableList; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import static org.testng.Assert.assertEquals; + +public class ImageKeyValuePairTest { + + @DataProvider + public Object[][] inputImageKeyValuePairData() { + return new Object[][]{ + {"image: dockerimage:3 # some comment", "image: dockerimage:3 # some comment"}, + {"image: dockerimage:3 AS test", "image: dockerimage:3 AS test"}, + {"image: dockerimage:3 \tAS \ttest # some comment", "image: dockerimage:3 \tAS \ttest # some comment"}, + {"image: dockerimage:3# some comment", "image: dockerimage:3 # some comment"}, + {" image: dockerimage ", "image: dockerimage"}, + {"\t image: \t dockerimage:4 \t #comment", "image: dockerimage:4 #comment"}, + {"image: dockerimage:4:4:4 #comment", "image: dockerimage:4 #comment"}, + {"image: dockerimage:4 #comment me # # ", "image: dockerimage:4 #comment me # # "} + }; + } + + @Test(dataProvider = "inputImageKeyValuePairData") + public void testStringResult(String fromInstruction, String expectedResult) { + assertEquals(new ImageKeyValuePair(fromInstruction).toString(), expectedResult); + } + + @DataProvider + public Object[][] isImageKeyValuePairData() { + return new Object[][]{ + {"image: dockerimage:3 # some comment", true}, + {"image: dockerimage:3# some comment", true}, + {"#image: dockerimage:3", false}, + {"# image: dockerimage:3", false}, + {" image: dockerimage ", true}, + {"RUN something", false}, + {"", false}, + {" ", false}, + {null, false}, + }; + } + + @Test(dataProvider = "isImageKeyValuePairData") + public void testLineToSplit(String input, boolean expectedResult) { + assertEquals(ImageKeyValuePair.isImageKeyValuePair(input), expectedResult); + } + + @DataProvider + public Object[][] baseImageNameData() { + return new Object[][] { + {"image: image:tag", "image"}, + {"image: image:tag\t", "image"}, + {" \t image: \t image:\t# comment", "image"}, + {"image: image:", "image"}, + {"image: image", "image"}, + {"image:", null}, + {"image: image:test # :comment", "image"} + }; + } + + @Test(dataProvider = "baseImageNameData") + public void testBaseImageNameParsedCorrectly(String input, String expectedResult) { + assertEquals(new ImageKeyValuePair(input).getBaseImageName(), expectedResult); + } + + @DataProvider + public Object[][] hasBaseImageData() { + return new Object[][] { + {"image: image", "image", true}, + {"image: registry.com/some/image", "image", true}, + {"image: image", null, false}, + {"image:", "something", false}, + {"image:", null, false} + }; + } + + @Test(dataProvider = "hasBaseImageData") + public void testHasBaseImage(String fromInstruction, String imageToFind, boolean expectedResult) { + assertEquals(new ImageKeyValuePair(fromInstruction).hasBaseImage(imageToFind), expectedResult); + } + + @DataProvider + public Object[][] tagData() { + return new Object[][] { + {"image: image:some-tag", "some-tag"}, + {"image: image:some-tag:with:weird:other:tags", "some-tag"}, + {"image: image", null}, + {"image: image:", null}, + {"image: image@some-digest", null}, + {"image: image# some comment", null}, + {"image: image:\tsome-tag # comment", "\tsome-tag"}, + {"image: image: some-tag # comment", " some-tag"} + }; + } + + @Test(dataProvider = "tagData") + public void testTagParsedCorrectly(String fromInstruction, String expectedResult) { + assertEquals(new ImageKeyValuePair(fromInstruction).getTag(), expectedResult); + } + + @DataProvider + public Object[][] hasTagData() { + return new Object[][] { + {"image: no tag", false}, + {"image: image", false}, + {"image: image:", false}, + {"image: image:tag#as builder", true} + }; + } + + @Test(dataProvider = "hasTagData") + public void testHasTag(String fromInstructions, boolean expectedResult) { + assertEquals(new ImageKeyValuePair(fromInstructions).hasTag(), expectedResult); + } + + @DataProvider + public Object[][] instructionWithNewTagData() { + return new Object[][] { + {"image: image:some-tag", "some-tag"}, + {"image: image:some-tag:with:weird:other:tags", "some-tag"}, + {"image: image", null}, + {"image: image:", null}, + {"image: image@some-digest", null}, + {"image: image# some comment", null}, + {"image: image:\tsome-tag # comment", null}, + {"image: image: some-tag # comment", null} + }; + } + + @Test(dataProvider = "instructionWithNewTagData") + public void testGetInstructionWithNewTag(String fromInstruction, String newTag) { + ImageKeyValuePair oldImageKeyValuePair = new ImageKeyValuePair(fromInstruction); + ImageKeyValuePair newImageKeyValuePair = oldImageKeyValuePair.getImageKeyValuePairWithNewTag(newTag); + assertEquals(newImageKeyValuePair.getBaseImageName(), oldImageKeyValuePair.getBaseImageName()); + assertEquals(newImageKeyValuePair.getComments(), oldImageKeyValuePair.getComments()); + assertEquals(newImageKeyValuePair.getAdditionalParts(), oldImageKeyValuePair.getAdditionalParts()); + assertEquals(newImageKeyValuePair.getTag(), newTag); + } + + @DataProvider + public Object[][] hasADifferentTagData() { + return new Object[][] { + {"image: image:tag", "another", true}, + {"image: image:tag", "tag", false}, + {"image: image:tag", "", true}, + {"image: image:tag", null, true}, + {"image: image", null, false}, + {"image: image:", null, false}, + {"image: image: # comment", null, false}, + {"image: image: # comment", "tag", true} + }; + } + + @Test(dataProvider = "hasADifferentTagData") + public void testHasADifferentTag(String fromInstruction, String tagToCheck, boolean expectedResult) { + assertEquals(new ImageKeyValuePair(fromInstruction).hasADifferentTag(tagToCheck), expectedResult); + } + +/* @DataProvider + public Object[][] additionalPartsData() { + return new Object[][] { + {"image: image:tag as builder", ImmutableList.of("as", "builder")}, + {"image: image:tag as builder of things", ImmutableList.of("as", "builder", "of", "things")}, + {"image: image:tag#as builder", ImmutableList.of()}, + {"image: image:tag\t# comment", ImmutableList.of()}, + {"image: image:tag \t# comment", ImmutableList.of()}, + {"image: image:tag some\tother \t thing \t# comment", ImmutableList.of("some", "other", "thing")}, + {"image: image:\t# comment # # # ", ImmutableList.of()}, + {"image: image:", ImmutableList.of()}, + {"image:", ImmutableList.of()} + }; + } + + @Test(dataProvider = "additionalPartsData") + public void testAdditionalPartsParsedCorrectly(String input, ImmutableList expectedResult) { + assertEquals(new ImageKeyValuePair(input).getAdditionalParts(), expectedResult); + }*/ + + @DataProvider + public Object[][] commentData() { + return new Object[][] { + {"image: image:tag as builder", null}, + {"image: image:tag#as builder", "#as builder"}, + {"image: image:tag # comment", "# comment"}, + {"image: image:tag\t# comment", "# comment"}, + {"image: image:\t# comment # # # ", "# comment # # # "}, + {"image: image:", null}, + {"image: image:test # :comment", "# :comment"} + }; + } + + @Test(dataProvider = "commentData") + public void testCommentsParsedCorrectly(String input, String expectedResult) { + assertEquals(new ImageKeyValuePair(input).getComments(), expectedResult); + } + + @DataProvider + public Object[][] hasCommentsData() { + return new Object[][] { + {"image: no comment", false}, + {"image: image:tag#as builder", true} + }; + } + + @Test(dataProvider = "hasCommentsData") + public void testHasComments(String fromInstructions, boolean expectedResult) { + assertEquals(new ImageKeyValuePair(fromInstructions).hasComments(), expectedResult); + } + + @DataProvider + public Object[][] invalidData() { + return new Object[][]{ + {""}, + {"RUN something"}, + {"# image: someimage"}, + {":tag # comment"}, + {null} + }; + } + + @Test(dataProvider = "invalidData", expectedExceptions = IllegalArgumentException.class) + public void testInvalidFromData(String input) { + new ImageKeyValuePair(input); + } + + @DataProvider + public Object[][] isImageKeyValuePairWithThisImageAndOlderTagData() { + return new Object[][]{ + {"image: someimage", "someimage", "sometag", true}, + {"image: someimage:sometag", "someimage", "sometag", false}, + {"not a valid image key-value pair", "someimage", "sometag", false}, + {"image: someimage:oldtag", "someimage", "sometag", true} + }; + } + + @Test(dataProvider = "isImageKeyValuePairWithThisImageAndOlderTagData") + public void isImageKeyValuePairWithThisImageAndOlderTag(String line, String imageName, String imageTag, boolean expectedResult) { + assertEquals(ImageKeyValuePair.isImageKeyValuePairWithThisImageAndOlderTag(line, imageName, imageTag), expectedResult); + } +} \ No newline at end of file From a8e548f8714761aa272bbc87d9e808fb001fda95 Mon Sep 17 00:00:00 2001 From: "pankaj.chugh" Date: Fri, 6 Jan 2023 16:30:18 +0530 Subject: [PATCH 2/6] Updated to read files types from command lines Additionally updated to ignore sending PR for latest tag in docker-compose. Also ignore changing invalid tags which include variables like ${ITEST}/${version} Updated mockito-all to mockito-core as prior is deprecated. --- dockerfile-image-update/pom.xml | 4 +- .../dockerfileimageupdate/CommandLine.java | 5 +++ .../model/ImageKeyValuePair.java | 45 +++++++++---------- .../process/ForkableRepoValidator.java | 2 +- .../subcommands/impl/All.java | 3 +- .../subcommands/impl/Parent.java | 4 +- .../utils/Constants.java | 2 +- .../utils/DockerfileGitHubUtil.java | 29 ++++++++---- .../model/ImageKeyValuePairTest.java | 36 +++------------ .../process/ForkableRepoValidatorTest.java | 4 +- .../process/GitHubPullRequestSenderTest.java | 1 - .../repository/GitHubTest.java | 5 ++- .../storage/GitHubJsonStoreTest.java | 5 ++- .../subcommands/impl/AllTest.java | 10 ++--- .../subcommands/impl/ChildTest.java | 1 - .../subcommands/impl/ParentTest.java | 7 ++- .../utils/DockerfileGitHubUtilTest.java | 30 +++++++------ .../utils/GitHubUtilTest.java | 1 - 18 files changed, 94 insertions(+), 100 deletions(-) diff --git a/dockerfile-image-update/pom.xml b/dockerfile-image-update/pom.xml index d00d215b..1d2ca806 100644 --- a/dockerfile-image-update/pom.xml +++ b/dockerfile-image-update/pom.xml @@ -97,8 +97,8 @@ org.mockito - mockito-all - 1.10.19 + mockito-core + 4.11.0 test diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java index ea10bfdf..5bfe7f5f 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java @@ -84,6 +84,11 @@ static ArgumentParser getArgumentParser() { parser.addArgument("-x") .help("comment snippet mentioned in line just before FROM instruction for ignoring a child image. " + "Defaults to 'no-dfiu'"); + parser.addArgument("-t", "--" + Constants.FILENAMES_TO_SEARCH) + .type(String.class) + .setDefault("Dockerfile,docker-compose") + .help("Comma seperated list of filenames to search & update for PR creation" + + "(default: Dockerfile,docker-compose)"); return parser; } diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java index 0db3269c..abe22bb3 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java @@ -1,14 +1,15 @@ package com.salesforce.dockerfileimageupdate.model; -import com.google.common.collect.ImmutableList; import org.apache.commons.lang3.StringUtils; -import java.util.Arrays; -import java.util.List; +import java.util.regex.Pattern; public class ImageKeyValuePair { private static final String IMAGE = "image"; - private static final String INVALID_IMAGE_VALUE = "You have not provided a valid value for image key."; + private static final String INVALID_IMAGE_VALUE = "It is not a valid value for image key."; + private static final String INVALID_TAG_VALUE = "It is not a valid value for image tag."; + private static final String TAG_VALUE_LATEST = "Tag value is latest. So ignoring it"; + /** * The name of the base image */ @@ -17,20 +18,16 @@ public class ImageKeyValuePair { * The tag of the base image */ private final String tag; - /** - * As of writing, this could include {@code AS name} to run a multi-stage build - */ - private final List additionalParts; /** * Comment starting with # */ private final String comments; /** - * Accepts a FROM instruction line from a Dockerfile + * Accepts an image key value pair line from a docker-compose file * See {@code isImageKeyValuePair} to ensure you're passing a valid line in. * - * @param imageKeyValuePair a FROM instruction line from a Dockerfile + * @param imageKeyValuePair an Image Key value pair from a docker-compose file e.g: image: imageName:imageTag */ public ImageKeyValuePair(String imageKeyValuePair) { if (!isImageKeyValuePair(imageKeyValuePair)) { @@ -54,14 +51,13 @@ public ImageKeyValuePair(String imageKeyValuePair) { baseImageName = imageAndTag[0]; if (imageAndTag.length > 1) { tag = imageAndTag[1]; + validateImageTag(tag); } else { tag = null; } - additionalParts = ImmutableList.of(); } else { baseImageName = null; tag = null; - additionalParts = ImmutableList.of(); } } @@ -69,13 +65,11 @@ public ImageKeyValuePair(String imageKeyValuePair) { * Internal API to get a new ComposeImageValuePair from an existing object * @param baseImageName baseImageName to add * @param tag tag to add - * @param additionalParts additionalParts to add * @param comments comments to add */ - private ImageKeyValuePair(String baseImageName, String tag, List additionalParts, String comments) { + private ImageKeyValuePair(String baseImageName, String tag, String comments) { this.baseImageName = baseImageName; this.tag = tag; - this.additionalParts = ImmutableList.copyOf(additionalParts); this.comments = comments; } @@ -102,7 +96,7 @@ public static boolean isImageKeyValuePairWithThisImageAndOlderTag(String lineInF * @return a new FROM with the new image tag */ public ImageKeyValuePair getImageKeyValuePairWithNewTag(String newTag) { - return new ImageKeyValuePair(baseImageName, newTag, additionalParts, comments); + return new ImageKeyValuePair(baseImageName, newTag, comments); } /** @@ -128,11 +122,6 @@ public String toString() { if (hasTag()) { stringBuilder.append(String.format(":%s", tag.trim())); } - for (String part : additionalParts) { - if (StringUtils.isNotBlank(part)) { - stringBuilder.append(String.format(" %s", part.trim())); - } - } if (hasComments()) { stringBuilder.append(String.format(" %s", comments)); @@ -183,10 +172,6 @@ public String getTag() { return tag; } - public List getAdditionalParts() { - return additionalParts; - } - public boolean hasComments() { return comments != null; } @@ -195,4 +180,14 @@ public String getComments() { return comments; } + public static void validateImageTag(String tag) { + String tagVersionRegexStr = "([a-zA-Z0-9_]([-._a-zA-Z0-9])*)"; + Pattern validTag = Pattern.compile(tagVersionRegexStr); + if (!validTag.matcher(tag).matches()) { + throw new IllegalArgumentException(INVALID_TAG_VALUE); + } + if (tag.equals("latest")) { + throw new IllegalArgumentException(TAG_VALUE_LATEST); + } + } } diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidator.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidator.java index 3618e55a..ea7347de 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidator.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidator.java @@ -102,7 +102,7 @@ protected boolean hasNoChanges(GHContent content, GitForkBranch gitForkBranch) { String line; while ( (line = reader.readLine()) != null ) { if (FromInstruction.isFromInstructionWithThisImageAndOlderTag(line, - gitForkBranch.getImageName(), gitForkBranch.getImageTag()) && + gitForkBranch.getImageName(), gitForkBranch.getImageTag()) || ImageKeyValuePair.isImageKeyValuePairWithThisImageAndOlderTag(line, gitForkBranch.getImageName(), gitForkBranch.getImageTag())) { return false; diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java index de9d27a7..833835de 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java @@ -76,10 +76,11 @@ protected Optional processImageWithTag(String image, String tag, Name PullRequests pullRequests = getPullRequests(); GitHubPullRequestSender pullRequestSender = getPullRequestSender(dockerfileGitHubUtil, ns); GitForkBranch gitForkBranch = getGitForkBranch(image, tag, ns); + String filenamesToSearch = ns.get(Constants.FILENAMES_TO_SEARCH); log.info("Finding Dockerfiles with the image name {}...", image); Optional>> contentsWithImage = - this.dockerfileGitHubUtil.findFilesWithImage(image, orgsToIncludeInSearch, gitApiSearchLimit); + this.dockerfileGitHubUtil.findFilesWithImage(image, orgsToIncludeInSearch, gitApiSearchLimit, filenamesToSearch); if (contentsWithImage.isPresent()) { Iterator> it = contentsWithImage.get().iterator(); while (it.hasNext()){ diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java index c3decdf5..8b955493 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java @@ -56,13 +56,15 @@ public void execute(final Namespace ns, DockerfileGitHubUtil dockerfileGitHubUti + "be skipped.", Constants.SKIP_PR_CREATION); return; } + String filenamesToSearch = ns.get(Constants.FILENAMES_TO_SEARCH); + PullRequests pullRequests = getPullRequests(); GitHubPullRequestSender pullRequestSender = getPullRequestSender(dockerfileGitHubUtil, ns); GitForkBranch gitForkBranch = getGitForkBranch(ns); log.info("Finding Dockerfiles with the given image..."); Integer gitApiSearchLimit = ns.get(Constants.GIT_API_SEARCH_LIMIT); - Optional>> contentsWithImage = dockerfileGitHubUtil.getGHContents(ns.get(Constants.GIT_ORG), img, gitApiSearchLimit); + Optional>> contentsWithImage = dockerfileGitHubUtil.getGHContents(ns.get(Constants.GIT_ORG), img, gitApiSearchLimit, filenamesToSearch); if (contentsWithImage.isPresent()) { List> contentsFoundWithImage = contentsWithImage.get(); diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java index 9aa0fa06..53f8986d 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java @@ -37,5 +37,5 @@ private Constants() { public static final String GIT_API_SEARCH_LIMIT = "ghapisearchlimit"; public static final String SKIP_PR_CREATION = "skipprcreation"; public static final String IGNORE_IMAGE_STRING = "x"; - + public static final String FILENAMES_TO_SEARCH = "filenamestosearch"; } diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java index c6397666..d5eb97a1 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java @@ -88,14 +88,23 @@ public GHRepository getRepo(String repoName) throws IOException { public Optional>> findFilesWithImage( String image, Map orgsToIncludeOrExclude, - Integer gitApiSearchLimit) throws IOException { + Integer gitApiSearchLimit, + String filenamesToSearch) throws IOException { + GHContentSearchBuilder search = gitHubUtil.startSearch(); // Filename search appears to yield better / more results than language:Dockerfile // Root cause: linguist doesn't currently deal with prefixes of files: // https://github.com/github/linguist/issues/4566 // This will work in OR mode i.e, either filename is Dockerfile or docker-compose - search.filename("Dockerfile"); - search.filename("docker-compose"); + if (StringUtils.isNotBlank(filenamesToSearch)) { + String[] filenames = filenamesToSearch.split(","); + for (String filename : filenames) { + search.filename(filename); + } + } else { + log.error("No filenames provided to search for, exiting!!!"); + System.exit(-1); + } if (!orgsToIncludeOrExclude.isEmpty()) { StringBuilder includeOrExcludeOrgsQuery = new StringBuilder(); @@ -149,7 +158,7 @@ public Optional>> findFilesWithImage( + " of {}. The orgs with the maximum number of hits will be recursively removed" + " to reduce the search space. For every org that is excluded, a separate " + "search will be performed specific to that org.", gitApiSearchLimit); - return getSearchResultsExcludingOrgWithMostHits(image, files, orgsToIncludeOrExclude, gitApiSearchLimit); + return getSearchResultsExcludingOrgWithMostHits(image, files, orgsToIncludeOrExclude, gitApiSearchLimit, filenamesToSearch); } List> filesList = new ArrayList<>(); filesList.add(files); @@ -171,7 +180,8 @@ protected Optional>> getSearchResultsExcludi String image, PagedSearchIterable files, Map orgsToExclude, - Integer gitApiSearchLimit) throws IOException { + Integer gitApiSearchLimit, + String filenamesToSearch) throws IOException { List> allContentsWithImage = new ArrayList<>(); String orgWithMaximumHits = getOrgNameWithMaximumHits(files); log.info("The org with the maximum number of hits is {}", orgWithMaximumHits); @@ -180,13 +190,13 @@ protected Optional>> getSearchResultsExcludi orgsToInclude.put(orgWithMaximumHits, true); log.info("Running search only for the org with maximum hits."); Optional>> contentsForOrgWithMaximumHits; - contentsForOrgWithMaximumHits = findFilesWithImage(image, orgsToInclude, gitApiSearchLimit); + contentsForOrgWithMaximumHits = findFilesWithImage(image, orgsToInclude, gitApiSearchLimit, filenamesToSearch); final Map orgsToExcludeFromSearch = new HashMap<>(orgsToExclude); orgsToExcludeFromSearch.put(orgWithMaximumHits, false); log.info("Running search by excluding the orgs {}.", orgsToExcludeFromSearch.keySet()); Optional>> contentsExcludingOrgWithMaximumHits; - contentsExcludingOrgWithMaximumHits = findFilesWithImage(image, orgsToExcludeFromSearch, gitApiSearchLimit); + contentsExcludingOrgWithMaximumHits = findFilesWithImage(image, orgsToExcludeFromSearch, gitApiSearchLimit, filenamesToSearch); if (contentsForOrgWithMaximumHits.isPresent()) { allContentsWithImage.addAll(contentsForOrgWithMaximumHits.get()); } @@ -452,11 +462,12 @@ public boolean thisUserIsOwner(GHRepository repo) throws IOException { * @param org GitHub organization * @param img image to find * @param gitApiSearchLimit git api search limit + * @param filenamesToSearch filenames to search for PR creation * @throws IOException if there is any failure while I/O from git. * @throws InterruptedException if interrupted while fetching git content * @return {@code Optional} of {@code PagedSearchIterable} */ - public Optional>> getGHContents(String org, String img, Integer gitApiSearchLimit) + public Optional>> getGHContents(String org, String img, Integer gitApiSearchLimit, String filenamesToSearch) throws IOException, InterruptedException { Optional>> contentsWithImage = Optional.empty(); Map orgsToIncludeInSearch = new HashMap<>(); @@ -467,7 +478,7 @@ public Optional>> getGHContents(String org, orgsToIncludeInSearch.put(org, true); } for (int i = 0; i < 5; i++) { - contentsWithImage = findFilesWithImage(img, orgsToIncludeInSearch, gitApiSearchLimit); + contentsWithImage = findFilesWithImage(img, orgsToIncludeInSearch, gitApiSearchLimit, filenamesToSearch); if (contentsWithImage .orElseThrow(IOException::new) .stream() diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java index f7162df5..32925e47 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java @@ -1,6 +1,5 @@ package com.salesforce.dockerfileimageupdate.model; -import com.google.common.collect.ImmutableList; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -12,8 +11,8 @@ public class ImageKeyValuePairTest { public Object[][] inputImageKeyValuePairData() { return new Object[][]{ {"image: dockerimage:3 # some comment", "image: dockerimage:3 # some comment"}, - {"image: dockerimage:3 AS test", "image: dockerimage:3 AS test"}, - {"image: dockerimage:3 \tAS \ttest # some comment", "image: dockerimage:3 \tAS \ttest # some comment"}, + //{"image: dockerimage:3 AS test", "image: dockerimage:3 AS test"}, + //{"image: dockerimage:3 \tAS \ttest # some comment", "image: dockerimage:3 \tAS \ttest # some comment"}, {"image: dockerimage:3# some comment", "image: dockerimage:3 # some comment"}, {" image: dockerimage ", "image: dockerimage"}, {"\t image: \t dockerimage:4 \t #comment", "image: dockerimage:4 #comment"}, @@ -90,8 +89,8 @@ public Object[][] tagData() { {"image: image:", null}, {"image: image@some-digest", null}, {"image: image# some comment", null}, - {"image: image:\tsome-tag # comment", "\tsome-tag"}, - {"image: image: some-tag # comment", " some-tag"} + //{"image: image:\tsome-tag # comment", "\tsome-tag"}, + //{"image: image: some-tag # comment", " some-tag"} }; } @@ -124,8 +123,8 @@ public Object[][] instructionWithNewTagData() { {"image: image:", null}, {"image: image@some-digest", null}, {"image: image# some comment", null}, - {"image: image:\tsome-tag # comment", null}, - {"image: image: some-tag # comment", null} + //{"image: image:\tsome-tag # comment", null}, + //{"image: image: some-tag # comment", null} }; } @@ -135,7 +134,6 @@ public void testGetInstructionWithNewTag(String fromInstruction, String newTag) ImageKeyValuePair newImageKeyValuePair = oldImageKeyValuePair.getImageKeyValuePairWithNewTag(newTag); assertEquals(newImageKeyValuePair.getBaseImageName(), oldImageKeyValuePair.getBaseImageName()); assertEquals(newImageKeyValuePair.getComments(), oldImageKeyValuePair.getComments()); - assertEquals(newImageKeyValuePair.getAdditionalParts(), oldImageKeyValuePair.getAdditionalParts()); assertEquals(newImageKeyValuePair.getTag(), newTag); } @@ -158,30 +156,10 @@ public void testHasADifferentTag(String fromInstruction, String tagToCheck, bool assertEquals(new ImageKeyValuePair(fromInstruction).hasADifferentTag(tagToCheck), expectedResult); } -/* @DataProvider - public Object[][] additionalPartsData() { - return new Object[][] { - {"image: image:tag as builder", ImmutableList.of("as", "builder")}, - {"image: image:tag as builder of things", ImmutableList.of("as", "builder", "of", "things")}, - {"image: image:tag#as builder", ImmutableList.of()}, - {"image: image:tag\t# comment", ImmutableList.of()}, - {"image: image:tag \t# comment", ImmutableList.of()}, - {"image: image:tag some\tother \t thing \t# comment", ImmutableList.of("some", "other", "thing")}, - {"image: image:\t# comment # # # ", ImmutableList.of()}, - {"image: image:", ImmutableList.of()}, - {"image:", ImmutableList.of()} - }; - } - - @Test(dataProvider = "additionalPartsData") - public void testAdditionalPartsParsedCorrectly(String input, ImmutableList expectedResult) { - assertEquals(new ImageKeyValuePair(input).getAdditionalParts(), expectedResult); - }*/ - @DataProvider public Object[][] commentData() { return new Object[][] { - {"image: image:tag as builder", null}, + //{"image: image:tag as builder", null}, {"image: image:tag#as builder", "#as builder"}, {"image: image:tag # comment", "# comment"}, {"image: image:tag\t# comment", "# comment"}, diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidatorTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidatorTest.java index 5457a817..941aacbb 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidatorTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidatorTest.java @@ -16,8 +16,8 @@ import static com.salesforce.dockerfileimageupdate.model.ShouldForkResult.shouldNotForkResult; import static com.salesforce.dockerfileimageupdate.process.ForkableRepoValidator.*; import static com.salesforce.dockerfileimageupdate.process.GitHubPullRequestSender.REPO_IS_FORK; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.eq; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.testng.Assert.*; diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/GitHubPullRequestSenderTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/GitHubPullRequestSenderTest.java index 117d6034..46c3e464 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/GitHubPullRequestSenderTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/GitHubPullRequestSenderTest.java @@ -12,7 +12,6 @@ import org.mockito.Mockito; import org.testng.annotations.Test; -import static org.mockito.Matchers.any; import static org.mockito.Mockito.*; import static org.testng.Assert.*; diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/repository/GitHubTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/repository/GitHubTest.java index c40ed2a6..715ce991 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/repository/GitHubTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/repository/GitHubTest.java @@ -11,8 +11,8 @@ import java.io.IOException; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyString; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.testng.Assert.assertFalse; @@ -65,6 +65,7 @@ public void testShouldProcess() { public void testIsForkStaleReturnsTrue() throws IOException { setupGitHubRepo(); GHBranch mainBranch = mock(GHBranch.class); + when(gitHubRepository.getDefaultBranch()).thenReturn("main"); when(gitHubRepository.getBranch(anyString())).thenReturn(mainBranch); fork = mock(GHRepository.class); when(fork.getTree(any())).thenThrow(new GHFileNotFoundException("oh noes")); diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/storage/GitHubJsonStoreTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/storage/GitHubJsonStoreTest.java index 2c291c16..93610c10 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/storage/GitHubJsonStoreTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/storage/GitHubJsonStoreTest.java @@ -19,7 +19,8 @@ import org.kohsuke.github.GHTreeEntry; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; -import static org.mockito.Matchers.anyString; + +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; @@ -178,6 +179,7 @@ public void testUpdateStoreOnGithubCommitFailure(String storeContent) throws IOE GHTree ghTree = mock(GHTree.class); GHTreeEntry ghTreeEntry = mock(GHTreeEntry.class); + when(repo.getDefaultBranch()).thenReturn("defaultBranch"); when(repo.getFileContent(anyString())).thenThrow(new IOException("file sized is too large. Error: too_large")); when(repo.getCommit(anyString())).thenReturn(ghCommit); when(ghCommit.getTree()).thenReturn(ghTree); @@ -201,6 +203,7 @@ public void testUpdateStoreOnGithubInvalidJson() throws IOException { GHTree ghTree = mock(GHTree.class); GHTreeEntry ghTreeEntry = mock(GHTreeEntry.class); + when(repo.getDefaultBranch()).thenReturn("defaultBranch"); when(repo.getFileContent(anyString())).thenThrow(new IOException("file sized is too large. Error: too_large")); when(repo.getCommit(anyString())).thenReturn(ghCommit); when(ghCommit.getTree()).thenReturn(ghTree); diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java index 3e2f1b19..c2f5a152 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java @@ -21,7 +21,6 @@ import org.kohsuke.github.*; import org.testng.annotations.Test; -import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; import static org.testng.Assert.assertEquals; @@ -63,7 +62,7 @@ public void testAllCommandSuccessful() throws Exception { when(all.getPullRequests()).thenReturn(pullRequests); doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil); - when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).thenReturn(optionalContentsWithImageList); + when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), any(), any())).thenReturn(optionalContentsWithImageList); all.execute(ns, dockerfileGitHubUtil); verify(all, times(1)).getGitForkBranch(anyString(), anyString(), any()); @@ -110,7 +109,7 @@ public void testAllCommandSkipsSendingPRsIfSearchReturnsEmpty() throws Exception Optional>> optionalContentsWithImageList = Optional.empty(); doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil); - when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).thenReturn(optionalContentsWithImageList); + when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt(), anyString())).thenReturn(optionalContentsWithImageList); all.execute(ns, dockerfileGitHubUtil); @@ -158,7 +157,7 @@ public void testAllCommandSkipsSendingPRsIfSearchRaisesException() throws Except PagedSearchIterable contentsWithImage = mock(PagedSearchIterable.class); doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil); - when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).thenThrow(new GHException("some exception")); + when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), any(), any())).thenThrow(new GHException("some exception")); all.execute(ns, dockerfileGitHubUtil); @@ -205,8 +204,7 @@ public void testAllCommandSkipsSendingPRsIfPRCreationRaisesException() throws Ex Optional>> optionalContentsWithImageList = Optional.of(contentsWithImageList); doThrow(new IOException()).when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil); - when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).thenReturn(optionalContentsWithImageList); - + when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), any(), any())).thenReturn(optionalContentsWithImageList); all.execute(ns, dockerfileGitHubUtil); verify(all, times(1)).getGitForkBranch(anyString(), anyString(), any()); diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ChildTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ChildTest.java index 48ebad25..49954327 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ChildTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ChildTest.java @@ -19,7 +19,6 @@ import java.util.Map; import static com.salesforce.dockerfileimageupdate.utils.Constants.*; -import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.*; /** diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ParentTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ParentTest.java index 6e55a77a..b5bf6e72 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ParentTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ParentTest.java @@ -19,7 +19,6 @@ import org.testng.annotations.Test; import java.util.*; -import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; import static org.testng.Assert.*; @@ -104,7 +103,7 @@ public void testParentCommandSuccessful() throws Exception { when(parent.getPullRequests()).thenReturn(pullRequests); doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil); - when(dockerfileGitHubUtil.getGHContents(anyString(), anyString(), anyInt())).thenReturn(optionalContentsWithImageList); + when(dockerfileGitHubUtil.getGHContents(any(), any(), any(), any())).thenReturn(optionalContentsWithImageList); when(dockerfileGitHubUtil.getGitHubJsonStore("store")).thenReturn(imageTagStore); parent.execute(ns, dockerfileGitHubUtil); @@ -139,7 +138,7 @@ public void testParentCommandSuccessfulForS3ImageStore() throws Exception { when(parent.getPullRequests()).thenReturn(pullRequests); doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil); - when(dockerfileGitHubUtil.getGHContents(anyString(), anyString(), anyInt())).thenReturn(optionalContentsWithImageList); + when(dockerfileGitHubUtil.getGHContents(any(), any(), any(), any())).thenReturn(optionalContentsWithImageList); parent.execute(ns, dockerfileGitHubUtil); @@ -215,7 +214,7 @@ public void testParentCommandThrowsException() throws Exception { Optional>> optionalContentsWithImageList = Optional.of(contentsWithImageList); doThrow(new InterruptedException("Exception")).when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil); - when(dockerfileGitHubUtil.getGHContents(anyString(), anyString(), anyInt())).thenReturn(optionalContentsWithImageList); + when(dockerfileGitHubUtil.getGHContents(anyString(), anyString(), anyInt(), anyString())).thenReturn(optionalContentsWithImageList); parent.execute(ns, dockerfileGitHubUtil); diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java index 96b1b078..116dbfe1 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java @@ -18,6 +18,7 @@ import java.io.*; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.Map; import java.util.Iterator; @@ -28,7 +29,6 @@ import java.util.HashMap; import static com.salesforce.dockerfileimageupdate.model.PullRequestInfo.DEFAULT_TITLE; -import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; import static org.testng.Assert.*; @@ -54,6 +54,7 @@ public void testParentIsForkedOnlyOnce() throws Exception { GHRepository parent = mock(GHRepository.class); GHBranch branch = mock(GHBranch.class); + when(parent.getDefaultBranch()).thenReturn("main"); when(parent.getBranch(anyString())).thenReturn(branch); GHRepository fork = mock(GHRepository.class); when(fork.getOwnerName()).thenReturn("me"); @@ -232,7 +233,7 @@ public void testFindFiles_EmptyQuery(String query, String org) throws Exception dockerfileGitHubUtil = new DockerfileGitHubUtil(gitHubUtil); Map orgs = Collections.unmodifiableMap(Collections.singletonMap(org, true)); - dockerfileGitHubUtil.findFilesWithImage(query, orgs, 1000); + dockerfileGitHubUtil.findFilesWithImage(query, orgs, 1000, "Dockerfile,docker-compose"); } @Test @@ -251,7 +252,7 @@ public void testFindFiles() throws Exception { dockerfileGitHubUtil = new DockerfileGitHubUtil(gitHubUtil); Map orgs = Collections.unmodifiableMap(Collections.singletonMap("test", true)); - assertEquals(dockerfileGitHubUtil.findFilesWithImage("test", orgs, 1000), optionalContentsWithImageList); + assertEquals(dockerfileGitHubUtil.findFilesWithImage("test", orgs, 1000, "Dockerfile,docker-compose"), optionalContentsWithImageList); } @Test @@ -293,7 +294,7 @@ public void testFindFilesWithMoreThan1000Results() throws Exception { dockerfileGitHubUtil = new DockerfileGitHubUtil(gitHubUtil); when(dockerfileGitHubUtil.getOrgNameWithMaximumHits(contentsWithImage)).thenReturn("org-1", "org-2", "org-3"); - assertEquals(dockerfileGitHubUtil.findFilesWithImage("test", orgsToIncludeOrExclude, 1000), optionalContentsWithImageList); + assertEquals(dockerfileGitHubUtil.findFilesWithImage("test", orgsToIncludeOrExclude, 1000, "Dockerfile,docker-compose"), optionalContentsWithImageList); } @Test @@ -328,7 +329,7 @@ public void getSearchResultsExcludingOrgWithMostHits() throws Exception { dockerfileGitHubUtil = new DockerfileGitHubUtil(gitHubUtil); Map orgsToIncludeOrExclude = new HashMap<>(); - assertEquals((dockerfileGitHubUtil.getSearchResultsExcludingOrgWithMostHits("image", contentsWithImage, orgsToIncludeOrExclude, 1000)).get().size(), 2); + assertEquals((dockerfileGitHubUtil.getSearchResultsExcludingOrgWithMostHits("image", contentsWithImage, orgsToIncludeOrExclude, 1000, "Dockerfile,docker-compose")).get().size(), 2); //This check ensures that the parameter passed to the method is not modified. Instead, the method creates a local copy of the map and modifies that. assertEquals(orgsToIncludeOrExclude.size(), 0); } @@ -671,10 +672,10 @@ public void testGetGHContents() throws Exception { when(contentsWithImageIterator.next()).thenReturn(content1, content2, content3, null); when(contentsWithImage.iterator()).thenReturn(contentsWithImageIterator); Map orgs = Collections.unmodifiableMap(Collections.singletonMap("org", true)); - when(dockerfileGitHubUtil.findFilesWithImage(anyString(), eq(orgs), anyInt())).thenReturn(optionalContentsWithImageList); - when(dockerfileGitHubUtil.getGHContents("org", "image", 1000)).thenCallRealMethod(); + when(dockerfileGitHubUtil.findFilesWithImage(anyString(), eq(orgs), anyInt(), anyString())).thenReturn(optionalContentsWithImageList); + when(dockerfileGitHubUtil.getGHContents("org", "image", 1000, "Dockerfile,docker-compose")).thenCallRealMethod(); - assertEquals(dockerfileGitHubUtil.getGHContents("org", "image", 1000), Optional.of(contentsWithImageList)); + assertEquals(dockerfileGitHubUtil.getGHContents("org", "image", 1000, "Dockerfile,docker-compose"), Optional.of(contentsWithImageList)); } @Test @@ -689,10 +690,10 @@ public void testGHContentsNoOutput() throws Exception { DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class); when(dockerfileGitHubUtil.getGitHubUtil()).thenReturn(gitHubUtil); Map orgs = Collections.unmodifiableMap(Collections.singletonMap("org", true)); - when(dockerfileGitHubUtil.findFilesWithImage(anyString(), eq(orgs), anyInt())).thenReturn(optionalContentsWithImageList); - when(dockerfileGitHubUtil.getGHContents("org", "image", 1000)).thenCallRealMethod(); + when(dockerfileGitHubUtil.findFilesWithImage(anyString(), eq(orgs), anyInt(), anyString())).thenReturn(optionalContentsWithImageList); + when(dockerfileGitHubUtil.getGHContents("org", "image", 1000, "Dockerfile,docker-compose")).thenCallRealMethod(); - assertEquals(dockerfileGitHubUtil.getGHContents("org", "image", 1000), Optional.empty()); + assertEquals(dockerfileGitHubUtil.getGHContents("org", "image", 1000, "Dockerfile,docker-compose"), Optional.empty()); } @Test @@ -800,14 +801,17 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio //when(dockerfileGitHubUtil.getPullRequestForImageBranch(any(), any())).thenReturn // (Optional.empty()); //when(dockerfileGitHubUtil.getRepo(forkedRepo.getFullName())).thenReturn(forkedRepo); + InputStream stream = new ByteArrayInputStream("someContent".getBytes(StandardCharsets.UTF_8)); GHContent forkedRepoContent1 = mock(GHContent.class); + when(forkedRepoContent1.read()).thenReturn(stream); when(gitHubUtil.tryRetrievingContent(eq(forkedRepo), eq("df11"), eq("image-tag"))).thenReturn(forkedRepoContent1); GHContent forkedRepoContent2 = mock(GHContent.class); + when(forkedRepoContent2.read()).thenReturn(stream); when(gitHubUtil.tryRetrievingContent(eq(forkedRepo), eq("df12"), eq("image-tag"))).thenReturn(forkedRepoContent2); doNothing().when(dockerfileGitHubUtil).modifyOnGithub(any(), eq("image-tag"), eq("image") - , eq("tag"), anyString(), anyString()); + , eq("tag"), any(), any()); dockerfileGitHubUtil.changeDockerfiles(ns, pathToDockerfilesInParentRepo, new GitHubContentToProcess(forkedRepo, parentRepo, ""), new ArrayList<>(), gitForkBranch); @@ -820,7 +824,7 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio // Both Dockerfiles modified verify(dockerfileGitHubUtil, times(2)) - .modifyOnGithub(any(), eq("image-tag"), eq("image"), eq("tag"), anyString(), anyString()); + .modifyOnGithub(any(), eq("image-tag"), eq("image"), eq("tag"), any(), any()); // Only one PR created on the repo with changes to both Dockerfiles. verify(dockerfileGitHubUtil, times(1)).createPullReq(eq(parentRepo), diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/GitHubUtilTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/GitHubUtilTest.java index 78805cc9..792a9e98 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/GitHubUtilTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/GitHubUtilTest.java @@ -19,7 +19,6 @@ import java.util.Map; import java.util.concurrent.TimeUnit; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.*; import static org.testng.Assert.*; From e0edacacde7f19e78c7fd6cfb49db59c6bdafc7f Mon Sep 17 00:00:00 2001 From: "pankaj.chugh" Date: Mon, 23 Jan 2023 11:46:59 +0530 Subject: [PATCH 3/6] Updated to support docker-compose DFIU update --- .../dockerfileimageupdate/CommandLine.java | 13 ++--- .../model/ImageKeyValuePair.java | 54 ++++++++++--------- .../search/GitHubImageSearchTermList.java | 10 ++-- .../subcommands/impl/All.java | 2 +- .../subcommands/impl/Parent.java | 3 +- .../utils/Constants.java | 2 +- .../utils/DockerfileGitHubUtil.java | 35 +++++++++--- .../model/ImageKeyValuePairTest.java | 8 +-- .../search/GitHubImageSearchTermListTest.java | 6 +-- .../subcommands/impl/AllTest.java | 32 ++++++----- .../subcommands/impl/ChildTest.java | 17 +++--- .../subcommands/impl/ParentTest.java | 21 +++++--- .../org.mockito.plugins.MockMaker | 1 + 13 files changed, 125 insertions(+), 79 deletions(-) create mode 100644 dockerfile-image-update/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java index a55569b8..f2ae7d83 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/CommandLine.java @@ -83,18 +83,19 @@ static ArgumentParser getArgumentParser() { .setDefault(false) .help("Only update image tag store. Skip creating PRs"); parser.addArgument("-x") - .help("comment snippet mentioned in line just before FROM instruction for ignoring a child image. " + + .help("comment snippet mentioned in line just before 'FROM' instruction(Dockerfile)" + + "or 'image' instruction(docker-compose) for ignoring a child image. " + "Defaults to 'no-dfiu'"); + parser.addArgument("-t", "--" + FILE_NAMES_TO_SEARCH) + .type(String.class) + .setDefault("Dockerfile,docker-compose") + .help("Comma seperated list of filenames to search & update for PR creation" + + "(default: Dockerfile,docker-compose)"); parser.addArgument("-r", "--" + RATE_LIMIT_PR_CREATION) .type(String.class) .setDefault("") .required(false) .help("Use RateLimiting when sending PRs. RateLimiting is enabled only if this value is set it's disabled by default."); - parser.addArgument("-t", "--" + Constants.FILENAMES_TO_SEARCH) - .type(String.class) - .setDefault("Dockerfile,docker-compose") - .help("Comma seperated list of filenames to search & update for PR creation" + - "(default: Dockerfile,docker-compose)"); return parser; } diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java index abe22bb3..fbba57fd 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java @@ -1,14 +1,16 @@ package com.salesforce.dockerfileimageupdate.model; +import com.salesforce.dockerfileimageupdate.utils.DockerfileGitHubUtil; import org.apache.commons.lang3.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.regex.Pattern; public class ImageKeyValuePair { + private static final Logger log = LoggerFactory.getLogger(ImageKeyValuePair.class); private static final String IMAGE = "image"; private static final String INVALID_IMAGE_VALUE = "It is not a valid value for image key."; - private static final String INVALID_TAG_VALUE = "It is not a valid value for image tag."; - private static final String TAG_VALUE_LATEST = "Tag value is latest. So ignoring it"; /** * The name of the base image @@ -22,6 +24,10 @@ public class ImageKeyValuePair { * Comment starting with # */ private final String comments; + /** + * Yaml Spacing # + */ + private final String spaces; /** * Accepts an image key value pair line from a docker-compose file @@ -41,6 +47,13 @@ public ImageKeyValuePair(String imageKeyValuePair) { } else { comments = null; } + // Get Yaml spacing in variable + if (lineWithoutComment.startsWith(" ")) { + spaces = lineWithoutComment.substring(0, lineWithoutComment.indexOf(IMAGE)); + } else { + spaces = ""; + } + // Remove "image:" from remaining string String lineWithoutImageKey = lineWithoutComment.trim(). replaceFirst(IMAGE, "").replaceFirst(":", ""). @@ -51,7 +64,6 @@ public ImageKeyValuePair(String imageKeyValuePair) { baseImageName = imageAndTag[0]; if (imageAndTag.length > 1) { tag = imageAndTag[1]; - validateImageTag(tag); } else { tag = null; } @@ -67,14 +79,15 @@ public ImageKeyValuePair(String imageKeyValuePair) { * @param tag tag to add * @param comments comments to add */ - private ImageKeyValuePair(String baseImageName, String tag, String comments) { + private ImageKeyValuePair(String baseImageName, String tag, String comments, String spaces) { this.baseImageName = baseImageName; this.tag = tag; this.comments = comments; + this.spaces = spaces; } /** - * Check if this {@code lineInFile} is a FROM instruction, + * Check if this {@code lineInFile} is a image instruction, * it is referencing {@code imageName} as a base image, * and the tag is not the same as {@code imageTag} (or there is no tag) * @param lineInFile Line a code file @@ -84,8 +97,10 @@ private ImageKeyValuePair(String baseImageName, String tag, String comments) { */ public static boolean isImageKeyValuePairWithThisImageAndOlderTag(String lineInFile, String imageName, String imageTag) { if (ImageKeyValuePair.isImageKeyValuePair(lineInFile)) { - ImageKeyValuePair ImageKeyValuePair = new ImageKeyValuePair(lineInFile); - return ImageKeyValuePair.hasBaseImage(imageName) && ImageKeyValuePair.hasADifferentTag(imageTag); + ImageKeyValuePair imageKeyValuePair = new ImageKeyValuePair(lineInFile); + return imageKeyValuePair.hasBaseImage(imageName) + && imageKeyValuePair.hasADifferentTag(imageTag) + && DockerfileGitHubUtil.isValidImageTag(imageKeyValuePair.getTag()); } return false; } @@ -93,16 +108,16 @@ public static boolean isImageKeyValuePairWithThisImageAndOlderTag(String lineInF /** * Get a new {@code ComposeImageValuePair} the same as this but with the {@code tag} set as {@code newTag} * @param newTag the new image tag - * @return a new FROM with the new image tag + * @return a new image instruction with the new image tag */ public ImageKeyValuePair getImageKeyValuePairWithNewTag(String newTag) { - return new ImageKeyValuePair(baseImageName, newTag, comments); + return new ImageKeyValuePair(baseImageName, newTag, comments, spaces); } /** - * Determines whether the line is a FROM instruction line in a Dockerfile - * @param composeImageKeyValueLine a single line(key:value) from a Docker-compose.yaml - * @return the line is a FROM instruction line or not + * Determines whether the line is a image instruction line in a docker-compose.yaml + * @param composeImageKeyValueLine a single line(key:value) from a docker-compose.yaml + * @return the line is a image instruction line or not */ public static boolean isImageKeyValuePair(String composeImageKeyValueLine) { if (StringUtils.isNotBlank(composeImageKeyValueLine)) { @@ -112,11 +127,11 @@ public static boolean isImageKeyValuePair(String composeImageKeyValueLine) { } /** - * @return a String representation of a FROM instruction line in Dockerfile. No new line at the end + * @return a String representation of a image instruction line in docker-compose.yaml file. No new line at the end */ @Override public String toString() { - StringBuilder stringBuilder = new StringBuilder(IMAGE); + StringBuilder stringBuilder = new StringBuilder(spaces + IMAGE); stringBuilder.append(": "); stringBuilder.append(baseImageName); if (hasTag()) { @@ -179,15 +194,4 @@ public boolean hasComments() { public String getComments() { return comments; } - - public static void validateImageTag(String tag) { - String tagVersionRegexStr = "([a-zA-Z0-9_]([-._a-zA-Z0-9])*)"; - Pattern validTag = Pattern.compile(tagVersionRegexStr); - if (!validTag.matcher(tag).matches()) { - throw new IllegalArgumentException(INVALID_TAG_VALUE); - } - if (tag.equals("latest")) { - throw new IllegalArgumentException(TAG_VALUE_LATEST); - } - } } diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/search/GitHubImageSearchTermList.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/search/GitHubImageSearchTermList.java index e77baf53..fcd20c64 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/search/GitHubImageSearchTermList.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/search/GitHubImageSearchTermList.java @@ -20,12 +20,12 @@ public class GitHubImageSearchTermList { * @param image the name of the image including registry * @return list of GitHub code search terms */ - public static List getSearchTerms(String image) { + public static List getSearchTerms(String image, String filenames) { if (image == null || image.trim().isEmpty()) { return ImmutableList.of(); } String[] imageParts = image.split("/"); - ProcessingState state = processDomainPartOfImage(imageParts[0]); + ProcessingState state = processDomainPartOfImage(imageParts[0], filenames); if (imageParts.length > 1) { for (int i = 1; i < imageParts.length - 1; i++) { String imagePart = imageParts[i]; @@ -90,12 +90,14 @@ private static void processFinalUrlSegment(ProcessingState state, String finalIm * @param domain the domain part of the image (which may or may not be the full registry name) * @return processing state for the search terms */ - static ProcessingState processDomainPartOfImage(String domain) { + static ProcessingState processDomainPartOfImage(String domain, String filenames) { ProcessingState state = new ProcessingState(); if (domain == null || domain.trim().isEmpty()) { return state; } else { - state.addToCurrentTerm("FROM "); + if (filenames.equalsIgnoreCase("Dockerfile")) { + state.addToCurrentTerm("FROM "); + } if (domain.contains("-")) { processDashedDomainParts(state, domain); } else { diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java index f942f7e5..c9970773 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java @@ -81,7 +81,7 @@ protected Optional processImageWithTag(String image, String tag, Name PullRequests pullRequests = getPullRequests(); GitHubPullRequestSender pullRequestSender = getPullRequestSender(dockerfileGitHubUtil, ns); GitForkBranch gitForkBranch = getGitForkBranch(image, tag, ns); - String filenamesToSearch = ns.get(Constants.FILENAMES_TO_SEARCH); + String filenamesToSearch = ns.get(Constants.FILE_NAMES_TO_SEARCH); log.info("Finding Dockerfiles with the image name {}...", image); Optional>> contentsWithImage = diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java index 54b447c7..634d5871 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java @@ -44,6 +44,8 @@ public void execute(final Namespace ns, DockerfileGitHubUtil dockerfileGitHubUti String store = ns.get(Constants.STORE); String img = ns.get(Constants.IMG); String tag = ns.get(Constants.TAG); + String filenamesToSearch = ns.getString(Constants.FILE_NAMES_TO_SEARCH); + log.info("Updating store..."); try { ImageTagStore imageTagStore = ImageStoreUtil.initializeImageTagStore(this.dockerfileGitHubUtil, store); @@ -57,7 +59,6 @@ public void execute(final Namespace ns, DockerfileGitHubUtil dockerfileGitHubUti + "be skipped.", Constants.SKIP_PR_CREATION); return; } - String filenamesToSearch = ns.get(Constants.FILENAMES_TO_SEARCH); PullRequests pullRequests = getPullRequests(); GitHubPullRequestSender pullRequestSender = getPullRequestSender(dockerfileGitHubUtil, ns); diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java index 87a44021..eb88861b 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java @@ -39,7 +39,7 @@ private Constants() { public static final String GIT_API_SEARCH_LIMIT = "ghapisearchlimit"; public static final String SKIP_PR_CREATION = "skipprcreation"; public static final String IGNORE_IMAGE_STRING = "x"; - public static final String FILENAMES_TO_SEARCH = "filenamestosearch"; + public static final String FILE_NAMES_TO_SEARCH = "filenamestosearch"; public static final String RATE_LIMIT_PR_CREATION = "rate-limit-pr-creations"; //max number of PRs to be sent (or tokens to be added) per DEFAULT_RATE_LIMIT_DURATION(per hour in this case) public static final long DEFAULT_RATE_LIMIT = 60; diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java index 7c49a80d..dd864d47 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java @@ -16,6 +16,8 @@ import net.sourceforge.argparse4j.inf.*; import org.apache.commons.lang3.StringUtils; import org.kohsuke.github.*; + +import java.util.regex.Pattern; import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -124,8 +126,8 @@ public Optional>> findFilesWithImage( if (image.substring(image.lastIndexOf(' ') + 1).length() <= 1) { throw new IOException("Invalid image name."); } - List terms = GitHubImageSearchTermList.getSearchTerms(image); - log.info("Searching for {} with terms: {}", image, terms); + List terms = GitHubImageSearchTermList.getSearchTerms(image, filenamesToSearch); + log.info("Searching for {}, in files: {}, with terms: {}", image, filenamesToSearch, terms); terms.forEach(search::q); PagedSearchIterable files = search.list(); int totalCount = files.getTotalCount(); @@ -292,7 +294,7 @@ protected void findImagesAndFix(GHContent content, String branch, String img, boolean modified = rewriteDockerfile(img, tag, reader, strB, ignoreImageString); if (modified) { content.update(strB.toString(), - "Fix Dockerfile base image in /" + content.getPath() + "\n\n" + customMessage, branch); + "Fix Docker base image in /" + content.getPath() + "\n\n" + customMessage, branch); } } @@ -317,7 +319,7 @@ protected boolean rewriteDockerfile(String img, String tag, } /** - * This method will read a line and see if the line contains a FROM instruction with the specified + * This method will read a line and see if the line contains a FROM instruction(Dockerfile) or imageKeyValuePair instruction(docker-compose) with the specified * {@code imageToFind}. If the image does not have the given {@code tag} then {@code stringBuilder} * will get a modified version of the line with the new {@code tag}. We return {@code true} in this * instance. @@ -337,7 +339,7 @@ protected boolean changeIfDockerfileBaseImageLine(String imageToFind, String tag boolean modified = false; String outputLine = line; - // Only check/modify lines which contain a FROM instruction + // Only check/modify lines which contain a FROM instruction or imageKeyValuePair instruction if (FromInstruction.isFromInstruction(line)) { FromInstruction fromInstruction = new FromInstruction(line); @@ -350,7 +352,8 @@ protected boolean changeIfDockerfileBaseImageLine(String imageToFind, String tag } else if (ImageKeyValuePair.isImageKeyValuePair(line)) { ImageKeyValuePair imageKeyValuePair = new ImageKeyValuePair(line); if (imageKeyValuePair.hasBaseImage(imageToFind) && - imageKeyValuePair.hasADifferentTag(tag)) { + imageKeyValuePair.hasADifferentTag(tag) && + DockerfileGitHubUtil.isValidImageTag(imageKeyValuePair.getTag())) { imageKeyValuePair = imageKeyValuePair.getImageKeyValuePairWithNewTag(tag); modified = true; } @@ -361,7 +364,7 @@ protected boolean changeIfDockerfileBaseImageLine(String imageToFind, String tag } /** - * Determines whether a comment before FROM line has {@code ignoreImageString} to ignore creating dfiu PR + * Determines whether a comment before FROM line or imageKeyValuePair line has {@code ignoreImageString} to ignore creating dfiu PR * If {@code ignoreImageString} present in comment, PR should be ignored * If {@code ignoreImageString} is empty, then by default 'no-dfiu' comment will be searched * @param line line to search for comment @@ -386,7 +389,7 @@ public void createPullReq(GHRepository origRepo, while (true) { // TODO: accept rateLimiter Optional with option to get no-op rateLimiter // where it's not required. - if(rateLimiter != null) { + if (rateLimiter != null) { log.info("Trying to consume a token before creating pull request.."); // Consume a token from the token bucket. // If a token is not available this method will block until @@ -566,4 +569,20 @@ public void changeDockerfiles(Namespace ns, rateLimiter); } } + + public static boolean isValidImageTag(String tag) { + String tagVersionRegexStr = "([a-zA-Z0-9_]([-._a-zA-Z0-9])*)"; + Pattern validTag = Pattern.compile(tagVersionRegexStr); + if (StringUtils.isNotBlank(tag)) { + if (!validTag.matcher(tag).matches()) { + log.warn("{} is not a valid value for image tag. So will be ignored!", tag); + return false; + } else if (tag.equals("latest")) { + log.warn("Tag value is latest. So will be ignored"); + return false; + } + return true; + } + return false; + } } diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java index 32925e47..422763b0 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java @@ -11,10 +11,10 @@ public class ImageKeyValuePairTest { public Object[][] inputImageKeyValuePairData() { return new Object[][]{ {"image: dockerimage:3 # some comment", "image: dockerimage:3 # some comment"}, - //{"image: dockerimage:3 AS test", "image: dockerimage:3 AS test"}, - //{"image: dockerimage:3 \tAS \ttest # some comment", "image: dockerimage:3 \tAS \ttest # some comment"}, + {" image: dockerimage:3", " image: dockerimage:3"}, + {"\timage: dockerimage:3", "image: dockerimage:3"}, {"image: dockerimage:3# some comment", "image: dockerimage:3 # some comment"}, - {" image: dockerimage ", "image: dockerimage"}, + {" image: dockerimage ", " image: dockerimage"}, {"\t image: \t dockerimage:4 \t #comment", "image: dockerimage:4 #comment"}, {"image: dockerimage:4:4:4 #comment", "image: dockerimage:4 #comment"}, {"image: dockerimage:4 #comment me # # ", "image: dockerimage:4 #comment me # # "} @@ -206,7 +206,7 @@ public void testInvalidFromData(String input) { @DataProvider public Object[][] isImageKeyValuePairWithThisImageAndOlderTagData() { return new Object[][]{ - {"image: someimage", "someimage", "sometag", true}, + {"image: someimage", "someimage", "sometag", false}, {"image: someimage:sometag", "someimage", "sometag", false}, {"not a valid image key-value pair", "someimage", "sometag", false}, {"image: someimage:oldtag", "someimage", "sometag", true} diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/search/GitHubImageSearchTermListTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/search/GitHubImageSearchTermListTest.java index 603e7063..677fe292 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/search/GitHubImageSearchTermListTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/search/GitHubImageSearchTermListTest.java @@ -34,7 +34,7 @@ public Object[][] imageAndParts() { @Test(dataProvider = "imageAndParts") public void testGetSearchTermsContent(String image, List expectedResult) { - List searchTerms = GitHubImageSearchTermList.getSearchTerms(image); + List searchTerms = GitHubImageSearchTermList.getSearchTerms(image, "Dockerfile"); assertEquals(joinListByComma(searchTerms), joinListByComma(expectedResult)); } @@ -44,7 +44,7 @@ private String joinListByComma(List list) { @Test(dataProvider = "imageAndParts") public void testGetSearchTermsSize(String image, List expectedResult) { - List searchTerms = GitHubImageSearchTermList.getSearchTerms(image); + List searchTerms = GitHubImageSearchTermList.getSearchTerms(image, "Dockerfile"); assertEquals(searchTerms, expectedResult); } @@ -63,7 +63,7 @@ public Object[][] domainCurrentTermAndProcessedTerms() { @Test(dataProvider = "domainCurrentTermAndProcessedTerms") public void testProcessDomainPartOfImage(String domain, String expectedCurrentTerm, List expectedProcessedTerms) { - GitHubImageSearchTermList.ProcessingState state = GitHubImageSearchTermList.processDomainPartOfImage(domain); + GitHubImageSearchTermList.ProcessingState state = GitHubImageSearchTermList.processDomainPartOfImage(domain, "Dockerfile"); assertEquals(joinListByComma(state.terms), joinListByComma(expectedProcessedTerms)); assertEquals(state.getCurrentTerm(), expectedCurrentTerm); } diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java index a2985436..67486d9d 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java @@ -19,7 +19,11 @@ import net.sourceforge.argparse4j.inf.Namespace; import org.kohsuke.github.*; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.testng.annotations.Test; + +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; import static org.testng.Assert.assertEquals; @@ -36,7 +40,7 @@ public void testAllCommandSuccessful() throws Exception { "store"); Namespace ns = new Namespace(nsMap); - RateLimiter rateLimiter = spy(new RateLimiter()); + RateLimiter rateLimiter = new RateLimiter(); All all = spy(new All()); DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class); GitHubPullRequestSender pullRequestSender = mock(GitHubPullRequestSender.class); @@ -61,20 +65,24 @@ public void testAllCommandSuccessful() throws Exception { when(all.getPullRequestSender(dockerfileGitHubUtil, ns)).thenReturn(pullRequestSender); when(all.getGitForkBranch("image1", "tag1", ns)).thenReturn(gitForkBranch); when(all.getPullRequests()).thenReturn(pullRequests); + doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, - contentsWithImage, gitForkBranch, dockerfileGitHubUtil, rateLimiter); + contentsWithImage, gitForkBranch, dockerfileGitHubUtil, + rateLimiter); when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), any(), any())).thenReturn(optionalContentsWithImageList); - - all.execute(ns, dockerfileGitHubUtil); - verify(all).getGitForkBranch(anyString(), anyString(), any()); - verify(all).getPullRequestSender(dockerfileGitHubUtil, ns); - verify(all).getPullRequests(); - verify(pullRequests).prepareToCreate(eq(ns), eq(pullRequestSender), - eq(contentsWithImage), eq(gitForkBranch), eq(dockerfileGitHubUtil), any(RateLimiter.class)); - verify(all, times(0)).processErrorMessages(anyString(), anyString(), any()); - verify(all).printSummary(anyList(), any()); - + try (MockedStatic mockedRateLimiter = Mockito.mockStatic(RateLimiter.class)) { + mockedRateLimiter.when(() -> RateLimiter.getInstance(ns)) + .thenReturn(rateLimiter); + all.execute(ns, dockerfileGitHubUtil); + verify(all).getGitForkBranch(anyString(), anyString(), any()); + verify(all).getPullRequestSender(dockerfileGitHubUtil, ns); + verify(all).getPullRequests(); + verify(pullRequests).prepareToCreate(eq(ns), eq(pullRequestSender), + eq(contentsWithImage), eq(gitForkBranch), eq(dockerfileGitHubUtil), eq(rateLimiter)); + verify(all, times(0)).processErrorMessages(anyString(), anyString(), any()); + verify(all).printSummary(anyList(), any()); + } } @Test diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ChildTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ChildTest.java index 5fb4fb61..db875771 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ChildTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ChildTest.java @@ -13,6 +13,8 @@ import com.salesforce.dockerfileimageupdate.utils.DockerfileGitHubUtil; import net.sourceforge.argparse4j.inf.Namespace; import org.kohsuke.github.GHRepository; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import com.salesforce.dockerfileimageupdate.utils.RateLimiter; @@ -48,7 +50,7 @@ public void checkPullRequestMade(Map inputMap) throws Exception Child child = spy(new Child()); Namespace ns = new Namespace(inputMap); DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class); - RateLimiter rateLimiter = spy(new RateLimiter()); + RateLimiter rateLimiter = new RateLimiter(); GitHubJsonStore imageTagStore = mock(GitHubJsonStore.class); when(dockerfileGitHubUtil.getRepo(any())).thenReturn(new GHRepository()); when(dockerfileGitHubUtil.getOrCreateFork(any())).thenReturn(new GHRepository()); @@ -57,11 +59,14 @@ public void checkPullRequestMade(Map inputMap) throws Exception when(dockerfileGitHubUtil.getGitHubJsonStore("test")).thenReturn(imageTagStore); doNothing().when(dockerfileGitHubUtil).createPullReq(any(), anyString(), any(), any(), eq(rateLimiter)); - - child.execute(ns, dockerfileGitHubUtil); - - verify(dockerfileGitHubUtil, times(1)) - .createPullReq(any(), anyString(), any(), any(), any(RateLimiter.class)); + try (MockedStatic mockedRateLimiter = Mockito.mockStatic(RateLimiter.class)) { + mockedRateLimiter.when(() -> RateLimiter.getInstance(ns)) + .thenReturn(rateLimiter); + child.execute(ns, dockerfileGitHubUtil); + + verify(dockerfileGitHubUtil, times(1)) + .createPullReq(any(), anyString(), any(), any(), any(RateLimiter.class)); + } } @Test diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ParentTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ParentTest.java index 9f78b344..9ad0599d 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ParentTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/ParentTest.java @@ -16,6 +16,8 @@ import com.salesforce.dockerfileimageupdate.utils.*; import net.sourceforge.argparse4j.inf.Namespace; import org.kohsuke.github.*; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.testng.annotations.Test; import java.util.*; @@ -142,14 +144,17 @@ public void testParentCommandSuccessfulForS3ImageStore() throws Exception { contentsWithImage, gitForkBranch, dockerfileGitHubUtil, rateLimiter); when(dockerfileGitHubUtil.getGHContents(any(), any(), any(), any())).thenReturn(optionalContentsWithImageList); - - parent.execute(ns, dockerfileGitHubUtil); - - verify(parent).getGitForkBranch(ns); - verify(parent).getPullRequestSender(dockerfileGitHubUtil, ns); - verify(parent).getPullRequests(); - verify(pullRequests).prepareToCreate(eq(ns), eq(pullRequestSender), - eq(contentsWithImage), eq(gitForkBranch), eq(dockerfileGitHubUtil), any(RateLimiter.class)); + try (MockedStatic mockedRateLimiter = Mockito.mockStatic(RateLimiter.class)) { + mockedRateLimiter.when(() -> RateLimiter.getInstance(ns)) + .thenReturn(rateLimiter); + parent.execute(ns, dockerfileGitHubUtil); + + verify(parent).getGitForkBranch(ns); + verify(parent).getPullRequestSender(dockerfileGitHubUtil, ns); + verify(parent).getPullRequests(); + verify(pullRequests).prepareToCreate(eq(ns), eq(pullRequestSender), + eq(contentsWithImage), eq(gitForkBranch), eq(dockerfileGitHubUtil), any(RateLimiter.class)); + } } @Test diff --git a/dockerfile-image-update/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/dockerfile-image-update/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 00000000..1f0955d4 --- /dev/null +++ b/dockerfile-image-update/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1 @@ +mock-maker-inline From 9fc5cad21635be36999477f70959bcd7bff6b6ef Mon Sep 17 00:00:00 2001 From: "pankaj.chugh" Date: Mon, 23 Jan 2023 16:37:34 +0530 Subject: [PATCH 4/6] Added unit tests --- .../utils/DockerfileGitHubUtil.java | 4 +- .../process/ForkableRepoValidatorTest.java | 4 ++ .../utils/DockerfileGitHubUtilTest.java | 68 ++++++++++++++++++- 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java index dd864d47..a7fd7d2d 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java @@ -574,10 +574,10 @@ public static boolean isValidImageTag(String tag) { String tagVersionRegexStr = "([a-zA-Z0-9_]([-._a-zA-Z0-9])*)"; Pattern validTag = Pattern.compile(tagVersionRegexStr); if (StringUtils.isNotBlank(tag)) { - if (!validTag.matcher(tag).matches()) { + if (!validTag.matcher(tag.trim()).matches()) { log.warn("{} is not a valid value for image tag. So will be ignored!", tag); return false; - } else if (tag.equals("latest")) { + } else if (tag.trim().equals("latest")) { log.warn("Tag value is latest. So will be ignored"); return false; } diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidatorTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidatorTest.java index 941aacbb..cb8b8ac5 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidatorTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidatorTest.java @@ -209,6 +209,10 @@ public Object[][] hasNoChangesData() { {"FROM imageName:tag", "imageName", "tag", true}, {"FROM imageName:anotherTag", "imageName", "tag", false}, {"FROM anotherImage:tag", "imageName", "tag", true}, + {"image: imageName", "imageName", "tag", true}, + {"image: imageName:tag", "imageName", "tag", true}, + {"image: imageName:anotherTag", "imageName", "tag", false}, + {"image: anotherImage:tag", "imageName", "tag", true}, }; } diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java index 05eae393..b2437a32 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java @@ -13,6 +13,7 @@ import net.sourceforge.argparse4j.inf.*; import org.kohsuke.github.*; import org.mockito.*; +import org.slf4j.Logger; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -29,6 +30,7 @@ import java.util.HashMap; import static com.salesforce.dockerfileimageupdate.model.PullRequestInfo.DEFAULT_TITLE; +import static com.salesforce.dockerfileimageupdate.utils.ResultsProcessor.REPOS_SKIPPED_MESSAGE; import static org.mockito.Mockito.*; import static org.testng.Assert.*; @@ -557,7 +559,7 @@ public void testFindImagesAndFix_notModifiedPostData() throws Exception { } @DataProvider - public Object[][] inputlines() throws Exception { + public Object[][] inputLinesForDockerfile() throws Exception { return new Object[][]{ {"image1", "3", "FROM image1_blahblah", false}, {"image1", "3", " FROM image1_blahblah", false}, @@ -585,7 +587,7 @@ public Object[][] inputlines() throws Exception { }; } - @Test(dataProvider = "inputlines") + @Test(dataProvider = "inputLinesForDockerfile") public void testChangeIfDockerfileBaseImageLine(String img, String tag, String line, boolean expected) throws Exception { gitHubUtil = mock(GitHubUtil.class); @@ -594,6 +596,47 @@ public void testChangeIfDockerfileBaseImageLine(String img, String tag, expected); } + @DataProvider + public Object[][] inputLinesForDockerCompose() { + return new Object[][]{ + {"image1", "3", "image: image1_blahblah", false}, + {"image1", "3", " image: image1_blahblah", false}, + {"image1", "3", "image: image1_blahblah ", false}, + {"image1", "3", "image: image1_blahblah\t", false}, + {"image1", "3", "image: image1_blahblah:2", false}, + {"image2", "4", "image: image2_blahblah:latest", false}, + {"image4", "9", "image: image4:9", false}, + {"image5", "246", "image: image5_", false}, + {"image6", "26", "image: image7", false}, + {"image8", "245", "image: image8:245", false}, + {"image8", "245", "image: image8: 245", false}, + {"imageName", "7", "image: imageName:_asasds", true}, + {"imageName", "7", " image: imageName:asAA_aa.as_ss", true}, + {"imageName", "7", " image: imageName:Asdf@#ggg", false}, + {"imageName", "7", " image: imageName:$ITEST_TAG", false}, + {"imageName", "7", " image: imageName:latest", false}, + {"image3", "7", "image: image3 ", false}, + {"image3", "7", "\timage: image3\t", false}, + {"image7", "98", "image: image7:4", true}, + {"image7", "98", "image: image7: 4", true}, + {"image124516418023_1085-1-1248571", "7357", + "image: image124516418023_1085-1-1248571:18026809126359806124890356219518632048125", true}, + {"image", "1234", + "image: image:1234", false}, + {"image", "1234", + "image: image:1234", false}, + }; + } + + @Test(dataProvider = "inputLinesForDockerCompose") + public void testChangeIfDockerfileBaseImageLineForCompose(String img, String tag, + String line, boolean expected) { + gitHubUtil = mock(GitHubUtil.class); + dockerfileGitHubUtil = new DockerfileGitHubUtil(gitHubUtil); + assertEquals(dockerfileGitHubUtil.changeIfDockerfileBaseImageLine(img, tag, new StringBuilder(), line), + expected); + } + @Test public void testChangeIfDockerfileBaseImageLine_modifyingStringBuilder() throws Exception { gitHubUtil = mock(GitHubUtil.class); @@ -888,4 +931,25 @@ public void testCommentsWithNoDfiuParsedCorrectly(String line, String ignoreImag dockerfileGitHubUtil = new DockerfileGitHubUtil(gitHubUtil); assertEquals(dockerfileGitHubUtil.ignorePRCommentPresent(line, ignoreImageString), expectedResult); } + + @DataProvider + public Object[][] tagList() { + return new Object[][] { + {"12345", true}, + {"assdfASFF11", true}, + {"_asadsd", true}, + {".weww", false}, + {"Aswd@#asdas", false}, + {"ASDF_SSS-CCC.Ssd", true}, + {"-asasd", false}, + {" ", false} + }; + } + + @Test(dataProvider = "tagList") + public void testValidImageTag(String tag, Boolean expectedResult) { + gitHubUtil = mock(GitHubUtil.class); + dockerfileGitHubUtil = new DockerfileGitHubUtil(gitHubUtil); + assertEquals(dockerfileGitHubUtil.isValidImageTag(tag), expectedResult); + } } From 87f8fcc785378a28648b7e41da156bb69f27132c Mon Sep 17 00:00:00 2001 From: "pankaj.chugh" Date: Mon, 23 Jan 2023 19:14:54 +0530 Subject: [PATCH 5/6] Updated to support different branch name for different file type run --- .../model/GitForkBranch.java | 26 ++++++++++++++++--- .../process/GitHubPullRequestSender.java | 2 +- .../search/GitHubImageSearchTermList.java | 3 ++- .../subcommands/impl/All.java | 4 +-- .../subcommands/impl/Child.java | 2 +- .../subcommands/impl/Parent.java | 2 +- .../utils/Constants.java | 2 ++ .../model/GitForkBranchTest.java | 26 ++++++++++++++++--- .../process/ForkableRepoValidatorTest.java | 12 ++++----- .../utils/DockerfileGitHubUtilTest.java | 11 ++++---- 10 files changed, 65 insertions(+), 25 deletions(-) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/GitForkBranch.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/GitForkBranch.java index e1f7e97b..1532720c 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/GitForkBranch.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/GitForkBranch.java @@ -1,5 +1,8 @@ package com.salesforce.dockerfileimageupdate.model; +import com.salesforce.dockerfileimageupdate.utils.Constants; +import org.apache.commons.lang3.StringUtils; + /** * Converts an imageName to a branchName. Primary conversion necessary is : to - * Also support backward compatible method of specifying a branch @@ -9,11 +12,12 @@ */ public class GitForkBranch { private final String branchPrefix; + private final String branchSuffix; private final String imageName; private final String imageTag; private final boolean specifiedBranchOverride; - public GitForkBranch(String imageName, String imageTag, String specifiedBranch) { + public GitForkBranch(String imageName, String imageTag, String specifiedBranch, String filenamesSearchedFor) { this.imageTag = (imageTag == null || imageTag.trim().isEmpty()) ? "" : imageTag.trim(); this.imageName = (imageName == null || imageName.trim().isEmpty()) ? "" : imageName.trim(); if (specifiedBranch == null || specifiedBranch.trim().isEmpty()) { @@ -22,9 +26,11 @@ public GitForkBranch(String imageName, String imageTag, String specifiedBranch) } this.branchPrefix = this.imageName.replace(":", "-").toLowerCase(); this.specifiedBranchOverride = false; + this.branchSuffix = getBranchSuffix(StringUtils.isNotBlank(filenamesSearchedFor) ? filenamesSearchedFor.toLowerCase():filenamesSearchedFor); } else { this.branchPrefix = specifiedBranch; this.specifiedBranchOverride = true; + this.branchSuffix = ""; } } @@ -71,9 +77,9 @@ public boolean useSpecifiedBranchOverride() { */ public String getBranchName() { if (this.imageTag == null || this.imageTag.trim().isEmpty()) { - return this.branchPrefix; + return this.branchPrefix + this.branchSuffix; } else { - return this.branchPrefix + "-" + this.imageTag; + return this.branchPrefix + "-" + this.imageTag + this.branchSuffix; } } @@ -84,4 +90,18 @@ public String getImageName() { public String getImageTag() { return imageTag; } + + private String getBranchSuffix(String filenamesSearchedFor) { + // To avoid branch with same imageName-tag getting overwritten in case multiple runs(with different file types) + // on same image tag store, adding suffix to branch name + if (StringUtils.isBlank(filenamesSearchedFor)) { + return ""; + } else if (filenamesSearchedFor.contains(Constants.FILENAME_DOCKERFILE) && !filenamesSearchedFor.contains(Constants.FILENAME_DOCKER_COMPOSE)) { + return "_dockerfile"; + } else if (filenamesSearchedFor.contains(Constants.FILENAME_DOCKER_COMPOSE) && !filenamesSearchedFor.contains(Constants.FILENAME_DOCKERFILE)) { + return "_dockercompose"; + } else { + return ""; + } + } } \ No newline at end of file diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/process/GitHubPullRequestSender.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/process/GitHubPullRequestSender.java index 1c310b7a..bb49a8e6 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/process/GitHubPullRequestSender.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/process/GitHubPullRequestSender.java @@ -79,7 +79,7 @@ public Multimap forkRepositoriesFoundAndGetPathT log.info("Out of {} content search results processed, {} were deemed eligible for forking to yield {} repositories to fork.", totalContentsFound, contentsShouldFork, pathToDockerfilesInParentRepo.keys().size()); - log.info("Path to Dockerfiles in repos: {}", pathToDockerfilesInParentRepo); + log.info("Path to files in repos: {}", pathToDockerfilesInParentRepo); return pathToDockerfilesInParentRepo; } diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/search/GitHubImageSearchTermList.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/search/GitHubImageSearchTermList.java index fcd20c64..dee04300 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/search/GitHubImageSearchTermList.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/search/GitHubImageSearchTermList.java @@ -1,6 +1,7 @@ package com.salesforce.dockerfileimageupdate.search; import com.google.common.collect.ImmutableList; +import com.salesforce.dockerfileimageupdate.utils.Constants; import java.util.ArrayList; import java.util.List; @@ -95,7 +96,7 @@ static ProcessingState processDomainPartOfImage(String domain, String filenames) if (domain == null || domain.trim().isEmpty()) { return state; } else { - if (filenames.equalsIgnoreCase("Dockerfile")) { + if (filenames.equalsIgnoreCase(Constants.FILENAME_DOCKERFILE)) { state.addToCurrentTerm("FROM "); } if (domain.contains("-")) { diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java index c9970773..e0088672 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java @@ -83,7 +83,7 @@ protected Optional processImageWithTag(String image, String tag, Name GitForkBranch gitForkBranch = getGitForkBranch(image, tag, ns); String filenamesToSearch = ns.get(Constants.FILE_NAMES_TO_SEARCH); - log.info("Finding Dockerfiles with the image name {}...", image); + log.info("Finding files:{} with the image name {}...", filenamesToSearch, image); Optional>> contentsWithImage = this.dockerfileGitHubUtil.findFilesWithImage(image, orgsToIncludeInSearch, gitApiSearchLimit, filenamesToSearch); if (contentsWithImage.isPresent()) { @@ -138,7 +138,7 @@ protected GitHubPullRequestSender getPullRequestSender(DockerfileGitHubUtil dock } protected GitForkBranch getGitForkBranch(String image, String tag, Namespace ns){ - return new GitForkBranch(image, tag, ns.get(Constants.GIT_BRANCH)); + return new GitForkBranch(image, tag, ns.get(Constants.GIT_BRANCH), ns.get(Constants.FILE_NAMES_TO_SEARCH)); } protected PullRequests getPullRequests(){ diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Child.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Child.java index 6e12a1ef..9301b923 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Child.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Child.java @@ -56,7 +56,7 @@ public void execute(final Namespace ns, final DockerfileGitHubUtil dockerfileGit return; } - GitForkBranch gitForkBranch = new GitForkBranch(img, forceTag, branch); + GitForkBranch gitForkBranch = new GitForkBranch(img, forceTag, branch, ns.get(Constants.FILE_NAMES_TO_SEARCH)); PullRequestInfo pullRequestInfo = new PullRequestInfo(ns.get(Constants.GIT_PR_TITLE), gitForkBranch.getImageName(), diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java index 634d5871..f610f5a5 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java @@ -89,7 +89,7 @@ protected PullRequests getPullRequests(){ } protected GitForkBranch getGitForkBranch(Namespace ns){ - return new GitForkBranch(ns.get(Constants.IMG), ns.get(Constants.TAG), ns.get(Constants.GIT_BRANCH)); + return new GitForkBranch(ns.get(Constants.IMG), ns.get(Constants.TAG), ns.get(Constants.GIT_BRANCH), ns.get(Constants.FILE_NAMES_TO_SEARCH)); } protected GitHubPullRequestSender getPullRequestSender(DockerfileGitHubUtil dockerfileGitHubUtil, Namespace ns){ diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java index eb88861b..b81dbd6c 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/Constants.java @@ -48,4 +48,6 @@ private Constants() { public static final Duration DEFAULT_RATE_LIMIT_DURATION = Duration.ofMinutes(DEFAULT_RATE_LIMIT); //token adding rate(here:a token added every 2 minutes in the bucket) public static final Duration DEFAULT_TOKEN_ADDING_RATE = Duration.ofMinutes(DEFAULT_CONSUMING_TOKEN_RATE); + public static final String FILENAME_DOCKERFILE = "dockerfile"; + public static final String FILENAME_DOCKER_COMPOSE = "docker-compose"; } diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/GitForkBranchTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/GitForkBranchTest.java index 1f74163c..42a19aaf 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/GitForkBranchTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/GitForkBranchTest.java @@ -19,7 +19,7 @@ public Object[][] imageNameAndExpectedBranch() { @Test(dataProvider = "imageNameAndExpectedBranch") public void testGetBranchNameForImageTagCombos(String imageName, String imageTag, String expectedResult) { - assertEquals(new GitForkBranch(imageName, imageTag, "").getBranchName(), expectedResult); + assertEquals(new GitForkBranch(imageName, imageTag, "", "Dockerfile,docker-compose").getBranchName(), expectedResult); } @DataProvider @@ -35,7 +35,7 @@ public Object[][] imageNameAndSpecifiedBranches() { @Test(dataProvider = "imageNameAndSpecifiedBranches") public void testGetBranchNameForImageSpecifiedBranchCombos(String imageName, String imageTag, String specifiedBranch, String expectedResult) { - assertEquals(new GitForkBranch(imageName, imageTag, specifiedBranch).getBranchName(), expectedResult); + assertEquals(new GitForkBranch(imageName, imageTag, specifiedBranch, "Dockerfile,docker-compose").getBranchName(), expectedResult); } @DataProvider @@ -61,11 +61,29 @@ public Object[][] sameBranchOrImageNamePrefix() { @Test(dataProvider = "sameBranchOrImageNamePrefix") public void testIsSameBranchOrHasImageNamePrefix(String imageName, String imageTag, String specifiedBranch, String inputBranch, boolean expectedResult) { - assertEquals(new GitForkBranch(imageName, imageTag, specifiedBranch).isSameBranchOrHasImageNamePrefix(inputBranch), expectedResult); + assertEquals(new GitForkBranch(imageName, imageTag, specifiedBranch, "Dockerfile,docker-compose").isSameBranchOrHasImageNamePrefix(inputBranch), expectedResult); } @Test(expectedExceptions = IllegalArgumentException.class) public void testIsSameBranchOrHasImageNamePrefix() { - new GitForkBranch("", "1", ""); + new GitForkBranch("", "1", "", null); + } + + @DataProvider + public Object[][] imageNameSpecifiedBranchAndFilenameSearched() { + return new Object[][]{ + /*{"docker.io/some/container", "", "blah", "dockerfile", "blah"}, + {"127.0.0.1:443/some/container", "", "test", "dockerfile,docker-compose", "test"}, + {"docker.io/some/container", "123", "", "dockerfile", "docker.io/some/container-123_dockerfile"}, + {"docker.io/some/container", "123", "", "dockerfile,docker-compose", "docker.io/some/container-123"}, + {"docker.io/some/container", "123", "", "docker-compose", "docker.io/some/container-123_dockercompose"}, + {"docker.io/some/container", " ", null, "abcdef", "docker.io/some/container"},*/ + {"docker.io/some/container", null, "", "docker-compose", "docker.io/some/container_dockercompose"}, + }; + } + + @Test(dataProvider = "imageNameSpecifiedBranchAndFilenameSearched") + public void testGetBranchNameForFileNamesSearched(String imageName, String imageTag, String specifiedBranch, String filenameSearchedFor, String expectedResult) { + assertEquals(new GitForkBranch(imageName, imageTag, specifiedBranch, filenameSearchedFor).getBranchName(), expectedResult); } } \ No newline at end of file diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidatorTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidatorTest.java index cb8b8ac5..83c931d9 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidatorTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/process/ForkableRepoValidatorTest.java @@ -146,7 +146,7 @@ public void testContentHasChangesInDefaultBranch() throws InterruptedException, ForkableRepoValidator validator = new ForkableRepoValidator(dockerfileGitHubUtil); GHContent content = mock(GHContent.class); when(content.getPath()).thenReturn("/Dockerfile"); - GitForkBranch gitForkBranch = new GitForkBranch("someImage", "someTag", null); + GitForkBranch gitForkBranch = new GitForkBranch("someImage", "someTag", null, "Dockerfile"); when(dockerfileGitHubUtil.tryRetrievingContent(eq(repo), any(), any())).thenReturn(content); InputStream inputStream = new ByteArrayInputStream("FROM someImage".getBytes()); when(content.read()).thenReturn(inputStream); @@ -161,7 +161,7 @@ public void testInterruptedExceptionWhenRetrievingContentInDefaultBranch() throw ForkableRepoValidator validator = new ForkableRepoValidator(dockerfileGitHubUtil); GHContent content = mock(GHContent.class); when(content.getPath()).thenReturn("/Dockerfile"); - GitForkBranch gitForkBranch = new GitForkBranch("someImage", "someTag", null); + GitForkBranch gitForkBranch = new GitForkBranch("someImage", "someTag", null, "Dockerfile"); when(dockerfileGitHubUtil.tryRetrievingContent(eq(repo), any(), any())).thenThrow(new InterruptedException("some exception")); assertEquals(validator.contentHasChangesInDefaultBranch(repo, content, gitForkBranch), shouldForkResult()); @@ -176,7 +176,7 @@ public void testContentHasNoChangesInDefaultBranch() throws InterruptedException String searchContentPath = "/Dockerfile"; when(content.getPath()).thenReturn(searchContentPath); String imageName = "someImage"; - GitForkBranch gitForkBranch = new GitForkBranch(imageName, "someTag", null); + GitForkBranch gitForkBranch = new GitForkBranch(imageName, "someTag", null, "Dockerfile"); when(dockerfileGitHubUtil.tryRetrievingContent(eq(repo), any(), any())).thenReturn(content); InputStream inputStream = new ByteArrayInputStream("nochanges".getBytes()); when(content.read()).thenReturn(inputStream); @@ -193,7 +193,7 @@ public void testCouldNotFindContentInDefaultBranch() throws InterruptedException GHContent content = mock(GHContent.class); String searchContentPath = "/Dockerfile"; when(content.getPath()).thenReturn(searchContentPath); - GitForkBranch gitForkBranch = new GitForkBranch("someImage", "someTag", null); + GitForkBranch gitForkBranch = new GitForkBranch("someImage", "someTag", null, "Dockerfile,docker-compose"); when(dockerfileGitHubUtil.tryRetrievingContent(eq(repo), any(), any())).thenReturn(null); InputStream inputStream = new ByteArrayInputStream("FROM someImage".getBytes()); @@ -223,7 +223,7 @@ public void testHasNoChanges(String contentText, String imageName, String imageT ForkableRepoValidator validator = new ForkableRepoValidator(dockerfileGitHubUtil); GHContent content = mock(GHContent.class); InputStream inputStream = new ByteArrayInputStream(contentText.getBytes()); - GitForkBranch gitForkBranch = new GitForkBranch(imageName, imageTag, null); + GitForkBranch gitForkBranch = new GitForkBranch(imageName, imageTag, null,"docker-compose"); when(content.read()).thenReturn(inputStream); @@ -237,7 +237,7 @@ public void testHasNoChangesIfExceptionThrownDuringRead() throws IOException { GHRepository repo = mock(GHRepository.class); ForkableRepoValidator validator = new ForkableRepoValidator(dockerfileGitHubUtil); GHContent content = mock(GHContent.class); - GitForkBranch gitForkBranch = new GitForkBranch("name", "tag", null); + GitForkBranch gitForkBranch = new GitForkBranch("name", "tag", null, ""); when(content.read()).thenThrow(new IOException("failed on IO")); diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java index b2437a32..c049d6fa 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java @@ -118,8 +118,7 @@ public void testReturnPullRequestForBranch() { GHPullRequestQueryBuilder queryBuilder = getGHPullRequestQueryBuilder(imageName, Optional.of(ghPullRequest)); GHRepository parent = mock(GHRepository.class); when(parent.queryPullRequests()).thenReturn(queryBuilder); - GitForkBranch gitForkBranch = new GitForkBranch(imageName, "", null); - + GitForkBranch gitForkBranch = new GitForkBranch(imageName, "", null, "Dockerfile,docker-compose"); gitHubUtil = mock(GitHubUtil.class); dockerfileGitHubUtil = new DockerfileGitHubUtil(gitHubUtil); @@ -133,7 +132,7 @@ public void testNoPullRequestForBranch() { GHPullRequestQueryBuilder queryBuilder = getGHPullRequestQueryBuilder(imageName, Optional.empty()); GHRepository parent = mock(GHRepository.class); when(parent.queryPullRequests()).thenReturn(queryBuilder); - GitForkBranch gitForkBranch = new GitForkBranch(imageName, "", null); + GitForkBranch gitForkBranch = new GitForkBranch(imageName, "", null, "Dockerfile,docker-compose"); gitHubUtil = mock(GitHubUtil.class); @@ -782,7 +781,7 @@ public void testCreateOrUpdateForkBranchToParentDefaultHasBranch() throws IOExce when(parentBranch.getSHA1()).thenReturn(sha); when(parent.getBranch(defaultBranch)).thenReturn(parentBranch); GHRepository fork = mock(GHRepository.class); - GitForkBranch gitForkBranch = new GitForkBranch("imageName", "imageTag", null); + GitForkBranch gitForkBranch = new GitForkBranch("imageName", "imageTag", null, "Dockerfile"); when(gitHubUtil.repoHasBranch(fork, gitForkBranch.getBranchName())).thenReturn(true); GHRef returnedRef = mock(GHRef.class); when(fork.getRef(anyString())).thenReturn(returnedRef); @@ -804,7 +803,7 @@ public void testCreateOrUpdateForkBranchToParentDefaultDoesNotHaveBranch() throw when(parentBranch.getSHA1()).thenReturn(sha); when(parent.getBranch(defaultBranch)).thenReturn(parentBranch); GHRepository fork = mock(GHRepository.class); - GitForkBranch gitForkBranch = new GitForkBranch("imageName", "imageTag", null); + GitForkBranch gitForkBranch = new GitForkBranch("imageName", "imageTag", null, "Dockerfile"); when(gitHubUtil.repoHasBranch(fork, gitForkBranch.getBranchName())).thenReturn(false); dockerfileGitHubUtil.createOrUpdateForkBranchToParentDefault(parent, fork, gitForkBranch); @@ -819,7 +818,7 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio "tag", Constants.STORE, "store"); Namespace ns = new Namespace(nsMap); - GitForkBranch gitForkBranch = new GitForkBranch("image", "tag", null); + GitForkBranch gitForkBranch = new GitForkBranch("image", "tag", null, ""); Multimap pathToDockerfilesInParentRepo = HashMultimap.create(); pathToDockerfilesInParentRepo.put("repo1", new GitHubContentToProcess(null, null, "df11")); pathToDockerfilesInParentRepo.put("repo1", new GitHubContentToProcess(null, null, "df12")); From 696e7e840a817b20cafd89b2097519ed90a1de76 Mon Sep 17 00:00:00 2001 From: "pankaj.chugh" Date: Tue, 24 Jan 2023 10:59:34 +0530 Subject: [PATCH 6/6] Uncommented the Unit tests --- .../dockerfileimageupdate/model/GitForkBranchTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/GitForkBranchTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/GitForkBranchTest.java index 42a19aaf..ee565992 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/GitForkBranchTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/GitForkBranchTest.java @@ -72,13 +72,13 @@ public void testIsSameBranchOrHasImageNamePrefix() { @DataProvider public Object[][] imageNameSpecifiedBranchAndFilenameSearched() { return new Object[][]{ - /*{"docker.io/some/container", "", "blah", "dockerfile", "blah"}, + {"docker.io/some/container", "", "blah", "dockerfile", "blah"}, {"127.0.0.1:443/some/container", "", "test", "dockerfile,docker-compose", "test"}, {"docker.io/some/container", "123", "", "dockerfile", "docker.io/some/container-123_dockerfile"}, {"docker.io/some/container", "123", "", "dockerfile,docker-compose", "docker.io/some/container-123"}, {"docker.io/some/container", "123", "", "docker-compose", "docker.io/some/container-123_dockercompose"}, - {"docker.io/some/container", " ", null, "abcdef", "docker.io/some/container"},*/ - {"docker.io/some/container", null, "", "docker-compose", "docker.io/some/container_dockercompose"}, + {"docker.io/some/container", " ", null, "abcdef", "docker.io/some/container"}, + {"docker.io/some/container", null, "", "docker-compose", "docker.io/some/container_dockercompose"} }; }