Properly validate configuration

Makefile

@@ -102,7 +102,7 @@ config/lint:  ## Lint the Config Proto generated code
 test:  ## Run all the tests
 	@go test $(TEST_OPTS) $(TEST_PKGS)
 
-COVERAGE_OPTS     ?=
+COVERAGE_OPTS ?=
 .PHONY: coverage
 coverage:  ## Creates coverage report for all projects
 	@echo "Running test coverage"

internal/config.go

@@ -57,5 +57,12 @@ func (l *LocalConfigFile) Validate() error {
 		return err
 	}
 
-	return protojson.Unmarshal(content, &l.Config)
+	if err = protojson.Unmarshal(content, &l.Config); err != nil {
+		return err
+	}
+
+	// Set reasonable defaults for non-supported values
+	l.Config.Threads = 1
+
+	return l.Config.ValidateAll()
 }

internal/config_test.go

@@ -19,33 +19,79 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"github.com/tetratelabs/run"
+	"github.com/tetratelabs/telemetry"
+	"google.golang.org/protobuf/proto"
+
+	configv1 "github.com/tetrateio/authservice-go/config/gen/go/v1"
+	mockv1 "github.com/tetrateio/authservice-go/config/gen/go/v1/mock"
 )
 
+type errCheck struct {
+	is  error
+	as  error
+	msg string
+}
+
+func (e errCheck) Check(t *testing.T, err error) {
+	switch {
+	case e.as != nil:
+		require.ErrorAs(t, err, &e.as)
+	case e.msg != "":
+		require.ErrorContains(t, err, e.msg)
+	default:
+		require.ErrorIs(t, err, e.is)
+	}
+}
+
 func TestLoadConfig(t *testing.T) {
 	tests := []struct {
-		name string
-		path string
-		err  error
+		name  string
+		path  string
+		check errCheck
 	}{
-		{"empty", "", ErrInvalidPath},
-		{"invalid", "unexisting", os.ErrNotExist},
-		{"valid", "testdata/mock.json", nil},
+		{"empty", "", errCheck{is: ErrInvalidPath}},
+		{"unexisting", "unexisting", errCheck{is: os.ErrNotExist}},
+		{"invalid-config", "testdata/invalid-config.json", errCheck{msg: `unknown field "foo"`}},
+		{"invalid-values", "testdata/invalid-values.json", errCheck{as: &configv1.ConfigMultiError{}}},
+		{"valid", "testdata/mock.json", errCheck{is: nil}},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			cfg := LocalConfigFile{path: tt.path}
-			require.ErrorIs(t, cfg.Validate(), tt.err)
+			err := (&LocalConfigFile{path: tt.path}).Validate()
+			tt.check.Check(t, err)
 		})
 	}
 }
 
 func TestLoadMock(t *testing.T) {
-	cfg := LocalConfigFile{path: "testdata/mock.json"}
+	want := &configv1.Config{
+		ListenAddress: "0.0.0.0",
+		ListenPort:    8080,
+		LogLevel:      "debug",
+		Threads:       1,
+		Chains: []*configv1.FilterChain{
+			{
+				Name: "mock",
+				Filters: []*configv1.Filter{
+					{
+						Type: &configv1.Filter_Mock{
+							Mock: &mockv1.MockConfig{
+								Allow: true,
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	var cfg LocalConfigFile
+	g := run.Group{Logger: telemetry.NoopLogger()}
+	g.Register(&cfg)
+	err := g.Run("", "--config-path", "testdata/mock.json")
 
-	require.NoError(t, cfg.Validate())
-	require.Len(t, cfg.Config.Chains, 1)
-	require.Equal(t, "mock", cfg.Config.Chains[0].Name)
-	require.Len(t, cfg.Config.Chains[0].Filters, 1)
-	require.True(t, cfg.Config.Chains[0].Filters[0].GetMock().Allow)
+	require.NoError(t, err)
+	require.True(t, proto.Equal(want, &cfg.Config))
 }

internal/logging_test.go

@@ -25,6 +25,15 @@ import (
 	configv1 "github.com/tetrateio/authservice-go/config/gen/go/v1"
 )
 
+func TestGetLogger(t *testing.T) {
+	l1 := scope.Register("l1", "test logger one")
+
+	NewLogSystem(telemetry.NoopLogger(), nil)
+
+	require.Equal(t, l1, Logger("l1"))
+	require.Equal(t, telemetry.NoopLogger(), Logger("l2"))
+}
+
 func TestLoggingSetup(t *testing.T) {
 	l1 := scope.Register("l1", "test logger one")
 	l2 := scope.Register("l2", "test logger two")
@@ -50,6 +59,8 @@ func TestLoggingSetup(t *testing.T) {
 		{",", telemetry.LevelInfo, telemetry.LevelInfo, true},
 		{":", telemetry.LevelInfo, telemetry.LevelInfo, true},
 		{"invalid", telemetry.LevelInfo, telemetry.LevelInfo, true},
+		{"l1:,l2:info", telemetry.LevelInfo, telemetry.LevelInfo, true},
+		{"l1:debug,l2:invalid", telemetry.LevelInfo, telemetry.LevelInfo, true},
 	}
 
 	for _, tt := range tests {

internal/server/authz_test.go

@@ -0,0 +1,204 @@
+// Copyright 2024 Tetrate
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Copyright (c) Tetrate, Inc 2024 All Rights Reserved.
+
+package server
+
+import (
+	"context"
+	"testing"
+
+	envoy "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+
+	configv1 "github.com/tetrateio/authservice-go/config/gen/go/v1"
+	mockv1 "github.com/tetrateio/authservice-go/config/gen/go/v1/mock"
+)
+
+func TestUnmatchedRequests(t *testing.T) {
+	tests := []struct {
+		name  string
+		allow bool
+		want  codes.Code
+	}{
+		{"allow", true, codes.OK},
+		{"deny", false, codes.PermissionDenied},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			e := NewExtAuthZFilter(&configv1.Config{AllowUnmatchedRequests: tt.allow})
+			got, err := e.Check(context.Background(), &envoy.CheckRequest{})
+			require.NoError(t, err)
+			require.Equal(t, int32(tt.want), got.Status.Code)
+		})
+	}
+}
+
+func TestFiltersMatch(t *testing.T) {
+	tests := []struct {
+		name    string
+		filters []*configv1.Filter
+		want    codes.Code
+	}{
+		{"no-filters", nil, codes.OK},
+		{"all-filters-match", []*configv1.Filter{mock(true), mock(true)}, codes.OK},
+		{"one-filter-deny", []*configv1.Filter{mock(true), mock(false)}, codes.PermissionDenied},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := &configv1.Config{Chains: []*configv1.FilterChain{{Filters: tt.filters}}}
+			e := NewExtAuthZFilter(cfg)
+
+			got, err := e.Check(context.Background(), &envoy.CheckRequest{})
+			require.NoError(t, err)
+			require.Equal(t, int32(tt.want), got.Status.Code)
+		})
+	}
+}
+
+func TestUseFirstMatchingChain(t *testing.T) {
+	cfg := &configv1.Config{
+		Chains: []*configv1.FilterChain{
+			{
+				// Chain to be ignored
+				Match:   eq("no-match"),
+				Filters: []*configv1.Filter{mock(false)},
+			},
+			{
+				// Chain to be used
+				Match:   eq("match"),
+				Filters: []*configv1.Filter{mock(true)},
+			},
+			{
+				// Always matches but should not be used as the previous
+				// chain already matched
+				Filters: []*configv1.Filter{mock(false)},
+			},
+		},
+	}
+
+	e := NewExtAuthZFilter(cfg)
+
+	got, err := e.Check(context.Background(), header("match"))
+	require.NoError(t, err)
+	require.Equal(t, int32(codes.OK), got.Status.Code)
+}
+
+func TestCheckMock(t *testing.T) {
+	tests := []struct {
+		name  string
+		allow bool
+		want  bool
+	}{
+		{"allow", true, true},
+		{"deny", false, false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			e := &ExtAuthZFilter{}
+			got, err := e.checkMock(
+				context.Background(),
+				&envoy.CheckRequest{},
+				&mockv1.MockConfig{Allow: tt.allow},
+			)
+			require.NoError(t, err)
+			require.Equal(t, tt.want, got)
+		})
+	}
+
+}
+
+func TestMatch(t *testing.T) {
+	tests := []struct {
+		name  string
+		match *configv1.Match
+		req   *envoy.CheckRequest
+		want  bool
+	}{
+		{"no-headers", eq("test"), &envoy.CheckRequest{}, false},
+		{"no-match-condition", nil, &envoy.CheckRequest{}, true},
+		{"equality-match", eq("test"), header("test"), true},
+		{"equality-no-match", eq("test"), header("no-match"), false},
+		{"prefix-match", prefix("test"), header("test-123"), true},
+		{"prefix-no-match", prefix("test"), header("no-match"), false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.want, matches(tt.match, tt.req))
+		})
+	}
+}
+
+func TestGrpcNoChainsMatched(t *testing.T) {
+	e := NewExtAuthZFilter(&configv1.Config{})
+	s := NewTestServer(e.Register)
+	go func() { require.NoError(t, s.Start()) }()
+	t.Cleanup(s.Stop)
+
+	conn, err := s.GRPCConn()
+	require.NoError(t, err)
+	client := envoy.NewAuthorizationClient(conn)
+
+	ok, err := client.Check(context.Background(), &envoy.CheckRequest{})
+	require.NoError(t, err)
+	require.Equal(t, int32(codes.PermissionDenied), ok.Status.Code)
+}
+
+func mock(allow bool) *configv1.Filter {
+	return &configv1.Filter{
+		Type: &configv1.Filter_Mock{
+			Mock: &mockv1.MockConfig{
+				Allow: allow,
+			},
+		},
+	}
+}
+
+func eq(value string) *configv1.Match {
+	return &configv1.Match{
+		Header: "X-Test-Headers",
+		Criteria: &configv1.Match_Equality{
+			Equality: value,
+		},
+	}
+}
+
+func prefix(value string) *configv1.Match {
+	return &configv1.Match{
+		Header: "X-Test-Headers",
+		Criteria: &configv1.Match_Prefix{
+			Prefix: value,
+		},
+	}
+}
+
+func header(value string) *envoy.CheckRequest {
+	return &envoy.CheckRequest{
+		Attributes: &envoy.AttributeContext{
+			Request: &envoy.AttributeContext_Request{
+				Http: &envoy.AttributeContext_HttpRequest{
+					Headers: map[string]string{
+						"x-test-headers": value,
+					},
+				},
+			},
+		},
+	}
+}

internal/server/server.go

@@ -15,7 +15,6 @@
 package server
 
 import (
-	"errors"
 	"fmt"
 	"net"
 
@@ -38,8 +37,6 @@ var (
 	_ run.Service   = (*Server)(nil)
 )
 
-var ErrInvalidAddress = errors.New("invalid address")
-
 // Server that runs as a unit in a run.Group.
 type Server struct {
 	log telemetry.Logger