Skip to content

Commit 20e4eab

Browse files
authored
test: make negative bats assertions fail under cozytest (#3132)
## What this PR does Every `*.bats` file in the repo runs through `hack/cozytest.sh`, not real bats — the e2e files via `packages/core/testing/Makefile` and the `hack/*.bats` unit tests via `make bats-unit-tests`. cozytest runs each `@test` under `set -e`, and POSIX errexit is suppressed for a pipeline beginning with `!`. So a bare `! cmd` must-not-happen assertion never fails the test: when the forbidden condition actually occurs (the command succeeds), the negated pipeline returns non-zero but errexit ignores it and the test passes anyway. Ten such assertions across six files were silently vacuous. This converts each to `if cmd; then echo "<reason>"; false; fi`. The `if` condition is exempt from errexit (so the probe itself never aborts the test), and the `false` in the taken branch is a plain command under `set -e`, so it aborts and fails the test with a legible message when the forbidden condition holds. This is the must-not-happen inverse of the `cmd || { echo …; false; }` pattern already used in the suite (e.g. harbor.bats, per docs/agents/e2e-testing.md). Original redirections and quoting are preserved, so happy-path behavior is unchanged — only the failure path now actually fails. Converted assertions: - `bucket`: a read-only user must not be able to upload (access-control regression guard) - `gateway`: a child tenant must not have its own gateway HelmRelease (inheritance) - `kuberture`: external-dns public/internal hostname isolation - `e2e-install-cozystack`: tenant-name validation must reject rather than warn; the version ConfigMap must be gone after delete - `build-matrix` (unit test): talos/installer must be excluded from the parallel matrix - `capture-dataplane` (unit test): kube-dns and pending rows must be filtered out A separate in-flight etcd test change owns two more such assertions in `etcd.bats`; they are left out of this PR to avoid a conflicting edit. ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Strengthened several end-to-end/Bats assertions to fail more explicitly when expected cluster conditions aren’t met. * Improved matrix validation to reliably confirm excluded components are absent. * Updated dataplane load balancer filtering, bucket readonly upload, and gateway inheritance checks to avoid false/ambiguous pass cases. * Tightened DNS hostname isolation probes and cozystack installer validations (tenant name dashes, deletion protection) with clearer “expected vs found” failures. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents 925cb87 + 12fe325 commit 20e4eab

6 files changed

Lines changed: 33 additions & 13 deletions

File tree

‎hack/build-matrix_test.bats‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616

1717
@test "talos and installer are excluded from the parallel matrix" {
1818
out=$(hack/build-matrix.sh)
19-
! echo "$out" | grep -q '"packages/core/talos"'
20-
! echo "$out" | grep -q '"packages/core/installer"'
19+
# `! cmd` would be vacuous: cozytest.sh runs each @test under `set -e`, which
20+
# is suppressed for a `!`-negated pipeline, so a regression that wrongly
21+
# included these paths would not fail the test. Assert via `if cmd; then ...`.
22+
if echo "$out" | grep -q '"packages/core/talos"'; then echo "FAIL: packages/core/talos must be excluded from the parallel matrix"; false; fi
23+
if echo "$out" | grep -q '"packages/core/installer"'; then echo "FAIL: packages/core/installer must be excluded from the parallel matrix"; false; fi
2124
}
2225

2326
@test "talos-only diff selects nothing (handled by the dedicated leg)" {

‎hack/capture-dataplane.bats‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,11 @@ E2E_CAPTURE_DATAPLANE_LIB=1
5353
[ "$(printf '%s\n' "$out" | grep -c .)" -eq 2 ]
5454
printf '%s\n' "$out" | grep -q '^tenant|app|LoadBalancer|192.0.2.50|'
5555
printf '%s\n' "$out" | grep -q '^tenant|db|LoadBalancer|192.0.2.51|'
56-
! printf '%s\n' "$out" | grep -q 'kube-dns'
57-
! printf '%s\n' "$out" | grep -q 'pending'
56+
# `! cmd` is vacuous under cozytest's `set -e` (errexit is suppressed for a
57+
# `!`-negated pipeline), so a filter regression that let these rows through
58+
# would not fail the test. Assert the absence via `if cmd; then ...; false`.
59+
if printf '%s\n' "$out" | grep -q 'kube-dns'; then echo "FAIL: lb_filter_services must drop the kube-dns row"; false; fi
60+
if printf '%s\n' "$out" | grep -q 'pending'; then echo "FAIL: lb_filter_services must drop the pending (no external IP) row"; false; fi
5861
}
5962

6063
@test "lb_first_ready_endpoint picks the first addressed endpoint that is not NotReady" {

‎hack/e2e-apps/bucket.bats‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,12 @@ EOF
8585
# Viewer can download
8686
mc cp --insecure ro-user/$BUCKET_NAME/rw-test.txt /tmp/ro-test-download.txt
8787

88-
# Viewer cannot upload (must fail with Access Denied)
88+
# Viewer cannot upload (must fail with Access Denied). A bare `! mc cp` is
89+
# vacuous under cozytest's `set -e` (errexit is suppressed for a `!` pipeline),
90+
# so a real access-control regression that let the upload through would pass
91+
# silently — assert via `if mc cp; then ...; false` so it fails loudly.
8992
echo "readonly test" > /tmp/ro-test.txt
90-
! mc cp --insecure /tmp/ro-test.txt ro-user/$BUCKET_NAME/ro-test.txt
93+
if mc cp --insecure /tmp/ro-test.txt ro-user/$BUCKET_NAME/ro-test.txt; then echo "FAIL: read-only user must NOT be able to upload (bucket access-control regression)"; false; fi
9194

9295
# --- Cleanup ---
9396
mc rm --insecure rw-user/$BUCKET_NAME/rw-test.txt

‎hack/e2e-apps/gateway.bats‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -540,8 +540,11 @@ EOF
540540
echo "$cozyvalues" | grep -E '^\s*gateway:\s*"tenant-test-gwparent"\s*$' >/dev/null
541541

542542
# The child must NOT have its own gateway HelmRelease — inheritance
543-
# means no separate Gateway resource for the child tenant.
544-
! kubectl -n tenant-test-gwparent-gwchild get helmrelease gateway 2>/dev/null
543+
# means no separate Gateway resource for the child tenant. A bare `! kubectl`
544+
# is vacuous under cozytest's `set -e` (suppressed for a `!` pipeline), so if
545+
# the child wrongly got its own HelmRelease the test would still pass; assert
546+
# via `if kubectl get; then ...; false`.
547+
if kubectl -n tenant-test-gwparent-gwchild get helmrelease gateway 2>/dev/null; then echo "FAIL: child tenant must NOT have its own gateway HelmRelease (gateway inheritance broken)"; false; fi
545548

546549
# Teardown child before parent, waiting out each uninstall: deleting
547550
# the parent while the child is still uninstalling wedges the parent's

‎hack/e2e-apps/kuberture.bats‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,12 +244,15 @@ EOF
244244
# kuberture-e2e-public.cozystack.test (kuberture-e2e-public Service carries
245245
# external-dns.alpha.kubernetes.io/hostname=...) and must NOT see the
246246
# internal hostname (kuberture-e2e-internal carries only the internal prefix).
247+
# A bare `! echo | grep` is vacuous under cozytest's `set -e` (errexit is
248+
# suppressed for a `!` pipeline), so a leak of the wrong hostname into a probe
249+
# would not fail the test; assert the absence via `if ... grep; then ...; false`.
247250
echo "${pub_logs}" | grep -F "${pub_host}" >/dev/null
248-
! echo "${pub_logs}" | grep -F "${int_host}" >/dev/null
251+
if echo "${pub_logs}" | grep -F "${int_host}" >/dev/null; then echo "FAIL: public-prefix probe must NOT see the internal hostname ${int_host}"; false; fi
249252

250253
# And the inverse for the internal-prefix probe.
251254
echo "${int_logs}" | grep -F "${int_host}" >/dev/null
252-
! echo "${int_logs}" | grep -F "${pub_host}" >/dev/null
255+
if echo "${int_logs}" | grep -F "${pub_host}" >/dev/null; then echo "FAIL: internal-prefix probe must NOT see the public hostname ${pub_host}"; false; fi
253256

254257
# _cleanup deletes the probe Pods, their SA, the ClusterRole+ClusterRoleBinding,
255258
# and finally the Package CR. Inlined here for the success path so a re-run

‎hack/e2e-install-cozystack.bats‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,10 @@ EOF
289289
# generic DNS-1035 errors and from network/auth failures).
290290
echo "$output" | grep -q "tenant names must"
291291
# And assert kubectl did NOT report creation — if validation regressed
292-
# into a "warn" variant, the server could still accept the object.
293-
! echo "$output" | grep -qi "created"
292+
# into a "warn" variant, the server could still accept the object. A bare
293+
# `! echo | grep` is vacuous under cozytest's `set -e` (suppressed for a `!`
294+
# pipeline), so the regression would slip through; assert via `if ...; false`.
295+
if echo "$output" | grep -qi "created"; then echo "FAIL: kubectl reported the tenant as created — validation must reject it, not warn"; false; fi
294296

295297
# Post-condition cleanup: even though we expect validation to reject the
296298
# create, removing foo-bar unconditionally keeps the cluster clean for
@@ -395,7 +397,10 @@ EOF
395397
version=$(kubectl get configmap cozystack-version -n cozy-system \
396398
-o jsonpath='{.data.version}')
397399
kubectl delete configmap cozystack-version -n cozy-system
398-
! kubectl get configmap cozystack-version -n cozy-system 2>/dev/null
400+
# A bare `! kubectl get` is vacuous under cozytest's `set -e` (errexit is
401+
# suppressed for a `!` pipeline), so a delete that silently failed would not
402+
# fail the test; assert the absence via `if kubectl get; then ...; false`.
403+
if kubectl get configmap cozystack-version -n cozy-system 2>/dev/null; then echo "FAIL: cozystack-version configmap must be gone after delete with the no-delete label removed"; false; fi
399404
# Reconstruct: declarative apply matches the chart template at
400405
# packages/core/platform/templates/cozystack-version.yaml — same label set
401406
# AND the helm.sh/resource-policy: keep annotation that pins the ConfigMap

0 commit comments

Comments
 (0)