test: add unit test for local crds client #46
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/L
|
I think we need to support yaml files with multiple objects too. |
k8s-ci-robot
added
size/XXL
eddycharly
changed the title
|
@alexzielenski this should be good ! |
pkg/validatorfactory/factory.go
Outdated
| nsScoped := namespaced.Has(gvk) | ||
| // Check schema extensions to see if the scope was manually added | ||
| if scope, ok := def.Extensions.GetString("x-kubectl-validate-scope"); ok { | ||
| nsScoped = scope == strings.ToLower(string(apiextensions.NamespaceScoped)) |
There was a problem hiding this comment.
probably want some integration tests that this is getting correctly propagated by:
- validating yaml for namespace-scoped resource that has no namespace specified
- validating yaml for cluster-scoped resource that has namespace specified
There was a problem hiding this comment.
yes, i can add some tests later if it's ok
pkg/openapiclient/local_crds.go
Outdated
| sch.AddExtension("x-kubernetes-group-version-kind", []interface{}{gvkObj}) | ||
|
|
||
| // Add schema extension to propagate the scope | ||
| sch.AddExtension("x-kubectl-validate-scope", strings.ToLower(string(crd.Spec.Scope))) |
There was a problem hiding this comment.
Since the value of this is an enum, I think it is more correct to leave the CamelCase version as we do in CRDs themselves.
pkg/validatorfactory/factory.go
Outdated
| nsScoped := namespaced.Has(gvk) | ||
| // Check schema extensions to see if the scope was manually added | ||
| if scope, ok := def.Extensions.GetString("x-kubectl-validate-scope"); ok { | ||
| nsScoped = scope == strings.ToLower(string(apiextensions.NamespaceScoped)) |
There was a problem hiding this comment.
use lowercase version of both strings for comparison (considering case where users custom schemas use this to communicate the scope, not just internal code)
There was a problem hiding this comment.
considering case where users custom schemas use this to communicate the scope, not just internal code
what does that mean ?
There was a problem hiding this comment.
considering case where users custom schemas use this to communicate the scope, not just internal code
i switched to lowercase version of both strings for comparison but not sure to understand the sentence above
k8s-ci-robot
added
approved
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
k8s-ci-robot
removed
the
needs-rebase
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexzielenski, eddycharly The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
test: add unit test for local crds client
This PR adds unit tests for local crds client.
Fixes #44
Fixes #47