Copilot commented on code in PR #8530:
URL: https://github.com/apache/hadoop/pull/8530#discussion_r3406376404
##########
.github/workflows/tmpl_cloud_aws.yml:
##########
@@ -87,6 +99,9 @@ jobs:
run: |
echo "Build image URL: ${{
needs.precondition.outputs.build_image_url }}"
- uses: actions/checkout@v6
+ with:
+ repository: ${{ inputs.checkout_repository || github.repository }}
+ ref: ${{ inputs.checkout_ref || github.ref }}
Review Comment:
In the `build-image` job checkout, `persist-credentials` is not disabled.
When this workflow is manually triggered for fork PRs it checks out untrusted
code, and leaving the `GITHUB_TOKEN` in `.git/config` makes it easier for that
code to read/exfiltrate the token. This checkout shouldn’t need persisted git
credentials (the GHCR push uses `${{ github.token }}` via
`docker/login-action`).
##########
.github/workflows/notify_cloud_aws.yml:
##########
@@ -0,0 +1,120 @@
+#
+# 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.
+#
+
+# Add a sticky comment to hadoop-aws PRs from forked repos with a hint that
+# integration tests must be manually triggered by a maintainer.
+#
+name: "Cloud-AWS PR Update"
+
+# Security: This privileged workflow uses pull_request_target but does not
+# check out or execute untrusted code. It only creates a check run and a PR
+# comment in the base repository.
+on:
+ pull_request_target:
+ types: [opened, reopened, synchronize]
+ paths:
+ - 'hadoop-tools/hadoop-aws/**'
+ - '.github/workflows/*cloud_aws.yml'
+ - '.github/actions/build_image**'
+ - '.github/gha-tests/hadoop-aws*excludes.txt'
+
+jobs:
+ notify:
+ if: github.event.pull_request.head.repo.full_name != github.repository
+ name: "Notify Cloud-AWS"
+ runs-on: ubuntu-slim
+ permissions:
+ checks: write
+ pull-requests: write
+ steps:
+ - name: Post approval-required check and sticky comment
+ uses: actions/github-script@v9
+ with:
+ github-token: ${{ secrets.GITHUB_TOKEN }}
+ script: |
+ const marker = '<!-- cloud-aws-fork-pr-note -->';
+ const pr = context.payload.pull_request;
+ const workflowUrl =
`https://github.com/${context.repo.owner}/${context.repo.repo}/actions/workflows/cloud_aws.yml`;
+ const dispatchCommand = `gh workflow run cloud_aws.yml -R
${context.repo.owner}/${context.repo.repo} -f pr_number=${pr.number}`;
+
+ const checkTitle = 'Cloud-AWS manual trigger instructions';
+ const checkSummary = [
+ 'S3A tests must be manually triggered for fork pull requests.',
+ 'A maintainer should:\n',
+ '- Confirm the PR does not contain questionable changes to
actions in',
+ ' .github/workflows.\n',
+ '- *Approve workflows* for this fork PR if prompted.\n',
+ `- Run [Cloud-AWS workflow](${workflowUrl}) with
\`pr_number=${pr.number}\``,
+ ` (or via CLI: \`${dispatchCommand}\`)\n`,
+ '\n',
+ `fork head: \`${pr.head.repo.full_name}@${pr.head.sha.slice(0,
12)}\``,
+ ].join(' ');
Review Comment:
`checkSummary` is built with `].join(' ')` while also embedding `\n` in some
elements; this makes the rendered markdown brittle (several bullets will end up
on the same line) and it currently only mentions reviewing `.github/workflows/`
even though fork runs also need scrutiny of `.github/actions/`. Consider
building the summary as real markdown lines with `join('\n')` and explicitly
calling out both paths.
##########
.github/gha-tests/hadoop-aws-localstack-excludes.txt:
##########
@@ -23,6 +23,8 @@
# TODO see if we can enable any of these...
# tests that depend on public S3 buckets
+# TODO may be able disable these with s3a test config (see testing.md under
+# third party)
Review Comment:
Grammar in the new TODO: it currently reads “may be able disable…”, which is
missing “to” and reads awkwardly in the comment.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
