Generalize the 'conditions' used to restrict ACL matches.

'compare' can be one of equal, not-equal, match or non-match.

'value' is either a fixed string for (in)equality tests, or a
regex to test for (non-)match.

deploy/varnishcfg-crd.yaml

@@ -128,7 +128,7 @@ spec:
                     maximum: 599
                   comparand:
                     type: string
-                    pattern: "^((client|server|local|remote)\\.ip|xff-(first|2ndlast)|req\\.http\\.[a-zA-Z0-9!#$%&'*+-.^_`|~]+)$"
+                    pattern: "^((client|server|local|remote)\\.ip|xff-(first|2ndlast)|req\\.http\\.[a-zA-Z0-9!#$%&'*+.^_`|~-]+)$"
                   conditions:
                     type: array
                     minItems: 1
@@ -136,17 +136,21 @@ spec:
                       type: object
                       required:
                         - comparand
-                        - regex
-                        - match
+                        - value
                       properties:
                         comparand:
                           type: string
-                          pattern: "^req\\.(url|http\\.[a-zA-Z0-9!#$%&'*+-.^_`|~]+)$"
-                        regex:
+                          pattern: "^req\\.(url|http\\.[a-zA-Z0-9!#$%&'*+.^_`|~-]+)$"
+                        compare:
+                          enum:
+                          - equal
+                          - not-equal
+                          - match
+                          - not-match
+                          type: string
+                        value:
                           type: string
                           minLength: 1
-                        match:
-                          type: boolean
 status:
   acceptedNames:
     kind: VarnishConfig

docs/ref-varnish-cfg.md

@@ -330,8 +330,8 @@ When ``type`` is ``whitelist``, the failure response is sent when an
 IP does not match the ACL. For ``blacklist``, the failure response is
 invoked for an IP that does match the ACL.
 
