Updated to support docker-compose DFIU update

Author: pchugh17

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;
     }
 

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,25 +97,27 @@ 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;
     }
 
     /**
      * 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);
-        }
-    }
 }

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<String> getSearchTerms(String image) {
+    public static List<String> 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 {

dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java

@@ -81,7 +81,7 @@ protected Optional<Exception> 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<List<PagedSearchIterable<GHContent>>> contentsWithImage =

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);

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;

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<List<PagedSearchIterable<GHContent>>> findFilesWithImage(
         if (image.substring(image.lastIndexOf(' ') + 1).length() <= 1) {
             throw new IOException("Invalid image name.");
         }
-        List<String> terms = GitHubImageSearchTermList.getSearchTerms(image);
-        log.info("Searching for {} with terms: {}", image, terms);
+        List<String> terms = GitHubImageSearchTermList.getSearchTerms(image, filenamesToSearch);
+        log.info("Searching for {}, in files: {}, with terms: {}", image, filenamesToSearch, terms);
         terms.forEach(search::q);
         PagedSearchIterable<GHContent> 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;
+    }
 }

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}

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<String> expectedResult) {
-        List<String> searchTerms = GitHubImageSearchTermList.getSearchTerms(image);
+        List<String> searchTerms = GitHubImageSearchTermList.getSearchTerms(image, "Dockerfile");
         assertEquals(joinListByComma(searchTerms), joinListByComma(expectedResult));
     }
 
@@ -44,7 +44,7 @@ private String joinListByComma(List<String> list) {
 
     @Test(dataProvider = "imageAndParts")
     public void testGetSearchTermsSize(String image, List<String> expectedResult) {
-        List<String> searchTerms = GitHubImageSearchTermList.getSearchTerms(image);
+        List<String> 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<String> expectedProcessedTerms) {
-        GitHubImageSearchTermList.ProcessingState state = GitHubImageSearchTermList.processDomainPartOfImage(domain);
+        GitHubImageSearchTermList.ProcessingState state = GitHubImageSearchTermList.processDomainPartOfImage(domain, "Dockerfile");
         assertEquals(joinListByComma(state.terms), joinListByComma(expectedProcessedTerms));
         assertEquals(state.getCurrentTerm(), expectedCurrentTerm);
     }