Kubernetes apimachinery dependency issue for InternalEvent

michaeljmarshall commented 5 years ago

Summary

I'm trying to use this library to generate the JSON openapi spec for the spark-on-k8s-operator, but I am running into an issue going from the generated models to the spec. Ultimately, I think root issue is a missing +k8s:openapi-gen=false annotation in the kubernetes apimachinery module. Since those annotations are used by this library, I wanted to verify my proposed change here before submitting a PR there to kubernetes/kubernetes.

Proprosal

Add the +k8s:openapi-gen=false annotation to the InternalEvent type defined in k8s apimachinery. Link to the actual type definition: type InternalEvent. Reference copied here for convenience:

// InternalEvent makes watch.Event versioned
// +protobuf=false
type InternalEvent watch.Event

Environment Setup

To simplify the duplication of my issue, I've created the following Dockerfile.

# Initial portion of Dockerfile is copied from here: https://github.com/GoogleCloudPlatform/spark-on-k8s-operator/blob/master/Dockerfile
FROM golang:1.12.5-alpine as builder
ARG DEP_VERSION="0.5.3"
RUN apk add --no-cache bash git
ADD https://github.com/golang/dep/releases/download/v${DEP_VERSION}/dep-linux-amd64 /usr/bin/dep
RUN chmod +x /usr/bin/dep

# Get necessary go sources
RUN go get -u k8s.io/code-generator/cmd/openapi-gen
RUN go get -d github.com/GoogleCloudPlatform/spark-on-k8s-operator

# Get the dependencies and run openapi-gen to get the models
WORKDIR ${GOPATH}/src/github.com/GoogleCloudPlatform/spark-on-k8s-operator
RUN dep ensure
RUN go run ../../../k8s.io/code-generator/cmd/openapi-gen/main.go \
-i github.com/GoogleCloudPlatform/spark-on-k8s-operator/pkg/apis/sparkoperator.k8s.io/v1beta2/,k8s.io/api/core/v1,k8s.io/apimachinery/pkg/api/resource,k8s.io/apimachinery/pkg/apis/meta/v1,k8s.io/apimachinery/pkg/util/intstr,k8s.io/apimachinery/pkg/runtime \
-p github.com/GoogleCloudPlatform/spark-on-k8s-operator/pkg/apis/sparkoperator.k8s.io/v1beta2/ \
-h hack/custom-boilerplate.go.txt

# Change directories for ease of finding generated code.
WORKDIR pkg/apis/sparkoperator.k8s.io/v1beta2
ENTRYPOINT /bin/bash

After running docker build -t spark-on-k8s-operator:codegen passing in the above Dockerfile, you can run docker run -it spark-on-k8s-operator:codegen to get access to the openapi_generated.go file in the initial directory after getting into the container.

Issue

Now that we have a shared set up, you can open openapi_generated.go and inside, find one of the defined models:

func schema_pkg_apis_meta_v1_InternalEvent(ref common.ReferenceCallback) common.OpenAPIDefinition {                                                                                                                             
        return common.OpenAPIDefinition{                                                                                                                                                                                        
                Schema: spec.Schema{                                                                                                                                                                                            
                        SchemaProps: spec.SchemaProps{                                                                       
                                Description: "InternalEvent makes watch.Event versioned",                                  
                                Type:        []string{"object"},                                                             
                                Properties: map[string]spec.Schema{                                                        
                                        "Type": {                                                                          
                                                SchemaProps: spec.SchemaProps{                                               
                                                        Type:   []string{"string"},                                         
                                                        Format: "",                                                                     
                                                },                                                                              
                                        },                                                                                          
                                        "Object": {                                                                        
                                                SchemaProps: spec.SchemaProps{                                              
                                                        Description: "Object is:\n * If Type is Added or Modified: the new state of the object.\n * If Type is Deleted: the state of the object immediately before deletion.\n * If Type is Error: *api.Status is recommended; other types may make sense\n   depending on context.",
                                                        Ref:         ref("k8s.io/apimachinery/pkg/runtime.Object"),                                                                                                                                                                                                                  
                                                },                                                                                                                                                                                                                                                                                   
                                        },                                                                                                                                                                                                                                                                                           
                                },                                                                                                                                                                                                                                                                                                   
                                Required: []string{"Type", "Object"},                                                                                                                                                                                                                                                                
                        },                                                                                                                                                                                                                                                                                                           
                },                                                                                                                                                                                                                                                                                                                   
                Dependencies: []string{                                                                                                                                                                                                                                                                                              
                        "k8s.io/apimachinery/pkg/runtime.Object"},                                                                                                                                                                                                                                                                   
        }                                                                                                                                                                                                                                                                                                                            
}

