You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "mumrah (via GitHub)" <gi...@apache.org> on 2024/08/20 14:33:39 UTC
[PR] MINOR: Create GH workflow for public forks [kafka]
mumrah opened a new pull request, #16936:
URL: https://github.com/apache/kafka/pull/16936
Github has different behavior for PRs within apache/kafka and those from public forks. For security reasons, ASF Infra disallows forks to trigger workflows using the `pull_request` event (as this would expose repository secrets). Instead, we can use the `pull_request_target` event which triggers when a fork creates a PR. This event will cause the workflow to run using the code of the PR target (i.e., trunk).
Per the ASF Infra policy https://infra.apache.org/github-actions-policy.html, we must take care not to inject secrets into `pull_request_target` workflows. For this reason, I've created a separate "Public Pull Requests" workflow that does not public Develocity build scans.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Separate GH workflows for PRs and trunk [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #16936:
URL: https://github.com/apache/kafka/pull/16936#issuecomment-2299663238
> That makes sense. BTW, how to announce this preview feature to developers to get enough feedback/evaluation? Send email to dev channel or add it to PULL_REQUEST_TEMPLATE.md?
Both of those sound good. Once we get it working :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Separate GH workflows for PRs and trunk [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16936:
URL: https://github.com/apache/kafka/pull/16936#issuecomment-2299576760
> The purpose of the "gh-" filter is to give developers a way to opt-in to Github actions while we are evaluating it. It won't affect the Jenkins build.
That makes sense. BTW, how to announce this preview feature to developers to get enough feedback/evaluation? Send email to dev channel or add it to `PULL_REQUEST_TEMPLATE.md`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Separate GH workflows for PRs and trunk [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #16936:
URL: https://github.com/apache/kafka/pull/16936#issuecomment-2299395186
@chia7712 I've updated this to have two workflows.
trunk
-----
* runs on each commit pushed to trunk
* runs compile check for 8, 11, 17, and 21
* runs the full test suite for 11 and 17
* publishes build scan to develocity
* writes to gradle cache
* lets previous action run finish
PR
---
* runs on each PR (opened, synchronized, reopened)
* runs compile check for 8, 11, 17, and 21
* runs the full test suite for 11 and 17
* publishes build scan to develocity only if PR from apache/kafka
* doesn't write to gradle cache (reads from it)
* only runs test suite if branch name is "gh-*"
* interrupts any action run for the current PR
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Separate GH workflows for PRs and trunk [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #16936:
URL: https://github.com/apache/kafka/pull/16936#issuecomment-2299895780
@chia7712 FYI after merging this, I saw that https://github.com/apache/kafka/pull/16943 needed to manually approved before it could run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Separate GH workflows for PRs and trunk [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #16936:
URL: https://github.com/apache/kafka/pull/16936#discussion_r1723730979
##########
.github/workflows/pr.yml:
##########
@@ -68,35 +50,17 @@ jobs:
fail-fast: false
matrix:
java: [ 11, 17 ]
+ if: contains(github.ref, 'gh-') # For now, only run tests on "gh-*" PRs
name: JUnit tests Java ${{ matrix.java }}
steps:
- - name: Checkout code
- uses: actions/checkout@v4
- with:
- persist-credentials: false
- - name: Setup Java
- uses: actions/setup-java@v4
- with:
- distribution: temurin
- java-version: ${{ matrix.java }}
- name: Setup Gradle
- uses: gradle/actions/setup-gradle@v4
- env:
- GRADLE_BUILD_ACTION_CACHE_DEBUG_ENABLED: true
+ uses: ./.github/actions/setup-gradle
with:
- gradle-version: wrapper
- develocity-access-key: ${{ secrets.GE_ACCESS_TOKEN }}
- # Only write to the cache from trunk
- #cache-read-only: ${{ github.ref != 'refs/heads/trunk' }}
- cache-read-only: false
- # Cache downloaded JDKs in addition to the default directories.
- gradle-home-cache-includes: |
- caches
- notifications
- jdks
- cache-cleanup: on-success
- cache-overwrite-existing: true
+ java-version: ${{ matrix.java }}
+ gradle-cache-read-only: true
- name: Test
+ # Publish build scans for now. This will only work on PRs from apache/kafka, not public forks.
Review Comment:
Do we open PRs from apache/kafka? it seems almost all PRs are from public forks.
Also, the failed tests in PR phase are expected. Maybe publishing the scans in CI phase is more acceptable since that is what running in code base.
##########
.github/workflows/pr.yml:
##########
@@ -32,33 +34,13 @@ jobs:
java: [8, 21]
Review Comment:
in the https://github.com/apache/kafka/pull/16936#issuecomment-2299395186, you say PR will run compile check for all JDKs
##########
.github/workflows/ci.yml:
##########
@@ -0,0 +1,65 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: CI
+
+on:
+ push:
+ branches:
+ - 'trunk'
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+ cancel-in-progress: false # Let each trunk commit finish a build
+
+jobs:
+ validate:
+ runs-on: ubuntu-latest
+ strategy:
+ fail-fast: false
+ matrix:
+ java: [8, 11, 17, 21]
+ name: Compile and Check Java ${{ matrix.java }}
+ steps:
+ - name: Setup Gradle
+ uses: ./.github/actions/setup-gradle
+ with:
+ java-version: ${{ matrix.java }}
+ gradle-cache-read-only: false
+ - name: Compile and validate
+ run: ./gradlew --build-cache --info --scan check -x test
+
+ test:
+ needs: validate
+ runs-on: ubuntu-latest
+ strategy:
+ fail-fast: false
+ matrix:
+ java: [ 11, 17 ]
+ name: JUnit tests Java ${{ matrix.java }}
+ steps:
+ - name: Setup Gradle
+ uses: ./.github/actions/setup-gradle
+ with:
+ java-version: ${{ matrix.java }}
+ gradle-cache-read-only: false
+ - name: Test
+ timeout-minutes: 180 # 3 hours
+ run: |
+ ./gradlew --build-cache --scan --continue \
+ -PtestLoggingEvents=started,passed,skipped,failed \
+ -PignoreFailures=true -PmaxParallelForks=2 \
+ -PmaxTestRetries=1 -PmaxTestRetryFailures=10 \
Review Comment:
Maybe we don't need retries in CI? That can helps us to find out more flaky
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Separate GH workflows for PRs and trunk [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #16936:
URL: https://github.com/apache/kafka/pull/16936#issuecomment-2299526420
The purpose of the "gh-" filter is to give developers a way to opt-in to Github actions while we are evaluating it. It won't affect the Jenkins build.
We could also use a PR label, but only committers can set those.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Separate GH workflows for PRs and trunk [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #16936:
URL: https://github.com/apache/kafka/pull/16936#discussion_r1723796433
##########
.github/workflows/pr.yml:
##########
@@ -68,35 +50,17 @@ jobs:
fail-fast: false
matrix:
java: [ 11, 17 ]
+ if: contains(github.ref, 'gh-') # For now, only run tests on "gh-*" PRs
name: JUnit tests Java ${{ matrix.java }}
steps:
- - name: Checkout code
- uses: actions/checkout@v4
- with:
- persist-credentials: false
- - name: Setup Java
- uses: actions/setup-java@v4
- with:
- distribution: temurin
- java-version: ${{ matrix.java }}
- name: Setup Gradle
- uses: gradle/actions/setup-gradle@v4
- env:
- GRADLE_BUILD_ACTION_CACHE_DEBUG_ENABLED: true
+ uses: ./.github/actions/setup-gradle
with:
- gradle-version: wrapper
- develocity-access-key: ${{ secrets.GE_ACCESS_TOKEN }}
- # Only write to the cache from trunk
- #cache-read-only: ${{ github.ref != 'refs/heads/trunk' }}
- cache-read-only: false
- # Cache downloaded JDKs in addition to the default directories.
- gradle-home-cache-includes: |
- caches
- notifications
- jdks
- cache-cleanup: on-success
- cache-overwrite-existing: true
+ java-version: ${{ matrix.java }}
+ gradle-cache-read-only: true
- name: Test
+ # Publish build scans for now. This will only work on PRs from apache/kafka, not public forks.
Review Comment:
I think all our PRs come from forks normally. Leaving this in here does give committers a way to get a build scan from a non-trunk code base.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Separate GH workflows for PRs and trunk [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #16936:
URL: https://github.com/apache/kafka/pull/16936#discussion_r1724000282
##########
.github/workflows/ci.yml:
##########
@@ -0,0 +1,75 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: CI
+
+on:
+ push:
+ branches:
+ - 'trunk'
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+ cancel-in-progress: false # Let each trunk commit finish a build
+
+jobs:
+ validate:
+ runs-on: ubuntu-latest
+ strategy:
+ fail-fast: false
+ matrix:
+ java: [8, 11, 17, 21]
+ name: Compile and Check Java ${{ matrix.java }}
+ steps:
+ - name: Checkout code
+ uses: actions/checkout@v4
+ with:
+ persist-credentials: false
+ - name: Setup Gradle
+ uses: ./.github/actions/setup-gradle
+ with:
+ java-version: ${{ matrix.java }}
+ gradle-cache-read-only: false
+ develocity-access-key: ${{ secrets.GE_ACCESS_TOKEN }}
+ - name: Compile and validate
+ run: ./gradlew --build-cache --info --scan check -x test
Review Comment:
I notice that you open https://issues.apache.org/jira/browse/KAFKA-17381, so maybe we can remove `--info` to reduce the log output?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Create GH workflow for public forks [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16936:
URL: https://github.com/apache/kafka/pull/16936#issuecomment-2299062852
> Per the ASF Infra policy https://infra.apache.org/github-actions-policy.html, we must take care not to inject secrets into pull_request_target workflows. For this reason, I've created a separate "Public Pull Requests" workflow that does not public Develocity build scans.
oh, that is a good point
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Separate GH workflows for PRs and trunk [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah merged PR #16936:
URL: https://github.com/apache/kafka/pull/16936
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Create GH workflow for public forks [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16936:
URL: https://github.com/apache/kafka/pull/16936#issuecomment-2299163145
> we must take care not to inject secrets into pull_request_target workflows.
not sure whether I misunderstand the "inject". IIRC, `secrets` is a safe way recommended by Github (https://docs.github.com/en/actions/security-for-github-actions/security-guides/using-secrets-in-github-actions), and other apache projects are using that to enable some extra services (gradle dvelocity, CodeCov, etc.)
in short, I don't think `secrets` will "export" the token but it does "inject" the secrets.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Separate GH workflows for PRs and trunk [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #16936:
URL: https://github.com/apache/kafka/pull/16936#discussion_r1723794648
##########
.github/workflows/ci.yml:
##########
@@ -0,0 +1,65 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: CI
+
+on:
+ push:
+ branches:
+ - 'trunk'
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+ cancel-in-progress: false # Let each trunk commit finish a build
+
+jobs:
+ validate:
+ runs-on: ubuntu-latest
+ strategy:
+ fail-fast: false
+ matrix:
+ java: [8, 11, 17, 21]
+ name: Compile and Check Java ${{ matrix.java }}
+ steps:
+ - name: Setup Gradle
+ uses: ./.github/actions/setup-gradle
+ with:
+ java-version: ${{ matrix.java }}
+ gradle-cache-read-only: false
+ - name: Compile and validate
+ run: ./gradlew --build-cache --info --scan check -x test
+
+ test:
+ needs: validate
+ runs-on: ubuntu-latest
+ strategy:
+ fail-fast: false
+ matrix:
+ java: [ 11, 17 ]
+ name: JUnit tests Java ${{ matrix.java }}
+ steps:
+ - name: Setup Gradle
+ uses: ./.github/actions/setup-gradle
+ with:
+ java-version: ${{ matrix.java }}
+ gradle-cache-read-only: false
+ - name: Test
+ timeout-minutes: 180 # 3 hours
+ run: |
+ ./gradlew --build-cache --scan --continue \
+ -PtestLoggingEvents=started,passed,skipped,failed \
+ -PignoreFailures=true -PmaxParallelForks=2 \
+ -PmaxTestRetries=1 -PmaxTestRetryFailures=10 \
Review Comment:
I agree, but this is how Jenkins is run today. Let's save this for a future PR :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Create GH workflow for public forks [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16936:
URL: https://github.com/apache/kafka/pull/16936#issuecomment-2299060806
> For security reasons, ASF Infra disallows forks to trigger workflows using the pull_request event (as this would expose repository secrets). Instead, we can use the pull_request_target event which triggers when a fork creates a PR. This event will cause the workflow to run using the code of the PR target (i.e., trunk).
not sure whether we need another ci file. we can change the limit of branches (https://github.com/apache/kafka/pull/16931#issuecomment-2298977317) to fix the issue (see https://github.com/apache/kafka/pull/16939 for non-committer's PR)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org