diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d2f88cfe3..8d507b823d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -106,6 +106,12 @@ Adding a new version? You'll need three changes: - Added `--kong-admin-token-file` flag to provide the Kong admin token via file [Providing Kong admin token via file](https://github.com/Kong/deck/blob/main/CHANGELOG.md#v1120). [#4808](https://github.com/Kong/kubernetes-ingress-controller/pull/4808) +- Credentials now use a `konghq.com/credential-type` label to indicate + credential type instead of the `kongCredType` field. This allows controller + compontents to avoid caching unnecessary Secrets. The `kongCredType` field is + still supported but is now deprecated. A script to generate commands to + update Secrets is available at TODO. + [#4825](https://github.com/Kong/kubernetes-ingress-controller/pull/4825) ### Fixed diff --git a/internal/admission/handler.go b/internal/admission/handler.go index fdcb5554d4..71b67ccdd5 100644 --- a/internal/admission/handler.go +++ b/internal/admission/handler.go @@ -14,6 +14,7 @@ import ( gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/kong/kubernetes-ingress-controller/v2/internal/gatewayapi" + "github.com/kong/kubernetes-ingress-controller/v2/internal/labels" kongv1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1" kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1beta1" ) @@ -242,7 +243,13 @@ func (h RequestHandler) handleSecret( if err != nil { return nil, err } - if _, ok := secret.Data["kongCredType"]; !ok { + // 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 + + _, labelOk := secret.Labels[labels.LabelPrefix+labels.CredentialKey] + _, fieldOk := secret.Data["kongCredType"] + + if !(labelOk || fieldOk) { // secret does not look like a credential resource in Kong return responseBuilder.Allowed(true).Build(), nil } diff --git a/internal/admission/validation/consumers/credentials/validation.go b/internal/admission/validation/consumers/credentials/validation.go index 276de4348d..ec70d510cf 100644 --- a/internal/admission/validation/consumers/credentials/validation.go +++ b/internal/admission/validation/consumers/credentials/validation.go @@ -6,6 +6,8 @@ import ( "strings" corev1 "k8s.io/api/core/v1" + + "github.com/kong/kubernetes-ingress-controller/v2/internal/labels" ) // ----------------------------------------------------------------------------- @@ -15,12 +17,19 @@ import ( // ValidateCredentials performs basic validation on a credential secret given // the Kubernetes secret which contains credentials data. func ValidateCredentials(secret *corev1.Secret) error { - // the indication of credential type is required to be present on all credentials. - credentialTypeB, ok := secret.Data[TypeKey] - if !ok { - return fmt.Errorf("missing required key %s", TypeKey) + var credentialType string + var labelOk bool + credentialType, labelOk = secret.Labels[labels.LabelPrefix+labels.CredentialKey] + + if !labelOk { + // the indication of credential type is required to be present on all credentials. + credentialTypeB, ok := secret.Data[TypeKey] + if !ok { + // while it's also missing the data field, encourage the new label convention + return fmt.Errorf("credential missing %s label", labels.LabelPrefix+labels.CredentialKey) + } + credentialType = string(credentialTypeB) } - credentialType := string(credentialTypeB) // verify that the credential type provided is valid if !SupportedTypes.Has(credentialType) { diff --git a/internal/dataplane/kongstate/kongstate.go b/internal/dataplane/kongstate/kongstate.go index 4c2eabba91..55adbd3f3e 100644 --- a/internal/dataplane/kongstate/kongstate.go +++ b/internal/dataplane/kongstate/kongstate.go @@ -13,6 +13,7 @@ import ( "github.com/kong/kubernetes-ingress-controller/v2/internal/admission/validation/consumers/credentials" "github.com/kong/kubernetes-ingress-controller/v2/internal/annotations" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/failures" + "github.com/kong/kubernetes-ingress-controller/v2/internal/labels" "github.com/kong/kubernetes-ingress-controller/v2/internal/store" "github.com/kong/kubernetes-ingress-controller/v2/internal/util" ) @@ -132,7 +133,7 @@ func (ks *KongState) FillConsumersAndCredentials( if err != nil { // add a translation error here to tell that parsing TTL failed. pushCredentialResourceFailures( - fmt.Sprintf("faield to parse ttl to int: %v, skipfilling the fiedl", err), + fmt.Sprintf("failed to parse ttl to int: %v, skipfilling the field", err), ) } else { credConfig[k] = intVal @@ -141,12 +142,23 @@ func (ks *KongState) FillConsumersAndCredentials( } credConfig[k] = string(v) } - credType, ok := credConfig["kongCredType"].(string) - if !ok { - pushCredentialResourceFailures( - fmt.Sprintf("failed to provision credential: invalid kongCredType: type '%T' not string", credType), - ) - continue + // try the label first. if it's present, no need to check the field + credType, labelOk := secret.Labels[labels.LabelPrefix+labels.CredentialKey] + var fieldOk bool + if !labelOk { + // if no label, fall back to the deprecated field + credType, fieldOk = credConfig["kongCredType"].(string) + if !fieldOk { + pushCredentialResourceFailures( + fmt.Sprintf("failed to provision credential: invalid kongCredType: type '%T' not string", credType), + ) + continue + } + } + if fieldOk && !labelOk { + // TODO this resource has not been migrated + // TODO we don't have a way to warn here yet, no logger or error return + // TODO hold that pending the log PR } if !credentials.SupportedTypes.Has(credType) { pushCredentialResourceFailures( @@ -154,12 +166,6 @@ func (ks *KongState) FillConsumersAndCredentials( ) continue } - if len(credConfig) <= 1 { // 1 key of credType itself - pushCredentialResourceFailures( - "failed to provision credential: empty secret", - ) - continue - } credTags := util.GenerateTagsForObject(secret) if err := c.SetCredential(credType, credConfig, credTags); err != nil { pushCredentialResourceFailures( diff --git a/internal/dataplane/kongstate/kongstate_test.go b/internal/dataplane/kongstate/kongstate_test.go index 249afc55c6..9f94af2bcd 100644 --- a/internal/dataplane/kongstate/kongstate_test.go +++ b/internal/dataplane/kongstate/kongstate_test.go @@ -19,6 +19,7 @@ import ( "github.com/kong/kubernetes-ingress-controller/v2/internal/annotations" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/failures" + "github.com/kong/kubernetes-ingress-controller/v2/internal/labels" "github.com/kong/kubernetes-ingress-controller/v2/internal/store" "github.com/kong/kubernetes-ingress-controller/v2/internal/util" kongv1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1" @@ -477,6 +478,43 @@ func TestFillConsumersAndCredentials(t *testing.T) { "foo": []byte("bar"), }, }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "labeledSecret", + Namespace: "default", + Labels: map[string]string{ + labels.LabelPrefix + labels.CredentialKey: "key-auth", + }, + }, + Data: map[string][]byte{ + "key": []byte("little-rabbits-be-good"), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "labeledSecretWithCredField", + Namespace: "default", + Labels: map[string]string{ + labels.LabelPrefix + labels.CredentialKey: "key-auth", + }, + }, + Data: map[string][]byte{ + "kongCredType": []byte("key-auth"), + "key": []byte("little-rabbits-be-good"), + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "badTypeLabeledSecret", + Namespace: "default", + Labels: map[string]string{ + labels.LabelPrefix + labels.CredentialKey: "bee-auth", + }, + }, + Data: map[string][]byte{ + "foo": []byte("bar"), + }, + }, } testCases := []struct { @@ -615,6 +653,103 @@ func TestFillConsumersAndCredentials(t *testing.T) { {Namespace: "default", Name: "foo"}: fmt.Sprintf("failed to provision credential: unsupported kongCredType: %q", "unsupported"), }, }, + { + name: "referring to secret with unsupported credential label", + k8sConsumers: []*kongv1.KongConsumer{ + { + TypeMeta: kongConsumerTypeMeta, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "default", + Annotations: map[string]string{ + "kubernetes.io/ingress.class": annotations.DefaultIngressClass, + }, + }, + Username: "foo", + Credentials: []string{ + "badTypeLabeledSecret", + }, + }, + }, + expectedKongStateConsumers: []Consumer{ + { + Consumer: kong.Consumer{ + Username: kong.String("foo"), + }, + }, + }, + expectedTranslationFailureMessages: map[k8stypes.NamespacedName]string{ + {Namespace: "default", Name: "foo"}: fmt.Sprintf("failed to provision credential: unsupported kongCredType: %q", "bee-auth"), + }, + }, + { + name: "KongConsumer with key-auth from label secret", + 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{ + "labeledSecret", + }, + }, + }, + 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: "labeledSecret"}, + }), + }}}, + }, + }, + }, + { + 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 new file mode 100644 index 0000000000..13b0b32d47 --- /dev/null +++ b/internal/labels/labels.go @@ -0,0 +1,11 @@ +package labels + +import "github.com/kong/kubernetes-ingress-controller/v2/internal/annotations" + +const ( + // LabelPrefix is the string used at the beginning of KIC-specific labels. + LabelPrefix = annotations.AnnotationPrefix + + // CredentialKey is the key used to indicate a Secret's credential type. + CredentialKey = "/credential" //nolint:gosec +) diff --git a/test/envtest/kongstate_consumer_failures_test.go b/test/envtest/kongstate_consumer_failures_test.go index f1cf916b00..816a0c91db 100644 --- a/test/envtest/kongstate_consumer_failures_test.go +++ b/test/envtest/kongstate_consumer_failures_test.go @@ -98,7 +98,7 @@ func TestKongStateFillConsumersAndCredentialsFailure(t *testing.T) { // KongConsumer name -> event message kongConsumerTranslationFailureMessages := map[string]string{ - "consumer-empty-cred": `credential "empty-cred" failure: failed to provision credential: empty secret`, + "consumer-empty-cred": `credential "empty-cred" failure: failed to provision credential: key-auth is invalid: no key`, "consumer-no-username": `no username or custom_id specified`, }