diff --git a/.github/workflows/dependencies_update.yml b/.github/workflows/dependencies_update.yml index 64ea7a56..b75fdc23 100644 --- a/.github/workflows/dependencies_update.yml +++ b/.github/workflows/dependencies_update.yml @@ -61,15 +61,22 @@ jobs: - name: Project Keeper Fix for updated Project Keeper version # Calling PK fix a second time is necessary because the first invocation potentially updated PK itself. # So we need to run PK fix again with the latest PK version. + # [impl->dsn~dependency-updater.workflow.start-pk-fix~1] run: | mvn --batch-mode com.exasol:project-keeper-maven-plugin:fix --projects . - name: Generate PR comment id: pr-comment + # [impl->dsn~dependency-updater.workflow.pull-request-trigger-ci-build~1] run: | echo 'comment<> "$GITHUB_OUTPUT" - echo 'This Pull Request was created by `dependencies_update.yml` workflow' >> "$GITHUB_OUTPUT" - echo $CREATED_ISSUES | jq --raw-output '. | "Closes " + .issue_url + " (" + .cve + ")"' >> "$GITHUB_OUTPUT" + echo 'This Pull Request was created by [`dependencies_update.yml`](https://github.com/exasol/project-keeper/blob/main/project-keeper/src/main/resources/templates/.github/workflows/dependencies_update.yml) workflow.' >> "$GITHUB_OUTPUT" + echo 'It updates dependencies to fix the following vulnerabilities:' >> "$GITHUB_OUTPUT" + echo $CREATED_ISSUES | jq --raw-output '. | "* Closes " + .issue_url + " (" + .cve + ")"' >> "$GITHUB_OUTPUT" + echo >> "$GITHUB_OUTPUT" + echo '# ⚠️ This PR does not trigger CI workflows by default ⚠️' >> "$GITHUB_OUTPUT" + echo 'Please click the **Close pull request** button and then **Reopen pull request** to trigger running checks.' >> "$GITHUB_OUTPUT" + echo 'See https://github.com/exasol/project-keeper/issues/534 for details.' >> "$GITHUB_OUTPUT" echo 'EOF' >> "$GITHUB_OUTPUT" env: CREATED_ISSUES: ${{ inputs.vulnerability_issues }} @@ -90,19 +97,25 @@ jobs: if: ${{ startsWith(github.ref, 'refs/heads/' ) }} run: | branch_name=$(git rev-parse --abbrev-ref HEAD) - echo "Current branch: $branch_name, local changes:" + echo "Current branch: $branch_name" + echo "git diff --stat" git diff --stat + echo "git diff --numstat" git diff --numstat + echo "git diff --name-status" + git diff --name-status + echo "Adding untracked files:" + git add . --verbose --all echo "Committing changes..." - git commit --all --message "Update dependencies" + git commit --message "🔐 Update dependencies to fix vulnerabilities" echo "Pushing branch $branch_name..." - git push --set-upstream origin $branch_name + git push --set-upstream origin "$branch_name" echo "Done." - name: Create pull request if: ${{ github.ref == 'refs/heads/main' }} run: | - gh pr create --base main --title "Update dependencies" --body "$COMMENT" + gh pr create --base main --title "🔐 Update dependencies to fix vulnerabilities" --body "$COMMENT" env: COMMENT: ${{ steps.pr-comment.outputs.comment }} GH_TOKEN: ${{ github.token }} diff --git a/doc/changes/changelog.md b/doc/changes/changelog.md index ddff9ab8..f0683740 100644 --- a/doc/changes/changelog.md +++ b/doc/changes/changelog.md @@ -1,5 +1,6 @@ # Changes +* [4.1.0](changes_4.1.0.md) * [4.0.0](changes_4.0.0.md) * [3.0.1](changes_3.0.1.md) * [3.0.0](changes_3.0.0.md) diff --git a/doc/changes/changes_4.1.0.md b/doc/changes/changes_4.1.0.md new file mode 100644 index 00000000..6e8e21f8 --- /dev/null +++ b/doc/changes/changes_4.1.0.md @@ -0,0 +1,55 @@ +# Project Keeper 4.1.0, released 2024-??-?? + +Code name: Trigger PR CI build + +## Summary + +This release updates the comment of the dependency updating Pull Request to instruct the user how to trigger the CI build for the Pull Request. + +## Bugfixes + +* #534: Fixed running checks for dependency update PR + +## Dependency Updates + +### Project Keeper Core + +#### Compile Dependency Updates + +* Updated `com.exasol:project-keeper-shared-model-classes:4.0.0` to `4.1.0` + +#### Runtime Dependency Updates + +* Updated `com.exasol:project-keeper-java-project-crawler:4.0.0` to `4.1.0` + +#### Test Dependency Updates + +* Updated `com.exasol:project-keeper-shared-test-setup:4.0.0` to `4.1.0` + +### Project Keeper Command Line Interface + +#### Compile Dependency Updates + +* Updated `com.exasol:project-keeper-core:4.0.0` to `4.1.0` + +#### Test Dependency Updates + +* Updated `com.exasol:project-keeper-shared-test-setup:4.0.0` to `4.1.0` + +### Project Keeper Maven Plugin + +#### Compile Dependency Updates + +* Updated `com.exasol:project-keeper-core:4.0.0` to `4.1.0` + +### Project Keeper Java Project Crawler + +#### Compile Dependency Updates + +* Updated `com.exasol:project-keeper-shared-model-classes:4.0.0` to `4.1.0` + +### Project Keeper Shared Test Setup + +#### Compile Dependency Updates + +* Updated `com.exasol:project-keeper-shared-model-classes:4.0.0` to `4.1.0` diff --git a/doc/requirements/design.md b/doc/requirements/design.md index 662ef963..e0d3a2ac 100644 --- a/doc/requirements/design.md +++ b/doc/requirements/design.md @@ -662,7 +662,7 @@ Rationale: * After updating dependency versions, the workflow runs PK `fix` to update the dependencies section in the changes file. * PK `fix` will potentially update PK to a newer version in `pom.xml` (see [`dsn~self-update~1`](#project-keeper-self-update)). So the workflow runs PK `fix` a second time (using the updated PK version) to update all generated files. --Needs: impl +Needs: impl Covers: * [`dsn~dependency-updater.workflow.generate~1`](#generate-dependencies_updateyml-workflow) @@ -696,6 +696,23 @@ Note: Implementing this in a workflow makes it hard to do integration tests. We Covers: * [`dsn~dependency-updater.workflow.generate~1`](#generate-dependencies_updateyml-workflow) +##### `dependencies_update.yml` Workflow Trigger Pull Request CI Build Manually +`dsn~dependency-updater.workflow.pull-request-trigger-ci-build~1` + +PK generates the `dependencies_update.yml` workflow so that it adds a note to the [created Pull Request](#dependencies_updateyml-workflow-creates-a-pull-request) that instructs the user how to trigger the CI build for the Pull Request. + +Rationale: + +* When the `dependencies_update.yml` creates the PR with dependency updates using the default `GITHUB_TOKEN`, the checks for this PR don't run initially. + * See the [GitHub documentation](https://docs.github.com/en/actions/using-workflows/triggering-a-workflow#triggering-a-workflow-from-a-workflow) about triggering workflows with `GITHUB_TOKEN`. +* This [list](https://github.com/peter-evans/create-pull-request/blob/main/docs/concepts-guidelines.md#triggering-further-workflow-runs) suggests possible workarounds for triggering the PR checks: + * Tell the user to modify the PR (e.g. by closing and reopening it) + * Use an alterative GitHub token for creating the PR, i.e. a Personal Access Token (PAT) or a GitHub App Token +* We decided to accept the inconvenience of the manual step and avoid the trouble of configuring the additional token. +* If necessary we can chose a different implementation later. + +Needs: impl + #### Generate `release.yml` workflow `dsn~release-workflow.generate~1` diff --git a/parent-pom/pom.xml b/parent-pom/pom.xml index 799c0ec2..22d4152a 100644 --- a/parent-pom/pom.xml +++ b/parent-pom/pom.xml @@ -28,7 +28,7 @@ - 4.0.0 + 4.1.0 17 3.9.6 diff --git a/pom.xml b/pom.xml index bdbf8e18..4ee051c8 100644 --- a/pom.xml +++ b/pom.xml @@ -41,7 +41,7 @@ org.itsallcode openfasttrace-maven-plugin - 1.7.0 + 1.8.0 trace-requirements diff --git a/project-keeper/src/main/java/com/exasol/projectkeeper/dependencyupdate/ChangesFileUpdater.java b/project-keeper/src/main/java/com/exasol/projectkeeper/dependencyupdate/ChangesFileUpdater.java index 959bb487..0a51fd88 100644 --- a/project-keeper/src/main/java/com/exasol/projectkeeper/dependencyupdate/ChangesFileUpdater.java +++ b/project-keeper/src/main/java/com/exasol/projectkeeper/dependencyupdate/ChangesFileUpdater.java @@ -1,10 +1,11 @@ package com.exasol.projectkeeper.dependencyupdate; +import static java.util.function.Predicate.not; import static java.util.stream.Collectors.joining; import java.nio.file.Path; -import java.util.*; -import java.util.function.Predicate; +import java.util.ArrayList; +import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -57,25 +58,23 @@ private Updater(final ChangesFile changesFile, final List vulnera private ChangesFile update() { return changesFile.toBuilder() // - .codeName(append(changesFile.getCodeName(), ", ", createCodeName())) // + .codeName(createCodeName()) // .summary(createSummary()) // .sections(createOtherSections()) // .build(); } private String createCodeName() { + String codeName = changesFile.getCodeName() != null ? changesFile.getCodeName().trim() : ""; + codeName += codeName.isBlank() ? "Fixed " : ", fixed "; if (vulnerabilities.size() == 1) { final Vulnerability vulnerability = vulnerabilities.get(0); - return "Fixed vulnerability " + vulnerability.cve() + " in " + vulnerability.coordinates(); + codeName += "vulnerability " + vulnerability.cve() + " in " + vulnerability.coordinates(); + } else { + codeName += "vulnerabilities " + + vulnerabilities.stream().map(Vulnerability::cve).collect(joining(", ")); } - return "Fixed vulnerabilities " + vulnerabilities.stream().map(Vulnerability::cve).collect(joining(", ")); - } - - private String append(final String currentText, final String separator, final String newText) { - return Optional.ofNullable(currentText) // - .filter(Predicate.not(String::isBlank)) // - .map(text -> text + separator + newText) // - .orElse(newText); + return codeName; } private ChangesFileSection createSummary() { @@ -107,10 +106,22 @@ private List createOtherSections() { final List sections = new ArrayList<>(); sections.add(buildSecuritySection()); sections.addAll(changesFile.getSections().stream() - .filter(section -> !section.getHeading().equals(SECURITY_SECTION_HEADER)).toList()); + .filter(section -> !section.getHeading().equals(SECURITY_SECTION_HEADER)) // + .filter(not(this::isDefaultFeaturesSection)).toList()); return sections; } + private boolean isDefaultFeaturesSection(final ChangesFileSection section) { + if (!section.getHeading().equals(ChangesFileValidator.FEATURES_SECTION)) { + return false; + } + return section.getContent().stream() // + .map(String::trim) // + .filter(not(String::isEmpty)) // + .filter(line -> !line.equals(ChangesFileValidator.FIXED_ISSUE_TEMPLATE)) // + .findAny().isEmpty(); + } + private ChangesFileSection buildSecuritySection() { final Builder securitySectionBuilder = getExistingSecuritySection(); securitySectionBuilder.addLines(vulnerabilities.stream().map(this::createIssueFixesEntry).toList()); diff --git a/project-keeper/src/main/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileSection.java b/project-keeper/src/main/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileSection.java index 1091464a..1efa678e 100644 --- a/project-keeper/src/main/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileSection.java +++ b/project-keeper/src/main/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileSection.java @@ -60,7 +60,8 @@ public boolean equals(final Object obj) { @Override public String toString() { - return heading + "\n" + String.join("\n", this.content); + final String text = heading + "\n" + String.join("\n", this.content); + return text.replaceAll("(?m)\n{3,}", "\n\n"); } /** diff --git a/project-keeper/src/main/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileValidator.java b/project-keeper/src/main/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileValidator.java index ba21db83..218c3897 100644 --- a/project-keeper/src/main/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileValidator.java +++ b/project-keeper/src/main/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileValidator.java @@ -17,6 +17,10 @@ * Validator that checks the existence of the doc/changes/changes_X.X.X.md file for the current project's version. */ public class ChangesFileValidator extends AbstractFileValidator { + /** Section heading for the "Features" section used in the template. */ + public static final String FEATURES_SECTION = "## Features"; + /** Template for a fixed issue in the changes file. */ + public static final String FIXED_ISSUE_TEMPLATE = "* ISSUE_NUMBER: description"; private final String projectName; private final List sources; private final String projectVersion; @@ -77,8 +81,7 @@ private ChangesFile getTemplate() { .releaseDate(releaseDate) // .codeName("") // .summary(ChangesFileSection.builder("## Summary").build()) - .addSection(ChangesFileSection.builder("## Features").addLines("", "* ISSUE_NUMBER: description", "") - .build()) // + .addSection(ChangesFileSection.builder(FEATURES_SECTION).addLines("", FIXED_ISSUE_TEMPLATE, "").build()) // .build(); return fixSections(changesFile); } diff --git a/project-keeper/src/main/resources/templates/.github/workflows/dependencies_update.yml b/project-keeper/src/main/resources/templates/.github/workflows/dependencies_update.yml index d19b11c0..7d663692 100644 --- a/project-keeper/src/main/resources/templates/.github/workflows/dependencies_update.yml +++ b/project-keeper/src/main/resources/templates/.github/workflows/dependencies_update.yml @@ -56,15 +56,22 @@ jobs: - name: Project Keeper Fix for updated Project Keeper version # Calling PK fix a second time is necessary because the first invocation potentially updated PK itself. # So we need to run PK fix again with the latest PK version. + # [impl->dsn~dependency-updater.workflow.start-pk-fix~1] run: | mvn --batch-mode com.exasol:project-keeper-maven-plugin:fix --projects . - name: Generate PR comment id: pr-comment + # [impl->dsn~dependency-updater.workflow.pull-request-trigger-ci-build~1] run: | echo 'comment<> "$GITHUB_OUTPUT" - echo 'This Pull Request was created by `dependencies_update.yml` workflow' >> "$GITHUB_OUTPUT" - echo $CREATED_ISSUES | jq --raw-output '. | "Closes " + .issue_url + " (" + .cve + ")"' >> "$GITHUB_OUTPUT" + echo 'This Pull Request was created by [`dependencies_update.yml`](https://github.com/exasol/project-keeper/blob/main/project-keeper/src/main/resources/templates/.github/workflows/dependencies_update.yml) workflow.' >> "$GITHUB_OUTPUT" + echo 'It updates dependencies to fix the following vulnerabilities:' >> "$GITHUB_OUTPUT" + echo $CREATED_ISSUES | jq --raw-output '. | "* Closes " + .issue_url + " (" + .cve + ")"' >> "$GITHUB_OUTPUT" + echo >> "$GITHUB_OUTPUT" + echo '# ⚠️ This PR does not trigger CI workflows by default ⚠️' >> "$GITHUB_OUTPUT" + echo 'Please click the **Close pull request** button and then **Reopen pull request** to trigger running checks.' >> "$GITHUB_OUTPUT" + echo 'See https://github.com/exasol/project-keeper/issues/534 for details.' >> "$GITHUB_OUTPUT" echo 'EOF' >> "$GITHUB_OUTPUT" env: CREATED_ISSUES: ${{ inputs.vulnerability_issues }} @@ -85,19 +92,25 @@ jobs: if: ${{ startsWith(github.ref, 'refs/heads/' ) }} run: | branch_name=$(git rev-parse --abbrev-ref HEAD) - echo "Current branch: $branch_name, local changes:" + echo "Current branch: $branch_name" + echo "git diff --stat" git diff --stat + echo "git diff --numstat" git diff --numstat + echo "git diff --name-status" + git diff --name-status + echo "Adding untracked files:" + git add . --verbose --all echo "Committing changes..." - git commit --all --message "Update dependencies" + git commit --message "🔐 Update dependencies to fix vulnerabilities" echo "Pushing branch $branch_name..." - git push --set-upstream origin $branch_name + git push --set-upstream origin "$branch_name" echo "Done." - name: Create pull request if: ${{ github.ref == 'refs/heads/main' }} run: | - gh pr create --base main --title "Update dependencies" --body "$COMMENT" + gh pr create --base main --title "🔐 Update dependencies to fix vulnerabilities" --body "$COMMENT" env: COMMENT: ${{ steps.pr-comment.outputs.comment }} GH_TOKEN: ${{ github.token }} diff --git a/project-keeper/src/test/java/com/exasol/projectkeeper/dependencyupdate/ChangesFileUpdaterTest.java b/project-keeper/src/test/java/com/exasol/projectkeeper/dependencyupdate/ChangesFileUpdaterTest.java index e7f22ad0..19db7db3 100644 --- a/project-keeper/src/test/java/com/exasol/projectkeeper/dependencyupdate/ChangesFileUpdaterTest.java +++ b/project-keeper/src/test/java/com/exasol/projectkeeper/dependencyupdate/ChangesFileUpdaterTest.java @@ -9,13 +9,13 @@ import java.nio.file.Path; import java.util.List; +import java.util.Optional; import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.*; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -59,9 +59,9 @@ static Stream codeName() { testCase(null, List.of(vulnerability(1), vulnerability(2)), "Fixed vulnerabilities cve-1, cve-2"), // testCase("", List.of(vulnerability(1), vulnerability(2)), "Fixed vulnerabilities cve-1, cve-2"), // testCase("Existing Text", List.of(vulnerability(1)), - "Existing Text, Fixed vulnerability cve-1 in coordinates-1"), // + "Existing Text, fixed vulnerability cve-1 in coordinates-1"), // testCase("Existing Text", List.of(vulnerability(1), vulnerability(2)), - "Existing Text, Fixed vulnerabilities cve-1, cve-2") // + "Existing Text, fixed vulnerabilities cve-1, cve-2") // ); } @@ -158,6 +158,33 @@ void securitySectionUpdated(final TestCase> test) { assertThat(updatedSections, equalTo(test.expected)); } + @ParameterizedTest + @ValueSource(strings = { "* ISSUE_NUMBER: description", " * ISSUE_NUMBER: description", + "* ISSUE_NUMBER: description ", "\t* ISSUE_NUMBER: description", "* ISSUE_NUMBER: description\n" }) + void defaultFeatureSectionRemoved(final String fixedIssuesTemplate) { + final Builder builder = ChangesFile.builder(); + builder.addSection( + ChangesFileSection.builder("## Features").addLine("").addLine(fixedIssuesTemplate).addLine("").build()); + final List updatedSections = getUpdatedChanges(List.of(vulnerability(1)), builder.build()) + .getSections(); + final Optional featuresSection = updatedSections.stream() + .filter(section -> section.getHeading().equals("## Features")).findAny(); + assertThat("Features section was removed", featuresSection.isEmpty(), is(true)); + } + + @Test + void nonDefaultFeatureSectionNotRemoved() { + final Builder builder = ChangesFile.builder(); + final ChangesFileSection originalFeaturesSection = ChangesFileSection.builder("## Features").addLine("") + .addLine("* #1: Fixed a bug").addLine("* ISSUE_NUMBER: description").addLine("").build(); + builder.addSection(originalFeaturesSection); + final List updatedSections = getUpdatedChanges(List.of(vulnerability(1)), builder.build()) + .getSections(); + final Optional featuresSection = updatedSections.stream() + .filter(section -> section.getHeading().equals("## Features")).findAny(); + assertThat(featuresSection.get(), equalTo(originalFeaturesSection)); + } + private static ChangesFileSection securitySection(final String... lines) { return section("## Security", lines); } diff --git a/project-keeper/src/test/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileSectionTest.java b/project-keeper/src/test/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileSectionTest.java index 929afba6..72130d7b 100644 --- a/project-keeper/src/test/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileSectionTest.java +++ b/project-keeper/src/test/java/com/exasol/projectkeeper/validators/changesfile/ChangesFileSectionTest.java @@ -1,5 +1,8 @@ package com.exasol.projectkeeper.validators.changesfile; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasToString; + import org.junit.jupiter.api.Test; import nl.jqno.equalsverifier.EqualsVerifier; @@ -10,4 +13,45 @@ class ChangesFileSectionTest { void equalsContract() { EqualsVerifier.forClass(ChangesFileSection.class).verify(); } + + @Test + void toStringWithoutBody() { + assertThat(ChangesFileSection.builder("## Heading").build(), hasToString("## Heading\n")); + } + + @Test + void toStringWithSingleLineBody() { + assertThat(ChangesFileSection.builder("## Heading").addLine("content line").build(), + hasToString("## Heading\ncontent line")); + } + + @Test + void toStringWithLineContainingNewline() { + assertThat(ChangesFileSection.builder("## Heading").addLine("content\nline").build(), + hasToString("## Heading\ncontent\nline")); + } + + @Test + void toStringEmptyLineRenderedAsNewline() { + assertThat(ChangesFileSection.builder("## Heading").addLine("").addLine("content line").build(), + hasToString("## Heading\n\ncontent line")); + } + + @Test + void toStringWithMultiLineBody() { + assertThat(ChangesFileSection.builder("## Heading").addLine("content line 1").addLine("content line 2").build(), + hasToString("## Heading\ncontent line 1\ncontent line 2")); + } + + @Test + void toStringCollapsesTwoNewlines() { + assertThat(ChangesFileSection.builder("## Heading").addLine("").addLine("").addLine("content line").build(), + hasToString("## Heading\n\ncontent line")); + } + + @Test + void toStringCollapsesThreeNewlines() { + assertThat(ChangesFileSection.builder("## Heading").addLine("").addLine("").addLine("").addLine("content line") + .build(), hasToString("## Heading\n\ncontent line")); + } }