refactor: replace glog in validation/appprotect.go (#6598)

AlexFenlon · web-flow · commit 297aa1b7d214 · 2024-10-07T17:09:46.000+01:00
diff --git a/internal/k8s/appprotect/app_protect_configuration.go b/internal/k8s/appprotect/app_protect_configuration.go
@@ -3,6 +3,7 @@ package appprotect
 import (
 	"errors"
 	"fmt"
+	"log/slog"
 	"sort"
 	"time"
 
@@ -116,19 +117,21 @@ type ConfigurationImpl struct {
 	Policies map[string]*PolicyEx
 	LogConfs map[string]*LogConfEx
 	UserSigs map[string]*UserSigEx
+	Logger   *slog.Logger
 }
 
 // NewConfiguration creates a new App Protect Configuration
-func NewConfiguration() Configuration {
-	return newConfigurationImpl()
+func NewConfiguration(l *slog.Logger) Configuration {
+	return newConfigurationImpl(l)
 }
 
 // NewConfiguration creates a new App Protect Configuration
-func newConfigurationImpl() *ConfigurationImpl {
+func newConfigurationImpl(l *slog.Logger) *ConfigurationImpl {
 	return &ConfigurationImpl{
 		Policies: make(map[string]*PolicyEx),
 		LogConfs: make(map[string]*LogConfEx),
 		UserSigs: make(map[string]*UserSigEx),
+		Logger:   l,
 	}
 }
 
@@ -205,8 +208,8 @@ func (s appProtectUserSigSlice) Swap(i, j int) {
 	s[i], s[j] = s[j], s[i]
 }
 
-func createAppProtectPolicyEx(policyObj *unstructured.Unstructured) (*PolicyEx, error) {
-	err := validation.ValidateAppProtectPolicy(policyObj)
+func createAppProtectPolicyEx(policyObj *unstructured.Unstructured, l *slog.Logger) (*PolicyEx, error) {
+	err := validation.ValidateAppProtectPolicy(policyObj, l)
 	if err != nil {
 		errMsg := fmt.Sprintf("Error validating policy %s: %v", policyObj.GetName(), err)
 		return &PolicyEx{Obj: policyObj, IsValid: false, ErrorMsg: failedValidationErrorMsg}, errors.New(errMsg)
@@ -347,7 +350,7 @@ func (ci *ConfigurationImpl) verifyPolicyAgainstUserSigs(policy *PolicyEx) bool
 // AddOrUpdatePolicy adds or updates an App Protect Policy to App Protect Configuration
 func (ci *ConfigurationImpl) AddOrUpdatePolicy(policyObj *unstructured.Unstructured) (changes []Change, problems []Problem) {
 	resNsName := appprotectcommon.GetNsName(policyObj)
-	policy, err := createAppProtectPolicyEx(policyObj)
+	policy, err := createAppProtectPolicyEx(policyObj, ci.Logger)
 	if err != nil {
 		ci.Policies[resNsName] = policy
 		return append(changes, Change{Op: Delete, Resource: policy}),
diff --git a/internal/k8s/appprotect/app_protect_configuration_test.go b/internal/k8s/appprotect/app_protect_configuration_test.go
@@ -1,9 +1,14 @@
 package appprotect
 
 import (
+	"io"
+	"log/slog"
 	"testing"
 	"time"
 
+	nic_glog "github.com/nginxinc/kubernetes-ingress/internal/logger/glog"
+	"github.com/nginxinc/kubernetes-ingress/internal/logger/levels"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -178,8 +183,8 @@ func TestCreateAppProtectPolicyEx(t *testing.T) {
 
 	for _, test := range tests {
 		test.expectedPolicyEx.Obj = test.policy
-
-		policyEx, err := createAppProtectPolicyEx(test.policy)
+		l := slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo}))
+		policyEx, err := createAppProtectPolicyEx(test.policy, l)
 		if (err != nil) != test.wantErr {
 			t.Errorf("createAppProtectPolicyEx() returned %v, for the case of %s", err, test.msg)
 		}
@@ -511,7 +516,8 @@ func TestAddOrUpdatePolicy(t *testing.T) {
 			},
 		},
 	}
-	apc := newConfigurationImpl()
+	l := slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo}))
+	apc := newConfigurationImpl(l)
 	apc.UserSigs["testing/TestUsersig"] = &UserSigEx{Tag: "test", RevTime: parseTime("2019-01-01T18:32:02Z"), IsValid: true}
 	tests := []struct {
 		policy           *unstructured.Unstructured
@@ -643,7 +649,8 @@ func TestAddOrUpdateLogConf(t *testing.T) {
 			},
 		},
 	}
-	apc := NewConfiguration()
+	l := slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo}))
+	apc := NewConfiguration(l)
 	tests := []struct {
 		logconf          *unstructured.Unstructured
 		expectedChanges  []Change
@@ -792,7 +799,8 @@ func TestAddOrUpdateUserSig(t *testing.T) {
 		},
 	}
 
