Merge pull request #4 from rafaeldtinoco/pr3632-nits

chore(controller): minor changes to controller

builder/Makefile.k8s

@@ -72,26 +72,26 @@ K8S_CONTNAME = tracee-k8s:latest
 .PHONY: build
 build: \
 	.check_$(CMD_DOCKER) \
-	.check_tree \
+	.check_tree
 #
 	$(CMD_DOCKER) build \
 		-f builder/Dockerfile.k8s \
 		-t $(K8S_CONTNAME) \
 		.
 
 .PHONY: manifests
-manifests:
+manifests: \
 	.check_$(CMD_DOCKER) \
-	.check_tree \
+	.check_tree
 #
 	$(CMD_DOCKER) run --rm \
 		-v $(shell pwd):/tracee \
 		$(K8S_CONTNAME) make k8s-manifests
 
 .PHONY: generate
-generate:
+generate: \
 	.check_$(CMD_DOCKER) \
-	.check_tree \
+	.check_tree
 #
 	$(CMD_DOCKER) run --rm \
 		-v $(shell pwd):/tracee \

cmd/tracee-operator/main.go

@@ -27,59 +27,52 @@ func init() {
 	utilruntime.Must(policyv1beta1.AddToScheme(scheme))
 }
 
-func main() {
-	var metricsAddr string
-	var enableLeaderElection bool
-	var probeAddr string
-	var traceeNamespace string
-	var traceeName string
-
-	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
-	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
-	flag.StringVar(&traceeNamespace, "tracee-namespace", "tracee-system", "The namespace where Tracee is installed.")
-	flag.StringVar(&traceeName, "tracee-name", "tracee", "The name of the Tracee DaemonSet.")
-	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
-		"Enable leader election for controller manager. "+
-			"Enabling this will ensure there is only one active controller manager.")
-	opts := zap.Options{
-		Development: true,
-	}
-	opts.BindFlags(flag.CommandLine)
-	flag.Parse()
+type Config struct {
+	MetricsAddr          string
+	ProbeAddr            string
+	TraceeNamespace      string
+	TraceeName           string
+	EnableLeaderElection bool
+	LoggingOpts          zap.Options
+}
 
-	// if there are env vars set, use them instead of the flags
-	if namespace := os.Getenv("TRACEE_NAMESPACE"); namespace != "" {
-		traceeNamespace = namespace
-	}
+func main() {
+	config := parseConfig()
 
-	if name := os.Getenv("TRACEE_NAME"); name != "" {
-		traceeName = name
-	}
+	// Set up a logger for the controller manager (prefix: "setup").
+	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&config.LoggingOpts)))
 
-	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
+	// Create a controller manager that will take care of caching, syncing, and making
+	// calls to the API server.
 
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Scheme:                 scheme,
-		Metrics:                metricsserver.Options{BindAddress: metricsAddr},
-		HealthProbeBindAddress: probeAddr,
-		LeaderElection:         enableLeaderElection,
+		Metrics:                metricsserver.Options{BindAddress: config.MetricsAddr},
+		HealthProbeBindAddress: config.ProbeAddr,
+		LeaderElection:         config.EnableLeaderElection,
 		LeaderElectionID:       "ecaf1259.my.domain",
 	})
 	if err != nil {
 		setupLog.Error(err, "unable to start manager")
 		os.Exit(1)
 	}
 
-	if err = (&controller.PolicyReconciler{
+	// Register the PolicyReconciler with the controller manager. This will cause the
+	// PolicyReconciler to watch for changes to Policy objects and deal with them.
+
+	reconciler := &controller.PolicyReconciler{
 		Client:          mgr.GetClient(),
 		Scheme:          mgr.GetScheme(),
-		TraceeNamespace: traceeNamespace,
-		TraceeName:      traceeName,
-	}).SetupWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "CronJob")
+		TraceeNamespace: config.TraceeNamespace,
+		TraceeName:      config.TraceeName,
+	}
+	if err := reconciler.SetupWithManager(mgr); err != nil {
+		setupLog.Error(err, "unable to create controller", "controller", "PolicyReconciler")
 		os.Exit(1)
 	}
 
+	// Create health and readyz endpoints for the controller manager to report its health.
+
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
 		setupLog.Error(err, "unable to set up health check")
 		os.Exit(1)
@@ -89,9 +82,38 @@ func main() {
 		os.Exit(1)
 	}
 
+	// Start the controller manager. This will block until the controller manager is
+	// stopped or until a fatal error occurs.
+
 	setupLog.Info("starting manager")
 	if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
 		setupLog.Error(err, "problem running manager")
 		os.Exit(1)
 	}
 }
