Read the local Secret for creating the broker credentials

...when setting up the secret syncer. This addresses a TODO. Signed-off-by: Tom Pantelis <[email protected]>
submariner-io · Jul 25, 2024 · 65b679d · 65b679d
1 parent 0c7682d
commit 65b679d
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 28 deletions.
diff --git a/controllers/submariner/submariner_controller.go b/controllers/submariner/submariner_controller.go
@@ -20,6 +20,7 @@ package submariner
 
 import (
 	"context"
+	"encoding/base64"
 	"reflect"
 	"sync"
 	"time"
@@ -72,7 +73,8 @@ type Config struct {
 	Scheme                       *runtime.Scheme
 	DynClient                    dynamic.Interface
 	ClusterNetwork               *network.ClusterNetwork
-	GetAuthorizedBrokerClientFor func(spec *submopv1a1.SubmarinerSpec, gvr schema.GroupVersionResource) (dynamic.Interface, error)
+	GetAuthorizedBrokerClientFor func(spec *submopv1a1.SubmarinerSpec, brokerToken, brokerCA string,
+		secretGVR schema.GroupVersionResource) (dynamic.Interface, error)
 }
 
 // Reconciler reconciles a Submariner object.
@@ -112,7 +114,7 @@ func NewReconciler(config *Config) *Reconciler {
 	}
 
 	if r.config.GetAuthorizedBrokerClientFor == nil {
-		r.config.GetAuthorizedBrokerClientFor = r.getAuthorizedBrokerClientFor
+		r.config.GetAuthorizedBrokerClientFor = getAuthorizedBrokerClientFor
 	}
 
 	return r
@@ -158,7 +160,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
 	}
 
 	// Ensure we have a secret syncer
