From 3eaaa7c33bfff1dad14bc0738ca07312820671a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Fri, 22 Sep 2023 15:29:11 +0200 Subject: [PATCH] fix: include Licenses and ConsumerGroups in sanitized copies of config --- CHANGELOG.md | 4 +- internal/dataplane/deckgen/generate.go | 2 +- internal/dataplane/kongstate/kongstate.go | 9 +++- .../dataplane/kongstate/kongstate_test.go | 45 +++++++++++++++++++ internal/dataplane/kongstate/license.go | 20 +++++++++ internal/dataplane/parser/parser.go | 2 +- 6 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 internal/dataplane/kongstate/license.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 9afb518948..e9bc3fb7f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -127,7 +127,7 @@ Adding a new version? You'll need three changes: - KongPlugins used on multiple resources will no longer result in `instance_name` collisions. [#4588](https://github.com/Kong/kubernetes-ingress-controller/issues/4588) -- Fix `panic` when last known configuratino fetcher gets a `nil` Status when requesting +- Fix `panic` when last known configuration fetcher gets a `nil` Status when requesting `/status` from Kong Gateway. This happens when Gateway is responding with a 50x HTTP status code. [#4627](https://github.com/Kong/kubernetes-ingress-controller/issues/4627) @@ -137,6 +137,8 @@ Adding a new version? You'll need three changes: deleting existing configuration during an API server restart. [#4641](https://github.com/Kong/kubernetes-ingress-controller/issues/4641) [#4643](https://github.com/Kong/kubernetes-ingress-controller/issues/4643) +- Fix `Licenses` and `ConsumerGroups` missing in sanitized copies of Kong configuration. + [#4710](https://github.com/Kong/kubernetes-ingress-controller/pull/4710 ## [2.11.1] diff --git a/internal/dataplane/deckgen/generate.go b/internal/dataplane/deckgen/generate.go index b11b4650b6..effd4dd43f 100644 --- a/internal/dataplane/deckgen/generate.go +++ b/internal/dataplane/deckgen/generate.go @@ -147,7 +147,7 @@ func ToDeckContent( for _, c := range k8sState.Licenses { content.Licenses = append(content.Licenses, - file.FLicense{License: c}) + file.FLicense{License: c.License}) } sort.SliceStable(content.Licenses, func(i, j int) bool { diff --git a/internal/dataplane/kongstate/kongstate.go b/internal/dataplane/kongstate/kongstate.go index 627108048d..46d97c5354 100644 --- a/internal/dataplane/kongstate/kongstate.go +++ b/internal/dataplane/kongstate/kongstate.go @@ -24,7 +24,7 @@ type KongState struct { Upstreams []Upstream Certificates []Certificate CACertificates []kong.CACertificate - Licenses []kong.License + Licenses []License Plugins []Plugin Consumers []Consumer ConsumerGroups []ConsumerGroup @@ -49,6 +49,13 @@ func (ks *KongState) SanitizedCopy() *KongState { } return }(), + Licenses: func() (res []License) { + for _, v := range ks.Licenses { + res = append(res, *v.SanitizedCopy()) + } + return + }(), + ConsumerGroups: ks.ConsumerGroups, } } diff --git a/internal/dataplane/kongstate/kongstate_test.go b/internal/dataplane/kongstate/kongstate_test.go index 558d79f4db..27a70cc2ae 100644 --- a/internal/dataplane/kongstate/kongstate_test.go +++ b/internal/dataplane/kongstate/kongstate_test.go @@ -16,6 +16,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8stypes "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "github.com/kong/kubernetes-ingress-controller/v2/internal/annotations" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/failures" @@ -31,6 +32,7 @@ var kongConsumerTypeMeta = metav1.TypeMeta{ } func TestKongState_SanitizedCopy(t *testing.T) { + testedFields := sets.New[string]() for _, tt := range []struct { name string in KongState @@ -47,6 +49,10 @@ func TestKongState_SanitizedCopy(t *testing.T) { Consumers: []Consumer{{ KeyAuths: []*KeyAuth{{kong.KeyAuth{ID: kong.String("1"), Key: kong.String("secret")}}}, }}, + Licenses: []License{{kong.License{ID: kong.String("1"), Payload: kong.String("secret")}}}, + ConsumerGroups: []ConsumerGroup{{ + ConsumerGroup: kong.ConsumerGroup{ID: kong.String("1"), Name: kong.String("consumer-group")}, + }}, }, want: KongState{ Services: []Service{{Service: kong.Service{ID: kong.String("1")}}}, @@ -57,14 +63,53 @@ func TestKongState_SanitizedCopy(t *testing.T) { Consumers: []Consumer{{ KeyAuths: []*KeyAuth{{kong.KeyAuth{ID: kong.String("1"), Key: redactedString}}}, }}, + Licenses: []License{{kong.License{ID: kong.String("1"), Payload: redactedString}}}, + ConsumerGroups: []ConsumerGroup{{ + ConsumerGroup: kong.ConsumerGroup{ID: kong.String("1"), Name: kong.String("consumer-group")}, + }}, }, }, } { t.Run(tt.name, func(t *testing.T) { + testedFields.Insert(extractNotEmptyFieldNames(tt.in)...) got := *tt.in.SanitizedCopy() assert.Equal(t, tt.want, got) }) } + + ensureAllKongStateFieldsAreCoveredInTest(t, testedFields.UnsortedList()) +} + +// extractNotEmptyFieldNames returns the names of all non-empty fields in the given KongState. +// This is to programmatically find out what fields are used in a test case. +func extractNotEmptyFieldNames(s KongState) []string { + var fields []string + typ := reflect.ValueOf(s).Type() + for i := 0; i < typ.NumField(); i++ { + f := typ.Field(i) + v := reflect.ValueOf(s).Field(i) + if !f.Anonymous && f.IsExported() && !v.IsZero() { + fields = append(fields, f.Name) + } + } + return fields +} + +// ensureAllKongStateFieldsAreCoveredInTest ensures that all fields in KongState are covered in a tests. +func ensureAllKongStateFieldsAreCoveredInTest(t *testing.T, testedFields []string) { + allKongStateFields := func() []string { + var fields []string + typ := reflect.ValueOf(KongState{}).Type() + for i := 0; i < typ.NumField(); i++ { + fields = append(fields, typ.Field(i).Name) + } + return fields + }() + + // Meta test - ensure we have testcases covering all fields in KongState. + for _, field := range allKongStateFields { + assert.True(t, lo.Contains(testedFields, field), "field %s wasn't tested", field) + } } func TestGetPluginRelations(t *testing.T) { diff --git a/internal/dataplane/kongstate/license.go b/internal/dataplane/kongstate/license.go new file mode 100644 index 0000000000..36faa14da9 --- /dev/null +++ b/internal/dataplane/kongstate/license.go @@ -0,0 +1,20 @@ +package kongstate + +import "github.com/kong/go-kong/kong" + +// License represents the license object in Kong. +type License struct { + kong.License +} + +// SanitizedCopy returns a shallow copy with sensitive values redacted best-effort. +func (l License) SanitizedCopy() *License { + return &License{ + License: kong.License{ + ID: l.ID, + Payload: redactedString, + CreatedAt: l.CreatedAt, + UpdatedAt: l.UpdatedAt, + }, + } +} diff --git a/internal/dataplane/parser/parser.go b/internal/dataplane/parser/parser.go index 481b016532..0f7d5ab8b2 100644 --- a/internal/dataplane/parser/parser.go +++ b/internal/dataplane/parser/parser.go @@ -264,7 +264,7 @@ func (p *Parser) BuildKongConfig() KongConfigBuildingResult { if p.licenseGetter != nil { optionalLicense := p.licenseGetter.GetLicense() if l, ok := optionalLicense.Get(); ok { - result.Licenses = append(result.Licenses, l) + result.Licenses = append(result.Licenses, kongstate.License{License: l}) } }