crossplane / crossplane

The Cloud Native Control Plane
https://crossplane.io
Apache License 2.0
9.14k stars 915 forks source link

Deprecate "native" patch and transform Composition #4746

Open negz opened 9 months ago

negz commented 9 months ago

What problem are you facing?

[!IMPORTANT] This is long-term thinking. πŸ™‚ We won't be deprecating anything until there's a clear and straightforward migration path.

With Crossplane v1.14 we'll be promoting Composition Functions to beta. Composition Functions are an alternative to "native" patch and transform (P&T Composition, where you specify spec.resources (and spec.patchSets) directly in your Composition. Functions are mutually exclusive with native P&T. You can either use Functions, or use native P&T. You can read more about the design of Functions here.

Early feedback around Functions has been very positive (see https://github.com/crossplane/crossplane/discussions/4639). The architecture has a number of benefits, including:

To that end, I think we should be encouraging everyone to use Composition Functions.

How could Crossplane help solve your problem?

I think we should mark the native P&T Composition fields - i.e. spec.resources and spec.patchSets as deprecated. Eventually (in a v2 of the Composition API) we would remove them. Until that time we would freeze the native P&T API - bug fixes only, no new features.

For everyone who today is using P&T, we have https://github.com/crossplane-contrib/function-patch-and-transform (see also https://github.com/crossplane/crossplane/issues/4571). Any deprecation warning would recommend folks migrate to this Function. We at @upbound are planning to make function-patch-and-transform an "Official Function" that we'll commit to maintaining, just like we have done with native P&T. So far we've found that migration from native P&T to function-patch-and-transform has been fairly straightforward, and has a number of benefits.

negz commented 9 months ago

Note that I don't think we'd make this change super soon. At the very least, we'd need to have Composition Functions promoted to GA and well-trusted within the community before we could mark native P&T deprecated.

My motivation for raising this issue now is to help illustrate my thinking on a number of related designs in Crossplane (the Composition environment, generic references, etc). If we take a slightly longer term view of the project, I think we should be optimizing for Functions, and not for making new things possible in native P&T.

negz commented 9 months ago

Another note on migration - folks (including I think @bobh66 and @haarchri) have noted that it could well be possible to transparently migrate existing Compositions to use function-patch-and-transform using a mutating admission webhook.

bobh66 commented 9 months ago

How does a mutating webhook work with ArgoCD reconciliation? We have our Compositions/XRDs reconciling from git rather than using packages, and I suspect that if the webhook mutates the Composition then ArgoCD will see that as out of sync and continually try to reconcile it.

stevendborrelli commented 9 months ago

After using P&T as a function, it makes sense to deprecate it in core. The function can be integrated with development tooling for a better experience. Advanced users can develop their own forks if needed.

TheControllerConfig deprecation caused a lot of confusion, I would like the community to have more time to process the change and be provided tooling.

As for migrating to Functions, my experience porting one of the reference platforms (see https://github.com/upbound/platform-ref-aws/pull/115) has been fairly straightforward.

  1. Set up the function in the Composition
    1. Enable mode: Pipeline
    2. Create the pipeline step for patch-and-transform
  2. Indent existing resources & patchsets under Input
  3. Update transforms to have their type specified
  4. Update ConnectionSecretKeys to have type and name set
hwinkel commented 9 months ago

In this context, what about a HCL (terraform) runtime for functions ? ;-)

negz commented 8 months ago

In this context, what about a HCL (terraform) runtime for functions ? ;-)

Sure, feel free to contribute one. πŸ™‚

negz commented 8 months ago

Capturing an idea from @bobh66, which could be a follow on after deprecating native P&T: we could potentially implement native P&T using function-patch-and-transform (or a similar function that exactly matched the legacy API and behavior). I like that idea because it would let us remove a whole lot of code from core, which would probably make maintenance easier.

Piotr1215 commented 8 months ago

@stevendborrelli the steps are indeed easy, and I think it would be very useful to have the below steps as a simple command.

  1. Set up the function in the Composition
  2. Enable mode: Pipeline
  3. Create the pipeline step for patch-and-transform
  4. Indent existing resources & patchsets under Input
  5. Update transforms to have their type specified
  6. Update ConnectionSecretKeys to have type and name set

imaginary-cli convert-to-function <existing composition.yaml> <new-functions-based.composition.yaml>

stevendborrelli commented 8 months ago

@Piotr1215 we do have a utility to convert existing Compositions to use pipelines at https://github.com/crossplane-contrib/crossplane-migrator, and planning to include it in the 1.15 crossplane CLI:

crossplane-migrator
Usage: crossplane-migrator <command>

Crossplane migration utilities

Commands:
  new-deployment-runtime      Convert deprecated ControllerConfigs to
                              DeploymentRuntimeConfigs.
  new-pipeline-composition    Convert Compositions to Composition Pipelines with
                              function-patch-and-transform.

Flags:
  -h, --help    Show context-sensitive help.

Run "crossplane-migrator <command> --help" for more information on a command.
MisterMX commented 8 months ago

When I look at this from an enterprise point of view I am very concerned about the implications that this would have. Considering that companies spent the last years building their whole infrastructure around Crossplane using compositions it would require them a lot of effort to migrate everything to a new technology (which functions kind of are). Since companies would run Crossplane as the core of their infrastructure platform and they do run this in production it would require them another lot of effort to perform this migration. Even if there is some form of generator that takes over some of the burden it still requires a lot of testing for it. Considering that some of these companies might even run critical infrastructure, it makes it even more difficult and complex.

While I fully understand the willingness and business decision behind it, I see the need for a solution that allows people to use compositions in the long-term.

If composition P&T are actually removed from Crossplane at some point in the future I would wish to see the respective controllers moved to another repo, i.e. https://github.com/crossplane-contrib/composition-operator where it can be maintained by the community to ensure support is given as long as needed.

phisco commented 7 months ago

Relevant to the discussion here: https://github.com/crossplane/crossplane/pull/5061

negz commented 6 months ago

When I look at this from an enterprise point of view I am very concerned about the implications that this would have.

I don't think there's a world where we can literally remove the P&T APIs from the v1 API, so I don't anticipate breaking anyone who is currently using native P&T.

Options that are on the table include:

negz commented 6 months ago

I've added this to the v1.17 milestone, due this August.

My goal is to have composition functions become GA in v1.16, per https://github.com/crossplane/crossplane/issues/4951. When functions are GA we'll update https://docs.crossplane.io to recommend using functions rather than native P&T.

At a minimum I expect this means we'll mark native P&T (i.e. the "resources" composition mode) as deprecated. You may see a deprecation warning logged when it's used. The functionality won't go away, but will be frozen for new features and will be security fix only. The intention of the deprecation will be to push folks toward adopting functions.

github-actions[bot] commented 3 months ago

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.

haarchri commented 3 months ago

/fresh