Loose classRef matching for resource claims

[suskin]

What seems to be the problem?

Hello friends! This is related to #703 .

I am working on making a stack that creates portable wordpress workloads. I'll refer to the set of resources created by the stack as the resource bundle or bundle. The current combination of required strongly typed classRefs for claims and namespace-specific defaults is making things a bit trickier than they were before. I'll explain.

The goal is to create a stack which will create a portable resource bundle for the user. I'll define a portable resource bundle as a resource bundle which is decoupled from the details of cloud provider configuration, and which can be used with multiple different cloud provider configurations (potentially one configuration at a time) without modification.

The old way

Before, I was using classRefs and no defaults, and was able to create a resource bundle for a portable workload. To get a workload running, here are what the steps were, roughly:

1. Create a ResourceClass for the dependencies (MySQL and Kubernetes)
2. In the workload, use the names and namespaces for the resource classes in the classRef field.
3. Create all the resources for the resource bundle
4. For portability, keep the bundle the same, but change or create the resource classes to use a different cloud provider

The new way

Here is what it is now:

1. Create strongly typed resource classes for the dependencies
2. In the workload, use strongly typed references in the classRef field, which includes the cloud provider for each of the dependencies
3. Create all of the resources for the resource bundle
4. For portability, create resource classes which use different cloud providers for the dependencies
5. For portability, change the workload to use the new resource classes (because the kind will be different and needs to be updated in the workload)

Now that the workload needs to be edited if the cloud provider changes, I would consider the workload to no longer be portable.

So, what does a portable resource bundle look like today? Here are the rough steps:

1. Create strongly types resource classes for the dependencies
2. Create the namespace N that the bundle's resources will be in
3. Create default resource policies, using namespace N
4. Create the bundle's resources (in namespace N so they bind with the default resource policies)
5. For portability, use different default resource policies, either in namespace N or another namespace

This works, but is also not ideal, because:

• The namespace needs to be created before the default is created.
• Only one default can be active at a time.

If only one default can be active at a time, the default policies should not be part of a portable resource bundle. If the default policies are not part of the resource bundle, the namespace must be created outside of the resource bundle too. This doesn't seem as nice as how it was before, where the bundle could create its own namespace as needed and the user didn't need to worry about creating the namespace ahead of time.

How can Crossplane help?

The proposal would be that we support loosely defined classRefs, which
only use the namespace and name to match (similar to what was happening
before strongly-typed resource classes). Benefits include:

• We can support portable resource bundles without requiring that the
  namespace is created outside of the resource bundle
• We can support multiple different portable configurations for the same
  claim type; for example, a resource bundle could depend on a MySQL
  instance in cloud provider A, and also depend on a MySQL instance in
  cloud provider B. It is not possible to do this in a portable way
  currently.

Here is an example of what a claim could look like in the proposed form:

apiVersion: database.crossplane.io/v1alpha1
kind: MySQLInstance
metadata:
  name: wordpress-mysql-instance
  namespace: wordpresses
  labels:
    stack: sample-stack-wordpress
spec:
  classRef:
    name: standard-mysql
    namespace: crossplane-system
  engineVersion: "5.7"
  writeConnectionSecretToRef:
    name: sql-secret

[negz]

I appreciate that omitting the type metadata (i.e. kind and apiVersion) from the classRef provides a potentially better user experience, but I suspect it is technically impossible with strongly typed resource classes.

A namespace and name alone are insufficient to find a resource in the Kubernetes API. Put otherwise, a namespaced resource is uniquely identified by the set of its apiVersion, kind, namespace, and name.

Previously all resource claim kinds used a single resource class kind; ResourceClass. Resource claims could omit the type metadata from their classRef because we effectively hard coded that type metadata in our resource claim controller code. Now, for example, a MySQLInstance resource claim could use one of several resource kinds, including CloudSQLInstanceClass, RDSInstanceClass, etc. Given that there could be a CloudSQLInstanceClass named mycoolclass in namespace ns and also an RDSInstanceClass named mycoolclass in namespace ns we need the type metadata to know which we should use.

Crossplane currently instantiates a resource claim controller for every possible (claim kind, class kind) tuple. Each of these controllers watches all claims of the kind they're concerned with, combined with a predicate that filters out any claims whose classRef is of a kind they're unconcerned with.