It is problematic that the model specifies k8s.io/apimachinery/pkg/runtime.Object as a dependency because runtime.Object is not annotated to be a part of the output of running openapi-gen. Because this dependency isn't defined within the generated output, I get the following error when I call the k8s.io/kube-openapi/pkg/builder.BuildOpenAPIDefinitionsForResources method (defined in this project):

2019/09/20 12:55:31 ERROR: cannot find model definition for k8s.io/apimachinery/pkg/runtime.Object. If you added a new type, you may need to add +k8s:openapi-gen=true to the package or type and run code-gen again
exit status 1

Possible Solution

I was able to get past this issue by going into the vendored file /go/src/github.com/GoogleCloudPlatform/spark-on-k8s-operator/vendor/k8s.io/apimachinery/pkg/apis/meta/v1/watch.go, adding the proposed annotation, and then re-running the openapi-gen code. That change successfully removes the reference to InternalEvent in the generated output, which removes the dependency on runtime.Object, and then the subsequent call to BuildOpenAPIDefinitionsForResources can complete successfully.

It seems to me that the most reasonable solution is to remove the InternalEvent from the openapi-gen output considering that this type is excluded from protobuf generation, that it's name implies it'd be unnecessary in a client, and that the runtime.Object is not new and is not annotated to be included in the openapi-gen output. However, I have not used this library before, and I am new to kubernetes code generation. It's possible that I'm not using this project correctly. Does it make sense to submit a PR to the kubernetes project to add the annotation? Any help or direction here is greatly appreciated. Thanks!

sttts commented 5 years ago

Are you trying to generate the spec for a CRD? Then better use kubebuilder's controller-tools, e.g. the schemapatch command (https://github.com/kubernetes-sigs/controller-tools/blob/master/cmd/controller-gen/main.go#L140).

I would consider the openapi generator here deprecated and only to be used for the very specific use-case of aggregated API servers.

michaeljmarshall commented 5 years ago

@sttts -- thanks for your insight here. I didn't realize the limited scope of this generator. I have a SparkApplication CRD that is defined in the spark-on-k8s-operator repo. My end goal is to generate a Java client that knows about the CRDs defined in that project so that I can introduce type safety as well as make it easier to handle updates to the spark operator project. After searching through various projects, I had thought I would need to use this project to generate the go code (modules?) that I could then use to generate the openapi-spec json file. I thought I'd need an output similar to the one found in the kubernetes project found here, and then I'd use the json file as the input to the kubernetes-client/gen project. Are you saying there is another way to make the openapi-spec json file for use by the client gen project? Or are you saying I can use the resulting yaml file from the controller-tools command to build the Java client?

sttts commented 5 years ago

https://github.com/kubernetes-sigs/controller-tools/blob/master/cmd/controller-gen/main.go#L140 will give you a schema in your CRD manifest. Create that in a cluster and download the OpenAPI spec from the cluster: kubectl get --raw /openapi/v2. That should do it for client generators.

michaeljmarshall commented 5 years ago

@sttts - I am not sure I am following you. It seems inverted to me that I'd need to inspect my kubernetes cluster in order to get an OpenApi spec for a well defined CRD. There should be a way to generate an OpenApi spec that I could then use to generate a client library containing the data models for that CRD. I'd use the generate CRD code as a library to simplify serialization and deserialization of the objects when calling the CustomObjectsApi.

I opened this issue because I was able to generate the OpenApi spec using this project along with my minor change (described above) to the k8s InternalEvent type defined in the k8s apimachinery package. From the OpenApi spec, I was able to generate a client library containing the classes for the CRD. (I used this project https://github.com/kubernetes-client/gen to generate the client.)

From my perspective, this project seems capable of generating a valid OpenApi spec that could be used to generate data models that align with CRDs.

sttts commented 4 years ago

It might partly work now, but is no supported use-case of openapi-gen and it won't be extended towards CRDs. The spec is incomplete and also partly wrong (AFAIK, int-or-string is not generated correctly, maybe other types too).

michaeljmarshall commented 4 years ago

It surprises me that there isn't an easy way to generate library definitions for CRDs. Thank you for the additional context and help here.

alexec commented 4 years ago

Reading comments on bug was key in helping me solve my own bug. Thank you.

kubernetes / kube-openapi