-	if err := r.setupSecretSyncer(instance, reqLogger, request.Namespace); err != nil {
+	if err := r.setupSecretSyncer(ctx, instance, reqLogger, request.Namespace); err != nil {
 		return reconcile.Result{}, err
 	}
 
@@ -326,18 +328,13 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func (r *Reconciler) setupSecretSyncer(instance *submopv1a1.Submariner, logger logr.Logger, namespace string) error {
+func (r *Reconciler) setupSecretSyncer(ctx context.Context, instance *submopv1a1.Submariner, logger logr.Logger, namespace string) error {
 	r.syncerMutex.Lock()
 	defer r.syncerMutex.Unlock()
 
 	if instance.Spec.BrokerK8sSecret != "" {
 		if _, ok := r.secretSyncCancelFuncs[instance.Spec.BrokerK8sSecret]; !ok {
-			_, gvr, err := util.ToUnstructuredResource(&corev1.Secret{}, r.config.ScopedClient.RESTMapper())
-			if err != nil {
-				return errors.Wrap(err, "error calculating the GVR for the Secret type")
-			}
-
-			brokerClient, err := r.config.GetAuthorizedBrokerClientFor(&instance.Spec, *gvr)
+			brokerClient, err := r.getBrokerClient(ctx, instance)
 			if err != nil {
 				return err
 			}
@@ -403,15 +400,39 @@ func (r *Reconciler) cancelSecretSyncer(instance *submopv1a1.Submariner) {
 	}
 }
 
-func (r *Reconciler) getAuthorizedBrokerClientFor(spec *submopv1a1.SubmarinerSpec, gvr schema.GroupVersionResource,
+func (r *Reconciler) getBrokerClient(ctx context.Context, instance *submopv1a1.Submariner) (dynamic.Interface, error) {
+	spec := &instance.Spec
+
+	_, secretGVR, err := util.ToUnstructuredResource(&corev1.Secret{}, r.config.ScopedClient.RESTMapper())
+	if err != nil {
+		return nil, errors.Wrap(err, "error calculating the GVR for the Secret type")
+	}
+
+	// We can't use files here since we don't have a mounted secret so read the broker Secret CR.
+
+	brokerToken := spec.BrokerK8sApiServerToken
+	brokerCA := spec.BrokerK8sCA
+
+	obj, err := r.config.DynClient.Resource(*secretGVR).Namespace(instance.Namespace).Get(ctx, spec.BrokerK8sSecret, metav1.GetOptions{})
+	if err == nil {
+		brokerSecret := resource.MustFromUnstructured(obj, &corev1.Secret{})
+		brokerToken = string(brokerSecret.Data["token"])
+		brokerCA = base64.StdEncoding.EncodeToString(brokerSecret.Data["ca.crt"])
+	} else if !apierrors.IsNotFound(err) {
+		return nil, errors.Wrapf(err, "error retrieving broker secret %q", spec.BrokerK8sSecret)
+	}
+
+	return r.config.GetAuthorizedBrokerClientFor(spec, brokerToken, brokerCA, *secretGVR)
+}
+
+func getAuthorizedBrokerClientFor(spec *submopv1a1.SubmarinerSpec, brokerToken, brokerCA string, secretGVR schema.GroupVersionResource,
 ) (dynamic.Interface, error) {
-	// We can't use files here, we don't have a mounted secret
 	brokerConfig, _, err := resource.GetAuthorizedRestConfigFromData(
 		spec.BrokerK8sApiServer,
-		spec.BrokerK8sApiServerToken, // TODO Read the secret
-		spec.BrokerK8sCA,
+		brokerToken,
+		brokerCA,
 		&rest.TLSClientConfig{Insecure: spec.BrokerK8sInsecure},
-		gvr,
+		secretGVR,
 		spec.BrokerK8sRemoteNamespace)
 	if err != nil {
 		return nil, errors.Wrap(err, "error building an authorized RestConfig for the broker")

diff --git a/controllers/submariner/submariner_controller_test.go b/controllers/submariner/submariner_controller_test.go
@@ -43,7 +43,9 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/dynamic"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -272,6 +274,16 @@ func testReconciliation() {
 		var brokerSecret *corev1.Secret
 
 		BeforeEach(func() {
+			t.submariner.Spec.BrokerK8sSecret = "submariner-broker-secret"
+
+			t.getAuthorizedBrokerClientFor = func(_ *v1alpha1.SubmarinerSpec, brokerToken, brokerCA string, _ schema.GroupVersionResource,
+			) (dynamic.Interface, error) {
+				Expect(brokerToken).To(Equal(t.submariner.Spec.BrokerK8sApiServerToken))
+				Expect(brokerCA).To(Equal(t.submariner.Spec.BrokerK8sCA))
+
+				return t.dynClient, nil
+			}
+
 			saName := opnames.ForClusterSA(t.submariner.Spec.ClusterID)
 			brokerSecret = &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
@@ -305,6 +317,36 @@ func testReconciliation() {
 				return reflect.DeepEqual(resource.MustFromUnstructured(obj, &corev1.Secret{}).Data, brokerSecret.Data)
 			})
 		})
+
+		Context("and the local secret already exists", func() {
+			BeforeEach(func() {
+				t.submariner.Spec.BrokerK8sSecret = "submariner-broker-secret"
+
+				localSecret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      t.submariner.Spec.BrokerK8sSecret,
+						Namespace: t.Namespace,
+					},
+					Type: corev1.SecretTypeServiceAccountToken,
+					Data: map[string][]byte{
+						"token":  {11, 22, 33, 44},
+						"ca.crt": {5, 6},
+					},
+				}
+
+				syncertest.CreateResource(t.secrets.Namespace(t.Namespace), localSecret)
+
+				t.getAuthorizedBrokerClientFor = func(_ *v1alpha1.SubmarinerSpec, brokerToken, _ string, _ schema.GroupVersionResource,
+				) (dynamic.Interface, error) {
+					Expect(brokerToken).To(Equal(string(localSecret.Data["token"])))
+					return t.dynClient, nil
+				}
+			})
+
+			It("should use the local secret's credentials to access the broker", func(ctx SpecContext) {
+				t.AssertReconcileSuccess(ctx)
+			})
+		})
 	})
 
 	When("the Submariner resource doesn't exist", func() {

diff --git a/controllers/submariner/submariner_suite_test.go b/controllers/submariner/submariner_suite_test.go
@@ -65,10 +65,11 @@ func TestSubmariner(t *testing.T) {
 
 type testDriver struct {
 	test.Driver
-	submariner     *v1alpha1.Submariner
-	clusterNetwork *network.ClusterNetwork
-	dynClient      *dynamicfake.FakeDynamicClient
-	secrets        dynamic.NamespaceableResourceInterface
+	submariner                   *v1alpha1.Submariner
+	clusterNetwork               *network.ClusterNetwork
+	dynClient                    *dynamicfake.FakeDynamicClient
+	secrets                      dynamic.NamespaceableResourceInterface
+	getAuthorizedBrokerClientFor func(*v1alpha1.SubmarinerSpec, string, string, schema.GroupVersionResource) (dynamic.Interface, error)
 }
 
 func newTestDriver() *testDriver {
@@ -101,14 +102,12 @@ func newTestDriver() *testDriver {
 		t.JustBeforeEach()
 
 		t.Controller = submarinerController.NewReconciler(&submarinerController.Config{
-			ScopedClient:   t.ScopedClient,
-			GeneralClient:  t.GeneralClient,
-			DynClient:      t.dynClient,
-			Scheme:         scheme.Scheme,
-			ClusterNetwork: t.clusterNetwork,
-			GetAuthorizedBrokerClientFor: func(_ *v1alpha1.SubmarinerSpec, _ schema.GroupVersionResource) (dynamic.Interface, error) {
-				return t.dynClient, nil
-			},
+			ScopedClient:                 t.ScopedClient,
+			GeneralClient:                t.GeneralClient,
+			DynClient:                    t.dynClient,
+			Scheme:                       scheme.Scheme,
+			ClusterNetwork:               t.clusterNetwork,
+			GetAuthorizedBrokerClientFor: t.getAuthorizedBrokerClientFor,
 		})
 	})
 
@@ -258,7 +257,6 @@ func newSubmariner() *v1alpha1.Submariner {
 			BrokerK8sApiServer:       "https://192.168.99.110:8443",
 			BrokerK8sApiServerToken:  "MIIDADCCAeigAw",
 			BrokerK8sCA:              "client.crt",
-			BrokerK8sSecret:          "submariner-broker-secret",
 			Broker:                   "k8s",
 			NatEnabled:               true,
 			ClusterID:                "east",