Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Determine credential type from label #4825

Merged
merged 5 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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-type` label to indicate
czeslavo marked this conversation as resolved.
Show resolved Hide resolved
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we track changing this changelog entry to reference a proper docs link (or an equivalent) rather than a github issue link

[#4825](https:/Kong/kubernetes-ingress-controller/pull/4825)


### Fixed

Expand Down
7 changes: 6 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/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"
)
Expand Down Expand Up @@ -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
}
Expand Down
21 changes: 10 additions & 11 deletions internal/admission/validation/consumers/credentials/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

// -----------------------------------------------------------------------------
Expand All @@ -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 {
rainest marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package credentials

import (
"fmt"
"testing"

"github.com/kong/kubernetes-ingress-controller/v2/internal/labels"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestUniqueConstraintsValidation(t *testing.T) {
Expand Down Expand Up @@ -59,3 +63,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)
})
}
}
33 changes: 13 additions & 20 deletions internal/dataplane/kongstate/kongstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,18 @@ 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 {
failuresCollector.PushResourceFailure("credential only has deprecated kongCredType field, needs "+
"konghq.com/credential label", secret)
}
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
Expand Down Expand Up @@ -132,7 +144,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 +153,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(
Expand Down
Loading
Loading