diff --git a/controllers/servicebinding_controller.go b/controllers/servicebinding_controller.go index a4f85d1f..e8ad11af 100644 --- a/controllers/servicebinding_controller.go +++ b/controllers/servicebinding_controller.go @@ -66,12 +66,11 @@ const ( // ServiceBindingReconciler reconciles a ServiceBinding object type ServiceBindingReconciler struct { client.Client - Log logr.Logger - Scheme *runtime.Scheme - GetSMClient func(ctx context.Context, secretResolver *utils.SecretResolver, resourceNamespace, btpAccessSecretName string) (sm.Client, error) - Config config.Config - SecretResolver *utils.SecretResolver - Recorder record.EventRecorder + Log logr.Logger + Scheme *runtime.Scheme + GetSMClient func(ctx context.Context, resourceNamespace, btpAccessSecretName string) (sm.Client, error) + Config config.Config + Recorder record.EventRecorder } // +kubebuilder:rbac:groups=services.cloud.sap.com,resources=servicebindings,verbs=get;list;watch;create;update;patch;delete @@ -205,7 +204,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding) } - smClient, err := r.GetSMClient(ctx, r.SecretResolver, getBTPAccessSecretNamespace(serviceBinding), serviceInstance.Spec.BTPAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, getBTPAccessSecretNamespace(serviceBinding), serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceBinding) } @@ -228,7 +227,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque func (r *ServiceBindingReconciler) updateSecret(ctx context.Context, serviceBinding *servicesv1.ServiceBinding, serviceInstance *servicesv1.ServiceInstance, log logr.Logger) error { log.Info("Updating secret according to the new template") - smClient, err := r.GetSMClient(ctx, r.SecretResolver, getBTPAccessSecretNamespace(serviceBinding), serviceInstance.Spec.BTPAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, getBTPAccessSecretNamespace(serviceBinding), serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { return err } @@ -260,7 +259,7 @@ func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient s log := utils.GetLogger(ctx) log.Info("Creating smBinding in SM") serviceBinding.Status.InstanceID = serviceInstance.Status.InstanceID - _, bindingParameters, err := utils.BuildSMRequestParameters(r.Client, serviceBinding.Namespace, serviceBinding.Spec.ParametersFrom, serviceBinding.Spec.Parameters) + _, bindingParameters, err := utils.BuildSMRequestParameters(serviceBinding.Namespace, serviceBinding.Spec.ParametersFrom, serviceBinding.Spec.Parameters) if err != nil { log.Error(err, "failed to parse smBinding parameters") return utils.MarkAsNonTransientError(ctx, r.Client, smClientTypes.CREATE, err.Error(), serviceBinding) @@ -323,7 +322,7 @@ func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient s func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *servicesv1.ServiceBinding, btpAccessCredentialsSecret string) (ctrl.Result, error) { log := utils.GetLogger(ctx) if controllerutil.ContainsFinalizer(serviceBinding, common.FinalizerName) { - smClient, err := r.GetSMClient(ctx, r.SecretResolver, getBTPAccessSecretNamespace(serviceBinding), btpAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, getBTPAccessSecretNamespace(serviceBinding), btpAccessCredentialsSecret) if err != nil { return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceBinding) } @@ -386,7 +385,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *ser log := utils.GetLogger(ctx) log.Info(fmt.Sprintf("resource is in progress, found operation url %s", serviceBinding.Status.OperationURL)) - smClient, err := r.GetSMClient(ctx, r.SecretResolver, getBTPAccessSecretNamespace(serviceBinding), btpAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, getBTPAccessSecretNamespace(serviceBinding), btpAccessCredentialsSecret) if err != nil { return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceBinding) } @@ -502,7 +501,9 @@ func (r *ServiceBindingReconciler) maintain(ctx context.Context, binding *servic shouldUpdateStatus = true } if !utils.IsFailed(binding) { - if _, err := r.getSecret(ctx, binding.Namespace, binding.Spec.SecretName); err != nil { + secret, err := r.getSecret(ctx, binding.Namespace, binding.Spec.SecretName) + if err != nil { + // secret was deleted if apierrors.IsNotFound(err) && !utils.IsMarkedForDeletion(binding.ObjectMeta) { log.Info(fmt.Sprintf("secret not found recovering binding %s", binding.Name)) binding.Status.BindingID = "" @@ -513,6 +514,17 @@ func (r *ServiceBindingReconciler) maintain(ctx context.Context, binding *servic } else { return ctrl.Result{}, err } + } else { // secret exists, validate it has the required labels + if secret.Labels == nil { + secret.Labels = map[string]string{} + } + if secret.Labels[common.ManagedByBTPOperatorLabel] != "true" { + secret.Labels[common.ManagedByBTPOperatorLabel] = "true" + if err = r.Client.Update(ctx, secret); err != nil { + log.Error(err, "failed to update secret labels") + return ctrl.Result{}, err + } + } } } @@ -605,7 +617,7 @@ func (r *ServiceBindingReconciler) storeBindingSecret(ctx context.Context, k8sBi return err } - if len(secret.Labels) == 0 { + if secret.Labels == nil { secret.Labels = map[string]string{} } secret.Labels[common.ManagedByBTPOperatorLabel] = "true" @@ -623,6 +635,7 @@ func (r *ServiceBindingReconciler) createBindingSecret(ctx context.Context, k8sB ObjectMeta: metav1.ObjectMeta{ Name: k8sBinding.Spec.SecretName, Annotations: map[string]string{"binding": k8sBinding.Name}, + Labels: map[string]string{common.ManagedByBTPOperatorLabel: "true"}, Namespace: k8sBinding.Namespace, }, Data: credentialsMap, @@ -715,6 +728,10 @@ func (r *ServiceBindingReconciler) createBindingSecretFromSecretTemplate(ctx con } secret.SetNamespace(k8sBinding.Namespace) secret.SetName(k8sBinding.Spec.SecretName) + if secret.Labels == nil { + secret.Labels = map[string]string{} + } + secret.Labels[common.ManagedByBTPOperatorLabel] = "true" // if no data provided use the default data if len(secret.Data) == 0 && len(secret.StringData) == 0 { @@ -797,7 +814,7 @@ func (r *ServiceBindingReconciler) deleteSecretAndRemoveFinalizer(ctx context.Co func (r *ServiceBindingReconciler) getSecret(ctx context.Context, namespace string, name string) (*corev1.Secret, error) { secret := &corev1.Secret{} - err := r.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, secret) + err := utils.GetSecretWithFallback(ctx, types.NamespacedName{Namespace: namespace, Name: name}, secret) return secret, err } @@ -942,7 +959,7 @@ func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, bindin } if len(bindings.Items) == 0 { - smClient, err := r.GetSMClient(ctx, r.SecretResolver, getBTPAccessSecretNamespace(binding), btpAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, getBTPAccessSecretNamespace(binding), btpAccessCredentialsSecret) if err != nil { return err } diff --git a/controllers/serviceinstance_controller.go b/controllers/serviceinstance_controller.go index a3f31a45..f4a0ec04 100644 --- a/controllers/serviceinstance_controller.go +++ b/controllers/serviceinstance_controller.go @@ -53,12 +53,11 @@ import ( // ServiceInstanceReconciler reconciles a ServiceInstance object type ServiceInstanceReconciler struct { client.Client - Log logr.Logger - Scheme *runtime.Scheme - GetSMClient func(ctx context.Context, secretResolver *utils.SecretResolver, resourceNamespace, btpAccessSecretName string) (sm.Client, error) - Config config.Config - SecretResolver *utils.SecretResolver - Recorder record.EventRecorder + Log logr.Logger + Scheme *runtime.Scheme + GetSMClient func(ctx context.Context, resourceNamespace, btpAccessSecretName string) (sm.Client, error) + Config config.Config + Recorder record.EventRecorder } // +kubebuilder:rbac:groups=services.cloud.sap.com,resources=serviceinstances,verbs=get;list;watch;create;update;patch;delete @@ -123,7 +122,7 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ log.Info(fmt.Sprintf("instance is not in final state, handling... (generation: %d, observedGen: %d", serviceInstance.Generation, serviceInstance.Status.ObservedGeneration)) serviceInstance.SetObservedGeneration(serviceInstance.Generation) - smClient, err := r.GetSMClient(ctx, r.SecretResolver, serviceInstance.Namespace, serviceInstance.Spec.BTPAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, serviceInstance.Namespace, serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { log.Error(err, "failed to get sm client") return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceInstance) @@ -173,7 +172,7 @@ func (r *ServiceInstanceReconciler) createInstance(ctx context.Context, smClient log := utils.GetLogger(ctx) log.Info("Creating instance in SM") updateHashedSpecValue(serviceInstance) - _, instanceParameters, err := utils.BuildSMRequestParameters(r.Client, serviceInstance.Namespace, serviceInstance.Spec.ParametersFrom, serviceInstance.Spec.Parameters) + _, instanceParameters, err := utils.BuildSMRequestParameters(serviceInstance.Namespace, serviceInstance.Spec.ParametersFrom, serviceInstance.Spec.Parameters) if err != nil { // if parameters are invalid there is nothing we can do, the user should fix it according to the error message in the condition log.Error(err, "failed to parse instance parameters") @@ -229,7 +228,7 @@ func (r *ServiceInstanceReconciler) updateInstance(ctx context.Context, smClient updateHashedSpecValue(serviceInstance) - _, instanceParameters, err := utils.BuildSMRequestParameters(r.Client, serviceInstance.Namespace, serviceInstance.Spec.ParametersFrom, serviceInstance.Spec.Parameters) + _, instanceParameters, err := utils.BuildSMRequestParameters(serviceInstance.Namespace, serviceInstance.Spec.ParametersFrom, serviceInstance.Spec.Parameters) if err != nil { log.Error(err, "failed to parse instance parameters") return utils.MarkAsNonTransientError(ctx, r.Client, smClientTypes.UPDATE, fmt.Sprintf("failed to parse parameters: %v", err.Error()), serviceInstance) @@ -267,7 +266,7 @@ func (r *ServiceInstanceReconciler) deleteInstance(ctx context.Context, serviceI log := utils.GetLogger(ctx) if controllerutil.ContainsFinalizer(serviceInstance, common.FinalizerName) { - smClient, err := r.GetSMClient(ctx, r.SecretResolver, serviceInstance.Namespace, serviceInstance.Spec.BTPAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, serviceInstance.Namespace, serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { log.Error(err, "failed to get sm client") return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceInstance) @@ -349,7 +348,7 @@ func (r *ServiceInstanceReconciler) handleInstanceSharing(ctx context.Context, s func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *servicesv1.ServiceInstance) (ctrl.Result, error) { log := utils.GetLogger(ctx) log.Info(fmt.Sprintf("resource is in progress, found operation url %s", serviceInstance.Status.OperationURL)) - smClient, err := r.GetSMClient(ctx, r.SecretResolver, serviceInstance.Namespace, serviceInstance.Spec.BTPAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, serviceInstance.Namespace, serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { log.Error(err, "failed to get sm client") return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceInstance) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index fe53cab8..d15579e5 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -28,10 +28,11 @@ import ( "testing" "time" + "github.com/SAP/sap-btp-service-operator/internal/utils" + "sigs.k8s.io/controller-runtime/pkg/metrics/server" "github.com/SAP/sap-btp-service-operator/api/common" - "github.com/SAP/sap-btp-service-operator/internal/utils" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" . "github.com/onsi/ginkgo" @@ -119,6 +120,8 @@ var _ = BeforeSuite(func(done Done) { Expect(err).ToNot(HaveOccurred()) Expect(k8sClient).ToNot(BeNil()) + utils.InitializeSecretsClient(k8sClient, nil, config.Config{EnableLimitedCache: false}) + webhookInstallOptions := &testEnv.WebhookInstallOptions k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ @@ -155,7 +158,7 @@ var _ = BeforeSuite(func(done Done) { Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme(), Log: ctrl.Log.WithName("controllers").WithName("ServiceInstance"), - GetSMClient: func(_ context.Context, _ *utils.SecretResolver, _, _ string) (sm.Client, error) { + GetSMClient: func(_ context.Context, _, _ string) (sm.Client, error) { return fakeClient, nil }, Config: testConfig, @@ -167,7 +170,7 @@ var _ = BeforeSuite(func(done Done) { Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme(), Log: ctrl.Log.WithName("controllers").WithName("ServiceBinding"), - GetSMClient: func(_ context.Context, _ *utils.SecretResolver, _, _ string) (sm.Client, error) { + GetSMClient: func(_ context.Context, _, _ string) (sm.Client, error) { return fakeClient, nil }, Config: testConfig, diff --git a/go.mod b/go.mod index a2bb3017..590cf713 100644 --- a/go.mod +++ b/go.mod @@ -27,6 +27,7 @@ require ( github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect + github.com/evanphx/json-patch v4.12.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.9.0 // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect github.com/go-logr/zapr v1.3.0 // indirect diff --git a/internal/utils/parameters.go b/internal/utils/parameters.go index 506d4064..7d4b18dc 100644 --- a/internal/utils/parameters.go +++ b/internal/utils/parameters.go @@ -9,10 +9,8 @@ import ( servicesv1 "github.com/SAP/sap-btp-service-operator/api/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/yaml" ) @@ -22,11 +20,11 @@ import ( // secret values. // The second return value is parameters marshalled to byt array // The third return value is any error that caused the function to fail. -func BuildSMRequestParameters(kubeClient client.Client, namespace string, parametersFrom []servicesv1.ParametersFromSource, parameters *runtime.RawExtension) (map[string]interface{}, []byte, error) { +func BuildSMRequestParameters(namespace string, parametersFrom []servicesv1.ParametersFromSource, parameters *runtime.RawExtension) (map[string]interface{}, []byte, error) { params := make(map[string]interface{}) if len(parametersFrom) > 0 { for _, p := range parametersFrom { - fps, err := fetchParametersFromSource(kubeClient, namespace, &p) + fps, err := fetchParametersFromSource(namespace, &p) if err != nil { return nil, nil, err } @@ -96,9 +94,9 @@ func unmarshalJSON(in []byte) (map[string]interface{}, error) { } // fetchSecretKeyValue requests and returns the contents of the given secret key -func fetchSecretKeyValue(kubeClient client.Client, namespace string, secretKeyRef *servicesv1.SecretKeyReference) ([]byte, error) { +func fetchSecretKeyValue(namespace string, secretKeyRef *servicesv1.SecretKeyReference) ([]byte, error) { secret := &corev1.Secret{} - err := kubeClient.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: secretKeyRef.Name}, secret) + err := GetSecretWithFallback(context.Background(), types.NamespacedName{Namespace: namespace, Name: secretKeyRef.Name}, secret) if err != nil { return nil, err @@ -108,10 +106,10 @@ func fetchSecretKeyValue(kubeClient client.Client, namespace string, secretKeyRe // fetchParametersFromSource fetches data from a specified external source and // represents it in the parameters map format -func fetchParametersFromSource(kubeClient client.Client, namespace string, parametersFrom *servicesv1.ParametersFromSource) (map[string]interface{}, error) { +func fetchParametersFromSource(namespace string, parametersFrom *servicesv1.ParametersFromSource) (map[string]interface{}, error) { var params map[string]interface{} if parametersFrom.SecretKeyRef != nil { - data, err := fetchSecretKeyValue(kubeClient, namespace, parametersFrom.SecretKeyRef) + data, err := fetchSecretKeyValue(namespace, parametersFrom.SecretKeyRef) if err != nil { return nil, err } diff --git a/internal/utils/parameters_test.go b/internal/utils/parameters_test.go index f503962b..d5e34ae0 100644 --- a/internal/utils/parameters_test.go +++ b/internal/utils/parameters_test.go @@ -13,7 +13,7 @@ var _ = Describe("Parameters", func() { var parametersFrom []v1.ParametersFromSource parameters := (*runtime.RawExtension)(nil) - params, rawParam, err := BuildSMRequestParameters(nil, "", parametersFrom, parameters) + params, rawParam, err := BuildSMRequestParameters("", parametersFrom, parameters) Expect(err).To(BeNil()) Expect(params).To(BeNil()) @@ -25,7 +25,7 @@ var _ = Describe("Parameters", func() { Raw: []byte(`{"key":"value"}`), } - params, rawParam, err := BuildSMRequestParameters(nil, "", parametersFrom, parameters) + params, rawParam, err := BuildSMRequestParameters("", parametersFrom, parameters) Expect(err).To(BeNil()) Expect(params).To(Equal(map[string]interface{}{"key": "value"})) diff --git a/internal/utils/secret_resolver.go b/internal/utils/secret_resolver.go index f0bf58ee..361dd1c2 100644 --- a/internal/utils/secret_resolver.go +++ b/internal/utils/secret_resolver.go @@ -2,9 +2,13 @@ package utils import ( "context" - "fmt" + "github.com/SAP/sap-btp-service-operator/internal/config" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + "k8s.io/apimachinery/pkg/api/errors" + "github.com/go-logr/logr" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -18,53 +22,80 @@ const ( SAPBTPOperatorTLSSecretName = "sap-btp-service-operator-tls" ) -type SecretResolver struct { +var secretsClient secretClient + +type secretClient struct { ManagementNamespace string ReleaseNamespace string EnableNamespaceSecrets bool + LimitedCacheEnabled bool Client client.Client + NonCachedClient client.Client Log logr.Logger } -func (sr *SecretResolver) GetSecretFromManagementNamespace(ctx context.Context, name string) (*v1.Secret, error) { +func InitializeSecretsClient(client, nonCachedClient client.Client, config config.Config) { + secretsClient = secretClient{ + Log: logf.Log.WithName("secret-resolver"), + ManagementNamespace: config.ManagementNamespace, + ReleaseNamespace: config.ReleaseNamespace, + EnableNamespaceSecrets: config.EnableNamespaceSecrets, + LimitedCacheEnabled: config.EnableLimitedCache, + Client: client, + NonCachedClient: nonCachedClient, + } +} + +func GetSecretWithFallback(ctx context.Context, namespacedName types.NamespacedName, secret *v1.Secret) error { + return secretsClient.getWithClientFallback(ctx, namespacedName, secret) +} + +func GetSecretFromManagementNamespace(ctx context.Context, name string) (*v1.Secret, error) { + return secretsClient.getSecretFromManagementNamespace(ctx, name) +} + +func GetSecretForResource(ctx context.Context, namespace, name string) (*v1.Secret, error) { + return secretsClient.getSecretForResource(ctx, namespace, name) +} + +func (sr *secretClient) getSecretFromManagementNamespace(ctx context.Context, name string) (*v1.Secret, error) { secretForResource := &v1.Secret{} - sr.Log.Info(fmt.Sprintf("Searching for secret name %s in namespace %s", - name, sr.ManagementNamespace)) - err := sr.Client.Get(ctx, types.NamespacedName{Name: name, Namespace: sr.ManagementNamespace}, secretForResource) + sr.Log.Info(fmt.Sprintf("Searching for secret %s in management namespace %s", name, sr.ManagementNamespace)) + err := sr.getWithClientFallback(ctx, types.NamespacedName{Name: name, Namespace: sr.ManagementNamespace}, secretForResource) if err != nil { - sr.Log.Error(err, fmt.Sprintf("Could not fetch secret named %s", name)) + sr.Log.Error(err, fmt.Sprintf("Could not fetch secret %s from management namespace %s", name, sr.ManagementNamespace)) return nil, err } return secretForResource, nil } -func (sr *SecretResolver) GetSecretForResource(ctx context.Context, namespace, name string) (*v1.Secret, error) { +func (sr *secretClient) getSecretForResource(ctx context.Context, namespace, name string) (*v1.Secret, error) { secretForResource := &v1.Secret{} // search namespace secret if sr.EnableNamespaceSecrets { - sr.Log.Info("Searching for secret in resource namespace", "namespace", namespace, "name", name) - err := sr.Client.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, secretForResource) + sr.Log.Info(fmt.Sprintf("Searching for secret %s in namespace %s", name, namespace)) + err := sr.getWithClientFallback(ctx, types.NamespacedName{Name: name, Namespace: namespace}, secretForResource) if err == nil { return secretForResource, nil } if client.IgnoreNotFound(err) != nil { - sr.Log.Error(err, "Could not fetch secret in resource namespace") + sr.Log.Error(err, fmt.Sprintf("Could not fetch secret %s from namespace %s", name, namespace)) return nil, err } } // secret not found in resource namespace, search for namespace-specific secret in management namespace - sr.Log.Info("Searching for namespace secret in management namespace", "namespace", namespace, "managementNamespace", sr.ManagementNamespace, "name", name) - err := sr.Client.Get(ctx, types.NamespacedName{Namespace: sr.ManagementNamespace, Name: fmt.Sprintf("%s-%s", namespace, name)}, secretForResource) + sr.Log.Info(fmt.Sprintf("Searching a secret for namespace %s in the management namespace %s", namespace, sr.ManagementNamespace)) + err := sr.getWithClientFallback(ctx, types.NamespacedName{Namespace: sr.ManagementNamespace, Name: fmt.Sprintf("%s-%s", namespace, name)}, secretForResource) if err == nil { return secretForResource, nil } if client.IgnoreNotFound(err) != nil { - sr.Log.Error(err, "Could not fetch secret in management namespace") + sr.Log.Error(err, fmt.Sprintf("Could not fetch secret %s-%s in the management namespace %s", namespace, name, sr.ManagementNamespace)) return nil, err } @@ -72,13 +103,31 @@ func (sr *SecretResolver) GetSecretForResource(ctx context.Context, namespace, n return sr.getDefaultSecret(ctx, name) } -func (sr *SecretResolver) getDefaultSecret(ctx context.Context, name string) (*v1.Secret, error) { +func (sr *secretClient) getDefaultSecret(ctx context.Context, name string) (*v1.Secret, error) { secretForResource := &v1.Secret{} - sr.Log.Info("Searching for cluster secret", "releaseNamespace", sr.ReleaseNamespace, "name", name) - err := sr.Client.Get(ctx, types.NamespacedName{Namespace: sr.ReleaseNamespace, Name: name}, secretForResource) + sr.Log.Info(fmt.Sprintf("Searching for cluster secret %s in releaseNamespace %s", name, sr.ReleaseNamespace)) + err := sr.getWithClientFallback(ctx, types.NamespacedName{Namespace: sr.ReleaseNamespace, Name: name}, secretForResource) if err != nil { - sr.Log.Error(err, "Could not fetch cluster secret") + sr.Log.Error(err, fmt.Sprintf("Could not fetch cluster secret %s from releaseNamespace %s", name, sr.ReleaseNamespace)) return nil, err } return secretForResource, nil } + +func (sr *secretClient) getWithClientFallback(ctx context.Context, key types.NamespacedName, secretForResource *v1.Secret) error { + err := sr.Client.Get(ctx, key, secretForResource) + if err != nil { + if errors.IsNotFound(err) && sr.LimitedCacheEnabled { + sr.Log.Info(fmt.Sprintf("secret %s not found in cache, falling back to non-cached client", key.String())) + err = sr.NonCachedClient.Get(ctx, key, secretForResource) + if err != nil { + return err + } + sr.Log.Info(fmt.Sprintf("secret %s found using non-cached client", key.String())) + return nil + } + return err + } + + return nil +} diff --git a/internal/utils/secret_resolver_test.go b/internal/utils/secret_resolver_test.go index feab4a40..6646c2cf 100644 --- a/internal/utils/secret_resolver_test.go +++ b/internal/utils/secret_resolver_test.go @@ -1,29 +1,27 @@ package utils import ( - "context" - + "github.com/SAP/sap-btp-service-operator/internal/config" "github.com/google/uuid" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "fmt" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" - - "fmt" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) // +kubebuilder:docs-gen:collapse=Imports var _ = Describe("Secrets Resolver", func() { - - var ctx context.Context - var resolver *SecretResolver - var expectedClientID string - var secret *corev1.Secret + var ( + expectedClientID string + secret *corev1.Secret + ) createSecret := func(namePrefix string, namespace string) *corev1.Secret { var name string @@ -63,26 +61,23 @@ var _ = Describe("Secrets Resolver", func() { } validateSecretResolved := func() { - resolvedSecret, err := resolver.GetSecretForResource(ctx, testNamespace, SAPBTPOperatorSecretName) + resolvedSecret, err := secretsClient.getSecretForResource(ctx, testNamespace, SAPBTPOperatorSecretName) Expect(err).ToNot(HaveOccurred()) Expect(resolvedSecret).ToNot(BeNil()) Expect(string(resolvedSecret.Data["clientid"])).To(Equal(expectedClientID)) } validateSecretNotResolved := func() { - _, err := resolver.GetSecretForResource(ctx, testNamespace, SAPBTPOperatorSecretName) + _, err := secretsClient.getSecretForResource(ctx, testNamespace, SAPBTPOperatorSecretName) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("not found")) } BeforeEach(func() { - ctx = context.Background() - resolver = &SecretResolver{ + InitializeSecretsClient(k8sClient, nil, config.Config{ ManagementNamespace: managementNamespace, ReleaseNamespace: managementNamespace, - Log: logf.Log.WithName("SecretResolver"), - Client: k8sClient, - } + }) }) AfterEach(func() { @@ -104,7 +99,7 @@ var _ = Describe("Secrets Resolver", func() { }) Context("Namespace secrets enabled", func() { BeforeEach(func() { - resolver.EnableNamespaceSecrets = true + secretsClient.EnableNamespaceSecrets = true }) It("should resolve the secret", func() { fmt.Printf("secret %v", secret) @@ -180,10 +175,47 @@ var _ = Describe("Secrets Resolver", func() { }) It("should resolve the secret", func() { - resolvedSecret, err := resolver.GetSecretFromManagementNamespace(ctx, secret.Name) + resolvedSecret, err := secretsClient.getSecretFromManagementNamespace(ctx, secret.Name) Expect(err).ToNot(HaveOccurred()) Expect(resolvedSecret).ToNot(BeNil()) Expect(string(resolvedSecret.Data["clientid"])).To(Equal(expectedClientID)) }) }) + + Context("getWithClientFallback unit", func() { + When("LimitedCacheEnabled is false", func() { + It("should not fallback to NonCachedClient", func() { + secretsClient.NonCachedClient = nil // we will get nil pointer in case of fallback + err := secretsClient.getWithClientFallback(ctx, types.NamespacedName{Name: "some-name", Namespace: testNamespace}, &corev1.Secret{}) + Expect(err).To(HaveOccurred()) + }) + }) + + When("LimitedCacheEnabled is true", func() { + It("should fallback to NonCachedClient and fail if not found", func() { + secretsClient.LimitedCacheEnabled = true + secretsClient.NonCachedClient = fake.NewFakeClient() + err := secretsClient.getWithClientFallback(ctx, types.NamespacedName{Name: "some-name", Namespace: testNamespace}, &corev1.Secret{}) + Expect(err).To(HaveOccurred()) + }) + + It("should fallback to NonCachedClient and succeed if found", func() { + secretsClient.LimitedCacheEnabled = true + fakeClient := fake.NewFakeClient(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-name", + Namespace: testNamespace, + }, + Data: map[string][]byte{ + "key": []byte("value"), + }, + }) + secretsClient.NonCachedClient = fakeClient + searchedSecret := &corev1.Secret{} + err := secretsClient.getWithClientFallback(ctx, types.NamespacedName{Name: "some-name", Namespace: testNamespace}, searchedSecret) + Expect(err).ToNot(HaveOccurred()) + Expect(searchedSecret.Data).To(HaveKey("key")) + }) + }) + }) }) diff --git a/internal/utils/sm_utils.go b/internal/utils/sm_utils.go index e09872f9..cb0855d2 100644 --- a/internal/utils/sm_utils.go +++ b/internal/utils/sm_utils.go @@ -9,14 +9,14 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -func GetSMClient(ctx context.Context, secretResolver *SecretResolver, resourceNamespace, btpAccessSecretName string) (sm.Client, error) { +func GetSMClient(ctx context.Context, resourceNamespace, btpAccessSecretName string) (sm.Client, error) { log := GetLogger(ctx) if len(btpAccessSecretName) > 0 { - return getBTPAccessClient(ctx, secretResolver, btpAccessSecretName) + return getBTPAccessClient(ctx, btpAccessSecretName) } - secret, err := secretResolver.GetSecretForResource(ctx, resourceNamespace, SAPBTPOperatorSecretName) + secret, err := GetSecretForResource(ctx, resourceNamespace, SAPBTPOperatorSecretName) if err != nil { return nil, err } @@ -39,7 +39,7 @@ func GetSMClient(ctx context.Context, secretResolver *SecretResolver, resourceNa //backward compatibility (tls data in a dedicated secret) if len(clientConfig.ClientSecret) == 0 && (len(clientConfig.TLSPrivateKey) == 0 || len(clientConfig.TLSCertKey) == 0) { - tlsSecret, err := secretResolver.GetSecretForResource(ctx, resourceNamespace, SAPBTPOperatorTLSSecretName) + tlsSecret, err := GetSecretForResource(ctx, resourceNamespace, SAPBTPOperatorTLSSecretName) if client.IgnoreNotFound(err) != nil { return nil, err } @@ -57,9 +57,9 @@ func GetSMClient(ctx context.Context, secretResolver *SecretResolver, resourceNa return sm.NewClient(ctx, clientConfig, nil) } -func getBTPAccessClient(ctx context.Context, secretResolver *SecretResolver, secretName string) (sm.Client, error) { +func getBTPAccessClient(ctx context.Context, secretName string) (sm.Client, error) { log := GetLogger(ctx) - secret, err := secretResolver.GetSecretFromManagementNamespace(ctx, secretName) + secret, err := GetSecretFromManagementNamespace(ctx, secretName) if err != nil { return nil, err } diff --git a/internal/utils/sm_utils_test.go b/internal/utils/sm_utils_test.go index 520a15bd..754dcb3d 100644 --- a/internal/utils/sm_utils_test.go +++ b/internal/utils/sm_utils_test.go @@ -1,26 +1,25 @@ package utils import ( + "github.com/SAP/sap-btp-service-operator/internal/config" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" ) var _ = Describe("SM Utils", func() { - var resolver *SecretResolver - var secret *corev1.Secret - var tlsSecret *corev1.Secret + var ( + secret *corev1.Secret + tlsSecret *corev1.Secret + ) BeforeEach(func() { - resolver = &SecretResolver{ + InitializeSecretsClient(k8sClient, nil, config.Config{ ManagementNamespace: managementNamespace, ReleaseNamespace: managementNamespace, - Log: logf.Log.WithName("SecretResolver"), - Client: k8sClient, - } + }) }) Context("GetSMClient", func() { @@ -56,7 +55,7 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, secret)).To(Succeed()) }) It("should succeed", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "") + client, err := GetSMClient(ctx, testNamespace, "") Expect(err).ToNot(HaveOccurred()) Expect(client).ToNot(BeNil()) }) @@ -80,7 +79,7 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, secret)).To(Succeed()) }) It("should succeed", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "") + client, err := GetSMClient(ctx, testNamespace, "") Expect(err).ToNot(HaveOccurred()) Expect(client).ToNot(BeNil()) }) @@ -103,7 +102,7 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, secret)).To(Succeed()) }) It("should return error", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "") + client, err := GetSMClient(ctx, testNamespace, "") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) Expect(client).To(BeNil()) @@ -126,7 +125,7 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, secret)).To(Succeed()) }) It("should return error", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "") + client, err := GetSMClient(ctx, testNamespace, "") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) Expect(client).To(BeNil()) @@ -148,7 +147,7 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, secret)).To(Succeed()) }) It("should return error", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "") + client, err := GetSMClient(ctx, testNamespace, "") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) Expect(client).To(BeNil()) @@ -186,14 +185,14 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, tlsSecret)).To(Succeed()) }) It("should succeed", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "") + client, err := GetSMClient(ctx, testNamespace, "") Expect(err).ToNot(HaveOccurred()) //tls: failed to find any PEM data in key input Expect(client).ToNot(BeNil()) }) }) When("tls secret not found", func() { It("should return error", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "") + client, err := GetSMClient(ctx, testNamespace, "") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) Expect(client).To(BeNil()) @@ -213,7 +212,7 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, tlsSecret)).To(Succeed()) }) It("should return error", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "") + client, err := GetSMClient(ctx, testNamespace, "") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) Expect(client).To(BeNil()) @@ -240,7 +239,7 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, secret)).To(Succeed()) }) It("should succeed", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "my-btp-access-secret") + client, err := GetSMClient(ctx, testNamespace, "my-btp-access-secret") Expect(err).ToNot(HaveOccurred()) Expect(client).ToNot(BeNil()) }) @@ -263,7 +262,7 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, secret)).To(Succeed()) }) It("should return error", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "my-btp-access-secret") + client, err := GetSMClient(ctx, testNamespace, "my-btp-access-secret") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) Expect(client).To(BeNil()) @@ -286,7 +285,7 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, secret)).To(Succeed()) }) It("should return error", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "my-btp-access-secret") + client, err := GetSMClient(ctx, testNamespace, "my-btp-access-secret") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) Expect(client).To(BeNil()) @@ -308,7 +307,7 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, secret)).To(Succeed()) }) It("should return error", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "my-btp-access-secret") + client, err := GetSMClient(ctx, testNamespace, "my-btp-access-secret") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) Expect(client).To(BeNil()) @@ -335,7 +334,7 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, secret)).To(Succeed()) }) It("should succeed", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "my-btp-access-secret") + client, err := GetSMClient(ctx, testNamespace, "my-btp-access-secret") Expect(err).ToNot(HaveOccurred()) Expect(client).ToNot(BeNil()) }) @@ -355,7 +354,7 @@ var _ = Describe("SM Utils", func() { Expect(k8sClient.Create(ctx, tlsSecret)).To(Succeed()) }) It("should return error", func() { - client, err := GetSMClient(ctx, resolver, testNamespace, "my-btp-access-secret") + client, err := GetSMClient(ctx, testNamespace, "my-btp-access-secret") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) Expect(client).To(BeNil()) diff --git a/main.go b/main.go index 4660cb2e..b734c347 100644 --- a/main.go +++ b/main.go @@ -21,17 +21,17 @@ import ( "flag" "os" - "github.com/SAP/sap-btp-service-operator/api/v1/webhooks" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "sigs.k8s.io/controller-runtime/pkg/metrics/server" "k8s.io/apimachinery/pkg/labels" - "github.com/SAP/sap-btp-service-operator/internal/utils" + "github.com/SAP/sap-btp-service-operator/api/v1/webhooks" v1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/SAP/sap-btp-service-operator/internal/utils" "k8s.io/client-go/rest" @@ -40,8 +40,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/healthz" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/SAP/sap-btp-service-operator/internal/config" "k8s.io/apimachinery/pkg/runtime" @@ -128,6 +126,7 @@ func main() { } } } + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), mgrOptions) if err != nil { setupLog.Error(err, "unable to start manager") @@ -141,34 +140,36 @@ func main() { panic(fmt.Sprintf("ClusterID changed, which is not supported. Please redeploy with --set cluster.id=%s", config.Get().InitialClusterID)) } - secretResolver := &utils.SecretResolver{ - ManagementNamespace: config.Get().ManagementNamespace, - ReleaseNamespace: config.Get().ReleaseNamespace, - EnableNamespaceSecrets: config.Get().EnableNamespaceSecrets, - Client: mgr.GetClient(), - Log: logf.Log.WithName("secret-resolver"), + var nonCachedClient client.Client + if config.Get().EnableLimitedCache { + var clErr error + nonCachedClient, clErr = client.New(mgr.GetConfig(), client.Options{Scheme: scheme}) + if clErr != nil { + setupLog.Error(clErr, "unable to create non cached client") + os.Exit(1) + } } + utils.InitializeSecretsClient(mgr.GetClient(), nonCachedClient, config.Get()) + if err = (&controllers.ServiceInstanceReconciler{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("controllers").WithName("ServiceInstance"), - Scheme: mgr.GetScheme(), - Config: config.Get(), - SecretResolver: secretResolver, - Recorder: mgr.GetEventRecorderFor("ServiceInstance"), - GetSMClient: utils.GetSMClient, + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("ServiceInstance"), + Scheme: mgr.GetScheme(), + Config: config.Get(), + Recorder: mgr.GetEventRecorderFor("ServiceInstance"), + GetSMClient: utils.GetSMClient, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ServiceInstance") os.Exit(1) } if err = (&controllers.ServiceBindingReconciler{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("controllers").WithName("ServiceBinding"), - Scheme: mgr.GetScheme(), - Config: config.Get(), - SecretResolver: secretResolver, - Recorder: mgr.GetEventRecorderFor("ServiceBinding"), - GetSMClient: utils.GetSMClient, + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("ServiceBinding"), + Scheme: mgr.GetScheme(), + Config: config.Get(), + Recorder: mgr.GetEventRecorderFor("ServiceBinding"), + GetSMClient: utils.GetSMClient, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ServiceBinding") os.Exit(1) @@ -208,8 +209,8 @@ func createClusterSecret(client client.Client) { clusterSecret := &v1.Secret{} clusterSecret.Name = "sap-btp-operator-clusterid" clusterSecret.Namespace = config.Get().ReleaseNamespace - clusterSecret.Labels = map[string]string{common.ManagedByBTPOperatorLabel: "true"} clusterSecret.StringData = map[string]string{"INITIAL_CLUSTER_ID": config.Get().ClusterID} + clusterSecret.Labels = map[string]string{common.ManagedByBTPOperatorLabel: "true"} if err := client.Create(context.Background(), clusterSecret); err != nil { setupLog.Error(err, "failed to create cluster secret") }