Skip to content

Add "enabledAPIs" to support selectively deploying package objects#2646

Closed
ulucinar wants to merge 2 commits intocrossplane:masterfrom
ulucinar:fix-2122
Closed

Add "enabledAPIs" to support selectively deploying package objects#2646
ulucinar wants to merge 2 commits intocrossplane:masterfrom
ulucinar:fix-2122

Conversation

@ulucinar
Copy link
Copy Markdown
Contributor

@ulucinar ulucinar commented Oct 20, 2021

Description of your changes

Fixes #2649

This PR proposes a change that introduces the field enabledAPIs for both package and package revisions to support deploying package objects selectively. Each object contained in the package manifest is checked against the regular expressions provided in the enabledAPIs list. If no match is found, package object is not established and skipped. If enabledAPIs is empty, then all objects are established.

Changing the enabledAPIs list results in a new package revision. However, duplicates are eliminated and the order of the regular expressions in the enabledAPIs do not matter. Thus the following specifications are all the same and will not result in a package revision (and corresponds to the same package revision):

  enabledAPIs:
  - virtual.*
  - lb
  enabledAPIs:
  - lb
  - virtual.*
  enabledAPIs:
  - lb
  - lb
  - virtual.*
  - virtual.*

. However, the following enabled APIs declaration belongs to a different package revision although with the current provider-tf-azure it selects the same set of package objects:

  enabledAPIs:
  - lb.*
  - virtual.*

Each item of enabledAPIs is a regular expression that's matched with a string representation of the schema.GVK of each package object. For this purpose, the package objects GVK is formatted as:

<API Group>/<API Version>.<Kind>

For instance, provider-tf-azures v1alpha1.ResourceGroup kind's GVK is formatted as the following and then is matched against every element of enabledAPIs if a non-zero valued enabledAPIs is supplied:

resource.azure.tf.crossplane.io/v1alpha1.ResourceGroup

This allows us to define fine-grained selectors to include/exclude APIs.

Also in this scheme, apiextensions.k8s.io/v1.CustomResourceDefinitions are treated in a special way. If the package object is a CRD, then it's not only the CRD that's matched against items in enabledAPIs but also all versions of the defined kind by that CRD. This allows us, for example, to enable the whole virtual API group in provider-tf-azure with a compact specification such as virtual or virtual.*.

Active package revision is determined by the package image's digest and a stable digest computed from the value of the enabledAPIs field and not from the actual set of package objects selected. I think this is a good enough approximation for practical purposes and also reflects the usual behavior of similar APIs.

This PR also changes the package revision name format if enabledAPIs is non-empty. A hash computed from the set of enabled APIs is appended to the package revision name so that the new format is <package name>-<image hash>-<enabled APIs hash>. If enabledAPIs has zero-length, then package revision format is still <package name>-<image hash>. Thus, Crossplane upgrade is not expected to affect existing provider installations unless the new API is explicitly used in the v1.Provider or v1.Configuration object defining the package.

Another important aspect of the changes proposed here is that the provider revision reconciler passes a new command-line option, --enabled-apis, to the provider container (defined in the provider deployment corresponding to the revision) to communicate the enabled APIs, the provider is expected to honor this command-line option and add controllers only for the enabled APIs. It may fail if it does not properly handle the new command-line option, as any CRDs not selected by a non-zero valued enabledAPIs field will not have been registered via the package revision controller. Or it may fail because of being passed an unknown command-line option. Example manifest for a provider deployment when enabledAPIs has a non-zero value is given in the "How has this code been tested" section. If enabledAPIs has a zero-length, then provider revision controller does not produce the --enabled-apis command-line option thus the behavior is backwards-compatible if the new API is not employed.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Add unit tests
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

There are two additional PRs accompanying this one, in terrajet repo for generating the required code for properly handling the communicated set of enabled APIs and another one in provider-tf-azure that has been generated using the new terrajet version and that introduces the --enabled-apis command-line option to the provider. This PR has been manually tested with these three PRs using the following provider manifest. The package referred in this manifest has been built and pushed to the registry. I can also produce an amd64 version (or maybe one will have already been produced in the CI pipeline of the corresponding change) if requested:

apiVersion: pkg.crossplane.io/v1
kind: Provider
metadata:
  name: crossplane-provider-tf-azure
spec:
  enabledAPIs:
  - Provider.*
  - virtual.*
  - lb
  - resource.azure.tf.crossplane.io/v1alpha1.ResourceGroup
  package: ulucinar/provider-tf-azure-arm64:build-83b9bfc0

Here is the partial status of the corresponding v1.Provider object when the above manifest is applied:

apiVersion: pkg.crossplane.io/v1
kind: Provider
metadata:
  name: crossplane-provider-tf-azure
...
status:
  conditions:
  - reason: HealthyPackageRevision
    status: "True"
    type: Healthy
    ...
  - reason: ActivePackageRevision
    status: "True"
    type: Installed
    ...
  currentAPIs:
  - Provider.*
  - virtual.*
  - lb
  - resource.azure.tf.crossplane.io/v1alpha1.ResourceGroup
  currentIdentifier: ulucinar/provider-tf-azure-arm64:build-83b9bfc0
  currentRevision: crossplane-provider-tf-azure-75613241815f-bc37ddfabb7c

Here is the corresponding (partial) provider revision manifest generated by the package revision controller:

