diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index e37582226e0..08154b1c348 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -14122,12 +14122,12 @@ "description": "CustomResourceConversion describes how to convert different versions of a CR.", "properties": { "strategy": { - "description": "strategy specifies how custom resources are converted between versions. Allowed values are: - `None`: The converter only change the apiVersion and would not touch any other field in the custom resource. - `Webhook`: API Server will call to an external webhook to do the conversion. Additional information\n is needed for this option. This requires spec.preserveUnknownFields to be false, and spec.conversion.webhook to be set.", + "description": "strategy specifies how custom resources are converted between versions. Allowed values are: - `\"None\"`: The converter only change the apiVersion and would not touch any other field in the custom resource. - `\"Webhook\"`: API Server will call to an external webhook to do the conversion. Additional information\n is needed for this option. This requires spec.preserveUnknownFields to be false, and spec.conversion.webhook to be set.", "type": "string" }, "webhook": { "$ref": "#/definitions/io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.WebhookConversion", - "description": "webhook describes how to call the conversion webhook. Required when `strategy` is set to `Webhook`." + "description": "webhook describes how to call the conversion webhook. Required when `strategy` is set to `\"Webhook\"`." } }, "required": [ diff --git a/api/openapi-spec/v3/apis__apiextensions.k8s.io__v1_openapi.json b/api/openapi-spec/v3/apis__apiextensions.k8s.io__v1_openapi.json index 6ff1bf6284c..11133e2ed78 100644 --- a/api/openapi-spec/v3/apis__apiextensions.k8s.io__v1_openapi.json +++ b/api/openapi-spec/v3/apis__apiextensions.k8s.io__v1_openapi.json @@ -45,7 +45,7 @@ "properties": { "strategy": { "default": "", - "description": "strategy specifies how custom resources are converted between versions. Allowed values are: - `None`: The converter only change the apiVersion and would not touch any other field in the custom resource. - `Webhook`: API Server will call to an external webhook to do the conversion. Additional information\n is needed for this option. This requires spec.preserveUnknownFields to be false, and spec.conversion.webhook to be set.", + "description": "strategy specifies how custom resources are converted between versions. Allowed values are: - `\"None\"`: The converter only change the apiVersion and would not touch any other field in the custom resource. - `\"Webhook\"`: API Server will call to an external webhook to do the conversion. Additional information\n is needed for this option. This requires spec.preserveUnknownFields to be false, and spec.conversion.webhook to be set.", "type": "string" }, "webhook": { @@ -54,7 +54,7 @@ "$ref": "#/components/schemas/io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.WebhookConversion" } ], - "description": "webhook describes how to call the conversion webhook. Required when `strategy` is set to `Webhook`." + "description": "webhook describes how to call the conversion webhook. Required when `strategy` is set to `\"Webhook\"`." } }, "required": [ diff --git a/cmd/fieldnamedocscheck/field_name_docs_check.go b/cmd/fieldnamedocscheck/field_name_docs_check.go new file mode 100644 index 00000000000..9271883c9e8 --- /dev/null +++ b/cmd/fieldnamedocscheck/field_name_docs_check.go @@ -0,0 +1,105 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "fmt" + "os" + "regexp" + "strings" + + flag "github.com/spf13/pflag" + kruntime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/klog/v2" +) + +var ( + typeSrc = flag.StringP("type-src", "s", "", "From where we are going to read the types") + re = regexp.MustCompile("`(\\b\\w+\\b)`") +) + +// kubeTypesMap is a map from field name to its tag name and doc. +type kubeTypesMap map[string]kruntime.Pair + +func main() { + flag.Parse() + + if *typeSrc == "" { + klog.Fatalf("Please define -s flag as it is the api type file") + } + + docsForTypes := kruntime.ParseDocumentationFrom(*typeSrc) + rc := false + + for _, ks := range docsForTypes { + typesMap := make(kubeTypesMap) + + for _, p := range ks[1:] { + // skip the field with no tag name + if p.Name != "" { + typesMap[strings.ToLower(p.Name)] = p + } + } + + structName := ks[0].Name + + rc = checkFieldNameAndDoc(structName, "", ks[0].Doc, typesMap) + for _, p := range ks[1:] { + rc = checkFieldNameAndDoc(structName, p.Name, p.Doc, typesMap) + } + } + + if rc { + os.Exit(1) + } +} + +func checkFieldNameAndDoc(structName, fieldName, doc string, typesMap kubeTypesMap) bool { + rc := false + visited := sets.Set[string]{} + + // The rule is: + // 1. Get all back-tick quoted names in the doc + // 2. Skip the name which is already found mismatched. + // 3. Skip the name whose lowercase is different from the lowercase of tag names, + // because some docs use back-tick to quote field value or nil + // 4. Check if the name is different from its tag name + + // TODO: a manual pass adding back-ticks to the doc strings, then update the linter to + // check the existence of back-ticks + nameGroups := re.FindAllStringSubmatch(doc, -1) + for _, nameGroup := range nameGroups { + name := nameGroup[1] + if visited.Has(name) { + continue + } + if p, ok := typesMap[strings.ToLower(name)]; ok && p.Name != name { + rc = true + visited.Insert(name) + + fmt.Fprintf(os.Stderr, "Error: doc for %s", structName) + if fieldName != "" { + fmt.Fprintf(os.Stderr, ".%s", fieldName) + } + + fmt.Fprintf(os.Stderr, " contains: %s, which should be: %s\n", name, p.Name) + } + } + + return rc +} diff --git a/hack/verify-fieldname-docs.sh b/hack/verify-fieldname-docs.sh new file mode 100755 index 00000000000..2997234cf65 --- /dev/null +++ b/hack/verify-fieldname-docs.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash + +# Copyright 2023 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# This script checks API-related files for mismatch in docs and field names, +# and outputs a list of fields that their docs and field names are mismatched. +# Usage: `hack/verify-fieldname-docs.sh`. + +set -o errexit +set -o nounset +set -o pipefail + +KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/.. +source "${KUBE_ROOT}/hack/lib/init.sh" +source "${KUBE_ROOT}/hack/lib/util.sh" + +kube::golang::setup_env + +make -C "${KUBE_ROOT}" WHAT=cmd/fieldnamedocscheck + +# Find binary +fieldnamedocscheck=$(kube::util::find-binary "fieldnamedocscheck") + +result=0 + +find_files() { + find . -not \( \ + \( \ + -wholename './output' \ + -o -wholename './_output' \ + -o -wholename './_gopath' \ + -o -wholename './release' \ + -o -wholename './target' \ + -o -wholename '*/third_party/*' \ + -o -wholename '*/vendor/*' \ + -o -wholename './pkg/*' \ + \) -prune \ + \) \ + \( -wholename './staging/src/k8s.io/api/*/v*/types.go' \ + -o -wholename './staging/src/k8s.io/kube-aggregator/pkg/apis/*/v*/types.go' \ + -o -wholename './staging/src/k8s.io/apiextensions-apiserver/pkg/apis/*/v*/types.go' \ + \) +} + +versioned_api_files=$(find_files) || true + +for file in ${versioned_api_files}; do + package="${file%"/types.go"}" + echo "Checking ${package}" + ${fieldnamedocscheck} -s "${file}" || result=$? +done + +exit ${result} diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index 2de4c7fd3a8..7aa1d381d8a 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -44441,7 +44441,7 @@ func schema_pkg_apis_apiextensions_v1_CustomResourceConversion(ref common.Refere Properties: map[string]spec.Schema{ "strategy": { SchemaProps: spec.SchemaProps{ - Description: "strategy specifies how custom resources are converted between versions. Allowed values are: - `None`: The converter only change the apiVersion and would not touch any other field in the custom resource. - `Webhook`: API Server will call to an external webhook to do the conversion. Additional information\n is needed for this option. This requires spec.preserveUnknownFields to be false, and spec.conversion.webhook to be set.", + Description: "strategy specifies how custom resources are converted between versions. Allowed values are: - `\"None\"`: The converter only change the apiVersion and would not touch any other field in the custom resource. - `\"Webhook\"`: API Server will call to an external webhook to do the conversion. Additional information\n is needed for this option. This requires spec.preserveUnknownFields to be false, and spec.conversion.webhook to be set.", Default: "", Type: []string{"string"}, Format: "", @@ -44449,7 +44449,7 @@ func schema_pkg_apis_apiextensions_v1_CustomResourceConversion(ref common.Refere }, "webhook": { SchemaProps: spec.SchemaProps{ - Description: "webhook describes how to call the conversion webhook. Required when `strategy` is set to `Webhook`.", + Description: "webhook describes how to call the conversion webhook. Required when `strategy` is set to `\"Webhook\"`.", Ref: ref("k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.WebhookConversion"), }, }, diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/generated.proto b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/generated.proto index d0b190fd564..643dda106b0 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/generated.proto +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/generated.proto @@ -107,12 +107,12 @@ message CustomResourceColumnDefinition { // CustomResourceConversion describes how to convert different versions of a CR. message CustomResourceConversion { // strategy specifies how custom resources are converted between versions. Allowed values are: - // - `None`: The converter only change the apiVersion and would not touch any other field in the custom resource. - // - `Webhook`: API Server will call to an external webhook to do the conversion. Additional information + // - `"None"`: The converter only change the apiVersion and would not touch any other field in the custom resource. + // - `"Webhook"`: API Server will call to an external webhook to do the conversion. Additional information // is needed for this option. This requires spec.preserveUnknownFields to be false, and spec.conversion.webhook to be set. optional string strategy = 1; - // webhook describes how to call the conversion webhook. Required when `strategy` is set to `Webhook`. + // webhook describes how to call the conversion webhook. Required when `strategy` is set to `"Webhook"`. // +optional optional WebhookConversion webhook = 2; } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types.go index 285058d77a2..59ec0e372b8 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types.go @@ -74,12 +74,12 @@ type CustomResourceDefinitionSpec struct { // CustomResourceConversion describes how to convert different versions of a CR. type CustomResourceConversion struct { // strategy specifies how custom resources are converted between versions. Allowed values are: - // - `None`: The converter only change the apiVersion and would not touch any other field in the custom resource. - // - `Webhook`: API Server will call to an external webhook to do the conversion. Additional information + // - `"None"`: The converter only change the apiVersion and would not touch any other field in the custom resource. + // - `"Webhook"`: API Server will call to an external webhook to do the conversion. Additional information // is needed for this option. This requires spec.preserveUnknownFields to be false, and spec.conversion.webhook to be set. Strategy ConversionStrategyType `json:"strategy" protobuf:"bytes,1,name=strategy"` - // webhook describes how to call the conversion webhook. Required when `strategy` is set to `Webhook`. + // webhook describes how to call the conversion webhook. Required when `strategy` is set to `"Webhook"`. // +optional Webhook *WebhookConversion `json:"webhook,omitempty" protobuf:"bytes,2,opt,name=webhook"` }