refactor: cleanup

Signed-off-by: Andrea Terzolo <andrea.terzolo@suse.com>

Author: Andreagit97

.golangci.yml

@@ -0,0 +1,13 @@
+fail_fast: false
+minimum_pre_commit_version: '0'
+repos:
+  # more info on the usage: https://github.com/golangci/golangci-lint/blob/main/.pre-commit-hooks.yaml
+  - repo: https://github.com/golangci/golangci-lint
+    rev: c0d3ddc9cf3faa61a4e378e879ece580256d76e5 # v2.12.2
+    hooks:
+      - id: golangci-lint-full
+        language_version: 1.26.4
+        stages: [pre-commit]
+      - id: golangci-lint-fmt
+        language_version: 1.26.4
+        stages: [pre-commit]

.pre-commit-config.yaml

@@ -79,8 +79,6 @@ test: manifests generate fmt vet setup-envtest ## Run tests.
 
 # TODO(user): To use a different vendor for e2e tests, modify the setup under 'tests/e2e'.
 # The default setup assumes Kind is pre-installed and builds/loads the Manager Docker image locally.
-# CertManager is installed by default; skip with:
-# - CERT_MANAGER_INSTALL_SKIP=true
 KIND_CLUSTER ?= network-enforcer-test-e2e
 
 .PHONY: setup-test-e2e

Makefile

@@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+// Package v1alpha1 contains API Schema definitions for the security v1alpha1 API group.
 // +kubebuilder:object:generate=true
 // +groupName=security.security.rancher.io
 package v1alpha1
@@ -24,7 +25,10 @@ import (
 )
 
 var (
-	GroupVersion  = schema.GroupVersion{Group: "security.security.rancher.io", Version: "v1alpha1"}
+	//nolint:gochecknoglobals // Kubebuilder API registration requires package-level variables
+	GroupVersion = schema.GroupVersion{Group: "security.security.rancher.io", Version: "v1alpha1"}
+	//nolint:gochecknoglobals // Kubebuilder API registration requires package-level variables
 	SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion}
-	AddToScheme   = SchemeBuilder.AddToScheme
+	//nolint:gochecknoglobals // Kubebuilder API registration requires package-level variables
+	AddToScheme = SchemeBuilder.AddToScheme
 )

api/v1alpha1/groupversion_info.go

@@ -56,6 +56,7 @@ type NetworkPolicyProposalList struct {
 	Items []NetworkPolicyProposal `json:"items"`
 }
 
+//nolint:gochecknoinits // Kubebuilder API types register themselves during package initialization
 func init() {
 	SchemeBuilder.Register(&NetworkPolicyProposal{}, &NetworkPolicyProposalList{})
 }

api/v1alpha1/networkpolicyproposal_types.go

