Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Kyverno CLI #7995

Merged
merged 8 commits into from
Aug 18, 2023
Merged

Conversation

anushkamittal2001
Copy link
Collaborator

@anushkamittal2001 anushkamittal2001 commented Aug 9, 2023

Explanation

Kyverno CLI is one of Kyverno’s oldest components that hasn't gotten much attention recently. This has led to gaps and inconsistency in its behavior providing for a bad user experience. Going forward, correcting the CLI with a new design and proper schema is of importance.

Related issue

Closes #7994

Milestone of this PR

What type of PR is this

/kind feature

Proposed Changes

Correct the way we invoke Kyverno engine in CLI

  1. Split Kyverno commands to more meaningful parts one by one
  2. Simplify .go files one by one - start with apply_command.go
    Split different aspects of the CLI - ex - showing results should be separate render package, just like load resource/policy
    Resolve issues related to CLI while making code changes per command

Proof Manifests

Checklist

  • I have read the contributing guidelines.
  • I have read the PR documentation guide and followed the process including adding proof manifests to this PR.
  • This is a bug fix and I have added unit tests that prove my fix is effective.
  • This is a feature and I have added CLI tests that are applicable.
  • My PR needs to be cherry picked to a specific release branch which is .
  • My PR contains new or altered behavior to Kyverno and
    • CLI support should be added and my PR doesn't contain that functionality.
    • I have added or changed the documentation myself in an existing PR and the link is:
    • I have raised an issue in kyverno/website to track the documentation update and the link is:

Further Comments

This is a draft PR, will keep updating this as and when changes are made.

Signed-off-by: anushkamittal2001 <anushka@nirmata.com>
Signed-off-by: anushkamittal2001 <anushka@nirmata.com>
Signed-off-by: anushkamittal2001 <anushka@nirmata.com>
@eddycharly
Copy link
Member

@anushkamittal2001 can you resolve conflicts ?

Signed-off-by: anushkamittal2001 <anushka@nirmata.com>
@eddycharly
Copy link
Member

@anushkamittal2001 you have linter errors

Signed-off-by: anushkamittal2001 <anushka@nirmata.com>
@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Merging #7995 (072afc2) into main (bdad59c) will increase coverage by 0.04%.
The diff coverage is 49.41%.

@@            Coverage Diff             @@
##             main    #7995      +/-   ##
==========================================
+ Coverage   33.16%   33.20%   +0.04%     
==========================================
  Files         245      245              
  Lines       23039    23086      +47     
==========================================
+ Hits         7640     7666      +26     
- Misses      14600    14614      +14     
- Partials      799      806       +7     
Files Changed Coverage Δ
...ctl-kyverno/utils/common/kyverno_policies_types.go 55.39% <20.00%> (ø)
cmd/cli/kubectl-kyverno/apply/apply_command.go 37.64% <50.29%> (+2.68%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@eddycharly eddycharly marked this pull request as ready for review August 18, 2023 09:57
@eddycharly eddycharly added this to the Kyverno Release 1.11.0 milestone Aug 18, 2023
@eddycharly eddycharly enabled auto-merge (squash) August 18, 2023 10:11
Copy link
Member

@eddycharly eddycharly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🤷

@eddycharly eddycharly merged commit 72ccc55 into kyverno:main Aug 18, 2023
197 of 199 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Refactored CLI
2 participants