[
https://issues.apache.org/jira/browse/HADOOP-19858?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18073578#comment-18073578
]
ASF GitHub Bot commented on HADOOP-19858:
-----------------------------------------
ajfabbri commented on code in PR #8412:
URL: https://github.com/apache/hadoop/pull/8412#discussion_r3081706599
##########
.github/workflows/labeler.yml:
##########
@@ -17,8 +17,16 @@
# under the License.
#
-name: "Pull Request Labeler"
-on: pull_request_target
+# Intentionally has a general name.
+# because the test status check created in GitHub Actions
+# currently randomly picks any associated workflow.
+# So, the name was changed to make sense in that context too.
+# See also
https://github.community/t/specify-check-suite-when-creating-a-checkrun/118380/10
+
+name: "PR"
Review Comment:
I don't understand this workaround. Using confusing or ambiguous workflow
names to address a bug in github's reporting logic (IIUC) is counterintuitive
to me.
##########
.github/workflows/update_build_status.yml:
##########
@@ -0,0 +1,108 @@
+#
+# 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: Update Build Status
+
+on:
+ schedule:
+ - cron: "*/15 * * * *"
Review Comment:
This is unfortunate. At least for committers, we shouldn't have to wait for
any polling to see the status of our PRs. How does this behave for non-forked
PRs?
##########
.github/workflows/tmpl_build_and_test.yml:
##########
@@ -0,0 +1,152 @@
+#
+# 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: Build and Test
+
+on:
+ workflow_call:
+ inputs:
+ java:
+ required: false
+ type: string
+ default: 17
+ branch:
+ required: false
+ type: string
+ description: Branch to run the build against
+ default: trunk
+ os:
+ required: false
+ type: string
+ description: Operating system to run the build on
+ default: ubuntu_24
+ jobs:
+ required: false
+ type: string
+ description: >-
+ Jobs to run, and should be in JSON Array format.
+ Candidates: "build-only".
+ default: '[ "build-only" ]'
+
+concurrency:
+ group: >-
+ build-and-test
+ ${{ github.workflow }}
+ ${{ github.repository == 'apache/hadoop' && github.run_id || github.ref }}
+ ${{ inputs.java }}
+ ${{ inputs.branch }}
+ ${{ inputs.os }}
+ ${{ inputs.jobs }}
+ cancel-in-progress: true
+
+env:
+ MAVEN_ARGS:
+ --batch-mode
+ --no-transfer-progress
+ -Pyarn-ui
+ -Pnative
+ -Drequire.test.libhadoop
+ -Drequire.fuse
+ -Drequire.openssl
+ -Drequire.snappy
+ -Drequire.valgrind
+ -Dmaven.test.failure.ignore=false
+
+jobs:
+ precondition:
+ name: Preparation
+ runs-on: ubuntu-24.04
+ outputs:
+ infra_image_url: ${{ steps.variables.outputs.infra_image_url }}
+ steps:
+ - name: Set up Outputs
+ id: variables
+ run: |
+ echo "infra_image_url=ghcr.io/${{ github.repository }}/gha-${{
inputs.os }}:${{ inputs.branch }}-${{ github.run_id }}" >> $GITHUB_OUTPUT
Review Comment:
Potential command injection site. `run` steps with variables are easy to
exploit. I think we can plumb these through some environment variables to
mitigate this. See [OWASP Github Actions Security
Landscape](https://owasp.org/www-chapter-minneapolis-st-paul/download/20240417_OWASP-MSP_Github_Actions_Security_Landscape.pdf),
pg. 25, "Sanitize your inputs"
##########
.github/workflows/build_and_test.yml:
##########
@@ -0,0 +1,52 @@
+#
+# 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: "Build"
+
+on:
+ push:
+ branches:
+ - "**"
+
+jobs:
+ default:
+ permissions:
+ packages: write
Review Comment:
Can you explain why `packages: write` is safe here? I'm wondering if we need
to implement branch protection and disallow GHCR writes except when pushing
from apache/hadoop?
##########
.github/workflows/notify_test_workflow.yml:
##########
@@ -0,0 +1,170 @@
+#
+# 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.
+#
+
+# Intentionally has a general name.
+# because the test status check created in GitHub Actions
+# currently randomly picks any associated workflow.
+# So, the name was changed to make sense in that context too.
+# See also
https://github.community/t/specify-check-suite-when-creating-a-checkrun/118380/10
+
+name: "PR update"
Review Comment:
Same comment here.
##########
.github/workflows/tmpl_build_and_test.yml:
##########
@@ -0,0 +1,152 @@
+#
+# 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: Build and Test
+
+on:
+ workflow_call:
+ inputs:
+ java:
+ required: false
+ type: string
+ default: 17
+ branch:
+ required: false
+ type: string
+ description: Branch to run the build against
+ default: trunk
+ os:
+ required: false
+ type: string
+ description: Operating system to run the build on
+ default: ubuntu_24
+ jobs:
+ required: false
+ type: string
+ description: >-
+ Jobs to run, and should be in JSON Array format.
+ Candidates: "build-only".
+ default: '[ "build-only" ]'
+
+concurrency:
+ group: >-
+ build-and-test
+ ${{ github.workflow }}
+ ${{ github.repository == 'apache/hadoop' && github.run_id || github.ref }}
+ ${{ inputs.java }}
+ ${{ inputs.branch }}
+ ${{ inputs.os }}
+ ${{ inputs.jobs }}
+ cancel-in-progress: true
+
+env:
+ MAVEN_ARGS:
+ --batch-mode
+ --no-transfer-progress
+ -Pyarn-ui
+ -Pnative
+ -Drequire.test.libhadoop
+ -Drequire.fuse
+ -Drequire.openssl
+ -Drequire.snappy
+ -Drequire.valgrind
+ -Dmaven.test.failure.ignore=false
+
+jobs:
+ precondition:
+ name: Preparation
+ runs-on: ubuntu-24.04
+ outputs:
+ infra_image_url: ${{ steps.variables.outputs.infra_image_url }}
+ steps:
+ - name: Set up Outputs
+ id: variables
+ run: |
+ echo "infra_image_url=ghcr.io/${{ github.repository }}/gha-${{
inputs.os }}:${{ inputs.branch }}-${{ github.run_id }}" >> $GITHUB_OUTPUT
+ infra-image:
+ name: Infra Image ${{ inputs.os }}-${{ inputs.branch }}
+ runs-on: ubuntu-24.04
+ needs: [ precondition ]
+ permissions:
+ packages: write
+ outputs:
+ uid: ${{ steps.variables.outputs.uid }}
+ steps:
+ - uses: actions/checkout@v6
+ with:
+ fetch-depth: 0
+ - name: Login to GitHub Container Registry
+ uses: docker/login-action@b45d80f862d83dbcd57f89517bcf500b2ab88fb2 #
v4.0.0
+ with:
+ registry: ghcr.io
+ username: ${{ github.actor }}
+ password: ${{ secrets.GITHUB_TOKEN }}
+ - name: Set up Docker Buildx
+ uses:
docker/setup-buildx-action@4d04d5d9486b7bd6fa91e7baf45bbb4f8b9deedd # v4.0.0
+ - name: Build and push ${{ inputs.os }} base infra image for ${{
inputs.branch }}
+ id: docker_build_base
+ uses:
docker/build-push-action@d08e5c354a6adb9ed34480a06d141179aa583294 # v7.0.0
+ with:
+ context: ./dev-support/docker/
+ file: ./dev-support/docker/Dockerfile_${{ inputs.os }}
+ push: true
+ tags: ${{ needs.precondition.outputs.infra_image_url }}-base
+ - name: User-specific Dockerfile
+ run: |
+ USER_ID=$(id -u "${USER}")
+ GROUP_ID=$(id -g "${USER}")
+ cat > /tmp/Dockerfile.gha <<UserSpecificDocker
+ FROM ${{ needs.precondition.outputs.infra_image_url }}-base
+ RUN rm -f /var/log/faillog /var/log/lastlog
+ RUN groupadd --non-unique -g ${GROUP_ID} ${USER}
Review Comment:
Wondering if it is possible for injection attacks via $USER here. Feels like
moving some of these riskier parts to a separate action (externally-hosted), or
only doing this on trusted branches and/or trusted workflow that doesn't run on
fork PRs, might be a good idea?
> Set up build workflow in GitHub Actions
> ---------------------------------------
>
> Key: HADOOP-19858
> URL: https://issues.apache.org/jira/browse/HADOOP-19858
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: build
> Reporter: Cheng Pan
> Priority: Major
> Labels: pull-request-available
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
