From 27f90a4d52e93b83167686195d745fe4682fdb5c Mon Sep 17 00:00:00 2001 From: Christoph Pirkl Date: Thu, 21 Nov 2024 16:50:18 +0100 Subject: [PATCH] Improve usability: add available jobs to error message --- .github/workflows/ci-build.yml | 86 ++++++++++++++++++- .project-keeper.yml | 6 ++ .../config/ProjectKeeperConfigReader.java | 5 -- .../files/CiBuildWorkflowGenerator.java | 4 +- .../validators/files/GitHubWorkflow.java | 5 ++ .../files/GitHubWorkflowStepCustomizer.java | 24 +++++- .../config/ProjectKeeperConfigReaderTest.java | 48 ++++++----- .../GitHubWorkflowStepCustomizerTest.java | 44 +++++++++- .../config/workflow/StepCustomization.java | 7 +- 9 files changed, 193 insertions(+), 36 deletions(-) diff --git a/.github/workflows/ci-build.yml b/.github/workflows/ci-build.yml index 41b11332..88d54afd 100644 --- a/.github/workflows/ci-build.yml +++ b/.github/workflows/ci-build.yml @@ -9,7 +9,7 @@ on: pull_request: null workflow_dispatch: null jobs: - build: + build-and-test: runs-on: ubuntu-latest defaults: run: { @@ -119,6 +119,88 @@ jobs: path: '${{ steps.build-pk-verify.outputs.release-artifacts }}', retention-days: 5 } + - name: Configure link check + id: configure-link-check + run: | + mkdir -p ./target + echo '{"aliveStatusCodes": [429, 200], "ignorePatterns": [' \ + '{"pattern": "^https?://(www|dev).mysql.com/"},' \ + '{"pattern": "^https?://(www.)?opensource.org"}' \ + '{"pattern": "^https?://(www.)?eclipse.org"}' \ + '{"pattern": "^https?://projects.eclipse.org"}' \ + ']}' > ./target/broken_links_checker.json + - uses: gaurav-nelson/github-action-markdown-link-check@v1 + id: run-link-check + with: { + use-quiet-mode: yes, + use-verbose-mode: yes, + config-file: ./target/broken_links_checker.json + } + next-java-compatibility: + runs-on: ubuntu-latest + defaults: + run: { + shell: bash + } + permissions: { + contents: read + } + concurrency: { + group: '${{ github.workflow }}-${{ github.ref }}', + cancel-in-progress: true + } + steps: + - name: Checkout the repository + id: checkout + uses: actions/checkout@v4 + with: { + fetch-depth: 0 + } + - name: Set up JDK 21 + id: setup-java + uses: actions/setup-java@v4 + with: { + distribution: temurin, + java-version: '21', + cache: maven + } + - { + name: Run tests and build with Maven 21, + run: mvn --batch-mode clean package -DtrimStackTrace=false -Djava.version=21 + } + build: + needs: [ + build-and-test, + next-java-compatibility + ] + runs-on: ubuntu-latest + defaults: + run: { + shell: bash + } + permissions: { + contents: read, + issues: read + } + outputs: { + release-required: '${{ steps.check-release.outputs.release-required }}' + } + steps: + - name: Checkout the repository + id: checkout + uses: actions/checkout@v4 + with: { + fetch-depth: 0 + } + - name: Set up JDKs + id: setup-java + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: |- + 11 + 17 + cache: maven - name: Check if release is needed id: check-release if: ${{ github.ref == 'refs/heads/main' }} @@ -127,7 +209,7 @@ jobs: echo "### ✅ Release preconditions met, start release" >> "$GITHUB_STEP_SUMMARY" echo "release-required=true" >> "$GITHUB_OUTPUT" else - echo "### 🛑 Release precondition not met, skipping release" >> "$GITHUB_STEP_SUMMARY" + echo "### 🛑 Not all release preconditions met, skipping release" >> "$GITHUB_STEP_SUMMARY" echo "See log output for details." >> "$GITHUB_STEP_SUMMARY" echo "release-required=false" >> "$GITHUB_OUTPUT" fi diff --git a/.project-keeper.yml b/.project-keeper.yml index 9f0bd284..8535ebea 100644 --- a/.project-keeper.yml +++ b/.project-keeper.yml @@ -89,6 +89,7 @@ build: - name: "ci-build.yml" stepCustomizations: - action: INSERT_AFTER + job: build-and-test stepId: setup-java content: name: Set up Go @@ -98,12 +99,14 @@ build: go-version: "1.22" cache-dependency-path: .project-keeper.yml - action: INSERT_AFTER + job: build-and-test stepId: setup-go content: name: Install Go tools id: install-go-tools run: go install github.com/google/go-licenses@v1.6.0 - action: REPLACE + job: build-and-test stepId: build-pk-verify content: name: Run tests and build with Maven @@ -115,12 +118,14 @@ build: env: GITHUB_TOKEN: ${{ github.token }} # Required for integration tests - action: INSERT_AFTER + job: build-and-test stepId: sonar-analysis content: name: Run project-keeper itself id: build-pk-verify run: mvn --batch-mode com.exasol:project-keeper-maven-plugin:verify --projects . - action: INSERT_AFTER + job: build-and-test stepId: build-pk-verify content: name: Verify that metrics.json was created @@ -134,6 +139,7 @@ build: - name: "dependencies_check.yml" stepCustomizations: - action: INSERT_AFTER + job: report_security_issues stepId: setup-jdks content: name: Install Project Keeper diff --git a/project-keeper/src/main/java/com/exasol/projectkeeper/config/ProjectKeeperConfigReader.java b/project-keeper/src/main/java/com/exasol/projectkeeper/config/ProjectKeeperConfigReader.java index d92ceeab..ea5f3823 100644 --- a/project-keeper/src/main/java/com/exasol/projectkeeper/config/ProjectKeeperConfigReader.java +++ b/project-keeper/src/main/java/com/exasol/projectkeeper/config/ProjectKeeperConfigReader.java @@ -170,11 +170,6 @@ private List convertSteps(final List st } private StepCustomization convertStep(final RawStepCustomization step) { - if (step.job == null || step.job.isBlank()) { - throw new IllegalArgumentException(ExaError.messageBuilder("E-PK-CORE-208") - .message("Missing job in step customization of file {{config file}}.", CONFIG_FILE_NAME) - .mitigation("Add job to the step customization.").toString()); - } if (step.action == null) { throw new IllegalArgumentException(ExaError.messageBuilder("E-PK-CORE-200") .message("Missing action in step customization of file {{config file}}.", CONFIG_FILE_NAME) diff --git a/project-keeper/src/main/java/com/exasol/projectkeeper/validators/files/CiBuildWorkflowGenerator.java b/project-keeper/src/main/java/com/exasol/projectkeeper/validators/files/CiBuildWorkflowGenerator.java index db8c6f60..19cff0d0 100644 --- a/project-keeper/src/main/java/com/exasol/projectkeeper/validators/files/CiBuildWorkflowGenerator.java +++ b/project-keeper/src/main/java/com/exasol/projectkeeper/validators/files/CiBuildWorkflowGenerator.java @@ -54,7 +54,7 @@ private FileTemplate createTemplate(final FileTemplateFromResource template, fin return new ContentCustomizingTemplate(template, new GitHubWorkflowCustomizer( // javaVersionCustomizer(), // [impl->dsn~customize-build-process.ci-build~0] - new GitHubWorkflowStepCustomizer(customizations), + new GitHubWorkflowStepCustomizer(workflowName, customizations), // [impl->dsn~customize-build-process.ci-build.environment~1] new GitHubWorkflowEnvironmentCustomizer(buildJobId, environmentName))); } @@ -119,7 +119,7 @@ private FileTemplate createCustomizedWorkflow(final String workflowName, templateCustomizer.accept(template); final List customizations = findCustomizations(workflowName); return new ContentCustomizingTemplate(template, new GitHubWorkflowCustomizer(javaVersionCustomizer(), - new GitHubWorkflowStepCustomizer(customizations))); + new GitHubWorkflowStepCustomizer(workflowName, customizations))); } enum CiTemplateType { diff --git a/project-keeper/src/main/java/com/exasol/projectkeeper/validators/files/GitHubWorkflow.java b/project-keeper/src/main/java/com/exasol/projectkeeper/validators/files/GitHubWorkflow.java index 201ff444..de6983f7 100644 --- a/project-keeper/src/main/java/com/exasol/projectkeeper/validators/files/GitHubWorkflow.java +++ b/project-keeper/src/main/java/com/exasol/projectkeeper/validators/files/GitHubWorkflow.java @@ -197,4 +197,9 @@ private Optional getOptionalString(final String key) { return Optional.ofNullable((String) rawStep.get(key)); } } + + public Job getJob(final Optional jobId) { + // TODO Auto-generated method stub + throw new UnsupportedOperationException("Unimplemented method 'getJob'"); + } } diff --git a/project-keeper/src/main/java/com/exasol/projectkeeper/validators/files/GitHubWorkflowStepCustomizer.java b/project-keeper/src/main/java/com/exasol/projectkeeper/validators/files/GitHubWorkflowStepCustomizer.java index b16af519..c2bf40d4 100644 --- a/project-keeper/src/main/java/com/exasol/projectkeeper/validators/files/GitHubWorkflowStepCustomizer.java +++ b/project-keeper/src/main/java/com/exasol/projectkeeper/validators/files/GitHubWorkflowStepCustomizer.java @@ -2,24 +2,44 @@ import java.util.List; +import com.exasol.errorreporting.ExaError; +import com.exasol.projectkeeper.config.ProjectKeeperConfigReader; import com.exasol.projectkeeper.shared.config.workflow.StepCustomization; import com.exasol.projectkeeper.validators.files.GitHubWorkflow.Job; class GitHubWorkflowStepCustomizer implements GitHubWorkflowCustomizer.WorkflowCustomizer { + private final String workflowName; private final List customizations; - GitHubWorkflowStepCustomizer(final List customizations) { + GitHubWorkflowStepCustomizer(final String workflowName, final List customizations) { + this.workflowName = workflowName; this.customizations = customizations; } @Override public void applyCustomization(final GitHubWorkflow workflow) { for (final StepCustomization customization : customizations) { - final Job job = workflow.getJob(customization.getJobId()); + final Job job = getJob(workflow, customization); applyCustomization(job, customization); } } + private Job getJob(final GitHubWorkflow workflow, final StepCustomization customization) { + if (customization.getJobId().isPresent()) { + return workflow.getJob(customization.getJobId().get()); + } + final List allJobs = workflow.getJobs(); + if (allJobs.size() == 1) { + return allJobs.get(0); + } + throw new IllegalStateException(ExaError.messageBuilder("E-PK-CORE-208") + .message("Missing job in step customization of workflow {{workflow}} in file {{config file}}.", + workflowName, ProjectKeeperConfigReader.CONFIG_FILE_NAME) + .mitigation("Add job with one of the following values: {{available jobs}}.", + allJobs.stream().map(Job::getId).toList()) + .toString()); + } + // [impl->dsn~customize-build-process.insert-step-after~0] // [impl->dsn~customize-build-process.replace-step~0] private void applyCustomization(final Job job, final StepCustomization customization) { diff --git a/project-keeper/src/test/java/com/exasol/projectkeeper/config/ProjectKeeperConfigReaderTest.java b/project-keeper/src/test/java/com/exasol/projectkeeper/config/ProjectKeeperConfigReaderTest.java index c51dae15..86cfe2a5 100644 --- a/project-keeper/src/test/java/com/exasol/projectkeeper/config/ProjectKeeperConfigReaderTest.java +++ b/project-keeper/src/test/java/com/exasol/projectkeeper/config/ProjectKeeperConfigReaderTest.java @@ -201,27 +201,6 @@ static Stream invalidConfig() { - stepCustomizations: """, equalTo( "E-PK-CORE-199: Missing workflow name in file '.project-keeper.yml'. Add a workflow name to the workflow configuration.")), - Arguments.of("workflow: missing job id", """ - build: - workflows: - - name: ci-build.yml - stepCustomizations: - - action: REPLACE - content: - id: new-step - """, equalTo( - "E-PK-CORE-208: Missing job in step customization of file '.project-keeper.yml'. Add job to the step customization.")), - Arguments.of("workflow: empty job id", """ - build: - workflows: - - name: ci-build.yml - stepCustomizations: - - job: '' - action: REPLACE - content: - id: new-step - """, equalTo( - "E-PK-CORE-208: Missing job in step customization of file '.project-keeper.yml'. Add job to the step customization.")), Arguments.of("workflow: missing step id", """ build: workflows: @@ -340,6 +319,33 @@ void readWorkflowCustomization() throws IOException { .build())); } + @Test + void jobIdIsOptional() throws IOException { + writeProjectKeeperConfig(""" + build: + workflows: + - name: ci-build.yml + stepCustomizations: + - action: REPLACE + stepId: step-id-to-replace + content: + name: New Step + id: new-step + run: echo 'new step' + env: + ENV_VARIABLE: 'value' + """); + assertThat(readConfig().getCiBuildConfig().getWorkflows(), contains(CustomWorkflow.builder() // + .workflowName("ci-build.yml") // + .addStep(StepCustomization.builder() // + .type(StepCustomization.Type.REPLACE) // + .stepId("step-id-to-replace") // + .step(WorkflowStep.createStep(Map.of("name", "New Step", "id", "new-step", "run", + "echo 'new step'", "env", Map.of("ENV_VARIABLE", "value")))) // + .build()) // + .build())); + } + @ParameterizedTest @CsvSource({ "REPLACE, REPLACE", "INSERT_AFTER, INSERT_AFTER" }) void readWorkflowStepAction(final String action, final StepCustomization.Type expected) throws IOException { diff --git a/project-keeper/src/test/java/com/exasol/projectkeeper/validators/files/GitHubWorkflowStepCustomizerTest.java b/project-keeper/src/test/java/com/exasol/projectkeeper/validators/files/GitHubWorkflowStepCustomizerTest.java index 165cf187..1df0a8af 100644 --- a/project-keeper/src/test/java/com/exasol/projectkeeper/validators/files/GitHubWorkflowStepCustomizerTest.java +++ b/project-keeper/src/test/java/com/exasol/projectkeeper/validators/files/GitHubWorkflowStepCustomizerTest.java @@ -19,6 +19,8 @@ class GitHubWorkflowStepCustomizerTest { + private static final String WORKFLOW_NAME = "workflow.yml"; + @Test void startsWithGeneratedComment() { final String yaml = getCustomizedContent(""" @@ -100,6 +102,46 @@ void replaceStep() { assertThat(getStepIds(workflow.getJob("build")), contains("step0", "custom-step", "step2")); } + @Test + void missingJobConfigurationWithSingleJob() { + final WorkflowStep newStep = WorkflowStep + .createStep(Map.of("name", "Custom Step", "id", "custom-step", "run", "echo custom-step")); + final GitHubWorkflow workflow = validate(""" + jobs: + build: + steps: + - name: step0 + id: step0 + - name: step1 + id: replaced-step + - name: step2 + id: step2 + """, StepCustomization.builder().jobId(null).type(Type.REPLACE).stepId("replaced-step").step(newStep) + .build()); + assertThat(getStepIds(workflow.getJob("build")), contains("step0", "custom-step", "step2")); + } + + @Test + void missingJobConfigurationWithMultipleJobsFails() { + final WorkflowStep newStep = WorkflowStep + .createStep(Map.of("name", "Custom Step", "id", "custom-step", "run", "echo custom-step")); + final IllegalStateException exception = assertThrows(IllegalStateException.class, () -> validate(""" + jobs: + build1: + steps: + - name: step0 + id: step0 + build2: + steps: + - name: step0 + id: step0 + """, StepCustomization.builder().jobId(null).type(Type.REPLACE).stepId("replaced-step").step(newStep) + .build())); + assertThat(exception.getMessage(), equalTo( + "E-PK-CORE-208: Missing job in step customization of workflow 'workflow.yml' in file '.project-keeper.yml'. " + + "Add job with one of the following values: ['build1', 'build2'].")); + } + @Test void replaceStepNextStepHasMissingId() { final WorkflowStep newStep = WorkflowStep @@ -221,7 +263,7 @@ private GitHubWorkflow validate(final String workflowTemplate, final StepCustomi } private String getCustomizedContent(final String workflowTemplate, final StepCustomization... customizations) { - return new GitHubWorkflowCustomizer(new GitHubWorkflowStepCustomizer(asList(customizations))) + return new GitHubWorkflowCustomizer(new GitHubWorkflowStepCustomizer(WORKFLOW_NAME, asList(customizations))) .customizeContent(workflowTemplate); } } diff --git a/shared-model-classes/src/main/java/com/exasol/projectkeeper/shared/config/workflow/StepCustomization.java b/shared-model-classes/src/main/java/com/exasol/projectkeeper/shared/config/workflow/StepCustomization.java index 0f0c2318..92a5fa8d 100644 --- a/shared-model-classes/src/main/java/com/exasol/projectkeeper/shared/config/workflow/StepCustomization.java +++ b/shared-model-classes/src/main/java/com/exasol/projectkeeper/shared/config/workflow/StepCustomization.java @@ -1,6 +1,7 @@ package com.exasol.projectkeeper.shared.config.workflow; import java.util.Objects; +import java.util.Optional; /** * This class defines how a GitHub workflow should be modified by inserting or replacing a step. @@ -13,7 +14,7 @@ public final class StepCustomization { private final WorkflowStep step; private StepCustomization(final Builder builder) { - this.jobId = Objects.requireNonNull(builder.jobId, "jobId"); + this.jobId = builder.jobId; this.type = Objects.requireNonNull(builder.type, "type"); this.stepId = Objects.requireNonNull(builder.stepId, "stepId"); this.step = Objects.requireNonNull(builder.step, "step"); @@ -24,8 +25,8 @@ private StepCustomization(final Builder builder) { * * @return job ID */ - public String getJobId() { - return jobId; + public Optional getJobId() { + return Optional.ofNullable(jobId); } /**