[NOREF] add support for queryParams and scheme

.github/workflows/go.yml

@@ -21,27 +21,11 @@ jobs:
         uname -a
         go version
         go env
-    - name: Vet
-      if: matrix.platform == 'ubuntu-latest'
-      run: go vet -v ./...
     - name: Lint
-      if: matrix.platform == 'ubuntu-latest'
-      run: |
-        export PATH=$PATH:$(go env GOPATH)/bin
-        go install golang.org/x/lint/golint@latest
-        golint -set_exit_status ./...
-    - name: staticcheck.io
-      if: matrix.platform == 'ubuntu-latest'
-      uses: dominikh/[email protected]
+      uses: golangci/golangci-lint-action@v3
       with:
-        install-go: false
-    - name: gofumpt formatting
-      if: matrix.platform == 'ubuntu-latest'
-      run: |
-        export PATH=$PATH:$(go env GOPATH)/bin
-        go install mvdan.cc/gofumpt@latest
-        gofumpt -d .
-        [ -z "$(gofumpt -l .)" ]
+        skip-build-cache: true
+        skip-pkg-cache: true
     - name: Test
       run: go test -count=1 ./...
     - name: Test with -race

.golangci.yml

@@ -0,0 +1,14 @@
+run:
+  timeout: 10m
+linters:
+  disable-all: true
+  enable:
+    - errcheck
+    - gofumpt
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - stylecheck
+    - typecheck
+    - unused

Makefile

@@ -1,10 +1,32 @@
 CURRENTPATH = $(shell echo $(PWD))
 WORKDIR = /src/github.com/getyourguide.com/istio-config-validator
+GOLINT_CMD=golangci-lint
+GO_FILES=$(shell find . -type f -regex ".*go")
 
+.PHONY: run lint fix
 
 run:
 	docker run -it --rm --name istio_config_validator \
 				-v ${CURRENTPATH}:${WORKDIR} \
 				-w ${WORKDIR} \
 				golang:1.21 \
 				go run cmd/istio-config-validator/main.go -t examples/ examples/
+
+define check_linter
+	@if ! command -v $(GOLINT_CMD) > /dev/null; then\
+		echo "$(GOLINT_CMD) must be installed to run linting: try brew install $(GOLINT_CMD)";\
+		exit 1;\
+	fi
+endef
+
+lint: .golangci.yml ## Lint the code
+
+fix: ## Fix any issues found during linting
+	@$(call check_linter)
+	$(GOLINT_CMD) run --fix
+
+.golangci.yml: $(GO_FILES)
+	@$(call check_linter)
+	$(GOLINT_CMD) version
+	$(GOLINT_CMD) run
+	touch .golangci.yml

cmd/istio-config-validator/main.go

