Proposal: Generic Cross-Resource References

[kasey]

What problem are you facing?

As mentioned in the design doc for terraform-based controller codegen(https://github.com/crossplane/crossplane/pull/1677/) , we will need to implement a generic cross-resource referencing scheme to compensate for the fact that terraform does not provide the necessary metadata for our code generator to understand which fields are cross-referenceable.

How could Crossplane help solve your problem?

A design document would allow us to collect feedback on the syntax and behavior of these references.

[negz]

Here's a strawman sketch of what generic cross-resource references might look like.

Today we have this for example:

---
apiVersion: network.aws.crossplane.io/v1alpha2
kind: VPC
metadata:
  name: my-vpc
spec:
  ...
---
apiVersion: network.aws.crossplane.io/v1alpha2
kind: Subnet
metadata:
  name: my-subnet
spec:
  forProvider
    vpcId: vpc-33fk3fjdf2  # The VPC's actual external name
    # Used to resolve the VPC's actual external name.
    # The resolution logic is written in Go as part of
    # adding support for a new managed resource.
    vpcIdRef:
      name: my-vpc
  ...

With generic cross-resource references this might look like:

---
apiVersion: network.aws.crossplane.io/v1alpha2
kind: VPC
metadata:
  name: my-vpc
spec:
  ...
---
apiVersion: network.aws.crossplane.io/v1alpha2
kind: Subnet
metadata:
  name: my-subnet
spec:
  # Open ended naming so we could extend this with other kinds
  # of field modifiers. Perhaps this is overthinking it?
  fieldModifiers:
  - fromRef:
      apiVersion: v1alpha2
      kind: VPC
      name: my-vpc
      fieldPath: metadata.annotations[crossplane.io/external-name]
    # Perhaps this should be scoped/limited to spec.forProvider?
    toFieldPath: spec.forProvider.vpcId
  forProvider:
    vpcId: vpc-33fk3fjdf2  # The VPC's actual external name
  ...

[muvaf]

That looks pretty good to me. Do we want to allow taking values from namespaced resources?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[negz]

Unstale please. I'm still pretty sure we're going to need this.

[negz]

Throwing out another idea (based on a demo of something related @apelisse put together) - I could see a distinct Reference type being in some ways better than an array of references embedded in a resource.

So we'd have something like:

---
apiVersion: network.aws.crossplane.io/v1alpha2
kind: VPC
metadata:
  name: my-vpc
  annotations:
    crossplane.io/external-name: vpc-123jdfdf
spec:
  ...
---
apiVersion: network.aws.crossplane.io/v1alpha2
kind: Subnet
metadata:
  name: my-subnet
spec:
  forProvider:
    vpcId: "" # I need a value to function!
  ...
---
apiVersion: crossplane.io/v1alpha1
type: Reference
metadata:
  name: subnet-to-vpc
spec:
  # A reference "from" a subnet "to" a VPC
  from:
    apiVersion: network.aws.crossplane.io/v1alpha2
    kind: Subnet
    name: my-subnet
    fieldPath: spec.forProvider.vpcId
  to:
  # An array to support the case where a reference is derived from multiple
  # fields in multiple objects? This would likely end up pretty close to
  # Composition patches, with transforms etc. We'd probably want to support
  # selectors here too, not only explicit references.
  - apiVersion: network.aws.crossplane.io/v1alpha2
    kind: VPC
    name: my-vpc
    fieldPath: metadata.annotations[crossplane.io/external-name]

Some challenges that come to mind:

• RBAC. The controller resolving the Reference type would need to read "to" resources and write to the "from" resource.
• Blocking reconciliation of the "from" (referencing) resource until the "to" (referenced) resource fieldpath value(s) were populated.
• Discoverability of references. You probably want to kubectl describe <from> <name> and see what it references (and vice versa?).

Presuming the controller responsible for resolving the Reference type was part of core Crossplane it would in most cases already have all the RBAC permissions it needed to resolve references from Crossplane types to other Crossplane types because the RBAC manager grants the Crossplane pod permission to read all XRs and MRs that it might need to compose.

I could imagine blocking and discoverability being handled by having the managed resource reconciler:

1. List all References and find any that are "from" its managed resource.
2. Record those references under (e.g.) status.references
3. Block further reconciliation until all recorded references were resolved

I could imagine the Reference type holding policy similar to that which @sergenyalcin recently added to contemporary references - i.e. whether a reference should be resolved once or on every reconcile, and whether a reference should block reconciliation or not.

[negz]

I was just reminded of one other shortcoming with generic references - when any field can be 'computed' by a reference no field can be required in the OpenAPI schema sense, because then the referencing (from) object could never be created in the first place.

One horrible hack we could do here would be to have folks create the resource with a "placeholder" required value then guarantee that references would be processed (switching out the placeholder value), but that sounds like a pretty terrible UX to me.

[apelisse]

One horrible hack we could do here would be to have folks create the resource with a "placeholder" required value then guarantee that references would be processed (switching out the placeholder value), but that sounds like a pretty terrible UX to me.

Yeah, then you need either loose validation or the placeholder to pass the validation, and even then you don't really know if the final value will pass the validation either.

[negz]

when any field can be 'computed' by a reference no field can be required in the OpenAPI schema sense

Last night I was wondering whether a mutating admission control webhook could resolve a Reference object to satisfy required fields, but in retrospect this wouldn't work. Probably for a bunch of reasons (client-side validation? Do mutating admission control webhooks even run before OpenAPI validation?), but mostly because the webhook would have to reject create requests if the Reference object referenced a thing that wasn't yet ready.

I could imagine potentially making all fields optional and adding an x-crossplane-required OpenAPI extension where "required" meant "either you specify a value explicitly or a Reference exists that will eventually specify the value".

[dudicoco]

Hi @negz, is there an update on this feature?

[github-actions[bot]]

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 7 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

[jbw976]

/fresh

[negz]

@turkenh and I discussed tackling this as part of the Usage type we're introducing to handle deletion ordering in https://github.com/crossplane/crossplane/pull/4215. We decided not to conflate them, at least for now. They're somewhat related but we're not sure whether a single API makes sense.

[negz]

Here's a strawman sketch of what generic cross-resource references might look like.

We may need to be careful that generic references don't devolve into something similar to patch and transform Composition.

Contemporary cross-resource references work at the MR level. So one MR can reference another even if they're not part of the same Composition. With Compositions we've seen that often folks don't just want to copy data from one field to another. They usually need to transform and potentially combine data to form a new field. I expect we'll see this in generic references too. Consider for example a reference from MR A to MR B. The reference needs to set a field on MR A in the form region/name, but region and name are two distinct fields on MR B. Doing this requires combining and formatting different values.

We've found with P&T Composition it's tough to invent a new API that is intuitive but also sufficiently expressive for this, and are leaning toward promoting Composition Functions as the answer. Perhaps supporting something like https://jmespath.org/ would help, though I'm wary of the devex around embedding a query language in an opaque YAML string.

[pedjak]

@negz I found this one pager, merged through https://github.com/crossplane/crossplane/pull/874. Is it obsolete?

[negz]

@pedjak The implementation has changed a few times since that design was implemented, but the API it describes is still valid. i.e. Many MRs have spec.forProvider.example fields that can be resolved by spec.forProvider.exampleRef and spec.forProvider.exampleSelector fields.

This issue tracks finding a more generic way to do this. The problems with the current implementation are:

• We need to manually identify every field that could be a reference and write or generate code to implement it.
• Our references are inflexible - you can't derive an arbitrary field in an arbitrary object from an arbitrary field in another arbitrary object unless the two objects are part of the same Composition.
• A provider must compile-in any types that it references. For example provider-aws-eks must import provider-aws-ec2. This creates a potential version dependency coupling between two providers.

[turkenh]

After the discussions on the latest design, we are aligned on enabling this at the upper layers rather than implementing it at the Managed Resource level. See this comment summarizing our latest thinking.

We also want to consider whether we should deprecate the existing Cross Resource References in favor of functionalities available at the composition layer.