Skip to content

Commit

Permalink
feat(crd) require username or customID for KongConsumer (#5137)
Browse files Browse the repository at this point in the history
Use CRD CEL rules to require at least one of username and customID fields be                                                                
present on KongConsumer resources.
  • Loading branch information
pmalek committed Nov 10, 2023
1 parent b4718ab commit 27755d7
Show file tree
Hide file tree
Showing 16 changed files with 63 additions and 31 deletions.
15 changes: 9 additions & 6 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,12 @@ Adding a new version? You'll need three changes:

### Changed

- `KongPlugin` and `KongClusterPlugin` now enforce only one of `config` and `configFrom`
to be set using the CRD validation expressions
[#5119](https://github.com/Kong/kubernetes-ingress-controller/pull/5119)
- CRD Validation Expressions
- `KongPlugin` and `KongClusterPlugin` now enforce only one of `config` and `configFrom`
to be set.
[#5119](https://github.com/Kong/kubernetes-ingress-controller/pull/5119)
- `KongConsumer` now enforces that at least one of `username` or `custom_id` is provided.
[#5137](https://github.com/Kong/kubernetes-ingress-controller/pull/5137)

## [3.0.0]

Expand All @@ -92,13 +95,13 @@ Adding a new version? You'll need three changes:
### Highlights

- 🚀 Support for [Gateway API](https://kubernetes.io/docs/concepts/services-networking/gateways/) is now GA!
- You only need to install Gateway API CRDs to use GA features of Gateway API with KIC.
- You only need to install Gateway API CRDs to use GA features of Gateway API with KIC.
- Check the [Ingress to Gateway migration guide] to learn how to start using Gateway API already.
- 🏎️ Performance boosting expression router is now the default for DB-less mode.
- 📈 Gateway Discovery feature is enabled by default both in DB-less and DB mode, allowing for scaling
your gateways independently of the controller.
- 📖 Brand-new docs: [The KIC docs] have been totally revamped to be Gateway API first, and every single guide
is as easy as copying and pasting your way down the page.
- 📖 Brand-new docs: [The KIC docs] have been totally revamped to be Gateway API first, and every single guide
is as easy as copying and pasting your way down the page.

[Ingress to Gateway migration guide]: https://docs.konghq.com/kubernetes-ingress-controller/latest/guides/migrate/ingress-to-gateway/
[The KIC docs]: https://docs.konghq.com/kubernetes-ingress-controller/latest/
Expand Down
3 changes: 3 additions & 0 deletions config/crd/bases/configuration.konghq.com_kongconsumers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ spec:
description: Username is a Kong cluster-unique username of the consumer.
type: string
type: object
x-kubernetes-validations:
- message: Need to provide either username or custom_id
rule: has(self.username) || has(self.custom_id)
served: true
storage: true
subresources:
Expand Down
20 changes: 10 additions & 10 deletions examples/kong-consumer-key-auth.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,3 @@
apiVersion: configuration.konghq.com/v1
kind: KongConsumer
metadata:
name: consumer1
annotations:
kubernetes.io/ingress.class: kong
username: consumer1
credentials:
- consumer1-auth
---
apiVersion: v1
kind: Secret
metadata:
Expand All @@ -18,6 +8,16 @@ type: Opaque
stringData:
key: password
---
apiVersion: configuration.konghq.com/v1
kind: KongConsumer
metadata:
name: consumer1
annotations:
kubernetes.io/ingress.class: kong
# username: consumer1
credentials:
- consumer1-auth
---
apiVersion: apps/v1
kind: Deployment
metadata:
Expand Down
1 change: 0 additions & 1 deletion internal/admission/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const (
ErrTextConsumerGroupUnsupported = "consumer group support requires Kong Enterprise"
ErrTextConsumerGroupUnlicensed = "consumer group support requires a valid Kong Enterprise license"
ErrTextConsumerGroupUnexpected = "unexpected error during checking support for consumer group"
ErrTextConsumerUsernameEmpty = "username cannot be empty"
ErrTextFailedToRetrieveSecret = "could not retrieve secrets from the kubernetes API" //nolint:revive,gosec
ErrTextPluginConfigInvalid = "could not parse plugin configuration"
ErrTextPluginConfigValidationFailed = "unable to validate plugin schema"
Expand Down
5 changes: 0 additions & 5 deletions internal/admission/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,6 @@ func (validator KongHTTPValidator) ValidateConsumer(
return true, "", nil
}

// a consumer without a username is not valid
if consumer.Username == "" {
return false, ErrTextConsumerUsernameEmpty, nil
}

errText, err := validator.ensureConsumerDoesNotExistInGateway(ctx, consumer.Username)
if err != nil || errText != "" {
return false, errText, err
Expand Down
2 changes: 2 additions & 0 deletions internal/dataplane/kongstate/kongstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ func (ks *KongState) FillConsumersAndCredentials(
// build consumer index
for _, consumer := range s.ListKongConsumers() {
var c Consumer
// This is now enforce that the CRD level but we're keeping this just for those
// rare cases where the CRD Validation Expressions are disabled.
if consumer.Username == "" && consumer.CustomID == "" {
failuresCollector.PushResourceFailure("no username or custom_id specified", consumer)
continue
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/configuration/v1/kongconsumer_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
// +kubebuilder:printcolumn:name="Username",type=string,JSONPath=`.username`,description="Username of a Kong Consumer"
// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`,description="Age"
// +kubebuilder:printcolumn:name="Programmed",type=string,JSONPath=`.status.conditions[?(@.type=="Programmed")].status`
// +kubebuilder:validation:XValidation:rule="has(self.username) || has(self.custom_id)", message="Need to provide either username or custom_id"

// KongConsumer is the Schema for the kongconsumers API.
type KongConsumer struct {
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/manifests/all-in-one-dbless-k4k8s-enterprise.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions test/e2e/manifests/all-in-one-dbless-konnect-enterprise.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions test/e2e/manifests/all-in-one-dbless-konnect.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions test/e2e/manifests/all-in-one-dbless.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions test/e2e/manifests/all-in-one-postgres-enterprise.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions test/e2e/manifests/all-in-one-postgres-multiple-gateways.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions test/e2e/manifests/all-in-one-postgres.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 14 additions & 9 deletions test/envtest/kongstate_consumer_failures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ func TestKongStateFillConsumersAndCredentialsFailure(t *testing.T) {
},
},
}
for _, secret := range secrets {
require.NoError(t, client.Create(ctx, secret))
}

kongConsumers := []*kongv1.KongConsumer{
{
Expand All @@ -82,9 +85,16 @@ func TestKongStateFillConsumersAndCredentialsFailure(t *testing.T) {
"empty-cred",
},
},
}
for _, kongConsumer := range kongConsumers {
require.NoError(t, client.Create(ctx, kongConsumer))
}

// These KongConsumers should fail admission via the CRD Validation Expressions.
brokenKongConsumers := []*kongv1.KongConsumer{
{
ObjectMeta: metav1.ObjectMeta{
Name: "consumer-no-username",
Name: "consumer-no-username-and-no-custom-id",
Namespace: ns.Name,
Annotations: map[string]string{annotations.IngressClassKey: annotations.DefaultIngressClass},
},
Expand All @@ -93,18 +103,13 @@ func TestKongStateFillConsumersAndCredentialsFailure(t *testing.T) {
},
},
}

for _, secret := range secrets {
require.NoError(t, client.Create(ctx, secret))
}
for _, kongConsumer := range kongConsumers {
require.NoError(t, client.Create(ctx, kongConsumer))
for _, brokenKongConsumer := range brokenKongConsumers {
require.Error(t, client.Create(ctx, brokenKongConsumer))
}

// KongConsumer name -> event message
kongConsumerTranslationFailureMessages := map[string]string{
"consumer-empty-cred": `credential "empty-cred" failure: failed to provision credential: key-auth is invalid: no key`,
"consumer-no-username": `no username or custom_id specified`,
"consumer-empty-cred": `credential "empty-cred" failure: failed to provision credential: key-auth is invalid: no key`,
}

RunManager(ctx, t, cfg, AdminAPIOptFns(), WithProxySyncSeconds(0.5))
Expand Down
3 changes: 3 additions & 0 deletions test/envtest/programmed_condition_envtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1beta1"
"github.com/kong/kubernetes-ingress-controller/v3/test"
"github.com/kong/kubernetes-ingress-controller/v3/test/helpers"
"github.com/kong/kubernetes-ingress-controller/v3/test/helpers/conditions"
)

Expand All @@ -28,10 +29,12 @@ func TestKongCRDs_ProgrammedCondition(t *testing.T) {

ctrlClient := NewControllerClient(t, scheme, envcfg)
ns := CreateNamespace(ctx, t, ctrlClient)
healthProbePort := helpers.GetFreePort(t)

RunManager(ctx, t, envcfg,
AdminAPIOptFns(),
WithUpdateStatus(),
WithHealthProbePort(healthProbePort),
WithPublishService(ns.Name),
WithPublishStatusAddress("http://localhost:8080"),
)
Expand Down

0 comments on commit 27755d7

Please sign in to comment.