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{})