diff --git a/cmd/operator/main.go b/cmd/operator/main.go index a943596..7cf00e7 100644 --- a/cmd/operator/main.go +++ b/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 { diff --git a/docs/sources/reference/vinylcache-spec.md b/docs/sources/reference/vinylcache-spec.md index 797a30f..cb63729 100644 --- a/docs/sources/reference/vinylcache-spec.md +++ b/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 `.backend(by=, 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 diff --git a/e2e/tests/vcl-validation/chainsaw-test.yaml b/e2e/tests/vcl-validation/chainsaw-test.yaml index 0926ef0..dc7d298 100644 --- a/e2e/tests/vcl-validation/chainsaw-test.yaml +++ b/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 diff --git a/internal/generator/generator.go b/internal/generator/generator.go index f830efd..9de4d36 100644 --- a/internal/generator/generator.go +++ b/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"] diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go index 21dc355..2b589b0 100644 --- a/internal/generator/generator_test.go +++ b/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)") +} diff --git a/internal/generator/templates/acls.vcl.tmpl b/internal/generator/templates/acls.vcl.tmpl index e94213a..a2c17f6 100644 --- a/internal/generator/templates/acls.vcl.tmpl +++ b/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 { diff --git a/internal/generator/templates/vcl_backend_response.vcl.tmpl b/internal/generator/templates/vcl_backend_response.vcl.tmpl index c0cb216..f1ea413 100644 --- a/internal/generator/templates/vcl_backend_response.vcl.tmpl +++ b/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; diff --git a/internal/generator/templates/vcl_deliver.vcl.tmpl b/internal/generator/templates/vcl_deliver.vcl.tmpl index 032e0e3..f078564 100644 --- a/internal/generator/templates/vcl_deliver.vcl.tmpl +++ b/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; diff --git a/internal/generator/templates/vcl_recv.vcl.tmpl b/internal/generator/templates/vcl_recv.vcl.tmpl index 37179b0..d7c3f04 100644 --- a/internal/generator/templates/vcl_recv.vcl.tmpl +++ b/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);