Fix: improve operator robustness, performance and code quality

api/v1alphav1/overcommitclass_types.go

@@ -8,7 +8,6 @@ package v1alphav1
 import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 )
 
 // EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
@@ -68,146 +67,6 @@ type OvercommitClass struct {
 	Status OvercommitClassStatus `json:"status,omitempty"`
 }
 
-// GetAnnotations implements v1.Object.
-func (in *OvercommitClass) GetAnnotations() map[string]string {
-	return in.ObjectMeta.Annotations
-}
-
-// GetCreationTimestamp implements v1.Object.
-func (in *OvercommitClass) GetCreationTimestamp() metav1.Time {
-	return in.ObjectMeta.CreationTimestamp
-}
-
-// GetDeletionGracePeriodSeconds implements v1.Object.
-func (in *OvercommitClass) GetDeletionGracePeriodSeconds() *int64 {
-	return in.ObjectMeta.DeletionGracePeriodSeconds
-}
-
-// GetDeletionTimestamp implements v1.Object.
-func (in *OvercommitClass) GetDeletionTimestamp() *metav1.Time {
-	return in.ObjectMeta.DeletionTimestamp
-}
-
-// GetFinalizers implements v1.Object.
-func (in *OvercommitClass) GetFinalizers() []string {
-	return in.ObjectMeta.Finalizers
-}
-
-// GetGenerateName implements v1.Object.
-func (in *OvercommitClass) GetGenerateName() string {
-	return in.ObjectMeta.GenerateName
-}
-
-// GetGeneration implements v1.Object.
-func (in *OvercommitClass) GetGeneration() int64 {
-	return in.ObjectMeta.Generation
-}
-
-// GetLabels implements v1.Object.
-func (in *OvercommitClass) GetLabels() map[string]string {
-	return in.ObjectMeta.Labels
-}
-
-// GetManagedFields implements v1.Object.
-func (in *OvercommitClass) GetManagedFields() []metav1.ManagedFieldsEntry {
-	return in.ObjectMeta.ManagedFields
-}
-
-// GetName implements v1.Object.
-func (in *OvercommitClass) GetName() string {
-	return in.ObjectMeta.Name
-}
-
-// GetNamespace implements v1.Object.
-func (in *OvercommitClass) GetNamespace() string {
-	return in.ObjectMeta.Namespace
-}
-
-// GetOwnerReferences implements v1.Object.
-func (in *OvercommitClass) GetOwnerReferences() []metav1.OwnerReference {
-	return in.ObjectMeta.OwnerReferences
-}
-
-// GetResourceVersion implements v1.Object.
-func (in *OvercommitClass) GetResourceVersion() string {
-	return in.ObjectMeta.ResourceVersion
-}
-
-// GetUID implements v1.Object.
-func (in *OvercommitClass) GetUID() types.UID {
-	return in.ObjectMeta.UID
-}
-
-// SetAnnotations implements v1.Object.
-func (in *OvercommitClass) SetAnnotations(annotations map[string]string) {
-	in.ObjectMeta.Annotations = annotations
-}
-
-// SetCreationTimestamp implements v1.Object.
-func (in *OvercommitClass) SetCreationTimestamp(timestamp metav1.Time) {
-	in.ObjectMeta.CreationTimestamp = timestamp
-}
-
-// SetDeletionGracePeriodSeconds implements v1.Object.
-func (in *OvercommitClass) SetDeletionGracePeriodSeconds(seconds *int64) {
-	in.ObjectMeta.DeletionGracePeriodSeconds = seconds
-}
-
-// SetDeletionTimestamp implements v1.Object.
-func (in *OvercommitClass) SetDeletionTimestamp(timestamp *metav1.Time) {
-	in.ObjectMeta.DeletionTimestamp = timestamp
-}
-
-// SetFinalizers implements v1.Object.
-func (in *OvercommitClass) SetFinalizers(finalizers []string) {
-	in.ObjectMeta.Finalizers = finalizers
-}
-
-// SetGenerateName implements v1.Object.
-func (in *OvercommitClass) SetGenerateName(name string) {
-	in.ObjectMeta.GenerateName = name
-}
-
-// SetGeneration implements v1.Object.
-func (in *OvercommitClass) SetGeneration(generation int64) {
-	in.ObjectMeta.Generation = generation
-}
-
-// SetLabels implements v1.Object.
-func (in *OvercommitClass) SetLabels(labels map[string]string) {
-	in.ObjectMeta.Labels = labels
-}
-
-// SetManagedFields implements v1.Object.
-func (in *OvercommitClass) SetManagedFields(managedFields []metav1.ManagedFieldsEntry) {
-	in.ObjectMeta.ManagedFields = managedFields
-}
-
-// SetName implements v1.Object.
-func (in *OvercommitClass) SetName(name string) {
-	in.ObjectMeta.Name = name
-}
-
-// SetNamespace implements v1.Object.
-func (in *OvercommitClass) SetNamespace(namespace string) {
-	in.ObjectMeta.Namespace = namespace
-}
-
-// SetOwnerReferences implements v1.Object.
-func (in *OvercommitClass) SetOwnerReferences(references []metav1.OwnerReference) {
-	in.ObjectMeta.OwnerReferences = references
-}
-
-// SetResourceVersion implements v1.Object.
-func (in *OvercommitClass) SetResourceVersion(version string) {
-	in.ObjectMeta.ResourceVersion = version
-}
-
-// SetUID implements v1.Object.
-func (in *OvercommitClass) SetUID(uid types.UID) {
-	in.ObjectMeta.UID = uid
-}
-
 // +kubebuilder:object:root=true
 
 // OvercommitClassList contains a list of OvercommitClass

