Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: validate for clusterfilter #359

Merged
merged 1 commit into from
Jan 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion apis/meta/v1alpha1/labels_annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ const (
// SecretTypeAnnotationKey annotation key for an existed secret with a different type
SecretTypeAnnotationKey = "katanomi.dev/secretType" //nolint:gosec
// ClusterNameAnnotationKey annotation key to store resource cluster name
ClusterNameAnnotationKey = "integrations.katanomi.dev/clusterName"
ClusterNameAnnotationKey = "katanomi.dev/clusterName"
l-qing marked this conversation as resolved.
Show resolved Hide resolved
// ClusterRefNamespaceAnnotationKey annotation key to store cluster reference namespace
ClusterRefNamespaceAnnotationKey = "katanomi.dev/clusterRefNamespace"
// TriggerNameAnnotationKey annotation key to store a friendly trigger name
TriggerNameAnnotationKey = "katanomi.dev/triggerName"
// SettingsConvertTypesKey annotation key to store the setting types need to be converted
Expand Down
4 changes: 3 additions & 1 deletion apis/selection/v1alpha1/cluster_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ func (n ClusterFilterRule) Filter(uClusters []unstructured.Unstructured) []corev
func (n *ClusterFilter) Validate(fld *field.Path) field.ErrorList {
errs := field.ErrorList{}

errs = append(errs, kvalidation.ValidateItemName(n.Namespace, false, field.NewPath("namespace"))...)
if n.Namespace != "" {
errs = append(errs, kvalidation.ValidateItemName(n.Namespace, false, fld.Child("namespace"))...)
}

bFilter := BaseFilter{
Selector: n.Selector,
Expand Down
57 changes: 45 additions & 12 deletions apis/selection/v1alpha1/cluster_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,13 @@ package v1alpha1
import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"

kvalidation "github.com/katanomi/pkg/apis/validation"
. "github.com/katanomi/pkg/testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var _ = Describe("Test.ClusterFilterRule", func() {
Expand Down Expand Up @@ -59,24 +63,32 @@ var _ = Describe("Test.ClusterFilter.Validate", func() {
clusterFilter *ClusterFilter
path *field.Path
errs field.ErrorList
err error
)

BeforeEach(func() {
path = field.NewPath("")
path = field.NewPath("prefix")
clusterFilter = &ClusterFilter{}
})

JustBeforeEach(func() {
errs = clusterFilter.Validate(path)
err = kvalidation.ReturnInvalidError(schema.GroupKind{}, "kind", errs)
})

Context("empty struct", func() {
It("should return validation error", func() {
Expect(errs).ToNot(BeNil(), "should return an error")
// Filter removes items from the ErrorList that match the provided fns.
Expect(errs.Filter(field.NewErrorTypeMatcher(field.ErrorTypeRequired))).To(HaveLen(1))
Expect(errs.Filter(field.NewErrorTypeMatcher(field.ErrorTypeInvalid))).To(HaveLen(1))
Expect(errs).To(HaveLen(2))
Expect(err).ToNot(BeNil(), "should return an error")
Expect(errors.IsInvalid(err)).To(BeTrue(), "should return an invalid error")

statusErr, _ := err.(*errors.StatusError)
Expect(statusErr.ErrStatus.Details.Causes).To(ContainElements(
l-qing marked this conversation as resolved.
Show resolved Hide resolved
metav1.StatusCause{
Type: "FieldValueRequired",
Message: "Required value: one of selector OR refs is required",
Field: "prefix",
},
))
})
})

Expand All @@ -86,11 +98,32 @@ var _ = Describe("Test.ClusterFilter.Validate", func() {
})

It("should return validation error", func() {
Expect(errs).ToNot(BeNil(), "should return an error")
// Filter removes items from the ErrorList that match the provided fns.
Expect(errs.Filter(field.NewErrorTypeMatcher(field.ErrorTypeRequired))).To(HaveLen(3))
Expect(errs.Filter(field.NewErrorTypeMatcher(field.ErrorTypeInvalid))).To(HaveLen(1))
Expect(errs).To(HaveLen(4))
Expect(err).ToNot(BeNil(), "should return an error")
Expect(errors.IsInvalid(err)).To(BeTrue(), "should return an invalid error")

statusErr, _ := err.(*errors.StatusError)
Expect(statusErr.ErrStatus.Details.Causes).To(ContainElements(
metav1.StatusCause{
Type: "FieldValueInvalid",
Message: "Invalid value: \"default-\": a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')",
Field: "prefix.namespace",
},
metav1.StatusCause{
Type: "FieldValueInvalid",
Message: "Invalid value: \"app-\": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')",
Field: "prefix.selector.matchLabels",
},
metav1.StatusCause{
Type: "FieldValueInvalid",
Message: "Invalid value: \"Unknown\": not a valid selector operator",
Field: "prefix.selector.matchExpressions[0].operator",
},
metav1.StatusCause{
Type: "FieldValueRequired",
Message: "Required value: must be specified when `operator` is 'In' or 'NotIn'",
Field: "prefix.selector.matchExpressions[1].values",
},
))
})
})

Expand All @@ -100,7 +133,7 @@ var _ = Describe("Test.ClusterFilter.Validate", func() {
})

It("should not return validation error", func() {
Expect(errs).To(HaveLen(0), "should NOT return an error")
Expect(err).To(BeNil(), "should NOT return an error")
})
})