apiVersion: pkg.crossplane.io/v1
kind: ProviderRevision
metadata:
  ...
  labels:
    pkg.crossplane.io/package: crossplane-provider-tf-azure
  name: crossplane-provider-tf-azure-75613241815f-bc37ddfabb7c
  ownerReferences:
  ...
spec:
  desiredState: Active
  enabledAPIs:
  - Provider.*
  - virtual.*
  - lb
  - resource.azure.tf.crossplane.io/v1alpha1.ResourceGroup
  image: ulucinar/provider-tf-azure-arm64:build-83b9bfc0
  ...
status:
  objectRefs:                                                                                                                                                                                                                                   
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualhubconnections.virtual.azure.tf.crossplane.io
    uid: b16c4a9b-f71c-421a-9b7a-5e143fa701bf
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualdesktopapplications.virtual.azure.tf.crossplane.io
    uid: 7ef33b35-9139-4e76-bc03-88ff9f22af6b
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: resourcegrouppolicyassignments.resource.azure.tf.crossplane.io
    uid: 08bf2ffc-1e6b-4324-a571-bcc7d0674113
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualhubs.virtual.azure.tf.crossplane.io
    uid: a0cef493-a876-4ef3-9372-6da631ee17cf
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualdesktopworkspaces.virtual.azure.tf.crossplane.io
    uid: 90f5d120-2926-4ad3-b14e-ad785660b296
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: providerconfigs.azure.tf.crossplane.io
    uid: c883fc76-38d2-4244-86d2-ec57849f1abc
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: providerconfigusages.azure.tf.crossplane.io
    uid: 792dc394-7e1e-4479-bd9c-12448f2592d5
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualnetworkpeerings.virtual.azure.tf.crossplane.io
    uid: dc88f728-fae0-46dd-af1a-a9996f06015a
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: loadbalancers.lb.azure.tf.crossplane.io
    uid: d7b70a98-23aa-4c3e-ac09-71fb2edaf359
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: resourcegroups.resource.azure.tf.crossplane.io
    uid: bd63b829-58e9-4a43-9f34-530ec67accad
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualdesktophostpools.virtual.azure.tf.crossplane.io
    uid: 6f389165-5513-440b-b7b0-4c9b667e8fc6
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualwans.virtual.azure.tf.crossplane.io
    uid: 4cef9530-150d-433a-b628-383d81182a5b
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualnetworkgateways.virtual.azure.tf.crossplane.io
    uid: 070f34a1-a477-4b35-a6f1-530f3b28ada3
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualhubroutetables.virtual.azure.tf.crossplane.io
    uid: 154ccdcd-445a-416b-b8f5-493881cae7cb
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: resourcegrouptemplatedeployments.resource.azure.tf.crossplane.io
    uid: dc616558-7b8d-416a-8c4c-d2bf89cb503f
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualhubsecuritypartnerproviders.virtual.azure.tf.crossplane.io
    uid: 38942ac7-5efd-4f78-bbbf-5db9f523bf4c
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualhubips.virtual.azure.tf.crossplane.io
    uid: eabfabe9-bc24-46dc-a236-f6d87582a212
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualhubbgpconnections.virtual.azure.tf.crossplane.io
    uid: 8404b3b4-4790-44c2-a51b-ef604e1165fd
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualnetworks.virtual.azure.tf.crossplane.io
    uid: 886fca2c-0565-4169-bfa1-ba3407793e36
  - apiVersion: apiextensions.k8s.io/v1
    kind: CustomResourceDefinition
    name: virtualnetworkgatewayconnections.virtual.azure.tf.crossplane.io
    uid: d49d9327-4809-45b9-a6df-84c5419548cf

Please note that CRDs not selected by enabledAPIs are not available in status.objectRefs.

Here is the (partial) provider deployment corresponding to that revision:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: crossplane-provider-tf-azure-75613241815f-bc37ddfabb7c
  ...
spec:
  ...
  template:
    ...
    spec:
      containers:
      - args:
        - --enabled-apis
        - Provider.*,virtual.*,lb,resource.azure.tf.crossplane.io/v1alpha1.ResourceGroup
      ...

- Fixes crossplane#2122

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
@ulucinar ulucinar requested a review from hasheddan October 20, 2021 08:11
@negz
Copy link
Copy Markdown
Member

negz commented Oct 20, 2021

I'd like to understand more about the use cases behind this change before we proceed with it.

I see in #2122 we discuss that it may help scope authz permissions for strategies like IRSA. I'm particularly curious how that would interplay with the ideas put forward in #2411. Would a user create multiple Provider.pkg each running (for example) provider-tf-azure with a different set of enabled API groups?

I also suspect that despite the above use case being what we have written down the more immediate motivation is that we're seeing issues scaling on number of CRDs. If that's really what this is solving for us today I'd like us to have an issue opened against crossplane/crossplane detailing it. I've heard us discuss potentially rate limiting CRD installs (to smooth out OpenAPI processing) and bumping REST mapper client rate limits as other options to alleviate this undocumented problem. Do we think this option is the better solution or is it a temporary workaround?

…ubectl plugin

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
@negz
Copy link
Copy Markdown
Member

negz commented Nov 19, 2021

We decided to hold this functionality for now; per #2649 we're addressing the performance issues we've seen by working with the Kubernetes folks to improve performance there. That said, we may revisit something like this functionality in future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API Server (and clients) becomes unresponsive with too many CRDs

2 participants