feat: add posibility for filtering applied crds when installing provider#3987
feat: add posibility for filtering applied crds when installing provider#3987akesser wants to merge 1 commit intocrossplane:masterfrom
Conversation
Signed-off-by: André Kesser <andre.kesser@dkb.de>
|
Cross-linking #2646, which was a similar POC to this.
Unfortunately I don't think this POC is very useful unless it does cover those problems. A large part of the problem with filtering is that while folks believe on the surface that it's a simple change to make, the devil is very much in the details. Any filtering implementation will also need to consider:
|
|
I tend to agree with Nic here around upgrading and updates to configuration. It is something we need to think about carefully and can get hairy very quickly. I think breaking up providers would be easier in that regard because it will likely be one-time significant change that users have to make. I still worry about potential impact to perception of Crossplane though. |
To solve this you can simply add the following to the documentation of crossplane and the in a similar form to the documentation of the provider: Some bigger providers allow to reduce the number of applied crd when installing them. Look at the documentation of the provider you want to install if supports this feature. To install only the subset of crds you need, filter them using the two properties
The answer is simple: The provider is not able to start because the crds are not applied. the Errormessages you get will explain this quite good. This behavior is not different from other misconfigurations. It is the same if you miss to install one ore many of splitter providers to need to manage a resource, it will just not work because you misconfigured it. To help a user to identify crds he can safely delete, because crossplane is no longer managing them due to to an updated provider yaml, the cli tool could be updated to apply the filter to the currently available crds and list them, perhaps by also give a warning if there are resources for the now no longer managed resources.
Implementing filtering would be mean no change for users, the installed crossplane instance would work like before, only possibilities for the users are added. |
I'm sorry, but "the most popular providers breaks by default unless you've read the documentation and know to supply optional configuration" seems like a really bad user experience to me. I know there are downsides to all options being considered here but this just seems like a non-starter level of bad UX to me.
Can you show me what these error messages look like today? I'm pretty sure they're going to be something pretty obscure and non-obvious like: |
Even though filtering would be a more powerful and granular approach, I also support @negz perspective here that essentially "broken by default" is not a user experience we want to enable. That would unfortunately not be taking a step forward experience wise for a large portion of adopters IMHO 😿 I do of course though appreciate the initiative here to share your thoughts around the implementation and engaging with us in discussion, thank you for taking the time to do that! 🤩 🙇♂️ |
|
Filtering should not be any precondition to configure and be an optional step. e.g. i would suggest to split providers when they reach a specific size or provide other best practices in the provider development hints |
@dr460neye please refer to #3939, where I discuss the constraints any filtering design would have to operate under at some length. I don't think anyone is suggesting filtering be required. It must be optional. Making it required would be a breaking API change. One of the biggest challenges is what to do when a filter is not specified. "Just install all the CRDs" is not on the table as a solution. The default behaviour of our software cannot be breaking clusters due to installing too many CRDs. |
This statement is simply not true, and I don't know where you have taken it from. Without the attributes for filtering simply all CRDs would be applied leading to the current behavior and state crossplane has now, that I definitely don't describe as broken. Applying no filtering to providers that support filtering would in my opinion speed up the setup of a provider for a test system, but should be considered when moving toward more mature usage of crossplane, and the documentation should reflect this. The "Getting Started" section should never be the only documentation read when running a software in production. If the this POC is used with a provider that does not support filtering via the used environment variable, the error message shown in the provider is the default error message, kubernetes prints if a CRD is unknown when registering a reconciler and contains all the information to inform the user that a CRD is missing the provider needs. The problem of filtering out CRDs for providers that do not support this can easily circumvented, if the Provider yaml that is shipped alongside the CRDS of the provider inside the gz file that again is downloaded when installing a provider. A default annotation could be used to signal crossplane that the provider supports filtering, something like So "breaking by default" is clearly not part of this POC. |
Today some very popular and widely used providers like We know that installing We've also been hearing this from a lot of folks at Kubecon. "I installed Crossplane, I installed a provider, it degraded my cluster, so I decided Crossplane wasn't for me." Under this proposal, all of the above would be true unless you configured a filter before you ever installed your provider. This is what I mean by "broken by default". |
|
I'm going to close this for now since I don't think it's being actively worked on. We're open to this feature being added if we can land on a design that satisfies the constraints we've identified (it's likely that may be easier once we don't have to worry about any one provider installing too many CRDs by default). |
Description of your changes
As a prove of concept for filtering crds when installing providers we updated Provider spec to include two new array fields
includeCrdsandexcludeCrdsthat can contain regex values:This poc avoids the proposal of breaking larges providers by service/group/type and shows the possibility to filter applied crds.
When applying a provider with these optional fields set, the list of applied crd gets filtered. If a crd matches a value in excludeCrds, it is ignored, unless it matches a value in includeCrds. Crds that are not matched by values in excludeCrds get applied. If excludeCrds is empty, all crds will be applied.
The deployment for the provider is created with an additional env variable that is used by the provider to filter the activated controllers.
This is described and can be seen in the PR in crosplane-contrib/provider-aws: crossplane-contrib/provider-aws#1727
The poc does not cover the following:
#3939 #3754
I have:
make reviewableto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR if necessary.How has this code been tested