🏃mark defaulted fields as optional

[sethp-nr]

Perhaps easier to iterate on than a full kubernetes cluster

What this PR does / why we need it:

Addresses some odd defaulting behavior in the KCP webhook, as well as smoothing out some UX.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1941

[sethp-nr]

I'm not sure omitempty is going to solve our troubles here without changing some of these types to pointers. I'd like to avoid that if possible, especially for the types that already have an IsZero implementation.

What do folks think? More pointers or more json serialization code?

[sethp-nr]

/test pull-cluster-api-capd-e2e

[sethp-nr]

Well, I have to say that teaching encoding/json to omit empty struct fields was both better and worse than I expected.

On one hand, the badness is almost entirely contained inside one or two files as opposed to spread out throughout the codebase.

On the other, I've done a limited survey of the code and it looks like we almost never actually use those fields from go-land, only after serializing to yaml to pass to kubeadm. At that point, the pointers are really only awkward the next time we'd need to copy over the config structures for the next kubeadm config version.

[chuckha]

@sethp-nr this capd-e2e is failing because the kubeadm configuration is now missing kind and apiVersion

[vincepri]

We shouldn't need this I think, rather when we're adding omitempty, we should have a pointer

[sethp-nr]

Ah, ok – I hadn't gotten to the point yet where I was running that locally. In the tests I did it was populating apiVersion but, truth be told, I couldn't figure out how my encoding code was handling embedded structs. Maybe it wasn't after all :)

After sleeping on it, I'm feeling like turning things into pointers is the better way forward anyway.

[sethp-nr]

Chatted with Vince in Slack: https://kubernetes.slack.com/archives/C8TSNPY4T/p1584373580418500

Since changing to pointers is a breaking API change for go clients, I'm moving that work to a different PR. This one is going to just change the // +optional tags for kube API clients.

[vincepri]

@sethp-nr changes lgtm, did you want to keep the IsZero methods? They don't seem to be used anywhere

[sethp-nr]

Oh, no, I just missed a few cleaning things up. will fix.

[vincepri]

/approve
/assign @ncdc
/milestone v0.3.1

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sethp-nr, vincepri

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [vincepri]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ncdc]

/lgtm
thanks!