Skip to content

Commit

Permalink
feat(*): validate namespace of containerpatch
Browse files Browse the repository at this point in the history
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
  • Loading branch information
jakubdyszkiewicz committed May 20, 2022
1 parent b0ebb21 commit adfae5c
Show file tree
Hide file tree
Showing 16 changed files with 82 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1875,7 +1875,7 @@ spec:
metadata:
annotations:
checksum/config: 47f8e00f6916a54e5c2c5c1f0369b8107436c15171abc1167b1fa35bd9afad5f
checksum/tls-secrets: cf73e8628484dfed58bd012abd61502d41e6c343ec4b248e3928db75d0166373
checksum/tls-secrets: 953b796aba8525a1a8557b0432b64ce88c9ddd0a97c490753fe978bfd95d823c
labels:
app.kubernetes.io/name: kuma
app.kubernetes.io/instance: kuma
Expand Down Expand Up @@ -2167,6 +2167,7 @@ webhooks:
- traffictraces
- virtualoutbounds
- zones
- containerpatches


sideEffects: None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1700,7 +1700,7 @@ spec:
metadata:
annotations:
checksum/config: 47f8e00f6916a54e5c2c5c1f0369b8107436c15171abc1167b1fa35bd9afad5f
checksum/tls-secrets: cf73e8628484dfed58bd012abd61502d41e6c343ec4b248e3928db75d0166373
checksum/tls-secrets: 953b796aba8525a1a8557b0432b64ce88c9ddd0a97c490753fe978bfd95d823c
labels:
app.kubernetes.io/name: kuma
app.kubernetes.io/instance: kuma
Expand Down Expand Up @@ -1992,6 +1992,7 @@ webhooks:
- traffictraces
- virtualoutbounds
- zones
- containerpatches


sideEffects: None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1710,7 +1710,7 @@ spec:
metadata:
annotations:
checksum/config: 47f8e00f6916a54e5c2c5c1f0369b8107436c15171abc1167b1fa35bd9afad5f
checksum/tls-secrets: cf73e8628484dfed58bd012abd61502d41e6c343ec4b248e3928db75d0166373
checksum/tls-secrets: 953b796aba8525a1a8557b0432b64ce88c9ddd0a97c490753fe978bfd95d823c
labels:
app.kubernetes.io/name: kuma
app.kubernetes.io/instance: kuma
Expand Down Expand Up @@ -2001,6 +2001,7 @@ webhooks:
- traffictraces
- virtualoutbounds
- zones
- containerpatches


sideEffects: None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1700,7 +1700,7 @@ spec:
metadata:
annotations:
checksum/config: 47f8e00f6916a54e5c2c5c1f0369b8107436c15171abc1167b1fa35bd9afad5f
checksum/tls-secrets: cf73e8628484dfed58bd012abd61502d41e6c343ec4b248e3928db75d0166373
checksum/tls-secrets: 953b796aba8525a1a8557b0432b64ce88c9ddd0a97c490753fe978bfd95d823c
labels:
app.kubernetes.io/name: kuma
app.kubernetes.io/instance: kuma
Expand Down Expand Up @@ -1992,6 +1992,7 @@ webhooks:
- traffictraces
- virtualoutbounds
- zones
- containerpatches


sideEffects: None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2113,7 +2113,7 @@ spec:
metadata:
annotations:
checksum/config: 652c0fee8359addb5b565f00943a4c64a5511bb208b041d6cdcca177d2772e61
checksum/tls-secrets: b8deca59ac509c99819cfab78ff0fe69ca375602f1eae5273153cff35ef8c2bc
checksum/tls-secrets: bed3f5682dc788625d4ddb36a4760691c53df7f50619cc6dabff49707dd4ae0a
labels:
app.kubernetes.io/name: kuma
app.kubernetes.io/instance: kuma
Expand Down Expand Up @@ -2452,6 +2452,7 @@ webhooks:
- traffictraces
- virtualoutbounds
- zones
- containerpatches


sideEffects: None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1729,7 +1729,7 @@ spec:
metadata:
annotations:
checksum/config: 47f8e00f6916a54e5c2c5c1f0369b8107436c15171abc1167b1fa35bd9afad5f
checksum/tls-secrets: cf73e8628484dfed58bd012abd61502d41e6c343ec4b248e3928db75d0166373
checksum/tls-secrets: 953b796aba8525a1a8557b0432b64ce88c9ddd0a97c490753fe978bfd95d823c
labels:
app.kubernetes.io/name: kuma
app.kubernetes.io/instance: kuma
Expand Down Expand Up @@ -2131,6 +2131,7 @@ webhooks:
- traffictraces
- virtualoutbounds
- zones
- containerpatches


