Edition gate by briiians · Pull Request #117 · varnish/helm-varnish

briiians · 2026-07-10T16:16:54Z

Main three changes:

Adds global.edition to the enterprise chart (enterprise by default) that gates all enterprise-only features. When set to community, implementing enterprise features will fail to compile.
Adds ci/publish-community.sh, a script that transforms the enterprise chart into a clean community chart at publish time. It switches the edition, points the image at docker.io/varnish, sets the correct OSS appVersion, and strips enterprise-only value sections entirely so they never appear in the published chart.
Updates the README to document the single-source-of-truth approach and the publish script.

The enterprise chart is now the single source of truth for both charts. They can never drift out of sync again.

Usage:

./ci/publish-community.sh <oss-version> [output-dir]
# e.g.
./ci/publish-community.sh 9.0.3
helm package dist/varnish-cache --destination dist/packages

What to discuss

The old varnish-cache/ directory is still in the repo. It was hand-maintained separately and is now superseded by this approach. Options:

Delete it and use only the generated chart going forward
Keep it temporarily as a reference while we validate the generated chart in CI
Something else?

Test plan (what I checked to validate this)

helm lint varnish-enterprise/ passes with default enterprise values
bash ci/publish-community.sh 9.0.3 generates a clean community chart that passes helm lint
Enterprise chart unit tests: 156 tests passing
Common unit tests: 678 tests passing
Both global.edition=community and global.edition=enterprise render the correct resources
Attempting to use enterprise features in community mode produces a clear error

Please give this a good review, I don't trust myself and there was indeed some Claude influence, so double don't trust.

…n parameter to the enterprise chart that gates all enterprise-only features.

gquintard · 2026-07-10T21:05:32Z

@@ -0,0 +1,74 @@
+#!/bin/sh


github actions should call this somewhere?

Should be fixed with 3d0cdef

gquintard · 2026-07-10T21:15:46Z

+{{- $mseConfig := "" }}
+{{- $mse4Config := "" }}
+{{- if eq .Values.global.edition "enterprise" }}
+{{- $mseConfig = include "varnish-enterprise.mseConfig" . }}
+{{- $mse4Config = include "varnish-enterprise.mse4Config" . }}
+{{- end }}


that's a repeat of the code on line 7, do we need it?

Yes according to Claude, "$mseConfig declared inside podAnnotations (line 7) is invisible to podVolumes (line 139). They're separate scopes, like two different functions.".

gquintard · 2026-07-10T21:18:43Z

@@ -204,21 +218,25 @@ volumes:
    name: {{ include "varnish-enterprise.fullname" $ }}-vcl-{{ regexReplaceAll "\\W+" $k "-" }}
 {{- end }}
 {{- end }}
+{{- if eq .Values.global.edition "enterprise" }}
 {{- if .Values.cluster.enabled }}
 - name: {{ $.Release.Name }}-config-vcl-{{ regexReplaceAll "\\W+" $wrappedDefaultVCL "-" }}
  configMap:
    name: {{ include "varnish-enterprise.fullname" $ }}-vcl-{{ regexReplaceAll "\\W+" $wrappedDefaultVCL "-" }}
 {{- end }}
+{{- end }}
 {{- if not (eq (include "varnish-enterprise.cmdfileConfig" .) "") }}
 - name: {{ .Release.Name }}-config-cmdfile
  configMap:
    name: {{ include "varnish-enterprise.fullname" . }}-cmdfile
 {{- end }}
 {{- end }}
+{{- if eq .Values.global.edition "enterprise" }}
 {{- if and .Values.server.agent.enabled (not .Values.server.agent.persistence.enabled) (eq .Values.server.agent.persistence.enableWithVolumeName "") }}
 - name: {{ .Release.Name }}-varnish-controller
  emptyDir: {}
 {{- end }}
+{{- end }}


could we get get a single if eq .Values.global.edition "enterprise" and stash all the configmaps in it? This would scope things a bit better.

Probably applicable to more sits of the PR, but I think it's quite clear here

Found some more and fixed them here 161f878

gquintard · 2026-07-10T21:19:25Z

+{{- $mseConfig := "" }}
+{{- $mse4Config := "" }}
+{{- if eq .Values.global.edition "enterprise" }}
+{{- $mseConfig = include "varnish-enterprise.mseConfig" . }}
+{{- $mse4Config = include "varnish-enterprise.mse4Config" . }}
+{{- end }}


I keep seeing this code, am I missing something?

Same as above, they need to be called each time they are used.

gquintard · 2026-07-10T21:19:54Z

+{{- if eq .Values.global.edition "enterprise" }}
+{{- $mse4 = .Values.server.mse4.enabled }}
+{{- end }}
+{{- if eq .Values.global.edition "enterprise" }}


merge the two ifs?

Done in f2f6865

gquintard · 2026-07-10T21:21:23Z

+  {{- if or (and (eq (kindOf .Values.server.mse.enabled) "string") (eq .Values.server.mse.enabled "-") (not $mse4) (not $malloc) ) (and (eq (kindOf .Values.server.mse.enabled) "bool") .Values.server.mse.enabled) }}
+    {{- $mse = true }}
+  {{- else }}
+    {{- $mse = false }}
+  {{- end }}


isn't this essentially

{{- $mse = or (and (eq (kindOf .Values.server.mse.enabled) "string") (eq .Values.server.mse.enabled "-") (not $mse4) (not $malloc) ) (and (eq (kindOf .Values.server.mse.enabled) "bool") .Values.server.mse.enabled) }}

?

fixed in ed97b2a

gquintard · 2026-07-10T21:22:37Z

+  {{- end }}
+{{- else }}
+  {{- if $mse4 }}
+    {{- fail "server.mse4 is not available in community edition, use server.malloc.enabled instead" }}


can we even reach this? we should never set mse4 if we are not in enterprise mode, should we?

fixed in eee23d4

audunmg and others added 2 commits June 16, 2026 15:31

Add malloc stevedore

bd8135d

Unify the enterprise and open source helmcharts. Adds a global.editio…

10fbb84

…n parameter to the enterprise chart that gates all enterprise-only features.

briiians requested review from audunmg and gquintard July 10, 2026 16:16

briiians self-assigned this Jul 10, 2026

gquintard requested changes Jul 10, 2026

View reviewed changes

briiians added 6 commits July 14, 2026 15:32

Wire publish-community.sh into CI for the varnish-cache chart build

3d0cdef

Use realpath instead of cd/pwd in publish script

4817eea

Collapse nested edition guards into single if-and conditions

161f878

Merge consecutive enterprise edition checks into one block

f2f6865

Simplify mse bool assignment to direct expression

ed97b2a

Remove unreachable mse4 community check

eee23d4

Uh oh!

Conversation

briiians commented Jul 10, 2026

Main three changes:

What to discuss

Test plan (what I checked to validate this)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants