feat: BAN ACL + handler (#39)

cmd/operator/main.go

@@ -212,6 +212,11 @@ func main() {
 			HTTPClient: &http.Client{Timeout: agentClientTimeout},
 			K8sClient:  mgr.GetClient(),
 		},
+		// POD_IP is injected by the Helm chart via the downward API.
+		// Empty is safe (the ACL template and EndpointSlice reconciler both
+		// no-op on empty), but it means invalidation-proxy PURGE/BAN requests
+		// are rejected by Varnish's ACL — so in production this must be set.
+		OperatorIP:  os.Getenv("POD_IP"),
 		ProxyRouter: proxyRouter,
 		ProxyPodMap: proxyPodMap,
 	}).SetupWithManager(mgr); err != nil {

docs/sources/reference/vinylcache-spec.md

@@ -70,9 +70,12 @@ See the [per-backend directors how-to](../how-to/per-backend-directors.md) for w
 | `shard.by` | string | `HASH` | Shard key source (`HASH`, `URL`). |
 | `shard.healthy` | string | `CHOSEN` | Health evaluation strategy (`CHOSEN`, `ALL`). |
 
-> Note: `shard.by` and `shard.healthy` are accepted and persisted on the
-> resource, but are not yet consumed by the generator for per-backend
-> directors (see follow-up `shard-params-not-plumbed`).
+> Note on `shard.by` and `shard.healthy`:
+> - **Cluster-peer director** (when `cluster.enabled: true`): honored automatically —
+>   the operator emits `<director>.backend(by=<by>, healthy=<healthy>)` in `vcl_recv`.
+> - **Per-backend directors**: still request-time arguments. The CRD accepts
+>   the fields but you must use them in your own VCL snippet, e.g.
+>   `set req.backend_hint = plone.backend(by=HASH, healthy=CHOSEN);`.
 
 ### cluster
 
@@ -86,8 +89,25 @@ See the [per-backend directors how-to](../how-to/per-backend-directors.md) for w
 | Field | Type | Default | Description |
 |-------|------|---------|-------------|
 | `purge.soft` | boolean | `true` | Use soft purge (stale-while-revalidate). |
+| `purge.allowedSources` | list | `[]` | CIDRs permitted to send `PURGE`. `127.0.0.1` and the operator pod IP are always included. |
 | `xkey` | object | nil | Xkey (surrogate key) configuration. When set, `vmod_xkey` is loaded. |
 | `xkey.softPurge` | boolean | `true` | Use soft purge for xkey invalidation. |
+| `ban.enabled` | boolean | `false` | When `true`, emit a `vinyl_ban_allowed` ACL and a `BAN` handler in `vcl_recv` that dispatches `std.ban(req.http.X-Vinyl-Ban)`. Also emits ban-lurker-friendly `x-url`/`x-host` headers on stored objects. |
+| `ban.allowedSources` | list | `[]` | CIDRs permitted to send `BAN` (in addition to `127.0.0.1` and the operator pod IP). |
+| `ban.rateLimitPerMinute` | integer | `0` | Inert in v0.4.2 — field is accepted but not enforced; rate-limiting is tracked as follow-up work. |
+
+> Note on BAN security: any client whose source IP is in `vinyl_ban_allowed`
+> can invalidate the entire cache with an arbitrary ban expression. Scope
+> `ban.allowedSources` tightly to trusted callers only. Ban expressions
+> should prefer `obj.http.x-url` / `obj.http.x-host` over `req.*` fields —
+> only the former can be processed by the ban lurker, so `req.*` bans
+> accumulate without being compacted.
+>
+> Clients send BAN requests like:
+>
+> ```bash
+> curl -X BAN -H 'X-Vinyl-Ban: obj.http.x-url ~ "^/news/"' http://varnish.svc/
+> ```
 
 ## Status fields
 

e2e/tests/vcl-validation/chainsaw-test.yaml

@@ -11,6 +11,7 @@ spec:
     apply: 10s
     assert: 60s
     delete: 60s
+    cleanup: 60s
     exec: 30s
   steps:
     - name: deploy-backend

internal/generator/generator.go

@@ -61,6 +61,7 @@ type TemplateData struct {
 	HasESI           bool
 	HasXkey          bool
 	HasSoftPurge     bool
+	HasBAN           bool
 	HasProxyProtocol bool
 	HasFullOverride  bool
 	VCLName          string         // sanitized name for vcl declaration
@@ -199,6 +200,9 @@ func buildTemplateData(input Input) TemplateData {
 	if input.Spec.Invalidation.Purge != nil {
 		data.HasSoftPurge = input.Spec.Invalidation.Purge.Soft
 	}
+	if input.Spec.Invalidation.BAN != nil {
+		data.HasBAN = input.Spec.Invalidation.BAN.Enabled
+	}
 
 	// ESI: check VarnishParams for explicit feature flag.
 	_, hasESI := input.Spec.VarnishParams["feature +esi"]

internal/generator/generator_test.go

@@ -835,3 +835,124 @@ func TestGenerate_OperatorIP_CoexistsWithAllowedSources(t *testing.T) {
 	assert.Contains(t, r.VCL, `"192.168.1.0/24";`)
 	assert.Contains(t, r.VCL, `"127.0.0.1";`)
 }
+
+func TestGenerate_BAN_DisabledByDefault(t *testing.T) {
+	g := newGenerator(t)
+	r, err := g.Generate(makeMinimalInput())
+	require.NoError(t, err)
+	assert.NotContains(t, r.VCL, "acl vinyl_ban_allowed",
+		"BAN ACL must NOT be emitted when spec.invalidation.ban is unset")
+	assert.NotContains(t, r.VCL, `req.method == "BAN"`,
+		"BAN handler must NOT be emitted when ban is disabled")
+}
+
+func TestGenerate_BAN_EnabledEmitsACLAndHandler(t *testing.T) {
+	g := newGenerator(t)
+	input := makeMinimalInput()
+	input.Spec.Invalidation.BAN = &vinylv1alpha1.BANSpec{Enabled: true}
+	r, err := g.Generate(input)
+	require.NoError(t, err)
+
+	// Extract just the BAN block so assertions don't accidentally match
+	// substrings emitted elsewhere in the VCL.
+	banBlock := strings.SplitAfter(r.VCL, `if (req.method == "BAN")`)[1]
+	banBlock = strings.SplitN(banBlock, "# Only handle GET", 2)[0]
+
+	assert.Contains(t, r.VCL, "acl vinyl_ban_allowed {",
+		"BAN ACL must be emitted when ban.enabled=true")
+	assert.Contains(t, banBlock, `client.ip ~ vinyl_ban_allowed`,
+		"BAN handler must check the BAN ACL")
+	assert.Contains(t, banBlock, `synth(403, "Forbidden")`,
+		"BAN handler must 403 outside the ACL")
+	assert.Contains(t, banBlock, `synth(400, "Missing X-Vinyl-Ban header")`,
+		"BAN handler must 400 when X-Vinyl-Ban header is absent")
+	assert.Contains(t, banBlock, `std.ban(req.http.X-Vinyl-Ban)`,
+		"BAN handler must use std.ban(), not deprecated ban()")
+	assert.Contains(t, banBlock, `std.ban_error()`,
+		"BAN handler must surface std.ban_error() on malformed expressions")
+	assert.Contains(t, banBlock, `return(synth(200, "Banned"))`,
+		"BAN handler must return synth 200 on success")
+
+	// Hard regression guard: the bare ban() call form must never be emitted.
+	// (Matches the original architecture checklist: always use std.ban().)
+	// std.ban(...) is fine; only the bare form (preceded by whitespace, not
+	// by `.`) is forbidden.
+	assert.NotContains(t, r.VCL, " ban(req.http.X-Vinyl-Ban)",
+		"deprecated bare ban() call must not appear; use std.ban()")
+}
+
+func TestGenerate_BAN_BanLurkerHeadersEmitted(t *testing.T) {
+	g := newGenerator(t)
+	input := makeMinimalInput()
+	input.Spec.Invalidation.BAN = &vinylv1alpha1.BANSpec{Enabled: true}
+	r, err := g.Generate(input)
+	require.NoError(t, err)
+	assert.Contains(t, r.VCL, "beresp.http.x-url = bereq.url",
+		"BAN requires x-url ban-lurker-friendly header (otherwise bans accumulate O(n*m))")
+	assert.Contains(t, r.VCL, "beresp.http.x-host = bereq.http.host",
+		"BAN requires x-host ban-lurker-friendly header")
+	assert.Contains(t, r.VCL, "unset resp.http.x-url",
+		"BAN-support internal headers must be stripped before delivery")
+	assert.Contains(t, r.VCL, "unset resp.http.x-host",
+		"BAN-support internal headers must be stripped before delivery")
+}
+
+func TestGenerate_BAN_RateLimit_CurrentlyInert(t *testing.T) {
+	// Regression guard: rate-limit field is accepted on the CR but not
+	// plumbed into VCL (tracked as a BAN follow-up). This test locks in
+	// the deferred state so nobody half-implements it.
+	g := newGenerator(t)
+	input := makeMinimalInput()
+	input.Spec.Invalidation.BAN = &vinylv1alpha1.BANSpec{
+		Enabled:            true,
+		RateLimitPerMinute: 60,
+	}
+	r, err := g.Generate(input)
+	require.NoError(t, err)
+	assert.NotContains(t, r.VCL, "vsthrottle",
+		"vsthrottle VMOD must not be imported until rate-limiting is wired")
+	assert.NotContains(t, r.VCL, "Rate limited",
+		"no rate-limit synth message until the feature is implemented")
+}
+
+func TestGenerate_BAN_OperatorIPInACL(t *testing.T) {
+	g := newGenerator(t)
+	input := makeMinimalInput()
+	input.OperatorIP = "10.244.1.7"
+	input.Spec.Invalidation.BAN = &vinylv1alpha1.BANSpec{Enabled: true}
+	r, err := g.Generate(input)
+	require.NoError(t, err)
+	banACL := strings.SplitAfter(r.VCL, "acl vinyl_ban_allowed {")[1]
+	banACL = strings.SplitN(banACL, "}", 2)[0]
+	assert.Contains(t, banACL, `"10.244.1.7";`,
+		"operator IP must appear in vinyl_ban_allowed so operator-proxied BAN requests are accepted")
+	assert.Contains(t, banACL, `"127.0.0.1";`,
+		"localhost entry must remain")
+}
+
+func TestGenerate_BAN_AllowedSourcesHonored(t *testing.T) {
+	g := newGenerator(t)
+	input := makeMinimalInput()
+	input.Spec.Invalidation.BAN = &vinylv1alpha1.BANSpec{
+		Enabled:        true,
+		AllowedSources: []string{"10.0.0.0/8", "192.168.1.0/24"},
+	}
+	r, err := g.Generate(input)
+	require.NoError(t, err)
+	banACL := strings.SplitAfter(r.VCL, "acl vinyl_ban_allowed {")[1]
+	banACL = strings.SplitN(banACL, "}", 2)[0]
+	assert.Contains(t, banACL, `"10.0.0.0/8";`)
+	assert.Contains(t, banACL, `"192.168.1.0/24";`)
+}
+
+func TestGenerate_BAN_EnabledButNotConfigured_OnlyLocalhost(t *testing.T) {
+	g := newGenerator(t)
+	input := makeMinimalInput()
+	input.Spec.Invalidation.BAN = &vinylv1alpha1.BANSpec{Enabled: true}
+	r, err := g.Generate(input)
+	require.NoError(t, err)
+	banACL := strings.SplitAfter(r.VCL, "acl vinyl_ban_allowed {")[1]
+	banACL = strings.SplitN(banACL, "}", 2)[0]
+	assert.Equal(t, 1, strings.Count(banACL, `"127.0.0.1";`),
+		"BAN ACL with no AllowedSources and no OperatorIP must contain exactly one entry (localhost)")
+}

internal/generator/templates/acls.vcl.tmpl

@@ -7,6 +7,18 @@ acl vinyl_purge_allowed {
     "{{ . }}";
 {{- end }}{{ end }}
 }
+{{- if .HasBAN }}
+
+acl vinyl_ban_allowed {
+    "127.0.0.1";  // localhost always allowed
+{{- if .OperatorIP }}
+    "{{ .OperatorIP }}";  // operator pod IP (invalidation proxy source)
+{{- end }}
+{{- if .Spec.Invalidation.BAN }}{{ range .Spec.Invalidation.BAN.AllowedSources }}
+    "{{ . }}";
+{{- end }}{{ end }}
+}
+{{- end }}
 {{- if .HasCluster }}
 
 acl vinyl_cluster_peers {

internal/generator/templates/vcl_backend_response.vcl.tmpl

@@ -10,7 +10,7 @@ sub vcl_backend_response {
     if (beresp.grace < 24h) {
         set beresp.grace = 24h;
     }
-{{- if .HasXkey }}
+{{- if or .HasXkey .HasBAN }}
 
     # Copy URL and Host for BAN lurker friendliness (ban-lurker cannot access req.*)
     set beresp.http.x-url = bereq.url;

internal/generator/templates/vcl_deliver.vcl.tmpl

@@ -1,6 +1,6 @@
 
 sub vcl_deliver {
-{{- if .HasXkey }}
+{{- if or .HasXkey .HasBAN }}
     # Strip internal BAN-support headers before sending to client
     unset resp.http.x-url;
     unset resp.http.x-host;

internal/generator/templates/vcl_recv.vcl.tmpl

@@ -47,7 +47,24 @@ sub vcl_recv {
 {{- end }}
         return(purge);
     }
+{{- if .HasBAN }}
 
+    # BAN handling — std.ban() preferred over deprecated ban() (Varnish >= 6.6).
+    # std.ban() returns true on success, false on malformed expression;
+    # std.ban_error() returns the last compile error for this session.
+    if (req.method == "BAN") {
+        if (!client.ip ~ vinyl_ban_allowed) {
+            return(synth(403, "Forbidden"));
+        }
+        if (!req.http.X-Vinyl-Ban) {
+            return(synth(400, "Missing X-Vinyl-Ban header"));
+        }
+        if (!std.ban(req.http.X-Vinyl-Ban)) {
+            return(synth(400, "Invalid ban expression: " + std.ban_error()));
+        }
+        return(synth(200, "Banned"));
+    }
+{{- end }}
     # Only handle GET and HEAD for caching (pass everything else)
     if (req.method != "GET" && req.method != "HEAD") {
         return(pass);