Skip to content

Separate Eden CI into untrusted Gate and trusted Runner workflows#5048

Merged
OhmSpectator merged 1 commit intolf-edge:masterfrom
OhmSpectator:feature/eden-gate
Jul 7, 2025
Merged

Separate Eden CI into untrusted Gate and trusted Runner workflows#5048
OhmSpectator merged 1 commit intolf-edge:masterfrom
OhmSpectator:feature/eden-gate

Conversation

@OhmSpectator
Copy link
Copy Markdown
Member

Description

Separate Eden test execution into two cooperating GitHub workflows: an untrusted "PR Gate," triggered by PR approvals and build completions, and a trusted "Eden Runner," triggered by the successful completion of the gate. This change enables secure access to Docker Hub secrets and streamlines test execution when PRs from forks are approved before the build is ready.

Previously, Eden tests ran within a single workflow triggered by pull_request_review events, which, when originating from forks, lacked access to repository secrets. As a result, the workflow couldn't log in to Docker Hub, causing test failures. The updated setup addresses this by splitting the logic:

  1. The "PR Gate" workflow (untrusted) triggers on two events:
  • Submission of a PR review (specifically approval).
  • Completion of the PR build workflow.

It always checks two conditions:

  • The PR is approved.
  • The necessary build artifact ("eve (amd64, kvm, generic)") exists and succeeded.