@@ -19,9 +19,12 @@ package main
 import (
 	"crypto/tls"
 	"flag"
+	"fmt"
 	"log/slog"
 	"os"
 
+	"sigs.k8s.io/controller-runtime/pkg/manager"
+
 	"github.com/go-logr/logr"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 
@@ -32,7 +35,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
-	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	securityv1alpha1 "secuity.rancher.io/network-enforcer/api/v1alpha1"
 	backendkubernetes "secuity.rancher.io/network-enforcer/internal/backend/kubernetes"
@@ -42,139 +44,135 @@ import (
 	// +kubebuilder:scaffold:imports
 )
 
-var (
-	scheme   = runtime.NewScheme()
-	setupLog = ctrl.Log.WithName("setup")
-)
-
-func init() {
-	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
-	utilruntime.Must(securityv1alpha1.AddToScheme(scheme))
-	// +kubebuilder:scaffold:scheme
+type config struct {
+	metricsAddr          string
+	metricsCertPath      string
+	metricsCertName      string
+	metricsCertKey       string
+	enableLeaderElection bool
+	probeAddr            string
+	secureMetrics        bool
+	enableHTTP2          bool
+	otlpPort             int
+	tlsOpts              []func(*tls.Config)
 }
 
-func main() {
-	var metricsAddr string
-	var metricsCertPath, metricsCertName, metricsCertKey string
-	var webhookCertPath, webhookCertName, webhookCertKey string
-	var enableLeaderElection bool
-	var probeAddr string
-	var secureMetrics bool
-	var enableHTTP2 bool
-	var otlpPort int
-	var tlsOpts []func(*tls.Config)
-	flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+
-		"Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.")
-	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
-	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
-		"Enable leader election for controller manager. "+
-			"Enabling this will ensure there is only one active controller manager.")
-	flag.BoolVar(&secureMetrics, "metrics-secure", true,
-		"If set, the metrics endpoint is served securely via HTTPS. Use --metrics-secure=false to use HTTP instead.")
-	flag.StringVar(&webhookCertPath, "webhook-cert-path", "", "The directory that contains the webhook certificate.")
-	flag.StringVar(&webhookCertName, "webhook-cert-name", "tls.crt", "The name of the webhook certificate file.")
-	flag.StringVar(&webhookCertKey, "webhook-cert-key", "tls.key", "The name of the webhook key file.")
-	flag.StringVar(&metricsCertPath, "metrics-cert-path", "",
-		"The directory that contains the metrics server certificate.")
-	flag.StringVar(&metricsCertName, "metrics-cert-name", "tls.crt", "The name of the metrics server certificate file.")
-	flag.StringVar(&metricsCertKey, "metrics-cert-key", "tls.key", "The name of the metrics server key file.")
-	flag.BoolVar(&enableHTTP2, "enable-http2", false,
-		"If set, HTTP/2 will be enabled for the metrics and webhook servers")
-	flag.IntVar(&otlpPort, "otlp-port", 4317, "The port the OTLP gRPC receiver listens on.")
-	flag.Parse()
-
-	slogHandler := slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelInfo})
-	slogger := slog.New(slogHandler).With("component", "agent")
-	slog.SetDefault(slogger)
-	ctrl.SetLogger(logr.FromSlogHandler(slogger.Handler()))
-
+func newControllerManager(conf *config) (manager.Manager, error) {
 	// Mitigate HTTP/2 Stream Cancellation / Rapid Reset CVEs.
 	disableHTTP2 := func(c *tls.Config) {
 		c.NextProtos = []string{"http/1.1"}
 	}
 
-	if !enableHTTP2 {
-		tlsOpts = append(tlsOpts, disableHTTP2)
+	if !conf.enableHTTP2 {
+		conf.tlsOpts = append(conf.tlsOpts, disableHTTP2)
 	}
 
-	webhookServerOptions := webhook.Options{
-		TLSOpts: tlsOpts,
-	}
-
-	if len(webhookCertPath) > 0 {
-		webhookServerOptions.CertDir = webhookCertPath
-		webhookServerOptions.CertName = webhookCertName
-		webhookServerOptions.KeyName = webhookCertKey
-	}
-
-	webhookServer := webhook.NewServer(webhookServerOptions)
-
 	metricsServerOptions := metricsserver.Options{
-		BindAddress:   metricsAddr,
-		SecureServing: secureMetrics,
-		TLSOpts:       tlsOpts,
+		BindAddress:   conf.metricsAddr,
+		SecureServing: conf.secureMetrics,
+		TLSOpts:       conf.tlsOpts,
 	}
 
-	if secureMetrics {
+	if conf.secureMetrics {
 		metricsServerOptions.FilterProvider = filters.WithAuthenticationAndAuthorization
 	}
 
-	if len(metricsCertPath) > 0 {
-		metricsServerOptions.CertDir = metricsCertPath
-		metricsServerOptions.CertName = metricsCertName
-		metricsServerOptions.KeyName = metricsCertKey
+	if len(conf.metricsCertPath) > 0 {
+		metricsServerOptions.CertDir = conf.metricsCertPath
+		metricsServerOptions.CertName = conf.metricsCertName
+		metricsServerOptions.KeyName = conf.metricsCertKey
 	}
 
-	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
+	scheme := runtime.NewScheme()
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
+	utilruntime.Must(securityv1alpha1.AddToScheme(scheme))
+	controllerOptions := ctrl.Options{
 		Scheme:                 scheme,
 		Metrics:                metricsServerOptions,
-		WebhookServer:          webhookServer,
-		HealthProbeBindAddress: probeAddr,
-		LeaderElection:         enableLeaderElection,
+		HealthProbeBindAddress: conf.probeAddr,
+		LeaderElection:         conf.enableLeaderElection,
 		LeaderElectionID:       "6163c1ee.security.rancher.io",
-	})
+	}
+	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), controllerOptions)
 	if err != nil {
-		setupLog.Error(err, "unable to start manager")
-		os.Exit(1)
+		return nil, fmt.Errorf("unable to start manager: %w", err)
+	}
+	return mgr, nil
+}
+
+func run(logger *slog.Logger, conf *config) error {
+	mgr, err := newControllerManager(conf)
+	if err != nil {
+		return fmt.Errorf("unable to create controller manager: %w", err)
 	}
 
 	store := topology.NewStore()
 
-	receiver := flowcollector.NewReceiver(store, otlpPort, slogger)
-	if err := mgr.Add(receiver); err != nil {
-		setupLog.Error(err, "add OTLP receiver")
-		os.Exit(1)
+	receiver := flowcollector.NewReceiver(store, conf.otlpPort, logger)
+	err = mgr.Add(receiver)
+	if err != nil {
+		return fmt.Errorf("unable to add OTLP receiver to manager: %w", err)
 	}
 
-	scanner := controller.NewTopologyScanner(mgr.GetClient(), store, slogger)
-	if err := mgr.Add(scanner); err != nil {
-		setupLog.Error(err, "add topology scanner")
-		os.Exit(1)
+	scanner := controller.NewTopologyScanner(mgr.GetClient(), store, logger)
+	err = mgr.Add(scanner)
+	if err != nil {
+		return fmt.Errorf("unable to add topology scanner to manager: %w", err)
 	}
 
-	if err := (&controller.EnforcementReconciler{
+	err = (&controller.EnforcementReconciler{
 		Client:  mgr.GetClient(),
 		Scheme:  mgr.GetScheme(),
 		Backend: &backendkubernetes.Backend{},
-	}).SetupWithManager(mgr); err != nil {
-		setupLog.Error(err, "setup controller", "controller", "Enforcement")
-		os.Exit(1)
+	}).SetupWithManager(mgr)
+	if err != nil {
+		return fmt.Errorf("unable to setup Enforcement controller: %w", err)
 	}
 
 	// +kubebuilder:scaffold:builder
 
-	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
-		setupLog.Error(err, "healthz check")
-		os.Exit(1)
+	if err = mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
+		return fmt.Errorf("unable to add healthz check: %w", err)
 	}
-	if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil {
-		setupLog.Error(err, "readyz check")
-		os.Exit(1)
+	if err = mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil {
+		return fmt.Errorf("unable to add readyz check: %w", err)
 	}
 
-	setupLog.Info("starting manager")
-	if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
-		setupLog.Error(err, "manager exited")
+	logger.Info("starting manager")
+	return mgr.Start(ctrl.SetupSignalHandler())
+}
+
+func main() {
+	conf := &config{}
+	flag.StringVar(&conf.metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+
+		"Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.")
+	flag.StringVar(&conf.probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
+	flag.BoolVar(&conf.enableLeaderElection, "leader-elect", false,
+		"Enable leader election for controller manager. "+
+			"Enabling this will ensure there is only one active controller manager.")
+	flag.BoolVar(&conf.secureMetrics, "metrics-secure", true,
+		"If set, the metrics endpoint is served securely via HTTPS. Use --metrics-secure=false to use HTTP instead.")
+	flag.StringVar(&conf.metricsCertPath, "metrics-cert-path", "",
+		"The directory that contains the metrics server certificate.")
+	flag.StringVar(
+		&conf.metricsCertName,
+		"metrics-cert-name",
+		"tls.crt",
+		"The name of the metrics server certificate file.",
+	)
+	flag.StringVar(&conf.metricsCertKey, "metrics-cert-key", "tls.key", "The name of the metrics server key file.")
+	flag.BoolVar(&conf.enableHTTP2, "enable-http2", false,
+		"If set, HTTP/2 will be enabled for the metrics server")
+	flag.IntVar(&conf.otlpPort, "otlp-port", 4317, "The port the OTLP gRPC receiver listens on.")
+	flag.Parse()
+
+	slogHandler := slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelInfo})
+	slogger := slog.New(slogHandler).With("component", "agent")
+	slog.SetDefault(slogger)
+	ctrl.SetLogger(logr.FromSlogHandler(slogger.Handler()))
+
+	if err := run(slogger, conf); err != nil {
+		slogger.Error("failed to run", "error", err)
 		os.Exit(1)
 	}
 }

cmd/main.go

@@ -20,6 +20,7 @@
     "ignoreWords": [
         "corev1",
         "metav1",
+        "nolint",
     ],
     "import": []
 }

cspell.json

@@ -17,5 +17,4 @@ type PolicyBackend interface {
 	) client.Object
 	// Empty returns a zero-value instance for client.Get.
 	Empty() client.Object
-	UpdateSpec(existing, desired client.Object)
 }

