search

kubernetes / kube-openapi

Kubernetes OpenAPI spec generation & serving
Apache License 2.0
319 stars 208 forks source link

Should openapi for types that fail validation be generated? #373

Closed Jefftree closed 1 year ago

Jefftree commented 1 year ago

While investigating https://github.com/kubernetes/kube-openapi/issues/343 I noticed that we have some validation as part of openapi-gen but the errors are silenced: https://github.com/kubernetes/kube-openapi/blob/master/pkg/generators/openapi.go#L501-L506

Errors are printed as a klog.V(2) and the execution of openapi-gen is unaffected if there is a validation error. Based on the comments this seems intentional? cc @apelisse @seans3 since you have worked closely with this code before.

Given the way we invoke hack/update-codegen.sh and call openapi-gen in k/k, the errors are never surfaced and hard for us to see. We already have types that fail validation and are not surfaced. Here's an output of the current logs from running KUBE_VERBOSE=2 ./hack/update-codegen.sh openapi

+++ [0215 21:20:44] Generating openapi code for KUBE
I0215 21:20:52.668368 1565423 openapi.go:504] [k8s.io/api/apps/v1.DeploymentSpec] Strategy k8s.io/api/apps/v1.DeploymentStrategy: tag patchStrategy on type Struct; only allowed on type Slice
I0215 21:20:52.827452 1565423 openapi.go:504] [k8s.io/api/apps/v1beta1.DeploymentSpec] Strategy k8s.io/api/apps/v1beta1.DeploymentStrategy: tag patchStrategy on type Struct; only allowed on type Slice
I0215 21:20:52.942085 1565423 openapi.go:504] [k8s.io/api/apps/v1beta2.DeploymentSpec] Strategy k8s.io/api/apps/v1beta2.DeploymentStrategy: tag patchStrategy on type Struct; only allowed on type Slice
I0215 21:20:54.131100 1565423 openapi.go:504] [k8s.io/api/core/v1.PersistentVolumeSpec] ClaimRef *k8s.io/api/core/v1.ObjectReference: tag structType on type Pointer; only allowed on type Struct
I0215 21:20:54.173002 1565423 openapi.go:504] [k8s.io/api/core/v1.PodSpec] Volumes []k8s.io/api/core/v1.Volume: [merge,retainKeys] not allowed for patchStrategy. Allowed values: [merge retainKeys]
I0215 21:20:54.175759 1565423 openapi.go:504] [k8s.io/api/core/v1.PodSpec] ResourceClaims []k8s.io/api/core/v1.PodResourceClaim: [merge,retainKeys] not allowed for patchStrategy. Allowed values: [merge retainKeys]
I0215 21:20:54.903671 1565423 openapi.go:504] [k8s.io/api/extensions/v1beta1.DeploymentSpec] Strategy k8s.io/api/extensions/v1beta1.DeploymentStrategy: tag patchStrategy on type Struct; only allowed on type Slice
I0215 21:20:55.751010 1565423 openapi.go:504] [k8s.io/api/policy/v1.PodDisruptionBudgetSpec] Selector *k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelector: [replace] not allowed for patchStrategy. Allowed values: [merge retainKeys]
I0215 21:20:55.751048 1565423 openapi.go:504] [k8s.io/api/policy/v1.PodDisruptionBudgetSpec] Selector *k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelector: tag patchStrategy on type Pointer; only allowed on type Slice
I0215 21:20:56.404681 1565423 openapi.go:504] [k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.JSONSchemaProps] XValidations k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.ValidationRules: tag listMapKey on type Alias; only allowed on type Slice
I0215 21:20:56.404719 1565423 openapi.go:504] [k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.JSONSchemaProps] XValidations k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.ValidationRules: tag listType on type Alias; only allowed on type Slice
I0215 21:20:56.404733 1565423 openapi.go:504] [k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.JSONSchemaProps] XValidations k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.ValidationRules: tag patchMergeKey on type Alias; only allowed on type Slice
I0215 21:20:56.404751 1565423 openapi.go:504] [k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.JSONSchemaProps] XValidations k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.ValidationRules: tag patchStrategy on type Alias; only allowed on type Slice
I0215 21:20:56.498787 1565423 openapi.go:504] [k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1.JSONSchemaProps] XValidations k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1.ValidationRules: tag listMapKey on type Alias; only allowed on type Slice
I0215 21:20:56.498821 1565423 openapi.go:504] [k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1.JSONSchemaProps] XValidations k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1.ValidationRules: tag listType on type Alias; only allowed on type Slice
I0215 21:20:56.498831 1565423 openapi.go:504] [k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1.JSONSchemaProps] XValidations k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1.ValidationRules: tag patchMergeKey on type Alias; only allowed on type Slice
I0215 21:20:56.498839 1565423 openapi.go:504] [k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1.JSONSchemaProps] XValidations k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1.ValidationRules: tag patchStrategy on type Alias; only allowed on type Slice
I0215 21:20:56.654604 1565423 openapi.go:504] [k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelectorRequirement] Key string: tag patchMergeKey on type Builtin; only allowed on type Slice
I0215 21:20:56.654639 1565423 openapi.go:504] [k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelectorRequirement] Key string: tag patchStrategy on type Builtin; only allowed on type Slice
I0215 21:20:58.581858 1565423 api_linter.go:43] Assembling file "_output/KUBE_violations.report"
+++ [0215 21:20:58] Generating openapi code for AGGREGATOR
I0215 21:21:00.076475 1569121 openapi.go:504] [k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelectorRequirement] Key string: tag patchMergeKey on type Builtin; only allowed on type Slice
I0215 21:21:00.076543 1569121 openapi.go:504] [k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelectorRequirement] Key string: tag patchStrategy on type Builtin; only allowed on type Slice
I0215 21:21:00.182487 1569121 api_linter.go:43] Assembling file "_output/AGGREGATOR_violations.report"
+++ [0215 21:21:00] Generating openapi code for APIEXTENSIONS
I0215 21:21:02.015522 1569302 openapi.go:504] [k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelectorRequirement] Key string: tag patchMergeKey on type Builtin; only allowed on type Slice
I0215 21:21:02.015602 1569302 openapi.go:504] [k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelectorRequirement] Key string: tag patchStrategy on type Builtin; only allowed on type Slice
I0215 21:21:02.108164 1569302 api_linter.go:43] Assembling file "_output/APIEXTENSIONS_violations.report"
+++ [0215 21:21:02] Generating openapi code for CODEGEN
I0215 21:21:03.606917 1569808 openapi.go:504] [k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelectorRequirement] Key string: tag patchMergeKey on type Builtin; only allowed on type Slice
I0215 21:21:03.606993 1569808 openapi.go:504] [k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelectorRequirement] Key string: tag patchStrategy on type Builtin; only allowed on type Slice
I0215 21:21:03.696560 1569808 api_linter.go:43] Assembling file "_output/CODEGEN_violations.report"
+++ [0215 21:21:03] Generating openapi code for SAMPLEAPISERVER
I0215 21:21:05.201578 1570144 openapi.go:504] [k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelectorRequirement] Key string: tag patchMergeKey on type Builtin; only allowed on type Slice
I0215 21:21:05.201652 1570144 openapi.go:504] [k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelectorRequirement] Key string: tag patchStrategy on type Builtin; only allowed on type Slice
I0215 21:21:05.292513 1570144 api_linter.go:43] Assembling file "_output/SAMPLEAPISERVER_violations.report"
apelisse commented 1 year ago

Yeah, I don't think these are meant to break anything. The way the openapi is generated is super confusing and convoluted.

If these should fail, then we should have a corresponding validation rule created to find them, and they might (or not) be excluded through the reports.

Jefftree commented 1 year ago

These are separate validations from the reports, as discussed offline we should make these more consistent with the violation reports as a first step.

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

k8s-ci-robot commented 1 year ago

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to [this](https://github.com/kubernetes/kube-openapi/issues/373#issuecomment-1637215839): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue with `/reopen` >- Mark this issue as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close not-planned > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.