Allow an alternative kubeconfig for evaluating policies

This allows the controller to run on a separate cluster than the one it
is managing.

As part of this, global variables were removed to make it simpler to
determine where the Kubernetes client and configuration is coming from.

Related:
https://github.com/stolostron/backlog/issues/24361

Signed-off-by: mprahl <mprahl@users.noreply.github.com>

.github/workflows/kind.yml

@@ -74,6 +74,17 @@ jobs:
         export GOPATH=$(go env GOPATH)
         make e2e-test-coverage
 
+    - name: Create K8s KinD Cluster to simulate hosted mode - ${{ matrix.kind }}
+      env:
+        KIND_VERSION: ${{ matrix.kind }}
+      run: |
+        make kind-additional-cluster
+
+    - name: E2E tests that simulate hosted mode
+      run: |
+        export GOPATH=$(go env GOPATH)
+        make e2e-test-hosted-mode-coverage
+
     - name: Test Coverage Verification
       if: ${{ github.event_name == 'pull_request' }}
       run: |

.gitignore

@@ -88,6 +88,7 @@ test/crds/test-policy.yaml
 !vbh/build-harness
 !vbh/build-harness-extensions 
 kubeconfig_managed
+kubeconfig_managed2
 kind_kubeconfig.yaml
 report.json
 report_*.json

Makefile

@@ -41,6 +41,7 @@ else
 endif
 # Test coverage threshold
 export COVERAGE_MIN ?= 75
+COVERAGE_E2E_OUT ?= coverage_e2e.out
 
 # Image URL to use all building/pushing image targets;
 # Use your own docker registry and image name for dev/test by overridding the IMG and REGISTRY environment variable.
@@ -258,9 +259,17 @@ kind-create-cluster:
 	kind create cluster --name $(KIND_NAME) $(KIND_ARGS)
 	kind get kubeconfig --name $(KIND_NAME) > $(PWD)/kubeconfig_managed
 
+.PHONY: kind-additional-cluster
+kind-additional-cluster:
+	@echo "creating cluster"
+	kind create cluster --name $(KIND_NAME)2 $(KIND_ARGS)
+	kind get kubeconfig --name $(KIND_NAME)2 > $(PWD)/kubeconfig_managed2
+	kubectl config use-context kind-$(KIND_NAME)
+
 .PHONY: kind-delete-cluster
 kind-delete-cluster:
-	kind delete cluster --name $(KIND_NAME) || true
+	-kind delete cluster --name $(KIND_NAME)
+	-kind delete cluster --name $(KIND_NAME)2
 
 .PHONY: kind-tests
 kind-tests: kind-delete-cluster kind-bootstrap-cluster-dev build-images kind-deploy-controller-dev e2e-test
@@ -290,16 +299,22 @@ e2e-test: e2e-dependencies
 	$(GINKGO) -v --fail-fast --slow-spec-threshold=10s $(E2E_TEST_ARGS) test/e2e
 
 .PHONY: e2e-test-coverage
-e2e-test-coverage: E2E_TEST_ARGS = --json-report=report_e2e.json --output-dir=.
+e2e-test-coverage: E2E_TEST_ARGS = --json-report=report_e2e.json --label-filter="!hosted-mode" --output-dir=.
 e2e-test-coverage: e2e-run-instrumented e2e-test e2e-stop-instrumented
 
+.PHONY: e2e-test-hosted-mode-coverage
+e2e-test-hosted-mode-coverage: E2E_TEST_ARGS = --json-report=report_e2e_hosted_mode.json --label-filter="hosted-mode" --output-dir=.
+e2e-test-hosted-mode-coverage: COVERAGE_E2E_OUT = coverage_e2e_hosted_mode.out
+e2e-test-hosted-mode-coverage: export TARGET_KUBECONFIG_PATH = $(PWD)/kubeconfig_managed2
+e2e-test-hosted-mode-coverage: e2e-run-instrumented e2e-test e2e-stop-instrumented
+
 .PHONY: e2e-build-instrumented
 e2e-build-instrumented:
 	go test -covermode=atomic -coverpkg=$(shell cat go.mod | head -1 | cut -d ' ' -f 2)/... -c -tags e2e ./ -o build/_output/bin/$(IMG)-instrumented
 
 .PHONY: e2e-run-instrumented
 e2e-run-instrumented: e2e-build-instrumented