sideEffects: None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1762,7 +1762,7 @@ spec:
metadata:
annotations:
checksum/config: 47f8e00f6916a54e5c2c5c1f0369b8107436c15171abc1167b1fa35bd9afad5f
checksum/tls-secrets: cf73e8628484dfed58bd012abd61502d41e6c343ec4b248e3928db75d0166373
checksum/tls-secrets: 953b796aba8525a1a8557b0432b64ce88c9ddd0a97c490753fe978bfd95d823c
labels:
app.kubernetes.io/name: kuma
app.kubernetes.io/instance: kuma
Expand Down Expand Up @@ -2278,6 +2278,7 @@ webhooks:
- traffictraces
- virtualoutbounds
- zones
- containerpatches


sideEffects: None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1700,7 +1700,7 @@ spec:
metadata:
annotations:
checksum/config: 47f8e00f6916a54e5c2c5c1f0369b8107436c15171abc1167b1fa35bd9afad5f
checksum/tls-secrets: cf73e8628484dfed58bd012abd61502d41e6c343ec4b248e3928db75d0166373
checksum/tls-secrets: 953b796aba8525a1a8557b0432b64ce88c9ddd0a97c490753fe978bfd95d823c
labels:
app.kubernetes.io/name: kuma
app.kubernetes.io/instance: kuma
Expand Down Expand Up @@ -1992,6 +1992,7 @@ webhooks:
- traffictraces
- virtualoutbounds
- zones
- containerpatches


sideEffects: None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1733,7 +1733,7 @@ spec:
metadata:
annotations:
checksum/config: 47f8e00f6916a54e5c2c5c1f0369b8107436c15171abc1167b1fa35bd9afad5f
checksum/tls-secrets: cf73e8628484dfed58bd012abd61502d41e6c343ec4b248e3928db75d0166373
checksum/tls-secrets: 953b796aba8525a1a8557b0432b64ce88c9ddd0a97c490753fe978bfd95d823c
labels:
app.kubernetes.io/name: kuma
app.kubernetes.io/instance: kuma
Expand Down Expand Up @@ -2139,6 +2139,7 @@ webhooks:
- traffictraces
- virtualoutbounds
- zones
- containerpatches


sideEffects: None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1704,7 +1704,7 @@ spec:
metadata:
annotations:
checksum/config: 47f8e00f6916a54e5c2c5c1f0369b8107436c15171abc1167b1fa35bd9afad5f
checksum/tls-secrets: cf73e8628484dfed58bd012abd61502d41e6c343ec4b248e3928db75d0166373
checksum/tls-secrets: 953b796aba8525a1a8557b0432b64ce88c9ddd0a97c490753fe978bfd95d823c
labels:
app.kubernetes.io/name: kuma
app.kubernetes.io/instance: kuma
Expand Down Expand Up @@ -2000,6 +2000,7 @@ webhooks:
- traffictraces
- virtualoutbounds
- zones
- containerpatches


sideEffects: None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ webhooks:
- traffictraces
- virtualoutbounds
- zones
- containerpatches
{{ .Values.controlPlane.webhooks.validator.additionalRules | nindent 6 }}
sideEffects: None
- name: service.validator.kuma-admission.kuma.io
Expand Down
5 changes: 5 additions & 0 deletions pkg/plugins/runtime/k8s/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,10 @@ func addValidators(mgr kube_ctrl.Manager, rt core_runtime.Runtime, converter k8s
mgr.GetWebhookServer().Register("/validate-v1alpha2-gateway-upstream", &upstreamValidatorHandler{})
}

composite.AddValidator(&k8s_webhooks.ContainerPatchValidator{
SystemNamespace: rt.Config().Store.Kubernetes.SystemNamespace,
})

return nil
}

