Skip to content

Commit

Permalink
[NET-9201] Validating webhook for registrations (#3990)
Browse files Browse the repository at this point in the history
* Add validating webhook for registrations

* cleaned up registration webhook setup

* fix setup for webhook, updated docs

* fix typo, remove debugging log, rename variables for readability
  • Loading branch information
jm96441n authored May 14, 2024
1 parent 6db22a7 commit 7456304
Show file tree
Hide file tree
Showing 9 changed files with 518 additions and 19 deletions.
24 changes: 14 additions & 10 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,14 @@ rebase the branch on main, fixing any conflicts along the way before the code ca
1. Replace the names
1. Ensure you've correctly replaced the names in the kubebuilder annotation, ensure the plurality is correct
```go
// +kubebuilder:webhook:verbs=create;update,path=/mutate-v1alpha1-ingressgateway,mutating=true,failurePolicy=fail,groups=consul.hashicorp.com,resources=ingressgateways,versions=v1alpha1,name=mutate-ingressgateway.consul.hashicorp.com,webhookVersions=v1beta1,sideEffects=None
// +kubebuilder:webhook:verbs=create;update,path=/mutate-v1alpha1-ingressgateway,mutating=true,failurePolicy=fail,groups=consul.hashicorp.com,resources=ingressgateways,versions=v1alpha1,name=mutate-ingressgateway.consul.hashicorp.com,sideEffects=None,admissionReviewVersions=v1beta1;v1
```
1. Ensure you update the path to match the annotation in the `SetupWithManager` method:
```go
func (v *IngressGatewayWebhook) SetupWithManager(mgr ctrl.Manager) {
v.decoder = admission.NewDecoder(mgr.GetScheme())
mgr.GetWebhookServer().Register("/mutate-v1alpha1-ingressgateway", &admission.Webhook{Handler: v})
}
```
### Update command.go
Expand All @@ -362,16 +369,13 @@ rebase the branch on main, fixing any conflicts along the way before the code ca
return 1
}
```
1. Update `control-plane/subcommand/inject-connect/command.go` and add your webhook (the path should match the kubebuilder annotation):
1. Update `control-plane/subcommand/inject-connect/command.go` and add your webhook
```go
mgr.GetWebhookServer().Register("/mutate-v1alpha1-ingressgateway",
&webhook.Admission{Handler: &v1alpha1.IngressGatewayWebhook{
Client: mgr.GetClient(),
ConsulClient: consulClient,
Logger: ctrl.Log.WithName("webhooks").WithName(common.IngressGateway),
EnableConsulNamespaces: c.flagEnableNamespaces,
EnableNSMirroring: c.flagEnableNSMirroring,
}})
(&v1alpha1.IngressGatewayWebhook
Client: mgr.GetClient(),
Logger: ctrl.Log.WithName("webhooks").WithName(common.IngressGateway),
ConsulMeta: consulMeta,
}).SetupWithManager(mgr)
```
### Generating YAML
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,20 @@ webhooks:
name: {{ template "consul.fullname" . }}-connect-injector
namespace: {{ .Release.Namespace }}
path: /validate-v1alpha1-gatewaypolicy
- name: validate-registration.consul.hashicorp.com
matchPolicy: Equivalent
rules:
- operations: [ "CREATE" , "UPDATE" ]
apiGroups: [ "consul.hashicorp.com" ]
apiVersions: [ "v1alpha1" ]
resources: [ "registrations" ]
failurePolicy: Fail
sideEffects: None
admissionReviewVersions:
- v1
clientConfig:
service:
name: {{ template "consul.fullname" . }}-connect-injector
namespace: {{ .Release.Namespace }}
path: /validate-v1alpha1-registration
{{- end }}
3 changes: 0 additions & 3 deletions charts/consul/templates/crd-registrations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,7 @@ spec:
udp:
type: string
required:
- deregisterCriticalServiceAfterDuration
- intervalDuration
- timeoutDuration
type: object
exposedPort:
type: integer
Expand Down Expand Up @@ -116,7 +114,6 @@ spec:
- checkId
- definition
- name
- node
- serviceId
- serviceName
- status
Expand Down
10 changes: 7 additions & 3 deletions control-plane/api/v1alpha1/registration_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ type Locality struct {

// HealthCheck is used to represent a single check.
type HealthCheck struct {
Node string `json:"node"`
Node string `json:"node,omitempty"`
CheckID string `json:"checkId"`
Name string `json:"name"`
Status string `json:"status"`
Expand Down Expand Up @@ -142,8 +142,8 @@ type HealthCheckDefinition struct {
OSService string `json:"osService,omitempty"`
GRPCUseTLS bool `json:"grpcUseTLS,omitempty"`
IntervalDuration string `json:"intervalDuration"`
TimeoutDuration string `json:"timeoutDuration"`
DeregisterCriticalServiceAfterDuration string `json:"deregisterCriticalServiceAfterDuration"`
TimeoutDuration string `json:"timeoutDuration,omitempty"`
DeregisterCriticalServiceAfterDuration string `json:"deregisterCriticalServiceAfterDuration,omitempty"`
}

// +kubebuilder:object:root=true
Expand Down Expand Up @@ -309,3 +309,7 @@ func (r *Registration) SetSyncedCondition(status corev1.ConditionStatus, reason
},
}
}

func (r *Registration) KubernetesName() string {
return r.ObjectMeta.Name
}
124 changes: 124 additions & 0 deletions control-plane/api/v1alpha1/registration_webhook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0

package v1alpha1

import (
"context"
"errors"
"fmt"
"net/http"
"time"

"github.com/go-logr/logr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"github.com/hashicorp/consul-k8s/control-plane/api/common"
)

// +kubebuilder:object:generate=false

type RegistrationWebhook struct {
Logger logr.Logger

// ConsulMeta contains metadata specific to the Consul installation.
ConsulMeta common.ConsulMeta

decoder *admission.Decoder
client.Client
}

// +kubebuilder:webhook:verbs=create;update,path=/validate-v1alpha1-registration,mutating=false,failurePolicy=fail,groups=consul.hashicorp.com,resources=registrations,versions=v1alpha1,name=validate-registration.consul.hashicorp.com,sideEffects=None,admissionReviewVersions=v1beta1;v1

func (v *RegistrationWebhook) Handle(ctx context.Context, req admission.Request) admission.Response {
resource := &Registration{}
decodeErr := v.decoder.Decode(req, resource)
if decodeErr != nil {
return admission.Errored(http.StatusBadRequest, decodeErr)
}

var err error

err = errors.Join(err, validateRequiredFields(resource))
err = errors.Join(err, validateHealthChecks(resource))
if err != nil {
return admission.Errored(http.StatusBadRequest, err)
}

return admission.Allowed("registration is valid")
}

func (v *RegistrationWebhook) SetupWithManager(mgr ctrl.Manager) {
v.Logger.Info("setting up registration webhook")
v.decoder = admission.NewDecoder(mgr.GetScheme())
mgr.GetWebhookServer().Register("/validate-v1alpha1-registration", &admission.Webhook{Handler: v})
}

func validateRequiredFields(registration *Registration) error {
var err error
if registration.Spec.Node == "" {
err = errors.Join(err, errors.New("registration.Spec.Node is required"))
}
if registration.Spec.Service.Name == "" {
err = errors.Join(err, errors.New("registration.Spec.Service.Name is required"))
}
if registration.Spec.Address == "" {
err = errors.Join(err, errors.New("registration.Spec.Address is required"))
}

if err != nil {
return err
}
return nil
}

var validStatuses = map[string]struct{}{
"passing": {},
"warning": {},
"critical": {},
}

func validateHealthChecks(registration *Registration) error {
if registration.Spec.HealthCheck == nil {
return nil
}

var err error

if registration.Spec.HealthCheck.Name == "" {
err = errors.Join(err, errors.New("registration.Spec.HealthCheck.Name is required"))
}

// status must be one "passing", "warning", or "critical"
if _, ok := validStatuses[registration.Spec.HealthCheck.Status]; !ok {
err = errors.Join(err, fmt.Errorf("invalid registration.Spec.HealthCheck.Status value, must be 'passing', 'warning', or 'critical', actual: %q", registration.Spec.HealthCheck.Status))
}

// parse all durations and check for any errors
_, parseErr := time.ParseDuration(registration.Spec.HealthCheck.Definition.IntervalDuration)
if parseErr != nil {
err = errors.Join(err, fmt.Errorf("invalid registration.Spec.HealthCheck.Definition.IntervalDuration value: %q", registration.Spec.HealthCheck.Definition.IntervalDuration))
}

if registration.Spec.HealthCheck.Definition.TimeoutDuration != "" {
_, timeoutErr := time.ParseDuration(registration.Spec.HealthCheck.Definition.TimeoutDuration)
if timeoutErr != nil {
err = errors.Join(err, fmt.Errorf("invalid registration.Spec.HealthCheck.Definition.TimeoutDuration value: %q", registration.Spec.HealthCheck.Definition.TimeoutDuration))
}
}

if registration.Spec.HealthCheck.Definition.DeregisterCriticalServiceAfterDuration != "" {
_, deregCriticalErr := time.ParseDuration(registration.Spec.HealthCheck.Definition.DeregisterCriticalServiceAfterDuration)
if deregCriticalErr != nil {
err = errors.Join(err, fmt.Errorf("invalid registration.Spec.HealthCheck.Definition.DeregisterCriticalServiceAfterDuration value: %q", registration.Spec.HealthCheck.Definition.DeregisterCriticalServiceAfterDuration))
}
}

if err != nil {
return err
}

return nil
}
Loading

0 comments on commit 7456304

Please sign in to comment.