feat: determine credential type from label (Kong#4825)

* feat(credentials) accept label credential type

* pr: remove bad copy/paste resource failure

* pr: reuse credential type code and add unit

* pr: log instead of failure, align test with new error

* use correct label in changelog

---------

Co-authored-by: Grzegorz Burzyński <[email protected]>

CHANGELOG.md

@@ -121,6 +121,13 @@ Adding a new version? You'll need three changes:
  It's nearly identical to `deploy/single/all-in-one-dbless-k4k8s-enterprise.yaml`
  which is used in the official docs.
  [#4873](https:/Kong/kubernetes-ingress-controller/pull/4873)
+- Credentials now use a `konghq.com/credential` 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 https:/Kong/kubernetes-ingress-controller/issues/2502#issuecomment-1758213596
+ [#4825](https:/Kong/kubernetes-ingress-controller/pull/4825)
+
 
 ### Fixed
 

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/util"
  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,11 @@ 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
+ // https:/Kong/kubernetes-ingress-controller/issues/4853
+
+ if _, credentialTypeSource := util.ExtractKongCredentialType(&secret); credentialTypeSource == util.CredentialTypeAbsent {
  // secret does not look like a credential resource in Kong
  return responseBuilder.Allowed(true).Build(), nil
  }

internal/admission/validation/consumers/credentials/validation.go

@@ -6,6 +6,9 @@ import (
  "strings"
 
  corev1 "k8s.io/api/core/v1"
+
+ "github.com/kong/kubernetes-ingress-controller/v2/internal/labels"
+ "github.com/kong/kubernetes-ingress-controller/v2/internal/util"
 )
 
 // -----------------------------------------------------------------------------
@@ -15,21 +18,17 @@ 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)
+ credentialType, credentialSource := util.ExtractKongCredentialType(secret)
+
+ if credentialSource == util.CredentialTypeAbsent {
+ // 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.LabelPrefix+labels.CredentialKey)
  }
- credentialType := string(credentialTypeB)
 
  // verify that the credential type provided is valid
  if !SupportedTypes.Has(credentialType) {
- return fmt.Errorf("invalid credential type %s", secret.Data[TypeKey])
- }
-
- // it's not valid to have a secret that ONLY has a type
- if len(secret.Data) == 1 {
- return fmt.Errorf("invalid credentials secret, no data present")
+ return fmt.Errorf("invalid credential type %s", credentialType)
  }
 
  // verify that all required fields are present

internal/admission/validation/consumers/credentials/validation_test.go

@@ -1,10 +1,15 @@
 package credentials
 
 import (
+ "fmt"
  "testing"
 
  "github.com/stretchr/testify/assert"
  "github.com/stretchr/testify/require"
+ corev1 "k8s.io/api/core/v1"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+ "github.com/kong/kubernetes-ingress-controller/v2/internal/labels"
 )
 
 func TestUniqueConstraintsValidation(t *testing.T) {
@@ -59,3 +64,110 @@ func TestUniqueConstraintsValidation(t *testing.T) {
  t.Log("verifying that unconstrained keys for types with constraints don't flag as violated")
  assert.False(t, IsKeyUniqueConstrained("basic-auth", "unconstrained-key"))
 }
+
+func TestValidateCredentials(t *testing.T) {
+ tests := []struct {
+ name string
+ secret *corev1.Secret
+ wantErr error
+ }{
+ {
+ name: "valid credential",
+ secret: &corev1.Secret{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "secret",
+ Namespace: "default",
+ Labels: map[string]string{
+ labels.LabelPrefix + labels.CredentialKey: "key-auth",
+ },
+ },
+ Data: map[string][]byte{
+ "key": []byte("little-rabbits-be-good"),
+ },
+ },
+ wantErr: nil,
+ },
+ {
+ // TODO https:/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{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "secret",
+ Namespace: "default",
+ Labels: map[string]string{
+ labels.LabelPrefix + labels.CredentialKey: "bee-auth",
+ },
+ },
+ Data: map[string][]byte{
+ "key": []byte("little-rabbits-be-good"),
+ },
+ },
+ wantErr: fmt.Errorf("invalid credential type bee-auth"),
+ },
+ {
+ name: "no credential type",
+ secret: &corev1.Secret{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "secret",
+ Namespace: "default",
+ },
+ Data: map[string][]byte{
+ "key": []byte("little-rabbits-be-good"),
+ },
+ },
+ wantErr: fmt.Errorf("secret has no credential type, add a %s label", labels.LabelPrefix+labels.CredentialKey),
+ },
+ {
+ name: "missing required field",
+ secret: &corev1.Secret{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "secret",
+ Namespace: "default",
+ Labels: map[string]string{
+ labels.LabelPrefix + labels.CredentialKey: "key-auth",
+ },
+ },
+ Data: map[string][]byte{
+ "bee": []byte("little-rabbits-be-good"),
+ },
+ },
+ wantErr: fmt.Errorf("missing required field(s): key"),
+ },
+ {
+ name: "empty required field",
+ secret: &corev1.Secret{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "secret",
+ Namespace: "default",
+ Labels: map[string]string{
+ labels.LabelPrefix + labels.CredentialKey: "key-auth",
+ },
+ },
+ Data: map[string][]byte{
+ "key": []byte(""),
+ },
+ },
+ wantErr: fmt.Errorf("some fields were invalid due to missing data: key"),
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ err := ValidateCredentials(tt.secret)
+ require.Equal(t, tt.wantErr, err)
+ })
+ }
+}

internal/dataplane/kongstate/kongstate.go

@@ -59,6 +59,7 @@ func (ks *KongState) SanitizedCopy() *KongState {
 }
 
 func (ks *KongState) FillConsumersAndCredentials(
+ logger logr.Logger,
  s store.Storer,
  failuresCollector *failures.ResourceFailuresCollector,
 ) {
@@ -102,6 +103,19 @@ func (ks *KongState) FillConsumersAndCredentials(
  continue
  }
  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)
+ }
+ if !credentials.SupportedTypes.Has(credType) {
+ pushCredentialResourceFailures(
+ fmt.Sprintf("failed to provision credential: unsupported credential type: %q", credType),
+ )
+ continue
+ }
  for k, v := range secret.Data {
  // TODO populate these based on schema from Kong
  // and remove this workaround
@@ -132,7 +146,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,25 +155,6 @@ 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
- }
- if !credentials.SupportedTypes.Has(credType) {
- pushCredentialResourceFailures(
- fmt.Sprintf("failed to provision credential: unsupported kongCredType: %q", credType),
- )
- 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(