From 442cb4edba7bf6e46845af965030636f24791fd6 Mon Sep 17 00:00:00 2001 From: Nico Andres Date: Mon, 13 Jul 2026 11:29:48 +0200 Subject: [PATCH 1/6] Add host override to localAccessReconciler On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres --- .../clusteraccess/clusteraccess.go | 9 +++++ .../clusteraccess/localaccess_advanced.go | 37 ++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/pkg/serviceprovider/clusteraccess/clusteraccess.go b/pkg/serviceprovider/clusteraccess/clusteraccess.go index 2245454..8895874 100644 --- a/pkg/serviceprovider/clusteraccess/clusteraccess.go +++ b/pkg/serviceprovider/clusteraccess/clusteraccess.go @@ -71,6 +71,11 @@ func (a *simpleProviderAdapter) Access(ctx context.Context, request reconcile.Re } } +// Cluster implements [AdvancedProvider]. The legacy Provider has no access to the platform Cluster CR. +func (a *simpleProviderAdapter) Cluster(_ context.Context, _ reconcile.Request, _ string, _ ...any) (*clustersv1alpha1.Cluster, error) { + return nil, nil +} + // AccessRequest implements [AdvancedProvider]. func (a *simpleProviderAdapter) AccessRequest(ctx context.Context, request reconcile.Request, id string, _ ...any) (*clustersv1alpha1.AccessRequest, error) { switch id { @@ -98,6 +103,10 @@ type AdvancedProvider interface { // Access returns an internal Cluster object granting access to the cluster for the specified request with the specified id. // Will fail if the cluster is not registered or no AccessRequest is registered for the cluster, or if some other error occurs. Access(ctx context.Context, request reconcile.Request, id string, additionalData ...any) (*clusters.Cluster, error) + // Cluster fetches the external Cluster object for the cluster for the specified request with the specified id. + // Will fail if the cluster is not registered or no Cluster can be determined, or if some other error occurs. + // The same additionalData must be passed into all methods of this ClusterAccessReconciler for the same request and id. + Cluster(ctx context.Context, request reconcile.Request, id string, additionalData ...any) (*clustersv1alpha1.Cluster, error) // AccessRequest fetches the AccessRequest object for the cluster for the specified request with the specified id. // Will fail if the cluster is not registered or no AccessRequest is registered for the cluster, or if some other error occurs. // The same additionalData must be passed into all methods of this ClusterAccessReconciler for the same request and id. diff --git a/pkg/serviceprovider/clusteraccess/localaccess_advanced.go b/pkg/serviceprovider/clusteraccess/localaccess_advanced.go index 0483a19..1a5e9e7 100644 --- a/pkg/serviceprovider/clusteraccess/localaccess_advanced.go +++ b/pkg/serviceprovider/clusteraccess/localaccess_advanced.go @@ -2,9 +2,11 @@ package clusteraccess import ( "context" + "fmt" "time" "github.com/openmcp-project/controller-utils/pkg/clusters" + clustersv1alpha1 "github.com/openmcp-project/openmcp-operator/api/clusters/v1alpha1" "github.com/openmcp-project/openmcp-operator/lib/clusteraccess/advanced" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -16,15 +18,23 @@ var _ advanced.ClusterAccessReconciler = &localAdvancedClusterAccessReconciler{} // instead of the wrapped reconciler. type localAdvancedClusterAccessReconciler struct { advanced.ClusterAccessReconciler + withWorkload bool } // NewLocalAdvancedClusterAccessReconciler returns a local advanced cluster access reconciler that wraps the given advanced cluster access reconciler. -func NewLocalAdvancedClusterAccessReconciler(car advanced.ClusterAccessReconciler) advanced.ClusterAccessReconciler { +func NewLocalAdvancedClusterAccessReconciler(car advanced.ClusterAccessReconciler) *localAdvancedClusterAccessReconciler { return &localAdvancedClusterAccessReconciler{ ClusterAccessReconciler: car, } } +// WithWorkloadCluster configures the reconciler to additionally patch the MCP RESTConfig.Host with the +// "apiserver-internal" endpoint address, suitable for injection into pod env vars via the service provider. +func (s *localAdvancedClusterAccessReconciler) WithWorkloadCluster() *localAdvancedClusterAccessReconciler { + s.withWorkload = true + return s +} + // Access implements [advanced.ClusterAccessReconciler]. func (s *localAdvancedClusterAccessReconciler) Access(ctx context.Context, request reconcile.Request, id string, additionalData ...any) (*clusters.Cluster, error) { cluster, err := s.ClusterAccessReconciler.Access(ctx, request, id, additionalData...) @@ -35,7 +45,30 @@ func (s *localAdvancedClusterAccessReconciler) Access(ctx context.Context, reque if err != nil { return cluster, err } - return MustPatchClusterClient(ctx, ar, cluster), nil + // Always patch the cluster client with the host value of the local AR annotation so that the service provider process can connect. + cluster = MustPatchClusterClient(ctx, ar, cluster) + + // If the service provider is using a workload cluster we additionally have to override the MCPs rest.Config.Host to the Docker-network address fetched from the "apiserver-internal" endpoint of the Cluster. + // Using this endpoint, the pod running on the workload cluster can reach the MCP API server if injected as KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT env vars. + // If we would not override it the rest.Config.Host would point to localhost. + // Warning: This does not affect the cluster client as we only initialize it in MustPatchClusterClient. As a result the rest.Config points to a different host than the cluster client! + if id == MCPClusterID && s.withWorkload && cluster.HasRESTConfig() { + mcpCluster, err := s.Cluster(ctx, request, id, additionalData...) + if err != nil { + return cluster, err + } + if mcpCluster == nil { + return cluster, fmt.Errorf("MCP cluster not found") + } + internalURL, ok := mcpCluster.Status.Endpoints.Get(clustersv1alpha1.APISERVER_ENDPOINT_INTERNAL) + if !ok { + return nil, fmt.Errorf("%s endpoint not found", clustersv1alpha1.APISERVER_ENDPOINT_INTERNAL) + } + cfg := *cluster.RESTConfig() + cfg.Host = internalURL + cluster.WithRESTConfig(&cfg) + } + return cluster, nil } // Register implements [advanced.ClusterAccessReconciler]. From e552b6cb2affabbde04836505e958b7538a4ed16 Mon Sep 17 00:00:00 2001 From: Nico Andres Date: Mon, 13 Jul 2026 16:55:09 +0200 Subject: [PATCH 2/6] Add test On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres --- pkg/serviceprovider/apireconciler_test.go | 5 + .../clusteraccess/localaccess_advanced.go | 7 +- .../localaccess_advanced_test.go | 103 +++++++++++++++++- 3 files changed, 107 insertions(+), 8 deletions(-) diff --git a/pkg/serviceprovider/apireconciler_test.go b/pkg/serviceprovider/apireconciler_test.go index 4be0b4a..eca4e57 100644 --- a/pkg/serviceprovider/apireconciler_test.go +++ b/pkg/serviceprovider/apireconciler_test.go @@ -467,6 +467,11 @@ type FakeAdvancedClusterAccessProvider struct { accessRequests map[string]*clustersv1alpha1.AccessRequest } +// Cluster implements [AdvancedClusterAccessProvider]. +func (f FakeAdvancedClusterAccessProvider) Cluster(_ context.Context, _ reconcile.Request, _ string, _ ...any) (*clustersv1alpha1.Cluster, error) { + return nil, nil +} + // Access implements [AdvancedClusterAccessProvider]. func (f FakeAdvancedClusterAccessProvider) Access(_ context.Context, _ reconcile.Request, id string, _ ...any) (*clusters.Cluster, error) { return f.clusters[id], nil diff --git a/pkg/serviceprovider/clusteraccess/localaccess_advanced.go b/pkg/serviceprovider/clusteraccess/localaccess_advanced.go index 1a5e9e7..5d2091b 100644 --- a/pkg/serviceprovider/clusteraccess/localaccess_advanced.go +++ b/pkg/serviceprovider/clusteraccess/localaccess_advanced.go @@ -28,8 +28,7 @@ func NewLocalAdvancedClusterAccessReconciler(car advanced.ClusterAccessReconcile } } -// WithWorkloadCluster configures the reconciler to additionally patch the MCP RESTConfig.Host with the -// "apiserver-internal" endpoint address, suitable for injection into pod env vars via the service provider. +// WithWorkloadCluster enables patching of the MCP RESTConfig.Host with the "apiserver-internal" endpoint address. func (s *localAdvancedClusterAccessReconciler) WithWorkloadCluster() *localAdvancedClusterAccessReconciler { s.withWorkload = true return s @@ -58,11 +57,11 @@ func (s *localAdvancedClusterAccessReconciler) Access(ctx context.Context, reque return cluster, err } if mcpCluster == nil { - return cluster, fmt.Errorf("MCP cluster not found") + return cluster, fmt.Errorf("mcp cluster not found") } internalURL, ok := mcpCluster.Status.Endpoints.Get(clustersv1alpha1.APISERVER_ENDPOINT_INTERNAL) if !ok { - return nil, fmt.Errorf("%s endpoint not found", clustersv1alpha1.APISERVER_ENDPOINT_INTERNAL) + return cluster, fmt.Errorf("%s endpoint not found", clustersv1alpha1.APISERVER_ENDPOINT_INTERNAL) } cfg := *cluster.RESTConfig() cfg.Host = internalURL diff --git a/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go b/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go index 403191b..e47acf2 100644 --- a/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go +++ b/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go @@ -136,11 +136,106 @@ func Test_advancedLocalAccessProvider_WorkloadCluster(t *testing.T) { } } +func Test_advancedLocalAccessProvider_WithWorkloadCluster(t *testing.T) { + tests := []struct { + name string + ar *clustersv1alpha1.AccessRequest + cluster *clusters.Cluster + withWorkload bool + mcpCluster *clustersv1alpha1.Cluster + wantHost string + wantErr bool + }{ + { + name: "WithWorkloadCluster results in MCP host changed to internalURL", + withWorkload: true, + ar: &clustersv1alpha1.AccessRequest{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{clusterprovider.LocalAccessAnnotation: localAPIServer}, + }, + }, + cluster: createFakeCluster().WithRESTConfig(&rest.Config{Host: localAPIServer}), + mcpCluster: &clustersv1alpha1.Cluster{ + Status: clustersv1alpha1.ClusterStatus{ + Endpoints: clustersv1alpha1.Endpoints{ + { + Name: clustersv1alpha1.APISERVER_ENDPOINT_INTERNAL, + URL: inclusterAPIServer, + }, + }, + }, + }, + wantHost: inclusterAPIServer, + }, + { + name: "Without WithWorkloadCluster results in MCP host patched to local annotation only", + withWorkload: false, + ar: &clustersv1alpha1.AccessRequest{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{clusterprovider.LocalAccessAnnotation: localAPIServer}, + }, + }, + cluster: createFakeCluster().WithRESTConfig(&rest.Config{Host: localAPIServer}), + wantHost: localAPIServer, + }, + { + name: "WithWorkloadCluster and nil mcpCluster results in error", + withWorkload: true, + ar: &clustersv1alpha1.AccessRequest{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{clusterprovider.LocalAccessAnnotation: localAPIServer}, + }, + }, + cluster: createFakeCluster().WithRESTConfig(&rest.Config{Host: localAPIServer}), + wantErr: true, + }, + { + name: "WithWorkloadCluster and missing APISERVER_ENDPOINT_INTERNAL results in error", + withWorkload: true, + ar: &clustersv1alpha1.AccessRequest{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{clusterprovider.LocalAccessAnnotation: localAPIServer}, + }, + }, + cluster: createFakeCluster().WithRESTConfig(&rest.Config{Host: localAPIServer}), + mcpCluster: &clustersv1alpha1.Cluster{ + Status: clustersv1alpha1.ClusterStatus{ + Endpoints: clustersv1alpha1.Endpoints{ + {}, + }, + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeProvider := &fakeAdvancedClusterAccessReconciler{ + clusters: map[string]*clusters.Cluster{mcpID: tt.cluster}, + accessRequests: map[string]*clustersv1alpha1.AccessRequest{mcpID: tt.ar}, + clusterResources: map[string]*clustersv1alpha1.Cluster{mcpID: tt.mcpCluster}, + } + provider := NewLocalAdvancedClusterAccessReconciler(fakeProvider) + if tt.withWorkload { + provider.WithWorkloadCluster() + } + got, err := provider.Access(context.Background(), reconcile.Request{}, mcpID) + if tt.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.wantHost, got.RESTConfig().Host) + }) + } +} + var _ advanced.ClusterAccessReconciler = &fakeAdvancedClusterAccessReconciler{} type fakeAdvancedClusterAccessReconciler struct { - clusters map[string]*clusters.Cluster - accessRequests map[string]*clustersv1alpha1.AccessRequest + clusters map[string]*clusters.Cluster + accessRequests map[string]*clustersv1alpha1.AccessRequest + clusterResources map[string]*clustersv1alpha1.Cluster } // Access implements [advanced.ClusterAccessReconciler]. @@ -159,8 +254,8 @@ func (f *fakeAdvancedClusterAccessReconciler) ClusterRequest(_ context.Context, } // Cluster implements [advanced.ClusterAccessReconciler]. -func (f *fakeAdvancedClusterAccessReconciler) Cluster(_ context.Context, _ reconcile.Request, _ string, _ ...any) (*clustersv1alpha1.Cluster, error) { - panic("unimplemented") +func (f *fakeAdvancedClusterAccessReconciler) Cluster(_ context.Context, _ reconcile.Request, id string, _ ...any) (*clustersv1alpha1.Cluster, error) { + return f.clusterResources[id], nil } // Reconcile implements [advanced.ClusterAccessReconciler]. From 1544b2696be766578adeb81fcf8584d42d2bcf27 Mon Sep 17 00:00:00 2001 From: Nico Andres Date: Mon, 13 Jul 2026 18:00:17 +0200 Subject: [PATCH 3/6] use param On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres --- .../clusteraccess/localaccess_advanced.go | 10 +++------- .../clusteraccess/localaccess_advanced_test.go | 9 +++------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/pkg/serviceprovider/clusteraccess/localaccess_advanced.go b/pkg/serviceprovider/clusteraccess/localaccess_advanced.go index 5d2091b..cb59cd7 100644 --- a/pkg/serviceprovider/clusteraccess/localaccess_advanced.go +++ b/pkg/serviceprovider/clusteraccess/localaccess_advanced.go @@ -22,18 +22,14 @@ type localAdvancedClusterAccessReconciler struct { } // NewLocalAdvancedClusterAccessReconciler returns a local advanced cluster access reconciler that wraps the given advanced cluster access reconciler. -func NewLocalAdvancedClusterAccessReconciler(car advanced.ClusterAccessReconciler) *localAdvancedClusterAccessReconciler { +// Set withWorkload to true when the service provider deploys to a workload cluster +func NewLocalAdvancedClusterAccessReconciler(car advanced.ClusterAccessReconciler, withWorkload bool) advanced.ClusterAccessReconciler { return &localAdvancedClusterAccessReconciler{ ClusterAccessReconciler: car, + withWorkload: withWorkload, } } -// WithWorkloadCluster enables patching of the MCP RESTConfig.Host with the "apiserver-internal" endpoint address. -func (s *localAdvancedClusterAccessReconciler) WithWorkloadCluster() *localAdvancedClusterAccessReconciler { - s.withWorkload = true - return s -} - // Access implements [advanced.ClusterAccessReconciler]. func (s *localAdvancedClusterAccessReconciler) Access(ctx context.Context, request reconcile.Request, id string, additionalData ...any) (*clusters.Cluster, error) { cluster, err := s.ClusterAccessReconciler.Access(ctx, request, id, additionalData...) diff --git a/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go b/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go index e47acf2..28fbb6d 100644 --- a/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go +++ b/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go @@ -62,7 +62,7 @@ func Test_advancedLocalAccessProvider_MCPCluster(t *testing.T) { clusters: map[string]*clusters.Cluster{mcpID: tt.cluster}, accessRequests: map[string]*clustersv1alpha1.AccessRequest{mcpID: tt.ar}, } - localAccessProvider := NewLocalAdvancedClusterAccessReconciler(fakeProvider) + localAccessProvider := NewLocalAdvancedClusterAccessReconciler(fakeProvider, false) got, gotErr := localAccessProvider.Access(context.Background(), reconcile.Request{}, mcpID) if gotErr != nil { if !tt.wantErr { @@ -120,7 +120,7 @@ func Test_advancedLocalAccessProvider_WorkloadCluster(t *testing.T) { clusters: map[string]*clusters.Cluster{workloadID: tt.cluster}, accessRequests: map[string]*clustersv1alpha1.AccessRequest{workloadID: tt.ar}, } - localAccessProvider := NewLocalAdvancedClusterAccessReconciler(fakeProvider) + localAccessProvider := NewLocalAdvancedClusterAccessReconciler(fakeProvider, false) got, gotErr := localAccessProvider.Access(context.Background(), reconcile.Request{}, workloadID) if gotErr != nil { if !tt.wantErr { @@ -215,10 +215,7 @@ func Test_advancedLocalAccessProvider_WithWorkloadCluster(t *testing.T) { accessRequests: map[string]*clustersv1alpha1.AccessRequest{mcpID: tt.ar}, clusterResources: map[string]*clustersv1alpha1.Cluster{mcpID: tt.mcpCluster}, } - provider := NewLocalAdvancedClusterAccessReconciler(fakeProvider) - if tt.withWorkload { - provider.WithWorkloadCluster() - } + provider := NewLocalAdvancedClusterAccessReconciler(fakeProvider, tt.withWorkload) got, err := provider.Access(context.Background(), reconcile.Request{}, mcpID) if tt.wantErr { assert.Error(t, err) From adbce35f048c0155f26e4355ba9091db4d6efb96 Mon Sep 17 00:00:00 2001 From: Nico Andres Date: Mon, 13 Jul 2026 18:22:01 +0200 Subject: [PATCH 4/6] move Cluster On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres --- pkg/serviceprovider/clusteraccess/clusteraccess.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/serviceprovider/clusteraccess/clusteraccess.go b/pkg/serviceprovider/clusteraccess/clusteraccess.go index 8895874..cc75c18 100644 --- a/pkg/serviceprovider/clusteraccess/clusteraccess.go +++ b/pkg/serviceprovider/clusteraccess/clusteraccess.go @@ -71,11 +71,6 @@ func (a *simpleProviderAdapter) Access(ctx context.Context, request reconcile.Re } } -// Cluster implements [AdvancedProvider]. The legacy Provider has no access to the platform Cluster CR. -func (a *simpleProviderAdapter) Cluster(_ context.Context, _ reconcile.Request, _ string, _ ...any) (*clustersv1alpha1.Cluster, error) { - return nil, nil -} - // AccessRequest implements [AdvancedProvider]. func (a *simpleProviderAdapter) AccessRequest(ctx context.Context, request reconcile.Request, id string, _ ...any) (*clustersv1alpha1.AccessRequest, error) { switch id { @@ -88,6 +83,11 @@ func (a *simpleProviderAdapter) AccessRequest(ctx context.Context, request recon } } +// Cluster implements [AdvancedProvider]. +func (a *simpleProviderAdapter) Cluster(_ context.Context, _ reconcile.Request, _ string, _ ...any) (*clustersv1alpha1.Cluster, error) { + return nil, nil +} + // Reconcile implements [AdvancedProvider]. func (a *simpleProviderAdapter) Reconcile(ctx context.Context, request reconcile.Request, _ ...any) (reconcile.Result, error) { return a.simple.Reconcile(ctx, request) From 9038158557d709f4c90afea640e169e4812538f8 Mon Sep 17 00:00:00 2001 From: Nico Andres Date: Tue, 14 Jul 2026 11:35:07 +0200 Subject: [PATCH 5/6] use functional option On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres --- .../clusteraccess/localaccess_advanced.go | 28 +++++++++++++------ .../localaccess_advanced_test.go | 10 +++++-- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/pkg/serviceprovider/clusteraccess/localaccess_advanced.go b/pkg/serviceprovider/clusteraccess/localaccess_advanced.go index cb59cd7..32ed6b6 100644 --- a/pkg/serviceprovider/clusteraccess/localaccess_advanced.go +++ b/pkg/serviceprovider/clusteraccess/localaccess_advanced.go @@ -18,16 +18,28 @@ var _ advanced.ClusterAccessReconciler = &localAdvancedClusterAccessReconciler{} // instead of the wrapped reconciler. type localAdvancedClusterAccessReconciler struct { advanced.ClusterAccessReconciler - withWorkload bool + withWorkloadCluster bool +} + +// LocalAccessOption is a functional option for NewLocalAdvancedClusterAccessReconciler. +type LocalAccessOption func(*localAdvancedClusterAccessReconciler) + +// WithWorkloadCluster configures the local reconciler to override the ControlPlane rest.Config with the host on the docker network. +func WithWorkloadCluster() LocalAccessOption { + return func(r *localAdvancedClusterAccessReconciler) { + r.withWorkloadCluster = true + } } // NewLocalAdvancedClusterAccessReconciler returns a local advanced cluster access reconciler that wraps the given advanced cluster access reconciler. -// Set withWorkload to true when the service provider deploys to a workload cluster -func NewLocalAdvancedClusterAccessReconciler(car advanced.ClusterAccessReconciler, withWorkload bool) advanced.ClusterAccessReconciler { - return &localAdvancedClusterAccessReconciler{ +func NewLocalAdvancedClusterAccessReconciler(car advanced.ClusterAccessReconciler, opts ...LocalAccessOption) advanced.ClusterAccessReconciler { + r := &localAdvancedClusterAccessReconciler{ ClusterAccessReconciler: car, - withWorkload: withWorkload, } + for _, opt := range opts { + opt(r) + } + return r } // Access implements [advanced.ClusterAccessReconciler]. @@ -43,11 +55,11 @@ func (s *localAdvancedClusterAccessReconciler) Access(ctx context.Context, reque // Always patch the cluster client with the host value of the local AR annotation so that the service provider process can connect. cluster = MustPatchClusterClient(ctx, ar, cluster) - // If the service provider is using a workload cluster we additionally have to override the MCPs rest.Config.Host to the Docker-network address fetched from the "apiserver-internal" endpoint of the Cluster. - // Using this endpoint, the pod running on the workload cluster can reach the MCP API server if injected as KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT env vars. + // If the service provider is using a workload cluster we additionally have to override the ControlPlanes rest.Config.Host to the Docker-network address fetched from the "apiserver-internal" endpoint of the Cluster. + // Using this endpoint, the pod running on the workload cluster can reach the ControlPlane API server if injected as KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT env vars. // If we would not override it the rest.Config.Host would point to localhost. // Warning: This does not affect the cluster client as we only initialize it in MustPatchClusterClient. As a result the rest.Config points to a different host than the cluster client! - if id == MCPClusterID && s.withWorkload && cluster.HasRESTConfig() { + if id == MCPClusterID && s.withWorkloadCluster && cluster.HasRESTConfig() { mcpCluster, err := s.Cluster(ctx, request, id, additionalData...) if err != nil { return cluster, err diff --git a/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go b/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go index 28fbb6d..7727441 100644 --- a/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go +++ b/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go @@ -62,7 +62,7 @@ func Test_advancedLocalAccessProvider_MCPCluster(t *testing.T) { clusters: map[string]*clusters.Cluster{mcpID: tt.cluster}, accessRequests: map[string]*clustersv1alpha1.AccessRequest{mcpID: tt.ar}, } - localAccessProvider := NewLocalAdvancedClusterAccessReconciler(fakeProvider, false) + localAccessProvider := NewLocalAdvancedClusterAccessReconciler(fakeProvider) got, gotErr := localAccessProvider.Access(context.Background(), reconcile.Request{}, mcpID) if gotErr != nil { if !tt.wantErr { @@ -120,7 +120,7 @@ func Test_advancedLocalAccessProvider_WorkloadCluster(t *testing.T) { clusters: map[string]*clusters.Cluster{workloadID: tt.cluster}, accessRequests: map[string]*clustersv1alpha1.AccessRequest{workloadID: tt.ar}, } - localAccessProvider := NewLocalAdvancedClusterAccessReconciler(fakeProvider, false) + localAccessProvider := NewLocalAdvancedClusterAccessReconciler(fakeProvider) got, gotErr := localAccessProvider.Access(context.Background(), reconcile.Request{}, workloadID) if gotErr != nil { if !tt.wantErr { @@ -215,7 +215,11 @@ func Test_advancedLocalAccessProvider_WithWorkloadCluster(t *testing.T) { accessRequests: map[string]*clustersv1alpha1.AccessRequest{mcpID: tt.ar}, clusterResources: map[string]*clustersv1alpha1.Cluster{mcpID: tt.mcpCluster}, } - provider := NewLocalAdvancedClusterAccessReconciler(fakeProvider, tt.withWorkload) + var opts []LocalAccessOption + if tt.withWorkload { + opts = append(opts, WithWorkloadCluster()) + } + provider := NewLocalAdvancedClusterAccessReconciler(fakeProvider, opts...) got, err := provider.Access(context.Background(), reconcile.Request{}, mcpID) if tt.wantErr { assert.Error(t, err) From fb0f48cee1dc6455acaf8ffb69199c110f3dbc03 Mon Sep 17 00:00:00 2001 From: Nico Andres Date: Tue, 14 Jul 2026 13:57:55 +0200 Subject: [PATCH 6/6] rename to controlplane On-behalf-of: @SAP nico.andres@sap.com Signed-off-by: Nico Andres --- .../clusteraccess/localaccess_advanced.go | 8 ++-- .../localaccess_advanced_test.go | 48 +++++++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/pkg/serviceprovider/clusteraccess/localaccess_advanced.go b/pkg/serviceprovider/clusteraccess/localaccess_advanced.go index 32ed6b6..1490517 100644 --- a/pkg/serviceprovider/clusteraccess/localaccess_advanced.go +++ b/pkg/serviceprovider/clusteraccess/localaccess_advanced.go @@ -60,14 +60,14 @@ func (s *localAdvancedClusterAccessReconciler) Access(ctx context.Context, reque // If we would not override it the rest.Config.Host would point to localhost. // Warning: This does not affect the cluster client as we only initialize it in MustPatchClusterClient. As a result the rest.Config points to a different host than the cluster client! if id == MCPClusterID && s.withWorkloadCluster && cluster.HasRESTConfig() { - mcpCluster, err := s.Cluster(ctx, request, id, additionalData...) + controlPlaneCluster, err := s.Cluster(ctx, request, id, additionalData...) if err != nil { return cluster, err } - if mcpCluster == nil { - return cluster, fmt.Errorf("mcp cluster not found") + if controlPlaneCluster == nil { + return cluster, fmt.Errorf("ControlPlane cluster not found") } - internalURL, ok := mcpCluster.Status.Endpoints.Get(clustersv1alpha1.APISERVER_ENDPOINT_INTERNAL) + internalURL, ok := controlPlaneCluster.Status.Endpoints.Get(clustersv1alpha1.APISERVER_ENDPOINT_INTERNAL) if !ok { return cluster, fmt.Errorf("%s endpoint not found", clustersv1alpha1.APISERVER_ENDPOINT_INTERNAL) } diff --git a/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go b/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go index 7727441..d8471cd 100644 --- a/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go +++ b/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go @@ -16,11 +16,11 @@ import ( ) const ( - mcpID = "mcp" - workloadID = "workload" + controlPlaneID = "mcp" + workloadID = "workload" ) -func Test_advancedLocalAccessProvider_MCPCluster(t *testing.T) { +func Test_advancedLocalAccessProvider_ControlPlaneCluster(t *testing.T) { tests := []struct { name string ar *clustersv1alpha1.AccessRequest @@ -32,7 +32,7 @@ func Test_advancedLocalAccessProvider_MCPCluster(t *testing.T) { name: "local annotation results in local client config", ar: &clustersv1alpha1.AccessRequest{ ObjectMeta: metav1.ObjectMeta{ - Name: "mcp-access", + Name: "control-plane-access", Namespace: metav1.NamespaceDefault, Annotations: map[string]string{ clusterprovider.LocalAccessAnnotation: localAPIServer, @@ -47,7 +47,7 @@ func Test_advancedLocalAccessProvider_MCPCluster(t *testing.T) { name: "no local annotation results in original cluster client config", ar: &clustersv1alpha1.AccessRequest{ ObjectMeta: metav1.ObjectMeta{ - Name: "mcp-access", + Name: "control-plane-access", Namespace: metav1.NamespaceDefault, }, }, @@ -59,11 +59,11 @@ func Test_advancedLocalAccessProvider_MCPCluster(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { fakeProvider := &fakeAdvancedClusterAccessReconciler{ - clusters: map[string]*clusters.Cluster{mcpID: tt.cluster}, - accessRequests: map[string]*clustersv1alpha1.AccessRequest{mcpID: tt.ar}, + clusters: map[string]*clusters.Cluster{controlPlaneID: tt.cluster}, + accessRequests: map[string]*clustersv1alpha1.AccessRequest{controlPlaneID: tt.ar}, } localAccessProvider := NewLocalAdvancedClusterAccessReconciler(fakeProvider) - got, gotErr := localAccessProvider.Access(context.Background(), reconcile.Request{}, mcpID) + got, gotErr := localAccessProvider.Access(context.Background(), reconcile.Request{}, controlPlaneID) if gotErr != nil { if !tt.wantErr { t.Errorf("Access() failed: %v", gotErr) @@ -138,16 +138,16 @@ func Test_advancedLocalAccessProvider_WorkloadCluster(t *testing.T) { func Test_advancedLocalAccessProvider_WithWorkloadCluster(t *testing.T) { tests := []struct { - name string - ar *clustersv1alpha1.AccessRequest - cluster *clusters.Cluster - withWorkload bool - mcpCluster *clustersv1alpha1.Cluster - wantHost string - wantErr bool + name string + ar *clustersv1alpha1.AccessRequest + cluster *clusters.Cluster + withWorkload bool + controlPlaneCluster *clustersv1alpha1.Cluster + wantHost string + wantErr bool }{ { - name: "WithWorkloadCluster results in MCP host changed to internalURL", + name: "WithWorkloadCluster results in ControlPlane host changed to internalURL", withWorkload: true, ar: &clustersv1alpha1.AccessRequest{ ObjectMeta: metav1.ObjectMeta{ @@ -155,7 +155,7 @@ func Test_advancedLocalAccessProvider_WithWorkloadCluster(t *testing.T) { }, }, cluster: createFakeCluster().WithRESTConfig(&rest.Config{Host: localAPIServer}), - mcpCluster: &clustersv1alpha1.Cluster{ + controlPlaneCluster: &clustersv1alpha1.Cluster{ Status: clustersv1alpha1.ClusterStatus{ Endpoints: clustersv1alpha1.Endpoints{ { @@ -168,7 +168,7 @@ func Test_advancedLocalAccessProvider_WithWorkloadCluster(t *testing.T) { wantHost: inclusterAPIServer, }, { - name: "Without WithWorkloadCluster results in MCP host patched to local annotation only", + name: "Without WithWorkloadCluster results in ControlPlane host patched to local annotation only", withWorkload: false, ar: &clustersv1alpha1.AccessRequest{ ObjectMeta: metav1.ObjectMeta{ @@ -179,7 +179,7 @@ func Test_advancedLocalAccessProvider_WithWorkloadCluster(t *testing.T) { wantHost: localAPIServer, }, { - name: "WithWorkloadCluster and nil mcpCluster results in error", + name: "WithWorkloadCluster and nil controlPlaneCluster results in error", withWorkload: true, ar: &clustersv1alpha1.AccessRequest{ ObjectMeta: metav1.ObjectMeta{ @@ -198,7 +198,7 @@ func Test_advancedLocalAccessProvider_WithWorkloadCluster(t *testing.T) { }, }, cluster: createFakeCluster().WithRESTConfig(&rest.Config{Host: localAPIServer}), - mcpCluster: &clustersv1alpha1.Cluster{ + controlPlaneCluster: &clustersv1alpha1.Cluster{ Status: clustersv1alpha1.ClusterStatus{ Endpoints: clustersv1alpha1.Endpoints{ {}, @@ -211,16 +211,16 @@ func Test_advancedLocalAccessProvider_WithWorkloadCluster(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { fakeProvider := &fakeAdvancedClusterAccessReconciler{ - clusters: map[string]*clusters.Cluster{mcpID: tt.cluster}, - accessRequests: map[string]*clustersv1alpha1.AccessRequest{mcpID: tt.ar}, - clusterResources: map[string]*clustersv1alpha1.Cluster{mcpID: tt.mcpCluster}, + clusters: map[string]*clusters.Cluster{controlPlaneID: tt.cluster}, + accessRequests: map[string]*clustersv1alpha1.AccessRequest{controlPlaneID: tt.ar}, + clusterResources: map[string]*clustersv1alpha1.Cluster{controlPlaneID: tt.controlPlaneCluster}, } var opts []LocalAccessOption if tt.withWorkload { opts = append(opts, WithWorkloadCluster()) } provider := NewLocalAdvancedClusterAccessReconciler(fakeProvider, opts...) - got, err := provider.Access(context.Background(), reconcile.Request{}, mcpID) + got, err := provider.Access(context.Background(), reconcile.Request{}, controlPlaneID) if tt.wantErr { assert.Error(t, err) return