Skip to content

Commit

Permalink
feat(credentials) accept label credential type
Browse files Browse the repository at this point in the history
  • Loading branch information
rainest committed Oct 13, 2023
1 parent 7c2a104 commit 4fef47b
Show file tree
Hide file tree
Showing 7 changed files with 194 additions and 20 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:/Kong/deck/blob/main/CHANGELOG.md#v1120).
[#4808](https:/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:/Kong/kubernetes-ingress-controller/pull/4825)


### Fixed
Expand Down
9 changes: 8 additions & 1 deletion internal/admission/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
Expand Down
19 changes: 14 additions & 5 deletions internal/admission/validation/consumers/credentials/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"strings"

corev1 "k8s.io/api/core/v1"

"github.com/kong/kubernetes-ingress-controller/v2/internal/labels"
)

// -----------------------------------------------------------------------------
Expand All @@ -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) {
Expand Down
32 changes: 19 additions & 13 deletions internal/dataplane/kongstate/kongstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand All @@ -141,25 +142,30 @@ 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(
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(
Expand Down
135 changes: 135 additions & 0 deletions internal/dataplane/kongstate/kongstate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
11 changes: 11 additions & 0 deletions internal/labels/labels.go
Original file line number Diff line number Diff line change
@@ -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
)
2 changes: 1 addition & 1 deletion test/envtest/kongstate_consumer_failures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
}

Expand Down

0 comments on commit 4fef47b

Please sign in to comment.