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

Kyverno CLI should lint and validate the structure of its test file using the test command #2302

Closed
chipzoller opened this issue Aug 25, 2021 · 20 comments
Assignees
Labels
cli:test The CLI test command related issue. end user This label is used to track the issue that is raised by the end user. enhancement New feature or request good first issue Good for newcomers type:cli cli releated issue

Comments

@chipzoller
Copy link
Contributor

Software version numbers
State the version numbers of applications involved in the bug.

  • Kubernetes version: 1.21
  • Kubernetes platform (if applicable; ex., EKS, GKE, OpenShift): K3d
  • Kyverno version: 1.4.2

Describe the bug
When running with the test command against a malformatted test file, the CLI runs, exits, and prints no message of any sort.

To Reproduce
Steps to reproduce the behavior:

  1. Create a test.yaml file in the structure outlined in the current version of the documentation. For example, like this:
- name: mytest
  policies:
    - pol.yaml
  resources:
    - pod.yaml
  results:
  - policy: evil-policy-match-foreign-pods
    rule: evil-validation
    resource: nginx
    status: pass
  1. Run the kyverno test command against this file (assuming you have the other referenced files in place)
kyverno test .
  1. See that the Kyverno CLI process executes and then exits with no output

Expected behavior

  1. The Kyverno CLI must print something in return that the test file is not valid. It should validate the syntax and linting of the file so it conforms to a known standard.

  2. In addition to item number one, it should validate that the schema of the file conforms to a known and operable schema appropriate/compatible to the CLI version itself. For example, with the latest change to the PolicyReport CRD which was recently merged, which is NOT reflected in the v1.4.2 release, the Kyverno CLI v1.4.2 should validate that the following test.yaml file structure is invalid and not proceed with tests. Currently it happily proceeds and scores results as all Fail.

name: mytest
policies:
  - pol.yaml
resources:
  - pod.yaml
results:
- policy: evil-policy-match-foreign-pods
  rule: evil-validation
  resource: nginx
  result: pass
@chipzoller chipzoller added bug Something isn't working type:cli cli releated issue labels Aug 25, 2021
@NoSkillGirl NoSkillGirl added enhancement New feature or request and removed bug Something isn't working labels Sep 1, 2021
@NoSkillGirl NoSkillGirl changed the title [BUG] Kyverno CLI should lint and validate the structure of its test file using the test command Kyverno CLI should lint and validate the structure of its test file using the test command Sep 1, 2021
@NoSkillGirl NoSkillGirl removed their assignment Nov 12, 2021
@JimBugwadia JimBugwadia added the good first issue Good for newcomers label Feb 14, 2022
@JimBugwadia JimBugwadia added this to the Kyverno Release 1.7.0 milestone Feb 14, 2022
@anutosh491
Copy link
Contributor

Hello @chipzoller and @vyankyGH , hope you're doing well , this is Anutosh here from India . I am an open source enthusiast and I am currently involved in symbolic/numeric computation based libraries like numpy, sympy, networkx and couple others.

I stumbled upon this issue as it was listed under a Kyverno project under the cncf mentoring portal. I am keen to take part in the LFX Mentorship program for the summer term and the project including the library as a whole interests me. But being new to the project , I would be glad if you could syggest any relevant resources/links I should be going through for getting to know the project and the library better . Thank you !

@chipzoller
Copy link
Contributor Author

Hello, @anutosh491. The first and most important thing is to understand Kyverno itself with special emphasis on the CLI. You can see our extensive documentation at kyverno.io which covers both the webhook and the CLI. You are also welcome to join the Slack community in the #kyverno channel in the Kubernetes workspace. Once you have a solid understanding of what Kyverno does and how it works, you should then go through those issues to understand the problem statement, the value in solving the problem, and what an abstract approach might look like. For any questions or problems, we're here to help.

@anutosh491
Copy link
Contributor

anutosh491 commented May 13, 2022

Hello, @anutosh491. The first and most important thing is to understand Kyverno itself with special emphasis on the CLI. You can see our extensive documentation at kyverno.io which covers both the webhook and the CLI.

Thanks for your prompt reply and hopefully I'm on the correct path as of now . After going through the readme and reading the description for Kyverno, I spent some time this morning to dig a bit deeper and get to know exactly what Kyverno does, some history behind it and the problems it is addressing . I also got some decent intuition regarding how it operate and the validate, mutate, generate rules. I will spend more time on the webhook and the CLI in the days to come.
I have also joined the slack channel :)

@Prateeknandle
Copy link
Contributor

