Skip to content

Commit 641c195

Browse files
emosbaughemosbaugh
authored
fix(collect.runPod): does not delete image pull secrets without name in spec (#1761)
* fix(collect.runPod): fix deleting image pull secrets * f * f
1 parent ef1cd66 commit 641c195

File tree

2 files changed

+170
-7
lines changed

2 files changed

+170
-7
lines changed

‎pkg/collect/run_pod.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,7 @@ func (c *CollectRunPod) Collect(progressChan chan<- interface{}) (result Collect
6262
}()
6363

6464
if c.Collector.ImagePullSecret != nil && c.Collector.ImagePullSecret.Data != nil {
65-
defer func() {
66-
if c.Collector.ImagePullSecret.Name != "" {
67-
if err := client.CoreV1().Secrets(pod.Namespace).Delete(ctx, c.Collector.ImagePullSecret.Name, metav1.DeleteOptions{}); err != nil {
68-
klog.Errorf("Failed to delete secret %s: %v", c.Collector.ImagePullSecret.Name, err)
69-
}
70-
}
71-
}()
65+
defer c.deleteImagePullSecret(context.Background(), client, pod)
7266
}
7367

7468
defer func() {
@@ -117,6 +111,27 @@ func (c *CollectRunPod) Collect(progressChan chan<- interface{}) (result Collect
117111
}
118112
}
119113

114+
func (c *CollectRunPod) deleteImagePullSecret(ctx context.Context, client kubernetes.Interface, pod *corev1.Pod) {
115+
for _, k := range pod.Spec.ImagePullSecrets {
116+
secret, err := client.CoreV1().Secrets(pod.Namespace).Get(ctx, k.Name, metav1.GetOptions{})
117+
if err != nil {
118+
if kuberneteserrors.IsNotFound(err) {
119+
klog.V(2).Infof("Secret %s in namespace %s not found", k.Name, pod.Namespace)
120+
} else {
121+
klog.Errorf("Failed to get secret %s in namespace %s: %v", k.Name, pod.Namespace, err)
122+
}
123+
continue
124+
}
125+
if secret.Labels["app.kubernetes.io/managed-by"] == "troubleshoot.sh" {
126+
if err := client.CoreV1().Secrets(pod.Namespace).Delete(context.Background(), k.Name, metav1.DeleteOptions{}); err != nil {
127+
klog.Errorf("Failed to delete secret %s in namespace %s: %v", k.Name, pod.Namespace, err)
128+
} else {
129+
klog.V(2).Infof("Deleted secret %s in namespace %s", k.Name, pod.Namespace)
130+
}
131+
}
132+
}
133+
}
134+
120135
func runPodWithSpec(ctx context.Context, client *kubernetes.Clientset, runPodCollector *troubleshootv1beta2.RunPod) (*corev1.Pod, error) {
121136
pod := createPodStruct(runPodCollector)
122137

‎pkg/collect/run_pod_test.go

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11
package collect
22

33
import (
4+
"context"
45
"testing"
56

67
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
710
corev1 "k8s.io/api/core/v1"
11+
kuberneteserrors "k8s.io/apimachinery/pkg/api/errors"
812
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/apimachinery/pkg/runtime"
14+
"k8s.io/client-go/kubernetes/fake"
915
)
1016

1117
func TestCreatePodStruct(t *testing.T) {
@@ -91,3 +97,145 @@ func TestCreatePodStruct(t *testing.T) {
9197
}
9298
}
9399
}
100+
101+
func Test_deleteImagePullSecret(t *testing.T) {
102+
tests := []struct {
103+
name string
104+
pod *corev1.Pod
105+
existingObjs []runtime.Object
106+
validateFunc func(t *testing.T, client *fake.Clientset)
107+
}{
108+
{
109+
name: "successfully deletes managed secret",
110+
pod: &corev1.Pod{
111+
ObjectMeta: metav1.ObjectMeta{
112+
Name: "test-pod",
113+
Namespace: "test-ns",
114+
},
115+
Spec: corev1.PodSpec{
116+
ImagePullSecrets: []corev1.LocalObjectReference{
117+
{Name: "managed-secret"},
118+
},
119+
},
120+
},
121+
existingObjs: []runtime.Object{
122+
&corev1.Secret{
123+
ObjectMeta: metav1.ObjectMeta{
124+
Name: "managed-secret",
125+
Namespace: "test-ns",
126+
Labels: map[string]string{
127+
"app.kubernetes.io/managed-by": "troubleshoot.sh",
128+
},
129+
},
130+
},
131+
},
132+
validateFunc: func(t *testing.T, client *fake.Clientset) {
133+
// Secret should be deleted
134+
_, err := client.CoreV1().Secrets("test-ns").Get(context.Background(), "managed-secret", metav1.GetOptions{})
135+
require.True(t, kuberneteserrors.IsNotFound(err))
136+
},
137+
},
138+
{
139+
name: "does not delete unmanaged secret",
140+
pod: &corev1.Pod{
141+
ObjectMeta: metav1.ObjectMeta{
142+
Name: "test-pod",
143+
Namespace: "test-ns",
144+
},
145+
Spec: corev1.PodSpec{
146+
ImagePullSecrets: []corev1.LocalObjectReference{
147+
{Name: "unmanaged-secret"},
148+
},
149+
},
150+
},
151+
existingObjs: []runtime.Object{
152+
&corev1.Secret{
153+
ObjectMeta: metav1.ObjectMeta{
154+
Name: "unmanaged-secret",
155+
Namespace: "test-ns",
156+
},
157+
},
158+
},
159+
validateFunc: func(t *testing.T, client *fake.Clientset) {
160+
// Secret should still exist
161+
secret, err := client.CoreV1().Secrets("test-ns").Get(context.Background(), "unmanaged-secret", metav1.GetOptions{})
162+
require.NoError(t, err)
163+
assert.NotNil(t, secret)
164+
},
165+
},
166+
{
167+
name: "handles non-existent secret",
168+
pod: &corev1.Pod{
169+
ObjectMeta: metav1.ObjectMeta{
170+
Name: "test-pod",
171+
Namespace: "test-ns",
172+
},
173+
Spec: corev1.PodSpec{
174+
ImagePullSecrets: []corev1.LocalObjectReference{
175+
{Name: "non-existent-secret"},
176+
},
177+
},
178+
},
179+
existingObjs: []runtime.Object{},
180+
validateFunc: func(t *testing.T, client *fake.Clientset) {
181+
// No error should occur
182+
},
183+
},
184+
{
185+
name: "does everything all at once",
186+
pod: &corev1.Pod{
187+
ObjectMeta: metav1.ObjectMeta{
188+
Name: "test-pod",
189+
Namespace: "test-ns",
190+
},
191+
Spec: corev1.PodSpec{
192+
ImagePullSecrets: []corev1.LocalObjectReference{
193+
{Name: "unmanaged-secret"},
194+
{Name: "non-existent-secret"},
195+
{Name: "managed-secret"},
196+
},
197+
},
198+
},
199+
existingObjs: []runtime.Object{
200+
&corev1.Secret{
201+
ObjectMeta: metav1.ObjectMeta{
202+
Name: "managed-secret",
203+
Namespace: "test-ns",
204+
Labels: map[string]string{
205+
"app.kubernetes.io/managed-by": "troubleshoot.sh",
206+
},
207+
},
208+
},
209+
&corev1.Secret{
210+
ObjectMeta: metav1.ObjectMeta{
211+
Name: "unmanaged-secret",
212+
Namespace: "test-ns",
213+
},
214+
},
215+
},
216+
validateFunc: func(t *testing.T, client *fake.Clientset) {
217+
// Secret should be deleted
218+
_, err := client.CoreV1().Secrets("test-ns").Get(context.Background(), "managed-secret", metav1.GetOptions{})
219+
require.True(t, kuberneteserrors.IsNotFound(err))
220+
221+
// Secret should still exist
222+
secret, err := client.CoreV1().Secrets("test-ns").Get(context.Background(), "unmanaged-secret", metav1.GetOptions{})
223+
require.NoError(t, err)
224+
assert.NotNil(t, secret)
225+
},
226+
},
227+
}
228+
229+
for _, tt := range tests {
230+
t.Run(tt.name, func(t *testing.T) {
231+
client := fake.NewSimpleClientset(tt.existingObjs...)
232+
collector := &CollectRunPod{}
233+
234+
collector.deleteImagePullSecret(context.Background(), client, tt.pod)
235+
236+
if tt.validateFunc != nil {
237+
tt.validateFunc(t, client)
238+
}
239+
})
240+
}
241+
}

0 commit comments

Comments
 (0)