-	WATCH_NAMESPACE="$(WATCH_NAMESPACE)" ./build/_output/bin/$(IMG)-instrumented -test.run "^TestRunMain$$" -test.coverprofile=coverage_e2e.out &>build/_output/controller.log &
+	WATCH_NAMESPACE="$(WATCH_NAMESPACE)" ./build/_output/bin/$(IMG)-instrumented -test.run "^TestRunMain$$" -test.coverprofile=$(COVERAGE_E2E_OUT) &>build/_output/controller.log &
 
 .PHONY: e2e-stop-instrumented
 e2e-stop-instrumented:

controllers/configurationpolicy_controller.go

@@ -48,8 +48,6 @@ var log = ctrl.Log.WithName(ControllerName)
 // PlcChan a channel used to pass policies ready for update
 var PlcChan chan *policyv1.ConfigurationPolicy
 
-var clientSet kubernetes.Interface
-
 var (
 	eventNormal  = "Normal"
 	eventWarning = "Warning"
@@ -65,11 +63,6 @@ var (
 	reasonWantNotFoundDNE    = "Resource not found as expected"
 )
 
-var config *rest.Config
-
-// NamespaceWatched defines which namespace we can watch for the GRC policies and ignore others
-var NamespaceWatched string
-
 var evalLoopHistogram = prometheus.NewHistogram(
 	prometheus.HistogramOpts{
 		Name:    "config_policies_evaluation_duration_seconds",
@@ -91,13 +84,6 @@ func (r *ConfigurationPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error
 		Complete(r)
 }
 
-// Initialize to initialize some controller variables
-func Initialize(kubeconfig *rest.Config, clientset kubernetes.Interface, namespace string) {
-	config = kubeconfig
-	clientSet = clientset
-	NamespaceWatched = namespace
-}
-
 // blank assignment to verify that ConfigurationPolicyReconciler implements reconcile.Reconciler
 var _ reconcile.Reconciler = &ConfigurationPolicyReconciler{}
 
@@ -124,6 +110,10 @@ type ConfigurationPolicyReconciler struct {
 	EvaluationConcurrency uint8
 	Scheme                *runtime.Scheme
 	Recorder              record.EventRecorder
+	// The Kubernetes client to use when evaluating/enforcing policies. Most times, this will be the same cluster
+	// where the controller is running.
+	TargetK8sClient kubernetes.Interface
+	TargetK8sConfig *rest.Config
 	discoveryInfo
 }
 
@@ -234,7 +224,7 @@ func (r *ConfigurationPolicyReconciler) handlePolicyWorker(
 func (r *ConfigurationPolicyReconciler) refreshDiscoveryInfo() error {
 	log.V(2).Info("Refreshing the discovery info")
 
-	dd := clientSet.Discovery()
+	dd := r.TargetK8sClient.Discovery()
 
 	_, apiResourceList, err := dd.ServerGroupsAndResources()
 	if err != nil {
@@ -380,7 +370,7 @@ func (r *ConfigurationPolicyReconciler) getObjectTemplateDetails(
 		} else {
 			// If an error occurred in the NamespaceSelector, update the policy status and abort
 			var err error
-			selectedNamespaces, err = common.GetSelectedNamespaces(selector)
+			selectedNamespaces, err = common.GetSelectedNamespaces(r.TargetK8sClient, selector)
 			if err != nil {
 				errMsg := "Error filtering namespaces with provided namespaceSelector"
 				log.Error(
@@ -496,7 +486,7 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
 		}
 	}
 
-	tmplResolver, err := templates.NewResolver(&clientSet, config, tmplResolverCfg)
+	tmplResolver, err := templates.NewResolver(&r.TargetK8sClient, r.TargetK8sConfig, tmplResolverCfg)
 	if err != nil {
 		// If the encryption key is invalid, clear the cache.
 		if errors.Is(err, templates.ErrInvalidAESKey) || errors.Is(err, templates.ErrAESKeyNotSet) {
@@ -923,7 +913,7 @@ func (r *ConfigurationPolicyReconciler) handleObjects(
 			namespace, objDetails.namespace))
 	}
 
-	dclient, rsrc := getResourceAndDynamicClient(mapping)
+	dclient, rsrc := r.getResourceAndDynamicClient(mapping)
 
 	if objDetails.isNamespaced && namespace == "" {
 		log.Info("The object template is namespaced but no namespace is specified. Cannot process.")
@@ -1244,10 +1234,10 @@ func (r *ConfigurationPolicyReconciler) isObjectNamespaced(
 
 // getResourceAndDynamicClient creates a dynamic client to query resources and pulls the groupVersionResource
 // for an object from its mapping
-func getResourceAndDynamicClient(mapping *meta.RESTMapping) (
+func (r *ConfigurationPolicyReconciler) getResourceAndDynamicClient(mapping *meta.RESTMapping) (
 	dclient dynamic.Interface, rsrc schema.GroupVersionResource,
 ) {
-	restconfig := config
+	restconfig := rest.CopyConfig(r.TargetK8sConfig)
 	restconfig.GroupVersion = &schema.GroupVersion{
 		Group:   mapping.GroupVersionKind.Group,
 		Version: mapping.GroupVersionKind.Version,
@@ -2124,15 +2114,11 @@ func (r *ConfigurationPolicyReconciler) createParentPolicyEvent(instance *policy
 }
 
 func createParentPolicy(instance *policyv1.ConfigurationPolicy) extpoliciesv1.Policy {
-	ns := common.ExtractNamespaceLabel(instance)
-	if ns == "" {
-		ns = NamespaceWatched
-	}
-
 	return extpoliciesv1.Policy{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      instance.OwnerReferences[0].Name,
-			Namespace: ns, // we assume that the parent policy is in the watched-namespace passed as flag
+			Name: instance.OwnerReferences[0].Name,
+			// It's assumed that the parent policy is in the same namespace as the configuration policy
+			Namespace: instance.Namespace,
 			UID:       instance.OwnerReferences[0].UID,
 		},
 		TypeMeta: metav1.TypeMeta{

controllers/configurationpolicy_controller_test.go

@@ -14,13 +14,11 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
-	testclient "k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	policyv1 "open-cluster-management.io/config-policy-controller/api/v1"
-	"open-cluster-management.io/config-policy-controller/pkg/common"
 )
 
 func TestReconcile(t *testing.T) {
@@ -69,9 +67,6 @@ func TestReconcile(t *testing.T) {
 		},
 	}
 
-	simpleClient := testclient.NewSimpleClientset()
-	common.Initialize(simpleClient, nil)
-
 	res, err := r.Reconcile(context.TODO(), req)
 	if err != nil {
 		t.Fatalf("reconcile: (%v)", err)

main.go

@@ -23,6 +23,7 @@ import (
 
 	// to ensure that exec-entrypoint and run can make use of them.
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/klog/v2"
@@ -66,7 +67,7 @@ func main() {
 	zflags.Bind(flag.CommandLine)
 	pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
 
-	var clusterName, hubConfigPath, metricsAddr, probeAddr string
+	var clusterName, hubConfigPath, targetKubeConfig, metricsAddr, probeAddr string
 	var frequency uint
 	var decryptionConcurrency, evaluationConcurrency uint8
 	var enableLease, enableLeaderElection, legacyLeaderElection bool
@@ -78,6 +79,12 @@ func main() {
 	pflag.StringVar(&clusterName, "cluster-name", "acm-managed-cluster", "Name of the cluster")
 	pflag.StringVar(&hubConfigPath, "hub-kubeconfig-path", "/var/run/klusterlet/kubeconfig",
 		"Path to the hub kubeconfig")
+	pflag.StringVar(
+		&targetKubeConfig,
+		"target-kubeconfig-path",
+		"",
+		"A path to an alternative kubeconfig for policy evaluation and enforcement.",
+	)
 	pflag.StringVar(
 		&metricsAddr, "metrics-bind-address", "localhost:8383", "The address the metrics endpoint binds to.",
 	)
@@ -206,12 +213,36 @@ func main() {
 		os.Exit(1)
 	}
 
+	var targetK8sClient kubernetes.Interface
+	var targetK8sConfig *rest.Config
+
+	if targetKubeConfig == "" {
+		targetK8sConfig = cfg
+		targetK8sClient = kubernetes.NewForConfigOrDie(targetK8sConfig)
+	} else {
+		var err error
+
+		targetK8sConfig, err = clientcmd.BuildConfigFromFlags("", targetKubeConfig)
+		if err != nil {
+			log.Error(err, "Failed to load the target kubeconfig", "path", targetKubeConfig)
+			os.Exit(1)
+		}
+
+		targetK8sClient = kubernetes.NewForConfigOrDie(targetK8sConfig)
+
+		log.Info(
+			"Overrode the target Kubernetes cluster for policy evaluation and enforcement", "path", targetKubeConfig,
+		)
+	}
+
 	reconciler := controllers.ConfigurationPolicyReconciler{
 		Client:                mgr.GetClient(),
 		DecryptionConcurrency: decryptionConcurrency,
 		EvaluationConcurrency: evaluationConcurrency,
 		Scheme:                mgr.GetScheme(),
 		Recorder:              mgr.GetEventRecorderFor(controllers.ControllerName),
+		TargetK8sClient:       targetK8sClient,
+		TargetK8sConfig:       targetK8sConfig,
 	}
 	if err = reconciler.SetupWithManager(mgr); err != nil {
 		log.Error(err, "Unable to create controller", "controller", "ConfigurationPolicy")
@@ -229,11 +260,6 @@ func main() {
 		os.Exit(1)
 	}
 
-	// Initialize some variables
-	clientset := kubernetes.NewForConfigOrDie(cfg)
-	common.Initialize(clientset, cfg)
-	controllers.Initialize(cfg, clientset, namespace)
-
 	// PeriodicallyExecConfigPolicies is the go-routine that periodically checks the policies
 	log.V(1).Info("Perodically processing Configuration Policies", "frequency", frequency)
 
@@ -256,9 +282,8 @@ func main() {
 			log.Info("Starting lease controller to report status")
 
 			leaseUpdater := lease.NewLeaseUpdater(
-				clientset,
-				"config-policy-controller",
-				operatorNs,
+				// Always use the cluster that is running the controller for the lease.
+				kubernetes.NewForConfigOrDie(cfg), "config-policy-controller", operatorNs,
 			)
 
 			hubCfg, err := clientcmd.BuildConfigFromFlags("", hubConfigPath)

main_test.go

@@ -7,14 +7,17 @@
 package main
 
 import (
+	"fmt"
 	"os"
 	"testing"
 )
 
 // TestRunMain wraps the main() function in order to build a test binary and collection coverage for
 // E2E/Integration tests. Controller CLI flags are also passed in here.
 func TestRunMain(t *testing.T) {
-	os.Args = append(os.Args, "--leader-elect=false")
+	os.Args = append(
+		os.Args, "--leader-elect=false", fmt.Sprintf("--target-kubeconfig-path=%s", os.Getenv("TARGET_KUBECONFIG_PATH")),
+	)
 
 	main()
 }