From 1e9e19a07818fbb6467bf7fccb437443c1fd2d42 Mon Sep 17 00:00:00 2001 From: Varsha Varadarajan Date: Thu, 27 Jun 2019 14:57:07 -0400 Subject: [PATCH 1/2] Unused secrets: check if there are unused secrets in the cluster. --- checks.md | 14 +++ checks/basic/unused_secrets.go | 119 ++++++++++++++++++++++ checks/basic/unused_secrets_test.go | 148 ++++++++++++++++++++++++++++ 3 files changed, 281 insertions(+) create mode 100644 checks/basic/unused_secrets.go create mode 100644 checks/basic/unused_secrets_test.go diff --git a/checks.md b/checks.md index abd4e51..54a9d50 100644 --- a/checks.md +++ b/checks.md @@ -293,3 +293,17 @@ How to fix: ```bash kubectl delete configmap ``` + +###### Unused Secrets + +Name: `unused-secret` + +Group: `basic` + +Description: This check reports all the secret names in the cluster that are not referenced by pods in the respective namespaces. The cluster can be cleaned up based on this information. + +How to fix: + +```bash +kubectl delete secret +``` diff --git a/checks/basic/unused_secrets.go b/checks/basic/unused_secrets.go new file mode 100644 index 0000000..95cec29 --- /dev/null +++ b/checks/basic/unused_secrets.go @@ -0,0 +1,119 @@ +package basic + +import ( + "sync" + + "github.com/digitalocean/clusterlint/checks" + "github.com/digitalocean/clusterlint/kube" + "golang.org/x/sync/errgroup" + corev1 "k8s.io/api/core/v1" +) + +func init() { + checks.Register(&unusedSecretCheck{}) +} + +type unusedSecretCheck struct{} + +type identifier struct { + Name string + Namespace string +} + +// Name returns a unique name for this check. +func (s *unusedSecretCheck) Name() string { + return "unused-secret" +} + +// Groups returns a list of group names this check should be part of. +func (s *unusedSecretCheck) Groups() []string { + return []string{"basic"} +} + +// Description returns a detailed human-readable description of what this check +// does. +func (s *unusedSecretCheck) Description() string { + return "Checks if there are unused secrets in the cluster. Ignores service account tokens" +} + +// Run runs this check on a set of Kubernetes objects. It can return warnings +// (low-priority problems) and errors (high-priority problems) as well as an +// error value indicating that the check failed to run. +func (s *unusedSecretCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error) { + var diagnostics []checks.Diagnostic + used := make(map[identifier]bool) + err := checkReferences(objects, &used) + if err != nil { + return nil, err + } + + for _, secret := range filter(objects.Secrets.Items) { + if _, ok := used[identifier{Name: secret.GetName(), Namespace: secret.GetNamespace()}]; !ok { + secret := secret + d := checks.Diagnostic{ + Severity: checks.Warning, + Message: "Unused secret", + Kind: checks.Secret, + Object: &secret.ObjectMeta, + Owners: secret.ObjectMeta.GetOwnerReferences(), + } + diagnostics = append(diagnostics, d) + } + } + return diagnostics, nil +} + +//checkReferences checks each pod for config map references in volumes and environment variables +func checkReferences(objects *kube.Objects, used *map[identifier]bool) error { + var mu sync.Mutex + var g errgroup.Group + for _, pod := range objects.Pods.Items { + pod := pod + namespace := pod.GetNamespace() + g.Go(func() error { + for _, volume := range pod.Spec.Volumes { + s := volume.VolumeSource.Secret + if s != nil { + mu.Lock() + (*used)[identifier{Name: s.SecretName, Namespace: namespace}] = true + mu.Unlock() + } + } + for _, imageSecret := range pod.Spec.ImagePullSecrets { + mu.Lock() + (*used)[identifier{Name: imageSecret.Name, Namespace: namespace}] = true + mu.Unlock() + } + checkEnvVars(pod.Spec.Containers, used, namespace, &mu) + checkEnvVars(pod.Spec.InitContainers, used, namespace, &mu) + + return nil + }) + } + + return g.Wait() +} + +// checkEnvVars checks for config map references in container environment variables +func checkEnvVars(containers []corev1.Container, used *map[identifier]bool, namespace string, mu *sync.Mutex) { + for _, container := range containers { + for _, env := range container.EnvFrom { + if env.SecretRef != nil { + mu.Lock() + (*used)[identifier{Name: env.SecretRef.LocalObjectReference.Name, Namespace: namespace}] = true + mu.Unlock() + } + } + } +} + +// filter returns Secrets that are not of type `kubernetes.io/service-account-token` +func filter(secrets []corev1.Secret) []corev1.Secret { + var filtered []corev1.Secret + for _, secret := range secrets { + if secret.Type != corev1.SecretTypeServiceAccountToken { + filtered = append(filtered, secret) + } + } + return filtered +} diff --git a/checks/basic/unused_secrets_test.go b/checks/basic/unused_secrets_test.go new file mode 100644 index 0000000..a33570f --- /dev/null +++ b/checks/basic/unused_secrets_test.go @@ -0,0 +1,148 @@ +package basic + +import ( + "testing" + + "github.com/digitalocean/clusterlint/checks" + "github.com/digitalocean/clusterlint/kube" + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const cmNamespace = "k8s" + +func TestUnusedSecretCheckMeta(t *testing.T) { + unusedSecretCheck := unusedSecretCheck{} + assert.Equal(t, "unused-secret", unusedSecretCheck.Name()) + assert.Equal(t, []string{"basic"}, unusedSecretCheck.Groups()) + assert.NotEmpty(t, unusedSecretCheck.Description()) +} + +func TestUnusedSecretCheckRegistration(t *testing.T) { + unusedSecretCheck := &unusedSecretCheck{} + check, err := checks.Get("unused-secret") + assert.NoError(t, err) + assert.Equal(t, check, unusedSecretCheck) +} + +func TestUnusedSecretWarning(t *testing.T) { + tests := []struct { + name string + objs *kube.Objects + expected []checks.Diagnostic + }{ + { + name: "no secrets", + objs: &kube.Objects{Pods: &corev1.PodList{}, Secrets: &corev1.SecretList{}}, + expected: nil, + }, + { + name: "secret volume", + objs: secretVolume(), + expected: nil, + }, + { + name: "environment variable references secret", + objs: secretEnvSource(), + expected: nil, + }, + { + name: "pod with image pull secrets", + objs: imagePullSecrets(), + expected: nil, + }, + { + name: "unused secret", + objs: initSecret(), + expected: []checks.Diagnostic{ + { + Severity: checks.Warning, + Message: "Unused secret", + Kind: checks.Secret, + Object: &metav1.ObjectMeta{Name: "secret_foo", Namespace: "k8s"}, + Owners: GetOwners(), + }, + }, + }, + } + + unusedSecretCheck := unusedSecretCheck{} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + d, err := unusedSecretCheck.Run(test.objs) + assert.NoError(t, err) + assert.ElementsMatch(t, test.expected, d) + }) + } +} + +func initSecret() *kube.Objects { + objs := &kube.Objects{ + Pods: &corev1.PodList{ + Items: []corev1.Pod{ + { + TypeMeta: metav1.TypeMeta{Kind: "Pod", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "pod_foo", Namespace: "k8s"}, + }, + }, + }, + Secrets: &corev1.SecretList{ + Items: []corev1.Secret{ + { + TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "secret_foo", Namespace: "k8s"}, + }, + }, + }, + } + return objs +} + +func secretVolume() *kube.Objects { + objs := initSecret() + objs.Pods.Items[0].Spec = corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "bar", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "secret_foo", + }, + }, + }}, + } + return objs +} + +func secretEnvSource() *kube.Objects { + objs := initSecret() + objs.Pods.Items[0].Spec = corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test-container", + Image: "docker.io/nginx", + EnvFrom: []corev1.EnvFromSource{ + { + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: "secret_foo"}, + }, + }, + }, + }}, + } + return objs +} + +func imagePullSecrets() *kube.Objects { + objs := initSecret() + objs.Pods.Items[0].Spec = corev1.PodSpec{ + ImagePullSecrets: []corev1.LocalObjectReference{ + { + Name: "secret_foo", + }, + }, + } + return objs +} From 9dc3f3f18c627cdb0dcc49c0a98dcf1874feb687 Mon Sep 17 00:00:00 2001 From: Varsha Varadarajan Date: Mon, 1 Jul 2019 10:40:48 -0400 Subject: [PATCH 2/2] Check for references to secret in projected volume. --- checks/basic/unused_secrets.go | 44 ++++++++++++++++++++--------- checks/basic/unused_secrets_test.go | 31 ++++++++++++++++++-- 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/checks/basic/unused_secrets.go b/checks/basic/unused_secrets.go index 95cec29..d8796e5 100644 --- a/checks/basic/unused_secrets.go +++ b/checks/basic/unused_secrets.go @@ -41,14 +41,13 @@ func (s *unusedSecretCheck) Description() string { // error value indicating that the check failed to run. func (s *unusedSecretCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, error) { var diagnostics []checks.Diagnostic - used := make(map[identifier]bool) - err := checkReferences(objects, &used) + used, err := checkReferences(objects) if err != nil { return nil, err } for _, secret := range filter(objects.Secrets.Items) { - if _, ok := used[identifier{Name: secret.GetName(), Namespace: secret.GetNamespace()}]; !ok { + if _, ok := used[kube.Identifier{Name: secret.GetName(), Namespace: secret.GetNamespace()}]; !ok { secret := secret d := checks.Diagnostic{ Severity: checks.Warning, @@ -64,7 +63,9 @@ func (s *unusedSecretCheck) Run(objects *kube.Objects) ([]checks.Diagnostic, err } //checkReferences checks each pod for config map references in volumes and environment variables -func checkReferences(objects *kube.Objects, used *map[identifier]bool) error { +func checkReferences(objects *kube.Objects) (map[kube.Identifier]struct{}, error) { + used := make(map[kube.Identifier]struct{}) + var empty struct{} var mu sync.Mutex var g errgroup.Group for _, pod := range objects.Pods.Items { @@ -75,36 +76,51 @@ func checkReferences(objects *kube.Objects, used *map[identifier]bool) error { s := volume.VolumeSource.Secret if s != nil { mu.Lock() - (*used)[identifier{Name: s.SecretName, Namespace: namespace}] = true + used[kube.Identifier{Name: s.SecretName, Namespace: namespace}] = empty mu.Unlock() } + if volume.VolumeSource.Projected != nil { + for _, source := range volume.VolumeSource.Projected.Sources { + s := source.Secret + if s != nil { + mu.Lock() + used[kube.Identifier{Name: s.LocalObjectReference.Name, Namespace: namespace}] = empty + mu.Unlock() + } + } + } } for _, imageSecret := range pod.Spec.ImagePullSecrets { mu.Lock() - (*used)[identifier{Name: imageSecret.Name, Namespace: namespace}] = true + used[kube.Identifier{Name: imageSecret.Name, Namespace: namespace}] = empty mu.Unlock() } - checkEnvVars(pod.Spec.Containers, used, namespace, &mu) - checkEnvVars(pod.Spec.InitContainers, used, namespace, &mu) + identifiers := envVarsSecretRefs(pod.Spec.Containers, namespace) + identifiers = append(identifiers, checkEnvVars(pod.Spec.InitContainers, namespace)...) + mu.Lock() + for _, i := range identifiers { + used[i] = empty + } + mu.Unlock() return nil }) } - return g.Wait() + return used, g.Wait() } -// checkEnvVars checks for config map references in container environment variables -func checkEnvVars(containers []corev1.Container, used *map[identifier]bool, namespace string, mu *sync.Mutex) { +// envVarsSecretRefs checks for config map references in container environment variables +func envVarsSecretRefs(containers []corev1.Container, namespace string) []kube.Identifier { + var refs []kube.Identifier for _, container := range containers { for _, env := range container.EnvFrom { if env.SecretRef != nil { - mu.Lock() - (*used)[identifier{Name: env.SecretRef.LocalObjectReference.Name, Namespace: namespace}] = true - mu.Unlock() + refs = append(refs, kube.Identifier{Name: env.SecretRef.LocalObjectReference.Name, Namespace: namespace}) } } } + return refs } // filter returns Secrets that are not of type `kubernetes.io/service-account-token` diff --git a/checks/basic/unused_secrets_test.go b/checks/basic/unused_secrets_test.go index a33570f..98e5ae3 100644 --- a/checks/basic/unused_secrets_test.go +++ b/checks/basic/unused_secrets_test.go @@ -10,8 +10,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -const cmNamespace = "k8s" - func TestUnusedSecretCheckMeta(t *testing.T) { unusedSecretCheck := unusedSecretCheck{} assert.Equal(t, "unused-secret", unusedSecretCheck.Name()) @@ -52,6 +50,11 @@ func TestUnusedSecretWarning(t *testing.T) { objs: imagePullSecrets(), expected: nil, }, + { + name: "projected volume references secret", + objs: secretProjection(), + expected: nil, + }, { name: "unused secret", objs: initSecret(), @@ -116,6 +119,30 @@ func secretVolume() *kube.Objects { return objs } +func secretProjection() *kube.Objects { + objs := initSecret() + objs.Pods.Items[0].Spec = corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "bar", + VolumeSource: corev1.VolumeSource{ + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{ + { + Secret: &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "secret_foo", + }, + }, + }, + }, + }, + }, + }}, + } + return objs +} + func secretEnvSource() *kube.Objects { objs := initSecret() objs.Pods.Items[0].Spec = corev1.PodSpec{