Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(consumer): duplicate ID2 breaking DB-mode #1007

Closed
wants to merge 6 commits into from
+365 −114
Closed

fix(consumer): duplicate ID2 breaking DB-mode #1007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
06624fe
fix(consumer): duplicate ID2 breaking DB-mode
mflendrich Jan 19, 2021
3b03f7e
refactor(controller): implement sortByString
mflendrich Jan 19, 2021
feff0fc
test(controller): toDeckContent sorts creds
mflendrich Jan 20, 2021
b97ed34
test(kongstate): deduplicates credentials
mflendrich Jan 20, 2021
87e167e
apply PR suggestion
mflendrich Jan 20, 2021
70dbb1a
apply PR suggestion: golint
mflendrich Jan 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 42 additions & 38 deletions internal/ingress/controller/kong.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,11 +292,14 @@ func (n *KongController) getIngressControllerTags() []string {
return res
}

// FormatVersion denotes the format version of decK files that we generate.
const FormatVersion = "1.1"
mflendrich marked this conversation as resolved.
Show resolved Hide resolved

func (n *KongController) toDeckContent(
ctx context.Context,
k8sState *kongstate.KongState) *file.Content {
var content file.Content
content.FormatVersion = "1.1"
content.FormatVersion = FormatVersion
var err error

for _, s := range k8sState.Services {
Expand All @@ -310,9 +313,7 @@ func (n *KongController) toDeckContent(
n.Logger.Errorf("failed to fill-in defaults for plugin: %s", *plugin.Name)
}
service.Plugins = append(service.Plugins, &plugin)
sort.SliceStable(service.Plugins, func(i, j int) bool {
return strings.Compare(*service.Plugins[i].Name, *service.Plugins[j].Name) > 0
})
sortByString(service.Plugins, func(i int) string { return *service.Plugins[i].Name })
}

for _, r := range s.Routes {
Expand All @@ -328,20 +329,14 @@ func (n *KongController) toDeckContent(
n.Logger.Errorf("failed to fill-in defaults for plugin: %s", *plugin.Name)
}
route.Plugins = append(route.Plugins, &plugin)
sort.SliceStable(route.Plugins, func(i, j int) bool {
return strings.Compare(*route.Plugins[i].Name, *route.Plugins[j].Name) > 0
})
sortByString(route.Plugins, func(i int) string { return *route.Plugins[i].Name })
}
service.Routes = append(service.Routes, &route)
}
sort.SliceStable(service.Routes, func(i, j int) bool {
return strings.Compare(*service.Routes[i].Name, *service.Routes[j].Name) > 0
})
sortByString(service.Routes, func(i int) string { return *service.Routes[i].Name })
content.Services = append(content.Services, service)
}
sort.SliceStable(content.Services, func(i, j int) bool {
return strings.Compare(*content.Services[i].Name, *content.Services[j].Name) > 0
})
sortByString(content.Services, func(i int) string { return *content.Services[i].Name })