2. In addition to item number one, it should validate that the schema of the file conforms to a known and operable schema appropriate/compatible to the CLI version itself. For example, with the latest change to the PolicyReport CRD which was recently merged, which is NOT reflected in the v1.4.2 release, the Kyverno CLI v1.4.2 should validate that the following test.yaml file structure is invalid and not proceed with tests. Currently it happily proceeds and scores results as all Fail.

hey @chipzoller I'm also applying in LFX summer, So basically we need to first validate kyverno-test.yaml and if the yaml file is okay only then apply the test cmd on to the resource otherwise not, and if the schema/format of kyverno-test.yaml is not valid then a proper message will be printed regarding the failure in validation. is it correct?

@chipzoller
Copy link
Contributor Author

The main thing we need to do is establish a formal schema and API spec for these Kyverno test manifests. Just like Kyverno ClusterPolicy and Policy resources, they conform to a formal schema described in OpenAPIv3 format, the same needs to be formalized with these test manifests. And by creating a formal, versioned schema we should then be able to validate the structure is correct and return proper messages when things are out of spec.

@Prateeknandle
Copy link
Contributor

we need to do is establish a formal schema and API spec for these Kyverno test manifests

this schema will be defined in api/kyverno/v1 and validation & execution for changes will be done in cmd/cli/test/test_command.go ?

@basit9958
Copy link

/assign

@chipzoller
Copy link
Contributor Author

@basit9958 this issue is part of an existing LFX mentorship with @Prateeknandle and may be partially, if not fully, completed in his work.

@basit9958 basit9958 removed their assignment Aug 18, 2022
@chipzoller
Copy link
Contributor Author

Should be addressed in #4426
cc @Prateeknandle

@chipzoller chipzoller added LFX mentorship project release-high High issues which SHOULD be addressed in the specified milestone. These may get bumped. labels Sep 8, 2022
@chipzoller chipzoller removed the release-high High issues which SHOULD be addressed in the specified milestone. These may get bumped. label Dec 13, 2022
@chipzoller chipzoller removed this from the Kyverno Release 1.9.0 milestone Dec 13, 2022
@realshuting realshuting added cli:test The CLI test command related issue. end user This label is used to track the issue that is raised by the end user. labels Jul 31, 2023
@eddycharly
Copy link
Member

@chipzoller i tried it and it triggers an error:

./cmd/cli/kubectl-kyverno/kubectl-kyverno test ./scratch/cli
Error: failed to apply test command 
Cause: failed to decode yaml 
Cause: json: cannot unmarshal array into Go value of type api.Test
Usage:
  kyverno test <path_to_folder_Containing_test.yamls> [flags]
  kyverno test <path_to_gitRepository_with_dir> --git-branch <branchName>
  kyverno test --manifest-mutate > kyverno-test.yaml
  kyverno test --manifest-validate > kyverno-test.yaml

...

@chipzoller
Copy link
Contributor Author

I'm not following you here.

@eddycharly
Copy link
Member

@chipzoller I created an invalid test file:

- name: mytest
  policies:
    - pol.yaml
  resources:
    - pod.yaml
  results:
  - policy: evil-policy-match-foreign-pods
    rule: evil-validation
    resource: nginx
    status: pass

Running kyverno test on it returns an error (the file is invalid).
Isn't what this issue is about ?

@chipzoller
Copy link
Contributor Author

Partly, but it has many dimensions as well. Syntactic validation is just one of them.

@eddycharly
Copy link
Member

Ok, would you mind detailing the expectations ?
Creating separate issues could help moving forward IMO.

@eddycharly
Copy link
Member

Right now I want to make sure an invalid file is correctly identified and returns a comprehensive error.

@chipzoller
Copy link
Contributor Author

There are two very basic things here:

  1. Validate the YAML file syntactically.
  2. Validate the YAML file according to a formal schema specific to Kyverno test manifests.

@chipzoller
Copy link
Contributor Author

This is syntactically valid but not schematically valid:

name: mytest
policies:
  - pol.yaml
resources:
  - pod.yaml
results:
- policy: evil-policy-match-foreign-pods
  rule: evil-validation
  resource: nginx
  foo: bar
  result: pass

@eddycharly
Copy link
Member

@chipzoller i created #8144 and opened #8145

@eddycharly
Copy link
Member

@chipzoller i updated the PR to validate the file content schematically too

@eddycharly
Copy link
Member

I think this is done. Strict marshalling is in place for:

  • test file
  • user info file
  • values file
  • policy files

Next we should add advanced validation.
I'll create separate issues for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli:test The CLI test command related issue. end user This label is used to track the issue that is raised by the end user. enhancement New feature or request good first issue Good for newcomers type:cli cli releated issue
Projects
None yet
Development

No branches or pull requests

9 participants