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

feat: add validation on secrets on generated plugin cofigs #5203

Merged
merged 4 commits into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
9 changes: 8 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,19 @@ Adding a new version? You'll need three changes:
- Added support for GRPC over HTTP (without TLS) in Gateway API.
[#5128](https://github.com/Kong/kubernetes-ingress-controller/pull/5128)
[#5283](https://github.com/Kong/kubernetes-ingress-controller/pull/5283)
- Added `-init-cache-sync-duration` CLI flag. This flag configures how long the controller waits for Kubernetes resources to populate at startup before generating the initial Kong configuration. It also fixes a bug that removed the default 5 second wait period.
- Added `--init-cache-sync-duration` CLI flag. This flag configures how long the
controller waits for Kubernetes resources to populate at startup before
generating the initial Kong configuration. It also fixes a bug that removed
the default 5 second wait period.
[#5238](https://github.com/Kong/kubernetes-ingress-controller/pull/5238)
- Added `--emit-kubernetes-events` CLI flag to disable the creation of events
in translating and applying configurations to Kong.
[#5296](https://github.com/Kong/kubernetes-ingress-controller/pull/5296)
[#5299](https://github.com/Kong/kubernetes-ingress-controller/pull/5299)
- Added validation on `Secret`s to reject the change if it will generate
invalid confiugration of plugins for `KongPlugin`s or `KongClusterPlugin`s
referencing to the secret.
[#5203](https://github.com/Kong/kubernetes-ingress-controller/pull/5203)

### Fixed

Expand Down
87 changes: 75 additions & 12 deletions internal/admission/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,28 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"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/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"
)

const (
KindKongPlugin = "KongPlugin"
KindKongClusterPlugin = "KongClusterPlugin"
)

// RequestHandler is an HTTP server that can validate Kong Ingress Controllers'
// Custom Resources using Kubernetes Admission Webhooks.
type RequestHandler struct {
// Validator validates the entities that the k8s API-server asks
// it the server to validate.
Validator KongValidator
// ReferenceIndexers gets the resources (KongPlugin and KongClusterPlugin)
// referring the validated resource (Secret) to check the changes on
// referred Secret will produce invalid configuration of the plugins.
ReferenceIndexers ctrlref.CacheIndexers

Logger logr.Logger
}
Expand Down Expand Up @@ -203,7 +213,7 @@ func (h RequestHandler) handleKongPlugin(
return nil, err
}

ok, message, err := h.Validator.ValidatePlugin(ctx, plugin)
ok, message, err := h.Validator.ValidatePlugin(ctx, plugin, nil)
if err != nil {
return nil, err
}
Expand All @@ -222,7 +232,7 @@ func (h RequestHandler) handleKongClusterPlugin(
return nil, err
}

ok, message, err := h.Validator.ValidateClusterPlugin(ctx, plugin)
ok, message, err := h.Validator.ValidateClusterPlugin(ctx, plugin, nil)
if err != nil {
return nil, err
}
Expand All @@ -240,24 +250,77 @@ func (h RequestHandler) handleSecret(
if err != nil {
return nil, err
}
// 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 {
// secret does not look like a credential resource in Kong
return responseBuilder.Allowed(true).Build(), nil
}

switch request.Operation {
case admissionv1.Update, admissionv1.Create:
ok, message := h.Validator.ValidateCredential(ctx, secret)
return responseBuilder.Allowed(ok).WithMessage(message).Build(), nil
// 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 {
ok, message := h.Validator.ValidateCredential(ctx, secret)
if !ok {
return responseBuilder.Allowed(ok).WithMessage(message).Build(), nil
}
}

ok, message, err := h.checkReferrersOfSecret(ctx, &secret)
if err != nil {
return responseBuilder.Allowed(false).WithMessage(fmt.Sprintf("failed to validate other objects referencing the secret: %v", err)).Build(), err
}
if !ok {
return responseBuilder.Allowed(false).WithMessage(message).Build(), nil
}

return responseBuilder.Allowed(true).Build(), nil

default:
return nil, fmt.Errorf("unknown operation %q", string(request.Operation))
}
}

// 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) {
referrers, err := h.ReferenceIndexers.ListReferrerObjectsByReferent(secret)
if err != nil {
return false, "", fmt.Errorf("failed to list referrers of secret: %w", err)
}

for _, obj := range referrers {
gvk := obj.GetObjectKind().GroupVersionKind()
if gvk.Group == kongv1.GroupVersion.Group && gvk.Version == kongv1.GroupVersion.Version && gvk.Kind == KindKongPlugin {
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",
plugin.Namespace, plugin.Name, err,
)
}
if !ok {
return false,
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 {
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",
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 true, "", nil
}

func (h RequestHandler) handleGateway(
ctx context.Context,
request admissionv1.AdmissionRequest,
Expand Down
196 changes: 195 additions & 1 deletion internal/admission/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,41 @@ import (
"context"
"encoding/json"
"fmt"
"net/http"
"testing"

"github.com/go-logr/logr"
"github.com/go-logr/logr/testr"
"github.com/stretchr/testify/require"
admissionv1 "k8s.io/api/admission/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
k8stypes "k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

"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/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"
)

var (
secretTypeMeta = metav1.TypeMeta{
APIVersion: "v1",
Kind: "Secret",
}
kongPluginTypeMeta = metav1.TypeMeta{
APIVersion: kongv1.GroupVersion.String(),
Kind: "KongPlugin",
}
kongClusterPluginTypeMeta = metav1.TypeMeta{
APIVersion: kongv1.GroupVersion.String(),
Kind: "KongClusterPlugin",
}
)

func TestHandleKongIngress(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -130,7 +150,6 @@ func TestHandleService(t *testing.T) {
Raw: raw,
},
}

handler := RequestHandler{
Validator: validator,
Logger: logr.Discard(),
Expand All @@ -145,3 +164,178 @@ func TestHandleService(t *testing.T) {
})
}
}

func TestHandleSecret(t *testing.T) {
testCases := []struct {
name string
secret *corev1.Secret
referrers []client.Object
validatorOK bool
validatorMessage string
validatorError error
expectAllowed bool
expectStatusCode int32
expectMessage string
expectError bool
}{
{
name: "secret used as a credential and passes the validation of credential",
secret: &corev1.Secret{
TypeMeta: secretTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "credential-0",
Labels: map[string]string{
"konghq.com/credential": "true",
},
},
Data: map[string][]byte{},
},
validatorOK: true,
expectAllowed: true,
},
{
name: "secret used as credential and fails the validation of credential",
secret: &corev1.Secret{
TypeMeta: secretTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "credential-1",
Labels: map[string]string{
"konghq.com/credential": "true",
},
},
Data: map[string][]byte{},
},
validatorOK: false,
validatorMessage: "invalid credential",
expectAllowed: false,
expectStatusCode: http.StatusBadRequest,
expectMessage: "invalid credential",
},
{
name: "secret used as KongPlugin config and KongClusterPlugin and passes validation of both CRDs",
secret: &corev1.Secret{
TypeMeta: secretTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "plugin-conf",
},
},
referrers: []client.Object{
&kongv1.KongPlugin{
TypeMeta: kongPluginTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "plugin-0",
},
PluginName: "test-plugin",
},
&kongv1.KongClusterPlugin{
TypeMeta: kongClusterPluginTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-plugin-0",
},
PluginName: "test-plugin",
},
},
validatorOK: true,
expectAllowed: true,
},
{
name: "secret used as KongPlugin config and fails validation of KongPlugin",
secret: &corev1.Secret{
TypeMeta: secretTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "plugin-conf",
},
},
referrers: []client.Object{
&kongv1.KongPlugin{
TypeMeta: kongPluginTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "plugin-0",
},
PluginName: "test-plugin",
},
},
validatorOK: false,
validatorMessage: "invalid KongPlugin",
expectAllowed: false,
expectStatusCode: http.StatusBadRequest,
expectMessage: "Change on secret will generate invalid configuration for KongPlugin default/plugin-0: invalid KongPlugin",
},
{
name: "secret used as KongClusterPlugin config and fails validation of KongClusterPlugin",
secret: &corev1.Secret{
TypeMeta: secretTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "plugin-conf",
},
},
referrers: []client.Object{
&kongv1.KongClusterPlugin{
TypeMeta: kongClusterPluginTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-plugin-0",
},
PluginName: "test-plugin",
},
},
validatorOK: false,
validatorMessage: "invalid KongClusterPlugin",
expectAllowed: false,
expectStatusCode: http.StatusBadRequest,
expectMessage: "Change on secret will generate invalid configuration for KongClusterPlugin cluster-plugin-0: invalid KongClusterPlugin",
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
validator := KongFakeValidator{
Result: tc.validatorOK,
Message: tc.validatorMessage,
Error: tc.validatorError,
}
raw, err := json.Marshal(tc.secret)
require.NoError(t, err)
request := admissionv1.AdmissionRequest{
Object: runtime.RawExtension{
Object: tc.secret,
Raw: raw,
},
Operation: admissionv1.Update,
}

logger := testr.NewWithOptions(t, testr.Options{Verbosity: util.DebugLevel})
referenceIndexer := ctrlref.NewCacheIndexers(logger)

handler := RequestHandler{
Validator: validator,
Logger: logger,
ReferenceIndexers: referenceIndexer,
}
for _, obj := range tc.referrers {
err := handler.ReferenceIndexers.SetObjectReference(obj, tc.secret)
require.NoError(t, err)
}

responseBuilder := NewResponseBuilder(k8stypes.UID(""))
got, err := handler.handleSecret(context.Background(), request, responseBuilder)
if tc.expectError {
require.Error(t, err)
return
}
require.NoError(t, err)
require.Equal(t, tc.expectAllowed, got.Allowed, "should return expected result of allowed")
require.Equal(t, tc.expectStatusCode, got.Result.Code)
if len(tc.expectMessage) > 0 {
require.Contains(t, got.Result.Message, tc.expectMessage)
}
})
}
}
Loading
Loading