-	appProtectConfiguration := newConfigurationImpl()
+	l := slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo}))
+	appProtectConfiguration := newConfigurationImpl(l)
 	appProtectConfiguration.UserSigs["testing/test1"] = &UserSigEx{
 		Obj:      testUserSig1,
 		Tag:      "test1",
@@ -906,7 +914,8 @@ func TestAddOrUpdateUserSig(t *testing.T) {
 
 func TestDeletePolicy(t *testing.T) {
 	t.Parallel()
-	appProtectConfiguration := newConfigurationImpl()
+	l := slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo}))
+	appProtectConfiguration := newConfigurationImpl(l)
 	appProtectConfiguration.Policies["testing/test"] = &PolicyEx{}
 	tests := []struct {
 		key              string
@@ -945,7 +954,8 @@ func TestDeletePolicy(t *testing.T) {
 
 func TestDeleteLogConf(t *testing.T) {
 	t.Parallel()
-	appProtectConfiguration := newConfigurationImpl()
+	l := slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo}))
+	appProtectConfiguration := newConfigurationImpl(l)
 	appProtectConfiguration.LogConfs["testing/test"] = &LogConfEx{}
 	tests := []struct {
 		key              string
@@ -1018,7 +1028,8 @@ func TestDeleteUserSig(t *testing.T) {
 			},
 		},
 	}
-	appProtectConfiguration := newConfigurationImpl()
+	l := slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo}))
+	appProtectConfiguration := newConfigurationImpl(l)
 	appProtectConfiguration.UserSigs["testing/test1"] = &UserSigEx{
 		IsValid: true,
 		Obj:     testUserSig1,
@@ -1165,7 +1176,8 @@ func TestGetAppProtectResource(t *testing.T) {
 			msg:     "Invalid kind, Negative",
 		},
 	}
-	appProtectConfiguration := newConfigurationImpl()
+	l := slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo}))
+	appProtectConfiguration := newConfigurationImpl(l)
 	appProtectConfiguration.Policies["testing/test1"] = &PolicyEx{IsValid: true, Obj: &unstructured.Unstructured{}}
 	appProtectConfiguration.Policies["testing/test2"] = &PolicyEx{IsValid: false, Obj: &unstructured.Unstructured{}, ErrorMsg: "validation failed"}
 	appProtectConfiguration.LogConfs["testing/test1"] = &LogConfEx{IsValid: true, Obj: &unstructured.Unstructured{}}
diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go
@@ -354,7 +354,7 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc
 		input.IsIPV6Disabled,
 	)
 
-	lbc.appProtectConfiguration = appprotect.NewConfiguration()
+	lbc.appProtectConfiguration = appprotect.NewConfiguration(lbc.Logger)
 	lbc.dosConfiguration = appprotectdos.NewConfiguration(input.AppProtectDosEnabled)
 
 	lbc.secretStore = secrets.NewLocalSecretStore(lbc.configurator)
