feat: prevent containers from running in NRI

As part of error handling, when we failed to apply protection on a
container, we will fail the container creation flow by default.

Users can use NRI_FAILOPEN envvar to change this behavior.

Signed-off-by: Sam Wang (holyspectral) <sam.wang@suse.com>

Author: holyspectral

cspell.json

@@ -84,8 +84,10 @@
         "printk",
         "ringbuf",
         "cgtracker",
-	"runtimeenforcer",
-	"agentv1"
+        "runtimeenforcer",
+        "agentv1",
+        "failopen",
+        "ttrpc"
     ],
     "ignoreWords": [
         "clientgoscheme",
@@ -118,7 +120,9 @@
         "workloadpolicyhandler",
         "rawtime",
         "exepath",
-        "pexepath"
+        "pexepath",
+        "workloadkind",
+        "policymode"
     ],
     "import": []
-}
+}

internal/nri/handler.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"log/slog"
 	"net"
+	"os"
 	"time"
 
 	retry "github.com/avast/retry-go/v4"
@@ -33,6 +34,7 @@ func newNRIPlugin(
 	p := &plugin{
 		logger:   logger.With("component", "nri-plugin"),
 		resolver: resolver,
+		failOpen: os.Getenv("NRI_FAILOPEN") == "true",
 	}
 
 	p.stub, err = stub.New(p, opts...)

internal/nri/plugin.go

@@ -18,6 +18,7 @@ type plugin struct {
 	logger   *slog.Logger
 	resolver *resolver.Resolver
 	lastErr  error
+	failOpen bool
 }
 
 func (p *plugin) getWorkloadInfoAndLog(ctx context.Context, pod *api.PodSandbox) (string, workloadkind.Kind) {
@@ -133,8 +134,13 @@ func (p *plugin) Synchronize(
 			"containers", containers,
 		)
 		if err := p.resolver.AddPodContainerFromNri(podData); err != nil {
+			// This could be recoverable. Returning an error so we can retry.
 			p.logger.ErrorContext(ctx, "failed to add pod container from NRI",
-				"error", err)
+				"pod", pod.GetName(),
+				"namespace", pod.GetNamespace(),
+				"error", err,
+			)
+			return nil, fmt.Errorf("failed to add pod container from NRI: %w", err)
 		}
 	}
 	// Mark resolver as synchronized, so old agent can be safely removed.
@@ -152,13 +158,46 @@ func (p *plugin) StartContainer(
 		"containerName", container.GetName(),
 	)
 
+	handleError := func(reason string, err error) error {
+		if p.failOpen {
+			p.logger.WarnContext(
+				ctx,
+				"container is starting WITHOUT enforcement due to NRI_FAILOPEN",
+				"reason", reason,
+				"error", err,
+				"containerID", container.GetId(),
+				"containerName", container.GetName(),
+				"podName", pod.GetName(),
+				"podID", pod.GetUid(),
+			)
+			return nil
+		}
+		p.logger.ErrorContext(
+			ctx,
+			"The container is prevented from starting. To change this behavior, set environment variable NRI_FAILOPEN to true",
+			"reason",
+			reason,
+			"containerName",
+			container.GetName(),
+			"podName",
+			pod.GetName(),
+			"error",
+			err,
+		)
+		return fmt.Errorf(
+			"%s: %w. The container '%s/%s' is prevented from starting. To change this behavior, set environment variable NRI_FAILOPEN to true",
+			reason,
+			err,
+			pod.GetName(),
+			container.GetName(),
+		)
+	}
+
 	cgroupID, err := cgroupFromContainer(container)
 	if err != nil {
-		// this should never happen but if we are not able to obtain the cgroup ID, it's useless to add the container
-		// to the cache, nobody will ever query this entry into the cache.
-		p.logger.ErrorContext(ctx, "failed to get cgroup ID from container",
-			"error", err)
-		return nil
+		// this should never happen because we've succeeded before in Synchronize() call.
+		// When this happens, it indicates a serious inconsistency in the system.
+		return handleError("failed to get cgroup ID from container", err)
 	}
 
 	workloadName, workloadKind := p.getWorkloadInfoAndLog(ctx, pod)
@@ -174,8 +213,7 @@ func (p *plugin) StartContainer(
 	}
 
 	if err = p.resolver.AddPodContainerFromNri(podData); err != nil {
-		p.logger.ErrorContext(ctx, "failed to add pod container from NRI",
-			"error", err)
+		return handleError("failed to add pod container from NRI", err)
 	}
 	return nil
 }

internal/resolver/nri_api.go

@@ -35,6 +35,8 @@ func (r *Resolver) AddPodContainerFromNri(pod PodInput) error {
 				// If everything is identical, as expected, we can just continue
 				continue
 			}
+			// If it's different, this is unexpected and we return an error to avoid potential issues
+			// with wrong cgroupID -> pod association in the cache.
 			return fmt.Errorf("containerID %s for pod %s already exists. old (name: %s,cID: %d) new (name: %s,cID: %d)",
 				containerID,
 				pod.Meta.Name,
@@ -51,21 +53,20 @@ func (r *Resolver) AddPodContainerFromNri(pod PodInput) error {
 
 		// update the cgtracker map
 		if err := r.cgTrackerUpdateFunc(container.CgroupID, ""); err != nil {
-			r.logger.Error("failed to update cgroup tracker map",
-				"pod", podID,
-				"containerID", containerID,
-				"error", err)
-			continue
+			return fmt.Errorf(
+				"failed to update cgroup tracker map for pod %s, container %s: %w",
+				pod.Meta.Name,
+				container.Name,
+				err,
+			)
 		}
 	}
 
 	// we update back the cache
 	r.podCache[podID] = state
 
 	if err := r.applyPolicyToPodIfPresent(state); err != nil {
-		r.logger.Error("failed to apply policy to pod",
-			"error", err,
-		)
+		return fmt.Errorf("failed to apply policy to pod: %w", err)
 	}
 	return nil
 }

internal/resolver/policy.go

@@ -132,12 +132,19 @@ func (r *Resolver) applyPolicyToPodIfPresent(state *podEntry) error {
 	key := fmt.Sprintf("%s/%s", state.podNamespace(), policyName)
 	info := r.wpState[key]
 	if info == nil {
-		return fmt.Errorf(
-			"pod has policy label but policy does not exist. pod-name: %s, pod-namespace: %s, policy-name: %s",
-			state.podName(),
-			state.podNamespace(),
-			policyName,
+		// This can happen when the pod runs before the policy is created/reconciled when using GitOps to deploy.
+		// After the policy is reconciled, the policy will be applied, so we can safely ignore it for now.
+		//
+		// Another case is that the policy is just not created at all, which is likely an user error.
+		// We log a warning for both cases and return without applying any policy.
+		// This is to avoid the risk of blocking the pod creation unexpectedly.
+		r.logger.Warn(
+			"pod has policy label but policy does not exist. .",
+			"pod-name", state.podName(),
+			"pod-namespace", state.podNamespace(),
+			"policy-name", policyName,
 		)
+		return nil
 	}
 
 	return r.applyPolicyToPod(state, info.polByContainer)