Expand All @@ -307,6 +311,7 @@ func addMutators(mgr kube_ctrl.Manager, rt core_runtime.Runtime, converter k8s_c
mgr.GetClient(),
converter,
rt.Config().GetEnvoyAdminPort(),
rt.Config().Store.Kubernetes.SystemNamespace,
)
if err != nil {
return err
Expand Down
34 changes: 34 additions & 0 deletions pkg/plugins/runtime/k8s/webhooks/containerpatch_validator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package webhooks

import (
"context"

"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

k8s_common "github.com/kumahq/kuma/pkg/plugins/common/k8s"
mesh_k8s "github.com/kumahq/kuma/pkg/plugins/resources/k8s/native/api/v1alpha1"
)

type ContainerPatchValidator struct {
SystemNamespace string
}

func NewContainerPatchValidatorWebhook() k8s_common.AdmissionValidator {
return &ContainerPatchValidator{}
}

func (h *ContainerPatchValidator) InjectDecoder(d *admission.Decoder) error {
return nil
}

func (h *ContainerPatchValidator) Handle(ctx context.Context, req admission.Request) admission.Response {
if req.Namespace != h.SystemNamespace {
return admission.Denied("ContainerPatch can only be placed in " + h.SystemNamespace + " namespace. It can be however referenced by pods in all namespaces")
}
return admission.Allowed("")
}

func (h *ContainerPatchValidator) Supports(req admission.Request) bool {
gvk := mesh_k8s.GroupVersion.WithKind("ContainerPatch")
return req.Kind.Kind == gvk.Kind && req.Kind.Version == gvk.Version && req.Kind.Group == gvk.Group
}
5 changes: 4 additions & 1 deletion pkg/plugins/runtime/k8s/webhooks/injector/injector.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func New(
client kube_client.Client,
converter k8s_common.Converter,
envoyAdminPort uint32,
systemNamespace string,
) (*KumaInjector, error) {
var caCert string
if cfg.CaCertFile != "" {
Expand All @@ -55,6 +56,7 @@ func New(
defaultAdminPort: envoyAdminPort,
proxyFactory: containers.NewDataplaneProxyFactory(controlPlaneURL, caCert, envoyAdminPort,
cfg.SidecarContainer.DataplaneContainer, cfg.BuiltinDNS),
systemNamespace: systemNamespace,
}, nil
}

Expand All @@ -64,6 +66,7 @@ type KumaInjector struct {
converter k8s_common.Converter
proxyFactory *containers.DataplaneProxyFactory
defaultAdminPort uint32
systemNamespace string
}

func (i *KumaInjector) InjectKuma(ctx context.Context, pod *kube_core.Pod) error {
Expand Down Expand Up @@ -256,7 +259,7 @@ func (i *KumaInjector) loadContainerPatches(

for _, patchName := range patchNames {
containerPatch := &mesh_k8s.ContainerPatch{}
if err := i.client.Get(ctx, kube_types.NamespacedName{Namespace: ns.GetName(), Name: patchName}, containerPatch); err != nil {
if err := i.client.Get(ctx, kube_types.NamespacedName{Namespace: i.systemNamespace, Name: patchName}, containerPatch); err != nil {
return namedContainerPatches{}, namedContainerPatches{}, errors.Wrap(err, "could not get a ContainerPatch")
}
if len(containerPatch.Spec.SidecarPatch) > 0 {
Expand Down
10 changes: 8 additions & 2 deletions pkg/plugins/runtime/k8s/webhooks/injector/injector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
kube_core "k8s.io/api/core/v1"
kube_meta "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/serializer"
kube_client "sigs.k8s.io/controller-runtime/pkg/client"

Expand All @@ -23,6 +24,8 @@ import (

var _ = Describe("Injector", func() {

systemNamespace := "kuma-system"

type testCase struct {
num string
mesh string
Expand All @@ -31,11 +34,14 @@ var _ = Describe("Injector", func() {
}

BeforeAll(func() {
err := k8sClient.Create(context.Background(), &kube_core.Namespace{ObjectMeta: kube_meta.ObjectMeta{Name: systemNamespace}})
Expect(err).ToNot(HaveOccurred())

cPatch := `
apiVersion: kuma.io/v1alpha1
kind: ContainerPatch
metadata:
namespace: default
namespace: kuma-system
name: container-patch-1
spec:
sidecarPatch:
Expand Down Expand Up @@ -71,7 +77,7 @@ spec:
var cfg conf.Injector
Expect(config.Load(filepath.Join("testdata", given.cfgFile), &cfg)).To(Succeed())
cfg.CaCertFile = filepath.Join("..", "..", "..", "..", "..", "..", "test", "certs", "server-cert.pem")
injector, err := inject.New(cfg, "http://kuma-control-plane.kuma-system:5681", k8sClient, k8s.NewSimpleConverter(), 9901)
injector, err := inject.New(cfg, "http://kuma-control-plane.kuma-system:5681", k8sClient, k8s.NewSimpleConverter(), 9901, systemNamespace)
Expect(err).ToNot(HaveOccurred())

// and create mesh
Expand Down
12 changes: 10 additions & 2 deletions test/e2e_env/kubernetes/container_patch/container_patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ spec:
BeforeAll(func() {
err := NewClusterSetup().
Install(NamespaceWithSidecarInjection(namespace)).
Install(YamlK8s(containerPatch(namespace))).
Install(YamlK8s(containerPatch2(namespace))).
Install(YamlK8s(containerPatch(Config.KumaNamespace))).
Install(YamlK8s(containerPatch2(Config.KumaNamespace))).
Install(MeshKubernetes(mesh)).
Install(testserver.Install(
testserver.WithNamespace(namespace),
Expand Down Expand Up @@ -101,4 +101,12 @@ spec:
// kuma-sidecar container should have value *true
Expect(pod.Spec.Containers[1].SecurityContext.Privileged).To(Equal(pointerTrue))
})

It("should reject ContainerPatch in non-system namespace", func() {
// when
err := k8s.KubectlApplyFromStringE(env.Cluster.GetTesting(), env.Cluster.GetKubectlOptions(), containerPatch(namespace))

// then
Expect(err).To(HaveOccurred())
})
}

0 comments on commit adfae5c

Please sign in to comment.