Update to crossplane-runtime v2.0.0#226
Conversation
jbw976
left a comment
There was a problem hiding this comment.
Thanks for taking the initiative on this @twobiers!
Would you also be up for taking this update for a test drive in building a function, e.g. building on what's in https://docs.crossplane.io/v2.0/guides/write-a-composition-function-in-go/, to see if everything is working as expected there for namespaced resources? That would be really helpful 🙏
|
@twobiers did you get a chance to check if your PR works with the linked docs? |
|
I was able to successfully test this version (redirect to twobiers:v2) of the function sdk in some internal functions with namespaced and cluster scoped resource functions. |
|
@jbw976 can we bring this in and cut a new release ? |
No, I didn't have enough capacity yet for a full trial. If anyone wants to take this PR over, feel free to do so. |
Thanks for your efforts so far @twobiers! and thank you @bakito for your testing validation, really helpful! 🙇♂️ I rebased this PR from latest on main to fix merge conflicts, mostly around the removal of provider-aws dependency in #224. I also bumped go to v1.24.8 and golangci-lint to v2.4.0 consistently across Makefile and ci.yml. I've done some testing using some local |
|
@jbw976 can we merge the PR then ? |
Signed-off-by: twobiers <22715034+twobiers@users.noreply.github.com>
Signed-off-by: twobiers <22715034+twobiers@users.noreply.github.com>
Signed-off-by: twobiers <22715034+twobiers@users.noreply.github.com>
Signed-off-by: Jared Watts <jbw976@gmail.com>
Signed-off-by: Jared Watts <jbw976@gmail.com>
|
Just resolved more merge conflicts now - @haarchri will add some details for how he's tested this PR as well 💪 |
📝 WalkthroughWalkthroughUpgrades dependencies and imports to crossplane-runtime v2, adjusts CI/lint tooling versions and config, removes two PublishConnectionDetailsTo methods from composed and composite resources, adds a test for namespace-scoped required resources, and switches listener creation to net.ListenConfig in SDK and tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SDK as SDK Server
participant Net as net.ListenConfig
User->>SDK: Start(options)
SDK->>Net: Listen(ctx, so.Network, so.Address)
alt listen ok
Net-->>SDK: net.Listener
SDK-->>User: Running with listener
else error
Net-->>SDK: error
SDK-->>User: return error
end
sequenceDiagram
autonumber
participant Fn as Function Logic
participant RR as RequiredResources
participant U as Unstructured
Fn->>RR: GetRequiredResources()
RR-->>Fn: items with {apiVersion, kind, metadata.name[, metadata.namespace]}
loop for each item
Fn->>U: Build Unstructured
note right of U: Preserve metadata.name<br/>and metadata.namespace if present
end
Fn-->>Fn: Return []Unstructured
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Thanks for driving the v2 migration. Two quick confirmations:
Pre-merge checks❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (12)
🧰 Additional context used📓 Path-based instructions (2)**/*.go⚙️ CodeRabbit configuration file
Files:
**/*_test.go⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (1)request/request_test.go (2)
🔇 Additional comments (13)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Jared Watts <jbw976@gmail.com>
jbw976
left a comment
There was a problem hiding this comment.
I was able to build a quick test function that uses the updates in this branch: https://github.com/jbw976/function-v2-test
That function just does the very basic action of returning a variable number of desired ConfigMap resources (which are namespaced). I was able to build and test this function in a live cluster and verify the expected resources are created when the function runs.
The changes in this PR look good to me! Thanks again for your initiative and effort @twobiers and @bakito @haarchri for also helping test 🙇♂️
Description of your changes
Fixes #225
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Running unit tests