🌱 standardize API conventions #31
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: TylerGillson The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
TylerGillson
force-pushed
the
chore/tidy-api-defaults
branch
3 times, most recently
from
f64b3a2 to
ecaf7a9
Compare
TylerGillson
changed the title
+optional in conjunction with every omitempty; tidy code accordingly+optional in conjunction with every omitempty
…junction with all omitemptys, tidy code Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
TylerGillson
force-pushed
the
chore/tidy-api-defaults
branch
from
ecaf7a9 to
3a0efd4
Compare
TylerGillson
changed the title
+optional in conjunction with every omitempty+optional in conjunction with every omitempty
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
TylerGillson
changed the title
+optional in conjunction with every omitempty
Member
|
/lgtm |
openshift-merge-bot
bot
merged commit 6a38ee3
into
open-cluster-management-io:main
7 checks passed
arturshadnik
pushed a commit
to arturshadnik/ocm-labs
that referenced
this pull request
Jul 22, 2025
* refactor: use pointers only for optional fields, use +optional in conjunction with all omitemptys, tidy code Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> * chore: clarify webhook for hosted mode Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> * fix: force specification of either clusterManager or singleton Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> * chore: make reviewable Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> --------- Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
TylerGillson
changed the title
openshift-merge-bot bot
pushed a commit
that referenced
this pull request
Jul 22, 2025
* refactor: standardize API conventions (#31) * refactor: use pointers only for optional fields, use +optional in conjunction with all omitemptys, tidy code Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> * chore: clarify webhook for hosted mode Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> * fix: force specification of either clusterManager or singleton Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> * chore: make reviewable Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> --------- Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> * feat: implement addon create and delete reconcile on the hub Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * chore: address review comments Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * refactor: do all CMA purging at the end Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * chore: clarify comment Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * fix: update manager rbac Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * feat: use separate CM for each addon's manifests Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * chore: error wording Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * chore: try to delete all addons before returning errors Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * fix: move err return to prevent orphaning addon CRs Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * chore: parse URLs using lib; use different keys depending on manifest source Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * chore: make reviewable Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * chore: make reviewable Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * chore: add annotations and labels to CMs Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * feat: add addon manifest validation to fcc webhook Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * chore: readibility Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * fix: dont generate deepcopy methods for custom validator Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> * fix: dont use pointer for addonconfig Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> --------- Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> Signed-off-by: Artur Shad Nik <arturshadnik@gmail.com> Co-authored-by: Tyler Gillson <tyler.gillson@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The original goal of this PR was to make API defaulting consistent for all fields and clean up a bit of unnecessary code. However, a bunch of other changes snuck in along the way.
API Conventions
// +kubebuilder:default:={}, to enable nested defaults) are no longer pointers. Use this approach to properly defaultregistrationAuth.+optionalin conjunction with everyomitemptyand+requiredotherwise.Fixes
clusterManagerorsingleton. Clarified webhook validation accordingly. Updated Helm chart values and FleetConfig template to specify either clusterManager or singleton mode.multicluster-controlplanechart with default values should be supported.ManagedClusterKubeconfigin hosted mode by ensuring a kubeconfig secret is provided.