From fd7a32048902c1ee11273e981ae287d0b2a37935 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Sun, 6 Aug 2023 18:18:36 -0400 Subject: [PATCH] Add unit tests to cover broker rest config settings ...in the broker Syncer. Signed-off-by: Tom Pantelis --- pkg/resource/rest.go | 7 +- pkg/syncer/broker/syncer.go | 4 +- pkg/syncer/broker/syncer_test.go | 148 +++++++++++++++++++++++++++++-- 3 files changed, 148 insertions(+), 11 deletions(-) diff --git a/pkg/resource/rest.go b/pkg/resource/rest.go index 8ab3abaa..87739a0f 100644 --- a/pkg/resource/rest.go +++ b/pkg/resource/rest.go @@ -32,6 +32,11 @@ import ( "k8s.io/client-go/rest" ) +var NewDynamicClient = func(config *rest.Config) (dynamic.Interface, error) { + c, err := dynamic.NewForConfig(config) + return c, err //nolint:wrapcheck // No need to wrap +} + func GetAuthorizedRestConfigFromData(apiServer, apiServerToken, caData string, tls *rest.TLSClientConfig, gvr schema.GroupVersionResource, namespace string, ) (restConfig *rest.Config, authorized bool, err error) { @@ -109,7 +114,7 @@ func BuildRestConfigFromFiles(apiServer, apiServerTokenFile, caFile string, tls } func IsAuthorizedFor(restConfig *rest.Config, gvr schema.GroupVersionResource, namespace string) (bool, error) { - client, err := dynamic.NewForConfig(restConfig) + client, err := NewDynamicClient(restConfig) if err != nil { return false, errors.Wrap(err, "error creating dynamic client") } diff --git a/pkg/syncer/broker/syncer.go b/pkg/syncer/broker/syncer.go index b388784c..d0063f62 100644 --- a/pkg/syncer/broker/syncer.go +++ b/pkg/syncer/broker/syncer.go @@ -164,7 +164,7 @@ func NewSyncer(config SyncerConfig) (*Syncer, error) { //nolint:gocritic // Mini } if config.LocalClient == nil { - config.LocalClient, err = dynamic.NewForConfig(config.LocalRestConfig) + config.LocalClient, err = resource.NewDynamicClient(config.LocalRestConfig) if err != nil { return nil, errors.Wrap(err, "error creating dynamic client") } @@ -305,7 +305,7 @@ func createBrokerClient(config *SyncerConfig) error { logger.Error(err, "Error accessing the broker API server") } - config.BrokerClient, err = dynamic.NewForConfig(config.BrokerRestConfig) + config.BrokerClient, err = resource.NewDynamicClient(config.BrokerRestConfig) return errors.Wrap(err, "error creating dynamic client") } diff --git a/pkg/syncer/broker/syncer_test.go b/pkg/syncer/broker/syncer_test.go index 9b275550..4dd6aafe 100644 --- a/pkg/syncer/broker/syncer_test.go +++ b/pkg/syncer/broker/syncer_test.go @@ -19,12 +19,18 @@ package broker_test import ( "context" + "crypto/x509" + "encoding/base64" + "errors" + "os" + "strings" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/prometheus/client_golang/prometheus" "github.com/submariner-io/admiral/pkg/fake" + resourceutils "github.com/submariner-io/admiral/pkg/resource" sync "github.com/submariner-io/admiral/pkg/syncer" "github.com/submariner-io/admiral/pkg/syncer/broker" "github.com/submariner-io/admiral/pkg/syncer/test" @@ -37,24 +43,38 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilrand "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/rest" ) var _ = Describe("Broker Syncer", func() { var ( syncer *broker.Syncer config *broker.SyncerConfig + localDynClient *fake.DynamicClient localClient *fake.DynamicResourceClient + brokerDynClient *fake.DynamicClient brokerClient *fake.DynamicResourceClient resource *corev1.Pod transformed *corev1.Pod initialLocalResources []runtime.Object initialBrokerResources []runtime.Object stopCh chan struct{} + actualBrokerRestConfig *rest.Config + expectInitError bool ) ctx := context.TODO() BeforeEach(func() { + os.Unsetenv("BROKER_K8S_APISERVER") + os.Unsetenv("BROKER_K8S_APISERVERTOKEN") + os.Unsetenv("BROKER_K8S_REMOTENAMESPACE") + os.Unsetenv("BROKER_K8S_INSECURE") + os.Unsetenv("BROKER_K8S_SECRET") + + expectInitError = false + actualBrokerRestConfig = nil initialLocalResources = nil initialBrokerResources = nil stopCh = make(chan struct{}) @@ -75,24 +95,72 @@ var _ = Describe("Broker Syncer", func() { }, Scheme: runtime.NewScheme(), } - }) - JustBeforeEach(func() { Expect(corev1.AddToScheme(config.Scheme)).To(Succeed()) + localDynClient = fake.NewDynamicClient(config.Scheme) + brokerDynClient = fake.NewDynamicClient(config.Scheme) + }) + + JustBeforeEach(func() { var gvr *schema.GroupVersionResource config.RestMapper, gvr = test.GetRESTMapperAndGroupVersionResourceFor(resource) - config.LocalClient = fake.NewDynamicClient(config.Scheme, test.PrepInitialClientObjs("", "", initialLocalResources...)...) - config.BrokerClient = fake.NewDynamicClient(config.Scheme, test.PrepInitialClientObjs(config.BrokerNamespace, - "", initialBrokerResources...)...) + if config.LocalRestConfig == nil { + config.LocalClient = localDynClient + } + + brokerAPIServer := os.Getenv("BROKER_K8S_APISERVER") + + if config.BrokerRestConfig == nil && brokerAPIServer == "" { + config.BrokerClient = brokerDynClient + } + + if config.LocalRestConfig != nil || config.BrokerRestConfig != nil || brokerAPIServer != "" { + resourceutils.NewDynamicClient = func(inConfig *rest.Config) (dynamic.Interface, error) { + if equality.Semantic.DeepDerivative(inConfig, config.LocalRestConfig) { + return localDynClient, nil + } else if equality.Semantic.DeepDerivative(inConfig, config.BrokerRestConfig) || + (brokerAPIServer != "" && strings.HasSuffix(inConfig.Host, brokerAPIServer)) { + actualBrokerRestConfig = inConfig + return brokerDynClient, nil + } + + Fail("Unexpected rest.Config instance") + + return nil, errors.New("unexpected rest.Config instance") + } + } + + localClient, _ = localDynClient.Resource(*gvr).Namespace(config.ResourceConfigs[0].LocalSourceNamespace).(*fake.DynamicResourceClient) + brokerClient, _ = brokerDynClient.Resource(*gvr).Namespace(config.BrokerNamespace).(*fake.DynamicResourceClient) - localClient, _ = config.LocalClient.Resource(*gvr).Namespace(config.ResourceConfigs[0].LocalSourceNamespace).(*fake.DynamicResourceClient) - brokerClient, _ = config.BrokerClient.Resource(*gvr).Namespace(config.BrokerNamespace).(*fake.DynamicResourceClient) + for i := range initialLocalResources { + test.CreateResource(localDynClient.Resource(*gvr).Namespace(resourceutils.MustToMeta(initialLocalResources[i]).GetNamespace()), + initialLocalResources[i]) + } + + for i := range initialBrokerResources { + test.CreateResource(brokerDynClient.Resource(*gvr).Namespace(resourceutils.MustToMeta(initialBrokerResources[i]).GetNamespace()), + initialBrokerResources[i]) + } + + configCopy := *config + + if os.Getenv("BROKER_K8S_REMOTENAMESPACE") != "" { + configCopy.BrokerNamespace = "" + } var err error - syncer, err = broker.NewSyncer(*config) + syncer, err = broker.NewSyncer(configCopy) + + if expectInitError { + Expect(err).To(HaveOccurred()) + + return + } + Expect(err).To(Succeed()) Expect(syncer.Start(stopCh)).To(Succeed()) @@ -491,4 +559,68 @@ var _ = Describe("Broker Syncer", func() { }) }) }) + + When("rest config instances are specified", func() { + BeforeEach(func() { + config.LocalRestConfig = &rest.Config{ + Host: "https://local", + } + + config.BrokerRestConfig = &rest.Config{ + Host: "https://broker", + } + }) + + It("should work correctly", func() { + test.CreateResource(localClient, resource) + test.AwaitResource(brokerClient, resource.GetName()) + }) + + Context("and broker authorization fails", func() { + BeforeEach(func() { + expectInitError = true + fake.FailOnAction(&brokerDynClient.Fake, "pods", "get", x509.UnknownAuthorityError{}, false) + }) + + It("should return an error", func() { + }) + }) + }) + + When("broker config environment vars are specified", func() { + apiServerToken := base64.StdEncoding.EncodeToString([]byte("token")) + + BeforeEach(func() { + os.Setenv("BROKER_K8S_APISERVER", "broker-host") + os.Setenv("BROKER_K8S_APISERVERTOKEN", apiServerToken) + os.Setenv("BROKER_K8S_REMOTENAMESPACE", test.RemoteNamespace) + os.Setenv("BROKER_K8S_INSECURE", "true") + }) + + It("should work correctly", func() { + Expect(actualBrokerRestConfig.BearerToken).To(Equal(apiServerToken)) + Expect(actualBrokerRestConfig.TLSClientConfig.Insecure).To(BeTrue()) + + test.CreateResource(localClient, resource) + test.AwaitResource(brokerClient, resource.GetName()) + }) + + Context("with a secret", func() { + secret := "test-secret" + + BeforeEach(func() { + os.Setenv("BROKER_K8S_SECRET", secret) + os.Unsetenv("BROKER_K8S_APISERVERTOKEN") + }) + + It("should work correctly", func() { + Expect(actualBrokerRestConfig.BearerToken).To(BeEmpty()) + Expect(actualBrokerRestConfig.BearerTokenFile).To(ContainSubstring(secret)) + Expect(actualBrokerRestConfig.TLSClientConfig.Insecure).To(BeTrue()) + + test.CreateResource(localClient, resource) + test.AwaitResource(brokerClient, resource.GetName()) + }) + }) + }) })