Swagger client API calls omit fields whose values have meaning, if the values happen to be Go zero values

[dlipovetsky]

Describe the bug

The generated swagger clients [1,2] annotate all fields with omitempty. When a field with this annotation has a value equal to the type's zero value, Go considers it empty, and omits from the marshaled output.

Some fields have a zero value that carries meaning. For example, the GracefulTimeoutPeriod has a zero value of 0. This value has meaning: it means the timeout should be disabled. In fact, the SDK client (in pkg/vcdsdk) disables this timeout. However, this has no effect: the swagger client removes the field from the API request, and so VCD service assigns the default timeout (a value of 1).

This problem is well-known by the Kubernetes community; it affected the core Kubernetes APIs. It is a topic of the Kubernetes API Conventions.

[1] https://github.com/vmware/cloud-provider-for-cloud-director/tree/868f15c9090e5b7799782047759cd0b5d069f4c7/pkg/vcdswaggerclient_36_0
[2] https://github.com/vmware/cloud-provider-for-cloud-director/tree/868f15c9090e5b7799782047759cd0b5d069f4c7/pkg/vcdswaggerclient_37_2

Reproduction steps

I'll try to create a failing unit test.

Expected behavior

The swagger client must not omit fields with values that have meaning, when the values happen to be Go zero values.

Additional context

For a quick demonstration of how omitempty works when marshaling, see https://go.dev/play/p/CAOw2aCY3Gk

[arunmk]

@dlipovetsky thanks for this report. This is resolved by #311 (458dc1c).

There will be a further merge of a larger change that removes omitempty fields across the swagger API. That will come in after the branch for this change is cut.

[dlipovetsky]

Thanks for the fix, @arunmk

I noticed the fix is for this GracefulTimeoutPeriod field only. Were you able to confirm that other fields are not affected?

[arunmk]

Thanks for the fix, @arunmk

I noticed the fix is for this GracefulTimeoutPeriod field only. Were you able to confirm that other fields are not affected?

I will create another PR for it once we release the .z version of this branch. We are at a small freeze now for this release. We wanted to minimize risk for this .z release. The PR should land by the end of next week.