Skip to content

feat: enable flag for varnish - #3586

Open
BrendanGalloway wants to merge 1 commit into
masterfrom
varnish-enable-flag
Open

feat: enable flag for varnish#3586
BrendanGalloway wants to merge 1 commit into
masterfrom
varnish-enable-flag

Conversation

@BrendanGalloway

Copy link
Copy Markdown
Contributor

Add a flag to enable/disable varnish for use in content cauldrons. Enabled by default to preserve current behaviour

@gitvelocity-reviewer

Copy link
Copy Markdown

📊 Code Quality Score: 27/100

Base Score 45 × ESF 0.6 = 27

Category Score Factors
🔭 Scope 13/20 18 files spanning nginx config, rollout specs (web, task, monitor, varnish), services (web, varnish, varnish-previous), secrets, KEDA scaledobject, and 3 prometheus/monitoring templates — touches the entire traffic routing path
🏗️ Architecture 10/20 Introduces a meaningful architectural toggle enabling cache-bypass mode; generalizes nginx upstream from Varnish-specific to generic UPSTREAM_HOST/UPSTREAM_PORT abstraction; follows existing Helm conditional patterns
⚙️ Implementation 6/20 Helm template conditionals and envsubst variable substitution changes; low algorithmic complexity but requires careful consistency across 18 files; hardcoded port values in nginx rollout reduce maintainability
⚠️ Risk 11/20 Changes live traffic routing path when varnish.enabled=false; stackRole label change on web service could silently break existing ServiceMonitor/NetworkPolicy selectors; no Helm unit tests; no documented rollback plan; monitor pod may fail if it unconditionally reads /varnish-secret
✅ Quality 4/15 Good inline comments explaining USE_VARNISH behavior and varnish.enabled semantics; no Helm unit tests for either enabled/disabled rendering paths; no example values file showing varnish.enabled=false configuration
🔒 Perf / Security 1/5 No explicit performance or security considerations beyond existing setup; change is infrastructure routing only

Was this score accurate? 👍 Yes · 👎 No

How this was scored →

Scored by GitVelocity · How are scores calculated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant