From 400990869b575664e3e3ff65a07d661abf3cea53 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Sat, 25 Jun 2022 14:57:03 -0700 Subject: [PATCH 1/9] wip --- .../api/v1alpha1/peeringacceptor_types.go | 12 ++++ .../api/v1alpha1/peeringacceptor_webhook.go | 67 +++++++++++++++++++ .../subcommand/inject-connect/command.go | 17 +++++ 3 files changed, 96 insertions(+) create mode 100644 control-plane/api/v1alpha1/peeringacceptor_webhook.go diff --git a/control-plane/api/v1alpha1/peeringacceptor_types.go b/control-plane/api/v1alpha1/peeringacceptor_types.go index 5dd70b0597..ba9fa2a28b 100644 --- a/control-plane/api/v1alpha1/peeringacceptor_types.go +++ b/control-plane/api/v1alpha1/peeringacceptor_types.go @@ -1,11 +1,14 @@ package v1alpha1 import ( + "github.com/hashicorp/consul-k8s/control-plane/api/common" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. +const PeeringAcceptorKubeKind = "peeringacceptor" + func init() { SchemeBuilder.Register(&PeeringAcceptor{}, &PeeringAcceptorList{}) } @@ -86,3 +89,12 @@ 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(consulMeta common.ConsulMeta) error { + return nil +} diff --git a/control-plane/api/v1alpha1/peeringacceptor_webhook.go b/control-plane/api/v1alpha1/peeringacceptor_webhook.go new file mode 100644 index 0000000000..55d300076d --- /dev/null +++ b/control-plane/api/v1alpha1/peeringacceptor_webhook.go @@ -0,0 +1,67 @@ +package v1alpha1 + +import ( + "context" + "fmt" + "net/http" + + "github.com/go-logr/logr" + "github.com/hashicorp/consul-k8s/control-plane/api/common" + 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-peeringacceptor,mutating=true,failurePolicy=fail,groups=consul.hashicorp.com,resources=peeringacceptor,versions=v1alpha1,name=mutate-peeringacceptor.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) + } + + 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) + } + + if len(acceptorList.Items) == 0 { + return admission.Errored(http.StatusBadRequest, + fmt.Errorf("%s validation wh cant create resource already defined - only one exportedservices entry is supported per Kubernetes cluster", + acceptor.KubeKind())) + } + } + + if err := acceptor.Validate(v.ConsulMeta); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + 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/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index 643e5705fd..38419a877d 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -11,6 +11,7 @@ import ( "strings" "sync" + apiCommon "github.com/hashicorp/consul-k8s/control-plane/api/common" "github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1" connectinject "github.com/hashicorp/consul-k8s/control-plane/connect-inject" "github.com/hashicorp/consul-k8s/control-plane/consul" @@ -459,6 +460,22 @@ func (c *Command) Run(args []string) int { setupLog.Error(err, "unable to create controller", "controller", "peering-dialer") return 1 } + + consulMeta := apiCommon.ConsulMeta{ + PartitionsEnabled: c.http.Partition() != "", + Partition: c.http.Partition(), + NamespacesEnabled: c.flagEnableNamespaces, + DestinationNamespace: c.flagConsulDestinationNamespace, + Mirroring: c.flagEnableK8SNSMirroring, + Prefix: c.flagK8SNSMirroringPrefix, + } + mgr.GetWebhookServer().Register("/mutate", + &webhook.Admission{Handler: &v1alpha1.PeeringAcceptorWebhook{ + Client: mgr.GetClient(), + ConsulClient: c.consulClient, + Logger: ctrl.Log.WithName("webhooks").WithName("peering-acceptor"), + ConsulMeta: consulMeta, + }}) } mgr.GetWebhookServer().CertDir = c.flagCertDir From dc4dd58f660d554d637face8eb2ff7c7b5114349 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Sat, 25 Jun 2022 16:17:37 -0700 Subject: [PATCH 2/9] peering acceptor webhook wiring works --- ...t-inject-mutatingwebhookconfiguration.yaml | 44 ++++++++++++ .../api/v1alpha1/peeringacceptor_webhook.go | 2 +- .../api/v1alpha1/peeringdialer_types.go | 12 ++++ .../api/v1alpha1/peeringdialer_webhook.go | 67 +++++++++++++++++++ control-plane/config/webhook/manifests.yaml | 21 ++++++ .../peering_acceptor_controller.go | 8 ++- .../subcommand/inject-connect/command.go | 9 ++- 7 files changed, 158 insertions(+), 5 deletions(-) create mode 100644 control-plane/api/v1alpha1/peeringdialer_webhook.go 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/control-plane/api/v1alpha1/peeringacceptor_webhook.go b/control-plane/api/v1alpha1/peeringacceptor_webhook.go index 55d300076d..fd792caec9 100644 --- a/control-plane/api/v1alpha1/peeringacceptor_webhook.go +++ b/control-plane/api/v1alpha1/peeringacceptor_webhook.go @@ -30,7 +30,7 @@ type PeeringAcceptorWebhook struct { // 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-peeringacceptor,mutating=true,failurePolicy=fail,groups=consul.hashicorp.com,resources=peeringacceptor,versions=v1alpha1,name=mutate-peeringacceptor.consul.hashicorp.com,sideEffects=None,admissionReviewVersions=v1beta1;v1 +// +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 diff --git a/control-plane/api/v1alpha1/peeringdialer_types.go b/control-plane/api/v1alpha1/peeringdialer_types.go index 0ffea7bee4..2a4f978cf1 100644 --- a/control-plane/api/v1alpha1/peeringdialer_types.go +++ b/control-plane/api/v1alpha1/peeringdialer_types.go @@ -1,11 +1,14 @@ package v1alpha1 import ( + "github.com/hashicorp/consul-k8s/control-plane/api/common" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. +const PeeringDialerKubeKind = "peeringdialer" + func init() { SchemeBuilder.Register(&PeeringDialer{}, &PeeringDialerList{}) } @@ -62,3 +65,12 @@ 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(consulMeta common.ConsulMeta) error { + return nil +} diff --git a/control-plane/api/v1alpha1/peeringdialer_webhook.go b/control-plane/api/v1alpha1/peeringdialer_webhook.go new file mode 100644 index 0000000000..8730cf6676 --- /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" + "github.com/hashicorp/consul-k8s/control-plane/api/common" + 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 + 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-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) + } + + 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) + } + + if len(dialerList.Items) == 0 { + return admission.Errored(http.StatusBadRequest, + fmt.Errorf("%s validation wh cant create resource already defined - only one exportedservices entry is supported per Kubernetes cluster", + dialer.KubeKind())) + } + } + + if err := dialer.Validate(v.ConsulMeta); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + 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/config/webhook/manifests.yaml b/control-plane/config/webhook/manifests.yaml index 013ec87f80..d7e0ee3cd2 100644 --- a/control-plane/config/webhook/manifests.yaml +++ b/control-plane/config/webhook/manifests.yaml @@ -68,6 +68,27 @@ 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 diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index 680dc5cc5f..81fb90b9a7 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -388,9 +388,11 @@ func (r *PeeringAcceptorController) requestsForPeeringTokens(object client.Objec return []ctrl.Request{} } for _, acceptor := range acceptorList.Items { - if 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}}} + if acceptor.SecretRef() != nil { + if 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 38419a877d..ab5d068700 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -469,13 +469,20 @@ func (c *Command) Run(args []string) int { Mirroring: c.flagEnableK8SNSMirroring, Prefix: c.flagK8SNSMirroringPrefix, } - mgr.GetWebhookServer().Register("/mutate", + 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"), ConsulMeta: consulMeta, }}) + 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"), + ConsulMeta: consulMeta, + }}) } mgr.GetWebhookServer().CertDir = c.flagCertDir From c1b962d29f326c6da96676dcc2a32a8af641608e Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Sat, 25 Jun 2022 16:20:37 -0700 Subject: [PATCH 3/9] pluralize --- .../api/v1alpha1/peeringacceptor_types.go | 2 +- .../api/v1alpha1/peeringdialer_types.go | 2 +- control-plane/config/webhook/manifests.yaml | 21 +++++++++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/control-plane/api/v1alpha1/peeringacceptor_types.go b/control-plane/api/v1alpha1/peeringacceptor_types.go index ba9fa2a28b..fe7b69a8ca 100644 --- a/control-plane/api/v1alpha1/peeringacceptor_types.go +++ b/control-plane/api/v1alpha1/peeringacceptor_types.go @@ -7,7 +7,7 @@ import ( // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. -const PeeringAcceptorKubeKind = "peeringacceptor" +const PeeringAcceptorKubeKind = "peeringacceptors" func init() { SchemeBuilder.Register(&PeeringAcceptor{}, &PeeringAcceptorList{}) diff --git a/control-plane/api/v1alpha1/peeringdialer_types.go b/control-plane/api/v1alpha1/peeringdialer_types.go index 2a4f978cf1..7b26696e63 100644 --- a/control-plane/api/v1alpha1/peeringdialer_types.go +++ b/control-plane/api/v1alpha1/peeringdialer_types.go @@ -7,7 +7,7 @@ import ( // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. -const PeeringDialerKubeKind = "peeringdialer" +const PeeringDialerKubeKind = "peeringdialers" func init() { SchemeBuilder.Register(&PeeringDialer{}, &PeeringDialerList{}) diff --git a/control-plane/config/webhook/manifests.yaml b/control-plane/config/webhook/manifests.yaml index d7e0ee3cd2..c1ffb5e0fe 100644 --- a/control-plane/config/webhook/manifests.yaml +++ b/control-plane/config/webhook/manifests.yaml @@ -89,6 +89,27 @@ webhooks: 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 From 92f946a835b8e711d0c2a7c9c9d92844f9793d00 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Tue, 28 Jun 2022 22:40:44 -0700 Subject: [PATCH 4/9] add some validation for peering acceptor --- .../api/v1alpha1/peeringacceptor_types.go | 28 +++++- .../v1alpha1/peeringacceptor_types_test.go | 87 +++++++++++++++++++ .../api/v1alpha1/peeringacceptor_webhook.go | 23 ++--- 3 files changed, 126 insertions(+), 12 deletions(-) create mode 100644 control-plane/api/v1alpha1/peeringacceptor_types_test.go diff --git a/control-plane/api/v1alpha1/peeringacceptor_types.go b/control-plane/api/v1alpha1/peeringacceptor_types.go index fe7b69a8ca..87a0d2eac9 100644 --- a/control-plane/api/v1alpha1/peeringacceptor_types.go +++ b/control-plane/api/v1alpha1/peeringacceptor_types.go @@ -1,8 +1,10 @@ package v1alpha1 import ( - "github.com/hashicorp/consul-k8s/control-plane/api/common" + 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. @@ -95,6 +97,28 @@ func (pa *PeeringAcceptor) KubeKind() string { func (pa *PeeringAcceptor) KubernetesName() string { return pa.ObjectMeta.Name } -func (pa *PeeringAcceptor) Validate(consulMeta common.ConsulMeta) error { +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) + } + if pa.Spec.Peer.Secret.Backend != "kubernetes" { + 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..1f13a8ea48 --- /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: "kubernetes", + }, + }, + }, + }, + }, + "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 index fd792caec9..3e7a3517f5 100644 --- a/control-plane/api/v1alpha1/peeringacceptor_webhook.go +++ b/control-plane/api/v1alpha1/peeringacceptor_webhook.go @@ -6,7 +6,6 @@ import ( "net/http" "github.com/go-logr/logr" - "github.com/hashicorp/consul-k8s/control-plane/api/common" capi "github.com/hashicorp/consul/api" admissionv1 "k8s.io/api/admission/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -20,7 +19,7 @@ type PeeringAcceptorWebhook struct { ConsulClient *capi.Client Logger logr.Logger decoder *admission.Decoder - ConsulMeta common.ConsulMeta + //ConsulMeta common.ConsulMeta } // NOTE: The path value in the below line is the path to the webhook. @@ -40,6 +39,11 @@ func (v *PeeringAcceptorWebhook) Handle(ctx context.Context, req admission.Reque 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()) @@ -47,15 +51,14 @@ func (v *PeeringAcceptorWebhook) Handle(ctx context.Context, req admission.Reque return admission.Errored(http.StatusInternalServerError, err) } - if len(acceptorList.Items) == 0 { - return admission.Errored(http.StatusBadRequest, - fmt.Errorf("%s validation wh cant create resource already defined - only one exportedservices entry is supported per Kubernetes cluster", - acceptor.KubeKind())) - } - } + 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.Spec.Peer.Secret.Name, acceptor.Namespace)) + } - if err := acceptor.Validate(v.ConsulMeta); err != nil { - return admission.Errored(http.StatusBadRequest, err) + } } return admission.Allowed(fmt.Sprintf("valid %s request", acceptor.KubeKind())) From e51403b1f0a194f366ca8e18fe358811f2e38b2d Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Wed, 29 Jun 2022 17:27:02 -0700 Subject: [PATCH 5/9] add tests --- .../api/v1alpha1/peeringacceptor_types.go | 1 + .../v1alpha1/peeringacceptor_webhook_test.go | 159 ++++++++++++++++++ 2 files changed, 160 insertions(+) create mode 100644 control-plane/api/v1alpha1/peeringacceptor_webhook_test.go diff --git a/control-plane/api/v1alpha1/peeringacceptor_types.go b/control-plane/api/v1alpha1/peeringacceptor_types.go index 87a0d2eac9..bde0f022c7 100644 --- a/control-plane/api/v1alpha1/peeringacceptor_types.go +++ b/control-plane/api/v1alpha1/peeringacceptor_types.go @@ -112,6 +112,7 @@ func (pa *PeeringAcceptor) Validate() error { schema.GroupKind{Group: ConsulHashicorpGroup, Kind: PeeringAcceptorKubeKind}, pa.KubernetesName(), errs) } + // Currently, the only supported backend is "kubernetes". if pa.Spec.Peer.Secret.Backend != "kubernetes" { errs = append(errs, field.Invalid(field.NewPath("spec").Child("peer").Child("secret").Child("backend"), pa.Spec.Peer.Secret.Backend, `backend must be "kubernetes"`)) } 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..ed07b03494 --- /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: "kubernetes", + }, + }, + }, + }}, + newResource: &PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer2", + Namespace: "default", + }, + Spec: PeeringAcceptorSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo2", + Key: "data2", + Backend: "kubernetes", + }, + }, + }, + }, + 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: "kubernetes", + }, + }, + }, + }}, + newResource: &PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer2", + Namespace: "other", + }, + Spec: PeeringAcceptorSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: "kubernetes", + }, + }, + }, + }, + expAllow: true, + }, + "invalid, 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: "kubernetes", + }, + }, + }, + }}, + newResource: &PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer2", + Namespace: "default", + }, + Spec: PeeringAcceptorSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: "kubernetes", + }, + }, + }, + }, + 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) + } + }) + } +} From 68c22b367e21d02918147cdd55ee48a008f54622 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Wed, 29 Jun 2022 17:44:12 -0700 Subject: [PATCH 6/9] remove consul meta, add dialer webhook tests --- .../api/v1alpha1/peeringacceptor_webhook.go | 3 +- .../api/v1alpha1/peeringdialer_types.go | 29 +++- .../api/v1alpha1/peeringdialer_types_test.go | 87 ++++++++++ .../api/v1alpha1/peeringdialer_webhook.go | 20 +-- .../v1alpha1/peeringdialer_webhook_test.go | 159 ++++++++++++++++++ .../subcommand/inject-connect/command.go | 11 -- 6 files changed, 284 insertions(+), 25 deletions(-) create mode 100644 control-plane/api/v1alpha1/peeringdialer_types_test.go create mode 100644 control-plane/api/v1alpha1/peeringdialer_webhook_test.go diff --git a/control-plane/api/v1alpha1/peeringacceptor_webhook.go b/control-plane/api/v1alpha1/peeringacceptor_webhook.go index 3e7a3517f5..728bd205ee 100644 --- a/control-plane/api/v1alpha1/peeringacceptor_webhook.go +++ b/control-plane/api/v1alpha1/peeringacceptor_webhook.go @@ -55,9 +55,8 @@ func (v *PeeringAcceptorWebhook) Handle(ctx context.Context, req admission.Reque // 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.Spec.Peer.Secret.Name, acceptor.Namespace)) + fmt.Errorf("an existing PeeringAcceptor resource has the same secret name `name: %s, namespace: %s`", acceptor.Secret().Name, acceptor.Namespace)) } - } } diff --git a/control-plane/api/v1alpha1/peeringdialer_types.go b/control-plane/api/v1alpha1/peeringdialer_types.go index 7b26696e63..4998b27543 100644 --- a/control-plane/api/v1alpha1/peeringdialer_types.go +++ b/control-plane/api/v1alpha1/peeringdialer_types.go @@ -1,8 +1,10 @@ package v1alpha1 import ( - "github.com/hashicorp/consul-k8s/control-plane/api/common" + 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. @@ -71,6 +73,29 @@ func (pd *PeeringDialer) KubeKind() string { func (pd *PeeringDialer) KubernetesName() string { return pd.ObjectMeta.Name } -func (pd *PeeringDialer) Validate(consulMeta common.ConsulMeta) error { +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..8af724a09c --- /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: "kubernetes", + }, + }, + }, + }, + }, + "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 index 8730cf6676..587f998155 100644 --- a/control-plane/api/v1alpha1/peeringdialer_webhook.go +++ b/control-plane/api/v1alpha1/peeringdialer_webhook.go @@ -6,7 +6,6 @@ import ( "net/http" "github.com/go-logr/logr" - "github.com/hashicorp/consul-k8s/control-plane/api/common" capi "github.com/hashicorp/consul/api" admissionv1 "k8s.io/api/admission/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -20,7 +19,6 @@ type PeeringDialerWebhook struct { 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. @@ -40,6 +38,11 @@ func (v *PeeringDialerWebhook) Handle(ctx context.Context, req admission.Request 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()) @@ -47,17 +50,14 @@ func (v *PeeringDialerWebhook) Handle(ctx context.Context, req admission.Request return admission.Errored(http.StatusInternalServerError, err) } - if len(dialerList.Items) == 0 { - return admission.Errored(http.StatusBadRequest, - fmt.Errorf("%s validation wh cant create resource already defined - only one exportedservices entry is supported per Kubernetes cluster", - dialer.KubeKind())) + 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)) + } } } - if err := dialer.Validate(v.ConsulMeta); err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - return admission.Allowed(fmt.Sprintf("valid %s request", dialer.KubeKind())) } 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..8112c425aa --- /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: "kubernetes", + }, + }, + }, + }}, + newResource: &PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer2", + Namespace: "default", + }, + Spec: PeeringDialerSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo2", + Key: "data2", + Backend: "kubernetes", + }, + }, + }, + }, + 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: "kubernetes", + }, + }, + }, + }}, + newResource: &PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer2", + Namespace: "other", + }, + Spec: PeeringDialerSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: "kubernetes", + }, + }, + }, + }, + expAllow: true, + }, + "invalid, 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: "kubernetes", + }, + }, + }, + }}, + newResource: &PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peer2", + Namespace: "default", + }, + Spec: PeeringDialerSpec{ + Peer: &Peer{ + Secret: &Secret{ + Name: "foo", + Key: "data", + Backend: "kubernetes", + }, + }, + }, + }, + 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/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index ab5d068700..059bfd6769 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -11,7 +11,6 @@ import ( "strings" "sync" - apiCommon "github.com/hashicorp/consul-k8s/control-plane/api/common" "github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1" connectinject "github.com/hashicorp/consul-k8s/control-plane/connect-inject" "github.com/hashicorp/consul-k8s/control-plane/consul" @@ -461,27 +460,17 @@ func (c *Command) Run(args []string) int { return 1 } - consulMeta := apiCommon.ConsulMeta{ - PartitionsEnabled: c.http.Partition() != "", - Partition: c.http.Partition(), - NamespacesEnabled: c.flagEnableNamespaces, - DestinationNamespace: c.flagConsulDestinationNamespace, - Mirroring: c.flagEnableK8SNSMirroring, - Prefix: c.flagK8SNSMirroringPrefix, - } 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"), - ConsulMeta: consulMeta, }}) 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"), - ConsulMeta: consulMeta, }}) } From a73c0f4ea0ef6921a85eedb5249b604ac4067d41 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Wed, 29 Jun 2022 18:07:40 -0700 Subject: [PATCH 7/9] add bats test --- ...ct-inject-mutatingwebhookconfiguration.bats | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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" ] +} From 92042e6f6ac99173b72e7e011cfbe7098bca27d3 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Thu, 30 Jun 2022 10:25:44 -0700 Subject: [PATCH 8/9] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index efd3bb6015..4b6cabdd5e 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 From dd59aa7a6eb2a9a6356aedda51f7eaad8454625b Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Wed, 6 Jul 2022 09:55:43 -0700 Subject: [PATCH 9/9] constant and change test descriptions --- .../api/v1alpha1/peeringacceptor_types.go | 3 ++- .../api/v1alpha1/peeringacceptor_types_test.go | 2 +- .../api/v1alpha1/peeringacceptor_webhook_test.go | 14 +++++++------- .../api/v1alpha1/peeringdialer_types_test.go | 2 +- .../api/v1alpha1/peeringdialer_webhook_test.go | 14 +++++++------- .../connect-inject/peering_acceptor_controller.go | 8 +++----- 6 files changed, 21 insertions(+), 22 deletions(-) diff --git a/control-plane/api/v1alpha1/peeringacceptor_types.go b/control-plane/api/v1alpha1/peeringacceptor_types.go index bde0f022c7..ad671a9e96 100644 --- a/control-plane/api/v1alpha1/peeringacceptor_types.go +++ b/control-plane/api/v1alpha1/peeringacceptor_types.go @@ -10,6 +10,7 @@ import ( // 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{}) @@ -113,7 +114,7 @@ func (pa *PeeringAcceptor) Validate() error { pa.KubernetesName(), errs) } // Currently, the only supported backend is "kubernetes". - if pa.Spec.Peer.Secret.Backend != "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 { diff --git a/control-plane/api/v1alpha1/peeringacceptor_types_test.go b/control-plane/api/v1alpha1/peeringacceptor_types_test.go index 1f13a8ea48..33d437f46a 100644 --- a/control-plane/api/v1alpha1/peeringacceptor_types_test.go +++ b/control-plane/api/v1alpha1/peeringacceptor_types_test.go @@ -22,7 +22,7 @@ func TestPeeringAcceptor_Validate(t *testing.T) { Secret: &Secret{ Name: "api-token", Key: "data", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, diff --git a/control-plane/api/v1alpha1/peeringacceptor_webhook_test.go b/control-plane/api/v1alpha1/peeringacceptor_webhook_test.go index ed07b03494..26ed3e2150 100644 --- a/control-plane/api/v1alpha1/peeringacceptor_webhook_test.go +++ b/control-plane/api/v1alpha1/peeringacceptor_webhook_test.go @@ -32,7 +32,7 @@ func TestValidatePeeringAcceptor(t *testing.T) { Secret: &Secret{ Name: "foo", Key: "data", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, @@ -47,7 +47,7 @@ func TestValidatePeeringAcceptor(t *testing.T) { Secret: &Secret{ Name: "foo2", Key: "data2", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, @@ -65,7 +65,7 @@ func TestValidatePeeringAcceptor(t *testing.T) { Secret: &Secret{ Name: "foo", Key: "data", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, @@ -80,14 +80,14 @@ func TestValidatePeeringAcceptor(t *testing.T) { Secret: &Secret{ Name: "foo", Key: "data", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, }, expAllow: true, }, - "invalid, unique secret name": { + "invalid, duplicate secret name and namespace": { existingResources: []runtime.Object{&PeeringAcceptor{ ObjectMeta: metav1.ObjectMeta{ Name: "peer1", @@ -98,7 +98,7 @@ func TestValidatePeeringAcceptor(t *testing.T) { Secret: &Secret{ Name: "foo", Key: "data", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, @@ -113,7 +113,7 @@ func TestValidatePeeringAcceptor(t *testing.T) { Secret: &Secret{ Name: "foo", Key: "data", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, diff --git a/control-plane/api/v1alpha1/peeringdialer_types_test.go b/control-plane/api/v1alpha1/peeringdialer_types_test.go index 8af724a09c..7e358facb8 100644 --- a/control-plane/api/v1alpha1/peeringdialer_types_test.go +++ b/control-plane/api/v1alpha1/peeringdialer_types_test.go @@ -22,7 +22,7 @@ func TestPeeringDialer_Validate(t *testing.T) { Secret: &Secret{ Name: "api-token", Key: "data", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, diff --git a/control-plane/api/v1alpha1/peeringdialer_webhook_test.go b/control-plane/api/v1alpha1/peeringdialer_webhook_test.go index 8112c425aa..abdca4f417 100644 --- a/control-plane/api/v1alpha1/peeringdialer_webhook_test.go +++ b/control-plane/api/v1alpha1/peeringdialer_webhook_test.go @@ -32,7 +32,7 @@ func TestValidatePeeringDialer(t *testing.T) { Secret: &Secret{ Name: "foo", Key: "data", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, @@ -47,7 +47,7 @@ func TestValidatePeeringDialer(t *testing.T) { Secret: &Secret{ Name: "foo2", Key: "data2", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, @@ -65,7 +65,7 @@ func TestValidatePeeringDialer(t *testing.T) { Secret: &Secret{ Name: "foo", Key: "data", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, @@ -80,14 +80,14 @@ func TestValidatePeeringDialer(t *testing.T) { Secret: &Secret{ Name: "foo", Key: "data", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, }, expAllow: true, }, - "invalid, unique secret name": { + "invalid, duplicate secret name and namespace": { existingResources: []runtime.Object{&PeeringDialer{ ObjectMeta: metav1.ObjectMeta{ Name: "peer1", @@ -98,7 +98,7 @@ func TestValidatePeeringDialer(t *testing.T) { Secret: &Secret{ Name: "foo", Key: "data", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, @@ -113,7 +113,7 @@ func TestValidatePeeringDialer(t *testing.T) { Secret: &Secret{ Name: "foo", Key: "data", - Backend: "kubernetes", + Backend: SecretBackendTypeKubernetes, }, }, }, diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index 81fb90b9a7..dff8651fd5 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -388,11 +388,9 @@ func (r *PeeringAcceptorController) requestsForPeeringTokens(object client.Objec return []ctrl.Request{} } for _, acceptor := range acceptorList.Items { - if acceptor.SecretRef() != nil { - if 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}}} - } + 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}}} } } }