Skip to content

Commit

Permalink
🌱 Add input validations for controllers (#11327)
Browse files Browse the repository at this point in the history
* Add input validations for controllers

* Fix test cases after input validation

* Update DockerMachinePoolReconciler to remove unused Scheme
  • Loading branch information
Karthik-K-N authored Oct 25, 2024
1 parent 0000f09 commit e5cbc4a
Show file tree
Hide file tree
Showing 29 changed files with 109 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ type Scope struct {

// SetupWithManager sets up the reconciler with the Manager.
func (r *KubeadmConfigReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.SecretCachingClient == nil || r.ClusterCache == nil || r.TokenTTL == time.Duration(0) {
return errors.New("Client, SecretCachingClient and ClusterCache must not be nil and TokenTTL must not be 0")
}

if r.KubeadmInitLock == nil {
r.KubeadmInitLock = locking.NewControlPlaneInitMutex(mgr.GetClient())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ type ClusterResourceSetReconciler struct {
}

func (r *ClusterResourceSetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options, partialSecretCache cache.Cache) error {
if r.Client == nil || r.ClusterCache == nil {
return errors.New("Client and ClusterCache must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "clusterresourceset")
err := ctrl.NewControllerManagedBy(mgr).
For(&addonsv1.ClusterResourceSet{}).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ type ClusterResourceSetBindingReconciler struct {
}

func (r *ClusterResourceSetBindingReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil {
return errors.New("Client must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "clusterresourcesetbinding")
err := ctrl.NewControllerManagedBy(mgr).
For(&addonsv1.ClusterResourceSetBinding{}).
Expand Down
4 changes: 4 additions & 0 deletions exp/internal/controllers/machinepool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ type scope struct {
}

func (r *MachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.APIReader == nil || r.ClusterCache == nil {
return errors.New("Client, APIReader and ClusterCache must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "machinepool")
clusterToMachinePools, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &expv1.MachinePoolList{}, mgr.GetScheme())
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions exp/internal/controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ func TestMain(m *testing.M) {

if err := (&MachinePoolReconciler{
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
ClusterCache: clusterCache,
recorder: mgr.GetEventRecorderFor("machinepool-controller"),
}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 1}); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ type Reconciler struct {
}

func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options, partialSecretCache cache.Cache) error {
if r.Client == nil || r.APIReader == nil || r.RuntimeClient == nil {
return errors.New("Client, APIReader and RuntimeClient must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "extensionconfig")
err := ctrl.NewControllerManagedBy(mgr).
For(&runtimev1.ExtensionConfig{}).
Expand Down
4 changes: 4 additions & 0 deletions internal/controllers/cluster/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ type Reconciler struct {
}

func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.APIReader == nil || r.ClusterCache == nil || r.RemoteConnectionGracePeriod == time.Duration(0) {
return errors.New("Client, APIReader and ClusterCache must not be nil and RemoteConnectionGracePeriod must not be 0")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "cluster")
c, err := ctrl.NewControllerManagedBy(mgr).
For(&clusterv1.Cluster{}).
Expand Down
7 changes: 4 additions & 3 deletions internal/controllers/cluster/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,10 @@ func TestMain(m *testing.M) {
panic(fmt.Sprintf("Failed to start ClusterReconciler: %v", err))
}
if err := (&machinecontroller.Reconciler{
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
ClusterCache: clusterCache,
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
ClusterCache: clusterCache,
RemoteConditionsGracePeriod: 5 * time.Minute,
}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 1}); err != nil {
panic(fmt.Sprintf("Failed to start MachineReconciler: %v", err))
}
Expand Down
4 changes: 4 additions & 0 deletions internal/controllers/clusterclass/clusterclass_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ type Reconciler struct {
}

func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.RuntimeClient == nil {
return errors.New("Client and RuntimeClient must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "clusterclass")
err := ctrl.NewControllerManagedBy(mgr).
For(&clusterv1.ClusterClass{}).
Expand Down
4 changes: 3 additions & 1 deletion internal/controllers/clusterclass/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/api/v1beta1/index"
"sigs.k8s.io/cluster-api/feature"
fakeruntimeclient "sigs.k8s.io/cluster-api/internal/runtime/client/fake"
"sigs.k8s.io/cluster-api/internal/test/envtest"
)

Expand All @@ -63,7 +64,8 @@ func TestMain(m *testing.M) {
}
setupReconcilers := func(ctx context.Context, mgr ctrl.Manager) {
if err := (&Reconciler{
Client: mgr.GetClient(),
Client: mgr.GetClient(),
RuntimeClient: fakeruntimeclient.NewRuntimeClientBuilder().Build(),
}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 5}); err != nil {
panic(fmt.Sprintf("unable to create clusterclass reconciler: %v", err))
}
Expand Down
4 changes: 4 additions & 0 deletions internal/controllers/machine/machine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ type Reconciler struct {
}

func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.APIReader == nil || r.ClusterCache == nil || r.RemoteConditionsGracePeriod == time.Duration(0) {
return errors.New("Client, APIReader and ClusterCache must not be nil and RemoteConditionsGracePeriod must not be 0")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "machine")
clusterToMachines, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &clusterv1.MachineList{}, mgr.GetScheme())
if err != nil {
Expand Down
7 changes: 4 additions & 3 deletions internal/controllers/machine/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,10 @@ func TestMain(m *testing.M) {
SetConnectionCreationRetryInterval(2 * time.Second)

if err := (&Reconciler{
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
ClusterCache: clusterCache,
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
ClusterCache: clusterCache,
RemoteConditionsGracePeriod: 5 * time.Minute,
}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 1}); err != nil {
panic(fmt.Sprintf("Failed to start MachineReconciler: %v", err))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ type Reconciler struct {
}

func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.APIReader == nil {
return errors.New("Client and APIReader must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "machinedeployment")
clusterToMachineDeployments, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &clusterv1.MachineDeploymentList{}, mgr.GetScheme())
if err != nil {
Expand Down
7 changes: 4 additions & 3 deletions internal/controllers/machinedeployment/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,10 @@ func TestMain(m *testing.M) {
}

if err := (&machinecontroller.Reconciler{
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
ClusterCache: clusterCache,
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
ClusterCache: clusterCache,
RemoteConditionsGracePeriod: 5 * time.Minute,
}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 1}); err != nil {
panic(fmt.Sprintf("Failed to start MachineReconciler: %v", err))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ type Reconciler struct {
}

func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.ClusterCache == nil {
return errors.New("Client and ClusterCache must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "machinehealthcheck")
c, err := ctrl.NewControllerManagedBy(mgr).
For(&clusterv1.MachineHealthCheck{}).
Expand Down
14 changes: 8 additions & 6 deletions internal/controllers/machinehealthcheck/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,10 @@ func TestMain(m *testing.M) {
SetConnectionCreationRetryInterval(2 * time.Second)

if err := (&clustercontroller.Reconciler{
Client: mgr.GetClient(),
APIReader: mgr.GetClient(),
ClusterCache: clusterCache,
Client: mgr.GetClient(),
APIReader: mgr.GetClient(),
ClusterCache: clusterCache,
RemoteConnectionGracePeriod: 50 * time.Second,
}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 1}); err != nil {
panic(fmt.Sprintf("Failed to start ClusterReconciler: %v", err))
}
Expand All @@ -106,9 +107,10 @@ func TestMain(m *testing.M) {
panic(fmt.Sprintf("Failed to start Reconciler : %v", err))
}
if err := (&machinecontroller.Reconciler{
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
ClusterCache: clusterCache,
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
ClusterCache: clusterCache,
RemoteConditionsGracePeriod: 5 * time.Minute,
}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 1}); err != nil {
panic(fmt.Sprintf("Failed to start MachineReconciler: %v", err))
}
Expand Down
4 changes: 4 additions & 0 deletions internal/controllers/machineset/machineset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ type Reconciler struct {
}

func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.APIReader == nil || r.ClusterCache == nil {
return errors.New("Client, APIReader and ClusterCache must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "machineset")
clusterToMachineSets, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &clusterv1.MachineSetList{}, mgr.GetScheme())
if err != nil {
Expand Down
7 changes: 4 additions & 3 deletions internal/controllers/machineset/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,10 @@ func TestMain(m *testing.M) {
panic(fmt.Sprintf("Failed to start MMachineSetReconciler: %v", err))
}
if err := (&machinecontroller.Reconciler{
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
ClusterCache: clusterCache,
Client: mgr.GetClient(),
APIReader: mgr.GetAPIReader(),
ClusterCache: clusterCache,
RemoteConditionsGracePeriod: 5 * time.Minute,
}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 1}); err != nil {
panic(fmt.Sprintf("Failed to start MachineReconciler: %v", err))
}
Expand Down
4 changes: 4 additions & 0 deletions internal/controllers/topology/cluster/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ type Reconciler struct {
}

func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.APIReader == nil || r.ClusterCache == nil {
return errors.New("Client, APIReader and ClusterCache must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "topology/cluster")
c, err := ctrl.NewControllerManagedBy(mgr).
For(&clusterv1.Cluster{}, builder.WithPredicates(
Expand Down
4 changes: 3 additions & 1 deletion internal/controllers/topology/cluster/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"sigs.k8s.io/cluster-api/controllers/remote"
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
"sigs.k8s.io/cluster-api/internal/controllers/clusterclass"
fakeruntimeclient "sigs.k8s.io/cluster-api/internal/runtime/client/fake"
"sigs.k8s.io/cluster-api/internal/test/envtest"
)

Expand Down Expand Up @@ -94,7 +95,8 @@ func TestMain(m *testing.M) {
panic(fmt.Sprintf("unable to create topology cluster reconciler: %v", err))
}
if err := (&clusterclass.Reconciler{
Client: mgr.GetClient(),
Client: mgr.GetClient(),
RuntimeClient: fakeruntimeclient.NewRuntimeClientBuilder().Build(),
}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 5}); err != nil {
panic(fmt.Sprintf("unable to create clusterclass reconciler: %v", err))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ type Reconciler struct {
}

func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.APIReader == nil {
return errors.New("Client and APIReader must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "topology/machinedeployment")
clusterToMachineDeployments, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &clusterv1.MachineDeploymentList{}, mgr.GetScheme())
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ type Reconciler struct {
}

func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.APIReader == nil {
return errors.New("Client and APIReader must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "topology/machineset")
clusterToMachineSets, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &clusterv1.MachineSetList{}, mgr.GetScheme())
if err != nil {
Expand Down
4 changes: 3 additions & 1 deletion internal/webhooks/test/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"sigs.k8s.io/cluster-api/api/v1beta1/index"
"sigs.k8s.io/cluster-api/feature"
"sigs.k8s.io/cluster-api/internal/controllers/clusterclass"
fakeruntimeclient "sigs.k8s.io/cluster-api/internal/runtime/client/fake"
"sigs.k8s.io/cluster-api/internal/test/envtest"
)

Expand All @@ -48,7 +49,8 @@ func TestMain(m *testing.M) {
}
setupReconcilers := func(ctx context.Context, mgr ctrl.Manager) {
if err := (&clusterclass.Reconciler{
Client: mgr.GetClient(),
Client: mgr.GetClient(),
RuntimeClient: fakeruntimeclient.NewRuntimeClientBuilder().Build(),
}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 5}); err != nil {
panic(fmt.Sprintf("unable to create clusterclass reconciler: %v", err))
}
Expand Down
3 changes: 0 additions & 3 deletions test/infrastructure/docker/exp/controllers/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package controllers
import (
"context"

"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
Expand All @@ -32,7 +31,6 @@ import (
// DockerMachinePoolReconciler reconciles a DockerMachinePool object.
type DockerMachinePoolReconciler struct {
Client client.Client
Scheme *runtime.Scheme
ContainerRuntime container.Runtime

// WatchFilterValue is the label value used to filter events prior to reconciliation.
Expand All @@ -43,7 +41,6 @@ type DockerMachinePoolReconciler struct {
func (r *DockerMachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
return (&dockermachinepoolcontrollers.DockerMachinePoolReconciler{
Client: r.Client,
Scheme: r.Scheme,
ContainerRuntime: r.ContainerRuntime,
WatchFilterValue: r.WatchFilterValue,
}).SetupWithManager(ctx, mgr, options)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (

"github.com/pkg/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
kerrors "k8s.io/apimachinery/pkg/util/errors"
Expand Down Expand Up @@ -63,7 +62,6 @@ const (
// DockerMachinePoolReconciler reconciles a DockerMachinePool object.
type DockerMachinePoolReconciler struct {
Client client.Client
Scheme *runtime.Scheme
ContainerRuntime container.Runtime

// WatchFilterValue is the label value used to filter events prior to reconciliation.
Expand Down Expand Up @@ -157,6 +155,10 @@ func (r *DockerMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Re

// SetupWithManager will add watches for this controller.
func (r *DockerMachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.ContainerRuntime == nil {
return errors.New("Client and ContainerRuntime must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "dockermachinepool")
clusterToDockerMachinePools, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &infraexpv1.DockerMachinePoolList{}, mgr.GetScheme())
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,9 @@ func (r *DockerClusterReconciler) reconcileDelete(ctx context.Context, dockerClu

// SetupWithManager will add watches for this controller.
func (r *DockerClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.ContainerRuntime == nil {
return errors.New("Client and ContainerRuntime must not be nil")
}
predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "dockercluster")
err := ctrl.NewControllerManagedBy(mgr).
For(&infrav1.DockerCluster{}).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,10 @@ func (r *DockerMachineReconciler) reconcileDelete(ctx context.Context, dockerClu

// SetupWithManager will add watches for this controller.
func (r *DockerMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.ContainerRuntime == nil || r.ClusterCache == nil {
return errors.New("Client, ContainerRuntime and ClusterCache must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "dockermachine")
clusterToDockerMachines, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &infrav1.DockerMachineList{}, mgr.GetScheme())
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,10 @@ func (r *InMemoryClusterReconciler) reconcileDelete(_ context.Context, cluster *

// SetupWithManager will add watches for this controller.
func (r *InMemoryClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.InMemoryManager == nil || r.APIServerMux == nil {
return errors.New("Client, InMemoryManager and APIServerMux must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "inmemorycluster")
err := ctrl.NewControllerManagedBy(mgr).
For(&infrav1.InMemoryCluster{}).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1151,6 +1151,10 @@ func (r *InMemoryMachineReconciler) reconcileDeleteControllerManager(ctx context

// SetupWithManager will add watches for this controller.
func (r *InMemoryMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
if r.Client == nil || r.InMemoryManager == nil || r.APIServerMux == nil {
return errors.New("Client, InMemoryManager and APIServerMux must not be nil")
}

predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "inmemorymachine")
clusterToInMemoryMachines, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &infrav1.InMemoryMachineList{}, mgr.GetScheme())
if err != nil {
Expand Down

0 comments on commit e5cbc4a

Please sign in to comment.