Clarify that functions shouldn't interact with external systems

[negz]

Description of your changes

We frequently give this guidance to function authors, but missed actually codifying it.

I went with SHOULD NOT rather than MUST NOT mutate external systems, mostly because I'm thinking of edge cases where a read operation might bump some innocuous counter or similar. I could be convinced to switch to MUST NOT.

I have:

• Read and followed Crossplane's contribution process.
• Run earthly +reviewable to ensure this PR is ready for review.
• Added or updated unit tests.
• Added or updated e2e tests.
• Linked a PR or a docs tracking issue to document this change.
• Added backport release-x.y labels to auto-backport this PR.

Need help with this checklist? See the cheat sheet.

[jbw976]

LGTM! 🙇‍♂️

[ytsarev]

Very helpful 👍

[mproffitt]

As indicated in the slack thread on this topic this statement can lead to some confusion over how communication with the API server may then take place as it is not immediately clear that there are avenues towards this goal.

To help clarify this would it be worthwhile extending this statement to explicitly cover how communication with the Kubernetes API may be achieved? For example

Suggested change

	A Function MUST NOT communicate directly with the Kubernetes API Server, for
	example to read or mutate API resources.
	A Function MUST NOT communicate directly with the Kubernetes API Server, for
	example to read or mutate API resources.
	A Function MAY read the Kubernetes API indirectly by requesting `ExtraResources` and SHOULD terminate gracefully if it is unable to continue without those resources. Similarly, a Function MAY mutate the state of the Kubernetes API indirectly, by composing managed resources (MRs).

Whilst this may seem like overkill, in my experience without a clear direction to look at, it is easy to become unsure regarding which options are available to then operate on the API, only that it is forbidden to directly operate on it.

[negz]

To help clarify this would it be worthwhile extending this statement to explicitly cover how communication with the Kubernetes API may be achieved?

I considered adding something like this. I didn't because "you can achieve it this way instead" felt less like a specification concern and more like a documentation concern. I see your point though, maybe worth it if it helps avoid confusion.

Regarding the thread, it seems the main concern is that the "MUST NOT communicate directly with the Kubernetes API Server" wording prevents a function accessing the API server to load IRSA credentials or similar, right?

That is a use case we want to support. I'm not sure how we will yet though. Some early thoughts:

• I don't think a function reading a ProviderConfig is the right path. A ProviderConfig should configure a provider (family), not functions.
• Whatever we do should work with Support passing credentials to composition functions #5543, e.g. I imagine a new function credentials source of type IRSA.
• Figuring out how this could work for functions that run outside a Kubernetes cluster will be tricky.

I understand the proposed update makes it impossible for a compliant function to use IRSA. My preference is to live with that for now. I expect we'll add language to relax this constraint in future once we know enough to be specific about what use cases (e.g. IRSA) we must allow and how.

In the meantime you can still choose to write a function that talks to the API server - nothing blocks it technically. You'll just need to be aware that your function isn't spec compliant, which could affect Crossplane's ability to successfully use it in future.

Alternatively, we could add wording today to allow talking to the API server only to get credentials. The risk there is that if we're not specific on how to do that it runs the risk of folks inventing patterns that we might not want to support long term.

[mproffitt]

felt less like a specification concern and more like a documentation concern

I see value here in codifying the interaction points for the different operations and then using the documentation to detail the "how". This would also align with the previous statement on external systems which clearly calls our using MRs to mutate external systems.

You are correct in that the main concern is about IRSA capabilities for functions. The way I see this is the lack of this capability is a blocker towards more generalized adoption of functions which offer cloud specific capabilities. I also see it as something of a hard sell to tell engineers that within a single composition they would be providing multiple credential sources for the same cloud provider which is why I went for the ProviderConfig option for my functions but this is maybe a conversation for a different ticket.

I would like to avoid the functions I am writing diverging too far from the published spec for similar reasons and it would potentially be difficult to align them in the future as additional capabilities are opened up. Alignment is something I can partially solve on my own function interfaces though.

Regarding the potential addition of wording to cover credential retrieval, I kind of think this is opening a can of worms towards functions requesting broader api access unless there was a clearly defined path to retrieve them potentially backed by the function SDK - although inversely this risks pushing crossplane into supporting a pattern it does not want or need long term.