🌱 Add server configuration for clusterManager helm chart #1239
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
WalkthroughAdds an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
deploy/cluster-manager/chart/cluster-manager/templates/cluster_manager.yaml(1 hunks)deploy/cluster-manager/chart/cluster-manager/values.yaml(1 hunks)pkg/operator/helpers/chart/config.go(1 hunks)pkg/operator/helpers/chart/render_test.go(2 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1086
File: test/integration/util/grpc.go:146-146
Timestamp: 2025-07-23T10:10:42.066Z
Learning: In OCM codebase, there are two different GRPCServerOptions types: the local one in pkg/server/grpc/options.go (which only has GRPCServerConfig field) and the SDK one from open-cluster-management.io/sdk-go/pkg/cloudevents/server/grpc/options (which has ServerBindPort and other fields with default values). Test code uses the SDK version via grpcoptions import alias.
📚 Learning: 2025-08-28T01:58:05.882Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1146
File: deploy/cluster-manager/chart/cluster-manager/crds/0000_01_operator.open-cluster-management.io_clustermanagers.crd.yaml:128-135
Timestamp: 2025-08-28T01:58:05.882Z
Learning: Files in deploy/cluster-manager/chart/cluster-manager/crds/ and similar CRD directories are often copied from vendor/upstream sources and should not be modified directly to avoid conflicts during updates.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-28T01:58:37.933Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1146
File: deploy/cluster-manager/chart/cluster-manager/crds/0000_01_operator.open-cluster-management.io_clustermanagers.crd.yaml:247-280
Timestamp: 2025-08-28T01:58:37.933Z
Learning: CRD files in deploy/cluster-manager/chart/cluster-manager/crds/ directory are copied from vendor and should not be modified locally as changes may be overwritten during vendor updates.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-28T01:58:23.958Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1146
File: deploy/cluster-manager/chart/cluster-manager/crds/0000_01_operator.open-cluster-management.io_clustermanagers.crd.yaml:192-225
Timestamp: 2025-08-28T01:58:23.958Z
Learning: CRD files in deploy/cluster-manager/chart/cluster-manager/crds/ and deploy/cluster-manager/config/crds/ directories are copied from vendor (open-cluster-management.io/api dependency) and should not be modified locally.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-28T04:09:12.357Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1146
File: deploy/cluster-manager/chart/cluster-manager/crds/0000_01_operator.open-cluster-management.io_clustermanagers.crd.yaml:94-176
Timestamp: 2025-08-28T04:09:12.357Z
Learning: CRD files in deploy/cluster-manager/chart/cluster-manager/crds/ directory are copied from vendor/upstream sources and should not be modified directly.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-28T01:59:04.611Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1146
File: deploy/cluster-manager/olm-catalog/latest/manifests/operator.open-cluster-management.io_clustermanagers.yaml:94-176
Timestamp: 2025-08-28T01:59:04.611Z
Learning: The file deploy/cluster-manager/olm-catalog/latest/manifests/operator.open-cluster-management.io_clustermanagers.yaml is copied from vendor and should not be modified directly as changes would be overwritten during vendor updates.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-28T02:00:03.385Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1146
File: deploy/klusterlet/chart/klusterlet/crds/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml:278-280
Timestamp: 2025-08-28T02:00:03.385Z
Learning: CRD files in deploy/klusterlet/chart/klusterlet/crds/ and similar directories are copied from vendor code and should not be modified locally. Grammar or other issues in these files should be reported upstream to the vendor instead.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-04T08:58:41.865Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1107
File: manifests/klusterlet/management/klusterlet-registration-deployment.yaml:111-115
Timestamp: 2025-08-04T08:58:41.865Z
Learning: In OCM klusterlet deployments, gRPC authentication uses different file naming conventions than CSR/kube authentication: gRPC auth expects config.yaml files (/spoke/bootstrap/config.yaml and /spoke/hub-kubeconfig/config.yaml) while CSR/kube auth uses kubeconfig files. The gRPC driver explicitly creates config.yaml files in the secret data via additionalSecretData["config.yaml"] = d.configTemplate.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yamlpkg/operator/helpers/chart/render_test.go
📚 Learning: 2025-09-24T00:18:33.339Z
Learnt from: zhiweiyin318
Repo: open-cluster-management-io/ocm PR: 1194
File: deploy/klusterlet/chart/klusterlet/templates/bootstrap_kubeconfig_secret.yaml:25-27
Timestamp: 2025-09-24T00:18:33.339Z
Learning: gRPC config in OCM klusterlet bootstrap secrets does not support multiHubBootstrapHubKubeConfigs scenarios - it is intentionally designed only for single hub bootstrap configurations.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-07-25T01:21:08.891Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1077
File: test/integration/registration/spokecluster_grpc_test.go:0-0
Timestamp: 2025-07-25T01:21:08.891Z
Learning: In OCM integration tests, gRPC and kube authentication mechanisms require different CSR handling approaches: gRPC authentication uses util.ApproveCSR since the hub controller signs client certificates, while kube authentication uses authn.ApproveSpokeClusterCSR to simulate the kube-controller-manager signing client certificates.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yamlpkg/operator/helpers/chart/render_test.go
📚 Learning: 2025-07-23T10:10:42.066Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1086
File: test/integration/util/grpc.go:146-146
Timestamp: 2025-07-23T10:10:42.066Z
Learning: In OCM codebase, there are two different GRPCServerOptions types: the local one in pkg/server/grpc/options.go (which only has GRPCServerConfig field) and the SDK one from open-cluster-management.io/sdk-go/pkg/cloudevents/server/grpc/options (which has ServerBindPort and other fields with default values). Test code uses the SDK version via grpcoptions import alias.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yamlpkg/operator/helpers/chart/render_test.go
📚 Learning: 2025-08-06T06:00:53.508Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1107
File: deploy/cluster-manager/config/rbac/cluster_role.yaml:165-168
Timestamp: 2025-08-06T06:00:53.508Z
Learning: In OCM gRPC deployments, both the cluster-manager operator ClusterRole and the gRPC server ClusterRole need "create" permission on "managedclustersets/join" resources for proper bootstrapping of managed clusters.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
🧬 Code graph analysis (2)
pkg/operator/helpers/chart/config.go (1)
vendor/open-cluster-management.io/api/operator/v1/types_clustermanager.go (1)
ServerConfiguration(166-176)
pkg/operator/helpers/chart/render_test.go (4)
pkg/operator/helpers/chart/config.go (1)
ClusterManagerChartConfig(9-36)pkg/operator/helpers/chart/render.go (1)
NewDefaultClusterManagerChartConfig(25-33)vendor/open-cluster-management.io/api/operator/v1/types_klusterlet.go (1)
RegistrationConfiguration(133-189)vendor/open-cluster-management.io/api/operator/v1/types_clustermanager.go (6)
RegistrationDriverHub(144-164)GRPCRegistrationConfig(254-258)ServerConfiguration(166-176)EndpointExposure(178-195)HostnameConfig(212-220)GRPCAuthType(131-131)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: e2e
- GitHub Check: e2e-hosted
- GitHub Check: e2e-singleton
- GitHub Check: unit
- GitHub Check: integration
- GitHub Check: build
- GitHub Check: verify
🔇 Additional comments (4)
pkg/operator/helpers/chart/config.go (1)
144-147: LGTM! Clean field addition.The
ServerConfigurationfield is properly typed, has appropriate JSON tags withomitempty, and includes a descriptive comment. The placement afterAddOnManagerConfigurationis logical and maintains consistency with the struct organization.pkg/operator/helpers/chart/render_test.go (1)
235-242: LGTM! Good validation logic.The assertion correctly validates that when a GRPC authentication driver is configured, the corresponding
ServerConfiguration.EndpointsExposuremust also be populated. This helps prevent incomplete GRPC configurations.deploy/cluster-manager/chart/cluster-manager/templates/cluster_manager.yaml (1)
44-47: LGTM! Consistent with existing patterns.The optional
serverConfigurationblock follows the same pattern as other configuration sections (workConfiguration,addOnManagerConfiguration), using thewithdirective for conditional rendering and proper YAML indentation.deploy/cluster-manager/chart/cluster-manager/values.yaml (1)
126-132: LGTM! Helpful configuration example.The commented
serverConfigurationexample provides clear guidance for configuring GRPC endpoint exposure, including the structure for hostname-based endpoints.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1239 +/- ##
=======================================
Coverage 62.15% 62.15%
=======================================
Files 209 209
Lines 16968 16968
=======================================
Hits 10547 10547
Misses 5304 5304
Partials 1117 1117
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zhiweiyin318
force-pushed
the
add-grpc-init
branch
from
ca12ae8 to
6be4ab5
Compare
Signed-off-by: Zhiwei Yin <[email protected]>
zhiweiyin318
force-pushed
the
add-grpc-init
branch
from
6be4ab5 to
735f93e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
deploy/cluster-manager/chart/cluster-manager/values.yaml (1)
126-132: Consider adding a comment explaining when serverConfiguration is needed.While the example is clear, users might not immediately understand that
serverConfigurationis typically required when using GRPC registration drivers (shown in lines 116-120 above). Adding a brief comment would improve discoverability.For example, you could add a comment before line 126:
+# serverConfiguration is required when using GRPC registration drivers. +# It defines how the GRPC server endpoints are exposed to managed clusters. # serverConfiguration: # endpointsExposure:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
deploy/cluster-manager/chart/cluster-manager/templates/cluster_manager.yaml(1 hunks)deploy/cluster-manager/chart/cluster-manager/values.yaml(1 hunks)pkg/operator/helpers/chart/config.go(1 hunks)pkg/operator/helpers/chart/render_test.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/operator/helpers/chart/render_test.go
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1086
File: test/integration/util/grpc.go:146-146
Timestamp: 2025-07-23T10:10:42.066Z
Learning: In OCM codebase, there are two different GRPCServerOptions types: the local one in pkg/server/grpc/options.go (which only has GRPCServerConfig field) and the SDK one from open-cluster-management.io/sdk-go/pkg/cloudevents/server/grpc/options (which has ServerBindPort and other fields with default values). Test code uses the SDK version via grpcoptions import alias.
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1107
File: manifests/klusterlet/management/klusterlet-registration-deployment.yaml:111-115
Timestamp: 2025-08-04T08:58:41.865Z
Learning: In OCM klusterlet deployments, gRPC authentication uses different file naming conventions than CSR/kube authentication: gRPC auth expects config.yaml files (/spoke/bootstrap/config.yaml and /spoke/hub-kubeconfig/config.yaml) while CSR/kube auth uses kubeconfig files. The gRPC driver explicitly creates config.yaml files in the secret data via additionalSecretData["config.yaml"] = d.configTemplate.
📚 Learning: 2025-08-28T01:58:05.882Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1146
File: deploy/cluster-manager/chart/cluster-manager/crds/0000_01_operator.open-cluster-management.io_clustermanagers.crd.yaml:128-135
Timestamp: 2025-08-28T01:58:05.882Z
Learning: Files in deploy/cluster-manager/chart/cluster-manager/crds/ and similar CRD directories are often copied from vendor/upstream sources and should not be modified directly to avoid conflicts during updates.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-28T01:58:37.933Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1146
File: deploy/cluster-manager/chart/cluster-manager/crds/0000_01_operator.open-cluster-management.io_clustermanagers.crd.yaml:247-280
Timestamp: 2025-08-28T01:58:37.933Z
Learning: CRD files in deploy/cluster-manager/chart/cluster-manager/crds/ directory are copied from vendor and should not be modified locally as changes may be overwritten during vendor updates.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-28T01:58:23.958Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1146
File: deploy/cluster-manager/chart/cluster-manager/crds/0000_01_operator.open-cluster-management.io_clustermanagers.crd.yaml:192-225
Timestamp: 2025-08-28T01:58:23.958Z
Learning: CRD files in deploy/cluster-manager/chart/cluster-manager/crds/ and deploy/cluster-manager/config/crds/ directories are copied from vendor (open-cluster-management.io/api dependency) and should not be modified locally.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-28T04:09:12.357Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1146
File: deploy/cluster-manager/chart/cluster-manager/crds/0000_01_operator.open-cluster-management.io_clustermanagers.crd.yaml:94-176
Timestamp: 2025-08-28T04:09:12.357Z
Learning: CRD files in deploy/cluster-manager/chart/cluster-manager/crds/ directory are copied from vendor/upstream sources and should not be modified directly.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-28T01:59:04.611Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1146
File: deploy/cluster-manager/olm-catalog/latest/manifests/operator.open-cluster-management.io_clustermanagers.yaml:94-176
Timestamp: 2025-08-28T01:59:04.611Z
Learning: The file deploy/cluster-manager/olm-catalog/latest/manifests/operator.open-cluster-management.io_clustermanagers.yaml is copied from vendor and should not be modified directly as changes would be overwritten during vendor updates.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-28T02:00:03.385Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1146
File: deploy/klusterlet/chart/klusterlet/crds/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml:278-280
Timestamp: 2025-08-28T02:00:03.385Z
Learning: CRD files in deploy/klusterlet/chart/klusterlet/crds/ and similar directories are copied from vendor code and should not be modified locally. Grammar or other issues in these files should be reported upstream to the vendor instead.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-04T08:58:41.865Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1107
File: manifests/klusterlet/management/klusterlet-registration-deployment.yaml:111-115
Timestamp: 2025-08-04T08:58:41.865Z
Learning: In OCM klusterlet deployments, gRPC authentication uses different file naming conventions than CSR/kube authentication: gRPC auth expects config.yaml files (/spoke/bootstrap/config.yaml and /spoke/hub-kubeconfig/config.yaml) while CSR/kube auth uses kubeconfig files. The gRPC driver explicitly creates config.yaml files in the secret data via additionalSecretData["config.yaml"] = d.configTemplate.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-07-25T01:21:08.891Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1077
File: test/integration/registration/spokecluster_grpc_test.go:0-0
Timestamp: 2025-07-25T01:21:08.891Z
Learning: In OCM integration tests, gRPC and kube authentication mechanisms require different CSR handling approaches: gRPC authentication uses util.ApproveCSR since the hub controller signs client certificates, while kube authentication uses authn.ApproveSpokeClusterCSR to simulate the kube-controller-manager signing client certificates.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-09-24T00:18:33.339Z
Learnt from: zhiweiyin318
Repo: open-cluster-management-io/ocm PR: 1194
File: deploy/klusterlet/chart/klusterlet/templates/bootstrap_kubeconfig_secret.yaml:25-27
Timestamp: 2025-09-24T00:18:33.339Z
Learning: gRPC config in OCM klusterlet bootstrap secrets does not support multiHubBootstrapHubKubeConfigs scenarios - it is intentionally designed only for single hub bootstrap configurations.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-07-23T10:10:42.066Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1086
File: test/integration/util/grpc.go:146-146
Timestamp: 2025-07-23T10:10:42.066Z
Learning: In OCM codebase, there are two different GRPCServerOptions types: the local one in pkg/server/grpc/options.go (which only has GRPCServerConfig field) and the SDK one from open-cluster-management.io/sdk-go/pkg/cloudevents/server/grpc/options (which has ServerBindPort and other fields with default values). Test code uses the SDK version via grpcoptions import alias.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
📚 Learning: 2025-08-06T06:00:53.508Z
Learnt from: skeeey
Repo: open-cluster-management-io/ocm PR: 1107
File: deploy/cluster-manager/config/rbac/cluster_role.yaml:165-168
Timestamp: 2025-08-06T06:00:53.508Z
Learning: In OCM gRPC deployments, both the cluster-manager operator ClusterRole and the gRPC server ClusterRole need "create" permission on "managedclustersets/join" resources for proper bootstrapping of managed clusters.
Applied to files:
deploy/cluster-manager/chart/cluster-manager/values.yaml
🧬 Code graph analysis (1)
pkg/operator/helpers/chart/config.go (1)
vendor/open-cluster-management.io/api/operator/v1/types_clustermanager.go (1)
ServerConfiguration(166-176)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: integration
- GitHub Check: e2e-singleton
- GitHub Check: unit
- GitHub Check: e2e-hosted
- GitHub Check: e2e
🔇 Additional comments (3)
deploy/cluster-manager/chart/cluster-manager/values.yaml (1)
116-120: LGTM - GRPC registration driver example is clear.The commented example correctly demonstrates the GRPC registration driver configuration with appropriate auto-approved identities for the bootstrap service account.
pkg/operator/helpers/chart/config.go (1)
144-146: LGTM - ServerConfiguration field properly integrated.The new field follows Go and JSON conventions, correctly uses the vendor type
operatorv1.ServerConfiguration, and is appropriately marked as optional. The placement afterAddOnManagerConfigurationmaintains logical grouping of configuration fields.deploy/cluster-manager/chart/cluster-manager/templates/cluster_manager.yaml (1)
44-47: LGTM - Template follows established patterns.The serverConfiguration block rendering uses the same conditional
withpattern andtoYamlserialization as other optional configuration blocks in this template. The indentation and positioning are correct.
Member
Author
|
/assign @qiujian16 |
Member
Author
|
cc @ncr38 feel free help review this PR |
Member
|
/approve |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qiujian16, zhiweiyin318 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 |
openshift-merge-bot
bot
merged commit d80ec55
into
open-cluster-management-io:main
17 checks passed
Member
|
/cherry-pick release-1.1 |
|
@qiujian16: new pull request created: #1244 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Summary
Related issue(s)
Fixes #
Summary by CodeRabbit
New Features
Documentation
Tests