@@ -61,15 +61,18 @@ func main() {
 func getFiles(names []string) []string {
 	var files []string
 	for _, name := range names {
-		filepath.Walk(name, func(path string, info os.FileInfo, err error) error {
+		err := filepath.Walk(name, func(path string, info os.FileInfo, err error) error {
 			if err != nil {
-				log.Fatal(err.Error())
+				return err
 			}
 			if !info.IsDir() && isYaml(info) {
 				files = append(files, path)
 			}
 			return nil
 		})
+		if err != nil {
+			log.Fatal(err.Error())
+		}
 	}
 	return files
 }

examples/destination.yml

@@ -1,4 +1,4 @@
-# This file is to test skiping non virtual services files
+# This file is to test skipping non virtual services files
 apiVersion: networking.istio.io/v1alpha3
 kind: DestinationRule
 metadata:

internal/pkg/parser/parser.go

@@ -1,11 +1,11 @@
 // Package parser is the package responsible for parsing test cases and istio configuration
-// to be use on test assertionpackage parser
+// to be used on test assertionpackage parser
 package parser
 
 import (
 	"fmt"
 
-	v1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3"
+	"istio.io/client-go/pkg/apis/networking/v1alpha3"
 )
 
 // Parser contains the parsed files needed to run tests

internal/pkg/parser/testcase.go

@@ -43,18 +43,23 @@ type TestCase struct {
 
 // Request define the crafted http request present in the test case file.
 type Request struct {
-	Authority []string          `yaml:"authority"`
-	Method    []string          `yaml:"method"`
-	URI       []string          `yaml:"uri"`
-	Headers   map[string]string `yaml:"headers"`
+	Authority []string `yaml:"authority"`
+	Method    []string `yaml:"method"`
+	URI       []string `yaml:"uri"`
+	// optional fields
+	Headers     map[string]string `yaml:"headers"`
+	QueryParams map[string]string `yaml:"queryParams"`
+	Scheme      string            `yaml:"scheme"`
 }
 
 // Input contains the data structure which will be used to assert
 type Input struct {
-	Authority string
-	Method    string
-	URI       string
-	Headers   map[string]string
+	Authority   string
+	Method      string
+	URI         string
+	Headers     map[string]string
+	Scheme      string
+	QueryParams map[string]string
 }
 
 // Destination define the destination we should assert
@@ -78,7 +83,7 @@ type Port struct {
 //		{Authority:"example.com", Method: "OPTIONS"},
 //	}
 func (r *Request) Unfold() ([]Input, error) {
-	out := []Input{}
+	var out []Input
 
 	if len(r.Authority) == 0 {
 		return out, ErrEmptyAuthorityList
@@ -98,7 +103,14 @@ func (r *Request) Unfold() ([]Input, error) {
 
 		for _, auth := range r.Authority {
 			for _, method := range r.Method {
-				out = append(out, Input{Authority: auth, Method: method, URI: u.Path, Headers: r.Headers})
+				out = append(out, Input{
+					Authority:   auth,
+					Method:      method,
+					URI:         u.Path,
+					Headers:     r.Headers,
+					QueryParams: r.QueryParams,
+					Scheme:      r.Scheme,
+				})
 			}
 		}
 	}
@@ -107,7 +119,7 @@ func (r *Request) Unfold() ([]Input, error) {
 }
 
 func parseTestCases(files []string) ([]*TestCase, error) {
-	out := []*TestCase{}
+	var out []*TestCase
 
 	for _, file := range files {
 		fileContent, err := os.ReadFile(file)
@@ -137,7 +149,7 @@ func parseTestCases(files []string) ([]*TestCase, error) {
 			yamlFile := &TestCaseYAML{}
 			err = json.Unmarshal(jsonBytes, yamlFile)
 			if err != nil {
-				slog.Debug("unmarshaling failed for file '%s': %w", file, err)
+				slog.Debug("unmarshalling failed for file '%s': %w", file, err)
 				return []*TestCase{}, err
 
 			}

internal/pkg/parser/virtualservice.go

@@ -15,7 +15,7 @@ import (
 )
 
 func parseVirtualServices(files []string) ([]*v1alpha3.VirtualService, error) {
-	out := []*v1alpha3.VirtualService{}
+	var out []*v1alpha3.VirtualService
 
 	for _, file := range files {
 		fileContent, err := os.ReadFile(file)

internal/pkg/unit/match_request.go

@@ -10,6 +10,37 @@ import (
 	"istio.io/api/networking/v1alpha3"
 )
 
+// MapMatcher checks map parameters, which requires executing multiple ExtendedStringMatch checks
+type MapMatcher struct {
+	matchers map[string]*v1alpha3.StringMatch
+}
+
+func (mm MapMatcher) Match(input map[string]string) (bool, error) {
+	for param, matcher := range mm.matchers {
+		value, exists := input[param]
+		if !exists {
+			return false, nil
+		}
+
+		if matcher.GetMatchType() == nil {
+			// match type exact: "", and {} checks for presence in query and headers respectively
+			continue
+		}
+
+		extendedMatcher := &ExtendedStringMatch{StringMatch: matcher}
+		matched, err := extendedMatcher.Match(value)
+		if err != nil {
+			return false, err
+		}
+
+		if !matched {
+			return false, nil
+		}
+	}
+
+	return true, nil
+}
+
 // ExtendedStringMatch copies istio ExtendedStringMatch definition and extends it to add helper methods.
 type ExtendedStringMatch struct {
 	*v1alpha3.StringMatch
@@ -26,57 +57,58 @@ func (sm *ExtendedStringMatch) Match(s string) (bool, error) {
 		return true, nil
 	}
 
-	switch {
-	case sm.GetExact() != "":
-		return sm.GetExact() == s, nil
-	case sm.GetPrefix() != "":
+	switch m := sm.GetMatchType().(type) {
+	case *v1alpha3.StringMatch_Exact:
+		return m.Exact == s, nil
+	case *v1alpha3.StringMatch_Prefix:
 		return strings.HasPrefix(s, sm.GetPrefix()), nil
-	case sm.GetRegex() != "":
+	case *v1alpha3.StringMatch_Regex:
 		// The rule will not match if only a subsequence of the string matches the regex.
 		// https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routematch-safe-regex
 		r, err := regexp.Compile("^" + sm.GetRegex() + "$")
 		if err != nil {
 			return false, fmt.Errorf("could not compile regex %s: %v", sm.GetRegex(), err)
 		}
 		return r.MatchString(s), nil
+	default:
+		return false, fmt.Errorf("unknown matcher type %T", sm.GetMatchType())
 	}
-	return false, nil
 }
 
 // matchRequest takes an Input and evaluates against a HTTPMatchRequest block. It replicates
 // Istio VirtualService semantic returning true when ALL conditions within the block are true.
 // TODO: Add support for all fields within a match block. The ones supported today are:
-// Authority, Uri, Method and Headers.
+// Authority, Uri, Method, Headers, Scheme, and QueryParams.
 func matchRequest(input parser.Input, httpMatchRequest *v1alpha3.HTTPMatchRequest) (bool, error) {
-	authority := &ExtendedStringMatch{httpMatchRequest.Authority}
 	uri := &ExtendedStringMatch{httpMatchRequest.Uri}
-	method := &ExtendedStringMatch{httpMatchRequest.Method}
+	if matched, err := uri.Match(input.URI); !matched || err != nil {
+		return false, err
+	}
 
-	for headerName, sm := range httpMatchRequest.Headers {
-		if _, ok := input.Headers[headerName]; !ok {
-			return false, nil
-		}
-		header := &ExtendedStringMatch{sm}
-		match, err := header.Match(input.Headers[headerName])
-		if err != nil {
-			return false, err
-		}
-		if !match {
-			return false, nil
-		}
+	authority := &ExtendedStringMatch{httpMatchRequest.Authority}
+	if matched, err := authority.Match(input.Authority); !matched || err != nil {
+		return false, err
 	}
 
-	uriMatch, err := uri.Match(input.URI)
-	if err != nil {
+	method := &ExtendedStringMatch{httpMatchRequest.Method}
+	if matched, err := method.Match(input.Method); !matched || err != nil {
 		return false, err
 	}
-	authorityMatch, err := authority.Match(input.Authority)
-	if err != nil {
+
+	scheme := &ExtendedStringMatch{httpMatchRequest.Scheme}
+	if matched, err := scheme.Match(input.Scheme); !matched || err != nil {
 		return false, err
 	}
-	methodMatch, err := method.Match(input.Method)
-	if err != nil {
+
+	headers := &MapMatcher{matchers: httpMatchRequest.Headers}
+	if matched, err := headers.Match(input.Headers); !matched || err != nil {
 		return false, err
 	}
-	return authorityMatch && uriMatch && methodMatch, nil
+
+	query := &MapMatcher{matchers: httpMatchRequest.QueryParams}
+	if matched, err := query.Match(input.QueryParams); !matched || err != nil {
+		return false, err
+	}
+
+	return true, nil
 }

internal/pkg/unit/testcases/header/service.yml

@@ -0,0 +1,31 @@
+apiVersion: networking.istio.io/v1alpha3
+kind: VirtualService
+metadata:
+  name: example
+  namespace: example
+spec:
+  gateways:
+    - mesh
+  hosts:
+    - www.example.com
+    - example.com
+  http:
+    - match:
+      - headers:
+          x-custom-header:
+            exact: ok
+          x-prefix-header:
+            prefix: start
+      route:
+        - destination:
+            host: has-header
+    - match:
+        - headers:
+            x-custom-header: {}
+      route:
+        - destination:
+            host: has-present-header
+    - match:
+      route:
+        - destination:
+            host: default

internal/pkg/unit/testcases/header/tests.yml

@@ -0,0 +1,33 @@
+TestCases:
+  - description: match headers with a variety of values
+    wantMatch: true
+    request:
+      authority: ["www.example.com"]
+      method: ["GET"]
+      uri: ["/headers/v1"]
+      headers:
+        x-custom-header: ok
+        x-prefix-header: start-with-prefix
+    route:
+      - destination:
+          host: has-header
+  - description: match a header by presence
+    wantMatch: true
+    request:
+      authority: ["www.example.com"]
+      method: ["GET"]
+      uri: ["/headers/v1"]
+      headers:
+        x-custom-header: ok
+    route:
+      - destination:
+          host: has-present-header
+  - description: missing headers goes to default
+    wantMatch: true
+    request:
+      authority: ["www.example.com"]
+      method: ["GET"]
+      uri: ["/headers/v1"]
+    route:
+      - destination:
+          host: default