feat: include e2e tests in CI#575
Open
rebEllieous wants to merge 8 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughExposes Docker-built image tag via workflow_call, adapts docker workflow for local ChangesE2E Test Workflow Integration
Sequence DiagramsequenceDiagram
participant Caller as CI caller / workflow_call
participant DockerWF as .github/workflows/docker.yaml
participant Registry as Registry (ghcr.io or localhost:5001)
participant ReusableE2E as .github/workflows/test-e2e.yaml
participant Kind as Kind cluster
Caller->>DockerWF: invoke buildAndPush (push/PR/workflow_call)
DockerWF->>Registry: build & push image (sets steps.meta.outputs.version -> image-tag)
DockerWF-->>Caller: outputs.image-tag
Caller->>ReusableE2E: call test-e2e with image-tag
ReusableE2E->>Kind: create cluster and run make test-e2e with REGISTRY/TAG
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
.github/workflows/golang.yaml (2)
106-116: 💤 Low valueOptional: Remove redundant tool installations.
Both
kubectlandhelmare pre-installed on ubuntu-24.04 GitHub runners, making these installation steps unnecessary. Thehelm/kind-actionalso handles Kind installation.♻️ Simplification
- name: Install Tools if: needs.buildAndPush.outputs.image-tag != '' run: | - # Install kubectl - curl -LO "https://dl.k8s.io/release/$(curl -L -s https://dl.k8s.io/release/stable.txt)/bin/linux/amd64/kubectl" - chmod +x ./kubectl - sudo mv ./kubectl /usr/local/bin/kubectl kubectl version --client - - # Install helm - curl https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3 | bash + helm version🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/golang.yaml around lines 106 - 116, Remove the redundant "Install Tools" step that manually installs kubectl and helm (the block named "Install Tools" containing the curl/chmod/mv kubectl and the curl | bash helm commands) since ubuntu-24.04 runners already include kubectl and helm and the existing helm/kind-action manages Kind; delete that step or replace it with a noop/comment explaining it's skipped, ensuring no other steps reference the removed commands.
95-96: 💤 Low valueConsider setting
persist-credentials: falsefor defense-in-depth.Same rationale as docker.yaml - explicitly disabling credential persistence reduces attack surface.
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 if: needs.buildAndPush.outputs.image-tag != '' + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/golang.yaml around lines 95 - 96, Modify the actions/checkout step that currently uses actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd (the checkout step guarded by if: needs.buildAndPush.outputs.image-tag != '') to explicitly set persist-credentials: false so credentials are not left in the runner; update that step's inputs to include persist-credentials: false while keeping the existing if condition and version of actions/checkout..github/workflows/docker.yaml (1)
55-55: 💤 Low valueConsider setting
persist-credentials: falsefor defense-in-depth.While not exploitable in this workflow's context, explicitly disabling credential persistence reduces the attack surface if the checkout is later reused in steps that export artifacts.
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docker.yaml at line 55, Update the GitHub Actions checkout step that uses actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd to add the persist-credentials: false option; specifically modify the checkout step (the actions/checkout invocation) to include persist-credentials: false so credentials are not left in the workspace after checkout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docker.yaml:
- Around line 46-50: Remove the unintended "|| true" from the if condition so
the job is no longer unconditional, and guard access to
github.event.pull_request.labels when github.event_name == 'workflow_call' by
treating workflow_call separately: either (Option A) allow workflow_call
invocations unconditionally by adding a separate clause that checks
github.event_name == 'workflow_call' OR the existing PR/push/release checks
(reference the existing if conditional using github.event_name and
github.event.pull_request.labels), or (Option B) add a workflow_call input
(e.g., an "ok_to_image" boolean input) and update the if to check that input for
workflow_call events instead of accessing pull_request.labels; implement one of
these options and remove the "|| true".
In @.github/workflows/golang.yaml:
- Around line 51-54: Remove the conditional installation block that checks for
shellcheck (the if ! command -v shellcheck &> /dev/null; then ... fi block)
since shellcheck is preinstalled on ubuntu-24.04 runners; delete those lines
entirely, or if you prefer to keep a guard, replace the German echo with an
English message and prepend sudo to the apt-get commands (sudo apt-get update &&
sudo apt-get install -y shellcheck) so the install would succeed on GitHub
runners.
- Around line 83-123: The E2E CI assumes localhost/local images
(test/fixtures/solar.values.yaml) but the workflow runs make e2e-cluster in CI
without building or loading local images; fix by either making the images
available in CI or changing Helm values: Option A — before "Run E2E Test Suite",
invoke the same targets used locally (docker-build-local-images and
kind-load-local-images or call setup-local-cluster) so the localhost/local/*:e2e
images exist in the Kind cluster (ensure steps run even when CI=true), or Option
B — parameterize test/fixtures/solar.values.yaml to use the REGISTRY/IMAGE_TAG
passed to make test-e2e (use the workflow env REGISTRY and IMAGE_TAG) and, if
switching to ghcr.io, create and mount image pull secrets in the cluster (add
registry auth steps) so pods can pull ghcr.io/... images; update the workflow
step that runs make test-e2e to pass the chosen REGISTRY/IMAGE_TAG consistently.
In `@Makefile`:
- Around line 94-98: The Makefile hardcodes TAG=e2e in the e2e cluster target
causing CI image-tag mismatch; update the recipe lines that call the targets
docker-build-local-images and kind-load-local-images and the invocation of
$(HACK_DIR)/dev-cluster.sh to use TAG=$(TAG) (and REGISTRY=$(REGISTRY)) instead
of TAG=e2e, and export TAG and REGISTRY so subprocesses (e.g., go tests invoking
make e2e-cluster or dev-cluster.sh) inherit the values; locate the lines
referencing docker-build-local-images, kind-load-local-images and the
dev-cluster.sh invocation to apply these changes.
---
Nitpick comments:
In @.github/workflows/docker.yaml:
- Line 55: Update the GitHub Actions checkout step that uses
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd to add the
persist-credentials: false option; specifically modify the checkout step (the
actions/checkout invocation) to include persist-credentials: false so
credentials are not left in the workspace after checkout.
In @.github/workflows/golang.yaml:
- Around line 106-116: Remove the redundant "Install Tools" step that manually
installs kubectl and helm (the block named "Install Tools" containing the
curl/chmod/mv kubectl and the curl | bash helm commands) since ubuntu-24.04
runners already include kubectl and helm and the existing helm/kind-action
manages Kind; delete that step or replace it with a noop/comment explaining it's
skipped, ensuring no other steps reference the removed commands.
- Around line 95-96: Modify the actions/checkout step that currently uses
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd (the checkout step
guarded by if: needs.buildAndPush.outputs.image-tag != '') to explicitly set
persist-credentials: false so credentials are not left in the runner; update
that step's inputs to include persist-credentials: false while keeping the
existing if condition and version of actions/checkout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e638080-a68e-49e7-9d43-c36c57530311
📒 Files selected for processing (3)
.github/workflows/docker.yaml.github/workflows/golang.yamlMakefile
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/golang.yaml (1)
117-123:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign e2e registry config:
REGISTRY/TAGpassed but unused
.github/workflows/golang.yamlpassesTAG=$IMAGE_TAGandREGISTRY=$REGISTRYintomake test-e2e, but the Makefiletest-e2etarget does not exportREGISTRY/TAGto thego testprocess (it only exportsHELM,KIND,KIND_CLUSTER,KUBECTL,MAKE,OCM).test/e2e/has noos.Getenv/LookupEnvusage forREGISTRY/TAG, andtest/fixtures/*.values.yamlhardcodelocalhost/local/...image repositories.
Either remove the unusedTAG/REGISTRYparameters from the workflow or wire them into the e2e manifests/fixtures (e.g., via runtime YAML patching) if pulling from GHCR is intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/golang.yaml around lines 117 - 123, The workflow sets REGISTRY and IMAGE_TAG and passes them to make test-e2e, but the Makefile target test-e2e and the e2e suite (test/e2e and test/fixtures/*.values.yaml) never consume them; either remove those args from the workflow or wire them through: update the Makefile target test-e2e to export REGISTRY and TAG into the go test environment (so the go test subprocess can read os.Getenv/LookupEnv), and update the e2e code (test/e2e) and fixtures to accept those env vars (or perform runtime YAML patching of test/fixtures/*.values.yaml to replace hardcoded localhost repos with "${REGISTRY}/${TAG}" values) so tests pull images from GHCR when IMAGE_TAG/REGISTRY are provided; reference symbols: REGISTRY, IMAGE_TAG, make target test-e2e, test/e2e, test/fixtures/*.values.yaml.
♻️ Duplicate comments (1)
.github/workflows/docker.yaml (1)
46-50:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
workflow_callevents lack PR context, so this condition always evaluates to false for reusable workflow calls.When this workflow is invoked via
workflow_call(e.g., fromgolang.yamlline 81),github.event.pull_requestisnull—there is no PR context. The expressioncontains(github.event.pull_request.labels.*.name, 'ok-to-e2e')evaluates tofalse, causing the job to be skipped and no images to be built. This defeats the purpose of the e2e integration.To allow
workflow_callinvocations unconditionally (the caller decides when to invoke), moveworkflow_callto a separate clause:🔧 Proposed fix
if: | github.event_name == 'push' || (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'ok-to-image')) || - (github.event_name == 'workflow_call' && contains(github.event.pull_request.labels.*.name, 'ok-to-e2e')) || + github.event_name == 'workflow_call' || (github.event_name == 'release' && github.event.action == 'published')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docker.yaml around lines 46 - 50, The current job-level if uses a combined expression that includes contains(github.event.pull_request.labels.*.name, 'ok-to-e2e') which will be false when github.event_name == 'workflow_call' because github.event.pull_request is null; update the if so that workflow_call is its own clause (e.g., (github.event_name == 'workflow_call') || ...), removing the label check for that branch so reusable workflow calls run unconditionally, while preserving the existing label checks for pull_request and other events.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/golang.yaml:
- Around line 117-123: The workflow sets REGISTRY and IMAGE_TAG and passes them
to make test-e2e, but the Makefile target test-e2e and the e2e suite (test/e2e
and test/fixtures/*.values.yaml) never consume them; either remove those args
from the workflow or wire them through: update the Makefile target test-e2e to
export REGISTRY and TAG into the go test environment (so the go test subprocess
can read os.Getenv/LookupEnv), and update the e2e code (test/e2e) and fixtures
to accept those env vars (or perform runtime YAML patching of
test/fixtures/*.values.yaml to replace hardcoded localhost repos with
"${REGISTRY}/${TAG}" values) so tests pull images from GHCR when
IMAGE_TAG/REGISTRY are provided; reference symbols: REGISTRY, IMAGE_TAG, make
target test-e2e, test/e2e, test/fixtures/*.values.yaml.
---
Duplicate comments:
In @.github/workflows/docker.yaml:
- Around line 46-50: The current job-level if uses a combined expression that
includes contains(github.event.pull_request.labels.*.name, 'ok-to-e2e') which
will be false when github.event_name == 'workflow_call' because
github.event.pull_request is null; update the if so that workflow_call is its
own clause (e.g., (github.event_name == 'workflow_call') || ...), removing the
label check for that branch so reusable workflow calls run unconditionally,
while preserving the existing label checks for pull_request and other events.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 96928845-5072-4d6e-8990-186126d83a69
📒 Files selected for processing (3)
.github/workflows/docker.yaml.github/workflows/golang.yaml.github/workflows/update-action-pins.yml
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/update-action-pins.yml
rebEllieous
force-pushed
the
feature/481-include-e2e-in-ci
branch
from
cf770cd to
bfbd212
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/e2e/e2e_suite_test.go (1)
303-316: ⚡ Quick winAdd
GinkgoHelper()to improve test failure reporting.This helper function should call
GinkgoHelper()at the start (like other helpers in this file do) to ensure that when test failures occur, the stack trace points to the calling test code rather than this helper function.📍 Proposed fix
func createPullSecret(namespace, token string) error { + GinkgoHelper() + if token == "" { return nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/e2e_suite_test.go` around lines 303 - 316, Add a GinkgoHelper() call at the start of the createPullSecret function so test failure stack traces point to the caller: inside createPullSecret (before any logic or calls to exec.Command/run), invoke GinkgoHelper() as other helper functions do, then proceed with the existing token-check and kubectl invocation (references: createPullSecret, kubectlBinary, run).test/fixtures/solar-e2e.values.yaml (1)
19-21: Confirmdiscoveryinsolar-e2e.values.yamlis not consumed by thesolarchart, so the image repo mismatch won’t affect e2e.
test/e2e/e2e_test.goinstalls thesolarchart usingtest/fixtures/solar-e2e.values.yaml, butcharts/solar/templatescontains no references todiscovery/.Values.discovery, so thatdiscovery.image.repository: ghcr.io/opendefensecloud/solar-discovery-workerentry is ignored.The e2e discovery components are deployed separately via
solar-discovery/solar-discovery-scancharts usingtest/fixtures/solar-discovery-*.values.yaml, which usesolar-discovery(notsolar-discovery-worker). If you want to reduce confusion, consider removing/correcting the unuseddiscoveryblock (and/or the relatedcharts/solar/README.mdentry).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/fixtures/solar-e2e.values.yaml` around lines 19 - 21, The discovery.image.repository entry in test/fixtures/solar-e2e.values.yaml is unused by the solar chart (charts/solar/templates contains no .Values.discovery) and therefore misleading for e2e; either remove the entire discovery block from solar-e2e.values.yaml or correct it to match the actual e2e deployment charts (solar-discovery / solar-discovery-scan) and their expected key (compare test/fixtures/solar-discovery-*.values.yaml), and update any mention in charts/solar/README.md to avoid confusion; verify test/e2e/e2e_test.go still installs the intended values after the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/e2e/e2e_suite_test.go`:
- Around line 303-316: Add a GinkgoHelper() call at the start of the
createPullSecret function so test failure stack traces point to the caller:
inside createPullSecret (before any logic or calls to exec.Command/run), invoke
GinkgoHelper() as other helper functions do, then proceed with the existing
token-check and kubectl invocation (references: createPullSecret, kubectlBinary,
run).
In `@test/fixtures/solar-e2e.values.yaml`:
- Around line 19-21: The discovery.image.repository entry in
test/fixtures/solar-e2e.values.yaml is unused by the solar chart
(charts/solar/templates contains no .Values.discovery) and therefore misleading
for e2e; either remove the entire discovery block from solar-e2e.values.yaml or
correct it to match the actual e2e deployment charts (solar-discovery /
solar-discovery-scan) and their expected key (compare
test/fixtures/solar-discovery-*.values.yaml), and update any mention in
charts/solar/README.md to avoid confusion; verify test/e2e/e2e_test.go still
installs the intended values after the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc94937c-3232-4d2e-850f-93942ac8a351
📒 Files selected for processing (6)
.github/workflows/docker.yaml.github/workflows/golang.yamlMakefiletest/e2e/e2e_suite_test.gotest/e2e/e2e_test.gotest/fixtures/solar-e2e.values.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/golang.yaml
- Makefile
rebEllieous
added
ok-to-e2e
Coverage Report for CI Build 27170612612Warning No base build found for commit Coverage: 73.209%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
rebEllieous
force-pushed
the
feature/481-include-e2e-in-ci
branch
3 times, most recently
from
52c274a to
187b65b
Compare
rebEllieous
force-pushed
the
feature/481-include-e2e-in-ci
branch
2 times, most recently
from
ac098ff to
8b5b645
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/golang.yaml (1)
21-22: ⚡ Quick winDisable persisted credentials in both checkout steps.
actions/checkoutpersists the workflow token in git config by default. Settingpersist-credentials: falsein both checkout steps reduces token exposure risk in later steps that execute repository code.🔧 Proposed patch
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + persist-credentials: false @@ - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 if: needs.buildAndPush.result == 'success' + with: + persist-credentials: falseAlso applies to: 111-112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/golang.yaml around lines 21 - 22, Two checkout steps currently use actions/checkout without disabling credential persistence; update both occurrences of the actions/checkout step (the lines referencing "uses: actions/checkout@...") to include the step option persist-credentials: false so the workflow token is not automatically written to git config for later steps.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/golang.yaml:
- Around line 105-110: The current GitHub Action step uses if:
needs.buildAndPush.result != 'success', which also fires on failure/cancelled
and produces a misleading "E2E Tests Skipped" summary; change the condition to
only trigger for the skipped case (e.g., if: needs.buildAndPush.result ==
'skipped') and update the step text accordingly, and optionally add separate
conditional steps/messages for needs.buildAndPush.result == 'failure' and
'cancelled' to provide accurate summaries for those outcomes; update references
to needs.buildAndPush.result and the existing step that writes to
$GITHUB_STEP_SUMMARY (the block containing the echo lines) to reflect these new
conditions.
---
Nitpick comments:
In @.github/workflows/golang.yaml:
- Around line 21-22: Two checkout steps currently use actions/checkout without
disabling credential persistence; update both occurrences of the
actions/checkout step (the lines referencing "uses: actions/checkout@...") to
include the step option persist-credentials: false so the workflow token is not
automatically written to git config for later steps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f236d81c-d997-48a6-9ae0-3a22b42fc728
📒 Files selected for processing (2)
.github/workflows/docker.yaml.github/workflows/golang.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/docker.yaml
rebEllieous
force-pushed
the
feature/481-include-e2e-in-ci
branch
from
8b5b645 to
2c03376
Compare
rebEllieous
force-pushed
the
feature/481-include-e2e-in-ci
branch
7 times, most recently
from
b23b179 to
a597d49
Compare
rebEllieous
force-pushed
the
feature/481-include-e2e-in-ci
branch
2 times, most recently
from
8fd4f6a to
5e05caf
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/test-e2e.yaml (2)
18-18: 💤 Low valueConsider adding
persist-credentials: falsefor defense-in-depth.The checkout action persists Git credentials by default. For a workflow that only needs to read the repository and run tests, disabling credential persistence reduces attack surface.
- - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test-e2e.yaml at line 18, The workflow currently uses the actions/checkout step (uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd) which persists Git credentials by default; update that checkout step to include a with: persist-credentials: false setting so credentials are not stored for downstream steps—locate the actions/checkout usage and add the persist-credentials: false key under its with block.Source: Linters/SAST tools
29-37: 💤 Low valueTool installation could be more robust.
The kubectl and Helm installation commands work but have minor robustness concerns:
- kubectl: The nested
$(curl ...)could fail silently if the version endpoint is unreachable.- Helm: Piping curl to bash without checksum verification follows common practice but provides no integrity guarantee.
For CI reliability, consider using pinned versions or official GitHub Actions for these tools. However, the current approach is functional and commonly used.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test-e2e.yaml around lines 29 - 37, The current inline installs for kubectl (the nested $(curl ...) fetch) and Helm (curl | bash) are brittle; replace them by using the official GitHub Actions (actions/setup-kubectl and azure/setup-helm or helm's official action) or at minimum pin a specific kubectl/helm version, check the curl exit codes, and verify downloads with a checksum before moving/executing; update the steps that run the kubectl download/chmod/mv and the curl | bash helm install to use the actions or add explicit version variables, curl -f/--silent checks, and checksum/verification logic so CI installation is robust and fails loudly on network or integrity errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docker.yaml:
- Around line 27-34: The temporary debug job "debug-context-docker" exposes a
template injection risk by interpolating "${{ github.event.action }}" directly
into a shell command and lacks an explicit permissions block; either remove the
job before merging or if keeping it, change the step to pass github.event.action
via an environment variable (e.g., set an env: ACTION: ${{ github.event.action
}} and echo "$ACTION" in the run) to avoid direct template expansion in the
shell, and add an explicit minimal permissions block (e.g., permissions: {} or
only the required scopes) at the job level to avoid inheriting broad defaults.
In @.github/workflows/test-e2e.yaml:
- Around line 38-44: The REGISTRY env expression relies on env.ACT which may not
exist in a called reusable workflow; add an explicit boolean input (e.g.,
inputs.is-act) to the workflow_call and use that input instead of env.ACT when
computing REGISTRY (update the REGISTRY env expression in the block that defines
REGISTRY and IMAGE_TAG), and update callers (like docker.yaml) to pass
is-act=true when running under act; alternatively set ACT at the workflow/job
env level so the expression can see it—ensure you reference the new input name
(is-act) where REGISTRY is computed and keep IMAGE_TAG usage unchanged.
---
Nitpick comments:
In @.github/workflows/test-e2e.yaml:
- Line 18: The workflow currently uses the actions/checkout step (uses:
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd) which persists Git
credentials by default; update that checkout step to include a with:
persist-credentials: false setting so credentials are not stored for downstream
steps—locate the actions/checkout usage and add the persist-credentials: false
key under its with block.
- Around line 29-37: The current inline installs for kubectl (the nested $(curl
...) fetch) and Helm (curl | bash) are brittle; replace them by using the
official GitHub Actions (actions/setup-kubectl and azure/setup-helm or helm's
official action) or at minimum pin a specific kubectl/helm version, check the
curl exit codes, and verify downloads with a checksum before moving/executing;
update the steps that run the kubectl download/chmod/mv and the curl | bash helm
install to use the actions or add explicit version variables, curl -f/--silent
checks, and checksum/verification logic so CI installation is robust and fails
loudly on network or integrity errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e68789b-233b-4df3-95f6-01bd9bd8c04e
📒 Files selected for processing (3)
.github/workflows/docker.yaml.github/workflows/golang.yaml.github/workflows/test-e2e.yaml
rebEllieous
force-pushed
the
feature/481-include-e2e-in-ci
branch
2 times, most recently
from
e1c6a45 to
c491a71
Compare
rebEllieous
force-pushed
the
feature/481-include-e2e-in-ci
branch
from
c491a71 to
2e8e0f6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Run e2e Tests in CI Pipelines
Closes #481
Why
Previously, the E2E test suite had to be triggered manually. To ensure continuous integration and prevent regressions, these tests are now automated within the main pipeline.
Additionally, the configuration includes adjustments for local execution runners (
act)Testing
actwith privileged container options and host networking configurations.Notes for reviewers
actnow requires an.actrccontaining--container-options "--privileged -v /var/run/docker.sock:/var/run/docker.sock"and--network host.Checklist
Summary by CodeRabbit
New Features
Tests
Bug Fixes
Chores