Merge pull request #2918 from sbueringer/pr-add-skip-name-validation

🌱 Add SkipNameValidation option

pkg/config/controller.go

@@ -20,6 +20,12 @@ import "time"
 
 // Controller contains configuration options for a controller.
 type Controller struct {
+	// SkipNameValidation allows skipping the name validation that ensures that every controller name is unique.
+	// Unique controller names are important to get unique metrics and logs for a controller.
+	// Can be overwritten for a controller via the SkipNameValidation setting on the controller.
+	// Defaults to false if SkipNameValidation setting on controller and Manager are unset.
+	SkipNameValidation *bool
+
 	// GroupKindConcurrency is a map from a Kind to the number of concurrent reconciliation
 	// allowed for that controller.
 	//
@@ -40,8 +46,8 @@ type Controller struct {
 	CacheSyncTimeout time.Duration
 
 	// RecoverPanic indicates whether the panic caused by reconcile should be recovered.
-	// Defaults to the Controller.RecoverPanic setting from the Manager if unset.
-	// Defaults to true if Controller.RecoverPanic setting from the Manager is also unset.
+	// Can be overwritten for a controller via the RecoverPanic setting on the controller.
+	// Defaults to true if RecoverPanic setting on controller and Manager are unset.
 	RecoverPanic *bool
 
 	// NeedLeaderElection indicates whether the controller needs to use leader election.

pkg/controller/controller.go

@@ -36,6 +36,12 @@ type Options = TypedOptions[reconcile.Request]
 
 // TypedOptions are the arguments for creating a new Controller.
 type TypedOptions[request comparable] struct {
+	// SkipNameValidation allows skipping the name validation that ensures that every controller name is unique.
+	// Unique controller names are important to get unique metrics and logs for a controller.
+	// Defaults to the Controller.SkipNameValidation setting from the Manager if unset.
+	// Defaults to false if Controller.SkipNameValidation setting from the Manager is also unset.
+	SkipNameValidation *bool
+
 	// MaxConcurrentReconciles is the maximum number of concurrent Reconciles which can be run. Defaults to 1.
 	MaxConcurrentReconciles int
 
@@ -140,8 +146,14 @@ func NewTypedUnmanaged[request comparable](name string, mgr manager.Manager, opt
 		return nil, fmt.Errorf("must specify Name for Controller")
 	}
 
-	if err := checkName(name); err != nil {
-		return nil, err
+	if options.SkipNameValidation == nil {
+		options.SkipNameValidation = mgr.GetControllerOptions().SkipNameValidation
+	}
+
+	if options.SkipNameValidation == nil || !*options.SkipNameValidation {
+		if err := checkName(name); err != nil {
+			return nil, err
+		}
 	}
 
 	if options.LogConstructor == nil {

pkg/controller/controller_test.go

@@ -74,6 +74,54 @@ var _ = Describe("controller.Controller", func() {
 			Expect(c2).To(BeNil())
 		})
 
+		It("should return an error if two controllers are registered with the same name and SkipNameValidation is set to false on the manager", func() {
+			m, err := manager.New(cfg, manager.Options{
+				Controller: config.Controller{
+					SkipNameValidation: ptr.To(false),
+				},
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			c1, err := controller.New("c4", m, controller.Options{Reconciler: rec})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(c1).ToNot(BeNil())
+
+			c2, err := controller.New("c4", m, controller.Options{Reconciler: rec})
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("controller with name c4 already exists"))
+			Expect(c2).To(BeNil())
+		})
+
+		It("should not return an error if two controllers are registered with the same name and SkipNameValidation is set on the manager", func() {
+			m, err := manager.New(cfg, manager.Options{
+				Controller: config.Controller{
+					SkipNameValidation: ptr.To(true),
+				},
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			c1, err := controller.New("c5", m, controller.Options{Reconciler: rec})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(c1).ToNot(BeNil())
+
+			c2, err := controller.New("c5", m, controller.Options{Reconciler: rec})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(c2).ToNot(BeNil())
+		})
+
+		It("should not return an error if two controllers are registered with the same name and SkipNameValidation is set on the controller", func() {
+			m, err := manager.New(cfg, manager.Options{})
+			Expect(err).NotTo(HaveOccurred())
+
+			c1, err := controller.New("c6", m, controller.Options{Reconciler: rec})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(c1).ToNot(BeNil())
+
+			c2, err := controller.New("c6", m, controller.Options{Reconciler: rec, SkipNameValidation: ptr.To(true)})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(c2).ToNot(BeNil())
+		})
+
 		It("should not return an error if two controllers are registered with different names", func() {
 			m, err := manager.New(cfg, manager.Options{})
 			Expect(err).NotTo(HaveOccurred())