Istio: Create Go client libraries for Istio CRD resources

[ozevren]

As a low-hanging fruit, we should generate Go client libraries for customers to programmatically access our resources.

The input should be protos in istio/api, along with any additional metadata we might need to generate the libraries (i.e. CRD version metadata). We need to generate enough basic Go code, so that we can use Kubernetes code generator (https://github.com/kubernetes/code-generator) to generate Go client libraries that are consistent with the rest of the Kubernetes client libraries.

The work here includes:

• [ ] Identifying missing metadata that we need to supply to the code generation tool (i.e. API version info, or any other serialization related hints). Design the input model for this (i.e. could be a crdinfo.yaml, that sits next to the proto).
• [ ] Write a protoc plugin in istio/tools repo that we can use in istio/api that can generate the basic Go code to be fed to the code generation tool.
• [ ] Update istio/api Makefile to generate the client libraries to within istio/api. A good place should be isito.io/api/client (with a top-level Readme.md in that folder).

[ozevren]

/cc @geeknoid

[wackxu]

/cc

[lichuqiang]

Another notable thing is the validation stuff.
I had some related experience in kubebuilder. There we defined some grammar, and users need to add comments for validation generation. But I feel the support kind of limited, and not so satisfactory. So I wonder what extent we want to make it now.

I noticed we don't have validation in current helm template (we do have some validation for pilot resource that are manually written and not registered to Kubernetes). So maybe we should focus on the clients/crds first ,and leave the part to next step?

[lichuqiang]

An input file for crd/client generation might look like this:

group: networking
version: v1alpha3  # extract the fields from file path?
annotations: "helm.sh/hook:crd-install"
labels: "app:istio-pilot"
categories: "istio-io,networking-istio-io"
typesToGenerate:
  - name: DestinationRule
    scope: namespaced
    validations:
      ....
  - name: ServiceEntry
    scope: namespaced
  - name: VirtualService
    scope: namespaced
  - name: Gateway
    scope: namespaced
  - name: EnvoyFilter
    scope: namespaced   

Users will need to specify the file per group. The plugin should be able to:

1. read the types specified in the file
2. range over the directory for go files with gengo, and search for the types to generate kubernetes formated types file.
3. generate clients/informer... with code-generator tools

And the plugin can get enough information for CRD template generation, except for the "validation and subresource" part. As validation should be defined in field level, but not resource level. I can't decide how it looks without detail type definition in the file. Any suggestions?

[ozevren]

I think getting something tactical to begin with will give us pretty good mileage. Istio's source of truth for configuration is protos. Having a toolchain that starts with protos as input will help a lot with consistency.

For the long term, I haven't done any homework in this regard (so there might be gaps that may make this infeasible), but at a very high-level, a CRD definition with enough schema definition in it could be a good source-of-truth for client code generation. It would also be independent of Istio, and is generally applicable to Kubernetes CRDs. If we had that, then we can create a tool for Istio that translates Proto => CRD w/ schema definition. If we use https://github.com/lyft/protoc-gen-validate to annotate protos with validation constraints, then we can use it to create a CRD with enough schema data in it.

So in short:
For short-term, having a basic proto plugin that can generate enough Go structs. (No validation stuff)
For long-term, create two tools, one that takes Open API schema annotated CRDs (https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#validation) and create client libraries from it, and another that takes validation annotated protos from Istio API repo and generate the CRD files.

[lichuqiang]

For long-term, create two tools, one that takes Open API schema annotated CRDs (https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#validation) and create client libraries from it

So for the long-term, you'd suggest we get rid of the intermediate GO structs (and also the code-generator tools), and should be able to create client libraries from CRD yaml templates directly?

[ozevren]

We can still create Go structs, but source of truth would be CRD
definition.

On Tue, Sep 18, 2018 at 6:38 PM lichuqiang [e-mail] wrote:

For long-term, create two tools, one that takes Open API schema annotated
CRDs (
https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#validation)
and create client libraries from it

So for the long-term, you'd suggest we get rid of the intermediate GO
structs (and also the code-generator tools), and should be able to create
client libraries from CRD yaml templates directly?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/istio/istio/issues/8772#issuecomment-422616799, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AQR8I_MvcrOSy9b4F_UoACn1lQNqIoyfks5ucaASgaJpZM4Wsfv_
.

[lichuqiang]

Okay, So you'd suggest we add annotation to existing proto files to indicate generation information; Or, like I described above, create extra file for that?

[ozevren]

I'd suggest an extra file. Designing and getting annotations right for this purpose is tricky, not to mention dependency problems between the istio/api and istio/tools repos. An on-the-side file, similar to you've shown above is helpful.

However, for the short term, this will need to be kept in sync with CRD definitions in install folder. As such, I'd suggest putting as little information as possible, only what is needed. We can expand it later.

[lichuqiang]

I did some investigation on the issue. Unfortunately, seems if we wanna make use of the standard client libraries, we must have the meta fiels(TypeMeta, ObjectMeta) defined in the types:

So, do you think it acceptable to add the fields to existing resources?
In fact kubernetes apiserver will polulate the meta fields for CRD objects automatically, currently in istio we just throw them away when doing unmarshal

[ozevren]

The protos we have should map to the spec portion of the CRD resources. We need to generate an envelope type that has TypeMeta and ObjectMeta.

[lichuqiang]

We need to generate an envelope type that has TypeMeta and ObjectMeta.

But the client libraries codes depend on the "envelope type" here, we can't treat the types as intermediate GO structs. we'll need to keep the types and introduce them to make the clients work. They would in fact replace current API types this way.

So we will expect to keep the "envelope type" as final API definition?

[ozevren]

I am not sure what you mean by API types. We will always generate proto Go code and will always use these types within Istio components. That will not change.

The idea is to create a completely independent set of Go based client library, for the benefit of Istio users. This doesn't have to use the generated proto Go code. Even though it would be great if it did (as it would be usable within Istio, and the generation code that needs to be written would be much simpler), it is not necessary to accomplish the goal of creating a set of client libraries for Istio.

So, TL;DR is: we generate all of the Go structs necessary from the input protos, independent of the Go code that is generated from protos themselves.

Here is a strawman:

// Cat represents a networkable, domesticated cat in Istio.
message Cat {
   string responds_to_name = 1;
   bool retained_claws = 2;
}

Run protoc tool with the to-be-developed plugin:

  $ protoc --istioapiplugin_out=${ROOT}/client  cat.proto

// Generate envelope:

// Auto-generated envelope struct for Cat.
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// +k8s:deepcopy-gen=true
struct CatResource {
    meta_v1.TypeMeta    `json:",inline"`
    meta_v1.ObjectMeta `json:"metadata"`

    Spec    Cat `json:"spec"`
}

// Cat represents a networkable, domesticated cat in Istio.
struct Cat {             // <=== Preferred if this is proto-generated (separately), but we can also generated this, if needed.
    RespondsToName string `json:...`
    RetainedClaws bool    `json:...`
}

[ozevren]

Yes, those types needs to be kept around. They need to be generated in a completely orthogonal way to the proto Go code we have today.

[lichuqiang]

The idea is to create a completely independent set of Go based client library, for the benefit of Istio users.

Ah, I thought we expect to switch istio components to use the new clients. If it's an alternative selection for Istio users, then that makes sense.

[ozevren]

Just following up on this. Any progress?

[lichuqiang]

One thing I can't decide is that in the proto files, we have some fields defined with google_protobuf types.
I guess users would not like to see this in k8s style codes. Take *google_protobuf.Duration as an example, the knative folks make it string: https://github.com/knative/pkg/blob/master/apis/istio/v1alpha3/virtualservice_types.go#L179

As I'm not so familiar with protobuf, wondering if there are well-known rules for converting it's types to generic go types?

[ozevren]

There are well-known conversion functions:
https://github.com/golang/protobuf/blob/master/ptypes/duration.go#L76

On Tue, Oct 9, 2018 at 9:07 PM lichuqiang [e-mail] wrote:

One thing I can't decide is that in the proto files, we have some fields
defined with google_protobuf types.
I guess users would not like to see this in k8s style codes. Take
*google_protobuf.Duration as an example, the knative folks make it
string:
https://github.com/knative/pkg/blob/master/apis/istio/v1alpha3/virtualservice_types.go#L179

As I'm not so familiar with protobuf, wondering if there are well-known
rules for converting it's types to generic go types?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/istio/istio/issues/8772#issuecomment-428430984, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AQR8IxFSBYb_PRi_p6rj5fpzwAUmgyUOks5ujXJygaJpZM4Wsfv_
.

[lichuqiang]

/unassign

[ayj]

cc @tcnghia

[michaelkipper]

This issue was referenced in https://groups.google.com/d/msg/istio-users/RNrQTudtIdk/cT5eq4KmAQAJ

I'd love to see a solution to this in the API repo.

[ozevren]

Well, we wouldn't mind getting community to help to get this implemented. :)

I think this is fairly straightforward to implement, but involves fair bit of coding and testing. We should be able to generate the client library via a plugin to the protoc compiler.

[tcnghia]

cc @ilackarms

[ozevren]

cc @davidebbo

[ekuefler]

It looks like the workaround for the time being is to use the dynamic client as described at https://dwmkerr.com/manipulating-istio-and-other-custom-kubernetes-resources-in-golang/.

[rcernich]

I've been looking into this and have written a code generator to generate types.go and register.go files for tagged istio types. The new generator can be run after generate-protos.sh. After that, the standard k8s code generators can be run to generate clientsets, informers, deepcopy, etc. The benefit of this approach is that type generation is essentially driven off the proto files, using a tag in the comments for the type. The only additional piece of required is a doc.go file in the package, which is used to identify the package and group/version to be used for the generated kube types.

I've got a fully functional prototype, but have not yet added it into the build.

[rcernich]

I've created istio/tools#37 with a tool to generate types.go and register.go, which wrap Istio types. istio/api#764 contains the generated k8s apis, including client sets, informers and listers.

There may be some tweaking needed in the builder for the api project (specifically, a new protoc builder image needs to be pushed and integrated into the build). Aside from that, the changes are ready to go.

[TimBozeman]

Links <3
PR for the tools project.
PR for the api project.

[ivelichkovich]

Is there an ETA on this?

[michaelkipper]

@ivelichkovich: In the meantime, you can use https://github.com/michaelkipper/istio-client-go

[Kyrremann]

@michaelkipper have you tried to convert your repo to go modules? Aka, does it work with go modules repos?

Cause I've been struggling today with not be able to "go get" the golang.org/x/sync module.

[nrjpoddar]

@michaelkipper thanks for patching istio-client-go with the correct JSON Marshalers/Unmarshalers. Feel free to provide a PR into https://github.com/aspenmesh/istio-client-go so that you don't have to maintain a fork if you wish, else we are working on addressing the marshaling issue.

[sdake]

There are 20 thumbs up on this PR, and I tend to agree, it is highly valuable. What is the next step required to move this work forward? Having different folks maintain different versions of typed APIs for Istio's custom resources is not really sustainable for our community.

I believe an annotation approach with the actual API in some other repo such as istio-ecosystem/istio-client makes sense to me.

Could we have a little more discussion on what is needed?

Cheers
-steve

[rcernich]

It's been a matter of me finding time to migrate the work to a new repo. I've since moved on to other tasks and haven't been able to get back to this. FWIW

[rcernich]

More details can be found in the PR https://github.com/istio/api/pull/764

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically closed because it has not had activity in the last month and a half. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted". Thank you for your contributions.

[ekuefler]

This would still be very valuable to have, is there any way someone can pick it back up?

[ilackarms]

@ekuefler while it's not a 'traditional' kubernetes clientset (it uses some custom client under the hood), the Supergloo Istio Clientset work for us at solo-io. Then again, most of our projects leverage solo-kit which provides the code gen and implementation for these clients.

if you're interested and need help, ping me in the thread (or come open an issue on supergloo) and i can guide you through it

[nrjpoddar]

@ekuefler you can use https://github.com/aspenmesh/istio-client-go which supports most of the Istio CRD resources.

[howardjohn]

I think this was unintentionally closed -- @nrjpoddar @ozevren there was some work ongoing on this right?

[lxfontes]

me thinks it is possible to get https://github.com/istio/istio/blob/master/galley/tools/gen-meta/metadata.yaml and go template our way out

[lxfontes]

something like this: https://github.com/lxfontes/istio-client-go ( note i had an old template, hence the DeepCopy method is still not there )

if this direction is something the istio team want to pursue, assign this to me and i can polish / wrap it up / move to istio org

[nrjpoddar]

@howardjohn there's a lot of activity on that front. We already have a repo https://github.com/istio/client-go where this will live. Various folks are working on getting all the changes needed in https://github.com/istio/api merged (like deepcopy, annotations, marshalers) so that we can add generated client in the new repo.

[rcernich]

The initial commit has been merged. Feel free to give it a spin: istio.io/client-go

What's missing:

• protobuf serialization (no pb.go files generated for k8s api types)
• openapi specs

k8s type names should match the names of their counterparts, which they wrap. https://github.com/istio/client-go/tree/master/pkg/apis

[nrjpoddar]

@rcernich I think you will have mismatching case issue (camel case vs snake case) as you only have custom marshalers and unmarshalers only for oneof fields like https://github.com/istio/api/blob/master/authentication/v1alpha1/policy_json.gen.go

Instead you need to add these functions for the entire struct that's used for client-go generation like we do here: https://github.com/aspenmesh/istio-client-go/blob/master/pkg/apis/authentication/v1alpha1/policy.go#L60

The reason is the json and protobuf tags differ in the generated go code for any fields which are declared snake case in the proto file. Here's an example:

https://github.com/istio/api/blob/33a483a29b8e654f7542b1939871d899eeb14da9/authentication/v1alpha1/policy.proto#L63 and it's corresponding generated go structs:

https://github.com/istio/api/blob/33a483a29b8e654f7542b1939871d899eeb14da9/authentication/v1alpha1/policy.pb.go#L206

Note this line:

AllowTls bool `protobuf:"varint,1,opt,name=allow_tls,json=allowTls,proto3" json:"allow_tls,omitempty"`

where the json tag is different inside protobuf tag. Thanks @m-eaton for pointing it out.

[rcernich]

@nrjpoddar, we should create a new issue to fix this. That said, to me, this seems like a problem with the protobuf generation. I'm not sure why it's generating different field names. Given your example, I would expect the json to be using allow_tls. Not sure why protobuf json serializer would be configured to use allowTls (iiuc).

[nrjpoddar]

@rcernich I don’t think that’s would work. All the example configuration plus tests use camel case so the only way to fix it is to generate Marshal/Unmarshal for the top level Go struct which can serialize/deserialize it correctly.

I will file an issue for it.

[geeknoid]

@rcernich Should we close this bug and open smaller bugs for remaining items?

[rcernich]

@geeknoid, I think we should. That will make it easier to track, plus the basic work has been completed: a golang based k8s client library exists.

[bappr]

Hey guys,
Trying to use your client-go and have a question about the mode field under spec.servers.*.tls in the gateway structure.
Is there anyway to make it a string to match Istio Doc? Right now it's an integer with

var Server_TLSOptions_TLSmode_name = map[int32]string{
    0: "PASSTHROUGH",
    1: "SIMPLE",
    2: "MUTUAL",
    3: "AUTO_PASSTHROUGH",
    4: "ISTIO_MUTUAL",
}

[rcernich]

Initially, there were some issues with the json serialization in client-go, but I think they should be fixed as of now. @nrjpoddar can you confirm whether or not this is one of the issues you were seeing with json serialization on client-go?

[nrjpoddar]

@bappr since this is a enum, the underlying representation will be int32. You can use the proto generated helper functions to convert it to a string using the String() method defined on the Server_TLSOptions_TLSmode_name type.

[bappr]

@nrjpoddar thanks for your answer.

                gw := istioclientv1alpha3.Gateway{
            ObjectMeta: v1.ObjectMeta{
                Name:   gatewayName,
                Labels: generateLabels(domains),
            },
            Spec: istiov1alpha3.Gateway{
                Selector: map[string]string{
                    "istio": config.IstioIngressGateway,
                },
                Servers: []*istiov1alpha3.Server{
                    {
                        Port: &istiov1alpha3.Port{
                            Number:   443,
                            Protocol: "HTTPS",
                            Name:     "https-" + resourceId,
                        },
                        Tls: &istiov1alpha3.Server_TLSOptions{
                            Mode:              istiov1alpha3.Server_TLSOptions_SIMPLE,
                            ServerCertificate: "sds",
                            PrivateKey:        "sds",
                            CredentialName:    certificateName,
                        },
                        Hosts: domains.Domains,
                    },
                    {
                        Port: &istiov1alpha3.Port{
                            Number:   80,
                            Protocol: "HTTP",
                            Name:     "http-" + resourceId,
                        },
                        Hosts: domains.Domains,
                    },
                },
            },
        }

If I push this gateway in kubernetes, it's gonna create the gateway with Mode: 1.

Due to this https://github.com/knative/serving/issues/5936 which is not an issue on your end, any workaround you have in mind to force Mode to use the proper string instead of an Integer?

[nrjpoddar]

@bappr I don't have an easy workaround for this. Any projects dependent on Istio Go API structs need to be aligned with Istio's way of serializing/de-serializing, so hopefully Knative folks will fix the issue at their end quickly.

[rcernich]

@nrjpoddar, are you implying that the standard k8s d/encode mechanisms don't work with our generated types?

[skaslev]

@rcernich No, currently Knative is using its own apis for istio that don't serialize quite the same way as those from istio/client-go. knative/pkg#882 and knative/serving#6041 migrate Knative to istio/client-go apis and are in review atm.