Skip to content

Commit

Permalink
Peering webhooks (#1310)
Browse files Browse the repository at this point in the history
- adds validation webhooks for PeeringAcceptor and Peering dialer controllers
- fixes panic by doing a nil check on the PeeringAcceptor SecretRef()
  • Loading branch information
ndhanushkodi authored Jul 6, 2022
1 parent 023007f commit faa3ecf
Show file tree
Hide file tree
Showing 14 changed files with 822 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Original file line number Diff line number Diff line change
Expand Up @@ -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" ]
}
38 changes: 38 additions & 0 deletions control-plane/api/v1alpha1/peeringacceptor_types.go
Original file line number Diff line number Diff line change
@@ -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{})
}
Expand Down Expand Up @@ -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
}
87 changes: 87 additions & 0 deletions control-plane/api/v1alpha1/peeringacceptor_types_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
69 changes: 69 additions & 0 deletions control-plane/api/v1alpha1/peeringacceptor_webhook.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading

0 comments on commit faa3ecf

Please sign in to comment.