Set BrokerK8sSecret in the ServiceDiscovery resource

...from the corresponding field in the Submariner resource so the Lighthouse agent deployment pod mounts the secret. The unit tests had to be adjusted b/c the BrokerK8sSecret field is now set in the Submariner resource. Signed-off-by: Tom Pantelis <[email protected]>
submariner-io · Jun 18, 2024 · 2721416 · 2721416
1 parent deee1d8
commit 2721416
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 23 deletions.
diff --git a/controllers/submariner/servicediscovery_resources.go b/controllers/submariner/servicediscovery_resources.go
@@ -48,6 +48,7 @@ func (r *Reconciler) serviceDiscoveryReconciler(ctx context.Context, submariner
 					BrokerK8sApiServerToken:  submariner.Spec.BrokerK8sApiServerToken,
 					BrokerK8sApiServer:       submariner.Spec.BrokerK8sApiServer,
 					BrokerK8sInsecure:        submariner.Spec.BrokerK8sInsecure,
+					BrokerK8sSecret:          submariner.Spec.BrokerK8sSecret,
 					HaltOnCertificateError:   submariner.Spec.HaltOnCertificateError,
 					Debug:                    submariner.Spec.Debug,
 					ClusterID:                submariner.Spec.ClusterID,

diff --git a/controllers/submariner/submariner_controller.go b/controllers/submariner/submariner_controller.go
@@ -42,6 +42,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/rest"
@@ -66,11 +67,12 @@ type Config struct {
 	// controller. Also it's a split client that reads objects from the cache and writes to the apiserver.
 	ScopedClient client.Client
 	// This client can be used to access any other resource not in the operator namespace.
-	GeneralClient  client.Client
-	RestConfig     *rest.Config
-	Scheme         *runtime.Scheme
-	DynClient      dynamic.Interface
-	ClusterNetwork *network.ClusterNetwork
+	GeneralClient                client.Client
+	RestConfig                   *rest.Config
+	Scheme                       *runtime.Scheme
+	DynClient                    dynamic.Interface
+	ClusterNetwork               *network.ClusterNetwork
+	GetAuthorizedBrokerClientFor func(spec *submopv1a1.SubmarinerSpec, gvr schema.GroupVersionResource) (dynamic.Interface, error)
 }
 
 // Reconciler reconciles a Submariner object.
@@ -103,11 +105,17 @@ var _ reconcile.Reconciler = &Reconciler{}
 
 // NewReconciler returns a new Reconciler.
 func NewReconciler(config *Config) *Reconciler {
-	return &Reconciler{
+	r := &Reconciler{
 		config:                *config,
 		log:                   ctrl.Log.WithName("controllers").WithName("Submariner"),
 		secretSyncCancelFuncs: make(map[string]context.CancelFunc),
 	}
+
+	if r.config.GetAuthorizedBrokerClientFor == nil {
+		r.config.GetAuthorizedBrokerClientFor = r.getAuthorizedBrokerClientFor
+	}
+
+	return r
 }
 
 // Reconcile reads that state of the cluster for a Submariner object and makes changes based on the state read
@@ -328,23 +336,15 @@ func (r *Reconciler) setupSecretSyncer(instance *submopv1a1.Submariner, logger l
 			if err != nil {
 				return errors.Wrap(err, "error calculating the GVR for the Secret type")
 			}
-			// We can't use files here, we don't have a mounted secret
-			brokerConfig, _, err := resource.GetAuthorizedRestConfigFromData(
-				instance.Spec.BrokerK8sApiServer,
-				instance.Spec.BrokerK8sApiServerToken, // TODO Read the secret
-				instance.Spec.BrokerK8sCA,
-				&rest.TLSClientConfig{Insecure: instance.Spec.BrokerK8sInsecure},
-				*gvr,
-				instance.Spec.BrokerK8sRemoteNamespace)
-			if err != nil {
-				return errors.Wrap(err, "error building an authorized RestConfig for the broker")
-			}
 
-			brokerClient, err := dynamic.NewForConfig(brokerConfig)
+			brokerClient, err := r.config.GetAuthorizedBrokerClientFor(&instance.Spec, *gvr)
 			if err != nil {
-				return errors.Wrap(err, "error building a dynamic client for the broker")
+				return err
 			}
 
+			clusterID := instance.Spec.ClusterID
+			transformedSecretName := instance.Spec.BrokerK8sSecret
+
 			secretSyncer, err := syncer.NewResourceSyncer(
 				&syncer.ResourceSyncerConfig{
 					Name:            "Broker secret syncer",
@@ -359,11 +359,11 @@ func (r *Reconciler) setupSecretSyncer(instance *submopv1a1.Submariner, logger l
 					Transform: func(from runtime.Object, _ int, _ syncer.Operation) (runtime.Object, bool) {
 						secret := from.(*corev1.Secret)
 						logger.V(level.TRACE).Info("Transforming secret", "secret", secret)
-						if saName, ok := secret.ObjectMeta.Annotations["kubernetes.io/service-account.name"]; ok &&
-							saName == names.ForClusterSA(instance.Spec.ClusterID) {
+						if saName, ok := secret.ObjectMeta.Annotations[corev1.ServiceAccountNameKey]; ok &&
+							saName == names.ForClusterSA(clusterID) {
 							transformedSecret := &corev1.Secret{
 								ObjectMeta: metav1.ObjectMeta{
-									Name: instance.Spec.BrokerK8sSecret,
+									Name: transformedSecretName,
 								},
 								Type: corev1.SecretTypeOpaque,
 								Data: secret.Data,
@@ -402,3 +402,22 @@ func (r *Reconciler) cancelSecretSyncer(instance *submopv1a1.Submariner) {
 		}
 	}
 }
+
+func (r *Reconciler) getAuthorizedBrokerClientFor(spec *submopv1a1.SubmarinerSpec, gvr 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,
+		&rest.TLSClientConfig{Insecure: spec.BrokerK8sInsecure},
+		gvr,
+		spec.BrokerK8sRemoteNamespace)
+	if err != nil {
+		return nil, errors.Wrap(err, "error building an authorized RestConfig for the broker")
+	}
+
+	brokerClient, err := dynamic.NewForConfig(brokerConfig)
+
+	return brokerClient, errors.Wrap(err, "error building a dynamic client for the broker")
+}
diff --git a/controllers/submariner/submariner_controller_test.go b/controllers/submariner/submariner_controller_test.go
@@ -20,14 +20,19 @@ package submariner_test
 
 import (
 	"context"
+	"fmt"
 	"os"
+	"reflect"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	v1config "github.com/openshift/api/config/v1"
 	"github.com/submariner-io/admiral/pkg/fake"
 	"github.com/submariner-io/admiral/pkg/names"
+	"github.com/submariner-io/admiral/pkg/resource"
+	syncertest "github.com/submariner-io/admiral/pkg/syncer/test"
+	testutil "github.com/submariner-io/admiral/pkg/test"
 	"github.com/submariner-io/submariner-operator/api/v1alpha1"
 	"github.com/submariner-io/submariner-operator/controllers/test"
 	"github.com/submariner-io/submariner-operator/controllers/uninstall"
@@ -37,6 +42,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"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/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -217,6 +223,7 @@ func testReconciliation() {
 			Expect(serviceDiscovery.Spec.BrokerK8sRemoteNamespace).To(Equal(t.submariner.Spec.BrokerK8sRemoteNamespace))
 			Expect(serviceDiscovery.Spec.BrokerK8sApiServerToken).To(Equal(t.submariner.Spec.BrokerK8sApiServerToken))
 			Expect(serviceDiscovery.Spec.BrokerK8sApiServer).To(Equal(t.submariner.Spec.BrokerK8sApiServer))
+			Expect(serviceDiscovery.Spec.BrokerK8sSecret).To(Equal(t.submariner.Spec.BrokerK8sSecret))
 			Expect(serviceDiscovery.Spec.ClusterID).To(Equal(t.submariner.Spec.ClusterID))
 			Expect(serviceDiscovery.Spec.Namespace).To(Equal(t.submariner.Spec.Namespace))
 			Expect(serviceDiscovery.Spec.GlobalnetEnabled).To(BeTrue())
@@ -261,6 +268,45 @@ func testReconciliation() {
 		})
 	})
 
+	When("the broker secret is created/updated", func() {
+		var brokerSecret *corev1.Secret
+
+		BeforeEach(func() {
+			saName := opnames.ForClusterSA(t.submariner.Spec.ClusterID)
+			brokerSecret = &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      fmt.Sprintf("%s-token", saName),
+					Namespace: t.submariner.Spec.BrokerK8sRemoteNamespace,
+					Annotations: map[string]string{
+						corev1.ServiceAccountNameKey: saName,
+					},
+				},
+				Type: corev1.SecretTypeServiceAccountToken,
+				Data: map[string][]byte{"data": {1, 2, 3}},
+			}
+
+			syncertest.CreateResource(t.secrets.Namespace(brokerSecret.Namespace), brokerSecret)
+		})
+
+		It("should update the local secret", func(ctx SpecContext) {
+			t.AssertReconcileSuccess(ctx)
+
+			ri := resource.ForDynamic(t.secrets.Namespace(t.submariner.Spec.Namespace))
+
+			obj := testutil.AwaitResource(ri, t.submariner.Spec.BrokerK8sSecret)
+			secret := resource.MustFromUnstructured(obj, &corev1.Secret{})
+			Expect(secret.Data).To(Equal(brokerSecret.Data))
+			Expect(secret.Type).To(Equal(corev1.SecretTypeOpaque))
+
+			brokerSecret.Data = map[string][]byte{"data": {40, 50, 60}}
+			syncertest.UpdateResource(t.secrets.Namespace(brokerSecret.Namespace), brokerSecret)
+
+			testutil.AwaitAndVerifyResource(ri, t.submariner.Spec.BrokerK8sSecret, func(obj *unstructured.Unstructured) bool {
+				return reflect.DeepEqual(resource.MustFromUnstructured(obj, &corev1.Secret{}).Data, brokerSecret.Data)
+			})
+		})
+	})
+
 	When("the Submariner resource doesn't exist", func() {
 		BeforeEach(func() {
 			t.InitScopedClientObjs = nil

diff --git a/controllers/submariner/submariner_suite_test.go b/controllers/submariner/submariner_suite_test.go
@@ -39,7 +39,10 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/dynamic"
+	dynamicfake "k8s.io/client-go/dynamic/fake"
 	"k8s.io/client-go/kubernetes/scheme"
 	controllerClient "sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -64,6 +67,8 @@ type testDriver struct {
 	test.Driver
 	submariner     *v1alpha1.Submariner
 	clusterNetwork *network.ClusterNetwork
+	dynClient      *dynamicfake.FakeDynamicClient
+	secrets        dynamic.NamespaceableResourceInterface
 }
 
 func newTestDriver() *testDriver {
@@ -84,6 +89,12 @@ func newTestDriver() *testDriver {
 			ServiceCIDRs:  []string{testDetectedServiceCIDR},
 			PodCIDRs:      []string{testDetectedClusterCIDR},
 		}
+
+		t.dynClient = dynamicfake.NewSimpleDynamicClient(scheme.Scheme)
+		t.secrets = t.dynClient.Resource(schema.GroupVersionResource{
+			Version:  "v1",
+			Resource: "secrets",
+		})
 	})
 
 	JustBeforeEach(func() {
@@ -92,8 +103,12 @@ func newTestDriver() *testDriver {
 		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
+			},
 		})
 	})
 
@@ -243,6 +258,7 @@ 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",

diff --git a/controllers/test/driver.go b/controllers/test/driver.go
@@ -23,6 +23,7 @@ import (
 
 	. "github.com/onsi/gomega"
 	"github.com/submariner-io/admiral/pkg/resource"
+	"github.com/submariner-io/admiral/pkg/syncer/test"
 	admtest "github.com/submariner-io/admiral/pkg/test"
 	"github.com/submariner-io/submariner-operator/api/v1alpha1"
 	"github.com/submariner-io/submariner-operator/controllers/uninstall"
@@ -71,7 +72,8 @@ func (d *Driver) JustBeforeEach() {
 
 func (d *Driver) NewScopedClient() client.Client {
 	return fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(d.InitScopedClientObjs...).
-		WithStatusSubresource(&v1alpha1.Submariner{}).WithInterceptorFuncs(d.InterceptorFuncs).Build()
+		WithStatusSubresource(&v1alpha1.Submariner{}).WithInterceptorFuncs(d.InterceptorFuncs).
+		WithRESTMapper(test.GetRESTMapperFor(&corev1.Secret{})).Build()
 }
 
 func (d *Driver) NewGeneralClient() client.Client {