diff --git a/pkg/apis/configuration/validation/appprotect.go b/pkg/apis/configuration/validation/appprotect.go
@@ -2,9 +2,11 @@ package validation
 
 import (
 	"fmt"
+	"log/slog"
 	"strings"
 
-	"github.com/golang/glog"
+	nl "github.com/nginxinc/kubernetes-ingress/internal/logger"
+
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
@@ -50,7 +52,7 @@ var appProtectPolicyExtRefs = [][]string{
 }
 
 // ValidateAppProtectPolicy validates Policy resource
-func ValidateAppProtectPolicy(policy *unstructured.Unstructured) error {
+func ValidateAppProtectPolicy(policy *unstructured.Unstructured, l *slog.Logger) error {
 	polName := policy.GetName()
 
 	err := ValidateRequiredFields(policy, appProtectPolicyRequiredFields)
@@ -65,7 +67,7 @@ func ValidateAppProtectPolicy(policy *unstructured.Unstructured) error {
 
 	if len(extRefs) > 0 {
 		for _, ref := range extRefs {
-			glog.V(2).Infof("Warning: Field %s (External reference) is Deprecated.", ref)
+			nl.Debugf(l, "Warning: Field %s (External reference) is Deprecated.", ref)
 		}
 	}
 
diff --git a/pkg/apis/configuration/validation/appprotect_test.go b/pkg/apis/configuration/validation/appprotect_test.go
@@ -1,8 +1,13 @@
 package validation
 
 import (
+	"io"
+	"log/slog"
 	"testing"
 
+	nic_glog "github.com/nginxinc/kubernetes-ingress/internal/logger/glog"
+	"github.com/nginxinc/kubernetes-ingress/internal/logger/levels"
+
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
@@ -49,7 +54,8 @@ func TestValidateAppProtectPolicy(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		err := ValidateAppProtectPolicy(test.policy)
+		l := slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo}))
+		err := ValidateAppProtectPolicy(test.policy, l)
 		if test.expectErr && err == nil {
 			t.Errorf("validateAppProtectPolicy() returned no error for the case of %s", test.msg)
 		}

Original file line number	Diff line number	Diff line change
`@@ -354,7 +354,7 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc`
`354`	`354`	`input.IsIPV6Disabled,`
`355`	`355`	`)`
`356`	`356`
`357`		`- lbc.appProtectConfiguration = appprotect.NewConfiguration()`
	`357`	`+ lbc.appProtectConfiguration = appprotect.NewConfiguration(lbc.Logger)`
`358`	`358`	`lbc.dosConfiguration = appprotectdos.NewConfiguration(input.AppProtectDosEnabled)`
`359`	`359`
`360`	`360`	`lbc.secretStore = secrets.NewLocalSecretStore(lbc.configurator)`
Original file line number	Diff line number	Diff line change
`@@ -2,9 +2,11 @@ package validation`
`2`	`2`
`3`	`3`	`import (`
`4`	`4`	`"fmt"`
	`5`	`+ "log/slog"`
`5`	`6`	`"strings"`
`6`	`7`
`7`		`- "github.com/golang/glog"`
	`8`	`+ nl "github.com/nginxinc/kubernetes-ingress/internal/logger"`
	`9`	`+`
`8`	`10`	`"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"`
`9`	`11`	`)`
`10`	`12`
`@@ -50,7 +52,7 @@ var appProtectPolicyExtRefs = [][]string{`
`50`	`52`	`}`
`51`	`53`
`52`	`54`	`// ValidateAppProtectPolicy validates Policy resource`
`53`		`-func ValidateAppProtectPolicy(policy *unstructured.Unstructured) error {`
	`55`	`+func ValidateAppProtectPolicy(policy unstructured.Unstructured, l slog.Logger) error {`
`54`	`56`	`polName := policy.GetName()`
`55`	`57`
`56`	`58`	`err := ValidateRequiredFields(policy, appProtectPolicyRequiredFields)`
`@@ -65,7 +67,7 @@ func ValidateAppProtectPolicy(policy *unstructured.Unstructured) error {`
`65`	`67`
`66`	`68`	`if len(extRefs) > 0 {`
`67`	`69`	`for _, ref := range extRefs {`
`68`		`- glog.V(2).Infof("Warning: Field %s (External reference) is Deprecated.", ref)`
	`70`	`+ nl.Debugf(l, "Warning: Field %s (External reference) is Deprecated.", ref)`
`69`	`71`	`}`
`70`	`72`	`}`
`71`	`73`
Original file line number	Diff line number	Diff line change
`@@ -1,8 +1,13 @@`
`1`	`1`	`package validation`
`2`	`2`
`3`	`3`	`import (`
	`4`	`+ "io"`
	`5`	`+ "log/slog"`
`4`	`6`	`"testing"`
`5`	`7`
	`8`	`+ nic_glog "github.com/nginxinc/kubernetes-ingress/internal/logger/glog"`
	`9`	`+ "github.com/nginxinc/kubernetes-ingress/internal/logger/levels"`
	`10`	`+`
`6`	`11`	`"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"`
`7`	`12`	`)`
`8`	`13`
`@@ -49,7 +54,8 @@ func TestValidateAppProtectPolicy(t *testing.T) {`
`49`	`54`	`}`
`50`	`55`
`51`	`56`	`for _, test := range tests {`
`52`		`- err := ValidateAppProtectPolicy(test.policy)`
	`57`	`+ l := slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo}))`
	`58`	`+ err := ValidateAppProtectPolicy(test.policy, l)`
`53`	`59`	`if test.expectErr && err == nil {`
`54`	`60`	`t.Errorf("validateAppProtectPolicy() returned no error for the case of %s", test.msg)`
`55`	`61`	`}`