yonahd / kor

A Golang Tool to discover unused Kubernetes Resources
MIT License
974 stars 91 forks source link

feat: allow grouping options for output #256

Closed cecobask closed 4 months ago

cecobask commented 5 months ago

What this PR does / why we need it

Adds a new flag --group-by to all subcommands of kor. The value must be one of namespace or resource. Please review the changes first; if all looks good, I will add tests.

PR Checklist

Github Issue

Closes #43

Notes for your reviewers

  1. kor all --group-by=resource:
    Unused ConfigMaps:
    +---+-----------+---------------+
    | # | NAMESPACE | RESOURCE NAME |
    +---+-----------+---------------+
    | 1 | ns1       | cm1           |
    | 2 | ns1       | cm2           |
    | 3 | ns2       | cm3           |
    +---+-----------+---------------+
    Unused Deployments:
    +---+-----------+---------------+
    | # | NAMESPACE | RESOURCE NAME |
    +---+-----------+---------------+
    | 1 | ns1       | deploy1       |
    | 2 | ns2       | deploy2       |
    +---+-----------+---------------+
    Unused ReplicaSets:
    +---+-----------+--------------------+
    | # | NAMESPACE |   RESOURCE NAME    |
    +---+-----------+--------------------+
    | 1 | ns1       | deploy1-654d48b75f |
    | 2 | ns2       | deploy2-79f48888c6 |
    +---+-----------+--------------------+
  2. kor all --group-by=namespace:
    Unused resources in namespace: "ns1"
    +---+---------------+--------------------+
    | # | RESOURCE TYPE |   RESOURCE NAME    |
    +---+---------------+--------------------+
    | 1 | ConfigMap     | cm1                |
    | 2 | ConfigMap     | cm2                |
    | 3 | ReplicaSet    | deploy1-654d48b75f |
    | 4 | Deployment    | deploy1            |
    +---+---------------+--------------------+
    Unused resources in namespace: "ns2"
    +---+---------------+--------------------+
    | # | RESOURCE TYPE |   RESOURCE NAME    |
    +---+---------------+--------------------+
    | 1 | ReplicaSet    | deploy2-79f48888c6 |
    | 2 | ConfigMap     | cm3                |
    | 3 | Deployment    | deploy2            |
    +---+---------------+--------------------+
codecov-commenter commented 5 months ago

Codecov Report

Attention: Patch coverage is 38.16514% with 337 lines in your changes are missing coverage. Please review.

Project coverage is 40.83%. Comparing base (97d3873) to head (db62e9a). Report is 1 commits behind head on main.

Files Patch % Lines
pkg/kor/all.go 0.00% 130 Missing :warning:
pkg/kor/kor.go 1.23% 80 Missing :warning:
pkg/kor/crds.go 0.00% 17 Missing :warning:
pkg/kor/clusterroles.go 62.50% 5 Missing and 1 partial :warning:
pkg/kor/configmaps.go 62.50% 5 Missing and 1 partial :warning:
pkg/kor/daemonsets.go 68.42% 5 Missing and 1 partial :warning:
pkg/kor/deployments.go 66.66% 5 Missing and 1 partial :warning:
pkg/kor/hpas.go 64.70% 5 Missing and 1 partial :warning:
pkg/kor/ingresses.go 64.70% 5 Missing and 1 partial :warning:
pkg/kor/jobs.go 68.42% 5 Missing and 1 partial :warning:
... and 13 more

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #256 +/- ## ========================================== - Coverage 43.37% 40.83% -2.55% ========================================== Files 58 58 Lines 2808 2902 +94 ========================================== - Hits 1218 1185 -33 - Misses 1400 1531 +131 + Partials 190 186 -4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

doronkg commented 4 months ago

Hi @cecobask thanks for this contribution! #43 was long overdue, I'll review it as soon as I can. From a glimpse it seems to me that this PR also resolves an issue with excessive newline padding in default table output. In the meantime, please udpate README.md with the --group-by flag and output examples.

cecobask commented 4 months ago

Thank you, @doronkg! I've updated the documentation as suggested and rebased the branch.

doronkg commented 4 months ago

Thank you, @doronkg! I've updated the documentation as suggested and rebased the branch.

Great, I've reviewed the main refactor in all.go - looks good, moving to the resource-based refactors & tests. It seems that many additions such as object literal restructuring were generated by IDE (?), it makes the code much more readable.

cecobask commented 4 months ago

Great, I've reviewed the main refactor in all.go - looks good, moving to the resource-based refactors & tests. It seems that many additions such as object literal restructuring were generated by IDE (?), it makes the code much more readable.

Yes, the IDE I'm using (GoLand) is quite opinionated about code formatting. It makes sense to enforce this style, although I don't know which golangci-lint rule can address that.