api/v1alphav1/webhook_validations.go

@@ -70,9 +70,14 @@ func isClassDefault(class OvercommitClass, client client.Client) error {
 }
 
 func checkIsRegexValid(regex string) error {
+	// Limit regex length to prevent ReDoS (catastrophic backtracking)
+	const maxRegexLen = 512
+	if len(regex) > maxRegexLen {
+		return fmt.Errorf("regex is too long (%d chars), maximum allowed is %d", len(regex), maxRegexLen)
+	}
 	_, err := regexp.Compile(regex)
 	if err != nil {
-		return errors.New("Error: the regex is not valid")
+		return fmt.Errorf("invalid regex for excludedNamespaces: %w", err)
 	}
 	return nil
 }

cmd/main.go

@@ -150,7 +150,7 @@ func main() {
 			os.Exit(1)
 		}
 
-		serviceAccountName, err := utils.GetPodServiceAccount(mgr.GetAPIReader())
+		serviceAccountName, err := utils.GetPodServiceAccount(ctx, mgr.GetAPIReader())
 		if err != nil {
 			setupLog.Error(err, "unable to get pod service account")
 			os.Exit(1)

go.mod

@@ -12,7 +12,6 @@ require (
 	k8s.io/apimachinery v0.32.1
 	k8s.io/client-go v0.32.1
 	sigs.k8s.io/controller-runtime v0.19.0
-	sigs.k8s.io/yaml v1.4.0
 )
 
 require (
@@ -101,4 +100,5 @@ require (
 	sigs.k8s.io/gateway-api v1.1.0 // indirect
 	sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.5.0 // indirect
+	sigs.k8s.io/yaml v1.4.0 // indirect
 )

internal/controller/overcommit/overcommit_controller.go

@@ -390,11 +390,8 @@ func (r *OvercommitReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		// Don't fail the reconciliation for status update errors
 	}
 
-	// Only requeue periodically for status checks, not immediately
-	logger.Info("Reconciliation completed successfully", "nextReconcile", "10 seconds", "time", time.Now().Format("15:04:05"))
-	return ctrl.Result{
-		RequeueAfter: time.Second * 10,
-	}, nil
+	logger.Info("Reconciliation completed successfully", "time", time.Now().Format("15:04:05"))
+	return ctrl.Result{}, nil
 }
 
 // +kubebuilder:rbac:groups=apps, resources=deployments;replicasets,verbs=get;list;watch;create;update;patch;delete

internal/controller/overcommitclass/overcommitclass_controller.go

@@ -393,9 +393,6 @@ func (r *OvercommitClassReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return ctrl.Result{}, err
 	}
 
-	// Only requeue periodically for status checks, not immediately
-	logger.Info("Reconciliation completed successfully", "nextReconcile", "10 seconds", "time", time.Now().Format("15:04:05"))
-	return ctrl.Result{
-		RequeueAfter: 10 * time.Second,
-	}, nil
+	logger.Info("Reconciliation completed successfully", "time", time.Now().Format("15:04:05"))
+	return ctrl.Result{}, nil
 }

