ambient: fix nil pointer when pod cache is stale

I ran into this in an *extremely* bespoke and unsupported environment,
but I think it could occur in real world. We are looping outside of
GetPodIfAmbient for the pod to show up, but if it fails we panic. We
want to instead get an error.

cni/pkg/nodeagent/cni-watcher.go

@@ -169,10 +169,7 @@ func (s *CniPluginServer) ReconcileCNIAddEvent(ctx context.Context, addCmd CNIPl
 
 	log.Debugf("Checking pod: %s in ns: %s is enabled for ambient", addCmd.PodName, addCmd.PodNamespace)
 	// The plugin already consulted the k8s API - but on this end handler caches may be stale, so retry a few times if we get no pod.
-	for ambientPod, err = s.handlers.GetPodIfAmbient(addCmd.PodName, addCmd.PodNamespace); (ambientPod == nil) && (retries < maxStaleRetries); retries++ {
-		if err != nil {
-			return err
-		}
+	for ambientPod = s.handlers.GetPodIfAmbient(addCmd.PodName, addCmd.PodNamespace); (ambientPod == nil) && (retries < maxStaleRetries); retries++ {
 		log.Warnf("got an event for pod %s in namespace %s not found in current pod cache, retry %d of %d",
 			addCmd.PodName, addCmd.PodNamespace, retries, maxStaleRetries)
 		time.Sleep(time.Duration(msInterval) * time.Millisecond)

cni/pkg/nodeagent/informers.go

@@ -39,7 +39,7 @@ var (
 )
 
 type K8sHandlers interface {
-	GetPodIfAmbient(podName, podNamespace string) (*corev1.Pod, error)
+	GetPodIfAmbient(podName, podNamespace string) *corev1.Pod
 	GetAmbientPods() []*corev1.Pod
 	Start()
 }
@@ -82,16 +82,19 @@ func setupHandlers(ctx context.Context, kubeClient kube.Client, dataplane MeshDa
 	return s
 }
 
-func (s *InformerHandlers) GetPodIfAmbient(podName, podNamespace string) (*corev1.Pod, error) {
+func (s *InformerHandlers) GetPodIfAmbient(podName, podNamespace string) *corev1.Pod {
 	ns := s.namespaces.Get(podNamespace, "")
 	if ns == nil {
-		return nil, fmt.Errorf("failed to find namespace %v", ns)
+		return nil
 	}
 	pod := s.pods.Get(podName, podNamespace)
+	if pod == nil {
+		return nil
+	}
 	if util.PodRedirectionEnabled(ns, pod) {
-		return pod, nil
+		return pod
 	}
-	return nil, nil
+	return nil
 }
 
 func (s *InformerHandlers) Start() {

cni/pkg/nodeagent/informers_test.go

@@ -435,9 +435,9 @@ func TestAmbientEnabledReturnsPodIfEnabled(t *testing.T) {
 
 	handlers := setupHandlers(ctx, client, server, "istio-system")
 	client.RunAndWait(ctx.Done())
-	_, err := handlers.GetPodIfAmbient(pod.Name, ns.Name)
+	p := handlers.GetPodIfAmbient(pod.Name, ns.Name)
 
-	assert.NoError(t, err)
+	assert.Equal(t, p, pod)
 }
 
 func TestAmbientEnabledReturnsNoPodIfNotEnabled(t *testing.T) {
@@ -478,9 +478,8 @@ func TestAmbientEnabledReturnsNoPodIfNotEnabled(t *testing.T) {
 
 	handlers := setupHandlers(ctx, client, server, "istio-system")
 	client.RunAndWait(ctx.Done())
-	disabledPod, err := handlers.GetPodIfAmbient(pod.Name, ns.Name)
+	disabledPod := handlers.GetPodIfAmbient(pod.Name, ns.Name)
 
-	assert.NoError(t, err)
 	assert.Equal(t, disabledPod, nil)
 }
 
@@ -522,9 +521,8 @@ func TestAmbientEnabledReturnsErrorIfBogusNS(t *testing.T) {
 
 	handlers := setupHandlers(ctx, client, server, "istio-system")
 	client.RunAndWait(ctx.Done())
-	disabledPod, err := handlers.GetPodIfAmbient(pod.Name, "what")
+	disabledPod := handlers.GetPodIfAmbient(pod.Name, "what")
 
-	assert.Error(t, err)
 	assert.Equal(t, disabledPod, nil)
 }