diff --git a/.github/actions/systemtests/parse-comment/action.yml b/.github/actions/systemtests/parse-comment/action.yml index d92a6a868da..d5d7cacbd78 100644 --- a/.github/actions/systemtests/parse-comment/action.yml +++ b/.github/actions/systemtests/parse-comment/action.yml @@ -77,7 +77,14 @@ runs: // Auto-run smoke tests on PR workflow changes shouldRun = true; } else if (ev === 'issue_comment' && context.payload?.issue?.pull_request) { - shouldRun = /^\s*\/gha\s+run\b/i.test(body || ''); + // For issue comments on PRs, check if PR is closed first + const issueState = context.payload?.issue?.state; + if (issueState === 'closed') { + core.info(`PR #${context.payload.issue.number} is closed, skipping execution`); + shouldRun = false; + } else { + shouldRun = /^\s*\/gha\s+run\b/i.test(body || ''); + } } core.setOutput('shouldRun', String(shouldRun)); @@ -134,8 +141,8 @@ runs: }; const DEFAULTS = { - pipelines: 'regression,upgrade', - profiles: 'operators,operands,brokers-and-security,azp_kraft_upgrade,azp_kafka_upgrade', + pipelines: '', + profiles: '', featureGate: '+DummyFeatureGate', rbacScope: 'CLUSTER', installType: 'yaml', diff --git a/.github/docs/README.md b/.github/docs/README.md index 20951b1aac3..c99f26a33d3 100644 --- a/.github/docs/README.md +++ b/.github/docs/README.md @@ -1,4 +1,4 @@ -# Building Strimzi +# Build & Test Strimzi in GitHub Actions The following lines put together basic information how is Strimzi built and tested via GitHub Actions. Currently, our main build system still lives on Azure Pipelines and running builds and tests on GHA is in experimental phase. All the steps mentioned bellow re-use scripts from our [azure](../../.azure) directory and thus all the steps are more or less the same on both systems. @@ -69,6 +69,7 @@ System tests execution actions: - [set-defaults](../actions/utils/set-defaults) - [log-variables](../actions/utils/log-variables) - [parse-comment](../actions/systemtests/parse-comment) +- [determine-ref](../actions/determine-ref) Utils actions: - [check-permissions](../actions/utils/check-permissions) @@ -87,7 +88,8 @@ The actions also had to be put together into workflow as we have in Azure: - `operator-release pipeline` -> `operator-release workflow` ## Running system tests -With GitHub Actions we are now able to propagate a specific parameters to our e2e jobs which allow us to just run a subset of tests for example or run it against different kubernetes version etc. +With GitHub Actions we are now able to propagate a specific parameters to our e2e jobs. +That allow us to run a subset of tests for example or run it against different kubernetes version etc. ### Triggers There are two options how the workflows could be triggered - manually via GitHub UI, via issue comment. @@ -97,32 +99,40 @@ Comment for triggering the workflow has to starts with string `/gha run` and the The whole script that parse the trigger even is part of [parse-comment](../actions/systemtests/parse-comment) action. Currently, we have these parameters that can be passed through the comment: -| Name | Info | Default | -|-------------------------------|------------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------| -| pipeline | Name of the pipeline from [pipelines.yaml](../actions/systemtests/generate-matrix/pipelines.yaml) that wil be executed | regression,upgrade,performance | -| profile | Testing profile from pom that will be executed | regression,upgrade,performance | -| agent | Agent that will be used for a specific pipeline (see list of runners in Strimzi org config for more info) | Value set in [pipelines.yaml](../actions/systemtests/generate-matrix/pipelines.yaml) | -| strimzi_feature_gates | Which Strimzi Feature Gates will be used | Value set in [pipelines.yaml](../actions/systemtests/generate-matrix/pipelines.yaml) | -| strimzi_rbac_scope | RBAC scope for Strimzi | Value set in [pipelines.yaml](../actions/systemtests/generate-matrix/pipelines.yaml) | -| cluster_operator_install_type | How Strimzi will be installed during the tests | Value set in [pipelines.yaml](../actions/systemtests/generate-matrix/pipelines.yaml) | -| parallel | Number of tests that will be executed in parallel | Value set in [pipelines.yaml](../actions/systemtests/generate-matrix/pipelines.yaml) | -| architecture | Which architecture will be used (should match with agent arch) | Value set in [pipelines.yaml](../actions/systemtests/generate-matrix/pipelines.yaml) | -| groups | Which Junit5 groups will be executed | all | -| tests | Which Junit5 tests will be executed | all | -| kubeVersion | Used Kubernetes version as part of Kind/Minikube setup | The one set as default in setup scripts | -| kafkaVersion | Which Kafka version will be used in the tests | Default one from STs config | - -Note that parameters are passed only to a `custom` pipeline except `kafkaVersion` and `kubeVersion` that are used for all jobs. - -For trigger via GitHub UI you can specify `releaseVersion`, `kafkaVersion`, and `profile`. -The first two parameters are working in the same manner as in Azure. -`profile` is used to filter out all pipelines defined in [pipelines.yaml](../actions/systemtests/generate-matrix/pipelines.yaml) that contains one of the profile specified in comma separated list in `profile` parameter. +| Name | Info | Default | +|-------------------------------|------------------------------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------| +| pipeline | Name of the pipeline from [pipelines.yaml](../actions/generate-matrix/pipelines.yaml) that wil be executed | regression,upgrade | +| profile | Testing profile from [pipelines.yaml](../actions/generate-matrix/pipelines.yaml) that will be executed, it has to be defined in pom file | smoke | +| agent | Agent that will be used for a specific pipeline (see list of runners in Strimzi org config for more info) | Value set in [pipelines.yaml](../actions/generate-matrix/pipelines.yaml) | +| strimzi_feature_gates | Which Strimzi Feature Gates will be used | Value set in [pipelines.yaml](../actions/generate-matrix/pipelines.yaml) | +| strimzi_rbac_scope | RBAC scope for Strimzi | Value set in [pipelines.yaml](../actions/generate-matrix/pipelines.yaml) | +| cluster_operator_install_type | How Strimzi will be installed during the tests | Value set in [pipelines.yaml](../actions/generate-matrix/pipelines.yaml) | +| parallel | Number of tests that will be executed in parallel | Value set in [pipelines.yaml](../actions/generate-matrix/pipelines.yaml) | +| architecture | Which architecture will be used (should match with agent arch) | Value set in [pipelines.yaml](../actions/generate-matrix/pipelines.yaml) | +| groups | Which Junit5 groups will be executed | all | +| tests | Which Junit5 tests will be executed | all | +| kubeVersion | Used Kubernetes version as part of Kind/Minikube setup | The one set as default in setup scripts | +| kafkaVersion | Which Kafka version will be used in the tests | Default one from STs config | + +The process of parameter usage is as follows: +- `pipeline` has the highest priority. If `pipeline` is defined, the jobs will be loaded with data from [pipelines.yaml](../actions/generate-matrix/pipelines.yaml) that match specific _pipeline_. +- `profile` has the second-highest priority. If `profile` is defined, the jobs will be loaded with data from [pipelines.yaml](../actions/generate-matrix/pipelines.yaml) that match specific _profile_. +- `agent` is used only for a `custom` pipeline when either `pipeline` or `profile` are not specified via comment. +- `strimzi_feature_gates` is used only for a `custom` pipeline when either `pipeline` or `profile` are not specified via comment. +- `strimzi_rbac_scope` is used only for a `custom` pipeline when either `pipeline` or `profile` are not specified via comment. +- `cluster_operator_install_type` is used only for a `custom` pipeline when either `pipeline` or `profile` are not specified via comment. +- `parallel` is used only for a `custom` pipeline when either `pipeline` or `profile` are not specified via comment. +- `architecture` is used only for a `custom` pipeline when either `pipeline` or `profile` are not specified via comment. +- `groups` are passed directly to `mvn` command. It should be used mostly only with `custom` pipeline as otherwise a specific `pipeline` could have the `groups` excluded. +- `tests` are passed directly to `mvn` command. It should be used mostly only with `custom` pipeline as otherwise a specific `pipeline` could have the `tests` excluded. +- `kubeVersion` is used for setup `kind` for every running jobs. +- `kafkaVersion` is used for every running job. ### Matrix generation -Once the event is parsed the mechanism will decide whether Strimzi should be built or already existing images will be used (release rc for example). -After that job matrix will be generated [run-system-tests](../workflows/run-system-tests.yml) workflow will be invoked for each job defined in the matrix. +Once the event is parsed the mechanism will decide whether Strimzi should be built or already existing images will be used (release RC for example). +After that job matrix will be generated and [run-system-tests](../workflows/run-system-tests.yml) workflow will be invoked for each job defined in the matrix. -Matrix is generated by action [generate-matrix](../actions/systemtests/generate-matrix) either for custom pipeline mentioned above or for pipelines defined in [pipelines.yaml](../actions/systemtests/generate-matrix/pipelines.yaml). +Matrix is generated by action [generate-matrix](../actions/systemtests/generate-matrix) either by filtering pipelines/profiles defined in [pipelines.yaml](../actions/systemtests/generate-matrix/pipelines.yaml) or by creating a custom pipeline from passed params. ### Workflow depiction ![sts-workflow.png](sts-workflow.png) @@ -130,11 +140,16 @@ Matrix is generated by action [generate-matrix](../actions/systemtests/generate- ## Security To achieve a better security we agreed to not allow everyone to trigger the system tests pipelines in the similar way as we do in Azure. The main part in access restriction is [check-permissions](../actions/utils/check-permissions) action that basically check access rights of the user who triggered the pipeline. -If the user has at least write access or is part of Strimzi org, then it will allow the execution. +If the user has at least write access or is part of Strimzi org, then it will allow the execution. The action is used only from main branch which removes the possibility that a random user will simply change the actions/workflow and the use the resources on his own. Also keep in mind that workflow for system tests will be used from a specific branch (main/release-X) for `workflow_dispatch` and `issue_comment` events. Each first-time contributor has to be approved by one of the maintainers which should avoid us to hit some unexpected changes to the workflow to me merged into main. -Regarding secrets, the forks are not allowed to use secrets by default. -Every generated `GITHUB_TOKEN` has only read access to the repo/org without access to the secrets. +Regarding secrets, the forks are not allowed to use secrets by default. +Every generated `GITHUB_TOKEN` has only read access to the repo/org without access to the secrets. + +## Testing workflows and actions +Unit and integration tests invoked via [actions-tests.yml](../workflows/actions-tests.yml) workflow. +It uses files specified within [tests](../tests) folder and via [act](https://github.com/nektos/act) it tries to execute the actions and check the outputs. +Currently, we tests `check-permissions`, `generate-matrix`, and `parse-comment` actions. \ No newline at end of file diff --git a/.github/tests/events/parse-comment/issue_comment_11.json b/.github/tests/events/parse-comment/issue_comment_11.json new file mode 100644 index 00000000000..fa65cedd870 --- /dev/null +++ b/.github/tests/events/parse-comment/issue_comment_11.json @@ -0,0 +1,24 @@ +{ + "event_name": "issue_comment", + "action": "created", + "comment": { + "body": "/gha run tests=random-tests" + }, + "issue": { + "number": 42, + "pull_request": { + "head": { + "ref": "feature-123", + "sha": "1111111111111111111111111111111111111111", + "repo": { + "name": "test-repo", + "owner": { "login": "batman" } + } + } + } + }, + "repository": { + "name": "test-repo", + "owner": { "login": "batman" } + } +} \ No newline at end of file diff --git a/.github/tests/events/parse-comment/issue_comment_closed_issue.json b/.github/tests/events/parse-comment/issue_comment_closed_issue.json new file mode 100644 index 00000000000..c31d6f961ca --- /dev/null +++ b/.github/tests/events/parse-comment/issue_comment_closed_issue.json @@ -0,0 +1,15 @@ +{ + "event_name": "issue_comment", + "action": "created", + "comment": { + "body": "/gha run profile=operators,operands" + }, + "issue": { + "number": 123, + "state": "closed" + }, + "repository": { + "name": "test-repo", + "owner": { "login": "batman" } + } +} \ No newline at end of file diff --git a/.github/tests/events/parse-comment/issue_comment_closed_pr.json b/.github/tests/events/parse-comment/issue_comment_closed_pr.json new file mode 100644 index 00000000000..36c985d4b47 --- /dev/null +++ b/.github/tests/events/parse-comment/issue_comment_closed_pr.json @@ -0,0 +1,25 @@ +{ + "event_name": "issue_comment", + "action": "created", + "comment": { + "body": "/gha run pipeline=regression,upgrade" + }, + "issue": { + "number": 42, + "state": "closed", + "pull_request": { + "head": { + "ref": "feature-123", + "sha": "1111111111111111111111111111111111111111", + "repo": { + "name": "test-repo", + "owner": { "login": "batman" } + } + } + } + }, + "repository": { + "name": "test-repo", + "owner": { "login": "batman" } + } +} \ No newline at end of file diff --git a/.github/tests/events/parse-comment/issue_comment_closed_pr_random_comment.json b/.github/tests/events/parse-comment/issue_comment_closed_pr_random_comment.json new file mode 100644 index 00000000000..1b626c87df9 --- /dev/null +++ b/.github/tests/events/parse-comment/issue_comment_closed_pr_random_comment.json @@ -0,0 +1,25 @@ +{ + "event_name": "issue_comment", + "action": "created", + "comment": { + "body": "This is random comment" + }, + "issue": { + "number": 42, + "state": "closed", + "pull_request": { + "head": { + "ref": "feature-123", + "sha": "1111111111111111111111111111111111111111", + "repo": { + "name": "test-repo", + "owner": { "login": "batman" } + } + } + } + }, + "repository": { + "name": "test-repo", + "owner": { "login": "batman" } + } +} \ No newline at end of file diff --git a/.github/tests/scenarios/parse-comment.yaml b/.github/tests/scenarios/parse-comment.yaml index 77e6ee54d3f..d204cd7935d 100644 --- a/.github/tests/scenarios/parse-comment.yaml +++ b/.github/tests/scenarios/parse-comment.yaml @@ -39,7 +39,7 @@ scenarios: fixture: .github/tests/events/parse-comment/issue_comment_3.json expectations: pipelines: "" - profiles: "operators,operands,brokers-and-security,azp_kraft_upgrade,azp_kafka_upgrade" + profiles: "" tests: "" groups: "nodeport" kafka: "4.1.0" @@ -90,7 +90,7 @@ scenarios: fixture: .github/tests/events/parse-comment/issue_comment_6.json expectations: pipelines: "" - profiles: "operators,operands,brokers-and-security,azp_kraft_upgrade,azp_kafka_upgrade" + profiles: "" tests: "" groups: "" kafka: "latest" @@ -107,7 +107,7 @@ scenarios: fixture: .github/tests/events/parse-comment/issue_comment_7.json expectations: pipelines: "" - profiles: "operators,operands,brokers-and-security,azp_kraft_upgrade,azp_kafka_upgrade" + profiles: "" tests: "" groups: "" kafka: "latest" @@ -169,6 +169,23 @@ scenarios: kube: "latest" release: "latest" + - id: comment-random-tests + description: "Pass tests param without specifying profile or pipeline" + event: issue_comment + fixture: .github/tests/events/parse-comment/issue_comment_11.json + expectations: + pipelines: "" + profiles: "" + tests: "random-tests" + groups: "" + kafka: "latest" + strimzi_feature_gates: "" + strimzi_rbac_scope: "" + cluster_operator_install_type: "yaml" + should_run: "true" + kube: "latest" + release: "latest" + - id: dispatch-default description: "Workflow dispatch with default values" event: workflow_dispatch @@ -233,4 +250,55 @@ scenarios: cluster_operator_install_type: "yaml" should_run: "false" kube: "latest" + release: "latest" + + - id: comment-closed-pr + description: "Comment on closed PR should not run" + event: issue_comment + fixture: .github/tests/events/parse-comment/issue_comment_closed_pr.json + expectations: + pipelines: "regression,upgrade" + profiles: "" + tests: "" + groups: "" + kafka: "latest" + strimzi_feature_gates: "" + strimzi_rbac_scope: "" + cluster_operator_install_type: "yaml" + should_run: "false" + kube: "latest" + release: "latest" + + - id: comment-closed-issue + description: "Comment on closed issue should not run" + event: issue_comment + fixture: .github/tests/events/parse-comment/issue_comment_closed_issue.json + expectations: + pipelines: "" + profiles: "operators,operands" + tests: "" + groups: "" + kafka: "latest" + strimzi_feature_gates: "" + strimzi_rbac_scope: "" + cluster_operator_install_type: "yaml" + should_run: "false" + kube: "latest" + release: "latest" + + - id: comment-closed-pr-random-comment + description: "Comment on closed issue should not run" + event: issue_comment + fixture: .github/tests/events/parse-comment/issue_comment_closed_pr_random_comment.json + expectations: + pipelines: "" + profiles: "" + tests: "" + groups: "" + kafka: "latest" + strimzi_feature_gates: "" + strimzi_rbac_scope: "" + cluster_operator_install_type: "yaml" + should_run: "false" + kube: "latest" release: "latest" \ No newline at end of file diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index 7e7a6b4e32f..8d52bc33947 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -59,13 +59,9 @@ jobs: - name: Determine checkout ref id: determine_ref uses: strimzi/strimzi-kafka-operator/.github/actions/utils/determine-ref@main - - uses: actions/checkout@v5 - with: - ref: ${{ steps.determine_ref.outputs.ref }} - # TODO - think about this how to handle it after check-rights - name: Parse Comment id: parse - uses: ./.github/actions/systemtests/parse-comment + uses: strimzi/strimzi-kafka-operator/.github/actions/systemtests/parse-comment@main with: # Parameters from manual trigger via UI releaseVersion: ${{ github.event.inputs.releaseVersion }}