Add automatic generation of CRDs for CNP and CCNP#11607
Merged
aanm merged 20 commits intocilium:masterfrom Aug 6, 2020
Merged
Conversation
|
Please set the appropriate release note label. |
Member
Author
|
test-me-please |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
868111a to
d0f027a
Compare
Member
Author
|
test-me-please |
09791d1 to
b498944
Compare
Member
Author
|
test-me-please |
b498944 to
03d0d6d
Compare
Member
Author
|
test-me-please |
Member
Author
|
Rebased again |
To generate CRD validation schemas (also known as structual validation schemas[1]), we bring in controller-tools. This houses a tool called controller-gen which performs various K8s related code generation. Note that we are using a fork of controller-tools due to the upstream (as of v0.3.0) lacking support in two main areas. 1) A type which implements custom JSON marshalling has its validation schema generation skipped. In our case, api.Rule (which is set as the Spec field in CNP and CCNP) implements custom JSON marshalling. With the upstream tool, this skips the validation generation for it. In our fork, we revert this feature. See PR: kubernetes-sigs/controller-tools#427 2) For CNP and CCNP, we require `oneOf` validation for the endpointSelector and nodeSelector fields. This is because we want to validate that either endpointSelector OR nodeSelector is present, but not both, and at least one. In the upstream, there's currently an open PR that implements support for this. We have manually merged it in our fork. See PR: kubernetes-sigs/controller-tools#298 This commit also brings updates to our dependencies: - gomega v1.7.0 -> v1.8.1 - golang.org/x/internal/{tools,event...} [1]: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#specifying-a-structural-schema Co-authored-by: Chris Tarazi <chris@isovalent.com> Signed-off-by: André Martins <andre@cilium.io> Signed-off-by: Chris Tarazi <chris@isovalent.com>
This is used for embedding the generated CRD YAMLs into binary form. Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
Now that the "Check k8s generated files" step includes generating CRDs and running go-bindata to serialize the CRDs into binary form, we need to install go-bindata. Signed-off-by: Chris Tarazi <chris@isovalent.com>
JSON tags are required for controller-gen, a tool for generating CRDs. This commit adds JSON tags to all struct fields that are part of the API, i.e. CRD. Signed-off-by: Chris Tarazi <chris@isovalent.com>
The reason for this commit is because the controller-gen tool used for CRD generation requires fields in the a CRD struct to be exported. In addition, the tool requires that all struct fields have JSON tags. This commit temporarily exports these fields and leaves indicators for future refactor. Once https://go-review.googlesource.com/c/tools/+/245857 is merged, this commit can be reverted. Signed-off-by: Chris Tarazi <chris@isovalent.com>
This commit separates out CiliumNetworkPolicy from v2/types.go to prepare for use with the controller-gen tool for CRD generation. Signed-off-by: Chris Tarazi <chris@isovalent.com>
This commit separates out CiliumClusterwideNetworkPolicy (CCNP) from v2/types.go to prepare for use with the controller-gen tool for automatic CRD generation. Signed-off-by: Chris Tarazi <chris@isovalent.com>
This commit explicitly embeds metav1.TypeMeta and metav1.ObjectMeta, as these are needed for the following commits which introduce automated CRD generation using the controller-gen tool. Due to the (redundant) embedding, changes were required in pkg/k8s. We need to embed these fields (for now) because controller-gen will not detect a CRD type unless it has those fields. This commit also modifies the deepequal code for CNP and CCNP to deduplicate the functions. The deepcopy generated code has also been updated. Signed-off-by: Chris Tarazi <chris@isovalent.com>
This allows us to avoid any JSON marshaling issues with controller-gen tool, and gets rid of our handmade Timestamp type. Note that this commit adds the extra JSON tag, as that will be important when we start using the controller-gen tool to automate the generation of CRDs. Without the tag, the tool will complain about struct fields which don't have JSON tags. Signed-off-by: Chris Tarazi <chris@isovalent.com>
This commit helps keep the description field in the CRD to have consistent formatting across all the types. It also moves all multi-line marker comments together akin to the formatting from the core K8s CRD files (pkg/k8s/**/k8s/apis/core/v1/types.go). Signed-off-by: Chris Tarazi <chris@isovalent.com>
This commit provides a replacement for handwritten CRD schema validation
via annotating struct fields with markers for use with the
controller-gen tool.
Running the tool and generating the binary representation have been
automated in these Makefile targets:
1) `make manifests` which will run the automatic CRD generator tool
and outputs CRD YAMLs into `examples/crds/`.
2) `make go-bindata` which will embed the binary representation of the
CRD YAMLs so that it can be imported directly into Cilium.
Note that this change bring back go-bindata, a tool that we previously
removed in cilium#10177. This can be swapped in the future if we decide
there's a better alternative.
Commits following this will add the bindata.go file which contains the
binary representation of the generated CRD.
Co-authored-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
The reason for this commit is to generate validation for MatchLabels. In upstream controller-tools, in order to generate validation for maps, a new type (either for key or value) or a type alias must be defined so that validation markers (`+kubebuilder...`) can be placed on the type. There is not upstream support which would allow not having to define new types. If this changes in the future, we should revert this commit, and apply the new markers appropriately. This commit relies on support in our fork of controller-tools: christarazi/controller-tools@1b05110 Signed-off-by: Chris Tarazi <chris@isovalent.com>
This commit allows us to reject policies with invalid entities at the apiserver level as soon as a user has applied the YAML. Signed-off-by: Chris Tarazi <chris@isovalent.com>
For unknown reasons, we need to manually add the metadata.creationTimestamp column (as "Age" in kubectl output) even though this is not a Cilium-specific field (it is part of metav1.ObjectMeta type). Thus, we are omitting adding the description to this column because we want to avoid hardcoding the documentation of the field. This CRD is generated using https://book.kubebuilder.io/reference/generating-crd.html. Signed-off-by: Chris Tarazi <chris@isovalent.com>
Fakes and deepcopy generated code have also been updated. This CRD is generated using https://book.kubebuilder.io/reference/generating-crd.html. Signed-off-by: Chris Tarazi <chris@isovalent.com>
This commit introduces a function called GetPregeneratedCRD. Now that we generate CRDs with the controller-gen tool, it allows us to also generate validation schema as well. This commit refactors how we retrieve the pregenerated CRDs. This refactor allows us to pass the pregenerated CRD to the validator package. Signed-off-by: Chris Tarazi <chris@isovalent.com>
Previously, this validation was done at the apiserver level with handwritten schema validation. Now that we have automated CRD schema validation, we must implement this check at the agent level as a stopgap. The reason for this is that given the relationship of CCNP and CNP (CCNP embeds CNP), it was impossible to represent the following in controller-gen marker comments to generate validation schema: - CNP only supports EndpointSelector. - CCNP supports both EndpointSelector and NodeSelector, but only one at a time in a policy. This commit is meant to be reverted once an approach for seperating the CNP and CCNP CRD types has been implemented. Signed-off-by: Chris Tarazi <chris@isovalent.com>
Bumping the version following updates allowing automated schema generation. Signed-off-by: Chris Tarazi <chris@isovalent.com>
Member
Author
|
As discussed offline, the following additions were made:
|
Member
Author
|
test-me-please |
Member
|
Travis was green in https://travis-ci.com/github/cilium/cilium/jobs/369231108 |
This was referenced Aug 10, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This paves the path for generating CRDs using controller-tools's controller-gen tool. This will help in preparation for the Kubernetes 1.19 release. A significant benefit is the ability to generate the validation schemas, instead of writing them by hand, which can be error-prone.
See commit msgs.
Updates #11476
Related #11142