diff --git a/dockerfile-image-update/pom.xml b/dockerfile-image-update/pom.xml index 41b63a51..7a475ff7 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 40c350ca..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 @@ -49,7 +49,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() { @@ -83,14 +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."); - return parser; } 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/model/ImageKeyValuePair.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java new file mode 100644 index 00000000..fbba57fd --- /dev/null +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePair.java @@ -0,0 +1,197 @@ +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."; + + /** + * The name of the base image + */ + private final String baseImageName; + /** + * The tag of the base image + */ + private final String tag; + /** + * 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 + * See {@code isImageKeyValuePair} to ensure you're passing a valid line in. + * + * @param imageKeyValuePair an Image Key value pair from a docker-compose file e.g: image: imageName:imageTag + */ + 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; + } + // 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(":", ""). + 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; + } + } else { + baseImageName = null; + tag = null; + } + } + + /** + * Internal API to get a new ComposeImageValuePair from an existing object + * @param baseImageName baseImageName to add + * @param tag tag to add + * @param comments comments to add + */ + 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 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 + * @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) + && 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 image instruction with the new image tag + */ + public ImageKeyValuePair getImageKeyValuePairWithNewTag(String newTag) { + return new ImageKeyValuePair(baseImageName, newTag, comments, spaces); + } + + /** + * 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)) { + return composeImageKeyValueLine.trim().startsWith(ImageKeyValuePair.IMAGE); + } + return false; + } + + /** + * @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(spaces + IMAGE); + stringBuilder.append(": "); + stringBuilder.append(baseImageName); + if (hasTag()) { + stringBuilder.append(String.format(":%s", tag.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 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..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 @@ -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/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 e77baf53..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; @@ -20,12 +21,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 +91,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(Constants.FILENAME_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 299fa652..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 @@ -81,10 +81,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.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); + this.dockerfileGitHubUtil.findFilesWithImage(image, orgsToIncludeInSearch, gitApiSearchLimit, filenamesToSearch); if (contentsWithImage.isPresent()) { Iterator> it = contentsWithImage.get().iterator(); while (it.hasNext()){ @@ -137,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 951920ff..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 @@ -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,6 +59,7 @@ public void execute(final Namespace ns, DockerfileGitHubUtil dockerfileGitHubUti + "be skipped.", Constants.SKIP_PR_CREATION); return; } + PullRequests pullRequests = getPullRequests(); GitHubPullRequestSender pullRequestSender = getPullRequestSender(dockerfileGitHubUtil, ns); GitForkBranch gitForkBranch = getGitForkBranch(ns); @@ -64,7 +67,7 @@ public void execute(final Namespace ns, DockerfileGitHubUtil dockerfileGitHubUti 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(); @@ -86,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 61f8a562..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 @@ -39,6 +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 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; @@ -47,5 +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/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java index defd2db0..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 @@ -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; @@ -88,12 +90,24 @@ 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 - search.filename("Dockerfile"); + // This will work in OR mode i.e, either filename is Dockerfile or 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(); for (Map.Entry org : orgsToIncludeOrExclude.entrySet()){ @@ -112,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(); @@ -146,7 +160,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); @@ -168,7 +182,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); @@ -177,13 +192,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()); } @@ -279,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); } } @@ -304,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. @@ -324,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); @@ -334,13 +349,22 @@ 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) && + DockerfileGitHubUtil.isValidImageTag(imageKeyValuePair.getTag())) { + imageKeyValuePair = imageKeyValuePair.getImageKeyValuePairWithNewTag(tag); + modified = true; + } + outputLine = imageKeyValuePair.toString(); } stringBuilder.append(outputLine).append("\n"); return modified; } /** - * 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 @@ -365,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 @@ -453,11 +477,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<>(); @@ -468,7 +493,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() @@ -544,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.trim()).matches()) { + log.warn("{} is not a valid value for image tag. So will be ignored!", tag); + return false; + } else if (tag.trim().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/GitForkBranchTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/GitForkBranchTest.java index 1f74163c..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 @@ -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/model/ImageKeyValuePairTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java new file mode 100644 index 00000000..422763b0 --- /dev/null +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/model/ImageKeyValuePairTest.java @@ -0,0 +1,220 @@ +package com.salesforce.dockerfileimageupdate.model; + +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", " image: dockerimage:3"}, + {"\timage: dockerimage:3", "image: dockerimage:3"}, + {"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.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[][] 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", false}, + {"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 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..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 @@ -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.*; @@ -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()); @@ -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}, }; } @@ -219,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); @@ -233,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/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/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/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 d4c44a10..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,10 +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 com.salesforce.dockerfileimageupdate.utils.Constants.RATE_LIMIT_PR_CREATION; -import static org.mockito.Matchers.*; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; import static org.testng.Assert.assertEquals; @@ -39,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); @@ -64,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); - when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).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()); + doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, + contentsWithImage, gitForkBranch, dockerfileGitHubUtil, + rateLimiter); + when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), + any(), any())).thenReturn(optionalContentsWithImageList); + 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 @@ -116,8 +121,8 @@ public void testAllCommandSkipsSendingPRsIfSearchReturnsEmpty() throws Exception Optional>> optionalContentsWithImageList = Optional.empty(); doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil, rateLimiter); - when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).thenReturn(optionalContentsWithImageList); - + when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), + anyInt(), anyString())).thenReturn(optionalContentsWithImageList); all.execute(ns, dockerfileGitHubUtil); verify(all).getGitForkBranch(anyString(), anyString(), any()); @@ -165,8 +170,8 @@ public void testAllCommandSkipsSendingPRsIfSearchRaisesException() throws Except PagedSearchIterable contentsWithImage = mock(PagedSearchIterable.class); doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil, rateLimiter); - 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); verify(all).getGitForkBranch(anyString(), anyString(), any()); @@ -212,8 +217,8 @@ public void testAllCommandSkipsSendingPRsIfPRCreationRaisesException() throws Ex Optional>> optionalContentsWithImageList = Optional.of(contentsWithImageList); doThrow(new IOException()).when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil, null); - when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).thenReturn(optionalContentsWithImageList); - + when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), + any(), any())).thenReturn(optionalContentsWithImageList); all.execute(ns, dockerfileGitHubUtil); verify(all).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 d9cc484f..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; @@ -20,7 +22,6 @@ import java.util.Map; import static com.salesforce.dockerfileimageupdate.utils.Constants.*; -import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.*; /** @@ -49,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()); @@ -58,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 0f8fe9b0..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,11 +16,11 @@ 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.*; -import static com.salesforce.dockerfileimageupdate.utils.Constants.RATE_LIMIT_PR_CREATION; -import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; import static org.testng.Assert.*; @@ -107,7 +107,7 @@ public void testParentCommandSuccessful() throws Exception { when(parent.getPullRequests()).thenReturn(pullRequests); doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil, null); - 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); @@ -142,16 +142,19 @@ public void testParentCommandSuccessfulForS3ImageStore() throws Exception { when(parent.getPullRequests()).thenReturn(pullRequests); doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil, rateLimiter); - when(dockerfileGitHubUtil.getGHContents(anyString(), anyString(), anyInt())).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)); + when(dockerfileGitHubUtil.getGHContents(any(), any(), + any(), any())).thenReturn(optionalContentsWithImageList); + 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 @@ -219,7 +222,8 @@ public void testParentCommandThrowsException() throws Exception { Optional>> optionalContentsWithImageList = Optional.of(contentsWithImageList); doThrow(new InterruptedException("Exception")).when(pullRequests).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil, rateLimiter); - 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 6364d208..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 @@ -13,11 +13,13 @@ 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; 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 +30,7 @@ import java.util.HashMap; import static com.salesforce.dockerfileimageupdate.model.PullRequestInfo.DEFAULT_TITLE; -import static org.mockito.Matchers.*; +import static com.salesforce.dockerfileimageupdate.utils.ResultsProcessor.REPOS_SKIPPED_MESSAGE; import static org.mockito.Mockito.*; import static org.testng.Assert.*; @@ -54,6 +56,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"); @@ -115,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); @@ -130,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); @@ -232,7 +234,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 +253,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 +295,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 +330,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); } @@ -556,7 +558,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}, @@ -584,7 +586,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); @@ -593,6 +595,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); @@ -674,10 +717,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 @@ -692,10 +735,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 @@ -738,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); @@ -760,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); @@ -775,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")); @@ -803,11 +846,15 @@ 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); + RateLimiter rateLimiter = mock(RateLimiter.class); 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") @@ -826,7 +873,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).createPullReq(eq(parentRepo), @@ -883,4 +930,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); + } } 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.*; 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