Repository: yonahd/kor
Pull request #163 (2023-11-22 07:52:18)

Feat: Find unused pv
Discovers PVs that are not bound and marks them as unused

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) Report
Attention: `81 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`b948807`)](https://app.codecov.io/gh/yonahd/kor/commit/b94880764196f8a1a00a4e7516a2ea740d12383e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) 45.86% compared to head [(`1e1cfc3`)](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) 45.52%.
> Report is 1 commits behind head on main.

| [Files](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) | Patch % | Lines |
|---|---|---|
| [pkg/kor/multi.go](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen#diff-cGtnL2tvci9tdWx0aS5nbw==) | 0.00% | [29 Missing :warning: ](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) |
| [pkg/kor/all.go](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen#diff-cGtnL2tvci9hbGwuZ28=) | 0.00% | [22 Missing :warning: ](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) |
| [pkg/kor/pv.go](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen#diff-cGtnL2tvci9wdi5nbw==) | 58.49% | [14 Missing and 8 partials :warning: ](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) |
| [pkg/kor/delete.go](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen#diff-cGtnL2tvci9kZWxldGUuZ28=) | 0.00% | [7 Missing :warning: ](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) |
| [pkg/kor/kor.go](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen#diff-cGtnL2tvci9rb3IuZ28=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen) |

:exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #163      +/-   ##
==========================================
- Coverage   45.86%   45.52%   -0.35%     
==========================================
  Files          20       21       +1     
  Lines        1888     1990     +102     
==========================================
+ Hits          866      906      +40     
- Misses        876      930      +54     
- Partials      146      154       +8     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/yonahd/kor/pull/163?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Yonah+Dissen).


Patch:
diff --git a/README.md b/README.md
index a6e6cd5..46365d4 100644
--- a/README.md
+++ b/README.md
@@ -21,6 +21,7 @@ Kor is a tool to discover unused Kubernetes resources. Currently, Kor can identi
 - Ingresses
 - PDBs
 - CRDs
+- PVs
 
 ![Kor Screenshot](/images/screenshot.png)
 
@@ -82,9 +83,10 @@ Kor provides various subcommands to identify and list unused resources. The avai
 - `role` - Gets unused Roles for the specified namespace or all namespaces.
 - `hpa` - Gets unused HPAs for the specified namespace or all namespaces.
 - `pvc` - Gets unused PVCs for the specified namespace or all namespaces.
+- `pv` - Gets unused PVs in the cluster(non namespaced resource).
 - `ingress` - Gets unused Ingresses for the specified namespace or all namespaces.
 - `pdb` - Gets unused PDBs for the specified namespace or all namespaces.
-- `crd` - Gets unused CRDs in the cluster.
+- `crd` - Gets unused CRDs in the cluster(non namespaced resource).
 - `exporter` - Export Prometheus metrics.
 
 ### Supported Flags
@@ -133,6 +135,7 @@ kor [subcommand] --help
 | Ingresses       | Ingresses not pointing at any Service                                                                                                                                                                                             |                                                                                                                              |
 | Hpas            | HPAs not used in Deployments<br/> HPAs not used in StatefulSets                                                                                                                                                                   |                                                                                                                              |
 | CRDs            | CRDs not used the cluster                                                                                                                                                                                                         |                                                                                                                              |
+| Pvs             | PVs not bound to a PVC                                                                                                                                                                                                            |                                                                                                                              |
 | Pdbs            | PDBs not used in Deployments<br/> PDBs not used in StatefulSets                                                                                                                                                                   |                                                                                                                              |
 
 
diff --git a/cmd/kor/pv.go b/cmd/kor/pv.go
new file mode 100644
index 0000000..a473034
--- /dev/null
+++ b/cmd/kor/pv.go
@@ -0,0 +1,31 @@
+package kor
+
+import (
+	"fmt"
+
+	"github.com/spf13/cobra"
+	"github.com/yonahd/kor/pkg/kor"
+	"github.com/yonahd/kor/pkg/utils"
+)
+
+var pvCmd = &cobra.Command{
+	Use:     "persistentvolume",
+	Aliases: []string{"pv", "persistentvolumes"},
+	Short:   "Gets unused pvs",
+	Args:    cobra.NoArgs,
+	Run: func(cmd *cobra.Command, args []string) {
+		clientset := kor.GetKubeClient(kubeconfig)
+
+		if response, err := kor.GetUnusedPvs(filterOptions, clientset, outputFormat, opts); err != nil {
+			fmt.Println(err)
+		} else {
+			utils.PrintLogo(outputFormat)
+			fmt.Println(response)
+		}
+
+	},
+}
+
+func init() {
+	rootCmd.AddCommand(pvCmd)
+}
diff --git a/pkg/kor/all.go b/pkg/kor/all.go
index ef33814..047d735 100644
--- a/pkg/kor/all.go
+++ b/pkg/kor/all.go
@@ -121,12 +121,21 @@ func getUnusedPdbs(clientset kubernetes.Interface, namespace string, filterOpts
 }
 
 func getUnusedCrds(apiExtClient apiextensionsclientset.Interface, dynamicClient dynamic.Interface) ResourceDiff {
-	pdbDiff, err := processCrds(apiExtClient, dynamicClient)
+	crdDiff, err := processCrds(apiExtClient, dynamicClient)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "Failed to get %s: %v\n", "Crds", err)
 	}
-	namespacePdbDiff := ResourceDiff{"Crd", pdbDiff}
-	return namespacePdbDiff
+	allCrdDiff := ResourceDiff{"Crd", crdDiff}
+	return allCrdDiff
+}
+
+func getUnusedPvs(clientset kubernetes.Interface, filterOpts *FilterOptions) ResourceDiff {
+	pvDiff, err := processPvs(clientset, filterOpts)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "Failed to get %s: %v\n", "Pvs", err)
+	}
+	allPvDiff := ResourceDiff{"Pv", pvDiff}
+	return allPvDiff
 }
 
 func GetUnusedAll(includeExcludeLists IncludeExcludeLists, filterOpts *FilterOptions, clientset kubernetes.Interface, apiExtClient apiextensionsclientset.Interface, dynamicClient dynamic.Interface, outputFormat string, opts Opts) (string, error) {
@@ -173,13 +182,24 @@ func GetUnusedAll(includeExcludeLists IncludeExcludeLists, filterOpts *FilterOpt
 	}
 
 	var allDiffs []ResourceDiff
+	noNamespaceResourceMap := make(map[string][]string)
 	crdDiff := getUnusedCrds(apiExtClient, dynamicClient)
-	allDiffs = append(allDiffs, crdDiff)
+	crdOutput := FormatOutputAll("", []ResourceDiff{crdDiff}, opts)
+	outputBuffer.WriteString(crdOutput)
+	outputBuffer.WriteString("\n")
+	noNamespaceResourceMap[crdDiff.resourceType] = crdDiff.diff
+
+	pvDiff := getUnusedPvs(clientset, filterOpts)
+	pvOutput := FormatOutputAll("", []ResourceDiff{pvDiff}, opts)
+	outputBuffer.WriteString(pvOutput)
+	outputBuffer.WriteString("\n")
+	noNamespaceResourceMap[pvDiff.resourceType] = pvDiff.diff
 
 	output := FormatOutputAll("", allDiffs, opts)
 
 	outputBuffer.WriteString(output)
 	outputBuffer.WriteString("\n")
+	response[""] = noNamespaceResourceMap
 
 	jsonResponse, err := json.MarshalIndent(response, "", "  ")
 	if err != nil {
diff --git a/pkg/kor/delete.go b/pkg/kor/delete.go
index e63e2bc..7c0dd24 100644
--- a/pkg/kor/delete.go
+++ b/pkg/kor/delete.go
@@ -52,6 +52,9 @@ func DeleteResourceCmd() map[string]func(clientset kubernetes.Interface, namespa
 		"ServiceAccount": func(clientset kubernetes.Interface, namespace, name string) error {
 			return clientset.CoreV1().ServiceAccounts(namespace).Delete(context.TODO(), name, metav1.DeleteOptions{})
 		},
+		"PV": func(clientset kubernetes.Interface, namespace, name string) error {
+			return clientset.CoreV1().PersistentVolumes().Delete(context.TODO(), name, metav1.DeleteOptions{})
+		},
 	}
 
 	return deleteResourceApiMap
@@ -103,6 +106,8 @@ func updateResource(clientset kubernetes.Interface, namespace, resourceType stri
 		return clientset.AppsV1().StatefulSets(namespace).Update(context.TODO(), resource.(*appsv1.StatefulSet), metav1.UpdateOptions{})
 	case "ServiceAccount":
 		return clientset.CoreV1().ServiceAccounts(namespace).Update(context.TODO(), resource.(*corev1.ServiceAccount), metav1.UpdateOptions{})
+	case "PV":
+		return clientset.CoreV1().PersistentVolumes().Update(context.TODO(), resource.(*corev1.PersistentVolume), metav1.UpdateOptions{})
 	}
 	return nil, fmt.Errorf("resource type '%s' is not supported", resourceType)
 }
@@ -131,6 +136,8 @@ func getResource(clientset kubernetes.Interface, namespace, resourceType, resour
 		return clientset.AppsV1().StatefulSets(namespace).Get(context.TODO(), resourceName, metav1.GetOptions{})
 	case "ServiceAccount":
 		return clientset.CoreV1().ServiceAccounts(namespace).Get(context.TODO(), resourceName, metav1.GetOptions{})
+	case "PV":
+		return clientset.CoreV1().PersistentVolumes().Get(context.TODO(), resourceName, metav1.GetOptions{})
 	}
 	return nil, fmt.Errorf("resource type '%s' is not supported", resourceType)
 }
diff --git a/pkg/kor/kor.go b/pkg/kor/kor.go
index 4cb9372..f937fdd 100644
--- a/pkg/kor/kor.go
+++ b/pkg/kor/kor.go
@@ -210,7 +210,7 @@ func FormatOutputAll(namespace string, allDiffs []ResourceDiff, opts Opts) strin
 
 	table.Render()
 	if namespace == "" {
-		return fmt.Sprintf("Unused CRDs: \n%s", buf.String())
+		return fmt.Sprintf("Unused %ss: \n%s", allDiffs[0].resourceType, buf.String())
 	}
 	return fmt.Sprintf("Unused Resources in Namespace: %s\n%s", namespace, buf.String())
 }
diff --git a/pkg/kor/multi.go b/pkg/kor/multi.go
index 5c7bc2d..26bc206 100644
--- a/pkg/kor/multi.go
+++ b/pkg/kor/multi.go
@@ -12,20 +12,33 @@ import (
 	"k8s.io/client-go/kubernetes"
 )
 
-func retrieveNoNamespaceDiff(apiExtClient apiextensionsclientset.Interface, dynamicClient dynamic.Interface, outputFormat string, opts Opts, resourceList []string) ([]ResourceDiff, []string) {
+func retrieveNoNamespaceDiff(clientset kubernetes.Interface, apiExtClient apiextensionsclientset.Interface, dynamicClient dynamic.Interface, resourceList []string, filterOpts *FilterOptions) ([]ResourceDiff, []string) {
 	var noNamespaceDiff []ResourceDiff
+	markedForRemoval := make([]bool, len(resourceList))
+	updatedResourceList := resourceList
+
 	for counter, resource := range resourceList {
-		if resource == "crd" || resource == "customresourcedefinition" || resource == "customresourcedefinitions" {
+		switch resource {
+		case "crd", "customresourcedefinition", "customresourcedefinitions":
 			crdDiff := getUnusedCrds(apiExtClient, dynamicClient)
 			noNamespaceDiff = append(noNamespaceDiff, crdDiff)
-			updatedResourceList := append(resourceList[:counter], resourceList[counter+1:]...)
-			return noNamespaceDiff, updatedResourceList
-		} else {
-			resourceList[counter] = resource
+			markedForRemoval[counter] = true
+		case "pv", "persistentvolume", "persistentvolumes":
+			pvDiff := getUnusedPvs(clientset, filterOpts)
+			noNamespaceDiff = append(noNamespaceDiff, pvDiff)
+			markedForRemoval[counter] = true
+		}
+	}
+
+	// Remove elements marked for removal
+	var clearedResourceList []string
+	for i, marked := range markedForRemoval {
+		if !marked {
+			clearedResourceList = append(clearedResourceList, updatedResourceList[i])
 		}
 	}
-	return noNamespaceDiff, resourceList
 
+	return noNamespaceDiff, clearedResourceList
 }
 
 func retrieveNamespaceDiffs(clientset kubernetes.Interface, namespace string, resourceList []string, filterOpts *FilterOptions) []ResourceDiff {
@@ -72,14 +85,22 @@ func GetUnusedMulti(includeExcludeLists IncludeExcludeLists, resourceNames strin
 	response := make(map[string]map[string][]string)
 	var err error
 
-	crdDiff, resourceList := retrieveNoNamespaceDiff(apiExtClient, dynamicClient, outputFormat, opts, resourceList)
-	if len(crdDiff) != 0 {
-		output := FormatOutputAll("", crdDiff, opts)
-		outputBuffer.WriteString(output)
-		outputBuffer.WriteString("\n")
+	noNamespaceDiff, resourceList := retrieveNoNamespaceDiff(clientset, apiExtClient, dynamicClient, resourceList, filterOpts)
+	if len(noNamespaceDiff) != 0 {
+		for _, diff := range noNamespaceDiff {
+			if len(diff.diff) != 0 {
+				output := FormatOutputAll("", []ResourceDiff{diff}, opts)
+				outputBuffer.WriteString(output)
+				outputBuffer.WriteString("\n")
+
+				resourceMap := make(map[string][]string)
+				resourceMap[diff.resourceType] = diff.diff
+				response[""] = resourceMap
+			}
+		}
 
 		resourceMap := make(map[string][]string)
-		for _, diff := range crdDiff {
+		for _, diff := range noNamespaceDiff {
 			resourceMap[diff.resourceType] = diff.diff
 		}
 		response[""] = resourceMap
diff --git a/pkg/kor/pv.go b/pkg/kor/pv.go
new file mode 100644
index 0000000..7f4fbd6
--- /dev/null
+++ b/pkg/kor/pv.go
@@ -0,0 +1,89 @@
+package kor
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"os"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+	_ "k8s.io/client-go/plugin/pkg/client/auth/oidc"
+)
+
+func processPvs(clientset kubernetes.Interface, filterOpts *FilterOptions) ([]string, error) {
+	pvs, err := clientset.CoreV1().PersistentVolumes().List(context.TODO(), metav1.ListOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	var unusedPvs []string
+
+	for _, pv := range pvs.Items {
+		if pv.Labels["kor/used"] == "true" {
+			continue
+		}
+
+		if excluded, _ := HasExcludedLabel(pv.Labels, filterOpts.ExcludeLabels); excluded {
+			continue
+		}
+
+		if included, _ := HasIncludedAge(pv.CreationTimestamp, filterOpts); !included {
+			continue
+		}
+
+		if pv.Status.Phase != "Bound" {
+			unusedPvs = append(unusedPvs, pv.Name)
+		}
+
+	}
+
+	return unusedPvs, nil
+
+}
+
+func GetUnusedPvs(filterOpts *FilterOptions, clientset kubernetes.Interface, outputFormat string, opts Opts) (string, error) {
+	var outputBuffer bytes.Buffer
+	response := make(map[string]map[string][]string)
+
+	diff, err := processPvs(clientset, filterOpts)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "Failed to process pvs: %v\n", err)
+	}
+
+	if len(diff) > 0 {
+		// We consider cluster scope resources in "" (empty string) namespace, as it is common in k8s
+		if response[""] == nil {
+			response[""] = make(map[string][]string)
+		}
+		response[""]["Pv"] = diff
+	}
+
+	if opts.DeleteFlag {
+		if diff, err = DeleteResource(diff, clientset, "", "PV", opts.NoInteractive); err != nil {
+			fmt.Fprintf(os.Stderr, "Failed to delete PV %s: %v\n", diff, err)
+		}
+	}
+
+	output := FormatOutput("", diff, "PVs", opts)
+	if output != "" {
+		outputBuffer.WriteString(output)
+		outputBuffer.WriteString("\n")
+
+		response[""]["Pv"] = diff
+
+	}
+
+	jsonResponse, err := json.MarshalIndent(response, "", "  ")
+	if err != nil {
+		return "", err
+	}
+
+	unusedPvs, err := unusedResourceFormatter(outputFormat, outputBuffer, opts, jsonResponse)
+	if err != nil {
+		fmt.Printf("err: %v\n", err)
+	}
+
+	return unusedPvs, nil
+}


Test patch:
diff --git a/pkg/kor/create_test_resources.go b/pkg/kor/create_test_resources.go
index 2f2e5ad..2659f86 100644
--- a/pkg/kor/create_test_resources.go
+++ b/pkg/kor/create_test_resources.go
@@ -216,6 +216,17 @@ func CreateTestPvc(namespace, name string) *corev1.PersistentVolumeClaim {
 	}
 }
 
+func CreateTestPv(name, phase string) *corev1.PersistentVolume {
+	return &corev1.PersistentVolume{
+		ObjectMeta: v1.ObjectMeta{
+			Name: name,
+		},
+		Status: corev1.PersistentVolumeStatus{
+			Phase: corev1.PersistentVolumePhase(phase),
+		},
+	}
+}
+
 func CreateTestPdb(namespace, name string, matchLabels map[string]string) *policyv1.PodDisruptionBudget {
 	return &policyv1.PodDisruptionBudget{
 		ObjectMeta: v1.ObjectMeta{
diff --git a/pkg/kor/pv_test.go b/pkg/kor/pv_test.go
new file mode 100644
index 0000000..97f119b
--- /dev/null
+++ b/pkg/kor/pv_test.go
@@ -0,0 +1,79 @@
+package kor
+
+import (
+	"context"
+	"encoding/json"
+	"reflect"
+	"testing"
+
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes/fake"
+)
+
+func createTestPvs(t *testing.T) *fake.Clientset {
+	clientset := fake.NewSimpleClientset()
+
+	pv1 := CreateTestPv("test-pv1", "Bound")
+	pv2 := CreateTestPv("test-pv2", "Available")
+	_, err := clientset.CoreV1().PersistentVolumes().Create(context.TODO(), pv1, v1.CreateOptions{})
+	if err != nil {
+		t.Fatalf("Error creating fake %s: %v", "PV", err)
+	}
+
+	_, err = clientset.CoreV1().PersistentVolumes().Create(context.TODO(), pv2, v1.CreateOptions{})
+	if err != nil {
+		t.Fatalf("Error creating fake %s: %v", "PV", err)
+	}
+
+	return clientset
+}
+
+func TestProcessPvs(t *testing.T) {
+	clientset := createTestPvs(t)
+	usedPvs, err := processPvs(clientset, &FilterOptions{})
+	if err != nil {
+		t.Errorf("Expected no error, got %v", err)
+	}
+
+	if len(usedPvs) != 1 {
+		t.Errorf("Expected 1 used pv, got %d", len(usedPvs))
+	}
+
+	if usedPvs[0] != "test-pv2" {
+		t.Errorf("Expected 'test-pv2', got %s", usedPvs[0])
+	}
+}
+
+func TestGetUnusedPvs(t *testing.T) {
+	clientset := createTestPvs(t)
+
+	opts := Opts{
+		WebhookURL:    "",
+		Channel:       "",
+		Token:         "",
+		DeleteFlag:    false,
+		NoInteractive: true,
+	}
+
+	output, err := GetUnusedPvs(&FilterOptions{}, clientset, "json", opts)
+	if err != nil {
+		t.Fatalf("Error calling GetUnusedPvs: %v", err)
+	}
+
+	expectedOutput := map[string]map[string][]string{
+		"": {
+			"Pv": {"test-pv2"},
+		},
+	}
+
+	var actualOutput map[string]map[string][]string
+	if err := json.Unmarshal([]byte(output), &actualOutput); err != nil {
+		t.Fatalf("Error unmarshaling actual output: %v", err)
+	}
+
+	if !reflect.DeepEqual(expectedOutput, actualOutput) {
+		t.Errorf("Expected output does not match actual output")
+	}
+}
+
+