-``comparand`` specifies the IP value against which the ACL is matched,
-and can have one of these values:
+``comparand`` (the "thing to be compared") specifies the IP value
+against which the ACL is matched, and can have one of these values:
 
 * ``client.ip``: interpreted as in
   [VCL](https://varnish-cache.org/docs/6.1/reference/vcl.html#local-server-remote-and-client)
@@ -411,16 +411,34 @@ X-Forwarded-For: 192.0.2.47, 203.0.113.11
 
 If ``conditions`` are specified for an ACL, they define restrictions
 for executing the match. Each element of ``conditions`` must specify
-these three fields (all required):
+these two required fields:
 
 * ``comparand`` (string): either ``req.url`` or ``req.http.$HEADER``,
   where ``$HEADER`` is the name of a client request header.
 
-* ``regex``: a regular expression
+* ``value`` (string): the value against which the ``comparand``
+  is compared
 
-* ``match`` (boolean): whether the condition term succeeds if the
-  ``comparand`` does or does not match ``regex`` -- ``true`` for
-  match, ``false`` for no-match.
+``conditions`` may also have this optional field:
+
+* ``compare``: one of the following (default ``equal``):
+
+    * ``equal`` for string equality
+
+    * ``not-equal`` for string inequality
+
+    * ``match`` for regex match
+
+    * ``not-match`` for regex non-match
+
+If ``compare`` is ``equal`` or ``not-equal``, then ``value`` is
+interpreted as a fixed string, and ``comparand`` is tested for
+(in)equality with ``value``. Otherwise, ``value`` is interpreted as a
+regular expression, and the ``comparand`` is tested for
+(non-)match. Regexen are implemented as
+[VCL regular expressions](https://varnish-cache.org/docs/6.1/reference/vcl.html#regular-expressions),
+and hence have the syntax and semantics of
+[PCRE](https://www.pcre.org/original/doc/html/).
 
 The ACL match is executed only if all of the ``conditions`` succeed;
 in other words, the ``conditions`` are the boolean AND of all of the
@@ -433,11 +451,10 @@ when the URL begins with "/tea", and the Host header is exactly
 ```
       conditions:
       - comparand: req.url
-        match: true
-        regex: ^/tea(/|$)
+        compare: match
+        value: ^/tea(/|$)
       - comparand: req.http.Host
-        match: yes
-        regex: ^cafe\.example\.com$
+        value: cafe.example.com
 ```
 
 See the [``examples/`` folder](/examples/acl) for working examples

examples/acl/README.md

@@ -74,11 +74,10 @@ The next example re-creates the ACL shown as an example in
       type: whitelist
       conditions:
       - comparand: req.url
-        match: true
-        regex: ^/tea(/|$)
+        compare: match
+        value: ^/tea(/|$)
       - comparand: req.http.Host
-        match: yes
-        regex: ^cafe\.example\.com$
+        value: cafe.example.com
 ```
 
 Addresses that match the ACL are:
@@ -109,6 +108,9 @@ The ``conditions`` specify that the ACL match is executed when:
 
 * the URL path begins with "/tea"
 * the Host header is exactly "cafe.example.com"
+    * The ``compare`` field is left out of the condition for
+      ``req.http.Host``, so it defaults to the value ``equal``,
+      meaning compare for string equality.
 
 According to the Ingress in the ["cafe" example](/examples/hello),
 requests are routed to the Service ``tea-svc`` under these
@@ -175,11 +177,11 @@ The next example defines a blacklist for the ranges 192.0.20/24 and
       fail-status: 404
       conditions:
       - comparand: req.url
-        match: true
-        regex: ^/coffee/black(/|$)
+        compare: match
+        value: ^/coffee/black(/|$)
       - comparand: req.http.Host
-        match: yes
-        regex: ^cafe\.example\.com$
+        compare: equal
+        value: cafe.example.com
 ```
 
 Type ``blacklist`` means that the failure status is returned for IPs
@@ -271,11 +273,10 @@ The final example defines a blacklist for the range 203.0.113.0/24:
       type: blacklist
       conditions:
       - comparand: req.url
-        match: true
-        regex: ^/coffee(/|$)
+        compare: match
+        value: ^/coffee(/|$)
       - comparand: req.http.Host
-        match: yes
-        regex: ^cafe\.example\.com$
+        value: cafe.example.com
 ```
 
 The ``comparand`` is ``xff-2ndlast``, which means that the ACL is

examples/acl/acl.yaml

@@ -71,11 +71,10 @@ spec:
       type: whitelist
       conditions:
       - comparand: req.url
-        match: true
-        regex: ^/tea(/|$)
+        compare: match
+        value: ^/tea(/|$)
       - comparand: req.http.Host
-        match: yes
-        regex: ^cafe\.example\.com$
+        value: cafe.example.com
 
     # This ACL defines a blacklist:
     # 192.0.20/24 and 198.51.100.0/24
@@ -105,11 +104,11 @@ spec:
       fail-status: 404
       conditions:
       - comparand: req.url
-        match: true
-        regex: ^/coffee/black(/|$)
+        compare: match
+        value: ^/coffee/black(/|$)
       - comparand: req.http.Host
-        match: yes
-        regex: ^cafe\.example\.com$
+        compare: equal
+        value: cafe.example.com
 
     # This ACL defines a blacklist for 203.0.113.0/24.
     #
@@ -137,8 +136,7 @@ spec:
       type: blacklist
       conditions:
       - comparand: req.url
-        match: true
-        regex: ^/coffee(/|$)
+        compare: match
+        value: ^/coffee(/|$)
       - comparand: req.http.Host
-        match: yes
-        regex: ^cafe\.example\.com$
+        value: cafe.example.com

pkg/apis/varnishingress/v1alpha1/types.go

@@ -119,12 +119,12 @@ type ACLAddress struct {
 	Negate   bool   `json:"negate,omitempty"`
 }
 
-// Condition represents a term in a boolean expression -- match or
-// non-match against a VCL object.
+// Condition represents a term in a boolean expression -- test the
+// Comparand against Value for equality or regex match.
 type Condition struct {
-	Comparand string `json:"comparand,omitempty"`
-	Regex     string `json:"regex,omitempty"`
-	Match     bool   `json:"match,omitempty"`
+	Comparand string      `json:"comparand,omitempty"`
+	Compare   CompareType `json:"compare,omitempty"`
+	Value     string      `json:"value,omitempty"`
 }
 
 // ACLType classifies an ACL.
@@ -139,6 +139,20 @@ const (
 	Blacklist = "blacklist"
 )
 
+// CompareType classifies a string comparison.
+type CompareType string
+
+const (
+	// Equal means compare strings with ==.
+	Equal CompareType = "equal"
+	// NotEqual means compare with !=.
+	NotEqual = "not-equal"
+	// Match means compare with ~ (the Value is a regex).
+	Match = "match"
+	// NotMatch means compare with !~.
+	NotMatch = "not-match"
+)
+
 // VarnishConfigStatus is the status for a VarnishConfig resource
 // type VarnishConfigStatus struct {
 //         AvailableReplicas int32 `json:"availableReplicas"`

pkg/controller/ingress.go

@@ -319,14 +319,15 @@ func (worker *NamespaceWorker) configACL(spec *vcl.Spec,
 			vcfg.Namespace, vcfg.Name)
 		return nil
 	}
-	spec.ACLs = make([]vcl.ACL, 0, len(vcfg.Spec.ACLs))
-	for _, acl := range vcfg.Spec.ACLs {
-		worker.log.Debugf("VarnishConfig %s/%s configuring VCL ACL "+
-			"from: %+v", vcfg.Namespace, vcfg.Name, acl)
+	spec.ACLs = make([]vcl.ACL, len(vcfg.Spec.ACLs))
+	for i, acl := range vcfg.Spec.ACLs {
+		worker.log.Infof("VarnishConfig %s/%s configuring ACL %s",
+			vcfg.Namespace, vcfg.Name, acl.Name)
+		worker.log.Debugf("ACL %s: %+v", acl.Name, acl)
 		vclACL := vcl.ACL{
 			Name:       acl.Name,
-			Addresses:  make([]vcl.ACLAddress, 0, len(acl.Addresses)),
-			Conditions: make([]vcl.MatchTerm, 0, len(acl.Conditions)),
+			Addresses:  make([]vcl.ACLAddress, len(acl.Addresses)),
+			Conditions: make([]vcl.MatchTerm, len(acl.Conditions)),
 		}
 		if acl.Comparand == "" {
 			vclACL.Comparand = defACLcomparand
@@ -341,7 +342,7 @@ func (worker *NamespaceWorker) configACL(spec *vcl.Spec,
 		} else {
 			vclACL.FailStatus = uint16(*acl.FailStatus)
 		}
-		for _, addr := range acl.Addresses {
+		for j, addr := range acl.Addresses {
 			vclAddr := vcl.ACLAddress{
 				Addr:   addr.Address,
 				Negate: addr.Negate,
@@ -351,19 +352,28 @@ func (worker *NamespaceWorker) configACL(spec *vcl.Spec,
 			} else {
 				vclAddr.MaskBits = uint8(*addr.MaskBits)
 			}
-			vclACL.Addresses = append(vclACL.Addresses, vclAddr)
+			vclACL.Addresses[j] = vclAddr
 		}
-		for _, cond := range acl.Conditions {
+		for j, cond := range acl.Conditions {
 			vclMatch := vcl.MatchTerm{
 				Comparand: cond.Comparand,
-				Regex:     cond.Regex,
-				Match:     cond.Match,
+				Value:     cond.Value,
 			}
-			vclACL.Conditions = append(vclACL.Conditions, vclMatch)
+			switch cond.Compare {
+			case vcr_v1alpha1.Equal:
+				vclMatch.Compare = vcl.Equal
+			case vcr_v1alpha1.NotEqual:
+				vclMatch.Compare = vcl.NotEqual
+			case vcr_v1alpha1.Match:
+				vclMatch.Compare = vcl.Match
+			case vcr_v1alpha1.NotMatch:
+				vclMatch.Compare = vcl.NotMatch
+			}
+			vclACL.Conditions[j] = vclMatch
 		}
 		worker.log.Debugf("VarnishConfig %s/%s add VCL ACL config: "+
 			"%+v", vcfg.Namespace, vcfg.Name, vclACL)
-		spec.ACLs = append(spec.ACLs, vclACL)
+		spec.ACLs[i] = vclACL
 	}
 	return nil
 }

pkg/varnish/vcl/acl.tmpl

@@ -16,7 +16,7 @@ sub vcl_recv {
 	{{- range .ACLs}}
 	if (
 	    {{- range $cond := .Conditions}}
-	    {{$cond.Comparand}} {{if not .Match}}!{{end}}~ "{{.Regex}}" &&
+	    {{$cond.Comparand}} {{cmpRelation .Compare}} "{{.Value}}" &&
 	    {{- end}}
 	    {{aclCmp .Comparand}} {{if .Whitelist}}!{{end}}~ {{aclName .Name}}
 	   ) {

pkg/varnish/vcl/testdata/acl.golden

@@ -28,13 +28,14 @@ acl vk8s_local_acl {
 sub vcl_recv {
         std.collect(req.http.X-Forwarded-For);
 	if (
-	    req.http.Host ~ "^cafe\.example\.com$" &&
+	    req.http.Host == "cafe.example.com" &&
 	    req.url ~ "^/coffee(/|$)" &&
 	    client.ip !~ vk8s_man_vcl_example_acl
 	   ) {
 		return(synth(403));
 	}
 	if (
+	    req.http.Host != "cafe.example.com" &&
 	    req.url !~ "^/tea(/|$)" &&
 	    server.ip ~ vk8s_wikipedia_example_acl
 	   ) {

pkg/varnish/vcl/vcl.go

@@ -242,20 +242,33 @@ func (a byACLAddr) Len() int           { return len(a) }
 func (a byACLAddr) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
 func (a byACLAddr) Less(i, j int) bool { return a[i].Addr < a[j].Addr }
 
+// CompareType classifies comparisons for MatchTerm.
+type CompareType uint8
+
+const (
+	// Equal means compare strings for equality (==).
+	Equal CompareType = iota
+	// NotEqual means compare with !=.
+	NotEqual
+	// Match means compare for regex match (~) -- the MatchTerm
+	// Value is a regular expression.
+	Match
+	// NotMatch means compare with !~.
+	NotMatch
+)
+
 // MatchTerm is a term describing the comparison of a VCL object with
 // a pattern.
 type MatchTerm struct {
 	Comparand string
-	Regex     string
-	Match     bool
+	Value     string
+	Compare   CompareType
 }
 
 func (match MatchTerm) hash(hash hash.Hash) {
 	hash.Write([]byte(match.Comparand))
-	hash.Write([]byte(match.Regex))
-	if match.Match {
-		hash.Write([]byte("Match"))
-	}
+	hash.Write([]byte(match.Value))
+	hash.Write([]byte{byte(match.Compare)})
 }
 
 // interface for sorting []MatchTerm
@@ -397,11 +410,12 @@ func (spec Spec) Canonical() Spec {
 }
 
 var fMap = template.FuncMap{
-	"plusOne":   func(i int) int { return i + 1 },
-	"vclMangle": func(s string) string { return mangle(s) },
-	"aclMask":   func(bits uint8) string { return aclMask(bits) },
-	"aclCmp":    func(comparand string) string { return aclCmp(comparand) },
-	"hasXFF":    func(acls []ACL) bool { return hasXFF(acls) },
+	"plusOne":     func(i int) int { return i + 1 },
+	"vclMangle":   func(s string) string { return mangle(s) },
+	"aclMask":     func(bits uint8) string { return aclMask(bits) },
+	"aclCmp":      func(comparand string) string { return aclCmp(comparand) },
+	"hasXFF":      func(acls []ACL) bool { return hasXFF(acls) },
+	"cmpRelation": func(cmp CompareType) string { return cmpRelation(cmp) },
 	"backendName": func(svc Service, addr string) string {
 		return backendName(svc, addr)
 	},
@@ -558,3 +572,18 @@ func hasXFF(acls []ACL) bool {
 	}
 	return false
 }
+
+func cmpRelation(cmp CompareType) string {
+	switch cmp {
+	case Equal:
+		return "=="
+	case NotEqual:
+		return "!="
+	case Match:
+		return "~"
+	case NotMatch:
+		return "!~"
+	default:
+		return "__INVALID_COMPARISON_TYPE__"
+	}
+}

pkg/varnish/vcl/vcl_test.go

@@ -425,13 +425,13 @@ var acls = Spec{
 			Conditions: []MatchTerm{
 				MatchTerm{
 					Comparand: "req.http.Host",
-					Match:     true,
-					Regex:     `^cafe\.example\.com$`,
+					Compare:   Equal,
+					Value:     "cafe.example.com",
 				},
 				MatchTerm{
 					Comparand: "req.url",
-					Match:     true,
-					Regex:     `^/coffee(/|$)`,
+					Compare:   Match,
+					Value:     `^/coffee(/|$)`,
 				},
 			},
 		},
@@ -458,10 +458,15 @@ var acls = Spec{
 				},
 			},
 			Conditions: []MatchTerm{
+				MatchTerm{
+					Comparand: "req.http.Host",
+					Compare:   NotEqual,
+					Value:     "cafe.example.com",
+				},
 				MatchTerm{
 					Comparand: "req.url",
-					Match:     false,
-					Regex:     `^/tea(/|$)`,
+					Compare:   NotMatch,
+					Value:     `^/tea(/|$)`,
 				},
 			},
 		},