Crossplane Composition Functions IO

fnio.yaml

apiVersion: apiextensions.crossplane.io/v1alpha1
kind: FunctionIO
# Optional arbitrary KRM-like resource. Each function in a pipeline will be
# called with its own function config (i.e. config is not pipelined).
config:
  apiVersion: database.example.org/v1alpha1
  kind: Config
  metadata:
    name: cloudsql
  spec:
    version: POSTGRES_9_6
# The composite resource (including metadata, spec, and status)
composite:
  apiVersion: database.example.org/v1alpha1
  kind: XPostgreSQLInstance
  metadata:
    name: my-db
  spec:
    parameters:
      storageGB: 20
    compositionSelector:
      matchLabels:
        provider: gcp
    writeConnectionSecretToRef:
      name: db-conn
# TODO(negz): What if we just took the resources array of a Composition,
# including patches etc? Is there value in that? Code could just ignore patches
# and return fully formed bases if it wanted to. I like that this would make
# the data structure identical to a Composition resources array, but is there
# any other value? Would it be limiting to try to keep them identical? (Probably yes.)
resources:
- name: cloudsqlinstance
  base:
    apiVersion: database.gcp.crossplane.io/v1beta1
    kind: CloudSQLInstance
    metadata:
      name: cloudsqlpostgresql
    spec:
      forProvider:
        databaseVersion: POSTGRES_9_6
        region: us-central1
        settings:
          tier: db-custom-1-3840
          dataDiskType: PD_SSD
          dataDiskSizeGb: 20
      writeConnectionSecretToRef:
        namespace: crossplane-system
        name: cloudsqlpostgresql-conn
  connectionDetails:
  - name: hostname
    fromConnectionSecretKey: hostname
  # TODO(negz): Do we want readiness checks too? Should the function be able to
  # write to the status of the XR directly to set a status?
results:
- severity: Error
  message: "Could not render Database.postgresql.crossplane.io/v1alpha1`

TODO(negz): Do we want readiness checks too? Should the function be able to write to the status of the XR directly to set a status?

My understanding is, a function will render and output the desired state similar to the existing patching mechanism. So, I don't think a function should be able to write the status and we would probably want the readiness checks too.

Relevant to the above, in the previous model (with ResourceList), I could imagine how I can build a helm chart (e.g. for a render-helm function) that composes resources based on an input ResourceList (except connection details though :) ). With FunctionIO however, I would need additional mechanism to set fields other than the manifest, namely connectionDetails and readinessChecks. I mean, how I would represent those fields in my helm chart for a given resource? Leveraging annotations could be a way, but then, may be we can also do that directly in ResourceList?

apiVersion: fn.apiextensions.crossplane.io/v1alpha1
kind: ResourceList
functionConfig:
  apiVersion: database.example.org/v1alpha1
  kind: Config
  metadata:
    name: cloudsql
  spec:
    version: POSTGRES_9_6
items:
- apiVersion: database.example.org/v1alpha1
  kind: XPostgreSQLInstance
  metadata:
    name: my-db
    annotations:
      fn.apiextensions.crossplane.io/type: "CompositeResource"
  spec:
    parameters:
      storageGB: 20
    compositionSelector:
      matchLabels:
        provider: gcp
    writeConnectionSecretToRef:
      name: db-conn
- apiVersion: database.gcp.crossplane.io/v1beta1
  kind: CloudSQLInstance
  metadata:
    name: cloudsqlpostgresql
    annotations:
++    fn.apiextensions.crossplane.io/connection-details: '[{"name": "hostname", "fromConnectionSecretKey": "hostname"}]'
      fn.apiextensions.crossplane.io/type: "ComposedResource"
      fn.apiextensions.crossplane.io/name: cloudsqlinstance
  spec:
    forProvider:
      databaseVersion: POSTGRES_9_6
      region: us-central1
      settings:
        tier: db-custom-1-3840
        dataDiskType: PD_SSD
        dataDiskSizeGb: 20
    writeConnectionSecretToRef:
      namespace: crossplane-system
      name: cloudsqlpostgresql-conn

My understanding is, a function will render and output the desired state similar to the existing patching mechanism. So, I don't think a function should be able to write the status and we would probably want the readiness checks too.

It's common practice to patch from a composed resource to the XR's status, so I think we need to allow functions to update the XRs status. This makes sense to me, since functions are part of the logic used to reconcile the XR. That said, I'm leaning toward supporting readiness checks regardless. This would allow function authors to avoid implementing their own readiness checks unless they had a special case.

I agree that functions shouldn't be able to write to composed resource status.

I mean, how I would represent those fields in my helm chart for a given resource?

I see what you're saying. Would it be an option to require that the Helm charts used for Composition render (i.e. "return") a single FunctionIO manifest, rather than a stream of manifests? Another way to think about this - is our goal only to allow people to use Helm to specify their Composition logic, or do we want to go further and allow them to adapt their existing Helm charts into an XR? My feeling is the former is more important.

++    fn.apiextensions.crossplane.io/connection-details: '[{"name": "hostname", "fromConnectionSecretKey": "hostname"}]'

I'm generally against structured data in annotations, and more broadly a little worried about how much we're stuffing into annotations in the ResourceList design (required name annotation, required type annotation, optional structured connection details annotation, etc).

Would it be an option to require that the Helm charts used for Composition render (i.e. "return") a single FunctionIO manifest, rather than a stream of manifests?

I would be worried that this might result in more complicated helm charts, either long FunctionIO yamls or more advanced templating. Also by not being as intuitive, this might have a negative affect on adoption.

Another way to think about this - is our goal only to allow people to use Helm to specify their Composition logic, or do we want to go further and allow them to adapt their existing Helm charts into an XR? My feeling is the former is more important.

I agree that the former is more important and not sure whether the latter should be in scope at all since existing helm charts does not produce managed resources which would mean crossplane to deploy arbitrary resources on control plane. My concern is more around UX as I mentioned above.

I'm generally against structured data in annotations, and more broadly a little worried about how much we're stuffing into annotations in the ResourceList design (required name annotation, required type annotation, optional structured connection details annotation, etc).

Yes, it is definitely not great.
Actually I am wondering if we need to pass this connectionDetails field at all? If we want to pass existing connection secrets as input and expect XR connection secret as output (as we discussed in a separate thread), I believe outputting the final XR secret should be enough.

@turkenh I ended up pushing a commit to crossplane/crossplane#2886 that adopts a stripped down version of the FunctionIO API. I'd like to roll this conversation into that PR - I'll respond to your latest comment there. Thanks!