Proposal: Stop installing Function input CRD(s)

[negz]

What problem are you facing?

Some function packages include CRDs in their xpkg artifact. If a function package includes a CRD, the package manager will install it. This was an unintentional implementation oversight. Functions were never supposed to use actual CRs, and the package manager was never supposed to install their CRDs.

Functions take an input, which looks like a Kubernetes CR written inline of a Composition. In practice this is an x-kubernetes-embedded-resource field, i.e. a schemaless resource, within the Composition. Most Go functions that want to take an input define an input Go type the same way they'd define a CRD - using controller-tools. They then deserialize the input from the RunFunctionRequest into the Go type. It's this use of controller-tools that lead to accidentally creating a CRD. Functions written in languages other than Go (e.g. Python) are very unlikely to include a CRD, since they never use controller-tools.

At least one function that I'm aware of actually uses its CR - i.e. the user is expected to create one, which the function then loads. This is https://www.hyrumslaw.com in action.

There are two known issues with installing these CRDs:

1. https://github.com/crossplane/crossplane/pull/6935 proposes we support running multiple different versions of one function simultaneously within a single control plane.
2. The RBAC manager doesn't handle them (like it does provider CRDs), so nothing has permission to actually use them. https://github.com/crossplane/crossplane/pull/6841 would address this.

I think doing 1 is important, and if we keep installing function CRDs it'll place awkward and unintuitive constraints on functions. They'd never be able to change their CRD - even typically backward compatible changes wouldn't be possible. We'd need guarantee that all running versions of the function will always understand any extant CR.

How could Crossplane help solve your problem?

I think Crossplane should stop installing function CRDs. We could perhaps do this as part of the feature flag that enables https://github.com/crossplane/crossplane/pull/6935.

The only viable alternative I can think of is to go with what I mentioned above - functions can install CRDs but they can never change them, at all. If you're running four different versions of the same function, Crossplane installs the CRD for whatever version you installed last.

(I reused an old issue for this proposal. Original issue below.)

---

What happened?

A Function package may optionally define a schema for its input. We use a CRD to capture the input schema, since it's a KRM-like object. This is mostly useful for documentation purposes, since many tools can extract and parse the schema from a CRD. e.g.:

https://marketplace.upbound.io/functions/crossplane-contrib/function-patch-and-transform/v0.2.1/resources/pt.fn.crossplane.io/Resources/v1beta1

Nothing in a control plane actually uses this CRD, so there's not currently any reason to install it. In fact I suspect our documentation says it doesn't get installed. It turns out it does.

How can we reproduce it?

Install a Function that has an input CRD and kubectl get crds.

What environment did it happen in?

Crossplane version:

[negz]

It's worth noting that we might actually want this behaviour in future. For example:

• If we wanted a webhook to validate the input fields of a Composition.
• If we wanted to allow providing function input using a distinct CR (not inline of a Composition).

[bobh66]

I believe this issue causes Functions to fail to install when they define the same input CRD name (like from the template):

  Warning  SyncPackage  <invalid> (x7 over <invalid>)  packages/functionrevision.pkg.crossplane.io  cannot establish control of object: inputs.template.fn.crossplane.io is already controlled by FunctionRevision function-foo-e1e5dc7b97ec (UID 52831e24-07b9-4c17-83de-0b09485a9e7b)

[negz]

FWIW I'm on the fence about changing this due to https://github.com/crossplane/crossplane/issues/5294#issuecomment-1912758609.

On the other hand, it's causing folks to run into collisions that they wouldn't need to otherwise (e.g. due to a forked function that doesn't change its input API version or kind).

[github-actions[bot]]

Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

[negz]

/fresh

[github-actions[bot]]

Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

[bobh66]

/fresh

[phisco]

@negz I'd vote for dropping the CRDs; they have already caused enough headaches. Regarding the two reasons to possibly keep them that you mentioned above:

• If we wanted a webhook to validate the input fields of a Composition.

We could just cache their schemas locally when we pull the images. Sure, it would be awesome to be able to offload that to the API server, but I don't think we'd want to hammer it even more just to get inputs validated. Moreover, for more complex validation, we'd probably have to make functions expose a validation endpoint for Crossplane to call while validating compositions, so we could just offload schema validation to that directly.

• If we wanted to allow providing function input using a distinct CR (not inline of a Composition).

I was expecting reusable inputs to be a popular request too, at least reading from ConfigMaps/Secrets, but as far as I can tell, that's not the case; mounting them using DRCs as function-go-templating or function-kcl is enough. So I guess we can live without it. We could just add native ConfigMap/Secret support down the line (it could already be implemented by functions using extra resources).

[github-actions[bot]]

Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

[phisco]

I agree this is the way to go. Installing CRDs was an oversight from the beginning. Should we have a deadline to take this decision, to avoid forgetting about this?