[WIP]:sparkles: Enhance ClusterProxy API. #401
Conversation
WalkthroughAdds a new public feature gate Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
/not-ok-to-test |
![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
🧹 Nitpick comments (1)
operator/v1/types_klusterlet.go (1)
357-362: Add validation for authentication types.The
ProxyAuthenticationTypeconstants are defined, but there's no validation to ensure only valid values are accepted.Apply this diff to add enum validation:
type ProxyAuthenticationType string const ( ProxyAuthenticationUserToken ProxyAuthenticationType = "userToken" ProxyAuthenticationImpersonation ProxyAuthenticationType = "impersonation" ) + +// +kubebuilder:validation:Enum=userToken;impersonationAlternatively, add the validation to the
Authenticationsfield inProxyConfig:// Authentications defines how the agent authenticates with the cluster. // If not specified, defaults to ["userToken"]. // +optional // +kubebuilder:default={userToken} + // +kubebuilder:validation:items:Enum=userToken;impersonation Authentications []ProxyAuthenticationType `json:"authentications,omitempty"`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
feature/feature.go(2 hunks)operator/v1/types_klusterlet.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
operator/v1/types_klusterlet.go (2)
addon/v1alpha1/types_addondeploymentconfig.go (1)
ProxyConfig(135-153)operator/v1/types_clustermanager.go (1)
EndpointExposure(178-195)
feature/feature.go (1)
vendor/k8s.io/component-base/featuregate/feature_gate.go (3)
Feature(40-40)FeatureSpec(72-83)Alpha(100-100)
⏰ 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). (1)
- GitHub Check: unit
🔇 Additional comments (3)
feature/feature.go (1)
96-115: LGTM!The feature gate constant is well-documented with comprehensive information about its purpose, behavior, configuration requirements, use cases, and fallback options.
operator/v1/types_klusterlet.go (2)
104-116: LGTM!The
ProxyConfigfield is well-documented with clear explanations of its purpose, behavior, and requirements. The documentation appropriately references the ClusterProxy feature gate and GRPCConfiguration dependencies.
348-362: No changes required: EndpointExposure is compatible with ProxyConfig. TheProxyConfig.GRPCfield correctly reuses theEndpointExposuretype fromtypes_clustermanager.go, which documentsUsage(“agentToHub”/“consumer”) andProtocol(enumgrpc;https, defaultgrpc).
xuezhaojun
force-pushed
the
enhance-cluster-proxy-api
branch
from
3bb3d36 to
91fa16d
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: xuezhaojun The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
operator/v1/types_klusterlet.go (1)
348-355: Fix field naming, add validation markers, and clarify semantics.Several issues need to be addressed:
Field naming inconsistency: The field is named
GRPCbut the JSON tag isgrpcEndpoint. This mismatch creates confusion. Consider renaming toGRPCEndpointto match the JSON tag and improve clarity, especially sinceEndpointExposureitself contains aGRPCfield (creating confusing nesting likeGRPC.GRPC).Missing validation markers:
- Line 350 lacks
+optionalor+requiredmarker and has noomitemptyin the JSON tag, making it unclear whether this field is mandatory.- Line 354 lacks
+optionalmarker despite havingomitempty.Unenforced default: Line 353 documents that the default is
userToken, but there's no kubebuilder default annotation to enforce this at the API level.Apply this diff to address the issues:
// ProxyConfig holds the configuration of klusterlet-proxy. type ProxyConfig struct { - GRPC *EndpointExposure `json:"grpcEndpoint"` + // GRPCEndpoint represents the gRPC endpoint configuration for the proxy connection. + // +required + // +kubebuilder:validation:Required + GRPCEndpoint *EndpointExposure `json:"grpcEndpoint"` // Authentications defines how the agent authenticates with the cluster. - // By default it is `userToken`, but it could also be `impersonation` or both. + // If not specified, defaults to ["userToken"]. + // +optional + // +kubebuilder:default={userToken} Authentications []ProxyAuthenticationType `json:"authentications,omitempty"` }
🧹 Nitpick comments (1)
operator/v1/types_klusterlet.go (1)
348-355: Consider a more specific name to avoid developer confusion.Two
ProxyConfigtypes now exist with different purposes: one inaddon/v1alpha1handles HTTP/HTTPS proxy settings, whileoperator/v1.ProxyConfig(new) handles gRPC endpoint exposure and authentication. While Go's package scoping prevents compilation issues, renaming the new type to something likeKlusterletProxyConfigorGRPCProxyConfigwould improve clarity and reduce developer confusion when working across packages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
feature/feature.go(2 hunks)operator/v1/types_klusterlet.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- feature/feature.go
🧰 Additional context used
🧬 Code graph analysis (1)
operator/v1/types_klusterlet.go (1)
operator/v1/types_clustermanager.go (1)
EndpointExposure(178-195)
🔇 Additional comments (1)
operator/v1/types_klusterlet.go (1)
104-116: LGTM! Well-documented API addition.The documentation clearly explains the klusterlet-proxy functionality, prerequisites (feature gate, hub-side configuration), and use case. The field is properly marked as optional.
xuezhaojun
force-pushed
the
enhance-cluster-proxy-api
branch
from
f10484a to
37e9d57
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
operator/v1/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml (2)
198-204: Add enum validation for authentication types.The
authenticationsfield accepts any string values, but only"userToken"and"impersonation"are valid based on the Go constants (ProxyAuthenticationUserTokenandProxyAuthenticationImpersonation). Consider adding an enum constraint to validate values at the CRD level, similar to theprotocolfield's enum at lines 268-271.Apply this diff to add enum validation:
authentications: description: |- Authentications defines how the agent authenticates with the cluster. By default it is `userToken`, but it could also be `impersonation` or both. items: + enum: + - userToken + - impersonation type: string type: array
205-279: Consider validation to match protocol with endpoint fields.The field name
grpcEndpointand its structure allow bothgrpcandhttpsconfigurations, controlled by theprotocolfield. While this provides flexibility, there's no validation to ensure:
- When
protocol: grpc, thegrpcfield is actually set- When
protocol: https, thehttpsfield is actually setThis could lead to runtime configuration errors. Consider adding CRD validation rules (e.g., using CEL validation) to enforce consistency between the
protocolvalue and the corresponding endpoint field, or rely on clear runtime validation in the controller.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
feature/feature.go(2 hunks)operator/v1/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml(1 hunks)operator/v1/types_klusterlet.go(2 hunks)operator/v1/zz_generated.deepcopy.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- feature/feature.go
- operator/v1/types_klusterlet.go
🧰 Additional context used
🧬 Code graph analysis (1)
operator/v1/zz_generated.deepcopy.go (2)
operator/v1/types_klusterlet.go (2)
ProxyConfig(349-355)ProxyAuthenticationType(357-357)operator/v1/types_clustermanager.go (1)
EndpointExposure(178-195)
🔇 Additional comments (3)
operator/v1/zz_generated.deepcopy.go (2)
644-648: LGTM! Correct deepcopy handling for ProxyConfig.The generated code properly checks for nil before allocating and deep copying the ProxyConfig field, following the standard pattern used throughout the file.
762-786: LGTM! Complete and correct deepcopy implementation.The generated deepcopy methods for ProxyConfig properly handle:
- Deep copying of the GRPC pointer field (EndpointExposure)
- Deep copying of the Authentications slice
The implementation follows the standard pattern and correctly handles nil checks.
operator/v1/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml (1)
184-196: LGTM! Clear documentation.The proxyConfig field documentation clearly explains the feature's purpose, prerequisites (ClusterProxy feature gate, hub-side GRPCConfiguration), and use case.
Signed-off-by: xuezhaojun <[email protected]>
xuezhaojun
force-pushed
the
enhance-cluster-proxy-api
branch
from
c419f51 to
e8a49ae
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
operator/v1/types_klusterlet.go (1)
348-360: Public API naming: “grpcEndpoint” is misleading; consider neutral name and avoid cross‑package ProxyConfig collision.
- Field supports protocol=https too; json name “grpcEndpoint” will age poorly.
- Another ProxyConfig exists in addon/v1alpha1; same name across packages is confusing for users/imports.
Propose before GA:
- Rename json field to “endpoint” (or “proxyEndpoint”) and adjust comment.
- Consider renaming this type to “KlusterletProxyConfig”.
type ProxyConfig struct { - // GRPCEndpoint represents the gRPC endpoint configuration for the proxy connection. + // EndpointExposure represents the proxy endpoint (grpc or https) used by the klusterlet-proxy. // +required // +kubebuilder:validation:Required - GRPCEndpoint *EndpointExposure `json:"grpcEndpoint"` + GRPCEndpoint *EndpointExposure `json:"endpoint"` // Authentications defines how the agent authenticates with the cluster. // If not specified, defaults to ["userToken"]. // +optional // +kubebuilder:default={userToken} Authentications []ProxyAuthenticationType `json:"authentications,omitempty"` }Optional (type rename; wider ripple):
- type ProxyConfig struct { + type KlusterletProxyConfig struct { ... - ProxyConfig *ProxyConfig `json:"proxyConfig,omitempty"` + ProxyConfig *KlusterletProxyConfig `json:"proxyConfig,omitempty"`Verification helper:
#!/usr/bin/env bash # Find other ProxyConfig definitions/usages to assess rename impact. rg -nP --type=go '^\s*type\s+ProxyConfig\s+struct|[^A-Za-z]ProxyConfig[^A-Za-z]' -C2
🧹 Nitpick comments (1)
operator/v1/types_klusterlet.go (1)
362-368: Enum added for ProxyAuthenticationType — good. Consider set semantics to prevent duplicates.CRD can still accept duplicated values (e.g., ["userToken","userToken"]). If desired, add list-type=set on the field via CRD (see CRD comment) or webhook-dedupe.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
feature/feature.go(2 hunks)operator/v1/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml(1 hunks)operator/v1/types_klusterlet.go(2 hunks)operator/v1/zz_generated.deepcopy.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- feature/feature.go
🧰 Additional context used
🧬 Code graph analysis (2)
operator/v1/types_klusterlet.go (1)
operator/v1/types_clustermanager.go (1)
EndpointExposure(178-195)
operator/v1/zz_generated.deepcopy.go (2)
operator/v1/types_klusterlet.go (2)
ProxyConfig(349-360)ProxyAuthenticationType(363-363)operator/v1/types_clustermanager.go (1)
EndpointExposure(178-195)
⏰ 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). (1)
- GitHub Check: verify
🔇 Additional comments (2)
operator/v1/zz_generated.deepcopy.go (2)
762-786: ProxyConfig DeepCopy functions look correct.Deep copy of EndpointExposure pointer and slice copy for Authentications are correct for value types.
644-648: ProxyConfig deep-copy implementation verified as correct.The code at lines 644-648 properly implements deep-copying: nil check prevents dereferencing errors, local pointers avoid aliasing, and
DeepCopyIntodelegates to ProxyConfig's own deepcopy logic. ProxyConfig's methods (lines 763–785) are correctly defined. No memory leaks or aliasing issues detected.
| proxyConfig: | ||
| description: |- | ||
| ProxyConfig holds the configuration for enabling klusterlet-proxy functionality, | ||
| which allows the hub cluster to access the managed cluster's API server through | ||
| a gRPC-based proxy tunnel established by the klusterlet agent. | ||
| When configured, the klusterlet agent establishes a gRPC connection to the hub's | ||
| proxy server and proxies incoming HTTP requests to the local managed cluster API server. | ||
| This enables hub-to-spoke API access even when the managed cluster is not directly | ||
| accessible from the hub (e.g., behind a firewall or NAT). | ||
| This feature requires the ClusterProxy feature gate to be enabled and corresponding | ||
| GRPCConfiguration to be set in the ClusterManager on the hub side. | ||
| properties: | ||
| authentications: | ||
| default: | ||
| - userToken | ||
| description: |- | ||
| Authentications defines how the agent authenticates with the cluster. | ||
| If not specified, defaults to ["userToken"]. | ||
| items: | ||
| enum: | ||
| - userToken | ||
| - impersonation | ||
| type: string | ||
| type: array | ||
| grpcEndpoint: | ||
| description: GRPCEndpoint represents the gRPC endpoint configuration | ||
| for the proxy connection. | ||
| properties: | ||
| grpc: | ||
| description: grpc represents the configuration for grpc endpoint. | ||
| properties: | ||
| hostname: | ||
| description: hostname points to a fixed hostname for serving | ||
| agents' handshakes. | ||
| properties: | ||
| caBundle: | ||
| description: caBundle of the endpoint. | ||
| format: byte | ||
| type: string | ||
| host: | ||
| description: host is the host name of the endpoint. | ||
| type: string | ||
| required: | ||
| - host | ||
| type: object | ||
| type: | ||
| default: hostname | ||
| description: |- | ||
| type specifies how the endpoint is exposed. | ||
| You may need to apply an object to expose the endpoint, for example: a route. | ||
| enum: | ||
| - hostname | ||
| type: string | ||
| required: | ||
| - type | ||
| type: object | ||
| https: | ||
| description: https represents the configuration for https | ||
| endpoint. | ||
| properties: | ||
| hostname: | ||
| description: hostname points to a fixed hostname for serving | ||
| agents' handshakes. | ||
| properties: | ||
| caBundle: | ||
| description: caBundle of the endpoint. | ||
| format: byte | ||
| type: string | ||
| host: | ||
| description: host is the host name of the endpoint. | ||
| type: string | ||
| required: | ||
| - host | ||
| type: object | ||
| type: | ||
| default: hostname | ||
| description: |- | ||
| type specifies how the endpoint is exposed. | ||
| You may need to apply an object to expose the endpoint, for example: a route. | ||
| enum: | ||
| - hostname | ||
| type: string | ||
| required: | ||
| - type | ||
| type: object | ||
| protocol: | ||
| default: grpc | ||
| description: protocol is the protocol used for the endpoint, | ||
| could be https or grpc. | ||
| enum: | ||
| - grpc | ||
| - https | ||
| type: string | ||
| usage: | ||
| description: |- | ||
| usage defines the usage of the endpoint. It could be "agentToHub" indicating the endpoint is used | ||
| for communication between agent and hub, or "consumer" indicating the endpoint is used for external consumer. | ||
| type: string | ||
| required: | ||
| - protocol | ||
| type: object | ||
| required: | ||
| - grpcEndpoint | ||
| type: object |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Harden schema: enforce grpc/https exclusivity, add list uniqueness, and validate usage.
Without cross-field rules, invalid configs (e.g., protocol=grpc but only https set, or both set) will pass CRD validation and fail at runtime. Also, authentications can duplicate entries, and usage is free-form.
Suggested CRD edits:
- Enforce exactly-one of grpc/https matching protocol using CEL.
- Make authentications a set and bound its size.
- Constrain usage to known values and set a default.
- Tighten hostname.host to FQDN pattern and length.
proxyConfig:
properties:
authentications:
items:
enum:
- userToken
- impersonation
type: string
type: array
+ x-kubernetes-list-type: set
+ minItems: 1
+ maxItems: 2
grpcEndpoint:
description: GRPCEndpoint represents the gRPC endpoint configuration
for the proxy connection.
properties:
grpc:
properties:
hostname:
properties:
caBundle:
format: byte
type: string
host:
description: host is the host name of the endpoint.
- type: string
+ type: string
+ maxLength: 253
+ pattern: ^(([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$
...
https:
properties:
hostname:
properties:
caBundle:
format: byte
type: string
host:
- type: string
+ type: string
+ maxLength: 253
+ pattern: ^(([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$
...
protocol:
default: grpc
enum:
- grpc
- https
type: string
usage:
- description: |-
+ description: |-
usage defines the usage of the endpoint. It could be "agentToHub" indicating the endpoint is used
for communication between agent and hub, or "consumer" indicating the endpoint is used for external consumer.
- type: string
+ type: string
+ enum:
+ - agentToHub
+ - consumer
+ default: agentToHub
required:
- protocol
type: object
+ x-kubernetes-validations:
+ - rule: "self.protocol == 'grpc' ? has(self.grpc) : has(self.https)"
+ message: "Provide grpc when protocol=grpc, and https when protocol=https."
+ - rule: "!(has(self.grpc) && has(self.https))"
+ message: "Specify only one of grpc or https."Run to confirm validations materialize after regeneration:
🏁 Script executed:
#!/usr/bin/env bash
# After applying Go marker changes + regen, check CRD for new validations.
fd 'klusterlets.crd.yaml' -a | xargs -I{} rg -n 'x-kubernetes-validations|x-kubernetes-list-type|enum:\s+- agentToHub' {}Length of output: 55
I need to verify the current state of the schema before finalizing the rewritten review comment. Let me check the actual content of the CRD file.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the actual current state of authentications, usage, and grpcEndpoint fields
fd 'klusterlets.crd.yaml' -a | head -1 | xargs -I{} cat {} | grep -A 50 "authentications:" | head -60
Length of output: 2412
🏁 Script executed:
#!/bin/bash
# Check for usage field and its current definition
fd 'klusterlets.crd.yaml' -a | head -1 | xargs -I{} rg -A 5 -B 2 '^\s+usage:' {}Length of output: 464
🏁 Script executed:
#!/bin/bash
# Check grpcEndpoint structure for grpc/https fields
fd 'klusterlets.crd.yaml' -a | head -1 | xargs -I{} rg -n 'grpc:|https:' {} | head -20Length of output: 550
🏁 Script executed:
#!/bin/bash
# Check for any x-kubernetes-validations already present in the file
fd 'klusterlets.crd.yaml' -a | head -1 | xargs -I{} rg -B 3 -A 3 'x-kubernetes-validations' {}Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check hostname.host field for patterns/constraints
fd 'klusterlets.crd.yaml' -a | head -1 | xargs -I{} rg -A 3 'host is the host name' {}Length of output: 406
🏁 Script executed:
#!/bin/bash
# Count occurrences of grpc and https at the same level (both present?)
fd 'klusterlets.crd.yaml' -a | head -1 | xargs -I{} rg -n 'properties:' {} | grep -i grpcLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look at grpcEndpoint properties structure
fd 'klusterlets.crd.yaml' -a | head -1 | xargs -I{} sed -n '184,289p' {}Length of output: 5270
CRD is missing critical validation rules: enforce protocol-endpoint matching, add authentications list constraints, validate usage values, and constrain hostname format.
The schema currently lacks cross-field validation and type constraints that allow invalid configurations:
- grpcEndpoint: Both grpc and https are optional; no rule ensures exactly one matches protocol (e.g., protocol=grpc but only https defined will pass CRD validation and fail at runtime).
- authentications: Unconstrained array—duplicates are allowed, and no size bounds.
- usage: Free-form string with no enum; accepts any value despite documentation indicating only "agentToHub" or "consumer" are valid.
- hostname.host: No FQDN pattern or length validation.
Apply the following to harden the schema:
proxyConfig:
properties:
authentications:
items:
enum:
- userToken
- impersonation
type: string
type: array
+ x-kubernetes-list-type: set
+ minItems: 1
+ maxItems: 2
grpcEndpoint:
description: GRPCEndpoint represents the gRPC endpoint configuration
for the proxy connection.
properties:
grpc:
properties:
hostname:
properties:
caBundle:
format: byte
type: string
host:
description: host is the host name of the endpoint.
type: string
+ maxLength: 253
+ pattern: ^(([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$
https:
properties:
hostname:
properties:
caBundle:
format: byte
type: string
host:
description: host is the host name of the endpoint.
type: string
+ maxLength: 253
+ pattern: ^(([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$
protocol:
default: grpc
enum:
- grpc
- https
type: string
usage:
description: |-
usage defines the usage of the endpoint. It could be "agentToHub" indicating the endpoint is used
for communication between agent and hub, or "consumer" indicating the endpoint is used for external consumer.
type: string
+ enum:
+ - agentToHub
+ - consumer
+ default: agentToHub
required:
- protocol
type: object
+ x-kubernetes-validations:
+ - rule: "self.protocol == 'grpc' ? has(self.grpc) : has(self.https)"
+ message: "Provide grpc when protocol=grpc, and https when protocol=https."
+ - rule: "!(has(self.grpc) && has(self.https))"
+ message: "Specify only one of grpc or https."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| proxyConfig: | |
| description: |- | |
| ProxyConfig holds the configuration for enabling klusterlet-proxy functionality, | |
| which allows the hub cluster to access the managed cluster's API server through | |
| a gRPC-based proxy tunnel established by the klusterlet agent. | |
| When configured, the klusterlet agent establishes a gRPC connection to the hub's | |
| proxy server and proxies incoming HTTP requests to the local managed cluster API server. | |
| This enables hub-to-spoke API access even when the managed cluster is not directly | |
| accessible from the hub (e.g., behind a firewall or NAT). | |
| This feature requires the ClusterProxy feature gate to be enabled and corresponding | |
| GRPCConfiguration to be set in the ClusterManager on the hub side. | |
| properties: | |
| authentications: | |
| default: | |
| - userToken | |
| description: |- | |
| Authentications defines how the agent authenticates with the cluster. | |
| If not specified, defaults to ["userToken"]. | |
| items: | |
| enum: | |
| - userToken | |
| - impersonation | |
| type: string | |
| type: array | |
| grpcEndpoint: | |
| description: GRPCEndpoint represents the gRPC endpoint configuration | |
| for the proxy connection. | |
| properties: | |
| grpc: | |
| description: grpc represents the configuration for grpc endpoint. | |
| properties: | |
| hostname: | |
| description: hostname points to a fixed hostname for serving | |
| agents' handshakes. | |
| properties: | |
| caBundle: | |
| description: caBundle of the endpoint. | |
| format: byte | |
| type: string | |
| host: | |
| description: host is the host name of the endpoint. | |
| type: string | |
| required: | |
| - host | |
| type: object | |
| type: | |
| default: hostname | |
| description: |- | |
| type specifies how the endpoint is exposed. | |
| You may need to apply an object to expose the endpoint, for example: a route. | |
| enum: | |
| - hostname | |
| type: string | |
| required: | |
| - type | |
| type: object | |
| https: | |
| description: https represents the configuration for https | |
| endpoint. | |
| properties: | |
| hostname: | |
| description: hostname points to a fixed hostname for serving | |
| agents' handshakes. | |
| properties: | |
| caBundle: | |
| description: caBundle of the endpoint. | |
| format: byte | |
| type: string | |
| host: | |
| description: host is the host name of the endpoint. | |
| type: string | |
| required: | |
| - host | |
| type: object | |
| type: | |
| default: hostname | |
| description: |- | |
| type specifies how the endpoint is exposed. | |
| You may need to apply an object to expose the endpoint, for example: a route. | |
| enum: | |
| - hostname | |
| type: string | |
| required: | |
| - type | |
| type: object | |
| protocol: | |
| default: grpc | |
| description: protocol is the protocol used for the endpoint, | |
| could be https or grpc. | |
| enum: | |
| - grpc | |
| - https | |
| type: string | |
| usage: | |
| description: |- | |
| usage defines the usage of the endpoint. It could be "agentToHub" indicating the endpoint is used | |
| for communication between agent and hub, or "consumer" indicating the endpoint is used for external consumer. | |
| type: string | |
| required: | |
| - protocol | |
| type: object | |
| required: | |
| - grpcEndpoint | |
| type: object | |
| proxyConfig: | |
| description: |- | |
| ProxyConfig holds the configuration for enabling klusterlet-proxy functionality, | |
| which allows the hub cluster to access the managed cluster's API server through | |
| a gRPC-based proxy tunnel established by the klusterlet agent. | |
| When configured, the klusterlet agent establishes a gRPC connection to the hub's | |
| proxy server and proxies incoming HTTP requests to the local managed cluster API server. | |
| This enables hub-to-spoke API access even when the managed cluster is not directly | |
| accessible from the hub (e.g., behind a firewall or NAT). | |
| This feature requires the ClusterProxy feature gate to be enabled and corresponding | |
| GRPCConfiguration to be set in the ClusterManager on the hub side. | |
| properties: | |
| authentications: | |
| default: | |
| - userToken | |
| description: |- | |
| Authentications defines how the agent authenticates with the cluster. | |
| If not specified, defaults to ["userToken"]. | |
| items: | |
| enum: | |
| - userToken | |
| - impersonation | |
| type: string | |
| type: array | |
| x-kubernetes-list-type: set | |
| minItems: 1 | |
| maxItems: 2 | |
| grpcEndpoint: | |
| description: GRPCEndpoint represents the gRPC endpoint configuration | |
| for the proxy connection. | |
| properties: | |
| grpc: | |
| description: grpc represents the configuration for grpc endpoint. | |
| properties: | |
| hostname: | |
| description: hostname points to a fixed hostname for serving | |
| agents' handshakes. | |
| properties: | |
| caBundle: | |
| description: caBundle of the endpoint. | |
| format: byte | |
| type: string | |
| host: | |
| description: host is the host name of the endpoint. | |
| type: string | |
| maxLength: 253 | |
| pattern: ^(([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$ | |
| required: | |
| - host | |
| type: object | |
| type: | |
| default: hostname | |
| description: |- | |
| type specifies how the endpoint is exposed. | |
| You may need to apply an object to expose the endpoint, for example: a route. | |
| enum: | |
| - hostname | |
| type: string | |
| required: | |
| - type | |
| type: object | |
| https: | |
| description: https represents the configuration for https | |
| endpoint. | |
| properties: | |
| hostname: | |
| description: hostname points to a fixed hostname for serving | |
| agents' handshakes. | |
| properties: | |
| caBundle: | |
| description: caBundle of the endpoint. | |
| format: byte | |
| type: string | |
| host: | |
| description: host is the host name of the endpoint. | |
| type: string | |
| maxLength: 253 | |
| pattern: ^(([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$ | |
| required: | |
| - host | |
| type: object | |
| type: | |
| default: hostname | |
| description: |- | |
| type specifies how the endpoint is exposed. | |
| You may need to apply an object to expose the endpoint, for example: a route. | |
| enum: | |
| - hostname | |
| type: string | |
| required: | |
| - type | |
| type: object | |
| protocol: | |
| default: grpc | |
| description: protocol is the protocol used for the endpoint, | |
| could be https or grpc. | |
| enum: | |
| - grpc | |
| - https | |
| type: string | |
| usage: | |
| description: |- | |
| usage defines the usage of the endpoint. It could be "agentToHub" indicating the endpoint is used | |
| for communication between agent and hub, or "consumer" indicating the endpoint is used for external consumer. | |
| type: string | |
| enum: | |
| - agentToHub | |
| - consumer | |
| default: agentToHub | |
| required: | |
| - protocol | |
| type: object | |
| x-kubernetes-validations: | |
| - rule: "self.protocol == 'grpc' ? has(self.grpc) : has(self.https)" | |
| message: "Provide grpc when protocol=grpc, and https when protocol=https." | |
| - rule: "!(has(self.grpc) && has(self.https))" | |
| message: "Specify only one of grpc or https." | |
| required: | |
| - grpcEndpoint | |
| type: object |
| // ProxyConfig holds the configuration for enabling klusterlet-proxy functionality, | ||
| // which allows the hub cluster to access the managed cluster's API server through | ||
| // a gRPC-based proxy tunnel established by the klusterlet agent. | ||
| // | ||
| // When configured, the klusterlet agent establishes a gRPC connection to the hub's | ||
| // proxy server and proxies incoming HTTP requests to the local managed cluster API server. | ||
| // This enables hub-to-spoke API access even when the managed cluster is not directly | ||
| // accessible from the hub (e.g., behind a firewall or NAT). | ||
| // | ||
| // This feature requires the ClusterProxy feature gate to be enabled and corresponding | ||
| // GRPCConfiguration to be set in the ClusterManager on the hub side. | ||
| // +optional | ||
| ProxyConfig *ProxyConfig `json:"proxyConfig,omitempty"` |
There was a problem hiding this comment.
Doc fix: reference correct hub-side config and protocol options.
Replace “GRPCConfiguration” (type not present) with the actual hub-side field and reflect that protocol can be grpc or https.
-// This feature requires the ClusterProxy feature gate to be enabled and corresponding
-// GRPCConfiguration to be set in the ClusterManager on the hub side.
+// This feature requires the ClusterProxy feature gate to be enabled and the hub's
+// ServerConfiguration.EndpointsExposure to publish the proxy endpoint (protocol grpc or https)
+// in the ClusterManager.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // ProxyConfig holds the configuration for enabling klusterlet-proxy functionality, | |
| // which allows the hub cluster to access the managed cluster's API server through | |
| // a gRPC-based proxy tunnel established by the klusterlet agent. | |
| // | |
| // When configured, the klusterlet agent establishes a gRPC connection to the hub's | |
| // proxy server and proxies incoming HTTP requests to the local managed cluster API server. | |
| // This enables hub-to-spoke API access even when the managed cluster is not directly | |
| // accessible from the hub (e.g., behind a firewall or NAT). | |
| // | |
| // This feature requires the ClusterProxy feature gate to be enabled and corresponding | |
| // GRPCConfiguration to be set in the ClusterManager on the hub side. | |
| // +optional | |
| ProxyConfig *ProxyConfig `json:"proxyConfig,omitempty"` | |
| // ProxyConfig holds the configuration for enabling klusterlet-proxy functionality, | |
| // which allows the hub cluster to access the managed cluster's API server through | |
| // a gRPC-based proxy tunnel established by the klusterlet agent. | |
| // | |
| // When configured, the klusterlet agent establishes a gRPC connection to the hub's | |
| // proxy server and proxies incoming HTTP requests to the local managed cluster API server. | |
| // This enables hub-to-spoke API access even when the managed cluster is not directly | |
| // accessible from the hub (e.g., behind a firewall or NAT). | |
| // | |
| // This feature requires the ClusterProxy feature gate to be enabled and the hub's | |
| // ServerConfiguration.EndpointsExposure to publish the proxy endpoint (protocol grpc or https) | |
| // in the ClusterManager. | |
| // +optional | |
| ProxyConfig *ProxyConfig `json:"proxyConfig,omitempty"` |
🤖 Prompt for AI Agents
In operator/v1/types_klusterlet.go around lines 104 to 116, the comment
incorrectly references a non-existent "GRPCConfiguration" and omits supported
protocols; update the text to reference the actual hub-side field
(ClusterManager.Spec.ProxyConfig or the correct field name used on the hub) and
note that the protocol may be "grpc" or "https"; change the sentence to mention
the ClusterProxy feature gate and the hub ClusterManager's ProxyConfig (or exact
hub-side config field) must be set, and add that the proxy protocol can be
either grpc or https.
1 participant
Summary
Related Design: https://github.com/open-cluster-management-io/enhancements/pull/151/files
Summary by CodeRabbit
New Features
Documentation