internal/resources/generate_resources_pod_mutating_webhooks.go

@@ -147,10 +147,10 @@ func CreateCertificate(name string, svc corev1.Service) *certmanager.Certificate
 		Spec: certmanager.CertificateSpec{
 			SecretName: name + "-webhook-secret",
 			Duration: &metav1.Duration{
-				Duration: 87600 * time.Hour,
+				Duration: 8760 * time.Hour, // 1 year
 			},
 			RenewBefore: &metav1.Duration{
-				Duration: 720 * time.Hour,
+				Duration: 720 * time.Hour, // 30 days
 			},
 			DNSNames: []string{
 				svc.Name + "." + svc.Namespace + ".svc",

internal/resources/generate_resources_pod_mutating_webhooks_test.go

@@ -123,7 +123,7 @@ func TestCreateCertificate(t *testing.T) {
 		t.Errorf("Expected secret name 'test-class-webhook-secret', got '%s'", certificate.Spec.SecretName)
 	}
 
-	expectedDuration := 87600 * time.Hour
+	expectedDuration := 8760 * time.Hour
 	if certificate.Spec.Duration.Duration != expectedDuration {
 		t.Errorf("Expected duration '%v', got '%v'", expectedDuration, certificate.Spec.Duration.Duration)
 	}

internal/utils/getOvercommitClass.go

@@ -45,14 +45,14 @@ func GetOvercommitClassSpec(ctx context.Context, name string, k8sClient client.C
 	return &overcommitClass.Spec, nil
 }
 
-func GetDefaultSpec(k8sClient client.Client) (*overcommit.OvercommitClassSpec, error) {
+func GetDefaultSpec(ctx context.Context, k8sClient client.Client) (*overcommit.OvercommitClassSpec, error) {
 	if k8sClient == nil {
 		return nil, errors.New("client parameter cannot be nil")
 	}
 
 	// List all OvercommitClass
 	var overcommitClasses overcommit.OvercommitClassList
-	if err := k8sClient.List(context.Background(), &overcommitClasses); err != nil {
+	if err := k8sClient.List(ctx, &overcommitClasses); err != nil {
 		return nil, fmt.Errorf("error listing OvercommitClass: %w", err)
 	}
 

internal/utils/getOvercommitClass_test.go

@@ -82,7 +82,7 @@ var _ = Describe("GetDefaultSpec", func() {
 
 	It("should retrieve the default OvercommitClassSpec correctly", func() {
 		// Try the GetDefaultSpec function
-		spec, err := GetDefaultSpec(k8sClient)
+		spec, err := GetDefaultSpec(context.Background(), k8sClient)
 		Expect(err).NotTo(HaveOccurred(), "Failed to get default OvercommitClassSpec")
 		Expect(spec).NotTo(BeNil(), "Spec should not be nil")
 		Expect(spec.IsDefault).To(BeTrue(), "Spec.IsDefault should be true")

internal/utils/getPodDetails.go

@@ -58,7 +58,7 @@ func GetPodImageDetails(ctx context.Context, client client.Reader) (string, stri
 	return "", "", "", fmt.Errorf("no containers found in pod")
 }
 
-func GetPodServiceAccount(client client.Reader) (string, error) {
+func GetPodServiceAccount(ctx context.Context, client client.Reader) (string, error) {
 	podName := os.Getenv("POD_NAME")
 	podNamespace := os.Getenv("POD_NAMESPACE")
 
@@ -67,7 +67,7 @@ func GetPodServiceAccount(client client.Reader) (string, error) {
 	}
 
 	pod := &corev1.Pod{}
-	err := client.Get(context.TODO(), types.NamespacedName{
+	err := client.Get(ctx, types.NamespacedName{
 		Name:      podName,
 		Namespace: podNamespace,
 	}, pod)

internal/webhook/v1alphav1/mutating/pod_webhook.go

@@ -51,11 +51,11 @@ func (d *PodCustomDefaulter) Default(ctx context.Context, obj runtime.Object) er
 	}
 
 	if isResize {
-		overcommit.OvercommitOnResize(pod, d.Recorder, d.Client)
+		overcommit.OvercommitOnResize(ctx, pod, d.Recorder, d.Client)
 		return nil
 	}
 
-	overcommit.Overcommit(pod, d.Recorder, d.Client)
+	overcommit.Overcommit(ctx, pod, d.Recorder, d.Client)
 	return nil
 }