for _, plugin := range k8sState.Plugins {
plugin := file.FPlugin{
Expand All @@ -353,10 +348,7 @@ func (n *KongController) toDeckContent(
}
content.Plugins = append(content.Plugins, plugin)
}
sort.SliceStable(content.Plugins, func(i, j int) bool {
return strings.Compare(pluginString(content.Plugins[i]),
pluginString(content.Plugins[j])) > 0
})
sortByString(content.Plugins, func(i int) string { return pluginString(content.Plugins[i]) })

for _, u := range k8sState.Upstreams {
n.fillUpstream(&u.Upstream)
Expand All @@ -365,47 +357,53 @@ func (n *KongController) toDeckContent(
target := file.FTarget{Target: t.Target}
upstream.Targets = append(upstream.Targets, &target)
}
sort.SliceStable(upstream.Targets, func(i, j int) bool {
return strings.Compare(*upstream.Targets[i].Target.Target, *upstream.Targets[j].Target.Target) > 0
})
sortByString(upstream.Targets, func(i int) string { return *upstream.Targets[i].Target.Target })
content.Upstreams = append(content.Upstreams, upstream)
}
sort.SliceStable(content.Upstreams, func(i, j int) bool {
return strings.Compare(*content.Upstreams[i].Name, *content.Upstreams[j].Name) > 0
})
sortByString(content.Upstreams, func(i int) string { return *content.Upstreams[i].Name })

for _, c := range k8sState.Certificates {
cert := getFCertificateFromKongCert(c.Certificate)
content.Certificates = append(content.Certificates, cert)
}
sort.SliceStable(content.Certificates, func(i, j int) bool {
return strings.Compare(*content.Certificates[i].Cert, *content.Certificates[j].Cert) > 0
})
sortByString(content.Certificates, func(i int) string { return *content.Certificates[i].Cert })

for _, c := range k8sState.CACertificates {
content.CACertificates = append(content.CACertificates,
file.FCACertificate{CACertificate: c})
}
sort.SliceStable(content.CACertificates, func(i, j int) bool {
return strings.Compare(*content.CACertificates[i].Cert, *content.CACertificates[j].Cert) > 0
})
sortByString(content.CACertificates, func(i int) string { return *content.CACertificates[i].Cert })

for _, c := range k8sState.Consumers {
consumer := file.FConsumer{Consumer: c.Consumer}
for _, p := range c.Plugins {
consumer.Plugins = append(consumer.Plugins, &file.FPlugin{Plugin: p})
}
consumer.KeyAuths = c.KeyAuths
consumer.HMACAuths = c.HMACAuths
consumer.BasicAuths = c.BasicAuths
consumer.JWTAuths = c.JWTAuths
consumer.ACLGroups = c.ACLGroups
consumer.Oauth2Creds = c.Oauth2Creds

for k := range c.KeyAuths {
consumer.KeyAuths = append(consumer.KeyAuths, c.KeyAuths[k])
}
sortByString(consumer.KeyAuths, func(i int) string { return *consumer.KeyAuths[i].Key })
for k := range c.HMACAuths {
consumer.HMACAuths = append(consumer.HMACAuths, c.HMACAuths[k])
}
sortByString(consumer.HMACAuths, func(i int) string { return *consumer.HMACAuths[i].Username })
for k := range c.BasicAuths {
consumer.BasicAuths = append(consumer.BasicAuths, c.BasicAuths[k])
}
sortByString(consumer.BasicAuths, func(i int) string { return *consumer.BasicAuths[i].Username })
for k := range c.JWTAuths {
consumer.JWTAuths = append(consumer.JWTAuths, c.JWTAuths[k])
}
sortByString(consumer.JWTAuths, func(i int) string { return *consumer.JWTAuths[i].Key })
for k := range c.Oauth2Creds {
consumer.Oauth2Creds = append(consumer.Oauth2Creds, c.Oauth2Creds[k])
}
sortByString(consumer.Oauth2Creds, func(i int) string { return *consumer.Oauth2Creds[i].ClientID })
content.Consumers = append(content.Consumers, consumer)
}
sort.SliceStable(content.Consumers, func(i, j int) bool {
return strings.Compare(*content.Consumers[i].Username, *content.Consumers[j].Username) > 0
})
sortByString(content.Consumers, func(i int) string { return *content.Consumers[i].Username })

selectorTags := n.getIngressControllerTags()
if len(selectorTags) > 0 {
content.Info = &file.Info{
Expand All @@ -415,6 +413,12 @@ func (n *KongController) toDeckContent(

return &content
}

func sortByString(slice interface{}, fieldFn func(i int) string) {
mflendrich marked this conversation as resolved.
Show resolved Hide resolved
mflendrich marked this conversation as resolved.
Show resolved Hide resolved
lessFn := func(i, j int) bool { return strings.Compare(fieldFn(i), fieldFn(j)) < 0 }
sort.SliceStable(slice, lessFn)
}

func getFCertificateFromKongCert(kongCert kong.Certificate) file.FCertificate {
var res file.FCertificate
if kongCert.ID != nil {
Expand Down
87 changes: 87 additions & 0 deletions internal/ingress/controller/kong_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package controller

import (
"context"
"reflect"
"testing"

"github.com/kong/deck/file"
"github.com/kong/go-kong/kong"
"github.com/kong/kubernetes-ingress-controller/internal/ingress/controller/parser/kongstate"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
mflendrich marked this conversation as resolved.
Show resolved Hide resolved
)

func Test_renderConfigWithCustomEntities(t *testing.T) {
Expand Down Expand Up @@ -118,3 +121,87 @@ func Test_renderConfigWithCustomEntities(t *testing.T) {
})
}
}

func Test_toDeckContent(t *testing.T) {
for _, tt := range []struct {
name string
in kongstate.KongState
want file.Content
}{
{
name: "sorts credentials consistently",
in: kongstate.KongState{
Consumers: []kongstate.Consumer{
{
KeyAuths: map[string]*kong.KeyAuth{
"a": {Key: kong.String("key-22")},
"b": {Key: kong.String("key-11")},
"c": {Key: kong.String("key-33")},
},
HMACAuths: map[string]*kong.HMACAuth{
"a": {Username: kong.String("hmac-22")},
"b": {Username: kong.String("hmac-11")},
"c": {Username: kong.String("hmac-33")},
},
JWTAuths: map[string]*kong.JWTAuth{
"a": {Key: kong.String("jwt-22")},
"b": {Key: kong.String("jwt-11")},
"c": {Key: kong.String("jwt-33")},
},
BasicAuths: map[string]*kong.BasicAuth{
"a": {Username: kong.String("basic-22")},
"b": {Username: kong.String("basic-11")},
"c": {Username: kong.String("basic-33")},
},
Oauth2Creds: map[string]*kong.Oauth2Credential{
"a": {ClientID: kong.String("oauth2-22")},
"b": {ClientID: kong.String("oauth2-11")},
"c": {ClientID: kong.String("oauth2-33")},
},
},
},
},
want: file.Content{
FormatVersion: FormatVersion,
Consumers: []file.FConsumer{
{
KeyAuths: []*kong.KeyAuth{
{Key: kong.String("key-11")},
{Key: kong.String("key-22")},
{Key: kong.String("key-33")},
},
HMACAuths: []*kong.HMACAuth{
{Username: kong.String("hmac-11")},
{Username: kong.String("hmac-22")},
{Username: kong.String("hmac-33")},
},
JWTAuths: []*kong.JWTAuth{
{Key: kong.String("jwt-11")},
{Key: kong.String("jwt-22")},
{Key: kong.String("jwt-33")},
},
BasicAuths: []*kong.BasicAuth{
{Username: kong.String("basic-11")},
{Username: kong.String("basic-22")},
{Username: kong.String("basic-33")},
},
Oauth2Creds: []*kong.Oauth2Credential{
{ClientID: kong.String("oauth2-11")},
{ClientID: kong.String("oauth2-22")},
{ClientID: kong.String("oauth2-33")},
},
},
},
},
},
} {
t.Run(tt.name, func(t *testing.T) {
n := KongController{
cfg: &Configuration{},
Logger: logrus.New(),
}
got := n.toDeckContent(context.Background(), &tt.in)
assert.Equal(t, tt.want, *got)
})
}
}
59 changes: 49 additions & 10 deletions internal/ingress/controller/parser/kongstate/consumer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,41 @@ import (
type Consumer struct {
kong.Consumer
Plugins []kong.Plugin
KeyAuths []*kong.KeyAuth
HMACAuths []*kong.HMACAuth
JWTAuths []*kong.JWTAuth
BasicAuths []*kong.BasicAuth
KeyAuths map[string]*kong.KeyAuth
HMACAuths map[string]*kong.HMACAuth
JWTAuths map[string]*kong.JWTAuth
BasicAuths map[string]*kong.BasicAuth
ACLGroups []*kong.ACLGroup

Oauth2Creds []*kong.Oauth2Credential
Oauth2Creds map[string]*kong.Oauth2Credential

K8sKongConsumer configurationv1.KongConsumer
}

// NewConsumer initializes an empty Consumer object.
func NewConsumer() Consumer {
mflendrich marked this conversation as resolved.
Show resolved Hide resolved
return Consumer{}.initEmpty()
}

func (c Consumer) initEmpty() Consumer {
mflendrich marked this conversation as resolved.
Show resolved Hide resolved
if c.KeyAuths == nil {
c.KeyAuths = map[string]*kong.KeyAuth{}
}
if c.HMACAuths == nil {
c.HMACAuths = map[string]*kong.HMACAuth{}
}
if c.JWTAuths == nil {
c.JWTAuths = map[string]*kong.JWTAuth{}
}
if c.BasicAuths == nil {
c.BasicAuths = map[string]*kong.BasicAuth{}
}
if c.Oauth2Creds == nil {
c.Oauth2Creds = map[string]*kong.Oauth2Credential{}
}
return c
}

func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credConfig interface{}) error {
switch credType {
case "key-auth", "keyauth_credential":
Expand All @@ -42,7 +66,10 @@ func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credCo
if cred.Key == nil {
return fmt.Errorf("key-auth for consumer %s is invalid: no key", *c.Username)
}
c.KeyAuths = append(c.KeyAuths, &cred)
if _, ok := c.KeyAuths[*cred.Key]; ok {
return fmt.Errorf("key-auth for consumer %s: duplicate key", *c.Username)
}
c.KeyAuths[*cred.Key] = &cred
case "basic-auth", "basicauth_credential":
var cred kong.BasicAuth
err := decodeCredential(credConfig, &cred)
Expand All @@ -52,7 +79,10 @@ func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credCo
if cred.Username == nil {
return fmt.Errorf("basic-auth for consumer %s is invalid: no username", *c.Username)
}
c.BasicAuths = append(c.BasicAuths, &cred)
if _, ok := c.BasicAuths[*cred.Username]; ok {
return fmt.Errorf("basic-auth for consumer %s: duplicate username %q", *c.Username, *cred.Username)
}
c.BasicAuths[*cred.Username] = &cred
case "hmac-auth", "hmacauth_credential":
var cred kong.HMACAuth
err := decodeCredential(credConfig, &cred)
Expand All @@ -62,7 +92,10 @@ func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credCo
if cred.Username == nil {
return fmt.Errorf("hmac-auth for consumer %s is invalid: no username", *c.Username)
}
c.HMACAuths = append(c.HMACAuths, &cred)
if _, ok := c.HMACAuths[*cred.Username]; ok {
return fmt.Errorf("hmac-auth for consumer %s: duplicate username %q", *c.Username, *cred.Username)
}
c.HMACAuths[*cred.Username] = &cred
case "oauth2":
var cred kong.Oauth2Credential
err := decodeCredential(credConfig, &cred)
Expand All @@ -72,7 +105,10 @@ func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credCo
if cred.ClientID == nil {
return fmt.Errorf("oauth2 for consumer %s is invalid: no client_id", *c.Username)
}
c.Oauth2Creds = append(c.Oauth2Creds, &cred)
if _, ok := c.Oauth2Creds[*cred.ClientID]; ok {
return fmt.Errorf("oauth2 for consumer %s: duplicate client ID %q", *c.Username, *cred.ClientID)
}
c.Oauth2Creds[*cred.ClientID] = &cred
case "jwt", "jwt_secret":
var cred kong.JWTAuth
err := decodeCredential(credConfig, &cred)
Expand All @@ -91,7 +127,10 @@ func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credCo
if cred.Key == nil {
return fmt.Errorf("jwt-auth for consumer %s is invalid: no key", *c.Username)
}
c.JWTAuths = append(c.JWTAuths, &cred)
if _, ok := c.JWTAuths[*cred.Key]; ok {
return fmt.Errorf("jwt-auth for consumer %s: duplicate key", *c.Username)
}
c.JWTAuths[*cred.Key] = &cred
case "acl":
var cred kong.ACLGroup
err := decodeCredential(credConfig, &cred)
Expand Down
Loading