From b37c3dbd0986927a2a609042e9b2e1eeec88fb6f Mon Sep 17 00:00:00 2001
From: Tomash Sidei <43379202+tomashibm@users.noreply.github.com>
Date: Wed, 17 Aug 2022 20:48:22 +0300
Subject: [PATCH] Allow setting additional listen addresses (-a options). The
 operator still ensures that Varnish listens on port 6081 since the service
 expects that port. But all other -a options will be respected and appended.
 (#76)

Signed-off-by: Tomash Sidei <tomash.sidei@ibm.com>
---
 api/v1alpha1/varnishcluster_webhook.go        |  1 -
 api/v1alpha1/varnishcluster_webhook_test.go   | 13 ++++++++++-
 api/v1alpha1/zz_generated.deepcopy.go         |  2 +-
 .../controller/varnishcluster_varnish_args.go |  2 +-
 .../varnishcluster_varnish_args_test.go       | 22 ++++++++++++++++++-
 5 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/api/v1alpha1/varnishcluster_webhook.go b/api/v1alpha1/varnishcluster_webhook.go
index 4014084c..4e1ccbd7 100644
--- a/api/v1alpha1/varnishcluster_webhook.go
+++ b/api/v1alpha1/varnishcluster_webhook.go
@@ -24,7 +24,6 @@ func SetWebhookLogger(l *logger.Logger) {
 var (
 	varnishArgsKeyRegexp  = regexp.MustCompile(`^-\w$`)
 	disallowedVarnishArgs = map[string]bool{
-		"-a": true,
 		"-f": true,
 		"-F": true,
 		"-n": true,
diff --git a/api/v1alpha1/varnishcluster_webhook_test.go b/api/v1alpha1/varnishcluster_webhook_test.go
index ef1ae083..af64c2cd 100644
--- a/api/v1alpha1/varnishcluster_webhook_test.go
+++ b/api/v1alpha1/varnishcluster_webhook_test.go
@@ -21,6 +21,17 @@ func TestValidatingWebhook(t *testing.T) {
 			},
 			valid: true,
 		},
+		{
+			name: "Additional listen addresses should be allowed",
+			vc: &VarnishCluster{
+				Spec: VarnishClusterSpec{
+					Varnish: &VarnishClusterVarnish{
+						Args: []string{"-a", ":8080,PROXY"},
+					},
+				},
+			},
+			valid: true,
+		},
 		{
 			name: "Invalid values",
 			vc: &VarnishCluster{
@@ -59,7 +70,7 @@ func TestValidatingWebhook(t *testing.T) {
 			vc: &VarnishCluster{
 				Spec: VarnishClusterSpec{
 					Varnish: &VarnishClusterVarnish{
-						Args: []string{"-a", "-f", "-F", "-n", "-S"},
+						Args: []string{"-f", "-F", "-n", "-S"},
 					},
 				},
 			},
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
index 0330a2d4..e7118a28 100644
--- a/api/v1alpha1/zz_generated.deepcopy.go
+++ b/api/v1alpha1/zz_generated.deepcopy.go
@@ -8,7 +8,7 @@ package v1alpha1
 import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/api/policy/v1"
+	v1 "k8s.io/api/policy/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
diff --git a/pkg/varnishcluster/controller/varnishcluster_varnish_args.go b/pkg/varnishcluster/controller/varnishcluster_varnish_args.go
index dc262a93..cefdf107 100644
--- a/pkg/varnishcluster/controller/varnishcluster_varnish_args.go
+++ b/pkg/varnishcluster/controller/varnishcluster_varnish_args.go
@@ -20,7 +20,6 @@ var (
 func getSanitizedVarnishArgs(spec *vcapi.VarnishClusterSpec) []string {
 	varnishArgsOverrides := [][]string{
 		{"-F"},
-		{"-a", fmt.Sprintf("0.0.0.0:%d", vcapi.VarnishPort)},
 		{"-S", "/etc/varnish-secret/secret"},
 		{"-b", "127.0.0.1:0"}, //start a varnishd without predefined backend. It has to be overridden by settings from ConfigMap
 		{"-T", fmt.Sprintf("0.0.0.0:%d", vcapi.VarnishAdminPort)},
@@ -52,6 +51,7 @@ func getSanitizedVarnishArgs(spec *vcapi.VarnishClusterSpec) []string {
 	}
 
 	varnishArgs := append(parsedArgs, varnishArgsOverrides...)
+	varnishArgs = append(varnishArgs, []string{"-a", fmt.Sprintf("0.0.0.0:%d", vcapi.VarnishPort)})
 
 	// sort the arguments so they won't appear in different order in different reconcile loops and trigger redeployment
 	sort.SliceStable(varnishArgs, func(i, j int) bool {
diff --git a/pkg/varnishcluster/controller/varnishcluster_varnish_args_test.go b/pkg/varnishcluster/controller/varnishcluster_varnish_args_test.go
index cdef6d1d..951afbcb 100644
--- a/pkg/varnishcluster/controller/varnishcluster_varnish_args_test.go
+++ b/pkg/varnishcluster/controller/varnishcluster_varnish_args_test.go
@@ -75,7 +75,7 @@ func TestGetSanitizedVarnishArgs(t *testing.T) {
 			spec: &v1alpha1.VarnishClusterSpec{
 				VCL: vclConfigMap,
 				Varnish: &v1alpha1.VarnishClusterVarnish{
-					Args: []string{"-S", "/etc/varnish/newsecret", "-T", "127.0.0.1:4235", "-a", "0.0.0.0:3425", "-b", "127.0.0.1:3456"},
+					Args: []string{"-S", "/etc/varnish/newsecret", "-T", "127.0.0.1:4235", "-b", "127.0.0.1:3456"},
 				},
 			},
 			expectedResult: []string{
@@ -122,6 +122,26 @@ func TestGetSanitizedVarnishArgs(t *testing.T) {
 				"-p", "default_ttl=3600",
 			},
 		},
+		{
+			name: "additional -a arguments can be specified",
+			spec: &v1alpha1.VarnishClusterSpec{
+				VCL: vclConfigMap,
+				Varnish: &v1alpha1.VarnishClusterVarnish{
+					Args: []string{"-a", ":8080,PROXY", "-a", ":4392", "-p", "default_grace=3600", "-p", "default_ttl=3600"},
+				},
+			},
+			expectedResult: []string{
+				"-F",
+				"-S", "/etc/varnish-secret/secret",
+				"-T", fmt.Sprintf("0.0.0.0:%d", v1alpha1.VarnishAdminPort),
+				"-a", fmt.Sprintf("0.0.0.0:%d", v1alpha1.VarnishPort),
+				"-a", ":4392",
+				"-a", ":8080,PROXY",
+				"-b", "127.0.0.1:0",
+				"-p", "default_grace=3600",
+				"-p", "default_ttl=3600",
+			},
+		},
 	}
 
 	for _, c := range cases {