Skip to content

test(e2e): migrate app suite from BATS to Chainsaw and wire into CI - #2826

Merged
myasnikovdaniil merged 32 commits into
mainfrom
feat/e2e-chainsaw-example
Jul 12, 2026
Merged

test(e2e): migrate app suite from BATS to Chainsaw and wire into CI#2826
myasnikovdaniil merged 32 commits into
mainfrom
feat/e2e-chainsaw-example

Conversation

@myasnikovdaniil

@myasnikovdaniil myasnikovdaniil commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Migrates the E2E app test suite from BATS to Kyverno Chainsaw — a declarative, Kubernetes-native E2E framework (CNCF, part of the Kyverno project; successor to KUTTL) — and wires it into CI as the replacement for the per-app BATS loop. This grows the original pilot (postgres + bucket) into the full suite.

All 22 app suites are ported under hack/e2e-chainsaw/ and the hack/e2e-apps/*.bats files are removed:

postgres, bucket, mariadb, mongodb, redis, qdrant, clickhouse, kafka, etcd, openbao, harbor, foundationdb, external-dns, kuberture, vminstance, gateway, kubernetes-latest, kubernetes-previous, kubernetes-oidc-system, kubernetes-oidc-customconfig, securitygroup, serviceexposure.

Approach

  • Declarative suites assert on status.conditions and concrete fields. The timeout N sh -ec "until kubectl get ..." + kubectl wait pair that appeared ~100 times collapses into a single assert that polls existence and state together, with a structured diff on failure and automatic events/describe/podLogs capture via catch (previously only harbor.bats did this, by hand).
  • Imperative suites keep their logic in script steps: openbao init/unseal, kuberture external-dns split-horizon probes, vminstance, the gateway admission/impersonation cases, and kubernetes-latest/previous, which wrap the relocated hack/e2e-chainsaw/_lib/run-kubernetes.sh verbatim (Kamaji bring-up, LB/NFS/ouroboros checks).
  • gateway tests derive the tenant apex from the namespace namespace.cozystack.io/host label at runtime, so they are host-independent.

CI

  • The e2e job now runs chainsaw test hack/e2e-chainsaw/ via a new test-chainsaw target and uploads the JUnit chainsaw-report.xml.
  • The chainsaw binary is added to the e2e-sandbox image.
  • install-cozystack and test-openapi stay BATS (cluster bootstrap + OpenAPI checks).
  • The per-app 3-retry loop is dropped — assertion polling replaces the fixed-timeout flakiness it papered over.

Validation

Ran against a development cluster: the DB/app, storage, and VM suites pass; the three suites that depend on platform features not present on that cluster (gateway, kuberture, external-dns) and the two heavyweight kubernetes-* suites are exercised by this PR's CI run on a freshly installed platform.

Note for reviewers: service-port asserts use the (ports[*].port) projection form rather than a number-literal filter (ports[?port == `N`]), because Chainsaw v0.2.15 mis-evaluates JMESPath number-literal comparisons.

2026-07-09: reconciled with main (~520 commits of drift)

  • etcd suite re-ported to the v1alpha2 operator contracts (feat(platform): migrate to etcd-operator v1alpha2 #2859): readyMembers=3 gate, pod-label//scale/WorkloadMonitor/metrics/defrag contracts, plus the ETCD_E2E_S3_ROUNDTRIP-gated backup round-trip driving examples/backups/etcd
  • 12 new ingress-hostname-policy gateway cases ported (apex derived at runtime from the tenant-root namespace label)
  • post-branch bats drift folded in: mariadb-single webhook guard, kafka c1.small presets, bucket/harbor 2m BucketClaim fail-fast, bucket readonly-denial promoted to a hard fail (cosi-driver v0.3.1), qdrant PVC-reclaim guard (qdrant: StatefulSet data PVC is orphaned after deletion #3059), kuberture fail-loud negations, SC-fallback-default test (feat(kubernetes): propagate remote-accessible LINSTOR StorageClasses to tenant clusters #2872 B1)
  • 4 suites that only exist on main since the branch was cut are ported: kubernetes-oidc-system, kubernetes-oidc-customconfig, securitygroup, serviceexposure
  • CI seams: nightly.yaml converted to the chainsaw invocation, run-kubernetes.sh reconciled (Talos/CABPT waits, tenant drain, LINSTOR pool wait, talos-image-cache under _lib/), e2e-capture-dataplane.sh replicated in the Chainsaw global catch

Release note

fix(kubernetes): tenant Kubernetes teardown no longer hangs when the cluster has no working nodes — the pre-delete hook now bounds its wait for the in-tenant HelmReleases and force-clears their Flux finalizers on timeout (#3271)

Summary by CodeRabbit

  • New Features
    • End-to-end testing is now driven by a unified Chainsaw suite (platform apps, Kubernetes, networking, security groups, service exposure).
    • Test Impact Analysis selects affected suites, with an option to run the full suite.
    • CI produces a JUnit report artifact and collects suite-scoped diagnostics on failure.
  • Bug Fixes
    • CI E2E now fails fast and emits clearer Kubernetes/HelmRelease and sandbox bucket state diagnostics.
    • Improved teardown reliability with bounded wait + finalizer handling to avoid hangs.
  • Documentation
    • Updated E2E testing docs and local guidance to reflect the new Chainsaw-based workflow and suite paths.

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR migrates application E2E coverage from per-app BATS scripts to Kyverno Chainsaw suites, updates TIA and CI execution/reporting, adds shared diagnostics and cleanup helpers, and introduces declarative tests for platform, database, storage, networking, security, and VM workflows.

Changes

Chainsaw E2E migration

Layer / File(s) Summary
CI entrypoints and suite selection
.github/workflows/*, hack/select-e2e*, packages/core/testing/*, Makefile
CI and Make targets now execute Chainsaw suites, select impacted suites through TIA, and collect JUnit reports.
Shared harness and diagnostics
hack/e2e-chainsaw/_lib/*, hack/e2e-chainsaw/.chainsaw.yaml
Global reporting, failure snapshots, remediation detection, cleanup helpers, and Talos cache diagnostics are added.
Application and infrastructure suites
hack/e2e-chainsaw/*
Chainsaw manifests and fixtures replace BATS coverage for databases, storage systems, gateways, Kubernetes variants, security groups, service exposure, and VM workflows.
Cleanup and documentation updates
packages/apps/kubernetes/templates/delete.yaml, docs/*, examples/*
HelmRelease deletion gains bounded finalizer cleanup, while E2E guidance and coverage references are updated for Chainsaw.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers: kvaps, lllamnyp, androndo, IvanHunters, sircthulhu, lexfrei

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: migrating the E2E app suite from BATS to Chainsaw and integrating it into CI.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/e2e-chainsaw-example

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added area/testing Issues or PRs related to testing (e2e, bats, unit tests) size/L This PR changes 100-499 lines, ignoring generated files labels Jun 4, 2026
@myasnikovdaniil

Copy link
Copy Markdown
Contributor Author

Verified on a live dev cluster running Cozystack v1.4.1 (chainsaw v0.2.15):

--- PASS: chainsaw/bucket   (21.82s)
--- PASS: chainsaw/postgres (50.65s)
Tests Summary: Passed 2, Failed 0, Skipped 0

Both tests ran end-to-end: resource creation, all readiness assertions, the functional probes (psql SELECT 1 on the CNPG primary; mc upload/list/download + readonly-denial through a port-forward), and automatic cleanup — the namespace was left with no leftover resources.

One fix came out of the live run (58d7558): the (conditions[?type == 'Ready'])[0]: assertion idiom shown in older chainsaw docs/blog posts fails on v0.2.15 with field not found in the input object. The filter-as-list form works and is equivalent, since the filter always yields exactly one matching condition:

status:
  (conditions[?type == 'Ready']):
  - status: "True"

Worth knowing when reading chainsaw failure output: the "actual" side of the diff is a projection onto the expected fields — status: {} means the key expression didn't evaluate, not that the live object's status is empty.

Compared to the BATS originals, the asserts complete as soon as conditions are met instead of burning fixed sleep-loop budgets, and on failure chainsaw auto-dumps events/describe/pod-logs via the catch blocks (run 1 of this verification exercised exactly that path).

@github-actions github-actions Bot added size/XXL This PR changes 1000+ lines, ignoring generated files and removed size/L This PR changes 100-499 lines, ignoring generated files labels Jun 8, 2026
@myasnikovdaniil myasnikovdaniil changed the title test(e2e): add chainsaw pilot with postgres and bucket tests test(e2e): migrate app suite from BATS to Chainsaw and wire into CI Jun 8, 2026
@myasnikovdaniil
myasnikovdaniil force-pushed the feat/e2e-chainsaw-example branch 2 times, most recently from 36edf2a to f5c7eb7 Compare June 8, 2026 15:58
@myasnikovdaniil
myasnikovdaniil changed the base branch from main to daniil/split-drop-retry June 8, 2026 15:58
@myasnikovdaniil myasnikovdaniil self-assigned this Jun 9, 2026
@myasnikovdaniil
myasnikovdaniil force-pushed the feat/e2e-chainsaw-example branch from 0e6dec0 to f24e264 Compare June 9, 2026 09:43
@myasnikovdaniil
myasnikovdaniil changed the base branch from daniil/split-drop-retry to main June 9, 2026 09:43
@myasnikovdaniil
myasnikovdaniil marked this pull request as ready for review June 9, 2026 10:38
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request modernizes the project's end-to-end testing infrastructure by migrating the application test suites from BATS to Kyverno Chainsaw. By adopting a declarative approach, the suite gains better failure visibility, automatic diagnostic collection, and improved reliability through native assertion polling. The migration covers 18 application suites, including database and infrastructure services, and updates the CI pipeline to support the new framework while retaining existing BATS tests for core cluster operations.

Highlights

  • Migration to Kyverno Chainsaw: Migrated the E2E application test suite from BATS to Kyverno Chainsaw, a declarative, Kubernetes-native testing framework.
  • Improved Test Reliability: Replaced flaky fixed-timeout polling loops with structured assertions that poll for both existence and state, significantly reducing test flakiness.
  • Enhanced Diagnostics: Implemented automatic capture of events, resource descriptions, and pod logs upon test failure, simplifying debugging.
  • CI Integration: Updated the CI pipeline to execute the new Chainsaw suite and generate JUnit reports, while maintaining BATS for cluster bootstrap and OpenAPI checks.
New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Ignored Files
  • Ignored by pattern: .github/workflows/** (1)
    • .github/workflows/pull-requests.yaml
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment Gemini (@gemini-code-assist) Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@dosubot dosubot Bot added the area/ci Issues or PRs related to CI workflows, GitHub Actions, automation label Jun 9, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request migrates the end-to-end (E2E) application test suite from BATS to Kyverno Chainsaw. It removes the legacy .bats test files under hack/e2e-apps/ and introduces declarative Chainsaw test configurations and resource manifests under hack/e2e-chainsaw/ for various platform applications, such as Postgres, MariaDB, Redis, Kafka, ClickHouse, FoundationDB, Harbor, and OpenBAO. Additionally, the sandbox Dockerfile is updated to install the Chainsaw binary, and the testing Makefiles are adjusted to execute Chainsaw tests. There are no review comments provided, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (6)
hack/e2e-chainsaw/vminstance/chainsaw-test.yaml (1)

117-127: ⚡ Quick win

Consider defensive array existence check before indexing.

Line 127 directly indexes status.interfaces[0].ipAddress without first verifying the interfaces array exists and has elements. While the 5-minute timeout should allow the array to populate during polling, a more robust assertion would check array presence first to avoid potential edge-case failures.

Consider adding a preliminary check or using a filter pattern:

- assert:
    timeout: 5m
    resource:
      apiVersion: kubevirt.io/v1
      kind: VirtualMachineInstance
      metadata:
        name: vm-instance-test
      (length(status.interfaces || `[]`) > `0`): true
- assert:
    timeout: 5m
    resource:
      apiVersion: kubevirt.io/v1
      kind: VirtualMachineInstance
      metadata:
        name: vm-instance-test
      (status.interfaces[0].ipAddress != null): true

This would make the test more resilient to variations in VMI startup timing.

🤖 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 `@hack/e2e-chainsaw/vminstance/chainsaw-test.yaml` around lines 117 - 127, The
assertion currently indexes status.interfaces[0].ipAddress on the
VirtualMachineInstance which can fail if status.interfaces is absent; add a
preceding defensive assertion that verifies the interfaces array exists and has
at least one element (e.g. assert (length(status.interfaces || `[]`) > `0`)
before asserting status.interfaces[0].ipAddress != null) so the subsequent
indexing in the VirtualMachineInstance check is safe and the test is more
resilient to delayed interface population.
packages/core/testing/Makefile (1)

32-32: ⚡ Quick win

Declare phony targets at the top of the Makefile.

The e2e and test-chainsaw targets should be declared as .PHONY since they don't produce files with those names. Add a .PHONY declaration near the top of the Makefile (similar to the root Makefile line 1).

📋 Proposed fix

Add near the top of the file (e.g., after line 10):

 include ../../../hack/common-envs.mk
+
+.PHONY: help image image-e2e-sandbox e2e copy-nocloud-image prepare-cluster install-cozystack test-openapi test-chainsaw collect-report collect-images delete exec apply
🤖 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 `@packages/core/testing/Makefile` at line 32, Add a .PHONY declaration near the
top of the Makefile that lists the non-file targets, e.g., include ".PHONY: e2e
test-chainsaw" so the e2e and test-chainsaw targets are treated as phony; place
this declaration close to the top of the file (similar to the root Makefile) so
it applies before the target definitions.
hack/e2e-chainsaw/etcd/chainsaw-test.yaml (1)

262-262: ⚡ Quick win

Avoid hardcoding the base64 credential value.

The assertion hardcodes AWS_ACCESS_KEY_ID: ZTJlLWFjY2Vzcy1rZXk= (base64 of e2e-access-key). If the test credential in etcd-backup.yaml line 23 changes, this assertion will break without a clear error message.

Consider computing the expected base64 value dynamically in the test script or adding a comment that cross-references the source credential.

🔄 Example: Compute base64 dynamically

Replace the hardcoded base64 with a computed value in the assertion step by adding a script operation that sets an expected value:

- script:
    content: |
      EXPECTED_KEY=$(echo -n "e2e-access-key" | base64)
      echo "expected_key=$EXPECTED_KEY" >> "$CHAINSAW_COMMAND_OUTPUT"
- assert:
    resource:
      apiVersion: v1
      kind: Secret
      metadata:
        name: etcd-s3-creds
      data:
        AWS_ACCESS_KEY_ID: (bindings.expected_key)

Alternatively, add a comment documenting the relationship:

# Base64 of "e2e-access-key" from etcd-backup.yaml line 23
AWS_ACCESS_KEY_ID: ZTJlLWFjY2Vzcy1rZXk=
🤖 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 `@hack/e2e-chainsaw/etcd/chainsaw-test.yaml` at line 262, The test currently
hardcodes the base64 value for AWS_ACCESS_KEY_ID in
hack/e2e-chainsaw/etcd/chainsaw-test.yaml which will break if the source
credential in etcd-backup.yaml changes; update the assertion to compute the
expected base64 dynamically (e.g., generate base64 of the credential string used
in etcd-backup.yaml and store it in a binding/variable like expected_key or
CHAINSAW_COMMAND_OUTPUT) and reference that binding in the Secret assertion for
AWS_ACCESS_KEY_ID, or alternatively add an inline comment that documents that
ZTJlLWFjY2Vzcy1rZXk= is the base64 of the etcd-backup.yaml credential to make
the coupling explicit.
hack/e2e-chainsaw/mariadb/mariadb.yaml (1)

19-27: ⚡ Quick win

Remove unused backup credential fields when backup is disabled.

The manifest includes S3 and restic credentials (lines 21-27) even though backup.enabled: false (line 20). Since these fields are not used when backup is disabled, consider removing them or replacing with obviously placeholder values to avoid confusion.

♻️ Proposed cleanup
   backup:
     enabled: false
-    s3Region: us-east-1
-    s3Bucket: s3.example.org/mariadb-backups
-    schedule: "0 2 * * *"
-    cleanupStrategy: "--keep-last=3 --keep-daily=3 --keep-within-weekly=1m"
-    s3AccessKey: oobaiRus9pah8PhohL1ThaeTa4UVa7gu
-    s3SecretKey: ju3eum4dekeich9ahM1te8waeGai0oog
-    resticPassword: ChaXoveekoh6eigh4siesheeda2quai0
   resources: {}
   resourcesPreset: "nano"
🤖 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 `@hack/e2e-chainsaw/mariadb/mariadb.yaml` around lines 19 - 27, The manifest
contains sensitive S3/restic fields (s3Region, s3Bucket, s3AccessKey,
s3SecretKey, resticPassword) while backup.enabled is false; remove these unused
credential entries or replace them with clearly non-sensitive placeholders
(e.g., "<placeholder>") so the file doesn’t expose secrets or confuse readers;
update the backup block (the keys listed above and schedule/cleanupStrategy if
desired) to only include fields relevant when backup.enabled is true and ensure
backup.enabled remains set to false.
hack/e2e-chainsaw/postgres/postgres.yaml (1)

24-32: 💤 Low value

Test credentials in disabled backup configuration.

The backup credentials (s3AccessKey, s3SecretKey, resticPassword) are present but backup.enabled: false means they won't be used. While these are clearly test values (example.org domain), including them when backups are disabled is unnecessary unless you're testing config validation.

This is not a security issue, but consider omitting the credential fields entirely when enabled: false for clarity, or add a comment explaining why they're included.

🤖 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 `@hack/e2e-chainsaw/postgres/postgres.yaml` around lines 24 - 32, The YAML
includes test credentials (s3AccessKey, s3SecretKey, resticPassword) while
backup.enabled is false; remove those credential fields from the backup block
(or replace them with a short inline comment) so the backup section only
contains relevant settings when backup.enabled: false; locate the backup: block
and update the fields named s3AccessKey, s3SecretKey, and resticPassword
accordingly.

Source: Linters/SAST tools

hack/e2e-chainsaw/kuberture/probes.yaml (1)

41-82: ⚡ Quick win

Lock down both probe Pods before policy-enforced clusters reject this step.

These Pods currently run with the default root-capable security context. On clusters with Pod Security / Kyverno-style admission, apply: probes.yaml can be denied before the split-horizon assertions even start. Since these are one-shot, read-only probes, it’s worth setting an explicit restrictive securityContext on both Pod specs and verifying that registry.k8s.io/external-dns/external-dns:v0.20.0 still works with runAsNonRoot and a read-only root filesystem.

Proposed hardening for both probe Pods
 spec:
+  securityContext:
+    seccompProfile:
+      type: RuntimeDefault
   serviceAccountName: kuberture-e2e-edns-probe
   restartPolicy: Never
   containers:
     - name: edns
       image: registry.k8s.io/external-dns/external-dns:v0.20.0
+      securityContext:
+        allowPrivilegeEscalation: false
+        readOnlyRootFilesystem: true
+        runAsNonRoot: true
+        capabilities:
+          drop:
+            - ALL
       args:
🤖 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 `@hack/e2e-chainsaw/kuberture/probes.yaml` around lines 41 - 82, Add a
restrictive securityContext to both Pod specs (the first Pod block and the Pod
named kuberture-e2e-edns-internal) and a container securityContext on the edns
container: set pod-level runAsNonRoot: true and runAsUser to a non-root UID
(e.g., 65534), set containers[0].securityContext.readOnlyRootFilesystem: true,
allowPrivilegeEscalation: false, drop all capabilities and set
seccompProfile.type: RuntimeDefault (or RuntimeDefault equivalent for your
cluster); ensure these changes are applied next to the existing container spec
for the edns container (image registry.k8s.io/external-dns/external-dns:v0.20.0)
so the one-shot probes run as non-root with a read-only root filesystem.

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 `@hack/e2e-chainsaw/etcd/chainsaw-test.yaml`:
- Around line 37-66: The inline "ensure-clean" shell block (the script: content
using the deadline variable, the while loop that checks kubectl -n "$NAMESPACE"
get hr etcd and kubectl get datastore.kamaji.clastix.io "$NAMESPACE", and the
kubectl patch call that clears finalizers) is duplicated across the three etcd
test scenarios; extract that logic into a single shared shell script named
etcd-cleanup.sh and replace each inline block with a short call to that script
(e.g., invoke etcd-cleanup.sh or source etcd-cleanup.sh), making sure the
extracted script preserves the deadline behavior, error/output handling and exit
codes used by the original block and is executable by the test runner.

In `@hack/e2e-chainsaw/kuberture/chainsaw-test.yaml`:
- Around line 123-135: The unconditional phase=$(kubectl -n cozy-kuberture get
pod "kuberture-e2e-edns-${probe}" -o jsonpath='{.status.phase}') can fail under
set -e when the Pod never materializes, preventing the subsequent
describe/logs/exit diagnostics; change this to a guarded capture (e.g.
phase=$(kubectl ... 2>/dev/null || echo "NotFound")) or test existence first
(e.g. if ! kubectl -n cozy-kuberture get pod "kuberture-e2e-edns-${probe}"
>/dev/null 2>&1; then phase=NotFound; else phase=$(kubectl -n cozy-kuberture get
pod "kuberture-e2e-edns-${probe}" -o jsonpath='{.status.phase}'); fi) so that
non-existent Pods funnel into the same describe/logs/exit block that references
probe and phase.
- Around line 63-113: wait-output-services only ensures the Service objects
exist but assert-annotation-shape immediately asserts controller-populated
annotations (e.g. "${pub_prefix}target", "${int_prefix}target", ttl/hostname)
which may not be present yet; modify assert-annotation-shape to poll until the
annotations appear (or convert each annotation check into Chainsaw assert steps
with timeouts) by repeatedly fetching kubectl -n cozy-kuberture get service ...
and re-evaluating the jq checks (hostname, ttl, target and absence of the
opposite prefix) until success or timeout, or replace the script block with
timed assert entries that assert the same annotation keys with a reasonable
timeout (e.g. 2m) to avoid flakiness.

In `@hack/e2e-chainsaw/mongodb/chainsaw-test.yaml`:
- Line 62: Change the E2E assertion to check the StatefulSet's
status.readyReplicas instead of status.replicas so the test verifies pods are
actually running and ready; locate the StatefulSet readiness assertion in the
test that currently expects "replicas: 1" (the StatefulSet object/assertion for
MongoDB in chainsaw-test.yaml) and update it to assert status.readyReplicas
equals the desired count (consistent with other tests like openbao and qdrant).

In `@hack/e2e-chainsaw/postgres/chainsaw-test.yaml`:
- Line 66: Update the PR description to accurately state the JMESPath being
tested: clarify that the postgres assertions use the indexed form
"(ports[0].port)" for single-port services (as seen in the manifest) and that
the Chainsaw/JMESPath v0.2.15 projection bug specifically affects
chained/indexed access after a projection like "ports[*].port"; mention that
"ports[*].port" is used only for multi-port services in this repo and that the
current postgres cases do not exercise the projection bug.

In `@hack/e2e-chainsaw/redis/chainsaw-test.yaml`:
- Around line 37-45: The test currently asserts only the PVC named
redisfailover-persistent-data-rfr-redis-test-0 is Bound, but the Redis spec has
replicas: 2 so you must also verify
redisfailover-persistent-data-rfr-redis-test-1; update the assertion block in
chainsaw-test.yaml to include a second assert (or expand the resource check) for
PersistentVolumeClaim with metadata.name
redisfailover-persistent-data-rfr-redis-test-1 and status.phase: Bound (use the
same timeout and structure as the existing check) so both PVCs created by the
StatefulSet are validated.
- Around line 56-64: The manifest assertion is checking status.replicas which
only verifies desired/current count; change the assertion to validate
status.readyReplicas for the StatefulSet named rfr-redis-test so the test
ensures pods are actually Ready; update the resource assertion under the
StatefulSet block to assert status.readyReplicas: 2 instead of status.replicas:
2.

---

Nitpick comments:
In `@hack/e2e-chainsaw/etcd/chainsaw-test.yaml`:
- Line 262: The test currently hardcodes the base64 value for AWS_ACCESS_KEY_ID
in hack/e2e-chainsaw/etcd/chainsaw-test.yaml which will break if the source
credential in etcd-backup.yaml changes; update the assertion to compute the
expected base64 dynamically (e.g., generate base64 of the credential string used
in etcd-backup.yaml and store it in a binding/variable like expected_key or
CHAINSAW_COMMAND_OUTPUT) and reference that binding in the Secret assertion for
AWS_ACCESS_KEY_ID, or alternatively add an inline comment that documents that
ZTJlLWFjY2Vzcy1rZXk= is the base64 of the etcd-backup.yaml credential to make
the coupling explicit.

In `@hack/e2e-chainsaw/kuberture/probes.yaml`:
- Around line 41-82: Add a restrictive securityContext to both Pod specs (the
first Pod block and the Pod named kuberture-e2e-edns-internal) and a container
securityContext on the edns container: set pod-level runAsNonRoot: true and
runAsUser to a non-root UID (e.g., 65534), set
containers[0].securityContext.readOnlyRootFilesystem: true,
allowPrivilegeEscalation: false, drop all capabilities and set
seccompProfile.type: RuntimeDefault (or RuntimeDefault equivalent for your
cluster); ensure these changes are applied next to the existing container spec
for the edns container (image registry.k8s.io/external-dns/external-dns:v0.20.0)
so the one-shot probes run as non-root with a read-only root filesystem.

In `@hack/e2e-chainsaw/mariadb/mariadb.yaml`:
- Around line 19-27: The manifest contains sensitive S3/restic fields (s3Region,
s3Bucket, s3AccessKey, s3SecretKey, resticPassword) while backup.enabled is
false; remove these unused credential entries or replace them with clearly
non-sensitive placeholders (e.g., "<placeholder>") so the file doesn’t expose
secrets or confuse readers; update the backup block (the keys listed above and
schedule/cleanupStrategy if desired) to only include fields relevant when
backup.enabled is true and ensure backup.enabled remains set to false.

In `@hack/e2e-chainsaw/postgres/postgres.yaml`:
- Around line 24-32: The YAML includes test credentials (s3AccessKey,
s3SecretKey, resticPassword) while backup.enabled is false; remove those
credential fields from the backup block (or replace them with a short inline
comment) so the backup section only contains relevant settings when
backup.enabled: false; locate the backup: block and update the fields named
s3AccessKey, s3SecretKey, and resticPassword accordingly.

In `@hack/e2e-chainsaw/vminstance/chainsaw-test.yaml`:
- Around line 117-127: The assertion currently indexes
status.interfaces[0].ipAddress on the VirtualMachineInstance which can fail if
status.interfaces is absent; add a preceding defensive assertion that verifies
the interfaces array exists and has at least one element (e.g. assert
(length(status.interfaces || `[]`) > `0`) before asserting
status.interfaces[0].ipAddress != null) so the subsequent indexing in the
VirtualMachineInstance check is safe and the test is more resilient to delayed
interface population.

In `@packages/core/testing/Makefile`:
- Line 32: Add a .PHONY declaration near the top of the Makefile that lists the
non-file targets, e.g., include ".PHONY: e2e test-chainsaw" so the e2e and
test-chainsaw targets are treated as phony; place this declaration close to the
top of the file (similar to the root Makefile) so it applies before the target
definitions.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ed983188-a5e6-4164-9cd3-7f71841724ab

📥 Commits

Reviewing files that changed from the base of the PR and between 5d72a32 and f24e264.

📒 Files selected for processing (71)
  • .github/workflows/pull-requests.yaml
  • Makefile
  • docs/agents/overview.md
  • hack/e2e-apps/bucket.bats
  • hack/e2e-apps/clickhouse.bats
  • hack/e2e-apps/etcd.bats
  • hack/e2e-apps/external-dns.bats
  • hack/e2e-apps/foundationdb.bats
  • hack/e2e-apps/gateway.bats
  • hack/e2e-apps/harbor.bats
  • hack/e2e-apps/kafka.bats
  • hack/e2e-apps/kubernetes-latest.bats
  • hack/e2e-apps/kubernetes-previous.bats
  • hack/e2e-apps/kuberture.bats
  • hack/e2e-apps/mariadb.bats
  • hack/e2e-apps/mongodb.bats
  • hack/e2e-apps/openbao.bats
  • hack/e2e-apps/postgres.bats
  • hack/e2e-apps/qdrant.bats
  • hack/e2e-apps/redis.bats
  • hack/e2e-apps/vminstance.bats
  • hack/e2e-chainsaw/.chainsaw.yaml
  • hack/e2e-chainsaw/.gitignore
  • hack/e2e-chainsaw/README.md
  • hack/e2e-chainsaw/_lib/remediation-guard.sh
  • hack/e2e-chainsaw/_lib/run-kubernetes.sh
  • hack/e2e-chainsaw/bucket/bucket.yaml
  • hack/e2e-chainsaw/bucket/chainsaw-test.yaml
  • hack/e2e-chainsaw/clickhouse/chainsaw-test.yaml
  • hack/e2e-chainsaw/clickhouse/clickhouse.yaml
  • hack/e2e-chainsaw/etcd/chainsaw-test.yaml
  • hack/e2e-chainsaw/etcd/etcd-backup.yaml
  • hack/e2e-chainsaw/etcd/etcd-basic.yaml
  • hack/e2e-chainsaw/etcd/etcd-empty-backup.yaml
  • hack/e2e-chainsaw/external-dns/chainsaw-test.yaml
  • hack/e2e-chainsaw/external-dns/externaldns-inmemory.yaml
  • hack/e2e-chainsaw/external-dns/externaldns-prefix.yaml
  • hack/e2e-chainsaw/foundationdb/chainsaw-test.yaml
  • hack/e2e-chainsaw/foundationdb/foundationdb.yaml
  • hack/e2e-chainsaw/gateway/chainsaw-test.yaml
  • hack/e2e-chainsaw/gateway/gw-minimal.yaml
  • hack/e2e-chainsaw/gateway/gw-route-probe.yaml
  • hack/e2e-chainsaw/gateway/package-attach-probe.yaml
  • hack/e2e-chainsaw/harbor/chainsaw-test.yaml
  • hack/e2e-chainsaw/harbor/harbor.yaml
  • hack/e2e-chainsaw/kafka/chainsaw-test.yaml
  • hack/e2e-chainsaw/kafka/kafka.yaml
  • hack/e2e-chainsaw/kubernetes-latest/chainsaw-test.yaml
  • hack/e2e-chainsaw/kubernetes-previous/chainsaw-test.yaml
  • hack/e2e-chainsaw/kuberture/chainsaw-test.yaml
  • hack/e2e-chainsaw/kuberture/package.yaml
  • hack/e2e-chainsaw/kuberture/probes.yaml
  • hack/e2e-chainsaw/mariadb/chainsaw-test.yaml
  • hack/e2e-chainsaw/mariadb/mariadb.yaml
  • hack/e2e-chainsaw/mongodb/chainsaw-test.yaml
  • hack/e2e-chainsaw/mongodb/mongodb.yaml
  • hack/e2e-chainsaw/openbao/chainsaw-test.yaml
  • hack/e2e-chainsaw/openbao/openbao.yaml
  • hack/e2e-chainsaw/postgres/chainsaw-test.yaml
  • hack/e2e-chainsaw/postgres/postgres.yaml
  • hack/e2e-chainsaw/qdrant/chainsaw-test.yaml
  • hack/e2e-chainsaw/qdrant/qdrant.yaml
  • hack/e2e-chainsaw/redis/chainsaw-test.yaml
  • hack/e2e-chainsaw/redis/redis.yaml
  • hack/e2e-chainsaw/vminstance/chainsaw-test.yaml
  • hack/e2e-chainsaw/vminstance/vmdisk-vmi.yaml
  • hack/e2e-chainsaw/vminstance/vmdisk.yaml
  • hack/e2e-chainsaw/vminstance/vminstance.yaml
  • hack/remediation-guard.bats
  • packages/core/testing/Makefile
  • packages/core/testing/images/e2e-sandbox/Dockerfile
💤 Files with no reviewable changes (18)
  • hack/e2e-apps/kafka.bats
  • hack/e2e-apps/qdrant.bats
  • hack/e2e-apps/external-dns.bats
  • hack/e2e-apps/postgres.bats
  • hack/e2e-apps/vminstance.bats
  • hack/e2e-apps/redis.bats
  • hack/e2e-apps/harbor.bats
  • hack/e2e-apps/kubernetes-latest.bats
  • hack/e2e-apps/etcd.bats
  • hack/e2e-apps/gateway.bats
  • hack/e2e-apps/clickhouse.bats
  • hack/e2e-apps/kubernetes-previous.bats
  • hack/e2e-apps/mariadb.bats
  • hack/e2e-apps/openbao.bats
  • hack/e2e-apps/mongodb.bats
  • hack/e2e-apps/foundationdb.bats
  • hack/e2e-apps/kuberture.bats
  • hack/e2e-apps/bucket.bats

Comment thread hack/e2e-chainsaw/etcd/chainsaw-test.yaml
Comment thread hack/e2e-chainsaw/kuberture/chainsaw-test.yaml
Comment thread hack/e2e-chainsaw/kuberture/chainsaw-test.yaml
Comment thread hack/e2e-chainsaw/mongodb/chainsaw-test.yaml Outdated
Comment thread hack/e2e-chainsaw/postgres/chainsaw-test.yaml
Comment thread hack/e2e-chainsaw/redis/chainsaw-test.yaml
Comment thread hack/e2e-chainsaw/redis/chainsaw-test.yaml Outdated
@myasnikovdaniil

Copy link
Copy Markdown
Contributor Author

Postponed out of the immediate #2397 push and tracked in #2979. Converting to draft — the branch feat/e2e-chainsaw-example stays as the reference implementation for the migration. See #2979 for scope and open questions (incl. the Chainsaw v0.2.15 JMESPath number-literal workaround).

@myasnikovdaniil
myasnikovdaniil marked this pull request as draft June 22, 2026 14:16
@myasnikovdaniil
myasnikovdaniil force-pushed the feat/e2e-chainsaw-example branch from 36e00f0 to e19499d Compare June 23, 2026 12:10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NOT LGTM — the migration is well-structured and CI is wired correctly, but three confirmed defects would make individual suites fail or flake once this replaces the BATS gate.

Business context: replaces the per-app BATS E2E loop with declarative Kyverno Chainsaw suites (one dir per app) and rewires the PR/release E2E jobs to run chainsaw test, growing the postgres/bucket pilot into the full suite.

What I verified holds up well: all 18 active BATS suites are ported 1:1 (none silently dropped); the disabled backup suite stays disabled in both, so no CI coverage is lost; the new Run E2E tests step is a real gate (exit 1 on failure, no continue-on-error); the Test-Impact selector is reworked to discover suites under hack/e2e-chainsaw/ with its unit tests updated in lockstep; the global error.catch host snapshot and per-suite catch diagnostics satisfy the diagnostics-first convention; and run-kubernetes.sh moved with comment/snapshot-path changes only, no dropped teardown ordering. The blockers below are localized, not structural.

Blockers

B1 — openbao cleanup deletes the PVC while the pod still mounts it (deterministic hang/failure)

File: hack/e2e-chainsaw/openbao/chainsaw-test.yaml:113-118

Evidence: the last step's finally deletes PVC data-openbao-test-0, but the OpenBAO CR is applied back in the create-openbao step (:24-25) and is never deleted before this. Chainsaw runs a step's finally before the test-level auto-cleanup that deletes applied resources, so at finally time the StatefulSet pod openbao-test-0 is still Running and still mounts the PVC. The kubernetes.io/pvc-protection finalizer then blocks PVC removal, and a Chainsaw delete waits for actual deletion up to the delete timeout (.chainsaw.yaml → 5m), so the operation times out and fails the test. The BATS original avoided this by deleting the OpenBAO CR first, then the PVC (hack/e2e-apps/openbao.bats teardown: kubectl delete openbao … ; kubectl delete pvc …).

Impact: the openbao suite fails (or hangs ~5m then fails) on every run.

Fix: delete the OpenBAO CR in the finally before the PVC (mirroring BATS), or drop the explicit PVC delete and let it go once the workload is torn down.

B2 — harbor BucketClaim readiness budget halved from a value the BATS comment says is needed

File: hack/e2e-chainsaw/harbor/chainsaw-test.yaml:54-61

Evidence: the Chainsaw assert gives BucketClaim … bucketReady: true a timeout: 5m. The BATS original used --timeout=600s (10m) with an explicit rationale: "readiness back onto the namespaced BucketClaim can lag several minutes on a loaded runner, so allow the same 10m budget the dependent HelmRelease gets" (hack/e2e-apps/harbor.bats:48-52). The seaweedfs/COSI provisioning this waits on is exactly the slow path the convention calls out (docs/agents/e2e-testing.md §6 lists harbor at 10m as a justified longer wait). The dependent harbor-test-system HR is correctly kept at 10m here, but the BucketClaim it depends on is now capped at 5m.

Impact: intermittent false-flake timeouts on loaded runners — reintroducing the flakiness this migration exists to remove.

Fix: restore the BucketClaim assert timeout to 10m.

B3 — the last etcd scenario is never torn down (skipDelete: true with no trailing cleanup)

File: hack/e2e-chainsaw/etcd/chainsaw-test.yaml:170-272

Evidence: all three etcd Tests set skipDelete: true and rely on the next scenario's ensure-clean step to drain the previous one (file header :8-16). The final Test etcd-backup-schedule has no scenario after it, so its EtcdCluster, HelmRelease, Kamaji DataStore, and pods stay running. The file header itself documents that this DataStore "leaks a Kamaji finalizer on uninstall … wedging the DataStore in Terminating and hanging the HelmRelease uninstall" — leaving that resource un-drained at the end of the suite contradicts the suite's own teardown design and docs/agents/e2e-testing.md §4 (do not leave stale finalizer-heavy state for subsequent suites).

Impact: on full-suite / release runs the orphaned etcd + wedged DataStore persist through every later suite; TIA-narrowed PR runs that select only etcd are unaffected.

Fix: add an explicit teardown for the final scenario (a closing cleanup step that runs the same etcd + DataStore-finalizer drain), or convert the etcd suite to clean up after each scenario instead of before the next.

Non-blocking follow-ups

  1. hack/e2e-chainsaw/kuberture/chainsaw-test.yaml:106-107 — the assert-annotation-shape script asserts the controller-populated …/target annotation is non-empty with a one-shot test -n and no retry, while the preceding wait-output-services step only asserts the Service objects exist (:62-79). If the kuberture controller has not yet copied target off the EndpointSlice, the script fails under set -e. Assert the annotation declaratively (a Chainsaw assert on metadata.annotations) or poll for it, so this is not a race.

  2. I concur with the existing bot nits worth folding in while iterating: the duplicated ensure-clean block across the three etcd Tests (extract to _lib/), mongodb/redis asserting status.replicas rather than status.readyReplicas, and the kuberture probe-Pod debug dump being skipped on timeout because set -e exits on the NotFound before the describe/logs run.

  3. hack/e2e-chainsaw/gateway/chainsaw-test.yaml:537(status.parents[0].conditions[?type == 'Accepted']) is correct (positional parents[0] plus filter-as-list, not the banned (filter)[0] form) and self-heals via polling; a filter over parents by parentRef/controllerName would be marginally more robust to ordering, but this is optional.

@myasnikovdaniil

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review, IvanHunters — all three non-blocking notes are addressed.

[MINOR] SC2148 — added # shellcheck shell=bash to the four sourced _lib/*.sh helpers (etcd-cleanup, remediation-guard, run-kubernetes, talos-image-cache). Commit 8af4ed5.

[MINOR] ports[0] / 0.2.15 coupling — added a # TODO(chainsaw>0.2.15) marker at the postgres Service asserts (also flagging that ports[0] checks only the first port), cross-referencing the same workaround in kafka/mariadb/harbor. Commit 8af4ed5. The full v0.2.15 trap list — including this number-literal-filter workaround, which previously lived only in the PR description — is now documented durably in a new "Chainsaw v0.2.15 gotchas" section in docs/agents/e2e-testing.md (commit 18715f8), alongside a note to revert on a chainsaw bump.

[MINOR] delete.yaml traceability#3271 is the tracking issue for the teardown-hang fix; it was filed just after your review, which is why it didn't resolve at the time — apologies for the timing. Kept the change in this PR since the new kubernetes-* Chainsaw suites are exactly what exercise the path, and added a release-note describing it. The fix is bounded/safe as you analysed. #3271 stays open intentionally: the bound is validated by the render-side e2e case, but the teardown path should be re-verified against a real production tenant before relying on it there.

Deferred follow-ups (non-blocking): revert the ports[*]/ports[0] projections to the number-literal filter form once chainsaw is bumped past 0.2.15, and expand hack/e2e-chainsaw/README.md with the cozytest→Chainsaw onboarding delta.

@myasnikovdaniil
myasnikovdaniil merged commit 99a28d8 into main Jul 12, 2026
76 of 77 checks passed
@myasnikovdaniil
myasnikovdaniil deleted the feat/e2e-chainsaw-example branch July 12, 2026 06:58
myasnikovdaniil added a commit that referenced this pull request Jul 15, 2026
The fork e2e path in e2e-fork.yaml still ran the pre-#2826 BATS app loop
(`hack/e2e-apps/*.bats` + `test-apps-%`), which the merged Chainsaw
migration deleted — so the fork run was already broken. Rewrite it to the
same Chainsaw + Test-Impact-Analysis path as the in-tree job, and clear the
remaining review follow-ups on the publish job.

- Run E2E via a single `make test-chainsaw` invocation (no per-app retry —
  e2e-testing.md §1). Suite selection and the full-e2e override come from
  `resolve`'s TRUSTED base-repo listFiles/labels (new `changed_b64` /
  `full_e2e` outputs), never a git diff of the fork-controlled merge ref, so
  TIA cannot be shaped into skipping the suite (keeps the #3257 property).
  Collect + upload the Chainsaw JUnit report like the in-tree job.
- Restrict fork image pushes to a trusted allowlist derived from the base
  parent (HEAD^1) Makefiles, so a fork cannot mint an arbitrary repo path
  under the CI registry by renaming an image. The charset guard stays as
  defence in depth.
- `skopeo copy --all` so a multi-platform build's index digest (the one
  baked into pr.patch) is preserved.
- Pin the flux CLI (FLUX_VERSION) in the privileged publish job instead of
  install.sh's rolling latest.

Docs: e2e-testing.md §10 records that the fork path now runs Chainsaw + TIA
and that fork suite selection must stay sourced from trusted data.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
myasnikovdaniil added a commit that referenced this pull request Jul 15, 2026
The fork e2e path in e2e-fork.yaml still ran the pre-#2826 BATS app loop
(`hack/e2e-apps/*.bats` + `test-apps-%`), which the merged Chainsaw
migration deleted — so the fork run was already broken. Rewrite it to the
same Chainsaw + Test-Impact-Analysis path as the in-tree job, and clear the
remaining review follow-ups on the publish job.

- Run E2E via a single `make test-chainsaw` invocation (no per-app retry —
  e2e-testing.md §1). Suite selection and the full-e2e override come from
  `resolve`'s TRUSTED base-repo listFiles/labels (new `changed_b64` /
  `full_e2e` outputs), never a git diff of the fork-controlled merge ref, so
  TIA cannot be shaped into skipping the suite (keeps the #3257 property).
  Collect + upload the Chainsaw JUnit report like the in-tree job.
- Restrict fork image pushes to a trusted allowlist derived from the base
  parent (HEAD^1) Makefiles, so a fork cannot mint an arbitrary repo path
  under the CI registry by renaming an image. The charset guard stays as
  defence in depth.
- `skopeo copy --all` so a multi-platform build's index digest (the one
  baked into pr.patch) is preserved.
- Pin the flux CLI (FLUX_VERSION) in the privileged publish job instead of
  install.sh's rolling latest.

Docs: e2e-testing.md §10 records that the fork path now runs Chainsaw + TIA
and that fork suite selection must stay sourced from trusted data.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
myasnikovdaniil added a commit that referenced this pull request Jul 21, 2026
The fork e2e path in e2e-fork.yaml still ran the pre-#2826 BATS app loop
(`hack/e2e-apps/*.bats` + `test-apps-%`), which the merged Chainsaw
migration deleted — so the fork run was already broken. Rewrite it to the
same Chainsaw + Test-Impact-Analysis path as the in-tree job, and clear the
remaining review follow-ups on the publish job.

- Run E2E via a single `make test-chainsaw` invocation (no per-app retry —
  e2e-testing.md §1). Suite selection and the full-e2e override come from
  `resolve`'s TRUSTED base-repo listFiles/labels (new `changed_b64` /
  `full_e2e` outputs), never a git diff of the fork-controlled merge ref, so
  TIA cannot be shaped into skipping the suite (keeps the #3257 property).
  Collect + upload the Chainsaw JUnit report like the in-tree job.
- Restrict fork image pushes to a trusted allowlist derived from the base
  parent (HEAD^1) Makefiles, so a fork cannot mint an arbitrary repo path
  under the CI registry by renaming an image. The charset guard stays as
  defence in depth.
- `skopeo copy --all` so a multi-platform build's index digest (the one
  baked into pr.patch) is preserved.
- Pin the flux CLI (FLUX_VERSION) in the privileged publish job instead of
  install.sh's rolling latest.

Docs: e2e-testing.md §10 records that the fork path now runs Chainsaw + TIA
and that fork suite selection must stay sourced from trusted data.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
myasnikovdaniil added a commit that referenced this pull request Jul 29, 2026
The fork e2e path in e2e-fork.yaml still ran the pre-#2826 BATS app loop
(`hack/e2e-apps/*.bats` + `test-apps-%`), which the merged Chainsaw
migration deleted — so the fork run was already broken. Rewrite it to the
same Chainsaw + Test-Impact-Analysis path as the in-tree job, and clear the
remaining review follow-ups on the publish job.

- Run E2E via a single `make test-chainsaw` invocation (no per-app retry —
  e2e-testing.md §1). Suite selection and the full-e2e override come from
  `resolve`'s TRUSTED base-repo listFiles/labels (new `changed_b64` /
  `full_e2e` outputs), never a git diff of the fork-controlled merge ref, so
  TIA cannot be shaped into skipping the suite (keeps the #3257 property).
  Collect + upload the Chainsaw JUnit report like the in-tree job.
- Restrict fork image pushes to a trusted allowlist derived from the base
  parent (HEAD^1) Makefiles, so a fork cannot mint an arbitrary repo path
  under the CI registry by renaming an image. The charset guard stays as
  defence in depth.
- `skopeo copy --all` so a multi-platform build's index digest (the one
  baked into pr.patch) is preserved.
- Pin the flux CLI (FLUX_VERSION) in the privileged publish job instead of
  install.sh's rolling latest.

Docs: e2e-testing.md §10 records that the fork path now runs Chainsaw + TIA
and that fork suite selection must stay sourced from trusted data.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
myasnikovdaniil added a commit that referenced this pull request Jul 31, 2026
Backport of #3344 onto release-1.5.

The bot's cherry-pick imported hack/e2e-chainsaw/mariadb/chainsaw-test.yaml
whole, because release-1.5 has no such file: the app suite here is still
BATS, and the Chainsaw port landed with #2826 on main only. That file is
dropped and its widening applied to the gate that actually runs on this
branch instead.

That gate needed the change more than the chainsaw one did. In
hack/e2e-apps/mariadb.bats both tests wait 80s for an endpoint address to
appear, and an address appears only once a replica has cleared its startup
then readiness probe. This commit raises the startup budget to 310s, so an
80s ceiling would fail runs the probe was still willing to wait for --
importing the chainsaw file and leaving the BATS suite alone would have
turned the fix into an e2e failure. Both waits go to 600s, matching the
10m the upstream chainsaw assert uses and for the same reason.

packages/apps/mariadb/Makefile gains the `test:` target. It is absent on
release-1.5 -- it arrived with #3071, which was not backported -- and
hack/helm-unit-tests.sh discovers suites by probing for that target, so
without it the backported tests/startup_probe_test.yaml would never be
executed by `make unit-tests`.

Verified locally: 4 assertions in tests/startup_probe_test.yaml pass.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
(cherry picked from commit a37715062e5da24e54956f075dd2d6417c4dff2e)
Aleksei Sviridkin (lexfrei) pushed a commit that referenced this pull request Aug 8, 2026
)

## What this PR does

Fixes #3257 — fork PRs never ran e2e, and the skipped required check let
them merge anyway.

Fork PRs get no secrets on `pull_request`, so `make image` pushed
anonymously and died with `denied`. That failed every build, collapsed
the `build → finalize → e2e` chain, and left the required **E2E Tests**
job *skipped* — which branch protection treats as satisfied, so an
approved fork PR could merge with e2e never having run.

This implements **Option A** from the issue (privileged `workflow_run`
split), pushing fork e2e images to the existing CI registry (OCIR):

**Build primitive (`hack/common-envs.mk`)**
- New `OCI_EXPORT_DIR` mode: build each image to a per-image OCI archive
instead of pushing, and force `PUSH`/`LOAD` off. The digest is captured
via `--metadata-file` regardless of output type, so the refs baked into
`pr.patch` match what the privileged run later pushes. Threaded through
the `image-tags` macro every package already uses — no per-package
Makefile change.

**`pull-requests.yaml` (unprivileged, fork branch)**
- `build` / `build-talos` / `finalize` export images to OCI archives and
upload them as artifacts instead of pushing → the red `denied` build
wall disappears and the chain no longer collapses.
- The in-tree e2e job is renamed off `"E2E Tests"` and guarded to
same-repo PRs. A new `e2e-report` job concludes the required `"E2E
Tests"` context as an **explicit check-run** (success on a docs-only PR
or when e2e passed, failure otherwise). A skipped job can no longer
satisfy the required check — this also closes the same hole for
same-repo PRs whose build fails.

**`e2e-fork.yaml` (new, privileged `workflow_run` from the default
branch)**
- Resolves the fork PR, opens a pending `"E2E Tests"` check-run, and
fails-closed if the fork build failed.
- Pushes the pre-built image archives to OCIR **by digest** and
flux-pushes the digest-pinned packages tree. The OCIR token is live only
for trusted binaries (`git`/`skopeo`/`flux`/`yq`) over data — never for
`make` or any fork-authored script.
- Runs the e2e suite against the pushed closure with **no push token**
and concludes the check-run on the PR head SHA.

Fork build code never receives registry credentials; the fork's images
and test scripts run only in the credential-less, ephemeral e2e job — a
stronger posture than a `pull_request_target` run that would hand fork
code the secrets directly.

### Validation

Local: buildx OCI-archive output + digest capture, macro expansion for
single- and multi-image packages, non-fork push path unchanged,
`actionlint` clean.

Needs live validation (why this is a **draft**): `workflow_run` artifact
download across runs; that the `"E2E Tests"` commit status satisfies the
required context and keeps a fork PR non-mergeable until the privileged
run concludes it; `refs/pull/<N>/merge` fetch + `git apply --3way`;
`skopeo --preserve-digests` end-to-end. `workflow_run` only runs from
the default branch, so the privileged half is exercised by mirroring the
workflow onto a personal fork's default branch and opening a throwaway
fork PR into it — not by merging first.

_Since review_: the fork `e2e` job ran the pre-#2826 BATS app loop that
the merged Chainsaw migration deleted (already broken); it now runs the
same Chainsaw + TIA path as the in-tree job, with suite selection and
the `full-e2e` override sourced from `resolve`'s trusted base-repo file
list / labels (not a merge-ref diff). Publish job hardened per the
non-blocking follow-ups: base-tree image allowlist, `skopeo copy --all`,
pinned flux CLI. The required `E2E Tests` gate is now published as a
commit **status** (default token + `statuses:write`), not a COZYSTACK_CI
app check-run — the app lacked `checks:write` (`checks.create` 403'd)
and a default-token check-run floated under a labeler suite; a commit
status has neither problem, and forks can't post one (their
`pull_request` token is read-only). Two review-bot findings fixed too:
fork `OCI_EXPORT_DIR` is now absolute (a relative path misplaced
archives under `make -C <pkg>` and every fork build would fail at
upload), and the "≥1 archive" guard no longer no-ops under `nullglob`.

### Repo-admin prerequisites

One repository setting lives outside this diff:

1. **Branch protection: keep `E2E Tests` required.** The gate is now a
commit **status** named `E2E Tests` (posted with the default
`GITHUB_TOKEN` — see `docs/agents/e2e-testing.md` §10), not a job or an
app check-run. Branch protection already requires `E2E Tests` (currently
the e2e job's name, renamed here to `E2E (in-tree)`), so the required
context now resolves to the status — **no GitHub App, no `checks:write`
grant, no app-id pin needed**. Just don't pin the context to a specific
app.
2. **Actions → "Require approval for all outside collaborators".** Each
fork run must be a deliberate maintainer click before it executes, since
the fork's unprivileged run produces the artifacts the privileged
`e2e-fork.yaml` consumes.

### Downstream repositories

Walked the trigger map in `docs/agents/contributing.md` against the diff
(7 files: two CI workflows, `docs/agents/e2e-testing.md`,
`hack/common-envs.mk` + its bats suite, and two package `Makefile`s).

The one trigger worth naming is "change developer tooling
(`cozyvalues-gen`, `cozypkg`, the package `Makefile`s) →
`development.md`" on the website. It does not fire here:
`OCI_EXPORT_DIR` is a CI-only build mode that defaults off, and the
local developer workflow is unchanged — `make image` still builds and
pushes exactly as before, which the new default-path case in
`hack/common-envs_test.bats` now pins. No chart values, no API surface,
no app list, no platform variant, no Talos version bump and no release
asset rename are touched.

- [x] No downstream repository is affected by this change

### Release note

```release-note
fix(ci): fork pull requests now run e2e via a privileged workflow, and a fork PR can no longer merge on a skipped (never-run) required E2E Tests check
```

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Added a dedicated fork E2E workflow that runs after the base PR and
reports results via the required **E2E Tests** commit status.
- Fork PRs now export build images as OCI archive artifacts for later
publishing.
- Added a same-repo E2E reporting job and renamed the in-tree E2E flow
to **E2E (in-tree)**.
- **Bug Fixes**
- Added tighter, fail-closed gating to ensure merge blocking when **E2E
Tests** isn’t correctly finalized.
- **Documentation**
- Documented fork vs same-repo E2E split, OCI export behavior, and
required fork security settings.
- **Tests**
- Added Bats coverage validating `OCI_EXPORT_DIR` and preventing
unintended registry pushes.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
myasnikovdaniil added a commit that referenced this pull request Aug 15, 2026
hack/e2e-apps/monitoring-oidc-system.bats and its customconfig twin
have never executed. #2826 deleted the `test-apps-%:` rule that ran
hack/e2e-apps/$*.bats along with every other file in that directory;
#3176 landed three days later and added these two into it. Git raises
no conflict when one branch empties a directory and another adds files
to it, so the only signal was a directory name that still looked live.

They could not pass as written either. hack/cozytest.sh defines no
skip(), so the `skip` guarding the Keycloak assertions is a
command-not-found and exit 127, and `kubectl api-resources
--api-group=v1.edp.epam.com` exits 0 whether or not the group is
served, so the guard was dead code in both directions. cac07db found
and dropped both when it ported the kubernetes-oidc twins out of the
same race.

The render-side coverage is superseded, strictly, by
packages/system/monitoring/tests/oidc_test.yaml -- 32 helm-unittest
cases over the same Phase-1 selector, several of them asserting more
than a live test can. `spec.public: false` is the example: the EDP CRD
strips the field off the applied object, which the deleted bats said
so itself.

Four comments described hack/e2e-apps/ as a live directory; they now
describe the shape rather than the instance, and two of them say what
an e2e- prefix does not mean, since arming the cluster captures was
never a claim that a runner exists.

Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
myasnikovdaniil added a commit that referenced this pull request Aug 15, 2026
Two suites sat in hack/e2e-apps/ for three weeks without executing
once, and nothing in the tree could notice: the property "something
runs this file" was described in prose in three comments and asserted
nowhere. This adds the assertion.

There are exactly two runners and that is a closed list by design --
the root Makefile's one-level, non-e2e glob, and the literal suite
names in packages/core/testing/Makefile. A file matching neither is
reported, as is the inverse: a name in the e2e Makefile with no file
on disk, which today surfaces forty minutes into the e2e job, or wedges
the bootstrap outright. A suite named through a make variable, the
shape of the rule #2826 deleted, is reported as unresolvable rather
than expanded, because a pattern rule nobody invokes is as dead as no
rule.

The Makefiles are parsed rather than asked. hack/common-envs.mk is
included at Makefile:3 and runs `git remote add upstream` and `git
fetch upstream --tags` in $(shell ...) at parse time, so `make -p` or
`make -n` would have a unit test mutate git remotes and reach the
network to answer a question about filenames. The cost is that the
model can drift from what it models, so a second test pins the modelled
lines verbatim -- that is the one direction which would otherwise widen
the covered set in silence.

No allowlist, deliberately: the off-switch is the .disabled suffix,
which the scan stops seeing on its own. An in-test list of exempt paths
would be the same artefact class as the runner that got deleted while
its documentation stayed.

That documentation is corrected here too. Makefile's comment claimed a
new .bats file in hack/ is picked up automatically, which holds only at
the top level and is the false claim this bug grew in.

Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci Issues or PRs related to CI workflows, GitHub Actions, automation area/testing Issues or PRs related to testing (e2e, bats, unit tests) kind/cleanup Categorizes issue or PR as related to cleanup of code, process, or technical debt kind/feature Categorizes issue or PR as related to a new feature size/XXL This PR changes 1000+ lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants