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/clusteraccess.go b/pkg/serviceprovider/clusteraccess/clusteraccess.go index 2245454..cc75c18 100644 --- a/pkg/serviceprovider/clusteraccess/clusteraccess.go +++ b/pkg/serviceprovider/clusteraccess/clusteraccess.go @@ -83,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) @@ -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..1490517 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,13 +18,28 @@ var _ advanced.ClusterAccessReconciler = &localAdvancedClusterAccessReconciler{} // instead of the wrapped reconciler. type localAdvancedClusterAccessReconciler struct { advanced.ClusterAccessReconciler + 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. -func NewLocalAdvancedClusterAccessReconciler(car advanced.ClusterAccessReconciler) advanced.ClusterAccessReconciler { - return &localAdvancedClusterAccessReconciler{ +func NewLocalAdvancedClusterAccessReconciler(car advanced.ClusterAccessReconciler, opts ...LocalAccessOption) advanced.ClusterAccessReconciler { + r := &localAdvancedClusterAccessReconciler{ ClusterAccessReconciler: car, } + for _, opt := range opts { + opt(r) + } + return r } // Access implements [advanced.ClusterAccessReconciler]. @@ -35,7 +52,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 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.withWorkloadCluster && cluster.HasRESTConfig() { + controlPlaneCluster, err := s.Cluster(ctx, request, id, additionalData...) + if err != nil { + return cluster, err + } + if controlPlaneCluster == nil { + return cluster, fmt.Errorf("ControlPlane cluster not found") + } + internalURL, ok := controlPlaneCluster.Status.Endpoints.Get(clustersv1alpha1.APISERVER_ENDPOINT_INTERNAL) + if !ok { + return cluster, 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]. diff --git a/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go b/pkg/serviceprovider/clusteraccess/localaccess_advanced_test.go index 403191b..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) @@ -136,11 +136,107 @@ 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 + controlPlaneCluster *clustersv1alpha1.Cluster + wantHost string + wantErr bool + }{ + { + name: "WithWorkloadCluster results in ControlPlane 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}), + controlPlaneCluster: &clustersv1alpha1.Cluster{ + Status: clustersv1alpha1.ClusterStatus{ + Endpoints: clustersv1alpha1.Endpoints{ + { + Name: clustersv1alpha1.APISERVER_ENDPOINT_INTERNAL, + URL: inclusterAPIServer, + }, + }, + }, + }, + wantHost: inclusterAPIServer, + }, + { + name: "Without WithWorkloadCluster results in ControlPlane 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 controlPlaneCluster 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}), + controlPlaneCluster: &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{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{}, controlPlaneID) + 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 +255,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].