From faa3ecf08d6a0dc4a6a0e892942c68a7ef451aaf Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Wed, 6 Jul 2022 11:28:14 -0700 Subject: [PATCH] Peering webhooks (#1310) - adds validation webhooks for PeeringAcceptor and Peering dialer controllers - fixes panic by doing a nil check on the PeeringAcceptor SecretRef() --- CHANGELOG.md | 1 + ...t-inject-mutatingwebhookconfiguration.yaml | 44 +++++ ...t-inject-mutatingwebhookconfiguration.bats | 18 ++ .../api/v1alpha1/peeringacceptor_types.go | 38 +++++ .../v1alpha1/peeringacceptor_types_test.go | 87 ++++++++++ .../api/v1alpha1/peeringacceptor_webhook.go | 69 ++++++++ .../v1alpha1/peeringacceptor_webhook_test.go | 159 ++++++++++++++++++ .../api/v1alpha1/peeringdialer_types.go | 37 ++++ .../api/v1alpha1/peeringdialer_types_test.go | 87 ++++++++++ .../api/v1alpha1/peeringdialer_webhook.go | 67 ++++++++ .../v1alpha1/peeringdialer_webhook_test.go | 159 ++++++++++++++++++ control-plane/config/webhook/manifests.yaml | 42 +++++ .../peering_acceptor_controller.go | 2 +- .../subcommand/inject-connect/command.go | 13 ++ 14 files changed, 822 insertions(+), 1 deletion(-) create mode 100644 control-plane/api/v1alpha1/peeringacceptor_types_test.go create mode 100644 control-plane/api/v1alpha1/peeringacceptor_webhook.go create mode 100644 control-plane/api/v1alpha1/peeringacceptor_webhook_test.go create mode 100644 control-plane/api/v1alpha1/peeringdialer_types_test.go create mode 100644 control-plane/api/v1alpha1/peeringdialer_webhook.go create mode 100644 control-plane/api/v1alpha1/peeringdialer_webhook_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ed3f02f5f..0c915ecd66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ FEATURES: * [Experimental] Cluster Peering: * Add support for secret watchers on the Peering Acceptor and Peering Dialer controllers. [[GH-1284](https://github.com/hashicorp/consul-k8s/pull/1284)] * Add support for version annotation on the Peering Acceptor and Peering Dialer controllers. [[GH-1302](https://github.com/hashicorp/consul-k8s/pull/1302)] + * Add validation webhooks for the Peering Acceptor and Peering Dialer CRDs [[GH-1310](https://github.com/hashicorp/consul-k8s/pull/1310)] IMPROVEMENTS: * Control Plane diff --git a/charts/consul/templates/connect-inject-mutatingwebhookconfiguration.yaml b/charts/consul/templates/connect-inject-mutatingwebhookconfiguration.yaml index c3164bab24..cd5bc8c290 100644 --- a/charts/consul/templates/connect-inject-mutatingwebhookconfiguration.yaml +++ b/charts/consul/templates/connect-inject-mutatingwebhookconfiguration.yaml @@ -38,4 +38,48 @@ webhooks: namespaceSelector: {{ tpl .Values.connectInject.namespaceSelector . | indent 6 }} {{- end }} +{{- if .Values.global.peering.enabled }} + - name: {{ template "consul.fullname" . }}-mutate-peeringacceptors.consul.hashicorp.com + clientConfig: + service: + name: {{ template "consul.fullname" . }}-connect-injector + namespace: {{ .Release.Namespace }} + path: "/mutate-v1alpha1-peeringacceptors" + rules: + - apiGroups: + - consul.hashicorp.com + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - peeringacceptors + failurePolicy: Fail + sideEffects: None + admissionReviewVersions: + - "v1beta1" + - "v1" + - name: {{ template "consul.fullname" . }}-mutate-peeringdialers.consul.hashicorp.com + clientConfig: + service: + name: {{ template "consul.fullname" . }}-connect-injector + namespace: {{ .Release.Namespace }} + path: "/mutate-v1alpha1-peeringdialers" + rules: + - apiGroups: + - consul.hashicorp.com + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - peeringdialers + failurePolicy: Fail + sideEffects: None + admissionReviewVersions: + - "v1beta1" + - "v1" +{{- end }} {{- end }} diff --git a/charts/consul/test/unit/connect-inject-mutatingwebhookconfiguration.bats b/charts/consul/test/unit/connect-inject-mutatingwebhookconfiguration.bats index 11c3a6b0a5..6745e690c3 100755 --- a/charts/consul/test/unit/connect-inject-mutatingwebhookconfiguration.bats +++ b/charts/consul/test/unit/connect-inject-mutatingwebhookconfiguration.bats @@ -47,3 +47,21 @@ load _helpers yq '.webhooks[0].clientConfig.service.namespace' | tee /dev/stderr) [ "${actual}" = "\"foo\"" ] } + +@test "connectInject/MutatingWebhookConfiguration: peering is enabled, so webhooks for peering exist" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/connect-inject-mutatingwebhookconfiguration.yaml \ + --set 'connectInject.enabled=true' \ + --set 'global.peering.enabled=true' \ + . | tee /dev/stderr | + yq '.webhooks[1].name | contains("peeringacceptors.consul.hashicorp.com")' | tee /dev/stderr) + [ "${actual}" = "true" ] + local actual=$(helm template \ + -s templates/connect-inject-mutatingwebhookconfiguration.yaml \ + --set 'connectInject.enabled=true' \ + --set 'global.peering.enabled=true' \ + . | tee /dev/stderr | + yq '.webhooks[2].name | contains("peeringdialers.consul.hashicorp.com")' | tee /dev/stderr) + [ "${actual}" = "true" ] +} diff --git a/control-plane/api/v1alpha1/peeringacceptor_types.go b/control-plane/api/v1alpha1/peeringacceptor_types.go index 5dd70b0597..ad671a9e96 100644 --- a/control-plane/api/v1alpha1/peeringacceptor_types.go +++ b/control-plane/api/v1alpha1/peeringacceptor_types.go @@ -1,11 +1,17 @@ package v1alpha1 import ( + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" ) // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. +const PeeringAcceptorKubeKind = "peeringacceptors" +const SecretBackendTypeKubernetes = "kubernetes" + func init() { SchemeBuilder.Register(&PeeringAcceptor{}, &PeeringAcceptorList{}) } @@ -86,3 +92,35 @@ func (pa *PeeringAcceptor) Secret() *Secret { func (pa *PeeringAcceptor) SecretRef() *SecretRefStatus { return pa.Status.SecretRef } +func (pa *PeeringAcceptor) KubeKind() string { + return PeeringAcceptorKubeKind +} +func (pa *PeeringAcceptor) KubernetesName() string { + return pa.ObjectMeta.Name +} +func (pa *PeeringAcceptor) Validate() error { + var errs field.ErrorList + // The nil checks must return since you can't do further validations. + if pa.Spec.Peer == nil { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("peer"), pa.Spec.Peer, "peer must be specified")) + return apierrors.NewInvalid( + schema.GroupKind{Group: ConsulHashicorpGroup, Kind: PeeringAcceptorKubeKind}, + pa.KubernetesName(), errs) + } + if pa.Spec.Peer.Secret == nil { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("peer").Child("secret"), pa.Spec.Peer.Secret, "secret must be specified")) + return apierrors.NewInvalid( + schema.GroupKind{Group: ConsulHashicorpGroup, Kind: PeeringAcceptorKubeKind}, + pa.KubernetesName(), errs) + } + // Currently, the only supported backend is "kubernetes". + if pa.Spec.Peer.Secret.Backend != SecretBackendTypeKubernetes { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("peer").Child("secret").Child("backend"), pa.Spec.Peer.Secret.Backend, `backend must be "kubernetes"`)) + } + if len(errs) > 0 { + return apierrors.NewInvalid( + schema.GroupKind{Group: ConsulHashicorpGroup, Kind: PeeringAcceptorKubeKind}, + pa.KubernetesName(), errs) + } + return nil +} diff --git a/control-plane/api/v1alpha1/peeringacceptor_types_test.go b/control-plane/api/v1alpha1/peeringacceptor_types_test.go new file mode 100644 index 0000000000..33d437f46a --- /dev/null +++ b/control-plane/api/v1alpha1/peeringacceptor_types_test.go @@ -0,0 +1,87 @@ +package v1alpha1 + +import ( + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestPeeringAcceptor_Validate(t *testing.T) { + cases := map[string]struct { + acceptor *PeeringAcceptor + expectedErrMsgs []string + }{ + "valid": { + acceptor: &PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "api", + }, + Spec: PeeringAcceptorSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "api-token", + Key: "data", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }, + }, + "no peer specified": { + acceptor: &PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "api", + }, + Spec: PeeringAcceptorSpec{}, + }, + expectedErrMsgs: []string{ + `spec.peer: Invalid value: "null": peer must be specified`, + }, + }, + "no secret specified": { + acceptor: &PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "api", + }, + Spec: PeeringAcceptorSpec{ + Peer: &Peer{}, + }, + }, + expectedErrMsgs: []string{ + `spec.peer.secret: Invalid value: "null": secret must be specified`, + }, + }, + "invalid secret backend": { + acceptor: &PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "api", + }, + Spec: PeeringAcceptorSpec{ + Peer: &Peer{ + Secret: &Secret{ + Backend: "invalid", + }, + }, + }, + }, + expectedErrMsgs: []string{ + `spec.peer.secret.backend: Invalid value: "invalid": backend must be "kubernetes"`, + }, + }, + } + + for name, testCase := range cases { + t.Run(name, func(t *testing.T) { + err := testCase.acceptor.Validate() + if len(testCase.expectedErrMsgs) != 0 { + require.Error(t, err) + for _, s := range testCase.expectedErrMsgs { + require.Contains(t, err.Error(), s) + } + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/control-plane/api/v1alpha1/peeringacceptor_webhook.go b/control-plane/api/v1alpha1/peeringacceptor_webhook.go new file mode 100644 index 0000000000..728bd205ee --- /dev/null +++ b/control-plane/api/v1alpha1/peeringacceptor_webhook.go @@ -0,0 +1,69 @@ +package v1alpha1 + +import ( + "context" + "fmt" + "net/http" + + "github.com/go-logr/logr" + capi "github.com/hashicorp/consul/api" + admissionv1 "k8s.io/api/admission/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// +kubebuilder:object:generate=false + +type PeeringAcceptorWebhook struct { + client.Client + ConsulClient *capi.Client + Logger logr.Logger + decoder *admission.Decoder + //ConsulMeta common.ConsulMeta +} + +// NOTE: The path value in the below line is the path to the webhook. +// If it is updated, run code-gen, update subcommand/controller/command.go +// and the consul-helm value for the path to the webhook. +// +// NOTE: The below line cannot be combined with any other comment. If it is +// it will break the code generation. +// +// +kubebuilder:webhook:verbs=create;update,path=/mutate-v1alpha1-peeringacceptors,mutating=true,failurePolicy=fail,groups=consul.hashicorp.com,resources=peeringacceptors,versions=v1alpha1,name=mutate-peeringacceptors.consul.hashicorp.com,sideEffects=None,admissionReviewVersions=v1beta1;v1 + +func (v *PeeringAcceptorWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { + var acceptor PeeringAcceptor + var acceptorList PeeringAcceptorList + err := v.decoder.Decode(req, &acceptor) + if err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + // Call validate first to ensure all the fields are validated before checking for secret name duplicates. + if err := acceptor.Validate(); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + if req.Operation == admissionv1.Create { + v.Logger.Info("validate create", "name", acceptor.KubernetesName()) + + if err := v.Client.List(ctx, &acceptorList); err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + for _, item := range acceptorList.Items { + // If any peering acceptor resource has the same secret name as this one, reject it. + if item.Namespace == acceptor.Namespace && item.Secret().Name == acceptor.Secret().Name { + return admission.Errored(http.StatusBadRequest, + fmt.Errorf("an existing PeeringAcceptor resource has the same secret name `name: %s, namespace: %s`", acceptor.Secret().Name, acceptor.Namespace)) + } + } + } + + return admission.Allowed(fmt.Sprintf("valid %s request", acceptor.KubeKind())) +} + +func (v *PeeringAcceptorWebhook) InjectDecoder(d *admission.Decoder) error { + v.decoder = d + return nil +} diff --git a/control-plane/api/v1alpha1/peeringacceptor_webhook_test.go b/control-plane/api/v1alpha1/peeringacceptor_webhook_test.go new file mode 100644 index 0000000000..26ed3e2150 --- /dev/null +++ b/control-plane/api/v1alpha1/peeringacceptor_webhook_test.go @@ -0,0 +1,159 @@ +package v1alpha1 + +import ( + "context" + "encoding/json" + "testing" + + logrtest "github.com/go-logr/logr/testing" + "github.com/stretchr/testify/require" + admissionv1 "k8s.io/api/admission/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +func TestValidatePeeringAcceptor(t *testing.T) { + cases := map[string]struct { + existingResources []runtime.Object + newResource *PeeringAcceptor + expAllow bool + expErrMessage string + }{ + "valid, unique secret name": { + existingResources: []runtime.Object{&PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer1", + Namespace: "default", + }, + Spec: PeeringAcceptorSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }}, + newResource: &PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer2", + Namespace: "default", + }, + Spec: PeeringAcceptorSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo2", + Key: "data2", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }, + expAllow: true, + }, + "valid, same secret name, different namespace": { + existingResources: []runtime.Object{&PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer1", + Namespace: "default", + }, + Spec: PeeringAcceptorSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }}, + newResource: &PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer2", + Namespace: "other", + }, + Spec: PeeringAcceptorSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }, + expAllow: true, + }, + "invalid, duplicate secret name and namespace": { + existingResources: []runtime.Object{&PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer1", + Namespace: "default", + }, + Spec: PeeringAcceptorSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }}, + newResource: &PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer2", + Namespace: "default", + }, + Spec: PeeringAcceptorSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }, + expAllow: false, + expErrMessage: "an existing PeeringAcceptor resource has the same secret name `name: foo, namespace: default`", + }, + } + for name, c := range cases { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + marshalledRequestObject, err := json.Marshal(c.newResource) + require.NoError(t, err) + s := runtime.NewScheme() + s.AddKnownTypes(GroupVersion, &PeeringAcceptor{}, &PeeringAcceptorList{}) + client := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(c.existingResources...).Build() + decoder, err := admission.NewDecoder(s) + require.NoError(t, err) + + validator := &PeeringAcceptorWebhook{ + Client: client, + ConsulClient: nil, + Logger: logrtest.TestLogger{T: t}, + decoder: decoder, + } + response := validator.Handle(ctx, admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: c.newResource.KubernetesName(), + Namespace: "default", + Operation: admissionv1.Create, + Object: runtime.RawExtension{ + Raw: marshalledRequestObject, + }, + }, + }) + + require.Equal(t, c.expAllow, response.Allowed) + if c.expErrMessage != "" { + require.Equal(t, c.expErrMessage, response.AdmissionResponse.Result.Message) + } + }) + } +} diff --git a/control-plane/api/v1alpha1/peeringdialer_types.go b/control-plane/api/v1alpha1/peeringdialer_types.go index 0ffea7bee4..4998b27543 100644 --- a/control-plane/api/v1alpha1/peeringdialer_types.go +++ b/control-plane/api/v1alpha1/peeringdialer_types.go @@ -1,11 +1,16 @@ package v1alpha1 import ( + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" ) // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. +const PeeringDialerKubeKind = "peeringdialers" + func init() { SchemeBuilder.Register(&PeeringDialer{}, &PeeringDialerList{}) } @@ -62,3 +67,35 @@ func (pd *PeeringDialer) Secret() *Secret { func (pd *PeeringDialer) SecretRef() *SecretRefStatus { return pd.Status.SecretRef } +func (pd *PeeringDialer) KubeKind() string { + return PeeringDialerKubeKind +} +func (pd *PeeringDialer) KubernetesName() string { + return pd.ObjectMeta.Name +} +func (pd *PeeringDialer) Validate() error { + var errs field.ErrorList + // The nil checks must return since you can't do further validations. + if pd.Spec.Peer == nil { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("peer"), pd.Spec.Peer, "peer must be specified")) + return apierrors.NewInvalid( + schema.GroupKind{Group: ConsulHashicorpGroup, Kind: PeeringDialerKubeKind}, + pd.KubernetesName(), errs) + } + if pd.Spec.Peer.Secret == nil { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("peer").Child("secret"), pd.Spec.Peer.Secret, "secret must be specified")) + return apierrors.NewInvalid( + schema.GroupKind{Group: ConsulHashicorpGroup, Kind: PeeringDialerKubeKind}, + pd.KubernetesName(), errs) + } + // Currently, the only supported backend is "kubernetes". + if pd.Spec.Peer.Secret.Backend != "kubernetes" { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("peer").Child("secret").Child("backend"), pd.Spec.Peer.Secret.Backend, `backend must be "kubernetes"`)) + } + if len(errs) > 0 { + return apierrors.NewInvalid( + schema.GroupKind{Group: ConsulHashicorpGroup, Kind: PeeringDialerKubeKind}, + pd.KubernetesName(), errs) + } + return nil +} diff --git a/control-plane/api/v1alpha1/peeringdialer_types_test.go b/control-plane/api/v1alpha1/peeringdialer_types_test.go new file mode 100644 index 0000000000..7e358facb8 --- /dev/null +++ b/control-plane/api/v1alpha1/peeringdialer_types_test.go @@ -0,0 +1,87 @@ +package v1alpha1 + +import ( + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestPeeringDialer_Validate(t *testing.T) { + cases := map[string]struct { + dialer *PeeringDialer + expectedErrMsgs []string + }{ + "valid": { + dialer: &PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "api", + }, + Spec: PeeringDialerSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "api-token", + Key: "data", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }, + }, + "no peer specified": { + dialer: &PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "api", + }, + Spec: PeeringDialerSpec{}, + }, + expectedErrMsgs: []string{ + `spec.peer: Invalid value: "null": peer must be specified`, + }, + }, + "no secret specified": { + dialer: &PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "api", + }, + Spec: PeeringDialerSpec{ + Peer: &Peer{}, + }, + }, + expectedErrMsgs: []string{ + `spec.peer.secret: Invalid value: "null": secret must be specified`, + }, + }, + "invalid secret backend": { + dialer: &PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "api", + }, + Spec: PeeringDialerSpec{ + Peer: &Peer{ + Secret: &Secret{ + Backend: "invalid", + }, + }, + }, + }, + expectedErrMsgs: []string{ + `spec.peer.secret.backend: Invalid value: "invalid": backend must be "kubernetes"`, + }, + }, + } + + for name, testCase := range cases { + t.Run(name, func(t *testing.T) { + err := testCase.dialer.Validate() + if len(testCase.expectedErrMsgs) != 0 { + require.Error(t, err) + for _, s := range testCase.expectedErrMsgs { + require.Contains(t, err.Error(), s) + } + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/control-plane/api/v1alpha1/peeringdialer_webhook.go b/control-plane/api/v1alpha1/peeringdialer_webhook.go new file mode 100644 index 0000000000..587f998155 --- /dev/null +++ b/control-plane/api/v1alpha1/peeringdialer_webhook.go @@ -0,0 +1,67 @@ +package v1alpha1 + +import ( + "context" + "fmt" + "net/http" + + "github.com/go-logr/logr" + capi "github.com/hashicorp/consul/api" + admissionv1 "k8s.io/api/admission/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// +kubebuilder:object:generate=false + +type PeeringDialerWebhook struct { + client.Client + ConsulClient *capi.Client + Logger logr.Logger + decoder *admission.Decoder +} + +// NOTE: The path value in the below line is the path to the webhook. +// If it is updated, run code-gen, update subcommand/controller/command.go +// and the consul-helm value for the path to the webhook. +// +// NOTE: The below line cannot be combined with any other comment. If it is +// it will break the code generation. +// +// +kubebuilder:webhook:verbs=create;update,path=/mutate-v1alpha1-peeringdialers,mutating=true,failurePolicy=fail,groups=consul.hashicorp.com,resources=peeringdialers,versions=v1alpha1,name=mutate-peeringdialers.consul.hashicorp.com,sideEffects=None,admissionReviewVersions=v1beta1;v1 + +func (v *PeeringDialerWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { + var dialer PeeringDialer + var dialerList PeeringDialerList + err := v.decoder.Decode(req, &dialer) + if err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + // Call validate first to ensure all the fields are validated before checking for secret name duplicates. + if err := dialer.Validate(); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + if req.Operation == admissionv1.Create { + v.Logger.Info("validate create", "name", dialer.KubernetesName()) + + if err := v.Client.List(ctx, &dialerList); err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + for _, item := range dialerList.Items { + if item.Namespace == dialer.Namespace && item.Secret().Name == dialer.Secret().Name { + return admission.Errored(http.StatusBadRequest, + fmt.Errorf("an existing PeeringDialer resource has the same secret name `name: %s, namespace: %s`", dialer.Secret().Name, dialer.Namespace)) + } + } + } + + return admission.Allowed(fmt.Sprintf("valid %s request", dialer.KubeKind())) +} + +func (v *PeeringDialerWebhook) InjectDecoder(d *admission.Decoder) error { + v.decoder = d + return nil +} diff --git a/control-plane/api/v1alpha1/peeringdialer_webhook_test.go b/control-plane/api/v1alpha1/peeringdialer_webhook_test.go new file mode 100644 index 0000000000..abdca4f417 --- /dev/null +++ b/control-plane/api/v1alpha1/peeringdialer_webhook_test.go @@ -0,0 +1,159 @@ +package v1alpha1 + +import ( + "context" + "encoding/json" + "testing" + + logrtest "github.com/go-logr/logr/testing" + "github.com/stretchr/testify/require" + admissionv1 "k8s.io/api/admission/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +func TestValidatePeeringDialer(t *testing.T) { + cases := map[string]struct { + existingResources []runtime.Object + newResource *PeeringDialer + expAllow bool + expErrMessage string + }{ + "valid, unique secret name": { + existingResources: []runtime.Object{&PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer1", + Namespace: "default", + }, + Spec: PeeringDialerSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }}, + newResource: &PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer2", + Namespace: "default", + }, + Spec: PeeringDialerSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo2", + Key: "data2", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }, + expAllow: true, + }, + "valid, same secret name, different namespace": { + existingResources: []runtime.Object{&PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer1", + Namespace: "default", + }, + Spec: PeeringDialerSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }}, + newResource: &PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer2", + Namespace: "other", + }, + Spec: PeeringDialerSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }, + expAllow: true, + }, + "invalid, duplicate secret name and namespace": { + existingResources: []runtime.Object{&PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer1", + Namespace: "default", + }, + Spec: PeeringDialerSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }}, + newResource: &PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer2", + Namespace: "default", + }, + Spec: PeeringDialerSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: SecretBackendTypeKubernetes, + }, + }, + }, + }, + expAllow: false, + expErrMessage: "an existing PeeringDialer resource has the same secret name `name: foo, namespace: default`", + }, + } + for name, c := range cases { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + marshalledRequestObject, err := json.Marshal(c.newResource) + require.NoError(t, err) + s := runtime.NewScheme() + s.AddKnownTypes(GroupVersion, &PeeringDialer{}, &PeeringDialerList{}) + client := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(c.existingResources...).Build() + decoder, err := admission.NewDecoder(s) + require.NoError(t, err) + + validator := &PeeringDialerWebhook{ + Client: client, + ConsulClient: nil, + Logger: logrtest.TestLogger{T: t}, + decoder: decoder, + } + response := validator.Handle(ctx, admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: c.newResource.KubernetesName(), + Namespace: "default", + Operation: admissionv1.Create, + Object: runtime.RawExtension{ + Raw: marshalledRequestObject, + }, + }, + }) + + require.Equal(t, c.expAllow, response.Allowed) + if c.expErrMessage != "" { + require.Equal(t, c.expErrMessage, response.AdmissionResponse.Result.Message) + } + }) + } +} diff --git a/control-plane/config/webhook/manifests.yaml b/control-plane/config/webhook/manifests.yaml index 013ec87f80..c1ffb5e0fe 100644 --- a/control-plane/config/webhook/manifests.yaml +++ b/control-plane/config/webhook/manifests.yaml @@ -68,6 +68,48 @@ webhooks: resources: - mesh sideEffects: None +- admissionReviewVersions: + - v1beta1 + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-v1alpha1-peeringacceptors + failurePolicy: Fail + name: mutate-peeringacceptors.consul.hashicorp.com + rules: + - apiGroups: + - consul.hashicorp.com + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - peeringacceptors + sideEffects: None +- admissionReviewVersions: + - v1beta1 + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-v1alpha1-peeringdialers + failurePolicy: Fail + name: mutate-peeringdialers.consul.hashicorp.com + rules: + - apiGroups: + - consul.hashicorp.com + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - peeringdialers + sideEffects: None - admissionReviewVersions: - v1beta1 - v1 diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index 680dc5cc5f..dff8651fd5 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -388,7 +388,7 @@ func (r *PeeringAcceptorController) requestsForPeeringTokens(object client.Objec return []ctrl.Request{} } for _, acceptor := range acceptorList.Items { - if acceptor.SecretRef().Backend == "kubernetes" { + if acceptor.SecretRef() != nil && acceptor.SecretRef().Backend == "kubernetes" { if acceptor.SecretRef().Name == object.GetName() && acceptor.Namespace == object.GetNamespace() { return []ctrl.Request{{NamespacedName: types.NamespacedName{Namespace: acceptor.Namespace, Name: acceptor.Name}}} } diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index 643e5705fd..059bfd6769 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -459,6 +459,19 @@ func (c *Command) Run(args []string) int { setupLog.Error(err, "unable to create controller", "controller", "peering-dialer") return 1 } + + mgr.GetWebhookServer().Register("/mutate-v1alpha1-peeringacceptors", + &webhook.Admission{Handler: &v1alpha1.PeeringAcceptorWebhook{ + Client: mgr.GetClient(), + ConsulClient: c.consulClient, + Logger: ctrl.Log.WithName("webhooks").WithName("peering-acceptor"), + }}) + mgr.GetWebhookServer().Register("/mutate-v1alpha1-peeringdialers", + &webhook.Admission{Handler: &v1alpha1.PeeringDialerWebhook{ + Client: mgr.GetClient(), + ConsulClient: c.consulClient, + Logger: ctrl.Log.WithName("webhooks").WithName("peering-dialer"), + }}) } mgr.GetWebhookServer().CertDir = c.flagCertDir