Skip to content

Commit

Permalink
webhooks: defaulting and validation for ExtensionConfig
Browse files Browse the repository at this point in the history 
Co-authored-by: Yuvaraj Kakaraparthi <[email protected]>
Co-authored-by: Christian Schlotter <[email protected]>
  • Loading branch information
@chrischdi
Yuvaraj Kakaraparthi and chrischdi committed May 3, 2022
1 parent 2982a32 commit 224aef1
Show file tree
Hide file tree
Showing 5 changed files with 323 additions and 10 deletions.
7 changes: 4 additions & 3 deletions internal/webhooks/cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/feature"
"sigs.k8s.io/cluster-api/internal/test/builder"
"sigs.k8s.io/cluster-api/internal/webhooks/util"
)

func TestClusterDefaultNamespaces(t *testing.T) {
Expand All @@ -46,7 +47,7 @@ func TestClusterDefaultNamespaces(t *testing.T) {
},
}
webhook := &Cluster{}
tFunc := customDefaultValidateTest(ctx, c, webhook)
tFunc := util.CustomDefaultValidateTest(ctx, c, webhook)

t.Run("for Cluster", tFunc)
g.Expect(webhook.Default(ctx, c)).To(Succeed())
Expand Down Expand Up @@ -317,7 +318,7 @@ func TestClusterDefaultVariables(t *testing.T) {

t.Run(tt.name, func(t *testing.T) {
// Test if defaulting works in combination with validation.
customDefaultValidateTest(ctx, cluster, webhook)(t)
util.CustomDefaultValidateTest(ctx, cluster, webhook)(t)
// Test defaulting.
t.Run("default", func(t *testing.T) {
g := NewWithT(t)
Expand Down Expand Up @@ -350,7 +351,7 @@ func TestClusterDefaultTopologyVersion(t *testing.T) {

// Create the webhook and add the fakeClient as its client.
webhook := &Cluster{Client: fakeClient}
tFunc := customDefaultValidateTest(ctx, c, webhook)
tFunc := util.CustomDefaultValidateTest(ctx, c, webhook)
t.Run("for Cluster", tFunc)
g.Expect(webhook.Default(ctx, c)).To(Succeed())

Expand Down
3 changes: 2 additions & 1 deletion internal/webhooks/clusterclass_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/feature"
"sigs.k8s.io/cluster-api/internal/test/builder"
"sigs.k8s.io/cluster-api/internal/webhooks/util"
)

var (
Expand Down Expand Up @@ -76,7 +77,7 @@ func TestClusterClassDefaultNamespaces(t *testing.T) {

// Create the webhook and add the fakeClient as its client.
webhook := &ClusterClass{Client: fakeClient}
tFunc := customDefaultValidateTest(ctx, in, webhook)
tFunc := util.CustomDefaultValidateTest(ctx, in, webhook)

t.Run("for ClusterClass", tFunc)

Expand Down
113 changes: 112 additions & 1 deletion internal/webhooks/runtime/extensionconfig_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ package runtime
import (
"context"
"fmt"
"net/url"

apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/utils/pointer"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"

Expand Down Expand Up @@ -58,6 +61,11 @@ func (webhook *ExtensionConfig) Default(ctx context.Context, obj runtime.Object)
if extensionConfig.Spec.NamespaceSelector == nil {
extensionConfig.Spec.NamespaceSelector = &metav1.LabelSelector{}
}
if extensionConfig.Spec.ClientConfig.Service != nil {
if extensionConfig.Spec.ClientConfig.Service.Port == nil {
extensionConfig.Spec.ClientConfig.Service.Port = pointer.Int32(8443)
}
}
return nil
}

Expand All @@ -84,7 +92,7 @@ func (webhook *ExtensionConfig) ValidateUpdate(ctx context.Context, old, updated
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
func (webhook *ExtensionConfig) validate(_ context.Context, _, _ *runtimev1.ExtensionConfig) error {
func (webhook *ExtensionConfig) validate(_ context.Context, _, newExtensionConfig *runtimev1.ExtensionConfig) error {
// NOTE: ExtensionConfig is behind the RuntimeSDK feature gate flag; the web hook
// must prevent creating and updating objects in case the feature flag is disabled.
if !feature.Gates.Enabled(feature.RuntimeSDK) {
Expand All @@ -93,10 +101,113 @@ func (webhook *ExtensionConfig) validate(_ context.Context, _, _ *runtimev1.Exte
"can be set only if the RuntimeSDK feature flag is enabled",
)
}

var allErrs field.ErrorList
allErrs = append(allErrs, validateExtensionConfigSpec(newExtensionConfig)...)

if len(allErrs) > 0 {
return apierrors.NewInvalid(runtimev1.GroupVersion.WithKind("ExtensionConfig").GroupKind(), newExtensionConfig.Name, allErrs)
}
return nil
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
func (webhook *ExtensionConfig) ValidateDelete(_ context.Context, _ runtime.Object) error {
return nil
}

func validateExtensionConfigSpec(e *runtimev1.ExtensionConfig) field.ErrorList {
var allErrs field.ErrorList

specPath := field.NewPath("spec")

if e.Spec.ClientConfig.URL == nil && e.Spec.ClientConfig.Service == nil {
allErrs = append(allErrs, field.Required(
specPath.Child("clientConfig"),
"either URL or Service must be defined",
))
}
if e.Spec.ClientConfig.URL != nil && e.Spec.ClientConfig.Service != nil {
allErrs = append(allErrs, field.Forbidden(
specPath.Child("clientConfig"),
"only one of URL or Service can be defined",
))
}

// Validate URL
if e.Spec.ClientConfig.URL != nil {
if uri, err := url.ParseRequestURI(*e.Spec.ClientConfig.URL); err != nil {
allErrs = append(allErrs, field.Invalid(
specPath.Child("clientConfig", "url"),
*e.Spec.ClientConfig.URL,
"must be a valid URL, e.g. https://example.com",
))
} else if uri.Scheme != "http" && uri.Scheme != "https" {
allErrs = append(allErrs, field.Invalid(
specPath.Child("clientConfig", "url"),
*e.Spec.ClientConfig.URL,
"'https' and 'http' are the only allowed URL schemes, e.g. https://example.com",
))
}
}

// Validate Service if defined
if e.Spec.ClientConfig.Service != nil {
// Validate that the name is not empty and is a Valid RFC1123 name.
if e.Spec.ClientConfig.Service.Name == "" {
allErrs = append(allErrs, field.Required(
specPath.Child("clientConfig", "service", "name"),
"must not be empty",
))
}

if errs := validation.IsDNS1123Subdomain(e.Spec.ClientConfig.Service.Name); len(errs) != 0 {
allErrs = append(allErrs, field.Invalid(
specPath.Child("clientConfig", "service", "name"),
e.Spec.ClientConfig.Service.Name,
"invalid name",
))
}

if e.Spec.ClientConfig.Service.Namespace == "" {
allErrs = append(allErrs, field.Required(
specPath.Child("clientConfig", "service", "namespace"),
"must not be empty",
))
}

if e.Spec.ClientConfig.Service.Path != nil {
path := *e.Spec.ClientConfig.Service.Path
if _, err := url.ParseRequestURI(path); err != nil {
allErrs = append(allErrs, field.Invalid(
specPath.Child("clientConfig", "service", "path"),
path,
"must be a valid URL path e.g. /path/to/hook",
))
}
if path[0:1] != "/" {
allErrs = append(allErrs, field.Invalid(
specPath.Child("clientConfig", "service", "path"),
path,
"must be a valid URL path e.g. /path/to/hook",
))
}
}
if e.Spec.ClientConfig.Service.Port != nil {
if errs := validation.IsValidPortNum(int(*e.Spec.ClientConfig.Service.Port)); len(errs) != 0 {
allErrs = append(allErrs, field.Invalid(
specPath.Child("clientConfig", "service", "port"),
*e.Spec.ClientConfig.Service.Port,
"must be in range 1-65535 inclusive",
))
}
}
}
if e.Spec.NamespaceSelector == nil {
allErrs = append(allErrs, field.Required(
specPath.Child("NamespaceSelector"),
"must be defined",
))
}
return allErrs
}
Loading

0 comments on commit 224aef1

Please sign in to comment.