diff --git a/internal/ingress/controller/parser/kongstate/consumer.go b/internal/ingress/controller/parser/kongstate/consumer.go index 55f26172e9..f7622520a8 100644 --- a/internal/ingress/controller/parser/kongstate/consumer.go +++ b/internal/ingress/controller/parser/kongstate/consumer.go @@ -33,6 +33,15 @@ func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credCo return fmt.Errorf("failed to decode key-auth credential: %w", err) } + // TODO we perform these validity checks here because passing credentials without these fields will panic deck + // later on. Ideally this should not be handled in the controller, but we cannot currently handle it elsewhere + // (i.e. in deck or go-kong) without entering a sync failure loop that cannot actually report the problem + // piece of configuration. if we can address those limitations, we should remove these checks. + // See https://github.com/Kong/deck/pull/223 and https://github.com/Kong/kubernetes-ingress-controller/issues/532 + // for more discussion. + if cred.Key == nil { + return fmt.Errorf("key-auth for consumer %s is invalid: no key", *c.Username) + } c.KeyAuths = append(c.KeyAuths, &cred) case "basic-auth", "basicauth_credential": var cred kong.BasicAuth @@ -40,6 +49,9 @@ func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credCo if err != nil { return fmt.Errorf("failed to decode basic-auth credential: %w", err) } + if cred.Username == nil { + return fmt.Errorf("basic-auth for consumer %s is invalid: no username", *c.Username) + } c.BasicAuths = append(c.BasicAuths, &cred) case "hmac-auth", "hmacauth_credential": var cred kong.HMACAuth @@ -47,6 +59,9 @@ func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credCo if err != nil { return fmt.Errorf("failed to decode hmac-auth credential: %w", err) } + if cred.Username == nil { + return fmt.Errorf("hmac-auth for consumer %s is invalid: no username", *c.Username) + } c.HMACAuths = append(c.HMACAuths, &cred) case "oauth2": var cred kong.Oauth2Credential @@ -54,6 +69,9 @@ func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credCo if err != nil { return fmt.Errorf("failed to decode oauth2 credential: %w", err) } + if cred.ClientID == nil { + return fmt.Errorf("oauth2 for consumer %s is invalid: no client_id", *c.Username) + } c.Oauth2Creds = append(c.Oauth2Creds, &cred) case "jwt", "jwt_secret": var cred kong.JWTAuth @@ -70,6 +88,9 @@ func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credCo if cred.Algorithm == nil || *cred.Algorithm == "" { cred.Algorithm = kong.String("HS256") } + if cred.Key == nil { + return fmt.Errorf("jwt-auth for consumer %s is invalid: no key", *c.Username) + } c.JWTAuths = append(c.JWTAuths, &cred) case "acl": var cred kong.ACLGroup @@ -77,6 +98,9 @@ func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credCo if err != nil { log.Errorf("failed to process ACL group: %v", err) } + if cred.Group == nil { + return fmt.Errorf("acl for consumer %s is invalid: no group", *c.Username) + } c.ACLGroups = append(c.ACLGroups, &cred) default: return fmt.Errorf("invalid credential type: '%v'", credType) diff --git a/internal/ingress/controller/parser/kongstate/consumer_test.go b/internal/ingress/controller/parser/kongstate/consumer_test.go index 88b5a6c867..9a158f1436 100644 --- a/internal/ingress/controller/parser/kongstate/consumer_test.go +++ b/internal/ingress/controller/parser/kongstate/consumer_test.go @@ -9,6 +9,7 @@ import ( ) func TestConsumer_SetCredential(t *testing.T) { + username := "example" type args struct { credType string consumer *Consumer @@ -46,6 +47,16 @@ func TestConsumer_SetCredential(t *testing.T) { }, wantErr: false, }, + { + name: "key-auth without key", + args: args{ + credType: "key-auth", + consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, + credConfig: map[string]string{}, + }, + result: &Consumer{Consumer: kong.Consumer{Username: &username}}, + wantErr: true, + }, { name: "keyauth_credential", args: args{ @@ -82,6 +93,16 @@ func TestConsumer_SetCredential(t *testing.T) { }, wantErr: false, }, + { + name: "basic-auth without username", + args: args{ + credType: "basic-auth", + consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, + credConfig: map[string]string{}, + }, + result: &Consumer{Consumer: kong.Consumer{Username: &username}}, + wantErr: true, + }, { name: "basicauth_credential", args: args{ @@ -122,6 +143,16 @@ func TestConsumer_SetCredential(t *testing.T) { }, wantErr: false, }, + { + name: "hmac-auth without username", + args: args{ + credType: "hmac-auth", + consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, + credConfig: map[string]string{}, + }, + result: &Consumer{Consumer: kong.Consumer{Username: &username}}, + wantErr: true, + }, { name: "hmacauth_credential", args: args{ @@ -166,6 +197,16 @@ func TestConsumer_SetCredential(t *testing.T) { }, wantErr: false, }, + { + name: "oauth2 without client_id", + args: args{ + credType: "oauth2", + consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, + credConfig: map[string]string{}, + }, + result: &Consumer{Consumer: kong.Consumer{Username: &username}}, + wantErr: true, + }, { name: "jwt", args: args{ @@ -190,6 +231,16 @@ func TestConsumer_SetCredential(t *testing.T) { }, wantErr: false, }, + { + name: "jwt without key", + args: args{ + credType: "jwt", + consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, + credConfig: map[string]string{}, + }, + result: &Consumer{Consumer: kong.Consumer{Username: &username}}, + wantErr: true, + }, { name: "jwt_secret", args: args{ @@ -230,6 +281,16 @@ func TestConsumer_SetCredential(t *testing.T) { }, wantErr: false, }, + { + name: "acl without group", + args: args{ + credType: "acl", + consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, + credConfig: map[string]string{}, + }, + result: &Consumer{Consumer: kong.Consumer{Username: &username}}, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {