Tweak `apiextensions.fn.proto.v1beta1` message field names

[negz]

What problem are you facing?

In a CEL or Python function you'd write for example today:

• observed.composite.resource
• desired.resources['name'].resource
• observed.resources['name'].resource
• extra_resources['group-name'][0].resource

I think observed.composite.resource reads really well, but the rest of the examples stutter a bit.

How could Crossplane help solve your problem?

I believe renaming protobuf fields isn't very disruptive, given the fields match on number. While we're still in beta we might consider tweaking the field names to read like:

• desired.composed['name'].resource
• observed.composed['name'].resource
• extra['group-name'][0].resource

So:

• We drop _resources from extra_resources to match desired and observed.
• We rename the resources field to composed to more clearly differentiate it from the composite resource.

[negz]

For the record: I'm personally ambivalent about whether this is worth it. I'll wait a while to hear whether folks think the proposed names are an improvement or not.

[Sanket-0510]

should be a easy tweak. If it is finalized should I try this out @negz ?

[negz]

@Sanket-0510 I'm still feeling ambivalent about this personally. Let me see if I can get some more feedback from others.

[negz]

I believe renaming protobuf fields isn't very disruptive, given the fields match on number.

To elaborate a little more, this change is backward compatible at the protobuf level but it wouldn't be completely nondisruptive.

Protobuf fields match on field number, not name. So older functions would work with newer Crossplanes and vice versa if we renamed the field. They'd just refer to the field using different names in their generated code.

It would be disruptive to the generated protobuf code when updated - e.g. the fields and methods in function-sdk-go and function-sdk-python would change. Folks would need to refactor their functions a little when they updated to a new SDK version.

I think this would be true for functions like function-kcl and function-cue that just use the protobufs as their data model too. Things like oxr.resource['name'] would need to be updated to oxr.composed['name'] in the KCL/CUE/Go template/etc.

[gotwarlost]

I think this would be true for functions like function-kcl and function-cue that just use the protobufs as their data model too. Things like oxr.resource['name'] would need to be updated to oxr.composed['name'] in the KCL/CUE/Go template/etc.

Yes, this would be disruptive to function-cue for sure since the protobuf JSON is the data model. That said, it is also true that the proposed names read way better than the current ones.

[TerjeLafton]

The new field name reads a lot better though and matches more with how I think about it!

The desired way of doing this in Protobuf would be to create a new field with the new name and a new number, and also reserve the existing field name and number. The old field can be deprecated. This is at least what they describe as best practice.

As long as you build the server to respect the old field if the new one is not set, this should work fine, and people can update whenever they feel like it. The SDKs would have a breaking change though, but the dependency can be updated whenever you want.

[bobh66]

I'm also ambivalent about the change. composed is more of an internal Crossplane term than something we expect users to think about - we generally refer to managed or nested composite resources in this context. If we had started out with this naming I think it would have been fine, but now I think we're at least somewhat stuck with it. If GRPC could handle aliases then I would say definitely create an alias and deprecate the old name. Duplicating the data seems inefficient, given the amount of data that is potentially involved and the number of times the functions are called. I can go either way, but the impact of a "correct" implementation on some of the functions would be considerable.

[negz]

Thanks folks. I'm leaning toward not doing this.

It does feel like an improvement, but I'm not convinced it's worth the disruption. Especially to functions that are tightly coupled to the names of the protobuf fields.

The desired way of doing this in Protobuf would be to create a new field with the new name and a new number, and also reserve the existing field name and number. The old field can be deprecated. This is at least what they describe as best practice.

As long as you build the server to respect the old field if the new one is not set, this should work fine, and people can update whenever they feel like it.

@TerjeLafton In Crossplane, the function is the server and Crossplane is the client. If we remove and reserve the current field numbers, a Crossplane built after the change wouldn't work with a function built before the change. Crossplane would send the new field (e.g. desired.composed), which the function would know nothing about.

If we instead renamed the fields, keeping the same field numbers and data structures things would be less disruptive but still disruptive. A Crossplane built after the change would send valid requests to a function built before the change, the function would just use the old names to access the fields. Similarly, a Crossplane built before the change would send valid requests to a function built after the change. Crossplane would use the old names to build the on-wire message, and the function would use the new names to read and respond.

The issue is that some functions very quickly violated https://protobuf.dev/programming-guides/dos-donts/#text-format-interchange. 🙂 That's understandable, as we didn't warn them about it. Once such a function updated to the new SDK, its user-facing data structures would have their fields renamed and need updates.

[TerjeLafton]

@TerjeLafton In Crossplane, the function is the server and Crossplane is the client. If we remove and reserve the current field numbers, a Crossplane built after the change wouldn't work with a function built before the change. Crossplane would send the new field (e.g. desired.composed), which the function would know nothing about.

The issue is that some functions very quickly violated https://protobuf.dev/programming-guides/dos-donts/#text-format-interchange. 🙂 That's understandable, as we didn't warn them about it. Once such a function updated to the new SDK, its user-facing data structures would have their fields renamed and need updates.

I didn't think this through; what you say make a lot of sense 😄 Of course the function is the server, which compliactes this change a lot.

While the proposed field name looks better and makes more sense for me personally, I totally agree that this is not worth it.