+
+// parseConfig parses the command line flags and returns a Config struct
+func parseConfig() Config {
+	var cfg Config
+
+	flag.StringVar(&cfg.MetricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
+	flag.StringVar(&cfg.ProbeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
+	flag.StringVar(&cfg.TraceeNamespace, "tracee-namespace", "tracee-system", "The namespace where Tracee is installed.")
+	flag.StringVar(&cfg.TraceeName, "tracee-name", "tracee", "The name of the Tracee DaemonSet.")
+	flag.BoolVar(&cfg.EnableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.")
+	cfg.LoggingOpts = zap.Options{
+		Development: true,
+	}
+	cfg.LoggingOpts.BindFlags(flag.CommandLine)
+
+	flag.Parse()
+
+	if namespace := os.Getenv("TRACEE_NAMESPACE"); namespace != "" {
+		cfg.TraceeNamespace = namespace
+	}
+	if name := os.Getenv("TRACEE_NAME"); name != "" {
+		cfg.TraceeName = name
+	}
+
+	return cfg
+}

pkg/cmd/cobra/cobra.go

@@ -141,69 +141,64 @@ func GetTraceeRunner(c *cobra.Command, version string) (cmd.Runner, error) {
 		return runner, err
 	}
 
+	// Scope command line flags - via cobra flag
+
 	scopeFlags, err := c.Flags().GetStringArray("scope")
 	if err != nil {
 		return runner, err
 	}
+	if len(policyFlags) > 0 && len(scopeFlags) > 0 {
+		return runner, errors.New("policy and scope flags cannot be used together")
+	}
+
+	// Events command line flags - via cobra flag
 
 	eventFlags, err := c.Flags().GetStringArray("events")
 	if err != nil {
 		return runner, err
 	}
-
-	if len(policyFlags) > 0 && len(scopeFlags) > 0 {
-		return runner, errors.New("policy and scope flags cannot be used together")
-	}
 	if len(policyFlags) > 0 && len(eventFlags) > 0 {
 		return runner, errors.New("policy and event flags cannot be used together")
 	}
 
+	// Try to get policies from kubernetes CRD, policy files and CLI in that order
+
 	var k8sPolicies []v1beta1.PolicyInterface
+	var policies *policy.Policies
 
 	k8sClient, err := k8s.New()
+	if err == nil {
+		k8sPolicies, err = k8sClient.GetPolicy(c.Context())
+	}
 	if err != nil {
 		logger.Debugw("kubernetes cluster", "error", err)
 	}
-
-	// if k8sClient is not nil, then we are running inside a k8s cluster
-	if k8sClient != nil {
-		k8sPolicies, err = k8sClient.GetPolicy(c.Context())
-		if err != nil {
-			logger.Debugw("kubernetes cluster", "error", err)
-		}
-	}
-
-	var policies *policy.Policies
-
 	if len(k8sPolicies) > 0 {
 		logger.Debugw("using policies from kubernetes crd")
-
-		if len(k8sPolicies) > 0 {
-			policies, err = createPoliciesFromK8SPolicy(k8sPolicies)
-		}
+		policies, err = createPoliciesFromK8SPolicy(k8sPolicies)
 	} else if len(policyFlags) > 0 {
 		logger.Debugw("using policies from --policy flag")
 		policies, err = createPoliciesFromPolicyFiles(policyFlags)
 	} else {
 		logger.Debugw("using policies from --scope and --events flag")
 		policies, err = createPoliciesFromCLIFlags(scopeFlags, eventFlags)
 	}
-
-	// if any error returned while creating policies, return it
 	if err != nil {
 		return runner, err
 	}
 
 	cfg.Policies = policies
 
 	// Output command line flags
+
 	output, err := flags.PrepareOutput(viper.GetStringSlice("output"), true)
 	if err != nil {
 		return runner, err
 	}
 	cfg.Output = output.TraceeConfig
 
 	// Create printer
+
 	p, err := printer.NewBroadcast(output.PrinterConfigs, cmd.GetContainerMode(cfg))
 	if err != nil {
 		return runner, err

pkg/k8s/controller/controller.go

@@ -13,7 +13,9 @@ import (
 	"github.com/aquasecurity/tracee/pkg/k8s/apis/tracee.aquasec.com/v1beta1"
 )
 
-// PolicyReconciler reconciles a CronJob object
+// PolicyReconciler is the main controller for the Tracee Policy CRD. It is responsible
+// for updating the Tracee DaemonSet whenever a change is detected in a TraceePolicy
+// object.
 type PolicyReconciler struct {
 	client.Client
 	Scheme          *runtime.Scheme
@@ -24,10 +26,11 @@ type PolicyReconciler struct {
 // +kubebuilder:rbac:groups=tracee.aquasec.com,resources=policies,verbs=get;list;watch;
 // +kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=get;list;watch;patch;update;
 
-// Reconcile is part of the main kubernetes reconciliation loop which aims to
-// move the current state of the cluster closer to the desired state.
-// For more details, check Reconcile and its Result here:
-// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
+// Reconcile is where the reconciliation logic resides. Every time a change is detected in
+// a v1beta1.Policy object, this function will be called. It will update the Tracee
+// DaemonSet, so that the Tracee pods will be restarted with the new policy. It does this
+// by adding a timestamp annotation to the pod template, so that the daemonset controller
+// will rollout a new daemonset ("restarting" the daemonset).
 func (r *PolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	logger := log.FromContext(ctx)
 
@@ -59,7 +62,9 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 	return ctrl.Result{}, nil
 }
 
-// SetupWithManager sets up the controller with the Manager.
+// SetupWithManager is responsible for connecting the PolicyReconciler to the main
+// controller manager. It tells the manager that for changes in v1beta1Policy objects, the
+// PolicyReconciler should be invoked.
 func (r *PolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&v1beta1.Policy{}).