-
Notifications
You must be signed in to change notification settings - Fork 89
feat: Migrate provider-template from cluster scope to namespaced scope #132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Migrate provider-template from cluster scope to namespaced scope #132
Conversation
e511997 to
62d029c
Compare
|
hello Thanks for your contribution. I was just wondering if there is any plan on getting this PR merged any time soon? |
|
Hi @arashzz there are some minor things left to do and test, but will merge it soon. |
5200ef5 to
961fc35
Compare
|
Hi @cychiang |
4574df9 to
b172962
Compare
7e53dbd to
018ed63
Compare
|
@jbw976 Hey! Would be good to have your inputs. I have made some changes, and tested it locally. You can find ready-to-use image from: |
jbw976
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good overall @cychiang! I installed your built/published provider into a v2.0.2 control plane, created the examples/ resources and everything looked OK there.
Just a few questions, but I'll approve now so you can respond as appropriate and then merge when you're ready. Great work!! 🙇♂️ 🤩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this template file used to generate the mytype controller? curious if they are entirely in sync, or the mytype controller still has manual stuff added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to double check it again. I tried to update it, but I guess failed to compare the generated output. Will verify it again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have fixed issues in go templating. I think it's okay to merge it as a first v2 release.
106f1c2 to
4ba4e25
Compare
|
Thanks for the efforts. |
|
Hey! @arashzz sorry to keep you waiting, I faced some issues when trying to validate the template repository. Currently, it's too much moving pieces need to be done after click "use this template". You can run this repository without issues, but will hit some walls when adding new apis. Like changing module name in go.mod, make sure you change all import path with the new module name and so on. Also, if there are changes like new features require provider updates then this template repository won't work well in this scenario, and cause fraction on lack of support of provider updates. After hitting these kind of issues, I decided to work on a new tool to cover the full life cycle for Crossplane provider. Specifically for providers without upjet. I have a working version, and it helps you to:
It's not perfect at the moment, and I have introduced the tool in Crossplane meet up on September. Here is the tool I'm working on. Feel free to bootstrap your crossplane provider project with this, and feedbacks are more than welcome. I will put my main focus on perfection the tool at the moment, and have less focus on this PR. |
d04b1e6 to
6d7bfe5
Compare
…port This commit migrates the provider template from cluster-scoped to namespaced managed resources, aligning with Crossplane v2.0 best practices for improved multi-tenancy and resource isolation. It also implements safe-start support to prevent controller race conditions during startup. Key changes: API Updates: - Consolidate ProviderConfig and related types into unified types.go - Add namespaced ProviderConfig alongside cluster-scoped ClusterProviderConfig - Add namespaced ProviderConfigUsage and ClusterProviderConfigUsage - Update MyType to use namespaced scope by default - Remove deprecated StoreConfig (external secret stores no longer supported in v2) - Migrate to crossplane-runtime v2.0.0-rc.1 with breaking changes Controller Enhancements: - Implement safe-start pattern with SetupGated functions - Add gate-based controller registration to wait for CRDs - Add customresourcesgate controller to watch for CRD availability - Update credential extraction to support both namespaced and cluster-scoped configs - Support dynamic ProviderConfig reference kind switching - Remove deprecated feature flag system in favor of runtime feature package Dependencies: - Upgrade to crossplane-runtime v2.0.0-rc.1 - Add k8s.io/apiextensions-apiserver for CRD scheme registration - Update controller-runtime to v0.21.0 - Update Go toolchain to 1.24.5 Templates and Examples: - Update code generation templates for namespaced resources - Add namespace field to example manifests - Update ProviderConfig examples to demonstrate new structure - Remove StoreConfig examples This migration provides better resource isolation, simplified authentication patterns, and improved startup reliability through the safe-start mechanism. Signed-off-by: $(git config user.name) <$(git config user.email)> Signed-off-by: Chuan-Yen Chiang <[email protected]>
6d7bfe5 to
730aa21
Compare
This commit refactors the provider template code generation templates and removes the non-gated Setup function in favor of the safe-start pattern. Key changes: Template Updates: - Add SetupGated function to controller template for safe-start support - Update controller template to use v1alpha1 instead of dynamic API version - Fix typo: WithExternalConnecter -> WithExternalConnector - Remove unused ConnectionPublisher initialization - Add missing xpv1 import for ResourceStatus in type templates - Change Status to use xpv1.ResourceStatus instead of xpv2.ResourceStatus - Improve error constant alignment and formatting Controller Cleanup: - Remove deprecated Setup function from template.go - Keep only SetupGated function for safe-start pattern - All controllers now exclusively use gated setup Package Metadata: - Add safe-start capability to crossplane.yaml package metadata Documentation: - Fix capitalization: "Template provider" -> "template provider" - Fix capitalization: "group Sample" -> "group sample" These changes ensure all generated controllers follow the safe-start pattern and improve consistency across the template codebase. Signed-off-by: Chuan-Yen Chiang <[email protected]>
This commit renames the controller registration file for better clarity and updates the README with instructions for using the safe-start pattern. Changes: - Rename internal/controller/template.go -> internal/controller/register.go to better reflect its purpose as a controller registration file - Update README.md to include step for registering new types in SetupGated function in register.go The rename makes it clearer that this file is responsible for registering controllers with the manager, and the README update ensures developers know to use the SetupGated function when adding new resource types. Signed-off-by: Chuan-Yen Chiang <[email protected]>
|
@jbw976 I'm going to merge this PR:
|
|
Sounds good @cychiang, thanks for your effort on this!! You're doing a great job at keeping this provider up to date and that is really appreciated! 🙇♂️ |
|
@cychiang thank you so much for your effort making this happen. You're awesome. 💯 I was looking at this commit and noticed even though the filename was changed but the hack/helpers/controller/prepare.sh file is still using the also noticed in the README.file on step 8 it was mentioned |
|
@arashzz oh right, thanks for the feedback, I tested with |
Description of your changes
ClusterProviderConfigProviderConfigto namespaced.MyTypeto namespaced.ProviderConfigandClusterProviderConfigPlease note that it works for Crossplane v2 and not backward compatible.
I have:
make reviewableto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR if necessary.How has this code been tested