Refactor controller logic for getting RESTConfig to a remote cluster (#…

…3712)

We had two copies, rationalize and take the best of each.

Also remove the HACK_ENABLE_LOOPBACK hack now that we can target remote clusters.

porch/controllers/Makefile

@@ -29,4 +29,4 @@ build-image:
 
 .PHONY: run-local
 run-local:
-	GCP_PROJECT_ID=${GCP_PROJECT_ID} HACK_ENABLE_LOOPBACK=1 go run .
+	GCP_PROJECT_ID=${GCP_PROJECT_ID} go run .

porch/controllers/remoterootsyncsets/api/v1alpha1/remoterootsyncset_types.go

@@ -74,6 +74,26 @@ type ClusterRef struct {
 	Namespace  string `json:"namespace,omitempty"`
 }
 
+func (r *ClusterRef) GetKind() string {
+	return r.Kind
+}
+
+func (r *ClusterRef) GetName() string {
+	return r.Name
+}
+
+func (r *ClusterRef) GetNamespace() string {
+	return r.Namespace
+}
+
+func (r *ClusterRef) GetAPIVersion() string {
+	return r.ApiVersion
+}
+
+type PackageRef struct {
+	Name string `json:"name,omitempty"`
+}
+
 type RootSyncTemplate struct {
 	SourceFormat string `json:"sourceFormat,omitempty"`
 	// Git          *GitInfo `json:"git,omitempty"`

porch/controllers/remoterootsyncsets/pkg/controllers/remoterootsyncset/remoterootsync_controller.go

@@ -18,7 +18,6 @@ import (
 	"context"
 	"flag"
 	"fmt"
-	"os"
 
 	kptoci "github.com/GoogleContainerTools/kpt/pkg/oci"
 	api "github.com/GoogleContainerTools/kpt/porch/controllers/remoterootsyncsets/api/v1alpha1"
@@ -28,11 +27,7 @@ import (
 	"github.com/GoogleContainerTools/kpt/porch/pkg/oci"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/discovery"
-	memory "k8s.io/client-go/discovery/cached"
-	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/rest"
-	"k8s.io/client-go/restmapper"
 	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -59,6 +54,8 @@ func (o *Options) BindFlags(prefix string, flags *flag.FlagSet) {
 type RemoteRootSyncSetReconciler struct {
 	Options
 
+	remoteclient.RemoteClientGetter
+
 	client.Client
 
 	ociStorage *kptoci.Storage
@@ -229,38 +226,21 @@ func updateAggregateStatus(subject *api.RemoteRootSyncSet) bool {
 }
 
 func (r *RemoteRootSyncSetReconciler) applyToClusterRef(ctx context.Context, subject *api.RemoteRootSyncSet, clusterRef *api.ClusterRef) (*applyset.ApplyResults, error) {
-	var restConfig *rest.Config
-
-	if os.Getenv("HACK_ENABLE_LOOPBACK") != "" {
-		if clusterRef.Name == "loopback!" {
-			restConfig = r.localRESTConfig
-			klog.Warningf("HACK: using loopback! configuration")
-		}
-	}
-
-	if restConfig == nil {
-		rc, err := remoteclient.GetRemoteClient(ctx, r.Client, clusterRef, subject.Namespace)
-		if err != nil {
-			return nil, err
-		}
-		restConfig = rc
+	remoteClient, err := r.GetRemoteClient(ctx, clusterRef, subject.Namespace)
+	if err != nil {
+		return nil, err
 	}
 
-	client, err := dynamic.NewForConfig(restConfig)
+	restMapper, err := remoteClient.RESTMapper()
 	if err != nil {
-		return nil, fmt.Errorf("failed to create a new dynamic client: %w", err)
+		return nil, err
 	}
 
-	// TODO: Use a better discovery client
-	discovery, err := discovery.NewDiscoveryClientForConfig(restConfig)
+	dynamicClient, err := remoteClient.DynamicClient()
 	if err != nil {
-		return nil, fmt.Errorf("error building discovery client: %w", err)
+		return nil, err
 	}
 
-	cached := memory.NewMemCacheClient(discovery)
-
-	restMapper := restmapper.NewDeferredDiscoveryRESTMapper(cached)
-
 	objects, err := r.BuildObjectsToApply(ctx, subject)
 	if err != nil {
 		return nil, err
@@ -278,7 +258,7 @@ func (r *RemoteRootSyncSetReconciler) applyToClusterRef(ctx context.Context, sub
 
 	applyset, err := applyset.New(applyset.Options{
 		RESTMapper:   restMapper,
-		Client:       client,
+		Client:       dynamicClient,
 		PatchOptions: patchOptions,
 	})
 	if err != nil {
@@ -339,6 +319,10 @@ func (r *RemoteRootSyncSetReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		return err
 	}
 
+	if err := r.RemoteClientGetter.Init(mgr); err != nil {
+		return err
+	}
+
 	r.Client = mgr.GetClient()
 
 	if err := ctrl.NewControllerManagedBy(mgr).