Return native kube API types instead of a proto conversion

[jessesuen]

In Kargo API, we convert from kubernetes types, to a different type which is returned in our gRPC/Connect server. And so we have conversion code like the following:

func ToPromotionProto(p kargoapi.Promotion) *v1alpha1.Promotion {
	metadata := p.ObjectMeta.DeepCopy()
	metadata.SetManagedFields(nil)

	return &v1alpha1.Promotion{
		ApiVersion: p.APIVersion,
		Kind:       p.Kind,
		Metadata:   typesmetav1.ToObjectMetaProto(*metadata),
		Spec: &v1alpha1.PromotionSpec{
			Stage:   p.Spec.Stage,
			Freight: p.Spec.Freight,
		},
		Status: &v1alpha1.PromotionStatus{
			Phase: string(p.Status.Phase),
			Error: p.Status.Error,
		},
	}
}

This is undesirable for many reasons:

• it must be manually updated
• it is error prone (e.g. we forgot to add apiVersion and kind at one point)
• omitempty semantics are inconsistent from native kubernetes type, and so our API ends up returning fields set to be empty strings as opposed to omitted strings.
• We face limitations such as: bug: "enum" values don't look right in the ui #707

Instead of converting and maintaining two different identical types, we should generate proto files from go-to-protobuf and return the native type from our API.

[devholic]

@jessesuen @krancour We should investigate more into this issue, since the Kubernetes API relies on gogoproto, as you pointed out in #707. This means that issues like #490 may resurface.

I'm somewhat concerned because this is a kind of creating our own dialect like gogoproto, and eventually, we have to do the migration job again to following changes on Kubernetes after they remove gogoproto from their codebase.

[jessesuen]

Kube has moved off of gogoproto. They now maintain:

https://github.com/kubernetes/gengo

which is used in:

https://github.com/kubernetes/code-generator/tree/master/cmd/go-to-protobuf

[devholic]

No, I mean that the current Kubernetes native types (e.g. metav1.Time only works with gogoproto, not plain protobuf

[jessesuen]

Ah. I mistakenly thought Kube moved off of gogoproto, but I see they have not.

Hmmm, I agree we should not re-introduce gogoproto. Unfortunately, I'm at a loss on how to deal with the inconsistency that
is currently caused by redefining our types in our API. For example the omitempty inconsistency and the enum problem:

apiVersion: kargo.akuity.io/v1alpha1
kind: Stage
metadata:
  name: staging
  # native type would not return the following fields because omitempty works
  # properly, but these fields *are* returned in our API
  generateName: ""
  labels: {}
  ownerReferences: []
  finalizers: []

[jessesuen]

@devholic does EmitUnpopulated do what we expect? I see this in UI:

But this from kubectl:

  health:
    argoCDApps:
    - healthStatus:
        status: Healthy
      name: guestbook-prod-euw1
      namespace: argocd
      syncStatus:
        revision: d3083b73a25f1f606118d4ba40ef5c4ffd277c5e
        status: Synced
    status: Healthy

[devholic]

@jessesuen I think it's a problem of dashboard because API seems omit empty fields 🤔

#738 may fix the issue

[devholic]

@jessesuen Seems the issue has been resolved after #738 merged, could you check?

[krancour]

#738 treated the symptom rather than the cause.

It is still onerous to have to do type conversions for sending things over the wire.

I think this issue is asking that we send v1alpha1 resources directly over the wire without conversion.

[jessesuen]

Although we now omit empty strings and empty arrays in UI, we still have this problem that tripped Kent on:

qualified: false is actually a boolean omit empty. It's omitted from kubectl, but not from API or UI.

omitempty for boolean values seems impossible to solve using the same techniques we just used for empty arrays or empty strings.