Add support for generating defaults for external types

[detiber]

Adds support for generating defaults for external types, similar to what conversion-gen does.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: detiber
To complete the pull request process, please assign smarterclayton
You can assign the PR to them by writing /assign @smarterclayton in a comment when ready.

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

Details

Needs approval from an approver in each of these files:

• examples/defaulter-gen/OWNERS

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

[detiber]

@sttts I ran into this issue when trying to add Namespaces support to my minimal apiserver implementation, I used the changes here to generate the defaults used here: https://github.com/thetirefire/badidea/pull/12/files#diff-a80ecef7b7a6df7985c380dd10060f26

[sttts]

Why would you want defaults for external types?

[detiber]

@sttts In the current case I'm trying to re-implement support for namespaces in thetirefire/badidea#12, but we've also hit similar types of issues with Cluster API when trying to expose kubeadm config types.

Since the defaults (and validations) apply to the public types, it makes sense to either be able to import and/or generate them for various reasons in external projects.

[sttts]

I am not convinced this is needed.

Please add an example (as simple as possible) to the _output_tests directory.

/hold

[lavalamp]

Whoops, sorry, missed this until now-- @apelisse and @sttts have been working on defaults.

[detiber]

@lavalamp I'm not sure the defaulting changes (at least the ones in https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/1929-built-in-default and the related changes in https://github.com/kubernetes/enhancements/pull/2082/files) would resolve the issue.

Basically, since the defaulting logic is not defined as part of k8s.io/api, if I'm trying to re-use those types fully (not embedding within another type or CRD), there is no current way for me to use the same defaulting as k/k (without also importing k/k).

Currently the generated defaults in k/k are generated using hardcoded paths to the external types defined in the staging repository. While I suppose it would be possible to also introduce a similar staging directory and generate the defaulting logic the same way it is done in k/k, however that also comes with all the same downsides.

By allowing defaulter-gen to reference external types, it would also benefit a potential future k/k that leverages go modules without a staging directory.

Alternatively, I would love it if the defaulting logic was in k8s.io/api, since that would also allow for easier use in other places (such as client side code), but that does present more complications for updating or changing defaults.

[sttts]

@detiber you have k/api vendored. Why not just reference the vendored copy?

By allowing defaulter-gen to reference external types, it would also benefit a potential future k/k that leverages go modules without a staging directory.

I don't follow here. We do exactly that:

k/pkg/apis/apps/v1/doc.go:

// +k8s:defaulter-gen-input=../../../../vendor/k8s.io/api/apps/v1

It references external types. What is different in your use-case (other than that ../../../../vendor/k8s.io/api is a real directory and not a symlink) ?

[sttts]

Why would you want defaults for external types?

To clarify: defaulting is always operating on external types. But defaulting is something we don't want non-apiservers to consume. Hence, we have it in k/pkg/apis/<group>/<version>. The "real" external types package is at an external place already today (via +k8s:defaulter-gen-input`).

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[k8s-ci-robot]

@detiber: PR needs rebase.

Details

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.

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

[k8s-ci-robot]

@fejta-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

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.