It was suggested that we might remove the aforementioned predicate and let all (claim kind, class kind) controllers attempt to process all claims. Unfortunately, in the example above in which two resource classes both named mycoolclass existed in the same namespace, a race would occur between resource claim controllers to satsify any claim trying to use mycoolclass. We could work around this by requiring that no two resource class kinds have the same namespace and name by writing an admission webhook that first checked whether a resource class was uniquely named across all kinds before allowing it to be created. This would be a highly unusual constraint upon the Kubernetes API.

My suspicion here is that the best path forward is to improve the functionality of our nascent Policy API and controllers rather than reverting to type-less class references.

[prasek]

Would be great for Crossplane to continue enabling Admins to provide a self-service list of available classes-of-service (mysql-standard, mysql-HA) and for App Owners to pick from that list in a portable way, including portable claim kinds that can be satisfied by GCP, AWS, or Azure.

Previously this was supported with the singular ResourceClass CRD such that classes-of service were provided by Admins creating resource classes, and App Owners would consume them via claim.spec.classRef.name. App Owners were free to pick whatever class-of-service they wanted and the claims were portable; no classRef.kind or classRef.version required.

With the introduction of strongly-typed resource classes, App Owners creating claims with a specific class-of-service using classRef.name are now also required to use classRef.kind and classRef.version, making these claims non-portable across providers, that can only be satisfied by the claim.spec.classRef.kind.

Claim Policies were introduced to allow omitting the claim.spec.classRef (gaining portability) and using an Administrator defined default class-of-service. However the App Owner has to rely on Administrator defined defaults and doesn’t get to pick (self-service) from one-of-many available classes-of-service for use in their claims. Even if an App Owner creates the Claim Policies, they are defaultResourceClass.kind specific and not portable.

The App Owner shouldn’t be forced to choose between claim portability and self-service selection of different classes-of-service.

It may be possible to preserve claim portability and App Owner self-service for multiple classes-of-service, with a couple adjustments to Claim Policies and adding support for a portable claim.spec.policyRef. The Admin could populate a namespace with one Claim Policy per class-of-service (mysql-standard, mysql-ha). Claims could then consume an available class-of-service using claim.spec.policyRef.name and claim.spec.policyRef.kind would be portable.

apiVersion: database.crossplane.io/v1alpha1
kind: MySQLInstance
metadata:
  name: mysqldb-billing
  namespace: my-project
spec:
  policyRef:
    kind:MySQLInstancePolicy
    apiVersion: database.crossplane.io/v1alpha1 
    name: mysql-ha
    namespace: my-project
  engineVersion: "5.6"

A Claim Policy per class-of-service (mysql-ha, mysql-standard) could be created in the App Owners namespace, making those classes-of-service available for self-service provisioning

apiVersion: database.crossplane.io/v1alpha1
kind: MySQLInstancePolicy
metadata:
  name: mysql-ha
  namespace: my-project
classRef:
  kind: CloudsqlInstanceClass
  apiVersion: database.gcp.crossplane.io/v1alpha1
  name: cloudsqlinstancemysql-ha
  namespace: crossplane-system

apiVersion: database.crossplane.io/v1alpha1
kind: MySQLInstancePolicy
metadata:
  name: mysql-standard
  namespace: my-project
classRef:
  kind: CloudsqlInstanceClass
  apiVersion: database.gcp.crossplane.io/v1alpha1
  name: cloudsqlinstancemysql-standard
  namespace: crossplane-system

Claim Policies could use the default label originally proposed in the default-resource-classes.md that more closely mirrors the Kubernetes PVC default storage class.

labels:
    mysqlinstancepolicy.database.crossplane.io/default: "true"

//cc @hasheddan @jbw976

[hasheddan]

@prasek this is a very interesting proposition, and I definitely think an improvement of the Policy API is the way the we will solve this problem. Is the default label you mentioned at the end intended to serve as indicating the default policy to use for the claim if the policyRef field is omitted? If so, we could simplify it a bit as:

labels:
    default: "true"

due to the fact that the claim kind for which it is default is denoted by the policy kind being MySQLInstancePolicy. Really appreciate your insight on this issue!

[prasek]

After re-reading the vision doc, the main issue is that claims are currently defined as portable and permit selection of a class of service in a portable way. The move to strongly typed classes make the claim.classRef not portable, so suggest removing claim.classRef and replacing it with claim.policyRef so v0.3 claims are (a) portable and (b) allow the app owner to pick the class of service in a portable way via claim.policyRef.name.

Also definitely think non-portable claims make sense at some point, but they’d likely share the resource class schema and support patch/override/overlay for config settings, and a way for admins to define what settings can be overridden/patched, allowed values, etc.