Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API linter framework and namesMatch API rule #65737

Merged
merged 5 commits into from Jul 14, 2018
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+143 −8
Diff settings

Always

Just for now

@@ -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

This comment has been minimized.

Copy link
@mbohlool

mbohlool Jul 11, 2018

Member

should this be called `violation_exceptions.list" or something like that? (change list to the format of the file, csv, yaml, etc?)

This comment has been minimized.

Copy link
@roycaihw

roycaihw Jul 11, 2018

Author Member

Yes it's indeed an exception / whitelist file. I used "violations.report" instead of "whitelist" because I didn't want people to feel that it's okay to add more entries to it.

I updated the README to emphasize that the entries in the list should only be removed but not added. Since the file is not in csv format, I updated the filename to violation_exceptions.list

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) || \

This comment has been minimized.

Copy link
@mbohlool

mbohlool Jul 11, 2018

Member

I feel this should be a hack/verify-* thing. Is this being discussed? we are doing verify work in makefile which is odd.

This comment has been minimized.

Copy link
@roycaihw

roycaihw Jul 11, 2018

Author Member

We integrated the API rule checking into OpenAPI spec generation (in kube-openapi) to provide an "API compiler / toolchain". In Kubernetes we add an extra layer to allow checking existing violation exceptions, but I think we still want new violations to fail in the "compiling" step. If we move the diff to hack/verify-openapi-spec.sh we will have a standalone hack/update-openapi-spec.sh script that passes on new violations (although our CI ensures verification scripts always get run, so violations will get caught eventually).

(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 $@


Copy path View file
@@ -114,5 +114,6 @@

- baseImportPath: "./vendor/k8s.io/kube-openapi/"
allowedImports:
- k8s.io/apimachinery
- k8s.io/kube-openapi
- k8s.io/gengo
@@ -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: \<RULE\>,\<PACKAGE\>,\<TYPE\>,\<FIELD\>***

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
@@ -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
@@ -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}"))
Copy path View file
@@ -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"],

This comment has been minimized.

Copy link
@mbohlool

mbohlool Jul 11, 2018

Member

we should also remove openapi-gen from code-generator repo after this PR got merged.

This comment has been minimized.

Copy link
@roycaihw

roycaihw Jul 11, 2018

Author Member

Sounds good

tools = ["//vendor/k8s.io/kube-openapi/cmd/openapi-gen"],
)
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.