internal/backend/backend.go

@@ -19,10 +19,6 @@ func (b *Backend) Empty() client.Object {
 	return &networkingv1.NetworkPolicy{}
 }
 
-func (b *Backend) UpdateSpec(existing, desired client.Object) {
-	existing.(*networkingv1.NetworkPolicy).Spec = desired.(*networkingv1.NetworkPolicy).Spec
-}
-
 func (b *Backend) Build(
 	name, namespace string,
 	podSelector map[string]string,

internal/backend/kubernetes/kubernetes.go

@@ -71,9 +71,9 @@ func (r *EnforcementReconciler) SetupWithManager(mgr ctrl.Manager) error {
 				newLabels := e.ObjectNew.GetLabels()
 				return oldLabels[enforceLabelKey] != newLabels[enforceLabelKey]
 			},
-			CreateFunc:  func(e event.CreateEvent) bool { return true },
-			DeleteFunc:  func(e event.DeleteEvent) bool { return false },
-			GenericFunc: func(e event.GenericEvent) bool { return false },
+			CreateFunc:  func(_ event.CreateEvent) bool { return true },
+			DeleteFunc:  func(_ event.DeleteEvent) bool { return false },
+			GenericFunc: func(_ event.GenericEvent) bool { return false },
 		}).
 		Named("enforcement").
 		Complete(r)