yonahd / kor

A Golang Tool to discover unused Kubernetes Resources
MIT License
1.04k stars 96 forks source link

List of objects just returns results for last item in list #348

Closed mpatters72 closed 3 months ago

mpatters72 commented 3 months ago

Describe the bug

When I provide a list of objects, it seems to just process last one in list instead of all of them and ignores previous ones.

To Reproduce

Provide argument with objects in list when you know more than 1 object will return results normally.

kor "pdb,replicaset"

Expected behavior

Would return results for all objects in list.

OS version, architecture and kor version

/opt/homebrew/Cellar/kor/0.5.4 on arm64 - MacBook Pro - M2 Max - macOS Sonoma 14.6.1

Additional context

# replicaset 1st, pdb 2nd, only gives pdb report

kor -n ns-team-hz-ccweb-dev "replicaset,pdb"
kor version: vdev

  _  _____  ____
 | |/ / _ \|  _ \
 | ' / | | | |_) |
 | . \ |_| |  _ <
 |_|\_\___/|_| \_\

Unused resources in namespace: "ns-team-hz-ccweb-dev"
+---+---------------+----------------------------------+
| # | RESOURCE TYPE |          RESOURCE NAME           |
+---+---------------+----------------------------------+
| 1 | Pdb           | alertmanager                     |
| 2 | Pdb           | gneiss-service                   |
| 3 | Pdb           | prometheus                       |
| 4 | Pdb           | release-plane-management-service |
+---+---------------+----------------------------------+

# replicaset pdb 1st, replicaset 2nd, only gives replicaset

╭─ ~                                                                                                                                                                                                                           ✔  14:39:32
╰─ kor -n ns-team-hz-ccweb-dev "pdb,replicaset"
kor version: vdev

  _  _____  ____
 | |/ / _ \|  _ \
 | ' / | | | |_) |
 | . \ |_| |  _ <
 |_|\_\___/|_| \_\

Unused resources in namespace: "ns-team-hz-ccweb-dev"
+----+---------------+---------------------------------------------+
| #  | RESOURCE TYPE |                RESOURCE NAME                |
+----+---------------+---------------------------------------------+
|  1 | ReplicaSet    | app-kube-state-metrics-55965b4d8            |
|  2 | ReplicaSet    | app-kube-state-metrics-55cbddcc47           |
|  3 | ReplicaSet    | app-kube-state-metrics-56c454694d           |
+----+---------------+---------------------------------------------+

# big list of things with secret last, only returns secret

kor -n ns-team-hz-ccweb-dev "pdb,replicaset,hpa,deployment,secret"
kor version: vdev

  _  _____  ____
 | |/ / _ \|  _ \
 | ' / | | | |_) |
 | . \ |_| |  _ <
 |_|\_\___/|_| \_\

Unused resources in namespace: "ns-team-hz-ccweb-dev"
+---+---------------+--------------------------------+
| # | RESOURCE TYPE |         RESOURCE NAME          |
+---+---------------+--------------------------------+
| 1 | Secret        | alertmanager-alertmanager      |
| 2 | Secret        | prometheus-scrape-config       |
| 3 | Secret        | vault-secrets-operator-approle |
+---+---------------+--------------------------------+
doronkg commented 3 months ago

Hi @mpatters72, thanks for submitting this issue. I can confirm this as an issue which seems to have raised in #320 as from v0.5.3. This happens only when grouping resources by namespace (default), it works as expected when grouping by resource.

Example:

$ kor sa,cm -n kor --group-by=resource
kor version: vdev

  _  _____  ____
 | |/ / _ \|  _ \
 | ' / | | | |_) |
 | . \ |_| |  _ <
 |_|\_\___/|_| \_\

Unused ServiceAccounts:
+---+-----------+---------------+
| # | NAMESPACE | RESOURCE NAME |
+---+-----------+---------------+
| 1 | kor       | test          |
+---+-----------+---------------+

Unused ConfigMaps:
+---+-----------+---------------+
| # | NAMESPACE | RESOURCE NAME |
+---+-----------+---------------+
| 1 | kor       | test          |
+---+-----------+---------------+

$ kor sa,cm -n kor --group-by=namespace
kor version: vdev

  _  _____  ____
 | |/ / _ \|  _ \
 | ' / | | | |_) |
 | . \ |_| |  _ <
 |_|\_\___/|_| \_\

Unused resources in namespace: "kor"
+---+---------------+---------------+
| # | RESOURCE TYPE | RESOURCE NAME |
+---+---------------+---------------+
| 1 | ConfigMap     | test          |
+---+---------------+---------------+

I will submit a fix and keep you updated. @yonahd please label this as a bug and assign me.

doronkg commented 3 months ago

WIP in #349. The output is indeed overriden in a loop so it's initialized every iteration and only the last resource type applies. As seen in https://github.com/yonahd/kor/blob/main/pkg/kor/multi.go#L136:

    for _, namespace := range namespaces {
        allDiffs := retrieveNamespaceDiffs(clientset, namespace, resourceList, filterOpts)
        for _, diff := range allDiffs {
            if opts.DeleteFlag {
                if diff.diff, err = DeleteResource(diff.diff, clientset, namespace, diff.resourceType, opts.NoInteractive); err != nil {
                    fmt.Fprintf(os.Stderr, "Failed to delete %s %s in namespace %s: %v\n", diff.resourceType, diff.diff, namespace, err)
                }
            }
            switch opts.GroupBy {
            case "namespace":
------->            resources[namespace] = make(map[string][]ResourceInfo)
                resources[namespace][diff.resourceType] = diff.diff
            case "resource":
                appendResources(resources, diff.resourceType, namespace, diff.diff)
            }
        }
    }

I moved the initializtion outside of the diff loop, as is should be initialized once for every namespace, and not for every diff:

        if opts.GroupBy == "namespace" {
            resources[namespace] = make(map[string][]ResourceInfo)
        }

And it seemed to have resolved the issue:

$ kor sa,cm -n kor
kor version: vdev

  _  _____  ____
 | |/ / _ \|  _ \
 | ' / | | | |_) |
 | . \ |_| |  _ <
 |_|\_\___/|_| \_\

Unused resources in namespace: "kor"
+---+----------------+---------------+
| # | RESOURCE TYPE  | RESOURCE NAME |
+---+----------------+---------------+
| 1 | ServiceAccount | test          |
| 2 | ConfigMap      | test          |
+---+----------------+---------------+
yonahd commented 3 months ago

@mpatters72 Thanks for reporting the bug!

This should be solved in the latest release(v0.5.5)

Let us know if you have any further issues