Once both conditions are met, the PR Gate workflow creates a run-context.json file containing relevant information (PR number, SHA, original run ID, architecture details, etc.) and uploads it as an artifact named "run-context". If conditions aren't met, it uploads a sentinel string ("exit") to indicate tests shouldn't proceed.

  1. The "Eden Runner" workflow (trusted) triggers upon the successful completion of the "PR Gate" workflow using the workflow_run event, thus gaining access to repository secrets. It downloads the "run-context" artifact and verifies its content. If valid (i.e., doesn't contain "exit"), it triggers the actual Eden test execution via the reusable workflow lf-edge/eden/.github/workflows/test.yml.

Due to GitHub limitations, workflows triggered by workflow_run don't automatically appear in the PR's check status list. To maintain transparency, explicit manual status updates are performed at different stages:

  • Initially setting a "pending" status when tests start.
  • Finalizing with "success," "failure," or "error" after completion.

Additionally, each Eden job result is surfaced individually to maintain detailed test visibility within the PR.

Finally, this update eliminates a common bottleneck: Eden tests previously required manual re-runs if PR approval occurred before the build was ready. Now, the PR Gate automatically reevaluates whenever the PR build finishes, enabling automated testing and quicker feedback loops.

The obsolete monolithic workflow (eden.yml) has been removed to reflect these structural improvements clearly.

Note. To fully eliminate Docker Hub's 429 (rate limiting) errors and benefit from authenticated Docker Hub pulls, Eden tests may require additional adjustments beyond this PR. The current PR specifically addresses securely passing secrets into the Eden workflow. Further adaptation of Eden tests themselves is not covered here.

PR dependencies

None.

How to test and validate this PR

It has been tested in the scope of this test PR in the repo hosted by @rene: rene#86

Changelog notes

No user-facing changes.

PR Backports

  • 14.5-stable: to be backported
  • 13.4-stable: to be backported

Checklist

  • I've provided a proper description
  • I've added the proper documentation
  • I've tested my PR on amd64 device
  • I've tested my PR on arm64 device
  • I've written the test verification instructions
  • I've set the proper labels to this PR
  • I've checked the boxes above, or I've provided a good reason why I didn't
    check them.

@OhmSpectator OhmSpectator requested a review from rene July 6, 2025 23:50
@OhmSpectator OhmSpectator added the stable Should be backported to stable release(s) label Jul 6, 2025
@OhmSpectator OhmSpectator force-pushed the feature/eden-gate branch 2 times, most recently from 06ae00b to 7a06bf2 Compare July 6, 2025 23:56
@OhmSpectator OhmSpectator requested a review from rucoder July 6, 2025 23:58
@uncleDecart
Copy link
Copy Markdown
Member

I suggest we release new Eden (I can do that once we have everything we want in eden master), create commit to bump eden version in this PR

on: # yamllint disable-line rule:truthy
workflow_run:
workflows: ["PR Gate"]
types: [completed]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not trigger the workflow from the gate and pass the parameters to avoid parsing JSON files?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it will not give access access to the secrets. It's the idea: to separate them. I put it explicitly to the commit message.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

who has access to download artifacts? According to this

anyone who has read access to the repo can download artifacts using CLI

@uncleDecart
Copy link
Copy Markdown
Member

I'm a bit concerned about the security implications here.

As pointed out in this article, using pull_request_target in combination with actions/checkout can expose secrets. The risk comes from checking out untrusted code from a fork, which could potentially execute malicious actions.

With #1068 merged, we're now triggering the EVE version from the PR itself. This means the PR code could access Eden secrets and Docker credentials passed via the gate workflow — which introduces a non-trivial security risk.

We're essentially exposing ourselves to a class of supply chain attacks via malicious pull requests.

GitHub also discusses this in their own guidance:

We should review this setup carefully

@OhmSpectator
Copy link
Copy Markdown
Member Author

I suggest we release new Eden (I can do that once we have everything we want in eden master), create commit to bump eden version in this PR

I will not wait until the Eden is reworked. We can do it after. The tests have been broken for almost a week by now.

@OhmSpectator
Copy link
Copy Markdown
Member Author

I'm a bit concerned about the security implications here.

As pointed out in this article, using pull_request_target in combination with actions/checkout can expose secrets. The risk comes from checking out untrusted code from a fork, which could potentially execute malicious actions.

With #1068 merged, we're now triggering the EVE version from the PR itself. This means the PR code could access Eden secrets and Docker credentials passed via the gate workflow — which introduces a non-trivial security risk.

We're essentially exposing ourselves to a class of supply chain attacks via malicious pull requests.

GitHub also discusses this in their own guidance:

We should review this setup carefully

We already do it on other workflows, nothing new here, I will ignore this comment

@uncleDecart
Copy link
Copy Markdown
Member

I suggest we release new Eden (I can do that once we have everything we want in eden master), create commit to bump eden version in this PR

I will not wait until the Eden is reworked. We can do it after. The tests have been broken for almost a week by now.

I'm talking about latest commits from @rucoder don't we want them included?

@OhmSpectator
Copy link
Copy Markdown
Member Author

I suggest we release new Eden (I can do that once we have everything we want in eden master), create commit to bump eden version in this PR

I will not wait until the Eden is reworked. We can do it after. The tests have been broken for almost a week by now.

I'm talking about latest commits from @rucoder don't we want them included?

It's already released: 1.0.4

Copy link
Copy Markdown
Contributor

@rene rene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OhmSpectator I didn't understand all the details of these changes, but I will approve taking into account the working version that you provided and tested on my repo.

…flows.

Separate Eden test execution into two cooperating GitHub workflows: an
untrusted "PR Gate," triggered by PR approvals and build completions,
and a trusted "Eden Runner," triggered by the successful completion of
the gate. This change enables secure access to Docker Hub secrets and
streamlines test execution when PRs from forks are approved before the
build is ready.

Previously, Eden tests ran within a single workflow triggered by
`pull_request_review` events, which, when originating from forks, lacked
access to repository secrets. As a result, the workflow couldn't log in
to Docker Hub, causing test failures. The updated setup addresses this
by splitting the logic:

1. The "PR Gate" workflow (untrusted) triggers on two events:

- Submission of a PR review (specifically approval).
- Completion of the PR build workflow.

It always checks two conditions:

- The PR is approved.
- The necessary build artifact ("eve (amd64, kvm, generic)") exists and
  succeeded.

Once both conditions are met, the PR Gate workflow creates a
`run-context.json` file containing relevant information (PR number, SHA,
original run ID, architecture details, etc.) and uploads it as an
artifact named "run-context". If conditions aren't met, it uploads a
sentinel string ("exit") to indicate tests shouldn't proceed.

2. The "Eden Runner" workflow (trusted) triggers upon the successful
completion of the "PR Gate" workflow using the `workflow_run` event,
thus gaining access to repository secrets. It downloads the
"run-context" artifact and verifies its content. If valid (i.e., doesn't
contain "exit"), it triggers the actual Eden test execution via the
reusable workflow lf-edge/eden/.github/workflows/test.yml.

Due to GitHub limitations, workflows triggered by `workflow_run` don't
automatically appear in the PR's check status list. To maintain
transparency, explicit manual status updates are performed at different
stages:

- Initially setting a "pending" status when tests start.
- Finalizing with "success," "failure," or "error" after completion.

Additionally, each Eden job result is surfaced individually to maintain
detailed test visibility within the PR.

Finally, this update eliminates a common bottleneck: Eden tests
previously required manual re-runs if PR approval occurred before the
build was ready. Now, the PR Gate automatically reevaluates whenever the
PR build finishes, enabling automated testing and quicker feedback
loops.

The obsolete monolithic workflow (eden.yml) has been removed to reflect
these structural improvements clearly.

Signed-off-by: Nikolay Martyanov <nikolay@zededa.com>
@github-actions github-actions bot requested a review from uncleDecart July 7, 2025 09:56
@OhmSpectator
Copy link
Copy Markdown
Member Author

@OhmSpectator I didn't understand all the details of these changes, but I will approve taking into account the working version that you provided and tested on my repo.

@rene, could you please approve once again to check the PR Build status?

@OhmSpectator OhmSpectator requested a review from rene July 7, 2025 10:07
@OhmSpectator OhmSpectator merged commit 9c17dd5 into lf-edge:master Jul 7, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stable Should be backported to stable release(s)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants