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.

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

feat(crd): enforce setting KongConsumer's either of username of custom_id via CRD validation rules #5137

Merged
merged 1 commit into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
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.

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.

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