Skip to content

Commit

Permalink
feat: add validation on secrets on generated plugin cofigs
Browse files Browse the repository at this point in the history
  • Loading branch information
@randmonkey
randmonkey committed Nov 30, 2023
1 parent 73ab4d3 commit ab828ad
Show file tree
Hide file tree
Showing 6 changed files with 195 additions and 10 deletions.
53 changes: 50 additions & 3 deletions internal/admission/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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"
Expand All @@ -25,6 +26,10 @@ 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 +208,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 +227,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 Down Expand Up @@ -252,7 +257,49 @@ func (h RequestHandler) handleSecret(
switch request.Operation {
case admissionv1.Update, admissionv1.Create:
ok, message := h.Validator.ValidateCredential(ctx, secret)
return responseBuilder.Allowed(ok).WithMessage(message).Build(), nil
if !ok {
return responseBuilder.Allowed(ok).WithMessage(message).Build(), nil
}
referrers, err := h.ReferenceIndexers.ListReferrerObjectsByReferent(&secret)
if err != nil {
return responseBuilder.Allowed(false).WithMessage(err.Error()).Build(), err
}
for _, obj := range referrers {
gvk := obj.GetObjectKind().GroupVersionKind()
if gvk.Group == kongv1.GroupVersion.Group && gvk.Version == kongv1.GroupVersion.Version && gvk.Kind == "KongPlugin" {
plugin := obj.(*kongv1.KongPlugin)
ok, message, err := h.Validator.ValidatePlugin(ctx, *plugin, []*corev1.Secret{&secret})
if err != nil {
return nil, fmt.Errorf("failed to run validation on KongPlugin %s/%s: %w",
plugin.Namespace, plugin.Name, err,
)
}
if !ok {
return responseBuilder.Allowed(ok).WithMessage(
fmt.Sprintf("Change on secret will generate invalid configuration for KongPlugin %s/%s: %s",
plugin.Namespace, plugin.Name, message,
)).Build(), nil
}
}
if gvk.Group == kongv1.GroupVersion.Group && gvk.Version == kongv1.GroupVersion.Version && gvk.Kind == "KongClusterPlugin" {
plugin := obj.(*kongv1.KongClusterPlugin)
ok, message, err := h.Validator.ValidateClusterPlugin(ctx, *plugin, []*corev1.Secret{&secret})
if err != nil {
return nil, fmt.Errorf("failed to run validation on KongClusterPlugin %s: %w",
plugin.Name, err,
)
}
if !ok {
return responseBuilder.Allowed(ok).WithMessage(
fmt.Sprintf("Change on secret will generate invalid configuration for KongClusterPlugin %s: %s",
plugin.Name, message,
)).Build(), nil
}

}
}
return responseBuilder.Allowed(true).WithMessage("").Build(), nil

default:
return nil, fmt.Errorf("unknown operation %q", string(request.Operation))
}
Expand Down
2 changes: 2 additions & 0 deletions internal/admission/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,15 @@ func (v KongFakeValidator) ValidateConsumerGroup(
func (v KongFakeValidator) ValidatePlugin(
_ context.Context,
_ kongv1.KongPlugin,
_ []*corev1.Secret,
) (bool, string, error) {
return v.Result, v.Message, v.Error
}

func (v KongFakeValidator) ValidateClusterPlugin(
_ context.Context,
_ kongv1.KongClusterPlugin,
_ []*corev1.Secret,
) (bool, string, error) {
return v.Result, v.Message, v.Error
}
Expand Down
49 changes: 44 additions & 5 deletions internal/admission/validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import (

"github.com/go-logr/logr"
"github.com/kong/go-kong/kong"
"github.com/samber/lo"
corev1 "k8s.io/api/core/v1"
netv1 "k8s.io/api/networking/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8stypes "k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

credsvalidation "github.com/kong/kubernetes-ingress-controller/v3/internal/admission/validation/consumers/credentials"
Expand All @@ -30,8 +32,8 @@ import (
type KongValidator interface {
ValidateConsumer(ctx context.Context, consumer kongv1.KongConsumer) (bool, string, error)
ValidateConsumerGroup(ctx context.Context, consumerGroup kongv1beta1.KongConsumerGroup) (bool, string, error)
ValidatePlugin(ctx context.Context, plugin kongv1.KongPlugin) (bool, string, error)
ValidateClusterPlugin(ctx context.Context, plugin kongv1.KongClusterPlugin) (bool, string, error)
ValidatePlugin(ctx context.Context, plugin kongv1.KongPlugin, overrideSecrets []*corev1.Secret) (bool, string, error)
ValidateClusterPlugin(ctx context.Context, plugin kongv1.KongClusterPlugin, overrideSecrets []*corev1.Secret) (bool, string, error)
ValidateCredential(ctx context.Context, secret corev1.Secret) (bool, string)
ValidateGateway(ctx context.Context, gateway gatewayapi.Gateway) (bool, string, error)
ValidateHTTPRoute(ctx context.Context, httproute gatewayapi.HTTPRoute) (bool, string, error)
Expand All @@ -53,6 +55,38 @@ type ConsumerGetter interface {
ListAllConsumers(ctx context.Context) ([]kongv1.KongConsumer, error)
}

var _ kongstate.SecretGetter = SecretGetterWithOverride{}

type SecretGetterWithOverride struct {
overrideSecrets map[k8stypes.NamespacedName]*corev1.Secret
secretGetter kongstate.SecretGetter
}

func (s SecretGetterWithOverride) GetSecret(namespace, name string) (*corev1.Secret, error) {
nsName := k8stypes.NamespacedName{
Namespace: namespace,
Name: name,
}
overrideSecret, ok := s.overrideSecrets[nsName]
if ok {
return overrideSecret, nil
}

return s.secretGetter.GetSecret(namespace, name)
}

func NewSecretGetterWithOverride(s kongstate.SecretGetter, overrideSecrets []*corev1.Secret) SecretGetterWithOverride {
return SecretGetterWithOverride{
overrideSecrets: lo.SliceToMap(overrideSecrets, func(secret *corev1.Secret) (k8stypes.NamespacedName, *corev1.Secret) {
return k8stypes.NamespacedName{
Namespace: secret.Namespace,
Name: secret.Name,
}, secret
}),
secretGetter: s,
}
}

// KongHTTPValidator implements KongValidator interface to validate Kong
// entities using the Admin API of Kong.
type KongHTTPValidator struct {
Expand Down Expand Up @@ -280,13 +314,16 @@ func (validator KongHTTPValidator) ValidateCredential(ctx context.Context, secre
func (validator KongHTTPValidator) ValidatePlugin(
ctx context.Context,
k8sPlugin kongv1.KongPlugin,
overrideSecrets []*corev1.Secret,
) (bool, string, error) {
var plugin kong.Plugin
plugin.Name = kong.String(k8sPlugin.PluginName)
var err error

secretGetter := NewSecretGetterWithOverride(validator.SecretGetter, overrideSecrets)

plugin.Config, err = kongstate.RawConfigurationWithPatchesToConfiguration(
validator.SecretGetter,
secretGetter,
k8sPlugin.Namespace,
k8sPlugin.Config,
k8sPlugin.ConfigPatches,
Expand Down Expand Up @@ -324,13 +361,15 @@ func (validator KongHTTPValidator) ValidatePlugin(
func (validator KongHTTPValidator) ValidateClusterPlugin(
ctx context.Context,
k8sPlugin kongv1.KongClusterPlugin,
overrideSecrets []*corev1.Secret,
) (bool, string, error) {
var plugin kong.Plugin
plugin.Name = kong.String(k8sPlugin.PluginName)
var err error

secretGetter := NewSecretGetterWithOverride(validator.SecretGetter, overrideSecrets)
plugin.Config, err = kongstate.RawConfigurationWithNamespacedPatchesToConfiguration(
validator.SecretGetter,
secretGetter,
k8sPlugin.Config,
k8sPlugin.ConfigPatches,
)
Expand All @@ -339,7 +378,7 @@ func (validator KongHTTPValidator) ValidateClusterPlugin(
}

if k8sPlugin.ConfigFrom != nil {
config, err := kongstate.NamespacedSecretToConfiguration(validator.SecretGetter, k8sPlugin.ConfigFrom.SecretValue)
config, err := kongstate.NamespacedSecretToConfiguration(secretGetter, k8sPlugin.ConfigFrom.SecretValue)
if err != nil {
return false, fmt.Sprintf("%s: %s", ErrTextPluginSecretConfigUnretrievable, err), nil
}
Expand Down
4 changes: 2 additions & 2 deletions internal/admission/validator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ func TestKongHTTPValidator_ValidatePlugin(t *testing.T) {
},
ingressClassMatcher: fakeClassMatcher,
}
gotOK, gotMessage, err := validator.ValidatePlugin(context.Background(), tt.args.plugin)
gotOK, gotMessage, err := validator.ValidatePlugin(context.Background(), tt.args.plugin, nil)
assert.Equalf(t, tt.wantOK, gotOK,
"KongHTTPValidator.ValidatePlugin() want OK: %v, got OK: %v",
tt.wantOK, gotOK,
Expand Down Expand Up @@ -428,7 +428,7 @@ func TestKongHTTPValidator_ValidateClusterPlugin(t *testing.T) {
ingressClassMatcher: fakeClassMatcher,
}

gotOK, gotMessage, err := validator.ValidateClusterPlugin(context.Background(), tt.args.plugin)
gotOK, gotMessage, err := validator.ValidateClusterPlugin(context.Background(), tt.args.plugin, nil)
assert.Equalf(t, tt.wantOK, gotOK,
"KongHTTPValidator.ValidateClusterPlugin() want OK: %v, got OK: %v",
tt.wantOK, gotOK,
Expand Down
33 changes: 33 additions & 0 deletions internal/controllers/reference/indexer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,27 @@ func (c CacheIndexers) ListReferencesByReferrer(referrer client.Object) ([]*Obje
return returnRefList, nil
}

// ListReferencesByReferent lists all reference records referring to the same referent.
func (c CacheIndexers) ListReferencesByReferent(referent client.Object) ([]*ObjectReference, error) {
referenctKey, err := objectKeyFunc(referent)
if err != nil {
return nil, err
}
refList, err := c.indexer.ByIndex(IndexNameReferent, referenctKey)
if err != nil {
return nil, err
}
returnRefList := make([]*ObjectReference, 0, len(refList))
for _, ref := range refList {
retRef, ok := ref.(*ObjectReference)
if !ok {
return nil, ErrTypeNotObjectReference
}
returnRefList = append(returnRefList, retRef)
}
return returnRefList, nil
}

// DeleteReferencesByReferrer deletes all reference records where referrer has the same key
// (GroupVersionKind+NamespacedName, that means the same k8s object).
// called when a k8s object deleted in cluster, or when we do not care about it anymore.
Expand Down Expand Up @@ -245,3 +266,15 @@ func (c CacheIndexers) ListReferredObjects(referrer client.Object) ([]client.Obj
}
return objs, nil
}

func (c CacheIndexers) ListReferrerObjectsByReferent(referent client.Object) ([]client.Object, error) {
refs, err := c.ListReferencesByReferent(referent)
if err != nil {
return nil, err
}
objs := []client.Object{}
for _, ref := range refs {
objs = append(objs, ref.Referrer)
}
return objs, nil
}
64 changes: 64 additions & 0 deletions test/integration/webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,70 @@ func TestValidationWebhook(t *testing.T) {
}
_, err = env.Cluster().Client().CoreV1().Secrets(ns.Name).Create(ctx, invalidJWT, metav1.CreateOptions{})
require.ErrorContains(t, err, "some fields were invalid due to missing data: rsa_public_key, key, secret")

t.Log("verifying that the validation fails when secret generates invalid plugin configuration for KongPlugin")
for _, tt := range []struct {
name string
KongPlugin *kongv1.KongPlugin
secretBefore *corev1.Secret
secretAfter *corev1.Secret
errorOnUpdate bool
}{
{
name: "should fail the validation if secret produces invalid plugin configuration",
KongPlugin: &kongv1.KongPlugin{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns.Name,
Name: "rate-limiting",
},
PluginName: "rate-limiting",
ConfigFrom: &kongv1.ConfigSource{
SecretValue: kongv1.SecretValueFromSource{
Secret: "conf-secret",
Key: "rate-limiting-config",
},
},
},
secretBefore: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns.Name,
Name: "conf-secret",
},
Data: map[string][]byte{
"rate-limiting-config": []byte(`{"limit_by":"consumer","policy":"local","minute":5}`),
},
},
secretAfter: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns.Name,
Name: "conf-secret",
},
Data: map[string][]byte{
"rate-limiting-config": []byte(`{"limit_by":"consumer","policy":"local"}`),
},
},
errorOnUpdate: true,
},
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
_, err := env.Cluster().Client().CoreV1().Secrets(ns.Name).Create(ctx, tt.secretBefore, metav1.CreateOptions{})
require.NoError(t, err)
_, err = kongClient.ConfigurationV1().KongPlugins(ns.Name).Create(ctx, tt.KongPlugin, metav1.CreateOptions{})
require.NoError(t, err)
defer func() {
err := kongClient.ConfigurationV1().KongPlugins(ns.Name).Delete(ctx, tt.KongPlugin.Name, metav1.DeleteOptions{})
require.NoError(t, err)
}()

_, err = env.Cluster().Client().CoreV1().Secrets(ns.Name).Create(ctx, tt.secretAfter, metav1.CreateOptions{})
if tt.errorOnUpdate {
require.Error(t, err)
} else {
require.NoError(t, err)
}
})
}
}

func ensureWebhookService(ctx context.Context, t *testing.T, name string) {
Expand Down

0 comments on commit ab828ad

Please sign in to comment.