Skip to content

Commit

Permalink
refactor: replace glog in validation/appprotect.go (#6598)
Browse files Browse the repository at this point in the history
  • Loading branch information
AlexFenlon authored Oct 7, 2024
1 parent bcaec96 commit 297aa1b
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 20 deletions.
15 changes: 9 additions & 6 deletions internal/k8s/appprotect/app_protect_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package appprotect
import (
"errors"
"fmt"
"log/slog"
"sort"
"time"

Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}),
Expand Down
30 changes: 21 additions & 9 deletions internal/k8s/appprotect/app_protect_configuration_test.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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{}}
Expand Down
2 changes: 1 addition & 1 deletion internal/k8s/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 5 additions & 3 deletions pkg/apis/configuration/validation/appprotect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
}

Expand Down
8 changes: 7 additions & 1 deletion pkg/apis/configuration/validation/appprotect_test.go
Original file line number Diff line number Diff line change
@@ -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"
)

Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit 297aa1b

Please sign in to comment.