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

Add kubernetes manifest validation #317

Merged
merged 7 commits into from
Jul 23, 2019

Conversation

yoshi-1224
Copy link
Contributor

@yoshi-1224 yoshi-1224 commented Jul 5, 2019

(Partly) fixes issue #194

Proposed Changes

  • Add support for validation of compiled k8s manifests using json schemas from https://kubernetesjsonschema.dev
  • Can be extended for other manifests/compiled output by extending the base validator class. E.g. we might at the same time tackle Support to validate the inventory before compiling #258

@yoshi-1224
Copy link
Contributor Author

yoshi-1224 commented Jul 5, 2019

@uberspot @ramaro @adrianchifor
I have two options in mind regarding how validate should be specified in the inventory:

Option 1

kapitan:
   validate:
      - output_path: <corresponding_path_in_kapitan.compile>
        type: kubernetes
        kind: <kind>
        version: <kubectl-version>

Option 2

I think it is also possible to put validate options under kapitan.compile, such as

kapitan:
   compile:
      - output_path: <output_path>
        input_paths:
           - <input_path>
        validate: kubernetes
        kind: <kind>
        version: <kubectl-version>

But option 2 is not so great because it doesn't work well when one jsonnet file compiles to multiple files, or when there are more than one input_paths (or when input_path is a directory).

If we are going with Option 1, optimally we would want to validate while the object is still in memory, but it can be a bit complicated to do the matching of output_paths. Is it ok if I read the files written to compile_path (i.e. the temporary path before copy to the actual path) to validate them? we can first create hashmap of the output_paths in kapitan.validate, and then after each render operations in jsonnet and so on, we can find the corresponding validation option for that object, if exists.

If there is a better way over both options, feel free to let me know!

@uberspot
Copy link
Contributor

uberspot commented Jul 5, 2019

@uberspot @ramaro @adrianchifor
I have two options in mind regarding how validate should be specified in the inventory:

Option 1

kapitan:
   validate:
      - output_path: <corresponding_path_in_kapitan.compile>
        type: kubernetes
        kind: <kind>
        version: <kubectl-version>

Option 2

I think it is also possible to put validate options under kapitan.compile, such as

kapitan:
   compile:
      - output_path: <output_path>
        input_paths:
           - <input_path>
        validate: kubernetes
        kind: <kind>
        version: <kubectl-version>

But option 2 is not so great because it doesn't work well when one jsonnet file compiles to multiple files, or when there are more than one input_paths (or when input_path is a directory).

If we are going with Option 1, optimally we would want to validate while the object is still in memory, but it can be a bit complicated to do the matching of output_paths. Is it ok if I read the files written to compile_path (i.e. the temporary path before copy to the actual path) to validate them? we can first create hashmap of the output_paths in kapitan.validate, and then after each render operations in jsonnet and so on, we can find the corresponding validation option for that object, if exists.

If there is a better way over both options, feel free to let me know!

Option 1 does seem more versatile.
It makes sense to do Validation as a separate step and NOT combining it with compiling. Semantically it's a separate operation. And if it's implemented separately we can also in the future split it to a separate command kapitan validate, if it's something people want to use. :)

All compiled/ outputs should be generated by kapitan compile anyway (and non valid files are deleted after compilation). So I'd assume it's safe to just run schema validation on the compiled target directory directly (assuming those outputs are k8s manifests in this case).

Copy link
Contributor

@uberspot uberspot left a comment

Choose a reason for hiding this comment

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

Nice start! 👍

kapitan/dependency_manager/base.py Show resolved Hide resolved
kapitan/validator/kubernetes_validator.py Outdated Show resolved Hide resolved
kapitan/validator/kubernetes_validator.py Outdated Show resolved Hide resolved
tests/test_kubernetes_validator.py Outdated Show resolved Hide resolved
tests/test_kubernetes_validator.py Outdated Show resolved Hide resolved
tests/test_kubernetes_validator.py Outdated Show resolved Hide resolved
tests/test_kubernetes_validator.py Outdated Show resolved Hide resolved
@yoshi-1224
Copy link
Contributor Author

yoshi-1224 commented Jul 5, 2019

@uberspot
Thanks for the quick reply! I'll just run the validation on the compiled output dir then, after these lines in compile_targets function:

# compile done in parallel above
shutil.rmtree(compile_path)
shutil.copytree(temp_path, compile_path)
logger.debug("Copied %s into %s", temp_path, compile_path)

@ramaro
Copy link
Member

ramaro commented Jul 5, 2019

How about we detach validation from compile by default? I'm worried compilation will become too slow.

I propose we add a new $ kapitan validate command which will do this instead. We might also add (similar to what was done for deps) a validate_force inventory option to run validate on compile and even perhaps a flag: $ kapitan compile --validate.

I believe this will make it more versatile - but just a thought!

@yoshi-1224
Copy link
Contributor Author

@uberspot, @ramaro
I've created a separate command kapitan validate, which takes in

--schemas-path: defaults to ./schemas. If doesn't exist, will be created
--targets: same as compile i.e. targets to validate
--inventory-path: same as compile, defaults to ./inventory
--compiled-path: same as compile, defaults to ./compiled
-p: same as compile

kapitan compile now has --validate flag as well as --schemas-path (semantically the same as described above).

Copy link
Contributor

@uberspot uberspot left a comment

Choose a reason for hiding this comment

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

Nice work! :)
Small reminder to also add a paragraph in the README explaining this new validation functionality and update the new command line args so that everything is self-contained in the MR as well.

kapitan/targets.py Outdated Show resolved Hide resolved
@yoshi-1224
Copy link
Contributor Author

yoshi-1224 commented Jul 8, 2019

@uberspot
Actually I realized that by making this process parallel as it is now, we bump into the same issue we had with external dependencies fetch (e.g. imagine a situation where two different targets use the same schema for validation). Let me change the code in such a way that each kubernetes_validator corresponds to a unique pair of (kind, version) with a list of files (across different targets) that need to be validated against it, such that we can safely perform validation without race condition during fetching.

@yoshi-1224
Copy link
Contributor Author

@uberspot @ramaro
PR is ready for review, thanks!

README.md Outdated Show resolved Hide resolved
@uberspot uberspot merged commit 1f9f64f into kapicorp:master Jul 23, 2019
@yoshi-1224 yoshi-1224 deleted the manifest-validation branch August 18, 2019 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants