From 3af6061e764277ba0c0e57628d11968c556266d4 Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Tue, 10 Jul 2018 17:53:24 -0700 Subject: [PATCH] Use kube-openapi cmd in Make rules check in existing API rule violations; the Make rule fails if generated violation report differs from the checked-in violation file and prints error message; add documentation. --- build/root/Makefile.generated_files | 20 ++++- hack/import-restrictions.yaml | 1 + hack/testdata/api-rules/README.md | 22 +++++ hack/testdata/api-rules/violations.report | 98 +++++++++++++++++++++++ hack/verify-generated-files-remake.sh | 4 +- pkg/generated/openapi/def.bzl | 6 +- 6 files changed, 143 insertions(+), 8 deletions(-) create mode 100644 hack/testdata/api-rules/README.md create mode 100644 hack/testdata/api-rules/violations.report diff --git a/build/root/Makefile.generated_files b/build/root/Makefile.generated_files index b4d94fba72..8e4eab0531 100644 --- a/build/root/Makefile.generated_files +++ b/build/root/Makefile.generated_files @@ -402,6 +402,10 @@ $(CONVERSION_GEN): $(k8s.io/kubernetes/vendor/k8s.io/code-generator/cmd/conversi OPENAPI_BASENAME := $(GENERATED_FILE_PREFIX)openapi OPENAPI_FILENAME := $(OPENAPI_BASENAME).go OPENAPI_OUTPUT_PKG := pkg/generated/openapi +BOILERPLATE_FILENAME := vendor/k8s.io/code-generator/hack/boilerplate.go.txt +REPORT_FILENAME := $(OUT_DIR)/violations.report +KNOWN_VIOLATION_FILENAME := hack/testdata/api-rules/violations.report +API_RULE_CHECK_FAILURE_MESSAGE := "Error: API rules check failed. Reported violations \"$(REPORT_FILENAME)\" differ from known violations \"$(KNOWN_VIOLATION_FILENAME)\". Please fix API source file if new violation is detected, or update known violations \"$(KNOWN_VIOLATION_FILENAME)\" if existing violation is being fixed. Please refer to hack/testdata/api-rules/README.md and https://github.com/kubernetes/kube-openapi/tree/master/pkg/generators/rules for more information about the API rules being enforced." # The tool used to generate open apis. OPENAPI_GEN := $(BIN_DIR)/openapi-gen @@ -434,14 +438,22 @@ $(foreach dir, $(OPENAPI_DIRS), $(eval \ )) # How to regenerate open-api code. This emits a single file for all results. -$(OPENAPI_OUTFILE): $(OPENAPI_GEN) +# The Make rule fails if generated API rule violation report differs from the checked-in +# violation file, and prints error message to request developer to fix either the API +# source code, or the known API rule violation file. +$(OPENAPI_OUTFILE): $(OPENAPI_GEN) $(KNOWN_VIOLATION_FILENAME) ./hack/run-in-gopath.sh $(OPENAPI_GEN) \ --v $(KUBE_VERBOSE) \ --logtostderr \ -i $$(echo $(addprefix $(PRJ_SRC_PATH)/, $(OPENAPI_DIRS)) | sed 's/ /,/g') \ -p $(PRJ_SRC_PATH)/$(OPENAPI_OUTPUT_PKG) \ -O $(OPENAPI_BASENAME) \ - "$$@" + -h $(BOILERPLATE_FILENAME) \ + -r $(REPORT_FILENAME) \ + "$$@"; \ + diff $(REPORT_FILENAME) $(KNOWN_VIOLATION_FILENAME) || \ + (echo $(API_RULE_CHECK_FAILURE_MESSAGE); exit 1) + # How to build the generator tool. The deps for this are defined in # the $(GO_PKGDEPS_FILE), above. @@ -452,8 +464,8 @@ $(OPENAPI_OUTFILE): $(OPENAPI_GEN) # have to be rebuilt. In that case, make will forever see the dependency as # newer than the binary, and try to "rebuild" it over and over. So we touch # it, and make is happy. -$(OPENAPI_GEN): $(k8s.io/kubernetes/vendor/k8s.io/code-generator/cmd/openapi-gen) - hack/make-rules/build.sh ./vendor/k8s.io/code-generator/cmd/openapi-gen +$(OPENAPI_GEN): $(k8s.io/kubernetes/vendor/k8s.io/kube-openapi/cmd/openapi-gen) + hack/make-rules/build.sh ./vendor/k8s.io/kube-openapi/cmd/openapi-gen touch $@ diff --git a/hack/import-restrictions.yaml b/hack/import-restrictions.yaml index 19e12ac1e9..f9749300a8 100644 --- a/hack/import-restrictions.yaml +++ b/hack/import-restrictions.yaml @@ -114,5 +114,6 @@ - baseImportPath: "./vendor/k8s.io/kube-openapi/" allowedImports: + - k8s.io/apimachinery - k8s.io/kube-openapi - k8s.io/gengo diff --git a/hack/testdata/api-rules/README.md b/hack/testdata/api-rules/README.md new file mode 100644 index 0000000000..3db39936b4 --- /dev/null +++ b/hack/testdata/api-rules/README.md @@ -0,0 +1,22 @@ +## Existing API Rule Violations + +This folder contains the checked-in report file of known API rule violations. +This report file violations.report is used by Make rule during OpenAPI spec generation to make +sure that no new API rule violation is introduced into our code base. + +The report file [**violations.report**](./violations.report) is in format of: + + * ***API rule violation: \,\,\,\*** + +e.g. + + * ***API rule violation: names_match,k8s.io/api/core/v1,Event,ReportingController*** + +Make rule returns error when new generated violation report differs from this +checked-in violation report. If new API rule violation is detected, please fix +the API Go source file to pass the API rule check. The entries in the checked-in +violation report should only be removed when existing API rule violation is +being fixed, but not added. + +For more information about the API rules being checked, please refer to +https://github.com/kubernetes/kube-openapi/tree/master/pkg/generators/rules diff --git a/hack/testdata/api-rules/violations.report b/hack/testdata/api-rules/violations.report new file mode 100644 index 0000000000..eed14c0c50 --- /dev/null +++ b/hack/testdata/api-rules/violations.report @@ -0,0 +1,98 @@ +API rule violation: names_match,k8s.io/api/authorization/v1beta1,SubjectAccessReviewSpec,Groups +API rule violation: names_match,k8s.io/api/core/v1,AzureDiskVolumeSource,DataDiskURI +API rule violation: names_match,k8s.io/api/core/v1,ContainerStatus,LastTerminationState +API rule violation: names_match,k8s.io/api/core/v1,DaemonEndpoint,Port +API rule violation: names_match,k8s.io/api/core/v1,Event,ReportingController +API rule violation: names_match,k8s.io/api/core/v1,FCVolumeSource,WWIDs +API rule violation: names_match,k8s.io/api/core/v1,GlusterfsVolumeSource,EndpointsName +API rule violation: names_match,k8s.io/api/core/v1,ISCSIPersistentVolumeSource,DiscoveryCHAPAuth +API rule violation: names_match,k8s.io/api/core/v1,ISCSIPersistentVolumeSource,SessionCHAPAuth +API rule violation: names_match,k8s.io/api/core/v1,ISCSIVolumeSource,DiscoveryCHAPAuth +API rule violation: names_match,k8s.io/api/core/v1,ISCSIVolumeSource,SessionCHAPAuth +API rule violation: names_match,k8s.io/api/core/v1,NodeResources,Capacity +API rule violation: names_match,k8s.io/api/core/v1,NodeSpec,DoNotUse_ExternalID +API rule violation: names_match,k8s.io/api/core/v1,PersistentVolumeSource,CephFS +API rule violation: names_match,k8s.io/api/core/v1,PersistentVolumeSource,StorageOS +API rule violation: names_match,k8s.io/api/core/v1,PodSpec,DeprecatedServiceAccount +API rule violation: names_match,k8s.io/api/core/v1,RBDPersistentVolumeSource,CephMonitors +API rule violation: names_match,k8s.io/api/core/v1,RBDPersistentVolumeSource,RBDImage +API rule violation: names_match,k8s.io/api/core/v1,RBDPersistentVolumeSource,RBDPool +API rule violation: names_match,k8s.io/api/core/v1,RBDPersistentVolumeSource,RadosUser +API rule violation: names_match,k8s.io/api/core/v1,RBDVolumeSource,CephMonitors +API rule violation: names_match,k8s.io/api/core/v1,RBDVolumeSource,RBDImage +API rule violation: names_match,k8s.io/api/core/v1,RBDVolumeSource,RBDPool +API rule violation: names_match,k8s.io/api/core/v1,RBDVolumeSource,RadosUser +API rule violation: names_match,k8s.io/api/core/v1,VolumeSource,CephFS +API rule violation: names_match,k8s.io/api/core/v1,VolumeSource,StorageOS +API rule violation: names_match,k8s.io/api/extensions/v1beta1,CustomMetricCurrentStatus,CurrentValue +API rule violation: names_match,k8s.io/api/extensions/v1beta1,CustomMetricTarget,TargetValue +API rule violation: names_match,k8s.io/api/policy/v1beta1,PodDisruptionBudgetStatus,PodDisruptionsAllowed +API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,CustomResourceColumnDefinition,JSONPath +API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSON,Raw +API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaProps,Schema +API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaProps,Ref +API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaPropsOrArray,Schema +API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaPropsOrArray,JSONSchemas +API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaPropsOrBool,Allows +API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaPropsOrBool,Schema +API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaPropsOrStringArray,Schema +API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaPropsOrStringArray,Property +API rule violation: names_match,k8s.io/apimachinery/pkg/api/resource,Quantity,i +API rule violation: names_match,k8s.io/apimachinery/pkg/api/resource,Quantity,d +API rule violation: names_match,k8s.io/apimachinery/pkg/api/resource,Quantity,s +API rule violation: names_match,k8s.io/apimachinery/pkg/api/resource,Quantity,Format +API rule violation: names_match,k8s.io/apimachinery/pkg/api/resource,int64Amount,value +API rule violation: names_match,k8s.io/apimachinery/pkg/api/resource,int64Amount,scale +API rule violation: names_match,k8s.io/apimachinery/pkg/apis/meta/v1,APIResourceList,APIResources +API rule violation: names_match,k8s.io/apimachinery/pkg/apis/meta/v1,Duration,Duration +API rule violation: names_match,k8s.io/apimachinery/pkg/apis/meta/v1,InternalEvent,Type +API rule violation: names_match,k8s.io/apimachinery/pkg/apis/meta/v1,InternalEvent,Object +API rule violation: names_match,k8s.io/apimachinery/pkg/apis/meta/v1,MicroTime,Time +API rule violation: names_match,k8s.io/apimachinery/pkg/apis/meta/v1,StatusCause,Type +API rule violation: names_match,k8s.io/apimachinery/pkg/apis/meta/v1,Time,Time +API rule violation: names_match,k8s.io/apimachinery/pkg/runtime,RawExtension,Raw +API rule violation: names_match,k8s.io/apimachinery/pkg/runtime,Unknown,Raw +API rule violation: names_match,k8s.io/apimachinery/pkg/runtime,Unknown,ContentEncoding +API rule violation: names_match,k8s.io/apimachinery/pkg/runtime,Unknown,ContentType +API rule violation: names_match,k8s.io/apimachinery/pkg/util/intstr,IntOrString,Type +API rule violation: names_match,k8s.io/apimachinery/pkg/util/intstr,IntOrString,IntVal +API rule violation: names_match,k8s.io/apimachinery/pkg/util/intstr,IntOrString,StrVal +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,ClientConnectionConfiguration,KubeConfigFile +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,CloudControllerManagerConfiguration,CloudProvider +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,CloudControllerManagerConfiguration,Debugging +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,CloudControllerManagerConfiguration,GenericComponent +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,CloudControllerManagerConfiguration,KubeCloudShared +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,CloudControllerManagerConfiguration,ServiceController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,CloudControllerManagerConfiguration,NodeStatusUpdateFrequency +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,CloudProviderConfiguration,Name +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,JobControllerConfiguration,ConcurrentJobSyncs +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,CloudProvider +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,Debugging +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,GenericComponent +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,KubeCloudShared +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,AttachDetachController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,CSRSigningController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,DaemonSetController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,DeploymentController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,DeprecatedController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,EndPointController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,GarbageCollectorController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,HPAController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,JobController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,NamespaceController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,NodeIpamController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,NodeLifecycleController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,PersistentVolumeBinderController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,PodGCController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,ReplicaSetController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,ReplicationController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,ResourceQuotaController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,SAController +API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeControllerManagerConfiguration,ServiceController +API rule violation: names_match,k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig/v1beta1,KubeletConfiguration,ResolverConfig +API rule violation: names_match,k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig/v1beta1,KubeletConfiguration,IPTablesMasqueradeBit +API rule violation: names_match,k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig/v1beta1,KubeletConfiguration,IPTablesDropBit +API rule violation: names_match,k8s.io/kubernetes/pkg/proxy/apis/kubeproxyconfig/v1alpha1,ClientConnectionConfiguration,KubeConfigFile +API rule violation: names_match,k8s.io/kubernetes/pkg/proxy/apis/kubeproxyconfig/v1alpha1,KubeProxyConfiguration,IPTables +API rule violation: names_match,k8s.io/metrics/pkg/apis/custom_metrics/v1beta1,MetricValue,WindowSeconds +API rule violation: names_match,k8s.io/metrics/pkg/apis/external_metrics/v1beta1,ExternalMetricValue,WindowSeconds diff --git a/hack/verify-generated-files-remake.sh b/hack/verify-generated-files-remake.sh index 5cc44616a2..17ebe9ba41 100755 --- a/hack/verify-generated-files-remake.sh +++ b/hack/verify-generated-files-remake.sh @@ -276,7 +276,7 @@ fi assert_clean -touch staging/src/k8s.io/code-generator/cmd/openapi-gen/main.go +touch vendor/k8s.io/kube-openapi/cmd/openapi-gen/openapi-gen.go touch "${STAMP}" make generated_files >/dev/null X=($(older openapi "${STAMP}")) @@ -288,7 +288,7 @@ fi assert_clean -touch staging/src/k8s.io/code-generator/cmd/openapi-gen/ +touch vendor/k8s.io/kube-openapi/cmd/openapi-gen/ touch "${STAMP}" make generated_files >/dev/null X=($(older openapi "${STAMP}")) diff --git a/pkg/generated/openapi/def.bzl b/pkg/generated/openapi/def.bzl index 07dda76280..5f3f3195d9 100644 --- a/pkg/generated/openapi/def.bzl +++ b/pkg/generated/openapi/def.bzl @@ -38,15 +38,17 @@ def openapi_library(name, tags, srcs, go_prefix, vendor_prefix = "", openapi_tar cmd = " ".join([ "ORIG_WD=$$(pwd);", "cd $$GOPATH/src/" + go_prefix + ";", - "$$ORIG_WD/$(location //vendor/k8s.io/code-generator/cmd/openapi-gen)", + "$$ORIG_WD/$(location //vendor/k8s.io/kube-openapi/cmd/openapi-gen)", "--v 1", "--logtostderr", "--go-header-file $$ORIG_WD/$(location //" + vendor_prefix + "hack/boilerplate:boilerplate.go.txt)", "--output-file-base zz_generated.openapi", "--output-package " + go_prefix + "pkg/generated/openapi", + "--report-filename tmp_api_violations.report", "--input-dirs " + ",".join([go_prefix + target for target in openapi_targets] + [go_prefix + "vendor/" + target for target in vendor_targets]), "&& cp $$GOPATH/src/" + go_prefix + "pkg/generated/openapi/zz_generated.openapi.go $$ORIG_WD/$(location :zz_generated.openapi.go)", + "&& rm tmp_api_violations.report", ]), go_deps = deps, - tools = ["//vendor/k8s.io/code-generator/cmd/openapi-gen"], + tools = ["//vendor/k8s.io/kube-openapi/cmd/openapi-gen"], )