Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR to fix https://github.com/salesforce/dockerfile-image-update/issues/236, to bring in support for sending PRs to docker-compose files #687

Merged
merged 7 commits into from
Jan 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions dockerfile-image-update/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-all</artifactId>
<version>1.10.19</version>
<artifactId>mockito-core</artifactId>
<version>4.11.0</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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()) {
Expand All @@ -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 = "";
}
}

Expand Down Expand Up @@ -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;
}
}

Expand All @@ -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 "";
}
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public Multimap<String, GitHubContentToProcess> 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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<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];
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,11 @@ 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.FILE_NAMES_TO_SEARCH);

log.info("Finding Dockerfiles with the image name {}...", image);
log.info("Finding files:{} with the image name {}...", filenamesToSearch, image);
Optional<List<PagedSearchIterable<GHContent>>> contentsWithImage =
this.dockerfileGitHubUtil.findFilesWithImage(image, orgsToIncludeInSearch, gitApiSearchLimit);
this.dockerfileGitHubUtil.findFilesWithImage(image, orgsToIncludeInSearch, gitApiSearchLimit, filenamesToSearch);
if (contentsWithImage.isPresent()) {
Iterator<PagedSearchIterable<GHContent>> it = contentsWithImage.get().iterator();
while (it.hasNext()){
Expand Down Expand Up @@ -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(){
Expand Down
Loading