api

api copied to clipboard

try to resolve: https://github.com/istio/istio/issues/31630 follow up PR for https://github.com/istio/api/pull/1812

🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?

Courtesy of your friendly test nag.

@howardjohn Let me explain this. We found that Kubernetes types has some inline field, for example Handler field in Probe(https://github.com/kubernetes/api/blob/master/core/v1/types.go#L2098) and VolumeSource field in Volume(https://github.com/kubernetes/api/blob/master/core/v1/types.go#L46) These inline fields will not exist in the serialized objects(yaml/json), but do exist in golang type. If we reuse the Kubernetes types, which means we import Kubernetes' proto in istio's proto and generate the istio's golang types, then the inline metadata will be lost, the marshal/unmarshal will not work as expected, just as what you see in https://github.com/istio/istio/issues/31630

Besides, it seems that we use both jsonpb and json for the marshal/unmarshal, I'm not expert of protobuf, maybe there is better solution for this.

If we reuse the Kubernetes types, which means we import Kubernetes' proto in istio's proto and generate the istio's golang types, then the inline metadata will be lost, the marshal/unmarshal will not work as expected

Why? If we use k8s protos, then we will also use k8s go types, which have the inline tag, right?

I don't really feel great about attempting to mirror their massive API...

@morvencao: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@howardjohn this is partially lost in the mists of time but i think we had to go down this path because we use proto differently from k8s. we generate our structs from proto3 and use those, while k8s start with go structs and generate proto2 from those. this forces us to use different unmarshal logic, which doesn't integrate with the k8s json code. our unmarshal won't end up calling the k8s unmarshal for a given type. but i don't recall everything we tried so maybe it's worth trying this again. if we could eliminate duplicating k8s protos it would be wonderful.

@litong01 fyi

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2021-03-29. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.