feat: Restrict memberclustername length

apis/cluster/v1/membercluster_types.go

@@ -25,6 +25,7 @@ import (
 // +kubebuilder:printcolumn:JSONPath=`.status.resourceUsage.allocatable.memory`,name="Allocatable-Memory", priority=1, type=string
 
 // MemberCluster is a resource created in the hub cluster to represent a member cluster within a fleet.
+// +kubebuilder:validation:XValidation:rule="size(self.metadata.name) < 64",message="metadata.name max length is 63"
 type MemberCluster struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`

apis/cluster/v1beta1/membercluster_types.go

@@ -26,6 +26,7 @@ import (
 // +kubebuilder:printcolumn:JSONPath=`.status.resourceUsage.allocatable.memory`,name="Allocatable-Memory", priority=1, type=string
 
 // MemberCluster is a resource created in the hub cluster to represent a member cluster within a fleet.
+// +kubebuilder:validation:XValidation:rule="size(self.metadata.name) < 64",message="metadata.name max length is 63"
 type MemberCluster struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`

config/crd/bases/cluster.kubernetes-fleet.io_memberclusters.yaml

@@ -408,6 +408,9 @@ spec:
         required:
         - spec
         type: object
+        x-kubernetes-validations:
+        - message: metadata.name max length is 63
+          rule: size(self.metadata.name) < 64
     served: true
     storage: false
     subresources:
@@ -801,6 +804,9 @@ spec:
         required:
         - spec
         type: object
+        x-kubernetes-validations:
+        - message: metadata.name max length is 63
+          rule: size(self.metadata.name) < 64
     served: true
     storage: true
     subresources:

go.sum

@@ -320,6 +320,7 @@ golang.org/x/time v0.7.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
+golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
 golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
 golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
 golang.org/x/tools v0.26.0 h1:v/60pFQmzmT9ExmjDv2gGIfi3OqfKoEP6I5+umXlbnQ=
@@ -339,7 +340,12 @@ gopkg.in/dnaeon/go-vcr.v3 v3.2.0 h1:Rltp0Vf+Aq0u4rQXgmXgtgoRDStTnFN83cWgSGSoRzM=
 gopkg.in/dnaeon/go-vcr.v3 v3.2.0/go.mod h1:2IMOnnlx9I6u9x+YBsM3tAMx6AlOxnJ0pWxQAzZ79Ag=
 gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc=
 gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw=
+gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ=
+gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw=
+gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
 gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

test/apis/cluster/v1/api_validation_integration_test.go

@@ -0,0 +1,217 @@
+/*
+Copyright (c) Microsoft Corporation.
+Licensed under the MIT license.
+*/
+package v1
+
+import (
+	"errors"
+	"fmt"
+	"reflect"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	rbacv1 "k8s.io/api/rbac/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	clusterv1 "go.goms.io/fleet/apis/cluster/v1"
+)
+
+var _ = Describe("Test cluster v1 API validation", func() {
+	Context("Test MemberCluster API validation - invalid cases", func() {
+		It("should deny creating API with invalid name size", func() {
+			var name = "abcdef-123456789-123456789-123456789-123456789-123456789-123456789-123456789"
+			// Create the API.
+			memberClusterName := &clusterv1.MemberCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+				Spec: clusterv1.MemberClusterSpec{
+					Identity: rbacv1.Subject{
+						Name:      "fleet-member-agent-cluster-1",
+						Kind:      "ServiceAccount",
+						Namespace: "fleet-system",
+						APIGroup:  "",
+					},
+				},
+			}
+			By(fmt.Sprintf("expecting denial of CREATE API %s", name))
+			var err = hubClient.Create(ctx, memberClusterName)
+			var statusErr *k8serrors.StatusError
+			Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create API call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8serrors.StatusError{})))
+			Expect(statusErr.Status().Message).Should(ContainSubstring("metadata.name max length is 63"))
+		})
+
+		It("should deny creating API with invalid name starting with non-alphanumeric character", func() {
+			var name = "-abcdef-123456789-123456789-123456789-123456789-123456789"
+			// Create the API.
+			memberClusterName := &clusterv1.MemberCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+				Spec: clusterv1.MemberClusterSpec{
+					Identity: rbacv1.Subject{
+						Name:      "fleet-member-agent-cluster-1",
+						Kind:      "ServiceAccount",
+						Namespace: "fleet-system",
+						APIGroup:  "",
+					},
+				},
+			}
+			By(fmt.Sprintf("expecting denial of CREATE API %s", name))
+			err := hubClient.Create(ctx, memberClusterName)
+			var statusErr *k8serrors.StatusError
+			Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create API call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8serrors.StatusError{})))
+			Expect(statusErr.Status().Message).Should(ContainSubstring("a lowercase RFC 1123 subdomain"))
+		})
+
+		It("should deny creating API with invalid name ending with non-alphanumeric character", func() {
+			var name = "abcdef-123456789-123456789-123456789-123456789-123456789-"
+			// Create the API.
+			memberClusterName := &clusterv1.MemberCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+				Spec: clusterv1.MemberClusterSpec{
+					Identity: rbacv1.Subject{
+						Name:      "fleet-member-agent-cluster-1",
+						Kind:      "ServiceAccount",
+						Namespace: "fleet-system",
+						APIGroup:  "",
+					},
+				},
+			}
+			By(fmt.Sprintf("expecting denial of CREATE API %s", name))
+			err := hubClient.Create(ctx, memberClusterName)
+			var statusErr *k8serrors.StatusError
+			Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create API call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8serrors.StatusError{})))
+			Expect(statusErr.Status().Message).Should(ContainSubstring("a lowercase RFC 1123 subdomain"))
+		})
+
+		It("should deny creating API with invalid name containing character that is not alphanumeric and not -", func() {
+			var name = "a_bcdef-123456789-123456789-123456789-123456789-123456789-123456789-123456789"
+			// Create the API.
+			memberClusterName := &clusterv1.MemberCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+				Spec: clusterv1.MemberClusterSpec{
+					Identity: rbacv1.Subject{
+						Name:      "fleet-member-agent-cluster-1",
+						Kind:      "ServiceAccount",
+						Namespace: "fleet-system",
+						APIGroup:  "",
+					},
+				},
+			}
+			By(fmt.Sprintf("expecting denial of CREATE API %s", name))
+			err := hubClient.Create(ctx, memberClusterName)
+			var statusErr *k8serrors.StatusError
+			Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create API call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8serrors.StatusError{})))
+			Expect(statusErr.Status().Message).Should(ContainSubstring("a lowercase RFC 1123 subdomain"))
+		})
+	})
+
+	Context("Test Member Cluster creation API validation - valid cases", func() {
+		It("should allow creating API with valid name size", func() {
+			var name = "abc-123456789-123456789-123456789-123456789-123456789-123456789"
+			// Create the API.
+			memberClusterName := &clusterv1.MemberCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+				Spec: clusterv1.MemberClusterSpec{
+					Identity: rbacv1.Subject{
+						Name:      "fleet-member-agent-cluster-1",
+						Kind:      "ServiceAccount",
+						Namespace: "fleet-system",
+						APIGroup:  "",
+					},
+				},
+			}
+			Expect(hubClient.Create(ctx, memberClusterName)).Should(Succeed())
+			Expect(hubClient.Delete(ctx, memberClusterName)).Should(Succeed())
+		})
+
+		It("should allow creating API with valid name starting with alphabet character", func() {
+			var name = "abc-123456789"
+			// Create the API.
+			memberClusterName := &clusterv1.MemberCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+				Spec: clusterv1.MemberClusterSpec{
+					Identity: rbacv1.Subject{
+						Name:      "fleet-member-agent-cluster-1",
+						Kind:      "ServiceAccount",
+						Namespace: "fleet-system",
+						APIGroup:  "",
+					},
+				},
+			}
+			Expect(hubClient.Create(ctx, memberClusterName)).Should(Succeed())
+			Expect(hubClient.Delete(ctx, memberClusterName)).Should(Succeed())
+		})
+
+		It("should allow creating API with valid name starting with numeric character", func() {
+			var name = "123-123456789"
+			// Create the API.
+			memberClusterName := &clusterv1.MemberCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+				Spec: clusterv1.MemberClusterSpec{
+					Identity: rbacv1.Subject{
+						Name:      "fleet-member-agent-cluster-1",
+						Kind:      "ServiceAccount",
+						Namespace: "fleet-system",
+						APIGroup:  "",
+					},
+				},
+			}
+			Expect(hubClient.Create(ctx, memberClusterName)).Should(Succeed())
+			Expect(hubClient.Delete(ctx, memberClusterName)).Should(Succeed())
+		})
+
+		It("should allow creating API with valid name ending with alphabet character", func() {
+			var name = "123456789-abc"
+			// Create the API.
+			memberClusterName := &clusterv1.MemberCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+				Spec: clusterv1.MemberClusterSpec{
+					Identity: rbacv1.Subject{
+						Name:      "fleet-member-agent-cluster-1",
+						Kind:      "ServiceAccount",
+						Namespace: "fleet-system",
+						APIGroup:  "",
+					},
+				},
+			}
+			Expect(hubClient.Create(ctx, memberClusterName)).Should(Succeed())
+			Expect(hubClient.Delete(ctx, memberClusterName)).Should(Succeed())
+		})
+
+		It("should allow creating API with valid name ending with numeric character", func() {
+			var name = "123456789-123"
+			// Create the API.
+			memberClusterName := &clusterv1.MemberCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+				Spec: clusterv1.MemberClusterSpec{
+					Identity: rbacv1.Subject{
+						Name:      "fleet-member-agent-cluster-1",
+						Kind:      "ServiceAccount",
+						Namespace: "fleet-system",
+						APIGroup:  "",
+					},
+				},
+			}
+			Expect(hubClient.Create(ctx, memberClusterName)).Should(Succeed())
+			Expect(hubClient.Delete(ctx, memberClusterName)).Should(Succeed())
+		})
+	})
+})

