Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion api/v1/multiclusterengine_methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const (
ClusterManager = "cluster-manager"
ClusterPermission = "cluster-permission"
ClusterProxyAddon = "cluster-proxy-addon"
ClusterProxy = "cluster-proxy"
ConsoleMCE = "console-mce"
Comment on lines +42 to 43
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Register cluster-proxy as a valid component.

Adding the constant/CRD dir isn't enough on its own: ClusterProxy still isn't present in AllComponents or MCEComponents, so validation and any component-gated logic will continue to reject it. If ClusterProxyAddon must remain for compatibility, keep both entries.

🛠️ Suggested fix
 var AllComponents = []string{
   AssistedService,
   ClusterAPI,
   ClusterAPIPreview,
   ClusterAPIProviderAWS,
   ClusterAPIProviderAWSPreview,
   // ClusterAPIProviderAzure, Uncomment until stable release is available
   ClusterAPIProviderAzurePreview,
   ClusterAPIProviderMetal,
   ClusterAPIProviderMetalPreview,
   ClusterAPIProviderOAPreview,
   ClusterAPIProviderOA,
   ClusterLifecycle,
   ClusterManager,
   ClusterPermission,
   ClusterProxyAddon,
+  ClusterProxy,
   ConsoleMCE,
   Discovery,
   Hive,
   HyperShift,
@@
 var MCEComponents = []string{
   AssistedService,
   ClusterAPI,
   ClusterAPIProviderAWS,
   // ClusterAPIProviderAzure, Uncomment until stable release is available
   ClusterAPIProviderAzurePreview,
   ClusterAPIProviderMetal,
   ClusterAPIProviderOA,
   ClusterLifecycle,
   ClusterManager,
   ClusterPermission,
   ClusterProxyAddon,
+  ClusterProxy,
   ConsoleMCE,
   Discovery,
   Hive,
   HyperShift,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/v1/multiclusterengine_methods.go` around lines 42 - 43, The new
ClusterProxy constant is declared but not added to the component lists used for
validation/feature gating; update the collections that enumerate supported
components by adding "ClusterProxy" (and if needed keep the legacy
"ClusterProxyAddon") to AllComponents and to MCEComponents so the component is
recognized by validation and component-gated logic; locate the component
registry variables/arrays (e.g., AllComponents, MCEComponents) and append
ClusterProxy where other components like ConsoleMCE are listed.

Discovery = "discovery"
Hive = "hive"
Expand Down Expand Up @@ -68,7 +69,8 @@ const (
ClusterLifecycleCRDDir = "cluster-lifecycle"
ClusterManagerCRDDir = "cluster-manager"
ClusterPermissionCRDDir = "cluster-permission"
ClusterProxyAddonCRDDir = "cluster-proxy-addon"
ClusterProxyAddonCRDDir = "cluster-proxy-addon" // Deprecated: Moved to ClusterProxy in MCE 2.11
ClusterProxyCRDDir = "cluster-proxy"
DiscoveryCRDDir = "discovery-operator"
HiveCRDDir = "hive-operator"
ImageBasedInstallOperatorCRDDir = "image-based-install-operator"
Expand Down
18 changes: 11 additions & 7 deletions controllers/backplaneconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1003,6 +1003,7 @@ func (r *MultiClusterEngineReconciler) fetchChartOrCRDPath(component string) str
backplanev1.ClusterManager: toggle.ClusterManagerChartDir,
backplanev1.ClusterPermission: toggle.ClusterPermissionChartDir,
backplanev1.ClusterProxyAddon: toggle.ClusterProxyAddonDir,
backplanev1.ClusterProxy: toggle.ClusterProxyDir,
backplanev1.ConsoleMCE: toggle.ConsoleMCEChartsDir,
backplanev1.Discovery: toggle.DiscoveryChartDir,
backplanev1.Hive: toggle.HiveChartDir,
Expand Down Expand Up @@ -1284,26 +1285,29 @@ func (r *MultiClusterEngineReconciler) ensureToggleableComponents(ctx context.Co
log.Info(messages.SkippingExternallyManaged, "component", backplanev1.ServerFoundation)
}

if !r.isComponentExternallyManaged(backplaneConfig, backplanev1.ClusterProxyAddon) {
if backplaneConfig.Enabled(backplanev1.ClusterProxyAddon) && foundation.CanInstallAddons(ctx, r.Client) {
result, err = r.ensureClusterProxyAddon(ctx, backplaneConfig)
// Handle ClusterProxyAddon (deprecated) and ClusterProxy - both use same chart
if !r.isComponentExternallyManaged(backplaneConfig, backplanev1.ClusterProxyAddon) ||
!r.isComponentExternallyManaged(backplaneConfig, backplanev1.ClusterProxy) {
if (backplaneConfig.Enabled(backplanev1.ClusterProxyAddon) || backplaneConfig.Enabled(backplanev1.ClusterProxy)) &&
foundation.CanInstallAddons(ctx, r.Client) {
result, err = r.ensureClusterProxy(ctx, backplaneConfig)
if result != (ctrl.Result{}) {
requeue = true
}
if err != nil {
errs[backplanev1.ClusterProxyAddon] = err
errs[backplanev1.ClusterProxy] = err
}
} else {
result, err = r.ensureNoClusterProxyAddon(ctx, backplaneConfig)
result, err = r.ensureNoClusterProxy(ctx, backplaneConfig)
if result != (ctrl.Result{}) {
requeue = true
}
if err != nil {
errs[backplanev1.ClusterProxyAddon] = err
errs[backplanev1.ClusterProxy] = err
}
}
} else {
log.Info(messages.SkippingExternallyManaged, "component", backplanev1.ClusterProxyAddon)
log.Info(messages.SkippingExternallyManaged, "component", backplanev1.ClusterProxy)
}
Comment on lines +1288 to 1311
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Treat the proxy aliases as a single external-management decision.

Using || means this block still reconciles when only one of the two names is marked externally managed, which can recreate/delete the same shared resources anyway. Please skip reconciliation unless neither alias is external, and mirror the same alias handling in finalizer cleanup.

🔧 Suggested fix
-	if !r.isComponentExternallyManaged(backplaneConfig, backplanev1.ClusterProxyAddon) ||
-		!r.isComponentExternallyManaged(backplaneConfig, backplanev1.ClusterProxy) {
+	if !r.isComponentExternallyManaged(backplaneConfig, backplanev1.ClusterProxyAddon) &&
+		!r.isComponentExternallyManaged(backplaneConfig, backplanev1.ClusterProxy) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@controllers/backplaneconfig_controller.go` around lines 1288 - 1311, The
external-management check currently uses || with negations which allows
reconciliation when only one alias is externally managed; change the condition
in the ClusterProxy reconciliation block to require both aliases be not
externally managed (use && between the two !r.isComponentExternallyManaged(...)
calls) so reconciliation runs only when neither backplanev1.ClusterProxyAddon
nor backplanev1.ClusterProxy is externally managed; apply the same logic to the
finalizer/cleanup branch so the finalizer only acts when neither alias is
externally managed, and keep calls to ensureClusterProxy and
ensureNoClusterProxy unchanged except for their guarded condition.


if !r.isComponentExternallyManaged(backplaneConfig, backplanev1.ClusterAPI) {
Expand Down
14 changes: 7 additions & 7 deletions controllers/toggle_components.go
Original file line number Diff line number Diff line change
Expand Up @@ -1686,7 +1686,7 @@ func (r *MultiClusterEngineReconciler) removeHypershiftLocalHosting(ctx context.
return ctrl.Result{}, nil
}

func (r *MultiClusterEngineReconciler) ensureClusterProxyAddon(ctx context.Context,
func (r *MultiClusterEngineReconciler) ensureClusterProxy(ctx context.Context,
mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) {

namespacedName := types.NamespacedName{Name: "cluster-proxy-addon-manager", Namespace: mce.Spec.TargetNamespace}
Expand All @@ -1700,12 +1700,12 @@ func (r *MultiClusterEngineReconciler) ensureClusterProxyAddon(ctx context.Conte
r.StatusManager.RemoveComponent(toggle.DisabledStatus(namespacedName, []*unstructured.Unstructured{}))

// Ensure that the InternalHubComponent CR instance is created for component in MCE.
if result, err := r.ensureInternalEngineComponent(ctx, mce, backplanev1.ClusterProxyAddon); err != nil {
if result, err := r.ensureInternalEngineComponent(ctx, mce, backplanev1.ClusterProxy); err != nil {
return result, err
}

// Renders all templates from charts
chartPath := r.fetchChartOrCRDPath(backplanev1.ClusterProxyAddon)
chartPath := r.fetchChartOrCRDPath(backplanev1.ClusterProxy)
templates, errs := renderer.RenderChart(chartPath, mce, r.CacheSpec.ImageOverrides, r.CacheSpec.TemplateOverrides)

if len(errs) > 0 {
Expand All @@ -1716,7 +1716,7 @@ func (r *MultiClusterEngineReconciler) ensureClusterProxyAddon(ctx context.Conte
}

// Apply deployment config overrides
if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterProxyAddon); err != nil {
if result, err := r.applyComponentDeploymentOverrides(mce, templates, backplanev1.ClusterProxy); err != nil {
return result, err
}

Expand All @@ -1741,7 +1741,7 @@ func (r *MultiClusterEngineReconciler) ensureClusterProxyAddon(ctx context.Conte
return ctrl.Result{}, nil
}

func (r *MultiClusterEngineReconciler) ensureNoClusterProxyAddon(ctx context.Context,
func (r *MultiClusterEngineReconciler) ensureNoClusterProxy(ctx context.Context,
mce *backplanev1.MultiClusterEngine) (ctrl.Result, error) {

namespacedName := types.NamespacedName{Name: "cluster-proxy-addon-manager", Namespace: mce.Spec.TargetNamespace}
Expand All @@ -1756,12 +1756,12 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterProxyAddon(ctx context.Con

// Ensure that the InternalHubComponent CR instance is deleted for component in MCE.
if result, err := r.ensureNoInternalEngineComponent(ctx, mce,
backplanev1.ClusterProxyAddon); (result != ctrl.Result{}) || err != nil {
backplanev1.ClusterProxy); (result != ctrl.Result{}) || err != nil {
return result, err
}

// Renders all templates from charts
chartPath := r.fetchChartOrCRDPath(backplanev1.ClusterProxyAddon)
chartPath := r.fetchChartOrCRDPath(backplanev1.ClusterProxy)
templates, errs := renderer.RenderChart(chartPath, mce, r.CacheSpec.ImageOverrides, r.CacheSpec.TemplateOverrides)

if len(errs) > 0 {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Enable service proxy to generate user-deployment and user-service
enableServiceProxy: true

# Number of replicas
replicas: 1

# Image registry
registry: quay.io/open-cluster-management
image: cluster-proxy
tag:

# Enable kube-api-proxy
enableKubeApiProxy: true

# Enable impersonation
enableImpersonation: true
14 changes: 14 additions & 0 deletions hack/bundle-automation/charts-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,17 @@ components:
- "CLUSTER_NAME"
- "HUB_KUBECONFIG"
- "INSTALL_NAMESPACE"

- repo_name: "cluster-proxy"
github_ref: "https://github.com/stolostron/cluster-proxy.git"
branch: "backplane-5.0"
charts:
- name: "cluster-proxy"
chart-path: "charts/cluster-proxy"
always-or-toggle: "toggle"
imageMappings:
cluster-proxy: cluster_proxy
inclusions:
- "pullSecretOverride"
skipRBACOverrides: true
updateChartVersion: true # the chart version will be retrieved from trimmed branch name, e.g. backplane-2.6 -> 2.6
6 changes: 6 additions & 0 deletions pkg/templates/charts/toggle/cluster-proxy/Chart.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
apiVersion: v2
appVersion: 1.1.0
description: A Helm chart for Cluster-Proxy OCM Addon
name: cluster-proxy
type: application
version: '5.0'
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
apiVersion: apps/v1
kind: Deployment
metadata:
labels:
component: cluster-proxy-manager
name: cluster-proxy-addon-manager
spec:
replicas: 1
selector:
matchLabels:
component: cluster-proxy-manager
open-cluster-management.io/addon: cluster-proxy
template:
metadata:
labels:
component: cluster-proxy-manager
ocm-antiaffinity-selector: cluster-proxy-addon-manager
open-cluster-management.io/addon: cluster-proxy
spec:
affinity:
podAntiAffinity:
preferredDuringSchedulingIgnoredDuringExecution:
- podAffinityTerm:
labelSelector:
matchExpressions:
- key: ocm-antiaffinity-selector
operator: In
values:
- cluster-proxy-addon-manager
topologyKey: topology.kubernetes.io/zone
weight: 70
- podAffinityTerm:
labelSelector:
matchExpressions:
- key: ocm-antiaffinity-selector
operator: In
values:
- cluster-proxy-addon-manager
topologyKey: kubernetes.io/hostname
weight: 35
containers:
- args:
- --leader-elect=true
- --signer-secret-namespace={{ .Values.global.namespace }}
- --enable-kube-api-proxy=true
- --enable-service-proxy=true
- --image-pull-policy=IfNotPresent
- --feature-gates=ClusterProfile=false
command:
- /manager
env:
{{- if .Values.global.pullSecret }}
- name: AGENT_IMAGE_PULL_SECRET
value: {{ .Values.global.pullSecret }}
{{- end }}
{{- if .Values.hubconfig.proxyConfigs }}
- name: HTTP_PROXY
value: {{ .Values.hubconfig.proxyConfigs.HTTP_PROXY }}
- name: HTTPS_PROXY
value: {{ .Values.hubconfig.proxyConfigs.HTTPS_PROXY }}
- name: NO_PROXY
value: {{ .Values.hubconfig.proxyConfigs.NO_PROXY }}
{{- end }}
- name: POD_NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
image: '{{ .Values.global.imageOverrides.cluster_proxy }}'
imagePullPolicy: '{{ .Values.global.pullPolicy }}'
Comment on lines +68 to +69
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't template imagePullPolicy from a missing value.

global.pullPolicy is not defined in this chart's values, so Helm will render an empty value here and Kubernetes can reject the PodSpec. Add the missing value to values.yaml or default it inline.

🛠️ Suggested fix
-        imagePullPolicy: '{{ .Values.global.pullPolicy }}'
+        imagePullPolicy: '{{ default "IfNotPresent" .Values.global.pullPolicy }}'
📝 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.

Suggested change
image: '{{ .Values.global.imageOverrides.cluster_proxy }}'
imagePullPolicy: '{{ .Values.global.pullPolicy }}'
image: '{{ .Values.global.imageOverrides.cluster_proxy }}'
imagePullPolicy: '{{ default "IfNotPresent" .Values.global.pullPolicy }}'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@pkg/templates/charts/toggle/cluster-proxy/templates/cluster-proxy-addon-manager-deployment.yaml`
around lines 68 - 69, The chart templates reference .Values.global.pullPolicy in
the cluster-proxy Deployment (imagePullPolicy in
cluster-proxy-addon-manager-deployment.yaml) but that value is not defined,
which can render an empty imagePullPolicy; fix this by either adding a default
pullPolicy under global in the chart values.yaml (e.g., global.pullPolicy:
IfNotPresent) or by defaulting inline in the template (use a fallback when
referencing .Values.global.pullPolicy) so imagePullPolicy always emits a valid
Kubernetes value; update the values.yaml or the template reference accordingly
(look for .Values.global.pullPolicy in
cluster-proxy-addon-manager-deployment.yaml).

name: manager
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
privileged: false
readOnlyRootFilesystem: true
runAsNonRoot: true
hostIPC: false
hostNetwork: false
hostPID: false
{{- if .Values.global.pullSecret }}
imagePullSecrets:
- name: {{ .Values.global.pullSecret }}
{{- end }}
{{- with .Values.hubconfig.nodeSelector }}
nodeSelector:
{{ toYaml . | indent 8 }}
{{- end }}
securityContext:
runAsNonRoot: true
{{- if .Values.global.deployOnOCP }}
{{- if semverCompare ">=4.11.0" .Values.hubconfig.ocpVersion }}
seccompProfile:
type: RuntimeDefault
{{- end }}
{{- end }}
serviceAccount: cluster-proxy
{{- with .Values.hubconfig.tolerations }}
tolerations:
{{- range . }}
- {{ if .Key }} key: {{ .Key }} {{- end }}
{{ if .Operator }} operator: {{ .Operator }} {{- end }}
{{ if .Value }} value: {{ .Value }} {{- end }}
{{ if .Effect }} effect: {{ .Effect }} {{- end }}
{{ if .TolerationSeconds }} tolerationSeconds: {{ .TolerationSeconds }} {{- end }}
{{- end }}
{{- end }}
Loading
Loading