karmada-io / karmada

Open, Multi-Cloud, Multi-Cluster Kubernetes Orchestration
https://karmada.io
Apache License 2.0
4.14k stars 812 forks source link

Dependabot can not access the secrets #4761

Closed liangyuanpeng closed 1 month ago

liangyuanpeng commented 1 month ago

What would you like to be added:

/assign

I really tested the dependabot on my fork, and skiped some CI due to if: ${{ github.repository == 'karmada-io/karmada' }}.

Now, have two case need to resolve,

FOSSA failed

https://github.com/karmada-io/karmada/blob/ff7322acf252632ef2c20864b904a6cd65fa5400/.github/workflows/fossa.yml#L18

It can be resolved by add a secrets of dependabot. cc @RainbowMango image

upload sarif to codeql failed

https://github.com/karmada-io/karmada/blob/ff7322acf252632ef2c20864b904a6cd65fa5400/.github/workflows/ci-image-scanning.yaml#L47-L50

https://github.com/karmada-io/karmada/actions/runs/8445321687/job/23132353774?pr=4753#step:7:64 Warning: Workflows triggered by Dependabot on the "push" event run with read-only access. Uploading Code Scanning results requires write access. To use Code Scanning with Dependabot, please ensure you are using the "pull_request" event for this workflow and avoid triggering on the "push" event for Dependabot branches. See https://docs.github.com/en/code-security/secure-coding/configuring-code-scanning#scanning-on-push for more information on how to configure these events.

Follow the warning, let this CI working on pull_request for dependabot to resolve it.

verifying at https://github.com/liangyuanpeng/karmada/actions/runs/8460139871/job/23177775457?pr=53

Why is this needed:

RainbowMango commented 1 month ago

Just added the Secret for Dependabot as per Adding a repository secret for Dependabot.

zhzhuang-zju commented 1 month ago

To use Code Scanning with Dependabot, please ensure you are using the "pull_request" event for this workflow and avoid triggering on the "push" event for Dependabot branches

@liangyuanpeng Do you know why avoid triggering on the "push" event for Dependabot branches with Code Scanning? If it's just a permission issue, can we just add write permission?

liangyuanpeng commented 1 month ago

@zhzhuang-zju

https://github.com/github/codeql-action/pull/435#issuecomment-810350417 Although perhaps you could add "Uploading code scanning results requires write access." as second sentence?

I found the PR for this warnning log and seems like it just wanted to draw people's attention to the fact that this requires write permissions.

RainbowMango commented 1 month ago

Hi @liangyuanpeng

The workflows/fossa.yml and workflows/ci-image-scanning.yaml should not run for a PR. So I guess we can ignore the two workflows from running in PRs. Something like:

diff --git a/.github/workflows/ci-image-scanning.yaml b/.github/workflows/ci-image-scanning.yaml
index 1774ec0c1..c1b1c801b 100644
--- a/.github/workflows/ci-image-scanning.yaml
+++ b/.github/workflows/ci-image-scanning.yaml
@@ -1,6 +1,10 @@
 name: image-scanning
 on:
-  push: 
+  push:
+    # Exclude branches created by Dependabot to avoid triggering current workflow
+    # for PRs initiated by Dependabot.
+    branches-ignore:
+      - 'dependabot/**'
 jobs:
   use-trivy-to-scan-image:
     name: image-scanning
diff --git a/.github/workflows/fossa.yml b/.github/workflows/fossa.yml
index 05165eeb3..f9d41046d 100644
--- a/.github/workflows/fossa.yml
+++ b/.github/workflows/fossa.yml
@@ -1,6 +1,10 @@
 name: FOSSA
 on:
   push:
+    # Exclude branches created by Dependabot to avoid triggering current workflow
+    # for PRs initiated by Dependabot.
+    branches-ignore:
+      - 'dependabot/**'
 jobs:
   fossa:
     name: FOSSA

What do you think?

by the way: We don't know why Dependabot PR would trigger the two workflows yet, @zhzhuang-zju sent a ticket to GitHub support team, no response yet

liangyuanpeng commented 1 month ago

The workflows/fossa.yml and workflows/ci-image-scanning.yaml should not run for a PR

Since dependabot's PR is always updating dependencies and not add a dependenice, I agree it can be skipped.

We don't know why Dependabot PR would trigger the two workflows yet,

@RainbowMango The reason is the branch of dependbot exist the repo of karmada, so it will trigger the push event.

image

zhzhuang-zju commented 1 month ago

The workflows/fossa.yml and workflows/ci-image-scanning.yaml should not run for a PR

The workflows/ci.yml and workflows/cli.yaml are both triggered on push and pull request events, which may be dobule checked when dependabot submit a PR. image Whether these workflows need to aviod triggering for PRs initiated by Dependabot, like

on:
  push:
+    # Exclude branches created by Dependabot to avoid triggering current workflow
+    # for PRs initiated by Dependabot.
+    branches-ignore:
+      - 'dependabot/**'
  pull_request:
zhzhuang-zju commented 1 month ago

The reason is the branch of dependbot exist the repo of karmada, so it will trigger the push event.

Yes, the push event actually triggered in the branch of dependbot. We want to figure out why the [push] workflow appears in the check screen of an open PR and intercepts the merge of the PR if it fails. If it's a workflow mechanism, then we can circumvent it as suggested by @RainbowMango

RainbowMango commented 1 month ago

The workflows/ci.yml and workflows/cli.yaml are both triggered on push and pull request events, which may be dobule checked when dependabot submit a PR.

+1.

RainbowMango commented 1 month ago

So, @liangyuanpeng what do you say, shall we do it in #4762?