Expand Down
11 changes: 3 additions & 8 deletions apis/selection/v1alpha1/get_clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,14 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/dynamic"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/katanomi/pkg/multicluster"
knamespace "github.com/katanomi/pkg/namespace"
)

var (
// ClusterRegistryGroupVersion is the group version for the cluster registry
ClusterRegistryGroupVersion = schema.GroupVersion{Group: "clusterregistry.k8s.io", Version: "v1alpha1"}
// ClusterGVR is the group version resource for the cluster registry
ClusterGVR = ClusterRegistryGroupVersion.WithResource("clusters")

// passAllClusterFilterRule used to pass all clusters
passAllClusterFilterRule = ClusterFilterRule{
Exact: map[string]string{},
Expand Down Expand Up @@ -93,7 +88,7 @@ func getClustersByLabelSelector(ctx context.Context, clt dynamic.Interface, labe
LabelSelector: metav1.FormatLabelSelector(labelSelector),
}
clusterList := &unstructured.UnstructuredList{}
dyclient := clt.Resource(ClusterGVR).Namespace(defaultNS)
dyclient := clt.Resource(multicluster.ClusterGVR).Namespace(defaultNS)
l-qing marked this conversation as resolved.
Show resolved Hide resolved
if clusterList, err = dyclient.List(ctx, opts); err != nil {
return
}
Expand All @@ -116,7 +111,7 @@ func getClustersByRefs(ctx context.Context, clt dynamic.Interface, refs []corev1
if ns == "" {
ns = defaultNS
}
dyclient := clt.Resource(ClusterGVR).Namespace(ns)
dyclient := clt.Resource(multicluster.ClusterGVR).Namespace(ns)
var cluster *unstructured.Unstructured
cluster, err = dyclient.Get(ctx, clusterRef.Name, metav1.GetOptions{})
err = client.IgnoreNotFound(err)
Expand Down
5 changes: 3 additions & 2 deletions apis/selection/v1alpha1/get_clusters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
. "github.com/onsi/gomega"
"knative.dev/pkg/logging"

"github.com/katanomi/pkg/multicluster"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -51,11 +52,11 @@ var _ = Describe("Test.GetNamespacesBasedOnFilter", func() {
objs, err := LoadKubeResourcesAsUnstructured("testdata/clusterfilter.clusters.list.yaml")
Expect(err).Should(BeNil())
gvrToListKind := map[schema.GroupVersionResource]string{
ClusterGVR: "ClusterList",
multicluster.ClusterGVR: "ClusterList",
}
clt = dynamicfake.NewSimpleDynamicClientWithCustomListKinds(scheme, gvrToListKind)
for i, obj := range objs {
clt.Resource(ClusterGVR).Namespace(obj.GetNamespace()).Create(ctx, &objs[i], metav1.CreateOptions{})
clt.Resource(multicluster.ClusterGVR).Namespace(obj.GetNamespace()).Create(ctx, &objs[i], metav1.CreateOptions{})
}
})

Expand Down
2 changes: 1 addition & 1 deletion hack/boilerplate.go.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2022 The Katanomi Authors.
Copyright 2023 The Katanomi Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down
1 change: 1 addition & 0 deletions multicluster/clusterregistry_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func NewClusterRegistryClientOrDie(config *rest.Config) Interface {

var ClusterRegistryGroupVersion = schema.GroupVersion{Group: "clusterregistry.k8s.io", Version: "v1alpha1"}
var ClusterRegistryGVK = ClusterRegistryGroupVersion.WithKind("Cluster")
var ClusterGVR = ClusterRegistryGroupVersion.WithResource("clusters")

// GetConfig returns the configuration based on the Cluster
func (m *ClusterRegistryClient) GetConfig(ctx context.Context, clusterRef *corev1.ObjectReference) (config *rest.Config, err error) {
Expand Down