feat: automatic connection details support for v2 XRs #278
Conversation
Signed-off-by: Jared Watts <jbw976@gmail.com>
| A full [connection details guide][docs-connection-details] can be found in the | ||
| Crossplane documentation. | ||
|
|
||
| ### Setting name/namespace |
There was a problem hiding this comment.
it would be helpful to explicitly state that these rules apply only to Crossplane v2-style XRs
There was a problem hiding this comment.
cool, i will add some clarification here, good point!
| schema, this function will use that reference. This can be useful for maintaining | ||
| consistency with existing XR configurations. | ||
|
|
||
| **Default auto generated** |
There was a problem hiding this comment.
I’m sceptical about automatically generating a connection Secret. Creating a Secret is a security and lifecycle significant action, and I think it should require explicit user intent via the options above.
There was a problem hiding this comment.
thanks for bringing this up, I can see some merit to this point. However, I think it's still a good idea to automatically generate a connection secret because:
- this was already being done in the v1 flow, the XR would automatically get a connection secret created for it named
<XR-uuid> - i would argue the user has explicitly specified their intent to have a secret by including
connectionDetailsfor at least 1 of their resources in the composition. If they haven't specified any of those, then no secret will be created - this hopefully lowers the friction/changes for users migrating from v1 - if they do nothing with their composition for connection details, it "still works" and a secret is still created for them.
what do you think about that perspective?
There was a problem hiding this comment.
Although I can also see the security concern, I agree it makes sense to default to the v1 behavior here 👍
There was a problem hiding this comment.
i would argue the user has explicitly specified their intent to have a secret by including connectionDetails for at least 1 of their resources in the composition. If they haven't specified any of those, then no secret will be created
This is a valid point :)
Signed-off-by: Jared Watts <jbw976@gmail.com>
connection.go
Outdated
| // 1. input.writeConnectionSecretToRef - if name or namespace is provided | ||
| // then the whole ref will be used | ||
| // 2. xr.writeConnectionSecretToRef - this is no longer automatically added | ||
| // to v2 XR schemas, but the community has been adding it manually, so if | ||
| // it's present we will use it. |
There was a problem hiding this comment.
🤔 wouldn't it make sense to flip these? Given that there are many XRs and one input per Composition, I would expect the input's one to be a default, if the XR didn't specify one already.
There was a problem hiding this comment.
yeah when i was thinking about the order of precedence, i could have gone either way on it. the reason i didn't put xr.spec.writeConnectionSecretToRef is because it isn't built-in to the schema anymore, so it felt a bit odd making an optional thing that the user has to add themselves be the highest precedence.
but i didn't feel very strong about that, and it does make sense to put an XR instance (many) value at a higher priority than what's in the composition (one), due to the many:one relation you pointed out. I'll make that change! may need to update some docs too 😊
… v2 XR connection details Signed-off-by: Jared Watts <jbw976@gmail.com>
3 participants
This PR adds support for connection details in Crossplane v2 composite resources.
v2 XRs don't support the classic connection details flow where functions return details via the function response and Crossplane core creates the secret. Instead, for v2 XRs, this function now automatically composes a
Secretresource containing the connection details and includes it with the XR's other composed resources.The README has been updated with some details about how this works and some small snippet examples, I recommend reading that first.
The new connection details composition guide will be updated to include an example for function-patch-and-transform once this PR is merged and a release is published.