test/apis/cluster/v1/suite_test.go

@@ -0,0 +1,92 @@
+/*
+Copyright (c) Microsoft Corporation.
+Licensed under the MIT license.
+*/
+package v1
+
+import (
+	"context"
+	"flag"
+	"path/filepath"
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/klog/v2"
+	"k8s.io/klog/v2/textlogger"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/envtest"
+	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
+
+	clusterv1 "go.goms.io/fleet/apis/cluster/v1"
+)
+
+var (
+	hubTestEnv *envtest.Environment
+	hubClient  client.Client
+	ctx        context.Context
+	cancel     context.CancelFunc
+)
+
+func TestAPIs(t *testing.T) {
+	RegisterFailHandler(Fail)
+
+	RunSpecs(t, "ClusterResourcePlacement Controller Suite")
+}
+
+var _ = BeforeSuite(func() {
+	By("Setup klog")
+	fs := flag.NewFlagSet("klog", flag.ContinueOnError)
+	klog.InitFlags(fs)
+	Expect(fs.Parse([]string{"--v", "5", "-add_dir_header", "true"})).Should(Succeed())
+
+	ctx, cancel = context.WithCancel(context.TODO())
+
+	By("bootstrap the test environment")
+	// Start the cluster.
+	hubTestEnv = &envtest.Environment{
+		CRDDirectoryPaths: []string{
+			filepath.Join("..", "..", "..", "..", "config", "crd", "bases"),
+		},
+		ErrorIfCRDPathMissing: true,
+	}
+	hubCfg, err := hubTestEnv.Start()
+	Expect(err).NotTo(HaveOccurred())
+	Expect(hubCfg).NotTo(BeNil())
+
+	Expect(clusterv1.AddToScheme(scheme.Scheme)).Should(Succeed())
+
+	klog.InitFlags(flag.CommandLine)
+	flag.Parse()
+	// Create the hub controller manager.
+	hubCtrlMgr, err := ctrl.NewManager(hubCfg, ctrl.Options{
+		Scheme: scheme.Scheme,
+		Metrics: metricsserver.Options{
+			BindAddress: "0",
+		},
+		Logger: textlogger.NewLogger(textlogger.NewConfig(textlogger.Verbosity(4))),
+	})
+	Expect(err).NotTo(HaveOccurred())
+
+	// Set up the client.
+	// The client must be one with cache (i.e. configured by the controller manager) to make
+	// use of the cache indexes.
+	hubClient = hubCtrlMgr.GetClient()
+	Expect(hubClient).NotTo(BeNil())
+
+	go func() {
+		defer GinkgoRecover()
+		err = hubCtrlMgr.Start(ctx)
+		Expect(err).ToNot(HaveOccurred(), "failed to start manager for hub")
+	}()
+})
+
+var _ = AfterSuite(func() {
+	defer klog.Flush()
+	cancel()
+
+	By("tearing down the test environment")
+	Expect(hubTestEnv.Stop()).Should(Succeed())
+})