From b01456169cbcfabd8684bef9e162fdcee4386196 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Fri, 26 Apr 2024 13:05:01 -0700 Subject: [PATCH] feat(creds) remove kongCredType field support (#5856) Remove support for the kongCredType field in credential Secrets. Only honor the konghq.com/credential label. Add a konghq.com/plugin-config label. This label indicates that a Secret contains plugin configuration and should be validated against its referrers when updated. Add objectSelectors to the Secret blocks in the admission webhook definition. Admission will skip any Secrets without a label indicating they should be used by KIC. Add a "konghq.com/validate" label for other Secrets (currently plugin configuration) that require admission checks. Split the webhook manifest into the generated manifest and additional rule patches. Kubebuilder limitations require writing your own rules to use objectSelectors. Remove the kubebuilder Secret hook generation directive and document the workaround. Refactor the envtest runner to build a webhook manifest via Kustomize, rather than reading a static manifest. --- CHANGELOG.md | 16 ++ config/default/kustomization.yaml | 4 +- config/webhook/additional_secret_hooks.yaml | 58 +++++ config/webhook/kustomization.yaml | 7 + config/webhook/manifests.yaml | 21 -- internal/admission/handler.go | 50 ++-- internal/admission/handler_test.go | 13 + .../consumers/credentials/validation.go | 14 +- .../consumers/credentials/validation_test.go | 15 -- .../validation/consumers/credentials/vars.go | 6 +- internal/admission/validator.go | 7 +- internal/admission/validator_test.go | 18 +- internal/dataplane/kongstate/kongstate.go | 12 +- .../dataplane/kongstate/kongstate_test.go | 72 ++---- internal/labels/labels.go | 15 ++ internal/util/credential.go | 31 +-- internal/util/credential_test.go | 56 +---- internal/util/types_test.go | 9 +- test/envtest/admission_webhook.go | 8 +- .../envtest/admission_webhook_envtest_test.go | 234 ++++++++++++++---- .../kongstate_consumer_failures_test.go | 16 +- test/integration/consumer_group_test.go | 7 +- test/integration/consumer_test.go | 9 +- 23 files changed, 427 insertions(+), 271 deletions(-) create mode 100644 config/webhook/additional_secret_hooks.yaml create mode 100644 config/webhook/kustomization.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ccbe19a27..75f90976b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,22 @@ Adding a new version? You'll need three changes: ## Unreleased +### Breaking changes + +- Removed support for the deprecated `kongCredType` Secret field. If you have + not previously [updated to the credential label](https://docs.konghq.com/kubernetes-ingress-controller/latest/guides/migrate/credential-kongcredtype-label/) + you must do so before upgrading to this version. This removal includes an + update to the webhook configuration that checks only Secrets with + `konghq.com/credential` or `konghq.com/validate` labels (for Secrets that + contain plugin configuration). This filter improves performance and + reliability by not checking Secrets the controller will never use. Users that + wish to defer adding `konghq.com/validate` to Secrets with plugin + configuration can set the `ingressController.admissionWebhook.filterSecrets` + chart values.yaml key to `false`. Doing so does not benefit from the + performance benefits, however, so labeling plugin configuration Secrets and + enabling the filter is recommended as soon as is convenient. + [#5856](https://github.com/Kong/kubernetes-ingress-controller/pull/5856) + ### Added - Add a `/debug/config/raw-error` endpoint to the config dump diagnostic diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 6452992c18..8de9df532e 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -18,7 +18,9 @@ bases: - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml -#- ../webhook +# NOTE we enable this to allow patching the generated webhook. We do not have the crd/kustomization.yaml config +# enabled, as we don't use the mutating hooks in there currently. +- ../webhook # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. #- ../certmanager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. diff --git a/config/webhook/additional_secret_hooks.yaml b/config/webhook/additional_secret_hooks.yaml new file mode 100644 index 0000000000..48dc01d727 --- /dev/null +++ b/config/webhook/additional_secret_hooks.yaml @@ -0,0 +1,58 @@ +# https://github.com/kubernetes-sigs/controller-tools/issues/553 +# controller-tools, and by extension kubebuilder, do not support specifying objectSelector, +# which we need for the Secret rules. +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: validating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: / + failurePolicy: Fail + matchPolicy: Equivalent + name: secrets.credentials.validation.ingress-controller.konghq.com + objectSelector: + matchExpressions: + - key: "konghq.com/credential" + operator: "Exists" + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - secrets + sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: / + failurePolicy: Fail + matchPolicy: Equivalent + name: secrets.plugins.validation.ingress-controller.konghq.com + objectSelector: + matchExpressions: + - key: "konghq.com/validate" + operator: "Exists" + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - secrets + sideEffects: None diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml new file mode 100644 index 0000000000..b45651127d --- /dev/null +++ b/config/webhook/kustomization.yaml @@ -0,0 +1,7 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- manifests.yaml + +patchesStrategicMerge: +- additional_secret_hooks.yaml diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 525a5350c4..3281b20661 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -195,27 +195,6 @@ webhooks: resources: - kongvaults sideEffects: None -- admissionReviewVersions: - - v1 - clientConfig: - service: - name: webhook-service - namespace: system - path: / - failurePolicy: Fail - matchPolicy: Equivalent - name: secrets.validation.ingress-controller.konghq.com - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - resources: - - secrets - sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/internal/admission/handler.go b/internal/admission/handler.go index 518f323f35..9273f186a7 100644 --- a/internal/admission/handler.go +++ b/internal/admission/handler.go @@ -15,6 +15,7 @@ import ( "github.com/kong/kubernetes-ingress-controller/v3/internal/annotations" ctrlref "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/reference" "github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi" + "github.com/kong/kubernetes-ingress-controller/v3/internal/labels" "github.com/kong/kubernetes-ingress-controller/v3/internal/util" kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1" kongv1alpha1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1alpha1" @@ -256,7 +257,10 @@ func (h RequestHandler) handleKongClusterPlugin( return responseBuilder.Allowed(ok).WithMessage(message).Build(), nil } -// +kubebuilder:webhook:verbs=create;update,groups=core,resources=secrets,versions=v1,name=secrets.validation.ingress-controller.konghq.com,path=/,webhookVersions=v1,matchPolicy=equivalent,mutating=false,failurePolicy=fail,sideEffects=None,admissionReviewVersions=v1 +// NOTE this handler _does not_ use a kubebuilder directive, as our Secret handling requires webhook features +// kubebuilder does not support (objectSelector). Instead, config/webhook/additional_secret_hooks.yaml includes +// handwritten webhook rules that we patch into the webhook manifest. +// See https://github.com/kubernetes-sigs/controller-tools/issues/553 for further info. func (h RequestHandler) handleSecret( ctx context.Context, @@ -271,17 +275,28 @@ func (h RequestHandler) handleSecret( switch request.Operation { case admissionv1.Update, admissionv1.Create: - // TODO so long as we still handle the deprecated field, this has to remain - // once the deprecated field is removed, we must replace this with a label filter in the webhook itself - // https://github.com/Kong/kubernetes-ingress-controller/issues/4853 - if _, credentialTypeSource := util.ExtractKongCredentialType(&secret); credentialTypeSource != util.CredentialTypeAbsent { + // credential secrets + if credType, err := util.ExtractKongCredentialType(&secret); err == nil && credType != "" { ok, message := h.Validator.ValidateCredential(ctx, secret) if !ok { return responseBuilder.Allowed(ok).WithMessage(message).Build(), nil } } - ok, message, err := h.checkReferrersOfSecret(ctx, &secret) + // TODO https://github.com/Kong/kubernetes-ingress-controller/issues/5876 + // This catch-all block handles Secrets referenced by KongPlugin and KongClusterPlugin configuration. As of 3.2, + // these Secrets should use a "konghq.com/validate: plugin" label, but the original unfiltered behavior is still + // supported. It is slated for removal in 4.0. Once it is removed (or if we add additional Secret validation cases + // other than "plugin") this needs to change to a case that only applies if the valdiate label is present with the + // "plugin" value, probably using a 'switch validate := secret.Labels[labels.ValidateLabel]; labels.ValidateType(validate)' + // statement. + ok, count, message, err := h.checkReferrersOfSecret(ctx, &secret) + if count > 0 { + if secret.Labels[labels.ValidateLabel] != string(labels.PluginValidate) { + h.Logger.Info("Warning: Secret used in Kong(Cluster)Plugin, but missing 'konghq.com/validate: plugin' label."+ + "This label will be required in a future release", "namespace", secret.Namespace, "name", secret.Name) + } + } if err != nil { return responseBuilder.Allowed(false).WithMessage(fmt.Sprintf("failed to validate other objects referencing the secret: %v", err)).Build(), err } @@ -289,6 +304,7 @@ func (h RequestHandler) handleSecret( return responseBuilder.Allowed(false).WithMessage(message).Build(), nil } + // no reference found in the blanket block, this is some random unrelated Secret and KIC should ignore it. return responseBuilder.Allowed(true).Build(), nil default: @@ -298,45 +314,49 @@ func (h RequestHandler) handleSecret( // checkReferrersOfSecret validates all referrers (KongPlugins and KongClusterPlugins) of the secret // and rejects the secret if it generates invalid configurations for any of the referrers. -func (h RequestHandler) checkReferrersOfSecret(ctx context.Context, secret *corev1.Secret) (bool, string, error) { +func (h RequestHandler) checkReferrersOfSecret(ctx context.Context, secret *corev1.Secret) (bool, int, string, error) { referrers, err := h.ReferenceIndexers.ListReferrerObjectsByReferent(secret) if err != nil { - return false, "", fmt.Errorf("failed to list referrers of secret: %w", err) + return false, 0, "", fmt.Errorf("failed to list referrers of secret: %w", err) } + count := 0 for _, obj := range referrers { gvk := obj.GetObjectKind().GroupVersionKind() if gvk.Group == kongv1.GroupVersion.Group && gvk.Version == kongv1.GroupVersion.Version && gvk.Kind == KindKongPlugin { + count++ plugin := obj.(*kongv1.KongPlugin) ok, message, err := h.Validator.ValidatePlugin(ctx, *plugin, []*corev1.Secret{secret}) if err != nil { - return false, "", fmt.Errorf("failed to run validation on KongPlugin %s/%s: %w", + return false, count, "", fmt.Errorf("failed to run validation on KongPlugin %s/%s: %w", plugin.Namespace, plugin.Name, err, ) } if !ok { - return false, + return false, count, fmt.Sprintf("Change on secret will generate invalid configuration for KongPlugin %s/%s: %s", plugin.Namespace, plugin.Name, message, ), nil } } if gvk.Group == kongv1.GroupVersion.Group && gvk.Version == kongv1.GroupVersion.Version && gvk.Kind == KindKongClusterPlugin { + count++ plugin := obj.(*kongv1.KongClusterPlugin) ok, message, err := h.Validator.ValidateClusterPlugin(ctx, *plugin, []*corev1.Secret{secret}) if err != nil { - return false, "", fmt.Errorf("failed to run validation on KongClusterPlugin %s: %w", + return false, count, "", fmt.Errorf("failed to run validation on KongClusterPlugin %s: %w", plugin.Name, err, ) } if !ok { - return false, fmt.Sprintf("Change on secret will generate invalid configuration for KongClusterPlugin %s: %s", - plugin.Name, message, - ), nil + return false, count, + fmt.Sprintf("Change on secret will generate invalid configuration for KongClusterPlugin %s: %s", + plugin.Name, message, + ), nil } } } - return true, "", nil + return true, count, "", nil } // +kubebuilder:webhook:verbs=create;update,groups=gateway.networking.k8s.io,resources=gateways,versions=v1;v1beta1,name=gateways.validation.ingress-controller.konghq.com,path=/,webhookVersions=v1,matchPolicy=equivalent,mutating=false,failurePolicy=fail,sideEffects=None,admissionReviewVersions=v1 diff --git a/internal/admission/handler_test.go b/internal/admission/handler_test.go index 83caf9478b..6c28955c0d 100644 --- a/internal/admission/handler_test.go +++ b/internal/admission/handler_test.go @@ -19,6 +19,7 @@ import ( "github.com/kong/kubernetes-ingress-controller/v3/internal/annotations" ctrlref "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/reference" + "github.com/kong/kubernetes-ingress-controller/v3/internal/labels" "github.com/kong/kubernetes-ingress-controller/v3/internal/util" kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1" kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1beta1" @@ -220,6 +221,9 @@ func TestHandleSecret(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Namespace: "default", Name: "plugin-conf", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, }, referrers: []client.Object{ @@ -235,6 +239,9 @@ func TestHandleSecret(t *testing.T) { TypeMeta: kongClusterPluginTypeMeta, ObjectMeta: metav1.ObjectMeta{ Name: "cluster-plugin-0", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, PluginName: "test-plugin", }, @@ -249,6 +256,9 @@ func TestHandleSecret(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Namespace: "default", Name: "plugin-conf", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, }, referrers: []client.Object{ @@ -274,6 +284,9 @@ func TestHandleSecret(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Namespace: "default", Name: "plugin-conf", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, }, referrers: []client.Object{ diff --git a/internal/admission/validation/consumers/credentials/validation.go b/internal/admission/validation/consumers/credentials/validation.go index b3ce4a488f..ae921ad105 100644 --- a/internal/admission/validation/consumers/credentials/validation.go +++ b/internal/admission/validation/consumers/credentials/validation.go @@ -19,9 +19,8 @@ import ( // ValidateCredentials performs basic validation on a credential secret given // the Kubernetes secret which contains credentials data. func ValidateCredentials(secret *corev1.Secret) error { - credentialType, credentialSource := util.ExtractKongCredentialType(secret) - - if credentialSource == util.CredentialTypeAbsent { + credentialType, err := util.ExtractKongCredentialType(secret) + if err != nil { // this shouldn't occur, since we check this earlier in the admission controller's handleSecret function, but // checking here also in case a refactor removes that return fmt.Errorf("secret has no credential type, add a %s label", labels.CredentialTypeLabel) @@ -123,12 +122,9 @@ type Index map[string]map[string]map[string]struct{} // and will validate it for both normal structure validation and for // unique key constraint violations. func (cs Index) ValidateCredentialsForUniqueKeyConstraints(secret *corev1.Secret) error { - credentialType, credentialSource := util.ExtractKongCredentialType(secret) - if credentialSource == util.CredentialTypeAbsent { - return fmt.Errorf( - "secret has no credential type, add a %s label", - labels.CredentialTypeLabel, - ) + credentialType, err := util.ExtractKongCredentialType(secret) + if err != nil { + return fmt.Errorf("secret has no credential type, add a %s label", labels.CredentialTypeLabel) } // the additional key/values are optional, but must be validated diff --git a/internal/admission/validation/consumers/credentials/validation_test.go b/internal/admission/validation/consumers/credentials/validation_test.go index 5020f4f242..466ebf5587 100644 --- a/internal/admission/validation/consumers/credentials/validation_test.go +++ b/internal/admission/validation/consumers/credentials/validation_test.go @@ -176,21 +176,6 @@ func TestValidateCredentials(t *testing.T) { }, wantErr: fmt.Errorf("some fields were invalid due to missing data: rsa_public_key, key, secret"), }, - { - // TODO https://github.com/Kong/kubernetes-ingress-controller/issues/4853 to be removed after deprecation window - name: "valid credential with deprectated field", - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "secret", - Namespace: "default", - }, - Data: map[string][]byte{ - "key": []byte("little-rabbits-be-good"), - "kongCredType": []byte("key-auth"), - }, - }, - wantErr: nil, - }, { name: "invalid credential type", secret: &corev1.Secret{ diff --git a/internal/admission/validation/consumers/credentials/vars.go b/internal/admission/validation/consumers/credentials/vars.go index 624c1c2e44..2adc4d9f13 100644 --- a/internal/admission/validation/consumers/credentials/vars.go +++ b/internal/admission/validation/consumers/credentials/vars.go @@ -6,11 +6,7 @@ import "k8s.io/apimachinery/pkg/util/sets" // Validation - Vars // ----------------------------------------------------------------------------- -// TypeKey indicates the key in a consumer secret which identifies the type -// of credential that is being provided for the consumer. -const TypeKey = "kongCredType" - -// SupportedTypes indicates all the "kongCredType"s which are supported for KongConsumer credentials. +// SupportedTypes indicates all the Kong credential types which are supported for KongConsumer credentials. var SupportedTypes = sets.NewString( "basic-auth", "hmac-auth", diff --git a/internal/admission/validator.go b/internal/admission/validator.go index 965e1ee367..5bb3bbc03c 100644 --- a/internal/admission/validator.go +++ b/internal/admission/validator.go @@ -272,8 +272,11 @@ func (validator KongHTTPValidator) ValidateConsumerGroup( // else it returns false with the error message. If an error happens during // validation, error is returned. func (validator KongHTTPValidator) ValidateCredential(ctx context.Context, secret corev1.Secret) (bool, string) { - // If the secret doesn't specify a credential type (either by label or the secret's key) it's not a credentials secret. - if _, s := util.ExtractKongCredentialType(&secret); s == util.CredentialTypeAbsent { + // If the secret doesn't specify a credential type it's not a credentials secret. We shouldn't actually reach this + // codepath in practice because such secrets will be filtered out by the webhook secrets objectSelector and ignored. + // However, installs could potentially use an outdated webhook definition. Prior to 3.2 we only filtered in code and + // used a blanket selector. + if _, err := util.ExtractKongCredentialType(&secret); err != nil { return true, "" } diff --git a/internal/admission/validator_test.go b/internal/admission/validator_test.go index 73dab19135..56499efcae 100644 --- a/internal/admission/validator_test.go +++ b/internal/admission/validator_test.go @@ -754,9 +754,15 @@ func TestKongHTTPValidator_ValidateCredential(t *testing.T) { { name: "valid key-auth credential with no consumers gets accepted", secret: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "credential-0", + Labels: map[string]string{ + "konghq.com/credential": "key-auth", + }, + }, Data: map[string][]byte{ - "kongCredType": []byte("key-auth"), - "key": []byte("my-key"), + "key": []byte("my-key"), }, }, wantOK: true, @@ -787,8 +793,14 @@ func TestKongHTTPValidator_ValidateCredential(t *testing.T) { { name: "invalid key-auth credential with no consumers gets rejected", secret: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "credential-0", + Labels: map[string]string{ + "konghq.com/credential": "key-auth", + }, + }, Data: map[string][]byte{ - "kongCredType": []byte("key-auth"), // missing key }, }, diff --git a/internal/dataplane/kongstate/kongstate.go b/internal/dataplane/kongstate/kongstate.go index a25c46b31e..c30c9c1fec 100644 --- a/internal/dataplane/kongstate/kongstate.go +++ b/internal/dataplane/kongstate/kongstate.go @@ -68,7 +68,7 @@ func (ks *KongState) SanitizedCopy() *KongState { } func (ks *KongState) FillConsumersAndCredentials( - logger logr.Logger, + _ logr.Logger, s store.Storer, failuresCollector *failures.ResourceFailuresCollector, ) { @@ -115,11 +115,9 @@ func (ks *KongState) FillConsumersAndCredentials( } credConfig := map[string]interface{}{} // try the label first. if it's present, no need to check the field - credType, credTypeSource := util.ExtractKongCredentialType(secret) - if credTypeSource == util.CredentialTypeFromField { - logger.Error(nil, - fmt.Sprintf("Secret uses deprecated kongCredType field, needs konghq.com/credential=%s label", credType), - "namesapce", secret.Namespace, "name", secret.Name) + credType, err := util.ExtractKongCredentialType(secret) + if err != nil { + pushCredentialResourceFailures(fmt.Sprintf("could not load credential from Secret: %s", err)) } if !credentials.SupportedTypes.Has(credType) { pushCredentialResourceFailures( @@ -134,7 +132,7 @@ func (ks *KongState) FillConsumersAndCredentials( credConfig[k] = strings.Split(string(v), ",") continue } - // TODO this is a kongCredType-agnostic mutation that should only apply to Oauth2 credentials. + // TODO this is a credential type-agnostic mutation that should only apply to Oauth2 credentials. // However, the credential-specific code after deals only in interface{}s, and we can't fix individual // keys. To handle this properly we'd need to refactor the types used in all following code. if k == "hash_secret" { diff --git a/internal/dataplane/kongstate/kongstate_test.go b/internal/dataplane/kongstate/kongstate_test.go index 9961c6fc1d..522707a549 100644 --- a/internal/dataplane/kongstate/kongstate_test.go +++ b/internal/dataplane/kongstate/kongstate_test.go @@ -470,20 +470,24 @@ func TestFillConsumersAndCredentials(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "fooCredSecret", Namespace: "default", + Labels: map[string]string{ + labels.CredentialTypeLabel: "key-auth", + }, }, Data: map[string][]byte{ - "kongCredType": []byte("key-auth"), - "key": []byte("whatever"), - "ttl": []byte("1024"), + "key": []byte("whatever"), + "ttl": []byte("1024"), }, }, { ObjectMeta: metav1.ObjectMeta{ Name: "barCredSecret", Namespace: "default", + Labels: map[string]string{ + labels.CredentialTypeLabel: "oauth2", + }, }, Data: map[string][]byte{ - "kongCredType": []byte("oauth2"), "name": []byte("whatever"), "client_id": []byte("whatever"), "client_secret": []byte("whatever"), @@ -495,44 +499,34 @@ func TestFillConsumersAndCredentials(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "emptyCredSecret", Namespace: "default", + Labels: map[string]string{ + labels.CredentialTypeLabel: "key-auth", + }, }, - Data: map[string][]byte{ - "kongCredType": []byte("key-auth"), - }, + Data: map[string][]byte{}, }, { ObjectMeta: metav1.ObjectMeta{ Name: "unsupportedCredSecret", Namespace: "default", - }, - Data: map[string][]byte{ - "kongCredType": []byte("unsupported"), - "foo": []byte("bar"), - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "labeledSecret", - Namespace: "default", Labels: map[string]string{ - labels.CredentialTypeLabel: "key-auth", + labels.CredentialTypeLabel: "unsupported", }, }, Data: map[string][]byte{ - "key": []byte("little-rabbits-be-good"), + "foo": []byte("bar"), }, }, { ObjectMeta: metav1.ObjectMeta{ - Name: "labeledSecretWithCredField", + Name: "labeledSecret", Namespace: "default", Labels: map[string]string{ labels.CredentialTypeLabel: "key-auth", }, }, Data: map[string][]byte{ - "kongCredType": []byte("key-auth"), - "key": []byte("little-rabbits-be-good"), + "key": []byte("little-rabbits-be-good"), }, }, { @@ -748,40 +742,6 @@ func TestFillConsumersAndCredentials(t *testing.T) { }, }, }, - { - name: "KongConsumer with key-auth from label secret with the old cred field", - k8sConsumers: []*kongv1.KongConsumer{ - { - TypeMeta: kongConsumerTypeMeta, - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "default", - Annotations: map[string]string{ - "kubernetes.io/ingress.class": annotations.DefaultIngressClass, - }, - }, - Username: "foo", - CustomID: "foo", - Credentials: []string{ - "labeledSecretWithCredField", - }, - }, - }, - expectedKongStateConsumers: []Consumer{ - { - Consumer: kong.Consumer{ - Username: kong.String("foo"), - CustomID: kong.String("foo"), - }, - KeyAuths: []*KeyAuth{{kong.KeyAuth{ - Key: kong.String("little-rabbits-be-good"), - Tags: util.GenerateTagsForObject(&corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "labeledSecretWithCredField"}, - }), - }}}, - }, - }, - }, } for i, tc := range testCases { diff --git a/internal/labels/labels.go b/internal/labels/labels.go index 98981d7f34..7f8a341c4c 100644 --- a/internal/labels/labels.go +++ b/internal/labels/labels.go @@ -9,6 +9,21 @@ const ( // CredentialKey is the key used to indicate a Secret's credential type. CredentialKey = "/credential" //nolint:gosec + // ValidateKey is the key used to indicate a Secret contains plugin configuration. + ValidateKey = "/validate" + // CredentialTypeLabel is the label used to indicate a Secret's credential type. CredentialTypeLabel = LabelPrefix + CredentialKey + + // ValidateLabel is applied to plugins used for plugin configuration to allow the admission webhook to check + // updates to them. + ValidateLabel = LabelPrefix + ValidateKey +) + +// ValidateType indicates the type of validation applied to a Secret. +type ValidateType string + +const ( + // PluginValidate indicates a labeled Secret's contents require plugin configuration validation. + PluginValidate ValidateType = "plugin" ) diff --git a/internal/util/credential.go b/internal/util/credential.go index 1f7ad78046..9b53ecc4cb 100644 --- a/internal/util/credential.go +++ b/internal/util/credential.go @@ -1,36 +1,19 @@ package util import ( + "fmt" + corev1 "k8s.io/api/core/v1" "github.com/kong/kubernetes-ingress-controller/v3/internal/labels" ) -// TODO https://github.com/Kong/kubernetes-ingress-controller/issues/4853 remove field handling when no longer supported. - -// CredentialTypeSource indicates the source of credential type information (or lack thereof) in a Secret. -type CredentialTypeSource int - -const ( - // CredentialTypeAbsent indicates that no credential information is present in a Secret. - CredentialTypeAbsent CredentialTypeSource = iota - // CredentialTypeFromLabel indicates that a Secret's credential type was determined from a label. - CredentialTypeFromLabel - // CredentialTypeFromField indicates that a Secret's credential type was determined from a data field. - CredentialTypeFromField -) - -// ExtractKongCredentialType returns the credential type of a Secret and a code indicating whether the credential type -// was obtained from a label, field, or not at all. Labels take precedence over fields if both are present. -func ExtractKongCredentialType(secret *corev1.Secret) (string, CredentialTypeSource) { +// ExtractKongCredentialType returns the credential type of a Secret or an error if no credential type is present. +func ExtractKongCredentialType(secret *corev1.Secret) (string, error) { credType, labelOk := secret.Labels[labels.CredentialTypeLabel] if !labelOk { - // if no label, fall back to the deprecated field - credBytes, fieldOk := secret.Data["kongCredType"] - if !fieldOk { - return "", CredentialTypeAbsent - } - return string(credBytes), CredentialTypeFromField + return "", fmt.Errorf("Secret %s/%s used as credential, but lacks %s label", + secret.Namespace, secret.Name, labels.CredentialTypeLabel) } - return credType, CredentialTypeFromLabel + return credType, nil } diff --git a/internal/util/credential_test.go b/internal/util/credential_test.go index e1337f8ab2..c44245b2d4 100644 --- a/internal/util/credential_test.go +++ b/internal/util/credential_test.go @@ -1,6 +1,7 @@ package util import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -12,10 +13,10 @@ import ( func TestExtractKongCredentialType(t *testing.T) { tests := []struct { - name string - secret *corev1.Secret - credType string - credTypeSource CredentialTypeSource + name string + secret *corev1.Secret + credType string + wantErr error }{ { name: "labeled credential", @@ -31,43 +32,7 @@ func TestExtractKongCredentialType(t *testing.T) { "key": []byte("little-rabbits-be-good"), }, }, - credType: "key-auth", - credTypeSource: CredentialTypeFromLabel, - }, - { - // TODO https://github.com/Kong/kubernetes-ingress-controller/issues/4853 to be removed after deprecation window - name: "kongCredType field credential", - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "secret", - Namespace: "default", - }, - Data: map[string][]byte{ - "key": []byte("little-rabbits-be-good"), - "kongCredType": []byte("key-auth"), - }, - }, - credType: "key-auth", - credTypeSource: CredentialTypeFromField, - }, - { - // TODO https://github.com/Kong/kubernetes-ingress-controller/issues/4853 to be removed after deprecation window - name: "kongCredType field and labeled credential, label takes precedence", - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "secret", - Namespace: "default", - Labels: map[string]string{ - labels.CredentialTypeLabel: "key-auth", - }, - }, - Data: map[string][]byte{ - "key": []byte("little-rabbits-be-good"), - "kongCredType": []byte("bee-auth"), - }, - }, - credType: "key-auth", - credTypeSource: CredentialTypeFromLabel, + credType: "key-auth", }, { name: "no credential type", @@ -80,15 +45,16 @@ func TestExtractKongCredentialType(t *testing.T) { "key": []byte("little-rabbits-be-good"), }, }, - credType: "", - credTypeSource: CredentialTypeAbsent, + credType: "", + wantErr: fmt.Errorf("Secret %s/%s used as credential, but lacks %s label", + "default", "secret", labels.CredentialTypeLabel), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - credType, credTypeSource := ExtractKongCredentialType(tt.secret) + credType, err := ExtractKongCredentialType(tt.secret) require.Equal(t, tt.credType, credType) - require.Equal(t, tt.credTypeSource, credTypeSource) + require.Equal(t, tt.wantErr, err) }) } } diff --git a/internal/util/types_test.go b/internal/util/types_test.go index ea38283a16..68c20cf7e7 100644 --- a/internal/util/types_test.go +++ b/internal/util/types_test.go @@ -8,6 +8,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/kong/kubernetes-ingress-controller/v3/internal/labels" "github.com/kong/kubernetes-ingress-controller/v3/internal/manager/scheme" ) @@ -15,11 +16,13 @@ func TestPopulateTypeMeta(t *testing.T) { credential := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "corn", + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": "corn", - "password": "corn", + "username": "corn", + "password": "corn", }, } diff --git a/test/envtest/admission_webhook.go b/test/envtest/admission_webhook.go index d468b96e3a..9a99df903b 100644 --- a/test/envtest/admission_webhook.go +++ b/test/envtest/admission_webhook.go @@ -4,9 +4,9 @@ import ( "bytes" "context" "fmt" - "os" "testing" + "github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/kubectl" "github.com/samber/lo" "github.com/stretchr/testify/require" admregv1 "k8s.io/api/admissionregistration/v1" @@ -29,13 +29,13 @@ func setupValidatingWebhookConfiguration( } func validatingWebhookConfigWithClientConfig(t *testing.T, clientConfig admregv1.WebhookClientConfig) *admregv1.ValidatingWebhookConfiguration { - file, err := os.ReadFile("../../config/webhook/manifests.yaml") + manifest, err := kubectl.RunKustomize("../../config/webhook/") require.NoError(t, err) - file = bytes.ReplaceAll(file, []byte("---"), []byte("")) // We're only expecting one document in the file. + manifest = bytes.ReplaceAll(manifest, []byte("---"), []byte("")) // We're only expecting one document in the file. // Load the webhook configuration from the generated manifest. webhookConfig := &admregv1.ValidatingWebhookConfiguration{} - require.NoError(t, yaml.Unmarshal(file, webhookConfig)) + require.NoError(t, yaml.Unmarshal(manifest, webhookConfig)) // Set the client config. for i := range webhookConfig.Webhooks { diff --git a/test/envtest/admission_webhook_envtest_test.go b/test/envtest/admission_webhook_envtest_test.go index d3602c71b8..fc86115f3c 100644 --- a/test/envtest/admission_webhook_envtest_test.go +++ b/test/envtest/admission_webhook_envtest_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" + admregv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -21,6 +22,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kong/kubernetes-ingress-controller/v3/internal/annotations" + "github.com/kong/kubernetes-ingress-controller/v3/internal/labels" kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1" kongv1alpha1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1alpha1" "github.com/kong/kubernetes-ingress-controller/v3/test/helpers" @@ -213,6 +215,9 @@ func TestAdmissionWebhook_KongPlugins(t *testing.T) { kongPlugin: &kongv1.KongPlugin{ ObjectMeta: metav1.ObjectMeta{ Name: "rate-limiting-invalid-config-from", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, PluginName: "rate-limiting", ConfigFrom: &kongv1.ConfigSource{ @@ -225,6 +230,9 @@ func TestAdmissionWebhook_KongPlugins(t *testing.T) { secretBefore: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "conf-secret-invalid-config", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-config": []byte(`{"limit_by":"consumer","policy":"local","minute":5}`), @@ -233,6 +241,9 @@ func TestAdmissionWebhook_KongPlugins(t *testing.T) { secretAfter: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "conf-secret-invalid-config", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-config": []byte(`{"limit_by":"consumer","policy":"local","minute":"5"}`), @@ -266,6 +277,9 @@ func TestAdmissionWebhook_KongPlugins(t *testing.T) { secretBefore: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "conf-secret-invalid-field", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-config-minutes": []byte("10"), @@ -274,6 +288,9 @@ func TestAdmissionWebhook_KongPlugins(t *testing.T) { secretAfter: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "conf-secret-invalid-field", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-config-minutes": []byte(`"10"`), @@ -287,6 +304,9 @@ func TestAdmissionWebhook_KongPlugins(t *testing.T) { kongPlugin: &kongv1.KongPlugin{ ObjectMeta: metav1.ObjectMeta{ Name: "rate-limiting-valid-config", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, PluginName: "rate-limiting", Config: apiextensionsv1.JSON{ @@ -307,6 +327,9 @@ func TestAdmissionWebhook_KongPlugins(t *testing.T) { secretBefore: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "conf-secret-valid-field", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-config-minutes": []byte(`10`), @@ -315,6 +338,9 @@ func TestAdmissionWebhook_KongPlugins(t *testing.T) { secretAfter: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "conf-secret-valid-field", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-config-minutes": []byte(`15`), @@ -325,15 +351,69 @@ func TestAdmissionWebhook_KongPlugins(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - require.NoError(t, ctrlClient.Create(ctx, tc.secretBefore)) t.Cleanup(func() { - require.NoError(t, ctrlClient.Delete(ctx, tc.secretBefore)) + require.NoError(t, client.IgnoreNotFound(ctrlClient.Delete(ctx, tc.secretBefore))) + require.NoError(t, client.IgnoreNotFound(ctrlClient.Delete(ctx, tc.secretAfter))) + require.NoError(t, client.IgnoreNotFound(ctrlClient.Delete(ctx, tc.kongPlugin))) }) + require.NoError(t, ctrlClient.Create(ctx, tc.secretBefore)) require.NoError(t, ctrlClient.Create(ctx, tc.kongPlugin)) + + require.EventuallyWithT(t, func(c *assert.CollectT) { + err := ctrlClient.Update(ctx, tc.secretAfter) + if tc.errorOnUpdate { + if !assert.Error(c, err) { + return + } + assert.Contains(c, err.Error(), tc.expectErrorContains) + } else if !assert.NoError(c, err) { + t.Logf("Error: %v", err) + } + }, 10*time.Second, 100*time.Millisecond) + }) + } + + // TODO https://github.com/Kong/kubernetes-ingress-controller/issues/5876 + // This repeats all test cases without filtering Secrets in the webhook configuration. This behavior is slated + // for removal in 4.0, and the following block should be removed along with the behavior. + webhookConfig := validatingWebhookConfigWithClientConfig(t, admregv1.WebhookClientConfig{ + URL: lo.ToPtr(fmt.Sprintf("https://localhost:%d/", admissionWebhookPort)), + CABundle: webhookCert, + }) + // Update requires an object with generated fields populated, so we Get() after using the builder. The builder just + // ensures the name and namespace match the original. + require.NoError(t, ctrlClient.Get( + ctx, + k8stypes.NamespacedName{Name: webhookConfig.Name, Namespace: webhookConfig.Namespace}, + webhookConfig, + &client.GetOptions{}, + )) + for i, hook := range webhookConfig.Webhooks { + if hook.Name == "secrets.plugins.validation.ingress-controller.konghq.com" { + webhookConfig.Webhooks[i].ObjectSelector = nil + } + } + require.NoError(t, ctrlClient.Update(ctx, webhookConfig)) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Annoyingly, Create forcibly stores the resulting object in the tc field object we want to reuse. + // Clearing it manually is a bit silly, but works to let us reuse them. + tc.secretBefore.ResourceVersion = "" + tc.secretBefore.UID = "" + tc.secretAfter.ResourceVersion = "" + tc.secretAfter.UID = "" + tc.kongPlugin.ResourceVersion = "" + tc.kongPlugin.UID = "" t.Cleanup(func() { - require.NoError(t, ctrlClient.Delete(ctx, tc.kongPlugin)) + require.NoError(t, client.IgnoreNotFound(ctrlClient.Delete(ctx, tc.secretBefore))) + require.NoError(t, client.IgnoreNotFound(ctrlClient.Delete(ctx, tc.secretAfter))) + require.NoError(t, client.IgnoreNotFound(ctrlClient.Delete(ctx, tc.kongPlugin))) }) + require.NoError(t, ctrlClient.Create(ctx, tc.secretBefore)) + + require.NoError(t, ctrlClient.Create(ctx, tc.kongPlugin)) require.EventuallyWithT(t, func(c *assert.CollectT) { err := ctrlClient.Update(ctx, tc.secretAfter) @@ -408,6 +488,9 @@ func TestAdmissionWebhook_KongClusterPlugins(t *testing.T) { secretBefore: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster-conf-secret-valid", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-config": []byte(`{"limit_by":"consumer","policy":"local","minute":5}`), @@ -416,6 +499,9 @@ func TestAdmissionWebhook_KongClusterPlugins(t *testing.T) { secretAfter: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster-conf-secret-valid", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-config": []byte(`{"limit_by":"consumer","policy":"local","minute":10}`), @@ -444,6 +530,9 @@ func TestAdmissionWebhook_KongClusterPlugins(t *testing.T) { secretBefore: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster-conf-secret-invalid", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-config": []byte(`{"limit_by":"consumer","policy":"local","minute":5}`), @@ -452,6 +541,9 @@ func TestAdmissionWebhook_KongClusterPlugins(t *testing.T) { secretAfter: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster-conf-secret-invalid", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-config": []byte(`{"limit_by":"consumer","policy":"local","minute":"5"}`), @@ -489,6 +581,9 @@ func TestAdmissionWebhook_KongClusterPlugins(t *testing.T) { secretBefore: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster-conf-secret-valid-patch", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-minute": []byte(`5`), @@ -497,6 +592,9 @@ func TestAdmissionWebhook_KongClusterPlugins(t *testing.T) { secretAfter: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster-conf-secret-valid-patch", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-minute": []byte(`10`), @@ -533,6 +631,9 @@ func TestAdmissionWebhook_KongClusterPlugins(t *testing.T) { secretBefore: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster-conf-secret-invalid-patch", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-minute": []byte(`5`), @@ -541,6 +642,9 @@ func TestAdmissionWebhook_KongClusterPlugins(t *testing.T) { secretAfter: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster-conf-secret-invalid-patch", + Labels: map[string]string{ + labels.ValidateLabel: "plugin", + }, }, Data: map[string][]byte{ "rate-limiting-minute": []byte(`"10"`), @@ -611,21 +715,25 @@ func TestAdmissionWebhook_KongConsumers(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{ Name: "tuxcreds1", + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": "tux1", - "password": "testpass", + "username": "tux1", + "password": "testpass", }, }, { ObjectMeta: metav1.ObjectMeta{ Name: "tuxcreds2", + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": "tux2", - "password": "testpass", + "username": "tux2", + "password": "testpass", }, }, } { @@ -698,11 +806,13 @@ func TestAdmissionWebhook_KongConsumers(t *testing.T) { credentials: []*corev1.Secret{{ ObjectMeta: metav1.ObjectMeta{ Name: "electronscreds", + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": "electron", - "password": "testpass", + "username": "electron", + "password": "testpass", }, }}, wantErr: false, @@ -727,21 +837,25 @@ func TestAdmissionWebhook_KongConsumers(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{ Name: "protonscreds1", + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": "proton", - "password": "testpass", + "username": "proton", + "password": "testpass", }, }, { ObjectMeta: metav1.ObjectMeta{ Name: "protonscreds2", + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": "electron", // username is unique constrained - "password": "testpass", // password is not unique constrained + "username": "electron", // username is unique constrained + "password": "testpass", // password is not unique constrained }, }, }, @@ -785,21 +899,25 @@ func TestAdmissionWebhook_KongConsumers(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{ Name: "neutronscreds1", + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": "neutron", - "password": "testpass", + "username": "neutron", + "password": "testpass", }, }, { ObjectMeta: metav1.ObjectMeta{ Name: "neutronscreds2", + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": "neutron", // username is unique constrained - "password": "testpass", + "username": "neutron", // username is unique constrained + "password": "testpass", }, }, }, @@ -825,11 +943,13 @@ func TestAdmissionWebhook_KongConsumers(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{ Name: "reasonablehammer", + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": "reasonablehammer", - "password": "testpass", // not unique constrained, so even though someone else is using this password this should pass + "username": "reasonablehammer", + "password": "testpass", // not unique constrained, so even though someone else is using this password this should pass }, }, }, @@ -854,11 +974,13 @@ func TestAdmissionWebhook_KongConsumers(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{ Name: "unreasonablehammer", + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": "tux1", // unique constrained with previous created static consumer credentials - "password": "testpass", + "username": "tux1", // unique constrained with previous created static consumer credentials + "password": "testpass", }, }, }, @@ -939,11 +1061,13 @@ func TestAdmissionWebhook_SecretCredentials(t *testing.T) { &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "brokenfence", + Labels: map[string]string{ + labels.CredentialTypeLabel: "invalid-auth", + }, }, StringData: map[string]string{ - "kongCredType": "invalid-auth", // not a valid credential type - "username": "brokenfence", - "password": "testpass", + "username": "brokenfence", + "password": "testpass", }, }, ), @@ -954,11 +1078,13 @@ func TestAdmissionWebhook_SecretCredentials(t *testing.T) { validCredential := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "brokenfence", + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": "brokenfence", - "password": "testpass", + "username": "brokenfence", + "password": "testpass", }, } require.NoError(t, ctrlClient.Create(ctx, validCredential)) @@ -996,7 +1122,7 @@ func TestAdmissionWebhook_SecretCredentials(t *testing.T) { require.NoError(t, ctrlClient.Update(ctx, validCredential)) t.Log("verifying that validation fails if the now referenced and valid credential gets updated to become invalid") - validCredential.Data["kongCredType"] = []byte("invalid-auth") + validCredential.ObjectMeta.Labels[labels.CredentialTypeLabel] = "invalid-auth" err := ctrlClient.Update(ctx, validCredential) require.Error(t, err) require.ErrorContains(t, err, "invalid credential type") @@ -1012,10 +1138,12 @@ func TestAdmissionWebhook_SecretCredentials(t *testing.T) { ctrlClient.Create(ctx, &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "invalid-jwt-", + Labels: map[string]string{ + labels.CredentialTypeLabel: "jwt", + }, }, StringData: map[string]string{ - "kongCredType": "jwt", - "algorithm": "RS256", + "algorithm": "RS256", }, }), "missing required field(s): rsa_public_key, key, secret", @@ -1029,12 +1157,14 @@ func TestAdmissionWebhook_SecretCredentials(t *testing.T) { require.NoError(t, ctrlClient.Create(ctx, &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "valid-jwt-" + strings.ToLower(algo) + "-", + Labels: map[string]string{ + labels.CredentialTypeLabel: "jwt", + }, }, StringData: map[string]string{ - "kongCredType": "jwt", - "algorithm": algo, - "key": "key-name", - "secret": "secret-name", + "algorithm": algo, + "key": "key-name", + "secret": "secret-name", }, }), "failed to create JWT credential with algorithm %s", algo) }) @@ -1047,12 +1177,14 @@ func TestAdmissionWebhook_SecretCredentials(t *testing.T) { require.Error(t, ctrlClient.Create(ctx, &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "invalid-jwt-" + strings.ToLower(algo) + "-", + Labels: map[string]string{ + labels.CredentialTypeLabel: "jwt", + }, }, StringData: map[string]string{ - "kongCredType": "jwt", - "algorithm": algo, - "key": "key-name", - "secret": "secret-name", + "algorithm": algo, + "key": "key-name", + "secret": "secret-name", }, }), "expected failure when creating JWT %s", algo) }) @@ -1080,11 +1212,13 @@ func createKongConsumers(ctx context.Context, t *testing.T, cl client.Client, co credential := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: credentialName, + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": credentialName, - "password": "testpass", + "username": credentialName, + "password": "testpass", }, } t.Logf("creating %s Secret that contains credentials", credentialName) diff --git a/test/envtest/kongstate_consumer_failures_test.go b/test/envtest/kongstate_consumer_failures_test.go index 1bbf132cb0..c40c2a0d2b 100644 --- a/test/envtest/kongstate_consumer_failures_test.go +++ b/test/envtest/kongstate_consumer_failures_test.go @@ -15,6 +15,7 @@ import ( ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kong/kubernetes-ingress-controller/v3/internal/annotations" + "github.com/kong/kubernetes-ingress-controller/v3/internal/labels" kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1" ) @@ -41,21 +42,24 @@ func TestKongStateFillConsumersAndCredentialsFailure(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "key-auth-cred", Namespace: ns.Name, + Labels: map[string]string{ + labels.CredentialTypeLabel: "key-auth", + }, }, Data: map[string][]byte{ - "kongCredType": []byte("key-auth"), - "key": []byte("whatever"), - "ttl": []byte("1024"), + "key": []byte("whatever"), + "ttl": []byte("1024"), }, }, { ObjectMeta: metav1.ObjectMeta{ Name: "empty-cred", Namespace: ns.Name, + Labels: map[string]string{ + labels.CredentialTypeLabel: "key-auth", + }, }, - Data: map[string][]byte{ - "kongCredType": []byte("key-auth"), - }, + Data: map[string][]byte{}, }, } for _, secret := range secrets { diff --git a/test/integration/consumer_group_test.go b/test/integration/consumer_group_test.go index f3946dfd33..34e3d1ece6 100644 --- a/test/integration/consumer_group_test.go +++ b/test/integration/consumer_group_test.go @@ -20,6 +20,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/kong/kubernetes-ingress-controller/v3/internal/annotations" + "github.com/kong/kubernetes-ingress-controller/v3/internal/labels" 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/pkg/clientset" @@ -271,10 +272,12 @@ func configureConsumerWithAPIKey( &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: name, + Labels: map[string]string{ + labels.CredentialTypeLabel: "key-auth", + }, }, StringData: map[string]string{ - "key": name, - "kongCredType": "key-auth", + "key": name, }, }, metav1.CreateOptions{}, diff --git a/test/integration/consumer_test.go b/test/integration/consumer_test.go index c07757a257..9ea319eb5e 100644 --- a/test/integration/consumer_test.go +++ b/test/integration/consumer_test.go @@ -20,6 +20,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/kong/kubernetes-ingress-controller/v3/internal/annotations" + "github.com/kong/kubernetes-ingress-controller/v3/internal/labels" kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1" "github.com/kong/kubernetes-ingress-controller/v3/pkg/clientset" "github.com/kong/kubernetes-ingress-controller/v3/test" @@ -113,11 +114,13 @@ func TestConsumerCredential(t *testing.T) { credential := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: uuid.NewString(), + Labels: map[string]string{ + labels.CredentialTypeLabel: "basic-auth", + }, }, StringData: map[string]string{ - "kongCredType": "basic-auth", - "username": "test_consumer_credential", - "password": "test_consumer_credential", + "username": "test_consumer_credential", + "password": "test_consumer_credential", }, } _, err = env.Cluster().Client().CoreV1().Secrets(ns.Name).Create(ctx, credential, metav1.CreateOptions{})