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

[Feature] Refactored CLI #7994

Closed
2 tasks done
anushkamittal2001 opened this issue Aug 9, 2023 · 5 comments · Fixed by #7995
Closed
2 tasks done

[Feature] Refactored CLI #7994

anushkamittal2001 opened this issue Aug 9, 2023 · 5 comments · Fixed by #7995
Assignees
Labels
enhancement New feature or request type:cli cli releated issue

Comments

@anushkamittal2001
Copy link
Collaborator

Problem Statement

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.

Solution Description

A proper schema for Kyverno CLI
Resolving most CLI issues on github (Currently categorized as cli:apply, cli:test and cli:others)
Consistency between CLI and webhook
Offer flexibility and support features that Kyverno admission webhook offers
More input and output formats
Test command
Advanced features for CLI

Alternatives

No response

Additional Context

Link to design doc

Slack discussion

No response

Research

  • I have read and followed the documentation AND the troubleshooting guide.
  • I have searched other issues in this repository and mine is not recorded.
@anushkamittal2001 anushkamittal2001 added enhancement New feature or request triage Default label assigned to all new issues indicating label curation is needed to fully organize. labels Aug 9, 2023
@welcome
Copy link

welcome bot commented Aug 9, 2023

Thanks for opening your first issue here! Be sure to follow the issue template!

@chipzoller chipzoller added type:cli cli releated issue and removed triage Default label assigned to all new issues indicating label curation is needed to fully organize. labels Aug 11, 2023
@mvaalexp
Copy link
Contributor

mvaalexp commented Aug 14, 2023

Here are some of the scenarios I have ran into, hope this helps with some of the use cases.

apply

  • apply doesn't work well for rules using context calls (specifically apiCall from what I have seen). You need to include the variables.yaml file (which isn't super clear). A better or clearer approach here is needed. This starts jumbling up the apply and test cli commands
  • mutations are somewhat hidden, since they don't actually run any validation rules, its hard to tell if the mutation actually applied or not

test

  • not enough logging. high v logging should be dumping the context payloads and variable replacements, right now you have to use the debugger to do this
  • mutation - patchedResource doesn't work the majority of the time. If the label on the patched resources doesn't exist, it counts as a pass, if you add it and it is wrong, it fails. the comparison needs to be improved (this is probably difficult)
  • mutation - rules from previous tests are being applied to later test causing test failures (see [Bug] [CLI] kyverno test are applying previous mutation rules to subsequent test cases causing failures #6816)
  • mutation - there are too many files, and each file and resource needs to be perfectly named, otherwise you get unknown failures which makes making changes extremely tedious. This is particularly obvious with patchedResource, some of our directories have tens of files in them for a single test
  • variables file doesn't respect proper yaml for replacement, each variable must be put in as full length path
request.operation: UPDATE

vs

request:
  operation: UPDATE
  • auto-gen policies and variables files are hard to understand because the rule name is generated and prefixed. you have to be fully aware of this in order to get the tests to pass. currently I exclusively rely on resource names instead for consistency

Both apply and test sort of live in harmony together, but we have to split them in order to get them to work, so for example, this is how we organize our repos:

name: my-policy
policies:
  - ../../../policies/my-policy.yaml # policy
resources:
  - ../../../resources/my-policy-resources.yaml #policies for `apply`
  - resources.yaml # test cases for failed policies since these fail the `apply` since they are invalid

So we are using the resources for the apply as the passing use cases in test (so we get best of both worlds) but a separate file for the failing ones. Seems like these ideas should be combined so the files are not living in a bunch of different directories.

If you need clarity on any of these, feel free to reach out.

@anushkamittal2001
Copy link
Collaborator Author

Thank you very much for sharing these @mvaalexp

@realshuting
Copy link
Member

@anushkamittal2001 - what would be the best way to start refactoring? We can start building a list of tasks for the test command.

@eddycharly
Copy link
Member

Closing, all done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request type:cli cli releated issue
Projects
None yet
5 participants