diff --git a/pkg/collect/run_pod.go b/pkg/collect/run_pod.go index e430d6c9..64ab8627 100644 --- a/pkg/collect/run_pod.go +++ b/pkg/collect/run_pod.go @@ -62,13 +62,7 @@ func (c *CollectRunPod) Collect(progressChan chan<- interface{}) (result Collect }() if c.Collector.ImagePullSecret != nil && c.Collector.ImagePullSecret.Data != nil { - defer func() { - if c.Collector.ImagePullSecret.Name != "" { - if err := client.CoreV1().Secrets(pod.Namespace).Delete(ctx, c.Collector.ImagePullSecret.Name, metav1.DeleteOptions{}); err != nil { - klog.Errorf("Failed to delete secret %s: %v", c.Collector.ImagePullSecret.Name, err) - } - } - }() + defer c.deleteImagePullSecret(context.Background(), client, pod) } defer func() { @@ -117,6 +111,27 @@ func (c *CollectRunPod) Collect(progressChan chan<- interface{}) (result Collect } } +func (c *CollectRunPod) deleteImagePullSecret(ctx context.Context, client kubernetes.Interface, pod *corev1.Pod) { + for _, k := range pod.Spec.ImagePullSecrets { + secret, err := client.CoreV1().Secrets(pod.Namespace).Get(ctx, k.Name, metav1.GetOptions{}) + if err != nil { + if kuberneteserrors.IsNotFound(err) { + klog.V(2).Infof("Secret %s in namespace %s not found", k.Name, pod.Namespace) + } else { + klog.Errorf("Failed to get secret %s in namespace %s: %v", k.Name, pod.Namespace, err) + } + continue + } + if secret.Labels["app.kubernetes.io/managed-by"] == "troubleshoot.sh" { + if err := client.CoreV1().Secrets(pod.Namespace).Delete(context.Background(), k.Name, metav1.DeleteOptions{}); err != nil { + klog.Errorf("Failed to delete secret %s in namespace %s: %v", k.Name, pod.Namespace, err) + } else { + klog.V(2).Infof("Deleted secret %s in namespace %s", k.Name, pod.Namespace) + } + } + } +} + func runPodWithSpec(ctx context.Context, client *kubernetes.Clientset, runPodCollector *troubleshootv1beta2.RunPod) (*corev1.Pod, error) { pod := createPodStruct(runPodCollector) diff --git a/pkg/collect/run_pod_test.go b/pkg/collect/run_pod_test.go index 0e6d5d8b..7acdc112 100644 --- a/pkg/collect/run_pod_test.go +++ b/pkg/collect/run_pod_test.go @@ -1,11 +1,17 @@ package collect import ( + "context" "testing" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + kuberneteserrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" ) func TestCreatePodStruct(t *testing.T) { @@ -91,3 +97,145 @@ func TestCreatePodStruct(t *testing.T) { } } } + +func Test_deleteImagePullSecret(t *testing.T) { + tests := []struct { + name string + pod *corev1.Pod + existingObjs []runtime.Object + validateFunc func(t *testing.T, client *fake.Clientset) + }{ + { + name: "successfully deletes managed secret", + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-ns", + }, + Spec: corev1.PodSpec{ + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: "managed-secret"}, + }, + }, + }, + existingObjs: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "managed-secret", + Namespace: "test-ns", + Labels: map[string]string{ + "app.kubernetes.io/managed-by": "troubleshoot.sh", + }, + }, + }, + }, + validateFunc: func(t *testing.T, client *fake.Clientset) { + // Secret should be deleted + _, err := client.CoreV1().Secrets("test-ns").Get(context.Background(), "managed-secret", metav1.GetOptions{}) + require.True(t, kuberneteserrors.IsNotFound(err)) + }, + }, + { + name: "does not delete unmanaged secret", + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-ns", + }, + Spec: corev1.PodSpec{ + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: "unmanaged-secret"}, + }, + }, + }, + existingObjs: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unmanaged-secret", + Namespace: "test-ns", + }, + }, + }, + validateFunc: func(t *testing.T, client *fake.Clientset) { + // Secret should still exist + secret, err := client.CoreV1().Secrets("test-ns").Get(context.Background(), "unmanaged-secret", metav1.GetOptions{}) + require.NoError(t, err) + assert.NotNil(t, secret) + }, + }, + { + name: "handles non-existent secret", + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-ns", + }, + Spec: corev1.PodSpec{ + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: "non-existent-secret"}, + }, + }, + }, + existingObjs: []runtime.Object{}, + validateFunc: func(t *testing.T, client *fake.Clientset) { + // No error should occur + }, + }, + { + name: "does everything all at once", + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-ns", + }, + Spec: corev1.PodSpec{ + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: "unmanaged-secret"}, + {Name: "non-existent-secret"}, + {Name: "managed-secret"}, + }, + }, + }, + existingObjs: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "managed-secret", + Namespace: "test-ns", + Labels: map[string]string{ + "app.kubernetes.io/managed-by": "troubleshoot.sh", + }, + }, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unmanaged-secret", + Namespace: "test-ns", + }, + }, + }, + validateFunc: func(t *testing.T, client *fake.Clientset) { + // Secret should be deleted + _, err := client.CoreV1().Secrets("test-ns").Get(context.Background(), "managed-secret", metav1.GetOptions{}) + require.True(t, kuberneteserrors.IsNotFound(err)) + + // Secret should still exist + secret, err := client.CoreV1().Secrets("test-ns").Get(context.Background(), "unmanaged-secret", metav1.GetOptions{}) + require.NoError(t, err) + assert.NotNil(t, secret) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := fake.NewSimpleClientset(tt.existingObjs...) + collector := &CollectRunPod{} + + collector.deleteImagePullSecret(context.Background(), client, tt.pod) + + if tt.validateFunc != nil { + tt.validateFunc(t, client) + } + }) + } +}