From db8b38cddc9fc87c0cd14f2c7716539019622473 Mon Sep 17 00:00:00 2001 From: kerenlahav <45451976+kerenlahav@users.noreply.github.com> Date: Sun, 14 Jan 2024 14:50:58 +0200 Subject: [PATCH] code refactor (#384) --- api/{ => common}/common.go | 2 +- api/common/consts.go | 36 ++ api/v1/servicebinding_types.go | 8 +- api/v1/servicebinding_types_test.go | 8 +- api/v1/servicebinding_validating_webhook.go | 14 +- .../servicebinding_validating_webhook_test.go | 14 +- api/v1/serviceinstance_types.go | 8 +- api/v1/serviceinstance_types_test.go | 8 +- api/v1/serviceinstance_validating_webhook.go | 18 +- ...serviceinstance_validating_webhook_test.go | 40 +- .../serviceinstance_mutating_webhook.go | 10 +- api/v1alpha1/servicebinding_types.go | 8 +- api/v1alpha1/servicebinding_types_test.go | 6 +- api/v1alpha1/serviceinstance_types.go | 8 +- api/v1alpha1/serviceinstance_types_test.go | 8 +- client/sm/client.go | 11 +- client/sm/client_config.go | 11 + client/sm/client_config_test.go | 91 ++++ client/sm/client_test.go | 35 ++ controllers/base_controller.go | 435 ------------------ controllers/controller_util.go | 120 ----- controllers/controller_util_test.go | 80 ---- controllers/servicebinding_controller.go | 280 +++++------ controllers/servicebinding_controller_test.go | 99 ++-- controllers/serviceinstance_controller.go | 211 +++++---- .../serviceinstance_controller_test.go | 129 +++--- controllers/suite_test.go | 47 +- internal/config/config.go | 4 +- internal/utils/condition_utils.go | 244 ++++++++++ internal/utils/condition_utils_test.go | 339 ++++++++++++++ internal/utils/controller_util.go | 222 +++++++++ internal/utils/controller_util_test.go | 218 +++++++++ {controllers => internal/utils}/parameters.go | 42 +- internal/utils/parameters_test.go | 23 + .../resolver.go => utils/secret_resolver.go} | 24 +- .../secret_resolver_test.go} | 27 +- internal/utils/sm_utils.go | 81 ++++ internal/utils/sm_utils_test.go | 392 ++++++++++++++++ internal/{secrets => utils}/suite_test.go | 50 +- main.go | 35 +- 40 files changed, 2274 insertions(+), 1172 deletions(-) rename api/{ => common}/common.go (99%) create mode 100644 api/common/consts.go create mode 100644 client/sm/client_config_test.go delete mode 100644 controllers/base_controller.go delete mode 100644 controllers/controller_util.go delete mode 100644 controllers/controller_util_test.go create mode 100644 internal/utils/condition_utils.go create mode 100644 internal/utils/condition_utils_test.go create mode 100644 internal/utils/controller_util.go create mode 100644 internal/utils/controller_util_test.go rename {controllers => internal/utils}/parameters.go (94%) create mode 100644 internal/utils/parameters_test.go rename internal/{secrets/resolver.go => utils/secret_resolver.go} (80%) rename internal/{secrets/resolver_test.go => utils/secret_resolver_test.go} (87%) create mode 100644 internal/utils/sm_utils.go create mode 100644 internal/utils/sm_utils_test.go rename internal/{secrets => utils}/suite_test.go (67%) diff --git a/api/common.go b/api/common/common.go similarity index 99% rename from api/common.go rename to api/common/common.go index d2e2b02e..06940608 100644 --- a/api/common.go +++ b/api/common/common.go @@ -1,4 +1,4 @@ -package api +package common import ( "fmt" diff --git a/api/common/consts.go b/api/common/consts.go new file mode 100644 index 00000000..7a99cfd5 --- /dev/null +++ b/api/common/consts.go @@ -0,0 +1,36 @@ +package common + +const ( + NamespaceLabel = "_namespace" + K8sNameLabel = "_k8sname" + ClusterIDLabel = "_clusterid" + + Created = "Created" + Updated = "Updated" + Deleted = "Deleted" + Provisioned = "Provisioned" + NotProvisioned = "NotProvisioned" + + CreateInProgress = "CreateInProgress" + UpdateInProgress = "UpdateInProgress" + DeleteInProgress = "DeleteInProgress" + InProgress = "InProgress" + Finished = "Finished" + + CreateFailed = "CreateFailed" + UpdateFailed = "UpdateFailed" + DeleteFailed = "DeleteFailed" + Failed = "Failed" + ShareFailed = "ShareFailed" + ShareSucceeded = "ShareSucceeded" + ShareNotSupported = "ShareNotSupported" + UnShareFailed = "UnShareFailed" + UnShareSucceeded = "UnShareSucceeded" + + Blocked = "Blocked" + Unknown = "Unknown" + + // Cred Rotation + CredPreparing = "Preparing" + CredRotating = "Rotating" +) diff --git a/api/v1/servicebinding_types.go b/api/v1/servicebinding_types.go index 22466678..283ca78f 100644 --- a/api/v1/servicebinding_types.go +++ b/api/v1/servicebinding_types.go @@ -17,7 +17,7 @@ limitations under the License. package v1 import ( - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" "github.com/SAP/sap-btp-service-operator/client/sm/types" v1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -152,8 +152,8 @@ func (sb *ServiceBinding) SetConditions(conditions []metav1.Condition) { sb.Status.Conditions = conditions } -func (sb *ServiceBinding) GetControllerName() api.ControllerName { - return api.ServiceBindingController +func (sb *ServiceBinding) GetControllerName() common.ControllerName { + return common.ServiceBindingController } func (sb *ServiceBinding) GetParameters() *runtime.RawExtension { @@ -176,7 +176,7 @@ func (sb *ServiceBinding) SetObservedGeneration(newObserved int64) { sb.Status.ObservedGeneration = newObserved } -func (sb *ServiceBinding) DeepClone() api.SAPBTPResource { +func (sb *ServiceBinding) DeepClone() common.SAPBTPResource { return sb.DeepCopy() } diff --git a/api/v1/servicebinding_types_test.go b/api/v1/servicebinding_types_test.go index 14ce9c5a..7cc9f516 100644 --- a/api/v1/servicebinding_types_test.go +++ b/api/v1/servicebinding_types_test.go @@ -1,7 +1,7 @@ package v1 import ( - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/meta" @@ -14,7 +14,7 @@ var _ = Describe("Service Binding Type Test", func() { BeforeEach(func() { binding = getBinding() conditions := binding.GetConditions() - lastOpCondition := metav1.Condition{Type: api.ConditionSucceeded, Status: metav1.ConditionTrue, Reason: "reason", Message: "message"} + lastOpCondition := metav1.Condition{Type: common.ConditionSucceeded, Status: metav1.ConditionTrue, Reason: "reason", Message: "message"} meta.SetStatusCondition(&conditions, lastOpCondition) binding.SetConditions(conditions) }) @@ -63,7 +63,7 @@ var _ = Describe("Service Binding Type Test", func() { }) It("should return controller name", func() { - Expect(binding.GetControllerName()).To(Equal(api.ServiceBindingController)) + Expect(binding.GetControllerName()).To(Equal(common.ServiceBindingController)) }) It("should update observed generation", func() { @@ -100,7 +100,7 @@ var _ = Describe("Service Binding Type Test", func() { It("should update annotation", func() { annotation := map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "true", + common.IgnoreNonTransientErrorAnnotation: "true", } binding.SetAnnotations(annotation) Expect(binding.GetAnnotations()).To(Equal(annotation)) diff --git a/api/v1/servicebinding_validating_webhook.go b/api/v1/servicebinding_validating_webhook.go index 5c488652..8ce45a96 100644 --- a/api/v1/servicebinding_validating_webhook.go +++ b/api/v1/servicebinding_validating_webhook.go @@ -21,13 +21,13 @@ import ( "reflect" "time" + "github.com/SAP/sap-btp-service-operator/api/common" + "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "github.com/SAP/sap-btp-service-operator/api" ) // log is for logging in this package. @@ -76,7 +76,7 @@ func (sb *ServiceBinding) ValidateUpdate(old runtime.Object) (admission.Warnings oldBinding := old.(*ServiceBinding) isStale := false if oldBinding.Labels != nil { - if _, ok := oldBinding.Labels[api.StaleBindingIDLabel]; ok { + if _, ok := oldBinding.Labels[common.StaleBindingIDLabel]; ok { if sb.Spec.CredRotationPolicy.Enabled { return nil, fmt.Errorf("enabling cred rotation for rotated binding is not allowed") } @@ -95,10 +95,10 @@ func (sb *ServiceBinding) ValidateUpdate(old runtime.Object) (admission.Warnings } func (sb *ServiceBinding) validateRotationLabels(old *ServiceBinding) bool { - if sb.Labels[api.StaleBindingIDLabel] != old.Labels[api.StaleBindingIDLabel] { + if sb.Labels[common.StaleBindingIDLabel] != old.Labels[common.StaleBindingIDLabel] { return false } - return sb.Labels[api.StaleBindingRotationOfLabel] == old.Labels[api.StaleBindingRotationOfLabel] + return sb.Labels[common.StaleBindingRotationOfLabel] == old.Labels[common.StaleBindingRotationOfLabel] } func (sb *ServiceBinding) specChanged(oldBinding *ServiceBinding) bool { @@ -134,8 +134,8 @@ func (sb *ServiceBinding) validateCredRotatingConfig() error { func (sb *ServiceBinding) validateAnnotations() error { if sb.Annotations != nil { - if _, ok := sb.Annotations[api.IgnoreNonTransientErrorAnnotation]; ok { - return fmt.Errorf(AnnotationNotSupportedError, api.IgnoreNonTransientErrorAnnotation) + if _, ok := sb.Annotations[common.IgnoreNonTransientErrorAnnotation]; ok { + return fmt.Errorf(AnnotationNotSupportedError, common.IgnoreNonTransientErrorAnnotation) } } return nil diff --git a/api/v1/servicebinding_validating_webhook_test.go b/api/v1/servicebinding_validating_webhook_test.go index f2f631fb..31288d73 100644 --- a/api/v1/servicebinding_validating_webhook_test.go +++ b/api/v1/servicebinding_validating_webhook_test.go @@ -2,7 +2,7 @@ package v1 import ( "fmt" - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/runtime" @@ -22,11 +22,11 @@ var _ = Describe("Service Binding Webhook Test", func() { }) It("should failed", func() { binding.Annotations = map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "false", + common.IgnoreNonTransientErrorAnnotation: "false", } _, err := binding.ValidateCreate() Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring(fmt.Sprintf(AnnotationNotSupportedError, api.IgnoreNonTransientErrorAnnotation))) + Expect(err.Error()).To(ContainSubstring(fmt.Sprintf(AnnotationNotSupportedError, common.IgnoreNonTransientErrorAnnotation))) }) }) @@ -103,9 +103,9 @@ var _ = Describe("Service Binding Webhook Test", func() { }) It("should fail on update with stale label", func() { - binding.Labels = map[string]string{api.StaleBindingIDLabel: "true"} + binding.Labels = map[string]string{common.StaleBindingIDLabel: "true"} newBinding.Spec.ParametersFrom[0].SecretKeyRef.Name = "newName" - newBinding.Labels = map[string]string{api.StaleBindingIDLabel: "true"} + newBinding.Labels = map[string]string{common.StaleBindingIDLabel: "true"} _, err := newBinding.ValidateUpdate(binding) Expect(err).To(HaveOccurred()) }) @@ -122,11 +122,11 @@ var _ = Describe("Service Binding Webhook Test", func() { When("Annotation changed", func() { It("should fail", func() { newBinding.Annotations = map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "false", + common.IgnoreNonTransientErrorAnnotation: "false", } _, err := newBinding.ValidateUpdate(binding) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring(fmt.Sprintf(AnnotationNotSupportedError, api.IgnoreNonTransientErrorAnnotation))) + Expect(err.Error()).To(ContainSubstring(fmt.Sprintf(AnnotationNotSupportedError, common.IgnoreNonTransientErrorAnnotation))) }) }) diff --git a/api/v1/serviceinstance_types.go b/api/v1/serviceinstance_types.go index f56b6e4a..4d4845ac 100644 --- a/api/v1/serviceinstance_types.go +++ b/api/v1/serviceinstance_types.go @@ -17,7 +17,7 @@ limitations under the License. package v1 import ( - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" "github.com/SAP/sap-btp-service-operator/client/sm/types" v1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -150,8 +150,8 @@ func (si *ServiceInstance) SetConditions(conditions []metav1.Condition) { si.Status.Conditions = conditions } -func (si *ServiceInstance) GetControllerName() api.ControllerName { - return api.ServiceInstanceController +func (si *ServiceInstance) GetControllerName() common.ControllerName { + return common.ServiceInstanceController } func (si *ServiceInstance) GetParameters() *runtime.RawExtension { @@ -174,7 +174,7 @@ func (si *ServiceInstance) SetObservedGeneration(newObserved int64) { si.Status.ObservedGeneration = newObserved } -func (si *ServiceInstance) DeepClone() api.SAPBTPResource { +func (si *ServiceInstance) DeepClone() common.SAPBTPResource { return si.DeepCopy() } diff --git a/api/v1/serviceinstance_types_test.go b/api/v1/serviceinstance_types_test.go index 14d845f1..2d1fa411 100644 --- a/api/v1/serviceinstance_types_test.go +++ b/api/v1/serviceinstance_types_test.go @@ -1,7 +1,7 @@ package v1 import ( - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/meta" @@ -14,7 +14,7 @@ var _ = Describe("Service Instance Type Test", func() { BeforeEach(func() { instance = getInstance() conditions := instance.GetConditions() - lastOpCondition := metav1.Condition{Type: api.ConditionSucceeded, Status: metav1.ConditionTrue, Reason: "reason", Message: "message"} + lastOpCondition := metav1.Condition{Type: common.ConditionSucceeded, Status: metav1.ConditionTrue, Reason: "reason", Message: "message"} meta.SetStatusCondition(&conditions, lastOpCondition) instance.SetConditions(conditions) }) @@ -93,7 +93,7 @@ var _ = Describe("Service Instance Type Test", func() { }) It("should return controller name", func() { - Expect(instance.GetControllerName()).To(Equal(api.ServiceInstanceController)) + Expect(instance.GetControllerName()).To(Equal(common.ServiceInstanceController)) }) It("should update observed generation", func() { @@ -124,7 +124,7 @@ var _ = Describe("Service Instance Type Test", func() { It("should update annotation", func() { annotation := map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "true", + common.IgnoreNonTransientErrorAnnotation: "true", } instance.SetAnnotations(annotation) Expect(instance.GetAnnotations()).To(Equal(annotation)) diff --git a/api/v1/serviceinstance_validating_webhook.go b/api/v1/serviceinstance_validating_webhook.go index 3d18905e..c27d5f09 100644 --- a/api/v1/serviceinstance_validating_webhook.go +++ b/api/v1/serviceinstance_validating_webhook.go @@ -21,13 +21,13 @@ import ( "strings" "time" + "github.com/SAP/sap-btp-service-operator/api/common" + "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "github.com/SAP/sap-btp-service-operator/api" ) const ( @@ -65,7 +65,7 @@ func (si *ServiceInstance) ValidateUpdate(old runtime.Object) (warnings admissio func (si *ServiceInstance) ValidateDelete() (warnings admission.Warnings, err error) { serviceinstancelog.Info("validate delete", "name", si.Name) if si.Annotations != nil { - preventDeletion, ok := si.Annotations[api.PreventDeletion] + preventDeletion, ok := si.Annotations[common.PreventDeletion] if ok && strings.ToLower(preventDeletion) == "true" { return nil, fmt.Errorf("service instance '%s' is marked with \"prevent deletion\"", si.Name) } @@ -74,16 +74,16 @@ func (si *ServiceInstance) ValidateDelete() (warnings admission.Warnings, err er } func (si *ServiceInstance) validateNonTransientTimestampAnnotation() error { - if len(si.Annotations) > 0 && len(si.Annotations[api.IgnoreNonTransientErrorAnnotation]) > 0 { - serviceinstancelog.Info(fmt.Sprintf("%s annotation exist, validating %s annotation", api.IgnoreNonTransientErrorAnnotation, api.IgnoreNonTransientErrorTimestampAnnotation)) - annotationTime, err := time.Parse(time.RFC3339, si.Annotations[api.IgnoreNonTransientErrorTimestampAnnotation]) + if len(si.Annotations) > 0 && len(si.Annotations[common.IgnoreNonTransientErrorAnnotation]) > 0 { + serviceinstancelog.Info(fmt.Sprintf("%s annotation exist, validating %s annotation", common.IgnoreNonTransientErrorAnnotation, common.IgnoreNonTransientErrorTimestampAnnotation)) + annotationTime, err := time.Parse(time.RFC3339, si.Annotations[common.IgnoreNonTransientErrorTimestampAnnotation]) if err != nil { - serviceinstancelog.Error(err, fmt.Sprintf("failed to parse %s", api.IgnoreNonTransientErrorTimestampAnnotation)) - return fmt.Errorf(annotationNotValidTimestampError, api.IgnoreNonTransientErrorTimestampAnnotation) + serviceinstancelog.Error(err, fmt.Sprintf("failed to parse %s", common.IgnoreNonTransientErrorTimestampAnnotation)) + return fmt.Errorf(annotationNotValidTimestampError, common.IgnoreNonTransientErrorTimestampAnnotation) } if time.Since(annotationTime) < 0 { - return fmt.Errorf(annotationInFutureError, api.IgnoreNonTransientErrorTimestampAnnotation) + return fmt.Errorf(annotationInFutureError, common.IgnoreNonTransientErrorTimestampAnnotation) } } diff --git a/api/v1/serviceinstance_validating_webhook_test.go b/api/v1/serviceinstance_validating_webhook_test.go index 514a1e42..102d5c57 100644 --- a/api/v1/serviceinstance_validating_webhook_test.go +++ b/api/v1/serviceinstance_validating_webhook_test.go @@ -2,7 +2,7 @@ package v1 import ( "fmt" - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "time" @@ -29,8 +29,8 @@ var _ = Describe("Service Instance Webhook Test", func() { When("service instance IgnoreNonTransientErrorTimestampAnnotation annotation is not set with not a date", func() { It("should return error from webhook", func() { instance.Annotations = map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "true", - api.IgnoreNonTransientErrorTimestampAnnotation: "true", + common.IgnoreNonTransientErrorAnnotation: "true", + common.IgnoreNonTransientErrorTimestampAnnotation: "true", } _, err := instance.ValidateUpdate(instance) Expect(err).To(HaveOccurred()) @@ -40,8 +40,8 @@ var _ = Describe("Service Instance Webhook Test", func() { When("service instance IgnoreNonTransientErrorTimestampAnnotation annotation is not set with future date", func() { It("should return error from webhook", func() { instance.Annotations = map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "true", - api.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Add(48 * time.Hour).Format(time.RFC3339), + common.IgnoreNonTransientErrorAnnotation: "true", + common.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Add(48 * time.Hour).Format(time.RFC3339), } _, err := instance.ValidateUpdate(instance) Expect(err).To(HaveOccurred()) @@ -53,7 +53,7 @@ var _ = Describe("Service Instance Webhook Test", func() { When("service instance is marked as prevent deletion", func() { It("should return error from webhook", func() { instance.Annotations = map[string]string{ - api.PreventDeletion: "true", + common.PreventDeletion: "true", } _, err := instance.ValidateDelete() Expect(err).To(HaveOccurred()) @@ -70,7 +70,7 @@ var _ = Describe("Service Instance Webhook Test", func() { When("service instance prevent deletion annotation is not set with true", func() { It("should not return error from webhook", func() { instance.Annotations = map[string]string{ - api.PreventDeletion: "not-true", + common.PreventDeletion: "not-true", } _, err := instance.ValidateDelete() Expect(err).ToNot(HaveOccurred()) @@ -80,24 +80,24 @@ var _ = Describe("Service Instance Webhook Test", func() { When("service instance IgnoreNonTransientErrorTimestampAnnotation annotation is not set with not a date", func() { It("should not return error from webhook", func() { instance.Annotations = map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "true", - api.IgnoreNonTransientErrorTimestampAnnotation: "true", + common.IgnoreNonTransientErrorAnnotation: "true", + common.IgnoreNonTransientErrorTimestampAnnotation: "true", } _, err := instance.ValidateCreate() Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring(fmt.Sprintf(annotationNotValidTimestampError, api.IgnoreNonTransientErrorTimestampAnnotation))) + Expect(err.Error()).To(ContainSubstring(fmt.Sprintf(annotationNotValidTimestampError, common.IgnoreNonTransientErrorTimestampAnnotation))) }) }) When("service instance IgnoreNonTransientErrorTimestampAnnotation annotation is not set with future date", func() { It("should not return error from webhook", func() { instance.Annotations = map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "true", - api.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Add(48 * time.Hour).Format(time.RFC3339), + common.IgnoreNonTransientErrorAnnotation: "true", + common.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Add(48 * time.Hour).Format(time.RFC3339), } _, err := instance.ValidateCreate() Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring(fmt.Sprintf(annotationInFutureError, api.IgnoreNonTransientErrorTimestampAnnotation))) + Expect(err.Error()).To(ContainSubstring(fmt.Sprintf(annotationInFutureError, common.IgnoreNonTransientErrorTimestampAnnotation))) }) }) @@ -107,8 +107,8 @@ var _ = Describe("Service Instance Webhook Test", func() { Context("validateNonTransientTimestampAnnotation", func() { It("fails when not a valid date", func() { annotation := map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "true", - api.IgnoreNonTransientErrorTimestampAnnotation: "true", + common.IgnoreNonTransientErrorAnnotation: "true", + common.IgnoreNonTransientErrorTimestampAnnotation: "true", } instance.SetAnnotations(annotation) err := instance.validateNonTransientTimestampAnnotation() @@ -118,19 +118,19 @@ var _ = Describe("Service Instance Webhook Test", func() { It("fails when a future date", func() { annotation := map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "true", - api.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Add(48 * time.Hour).Format(time.RFC3339), + common.IgnoreNonTransientErrorAnnotation: "true", + common.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Add(48 * time.Hour).Format(time.RFC3339), } instance.SetAnnotations(annotation) err := instance.validateNonTransientTimestampAnnotation() Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring(fmt.Sprintf(annotationInFutureError, api.IgnoreNonTransientErrorTimestampAnnotation))) + Expect(err.Error()).To(ContainSubstring(fmt.Sprintf(annotationInFutureError, common.IgnoreNonTransientErrorTimestampAnnotation))) }) It("succeeds when timestamp is valid and in the past", func() { annotation := map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "true", - api.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Format(time.RFC3339), + common.IgnoreNonTransientErrorAnnotation: "true", + common.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Format(time.RFC3339), } instance.SetAnnotations(annotation) err := instance.validateNonTransientTimestampAnnotation() diff --git a/api/v1/webhooks/serviceinstance_mutating_webhook.go b/api/v1/webhooks/serviceinstance_mutating_webhook.go index 8ea43fd9..5131685b 100644 --- a/api/v1/webhooks/serviceinstance_mutating_webhook.go +++ b/api/v1/webhooks/serviceinstance_mutating_webhook.go @@ -8,7 +8,7 @@ import ( "reflect" "time" - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" v1admission "k8s.io/api/admission/v1" v1 "k8s.io/api/authentication/v1" @@ -42,11 +42,11 @@ func (s *ServiceInstanceDefaulter) Handle(_ context.Context, req admission.Reque return admission.Errored(http.StatusInternalServerError, err) } - if len(instance.Annotations) > 0 && len(instance.Annotations[api.IgnoreNonTransientErrorAnnotation]) > 0 { - if _, exist := instance.Annotations[api.IgnoreNonTransientErrorTimestampAnnotation]; !exist { + if len(instance.Annotations) > 0 && len(instance.Annotations[common.IgnoreNonTransientErrorAnnotation]) > 0 { + if _, exist := instance.Annotations[common.IgnoreNonTransientErrorTimestampAnnotation]; !exist { annotationValue := time.Now().Format(time.RFC3339) - instancelog.Info(fmt.Sprintf("%s annotation exists, adding %s annotation with value %s", api.IgnoreNonTransientErrorAnnotation, api.IgnoreNonTransientErrorTimestampAnnotation, annotationValue)) - instance.Annotations[api.IgnoreNonTransientErrorTimestampAnnotation] = annotationValue + instancelog.Info(fmt.Sprintf("%s annotation exists, adding %s annotation with value %s", common.IgnoreNonTransientErrorAnnotation, common.IgnoreNonTransientErrorTimestampAnnotation, annotationValue)) + instance.Annotations[common.IgnoreNonTransientErrorTimestampAnnotation] = annotationValue } } diff --git a/api/v1alpha1/servicebinding_types.go b/api/v1alpha1/servicebinding_types.go index 131e95f6..445d7292 100644 --- a/api/v1alpha1/servicebinding_types.go +++ b/api/v1alpha1/servicebinding_types.go @@ -17,7 +17,7 @@ limitations under the License. package v1alpha1 import ( - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" smClientTypes "github.com/SAP/sap-btp-service-operator/client/sm/types" v1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -144,8 +144,8 @@ func (sb *ServiceBinding) SetConditions(conditions []metav1.Condition) { sb.Status.Conditions = conditions } -func (sb *ServiceBinding) GetControllerName() api.ControllerName { - return api.ServiceBindingController +func (sb *ServiceBinding) GetControllerName() common.ControllerName { + return common.ServiceBindingController } func (sb *ServiceBinding) GetParameters() *runtime.RawExtension { @@ -168,7 +168,7 @@ func (sb *ServiceBinding) SetObservedGeneration(newObserved int64) { sb.Status.ObservedGeneration = newObserved } -func (sb *ServiceBinding) DeepClone() api.SAPBTPResource { +func (sb *ServiceBinding) DeepClone() common.SAPBTPResource { return sb.DeepCopy() } diff --git a/api/v1alpha1/servicebinding_types_test.go b/api/v1alpha1/servicebinding_types_test.go index a5166cdf..fc3a44bc 100644 --- a/api/v1alpha1/servicebinding_types_test.go +++ b/api/v1alpha1/servicebinding_types_test.go @@ -1,7 +1,7 @@ package v1alpha1 import ( - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/meta" @@ -14,7 +14,7 @@ var _ = Describe("Service Binding Type Test", func() { BeforeEach(func() { binding = getBinding() conditions := binding.GetConditions() - lastOpCondition := metav1.Condition{Type: api.ConditionSucceeded, Status: metav1.ConditionTrue, Reason: "reason", Message: "message"} + lastOpCondition := metav1.Condition{Type: common.ConditionSucceeded, Status: metav1.ConditionTrue, Reason: "reason", Message: "message"} meta.SetStatusCondition(&conditions, lastOpCondition) binding.SetConditions(conditions) }) @@ -63,7 +63,7 @@ var _ = Describe("Service Binding Type Test", func() { }) It("should return controller name", func() { - Expect(binding.GetControllerName()).To(Equal(api.ServiceBindingController)) + Expect(binding.GetControllerName()).To(Equal(common.ServiceBindingController)) }) It("should update observed generation", func() { diff --git a/api/v1alpha1/serviceinstance_types.go b/api/v1alpha1/serviceinstance_types.go index 04879bb2..c4cc1cc8 100644 --- a/api/v1alpha1/serviceinstance_types.go +++ b/api/v1alpha1/serviceinstance_types.go @@ -17,7 +17,7 @@ limitations under the License. package v1alpha1 import ( - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" smClientTypes "github.com/SAP/sap-btp-service-operator/client/sm/types" v1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -134,8 +134,8 @@ func (in *ServiceInstance) SetConditions(conditions []metav1.Condition) { in.Status.Conditions = conditions } -func (in *ServiceInstance) GetControllerName() api.ControllerName { - return api.ServiceInstanceController +func (in *ServiceInstance) GetControllerName() common.ControllerName { + return common.ServiceInstanceController } func (in *ServiceInstance) GetParameters() *runtime.RawExtension { @@ -158,7 +158,7 @@ func (in *ServiceInstance) SetObservedGeneration(newObserved int64) { in.Status.ObservedGeneration = newObserved } -func (in *ServiceInstance) DeepClone() api.SAPBTPResource { +func (in *ServiceInstance) DeepClone() common.SAPBTPResource { return in.DeepCopy() } diff --git a/api/v1alpha1/serviceinstance_types_test.go b/api/v1alpha1/serviceinstance_types_test.go index 536fa1d6..7c497ff6 100644 --- a/api/v1alpha1/serviceinstance_types_test.go +++ b/api/v1alpha1/serviceinstance_types_test.go @@ -1,7 +1,7 @@ package v1alpha1 import ( - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/meta" @@ -15,7 +15,7 @@ var _ = Describe("Service Instance Type Test", func() { BeforeEach(func() { instance = getInstance() conditions := instance.GetConditions() - lastOpCondition := metav1.Condition{Type: api.ConditionSucceeded, Status: metav1.ConditionTrue, Reason: "reason", Message: "message"} + lastOpCondition := metav1.Condition{Type: common.ConditionSucceeded, Status: metav1.ConditionTrue, Reason: "reason", Message: "message"} meta.SetStatusCondition(&conditions, lastOpCondition) instance.SetConditions(conditions) }) @@ -94,7 +94,7 @@ var _ = Describe("Service Instance Type Test", func() { }) It("should return controller name", func() { - Expect(instance.GetControllerName()).To(Equal(api.ServiceInstanceController)) + Expect(instance.GetControllerName()).To(Equal(common.ServiceInstanceController)) }) It("should update observed generation", func() { @@ -125,7 +125,7 @@ var _ = Describe("Service Instance Type Test", func() { It("should update annotation", func() { annotation := map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "true", + common.IgnoreNonTransientErrorAnnotation: "true", } instance.SetAnnotations(annotation) Expect(instance.GetAnnotations()).To(Equal(annotation)) diff --git a/client/sm/client.go b/client/sm/client.go index 26c21130..f813da53 100644 --- a/client/sm/client.go +++ b/client/sm/client.go @@ -28,7 +28,8 @@ import ( "strconv" "strings" - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" + "github.com/SAP/sap-btp-service-operator/client/sm/types" "github.com/SAP/sap-btp-service-operator/internal/auth" "github.com/SAP/sap-btp-service-operator/internal/httputil" @@ -69,10 +70,10 @@ type Client interface { } type ServiceManagerError struct { - ErrorType string `json:"error,omitempty"` - Description string `json:"description,omitempty"` - StatusCode int `json:"-"` - BrokerError *api.HTTPStatusCodeError `json:"broker_error,omitempty"` + ErrorType string `json:"error,omitempty"` + Description string `json:"description,omitempty"` + StatusCode int `json:"-"` + BrokerError *common.HTTPStatusCodeError `json:"broker_error,omitempty"` } func (e *ServiceManagerError) Error() string { diff --git a/client/sm/client_config.go b/client/sm/client_config.go index f869c7ff..301e8696 100644 --- a/client/sm/client_config.go +++ b/client/sm/client_config.go @@ -27,3 +27,14 @@ type ClientConfig struct { TLSPrivateKey string SSLDisabled bool } + +func (c ClientConfig) IsValid() bool { + if len(c.ClientID) == 0 || len(c.URL) == 0 || len(c.TokenURL) == 0 { + return false + } + if len(c.ClientSecret) == 0 && (len(c.TLSCertKey) == 0 || len(c.TLSPrivateKey) == 0) { + return false + } + + return true +} diff --git a/client/sm/client_config_test.go b/client/sm/client_config_test.go new file mode 100644 index 00000000..2ef75269 --- /dev/null +++ b/client/sm/client_config_test.go @@ -0,0 +1,91 @@ +package sm + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("ClientConfig", func() { + When("valid ClientSecret", func() { + It("returns true", func() { + config := ClientConfig{ + URL: "https://example.com", + TokenURL: "https://example.com/token", + ClientID: "validClientId", + ClientSecret: "validClientSecret", + } + Expect(config.IsValid()).To(BeTrue()) + }) + }) + + When("valid TLSCertKey and TLSPrivateKey", func() { + It("returns true", func() { + config := ClientConfig{ + URL: "https://example.com", + TokenURL: "https://example.com/token", + ClientID: "validClientId", + SSLDisabled: true, + TLSCertKey: "CertKey", + TLSPrivateKey: "PrivateKey", + } + Expect(config.IsValid()).To(BeTrue()) + }) + }) + + When("no ClientSecret", func() { + It("returns false", func() { + config := ClientConfig{ + URL: "https://example.com", + TokenURL: "https://example.com/token", + ClientID: "validClientId", + } + Expect(config.IsValid()).To(BeFalse()) + }) + }) + + When("no TLSCertKey", func() { + It("returns false", func() { + config := ClientConfig{ + URL: "https://example.com", + TokenURL: "https://example.com/token", + ClientID: "validClientId", + SSLDisabled: true, + TLSPrivateKey: "PrivateKey", + } + Expect(config.IsValid()).To(BeFalse()) + }) + }) + + When("no URL", func() { + It("returns false", func() { + config := ClientConfig{ + ClientID: "validClientId", + TokenURL: "https://example.com/token", + ClientSecret: "validClientSecret", + } + Expect(config.IsValid()).To(BeFalse()) + }) + }) + + When("no ClientId", func() { + It("returns false", func() { + config := ClientConfig{ + URL: "https://example.com", + TokenURL: "https://example.com/token", + ClientSecret: "validClientSecret", + } + Expect(config.IsValid()).To(BeFalse()) + }) + }) + + When("no TokenURL", func() { + It("returns false", func() { + config := ClientConfig{ + ClientID: "validClientId", + URL: "https://example.com", + ClientSecret: "validClientSecret", + } + Expect(config.IsValid()).To(BeFalse()) + }) + }) +}) diff --git a/client/sm/client_test.go b/client/sm/client_test.go index ef29c870..fb33cb80 100644 --- a/client/sm/client_test.go +++ b/client/sm/client_test.go @@ -532,6 +532,41 @@ var _ = Describe("Client test", func() { }) }) }) + + Describe("Share/Unshare", func() { + BeforeEach(func() { + responseBody, _ := json.Marshal(instance) + handlerDetails = []HandlerDetails{ + {Method: http.MethodPatch, Path: types.ServiceInstancesURL + "/" + instance.ID, ResponseBody: responseBody, ResponseStatusCode: http.StatusOK}, + } + }) + When("When valid instance is being shared", func() { + It("should be shared successfully", func() { + err := client.ShareInstance(instance.ID, "test-user") + Expect(err).ShouldNot(HaveOccurred()) + }) + }) + + When("When instance is being unshared", func() { + It("should be unshared successfully", func() { + err := client.UnShareInstance(instance.ID, "test-user") + Expect(err).ShouldNot(HaveOccurred()) + }) + }) + + When("When instance fails to be shared", func() { + BeforeEach(func() { + responseBody, _ := json.Marshal(instance) + handlerDetails = []HandlerDetails{ + {Method: http.MethodPatch, Path: types.ServiceInstancesURL + "/" + instance.ID, ResponseBody: responseBody, ResponseStatusCode: http.StatusInternalServerError}, + } + }) + It("returns error", func() { + err := client.UnShareInstance(instance.ID, "test-user") + Expect(err).Should(HaveOccurred()) + }) + }) + }) }) Describe("Bindings", func() { diff --git a/controllers/base_controller.go b/controllers/base_controller.go deleted file mode 100644 index 9bf46185..00000000 --- a/controllers/base_controller.go +++ /dev/null @@ -1,435 +0,0 @@ -package controllers - -import ( - "context" - "errors" - "fmt" - "net/http" - - "github.com/SAP/sap-btp-service-operator/api" - "github.com/SAP/sap-btp-service-operator/client/sm" - smClientTypes "github.com/SAP/sap-btp-service-operator/client/sm/types" - "github.com/SAP/sap-btp-service-operator/internal/config" - "github.com/SAP/sap-btp-service-operator/internal/secrets" - "github.com/go-logr/logr" - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - apimachinerytypes "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/tools/record" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" -) - -const ( - namespaceLabel = "_namespace" - k8sNameLabel = "_k8sname" - clusterIDLabel = "_clusterid" - - Created = "Created" - Updated = "Updated" - Deleted = "Deleted" - Finished = "Finished" - Provisioned = "Provisioned" - NotProvisioned = "NotProvisioned" - - CreateInProgress = "CreateInProgress" - UpdateInProgress = "UpdateInProgress" - DeleteInProgress = "DeleteInProgress" - InProgress = "InProgress" - - CreateFailed = "CreateFailed" - UpdateFailed = "UpdateFailed" - DeleteFailed = "DeleteFailed" - Failed = "Failed" - ShareFailed = "ShareFailed" - ShareSucceeded = "ShareSucceeded" - ShareNotSupported = "ShareNotSupported" - UnShareFailed = "UnShareFailed" - UnShareSucceeded = "UnShareSucceeded" - - Blocked = "Blocked" - Unknown = "Unknown" - - // Cred Rotation - CredPreparing = "Preparing" - CredRotating = "Rotating" -) - -type LogKey struct { -} - -type BaseReconciler struct { - client.Client - Log logr.Logger - Scheme *runtime.Scheme - SMClient func() sm.Client - Config config.Config - SecretResolver *secrets.SecretResolver - Recorder record.EventRecorder -} - -func GetLogger(ctx context.Context) logr.Logger { - return ctx.Value(LogKey{}).(logr.Logger) -} - -func (r *BaseReconciler) getSMClient(ctx context.Context, object api.SAPBTPResource, subaccountID string) (sm.Client, error) { - if r.SMClient != nil { - return r.SMClient(), nil - } - log := GetLogger(ctx) - - secret, err := r.SecretResolver.GetSecretForResource(ctx, object.GetNamespace(), secrets.SAPBTPOperatorSecretName, subaccountID) - if err != nil { - return nil, err - } - - clientConfig := &sm.ClientConfig{ - ClientID: string(secret.Data["clientid"]), - ClientSecret: string(secret.Data["clientsecret"]), - URL: string(secret.Data["sm_url"]), - TokenURL: string(secret.Data["tokenurl"]), - TokenURLSuffix: string(secret.Data["tokenurlsuffix"]), - SSLDisabled: false, - } - - if len(clientConfig.ClientID) == 0 || len(clientConfig.URL) == 0 || len(clientConfig.TokenURL) == 0 { - log.Info("credentials secret found but did not contain all the required data") - return nil, fmt.Errorf("invalid Service-Manager credentials, contact your cluster administrator") - } - - if len(clientConfig.ClientSecret) == 0 { - tlsSecret, err := r.SecretResolver.GetSecretForResource(ctx, object.GetNamespace(), secrets.SAPBTPOperatorTLSSecretName, subaccountID) - if client.IgnoreNotFound(err) != nil { - return nil, err - } - - if tlsSecret == nil || len(tlsSecret.Data) == 0 || len(tlsSecret.Data[v1.TLSCertKey]) == 0 || len(tlsSecret.Data[v1.TLSPrivateKeyKey]) == 0 { - log.Info("clientsecret not found in SM credentials, and tls secret is invalid") - return nil, fmt.Errorf("invalid Service-Manager credentials, contact your cluster administrator") - } - - log.Info("found tls configuration") - clientConfig.TLSCertKey = string(tlsSecret.Data[v1.TLSCertKey]) - clientConfig.TLSPrivateKey = string(tlsSecret.Data[v1.TLSPrivateKeyKey]) - } - - cl, err := sm.NewClient(ctx, clientConfig, nil) - return cl, err -} - -func (r *BaseReconciler) removeFinalizer(ctx context.Context, object api.SAPBTPResource, finalizerName string) error { - log := GetLogger(ctx) - if controllerutil.ContainsFinalizer(object, finalizerName) { - log.Info(fmt.Sprintf("removing finalizer %s", finalizerName)) - controllerutil.RemoveFinalizer(object, finalizerName) - if err := r.Client.Update(ctx, object); err != nil { - if err := r.Client.Get(ctx, apimachinerytypes.NamespacedName{Name: object.GetName(), Namespace: object.GetNamespace()}, object); err != nil { - return client.IgnoreNotFound(err) - } - controllerutil.RemoveFinalizer(object, finalizerName) - if err := r.Client.Update(ctx, object); err != nil { - return fmt.Errorf("failed to remove the finalizer '%s'. Error: %v", finalizerName, err) - } - } - log.Info(fmt.Sprintf("removed finalizer %s from %s", finalizerName, object.GetControllerName())) - return nil - } - return nil -} - -func (r *BaseReconciler) updateStatus(ctx context.Context, object api.SAPBTPResource) error { - log := GetLogger(ctx) - log.Info(fmt.Sprintf("updating %s status", object.GetObjectKind().GroupVersionKind().Kind)) - return r.Client.Status().Update(ctx, object) -} - -func (r *BaseReconciler) init(ctx context.Context, obj api.SAPBTPResource) error { - obj.SetReady(metav1.ConditionFalse) - setInProgressConditions(ctx, smClientTypes.CREATE, "Pending", obj) - return r.updateStatus(ctx, obj) -} - -func getConditionReason(opType smClientTypes.OperationCategory, state smClientTypes.OperationState) string { - switch state { - case smClientTypes.SUCCEEDED: - if opType == smClientTypes.CREATE { - return Created - } else if opType == smClientTypes.UPDATE { - return Updated - } else if opType == smClientTypes.DELETE { - return Deleted - } else { - return Finished - } - case smClientTypes.INPROGRESS, smClientTypes.PENDING: - if opType == smClientTypes.CREATE { - return CreateInProgress - } else if opType == smClientTypes.UPDATE { - return UpdateInProgress - } else if opType == smClientTypes.DELETE { - return DeleteInProgress - } else { - return InProgress - } - case smClientTypes.FAILED: - if opType == smClientTypes.CREATE { - return CreateFailed - } else if opType == smClientTypes.UPDATE { - return UpdateFailed - } else if opType == smClientTypes.DELETE { - return DeleteFailed - } else { - return Failed - } - } - - return Unknown -} - -func setInProgressConditions(ctx context.Context, operationType smClientTypes.OperationCategory, message string, object api.SAPBTPResource) { - log := GetLogger(ctx) - if len(message) == 0 { - if operationType == smClientTypes.CREATE { - message = fmt.Sprintf("%s is being created", object.GetControllerName()) - } else if operationType == smClientTypes.UPDATE { - message = fmt.Sprintf("%s is being updated", object.GetControllerName()) - } else if operationType == smClientTypes.DELETE { - message = fmt.Sprintf("%s is being deleted", object.GetControllerName()) - } - } - - conditions := object.GetConditions() - if len(conditions) > 0 { - meta.RemoveStatusCondition(&conditions, api.ConditionFailed) - } - lastOpCondition := metav1.Condition{ - Type: api.ConditionSucceeded, - Status: metav1.ConditionFalse, - Reason: getConditionReason(operationType, smClientTypes.INPROGRESS), - Message: message, - ObservedGeneration: object.GetObservedGeneration(), - } - meta.SetStatusCondition(&conditions, lastOpCondition) - meta.SetStatusCondition(&conditions, getReadyCondition(object)) - - object.SetConditions(conditions) - log.Info(fmt.Sprintf("setting inProgress conditions: reason: %s, message:%s, generation: %d", lastOpCondition.Reason, message, object.GetGeneration())) -} - -func setSuccessConditions(operationType smClientTypes.OperationCategory, object api.SAPBTPResource) { - var message string - if operationType == smClientTypes.CREATE { - message = fmt.Sprintf("%s provisioned successfully", object.GetControllerName()) - } else if operationType == smClientTypes.UPDATE { - message = fmt.Sprintf("%s updated successfully", object.GetControllerName()) - } else if operationType == smClientTypes.DELETE { - message = fmt.Sprintf("%s deleted successfully", object.GetControllerName()) - } - - conditions := object.GetConditions() - if len(conditions) > 0 { - meta.RemoveStatusCondition(&conditions, api.ConditionFailed) - } - lastOpCondition := metav1.Condition{ - Type: api.ConditionSucceeded, - Status: metav1.ConditionTrue, - Reason: getConditionReason(operationType, smClientTypes.SUCCEEDED), - Message: message, - ObservedGeneration: object.GetObservedGeneration(), - } - readyCondition := metav1.Condition{ - Type: api.ConditionReady, - Status: metav1.ConditionTrue, - Reason: Provisioned, - Message: message, - ObservedGeneration: object.GetObservedGeneration(), - } - meta.SetStatusCondition(&conditions, lastOpCondition) - meta.SetStatusCondition(&conditions, readyCondition) - - object.SetConditions(conditions) - object.SetReady(metav1.ConditionTrue) -} - -func setCredRotationInProgressConditions(reason, message string, object api.SAPBTPResource) { - if len(message) == 0 { - message = reason - } - conditions := object.GetConditions() - credRotCondition := metav1.Condition{ - Type: api.ConditionCredRotationInProgress, - Status: metav1.ConditionTrue, - Reason: reason, - Message: message, - ObservedGeneration: object.GetObservedGeneration(), - } - meta.SetStatusCondition(&conditions, credRotCondition) - object.SetConditions(conditions) -} - -func setFailureConditions(operationType smClientTypes.OperationCategory, errorMessage string, object api.SAPBTPResource) { - var message string - if operationType == smClientTypes.CREATE { - message = fmt.Sprintf("%s create failed: %s", object.GetControllerName(), errorMessage) - } else if operationType == smClientTypes.UPDATE { - message = fmt.Sprintf("%s update failed: %s", object.GetControllerName(), errorMessage) - } else if operationType == smClientTypes.DELETE { - message = fmt.Sprintf("%s deletion failed: %s", object.GetControllerName(), errorMessage) - } - - var reason string - if operationType != Unknown { - reason = getConditionReason(operationType, smClientTypes.FAILED) - } else { - reason = object.GetConditions()[0].Reason - } - - conditions := object.GetConditions() - lastOpCondition := metav1.Condition{ - Type: api.ConditionSucceeded, - Status: metav1.ConditionFalse, - Reason: reason, - Message: message, - ObservedGeneration: object.GetObservedGeneration(), - } - meta.SetStatusCondition(&conditions, lastOpCondition) - - failedCondition := metav1.Condition{ - Type: api.ConditionFailed, - Status: metav1.ConditionTrue, - Reason: reason, - Message: message, - ObservedGeneration: object.GetObservedGeneration(), - } - meta.SetStatusCondition(&conditions, failedCondition) - meta.SetStatusCondition(&conditions, getReadyCondition(object)) - - object.SetConditions(conditions) -} - -// blocked condition marks to the user that action from his side is required, this is considered as in progress operation -func setBlockedCondition(ctx context.Context, message string, object api.SAPBTPResource) { - setInProgressConditions(ctx, Unknown, message, object) - lastOpCondition := meta.FindStatusCondition(object.GetConditions(), api.ConditionSucceeded) - lastOpCondition.Reason = Blocked -} - -func isMarkedForDeletion(object metav1.ObjectMeta) bool { - return !object.DeletionTimestamp.IsZero() -} - -func (r *BaseReconciler) isTransientError(smError *sm.ServiceManagerError, log logr.Logger) bool { - statusCode := smError.GetStatusCode() - log.Info(fmt.Sprintf("SM returned error with status code %d", statusCode)) - return isTransientStatusCode(statusCode) || isConcurrentOperationError(smError) -} - -func isConcurrentOperationError(smError *sm.ServiceManagerError) bool { - // service manager returns 422 for resources that have another operation in progress - // in this case 422 status code is transient - return smError.StatusCode == http.StatusUnprocessableEntity && smError.ErrorType == "ConcurrentOperationInProgress" -} - -func isTransientStatusCode(StatusCode int) bool { - return StatusCode == http.StatusTooManyRequests || - StatusCode == http.StatusServiceUnavailable || - StatusCode == http.StatusGatewayTimeout || - StatusCode == http.StatusBadGateway || - StatusCode == http.StatusNotFound -} - -func (r *BaseReconciler) handleError(ctx context.Context, operationType smClientTypes.OperationCategory, err error, resource api.SAPBTPResource) (ctrl.Result, error) { - log := GetLogger(ctx) - var smError *sm.ServiceManagerError - ok := errors.As(err, &smError) - if !ok { - log.Info("unable to cast error to SM error, will be treated as non transient") - return r.markAsNonTransientError(ctx, operationType, err.Error(), resource) - } - if r.isTransientError(smError, log) || shouldIgnoreNonTransient(log, resource, r.Config.IgnoreNonTransientTimeout) { - return r.markAsTransientError(ctx, operationType, smError, resource) - } - - return r.markAsNonTransientError(ctx, operationType, smError.Error(), resource) -} - -func (r *BaseReconciler) markAsNonTransientError(ctx context.Context, operationType smClientTypes.OperationCategory, errMsg string, object api.SAPBTPResource) (ctrl.Result, error) { - log := GetLogger(ctx) - setFailureConditions(operationType, errMsg, object) - if operationType != smClientTypes.DELETE { - log.Info(fmt.Sprintf("operation %s of %s encountered a non transient error %s, giving up operation :(", operationType, object.GetControllerName(), errMsg)) - } - object.SetObservedGeneration(object.GetGeneration()) - err := r.updateStatus(ctx, object) - if err != nil { - return ctrl.Result{}, err - } - if operationType == smClientTypes.DELETE { - return ctrl.Result{}, fmt.Errorf(errMsg) - } - return ctrl.Result{}, nil -} - -func (r *BaseReconciler) markAsTransientError(ctx context.Context, operationType smClientTypes.OperationCategory, err error, object api.SAPBTPResource) (ctrl.Result, error) { - log := GetLogger(ctx) - //DO NOT REMOVE - 429 error is not reflected to the status - if smError, ok := err.(*sm.ServiceManagerError); !ok || smError.StatusCode != http.StatusTooManyRequests { - setInProgressConditions(ctx, operationType, err.Error(), object) - log.Info(fmt.Sprintf("operation %s of %s encountered a transient error %s, retrying operation :)", operationType, object.GetControllerName(), err.Error())) - if updateErr := r.updateStatus(ctx, object); updateErr != nil { - return ctrl.Result{}, updateErr - } - } - - return ctrl.Result{}, err -} - -func (r *BaseReconciler) removeAnnotation(ctx context.Context, object api.SAPBTPResource, keys ...string) error { - log := GetLogger(ctx) - annotations := object.GetAnnotations() - shouldUpdate := false - if annotations != nil { - for _, key := range keys { - if _, ok := annotations[key]; ok { - log.Info(fmt.Sprintf("deleting annotation with key %s", key)) - delete(annotations, key) - shouldUpdate = true - } - } - if shouldUpdate { - object.SetAnnotations(annotations) - return r.Client.Update(ctx, object) - } - } - return nil -} - -func isInProgress(object api.SAPBTPResource) bool { - conditions := object.GetConditions() - return meta.IsStatusConditionPresentAndEqual(conditions, api.ConditionSucceeded, metav1.ConditionFalse) && - !meta.IsStatusConditionPresentAndEqual(conditions, api.ConditionFailed, metav1.ConditionTrue) -} - -func isFailed(resource api.SAPBTPResource) bool { - if len(resource.GetConditions()) == 0 { - return false - } - return meta.IsStatusConditionPresentAndEqual(resource.GetConditions(), api.ConditionFailed, metav1.ConditionTrue) || - (resource.GetConditions()[0].Status == metav1.ConditionFalse && - resource.GetConditions()[0].Type == api.ConditionSucceeded && - resource.GetConditions()[0].Reason == Blocked) -} - -func getReadyCondition(object api.SAPBTPResource) metav1.Condition { - status := metav1.ConditionFalse - reason := NotProvisioned - if object.GetReady() == metav1.ConditionTrue { - status = metav1.ConditionTrue - reason = Provisioned - } - - return metav1.Condition{Type: api.ConditionReady, Status: status, Reason: reason} -} diff --git a/controllers/controller_util.go b/controllers/controller_util.go deleted file mode 100644 index 84f599a6..00000000 --- a/controllers/controller_util.go +++ /dev/null @@ -1,120 +0,0 @@ -package controllers - -import ( - "context" - "encoding/json" - "fmt" - "strings" - "time" - - "github.com/SAP/sap-btp-service-operator/api" - "github.com/go-logr/logr" - - "k8s.io/apimachinery/pkg/util/rand" - - v1 "k8s.io/api/authentication/v1" -) - -type SecretMetadataProperty struct { - Name string `json:"name"` - Container bool `json:"container,omitempty"` - Format string `json:"format"` -} - -type format string - -const ( - TEXT format = "text" - JSON format = "json" - UNKNOWN format = "unknown" -) - -func shouldIgnoreNonTransient(log logr.Logger, resource api.SAPBTPResource, timeout time.Duration) bool { - annotations := resource.GetAnnotations() - if len(annotations) == 0 || len(annotations[api.IgnoreNonTransientErrorAnnotation]) == 0 { - return false - } - - // we ignore the error - // for service instances, the value is validated in webhook - // for service bindings, the annotation is not allowed - annotationTime, _ := time.Parse(time.RFC3339, annotations[api.IgnoreNonTransientErrorTimestampAnnotation]) - sinceAnnotation := time.Since(annotationTime) - if sinceAnnotation > timeout { - log.Info(fmt.Sprintf("timeout of %s reached - error is considered to be non transient. time passed since annotation timestamp %s", timeout, sinceAnnotation)) - return false - } - log.Info(fmt.Sprintf("timeout of %s was not reached - error is considered to be transient. ime passed since annotation timestamp %s", timeout, sinceAnnotation)) - return true -} - -func normalizeCredentials(credentialsJSON json.RawMessage) (map[string][]byte, []SecretMetadataProperty, error) { - var credentialsMap map[string]interface{} - err := json.Unmarshal(credentialsJSON, &credentialsMap) - if err != nil { - return nil, nil, err - } - - normalized := make(map[string][]byte) - metadata := make([]SecretMetadataProperty, 0) - for propertyName, value := range credentialsMap { - keyString := strings.Replace(propertyName, " ", "_", -1) - normalizedValue, typpe, err := serialize(value) - if err != nil { - return nil, nil, err - } - metadata = append(metadata, SecretMetadataProperty{ - Name: keyString, - Format: string(typpe), - }) - normalized[keyString] = normalizedValue - } - return normalized, metadata, nil -} - -func buildUserInfo(ctx context.Context, userInfo *v1.UserInfo) string { - log := GetLogger(ctx) - if userInfo == nil { - return "" - } - userInfoStr, err := json.Marshal(userInfo) - if err != nil { - log.Error(err, "failed to prepare user info") - return "" - } - - return string(userInfoStr) -} - -func serialize(value interface{}) ([]byte, format, error) { - if byteArrayVal, ok := value.([]byte); ok { - return byteArrayVal, JSON, nil - } - if strVal, ok := value.(string); ok { - return []byte(strVal), TEXT, nil - } - data, err := json.Marshal(value) - if err != nil { - return nil, UNKNOWN, err - } - return data, JSON, nil -} - -func contains(slice []string, i string) bool { - for _, s := range slice { - if s == i { - return true - } - } - - return false -} - -func RandStringRunes(n int) string { - var letterRunes = []rune("abcdefghijklmnopqrstuvwxyz1234567890") - b := make([]rune, n) - for i := range b { - b[i] = letterRunes[rand.Intn(len(letterRunes))] - } - return string(b) -} diff --git a/controllers/controller_util_test.go b/controllers/controller_util_test.go deleted file mode 100644 index a78018a6..00000000 --- a/controllers/controller_util_test.go +++ /dev/null @@ -1,80 +0,0 @@ -package controllers - -import ( - "encoding/json" - "github.com/SAP/sap-btp-service-operator/api" - v1 "github.com/SAP/sap-btp-service-operator/api/v1" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "time" -) - -var _ = Describe("Controller Util", func() { - - Context("normalize credentials", func() { - var credentialsJSON json.RawMessage - - BeforeEach(func() { - credentialsJSON = []byte(`{"keyStr":"val", "keyBool":true,"keyNum":8,"keyJSON":{"a":"b"}}`) - }) - - It("should normalize correctly", func() { - res, metadata, err := normalizeCredentials(credentialsJSON) - str := SecretMetadataProperty{ - Name: "keyStr", - Format: string(TEXT), - } - boolean := SecretMetadataProperty{ - Name: "keyBool", - Format: string(JSON), - } - num := SecretMetadataProperty{ - Name: "keyNum", - Format: string(JSON), - } - json := SecretMetadataProperty{ - Name: "keyJSON", - Format: string(JSON), - } - Expect(err).ToNot(HaveOccurred()) - Expect(len(res)).To(Equal(4)) - Expect(metadata).To(ContainElements(str, boolean, num, json)) - }) - - }) - - Context("shouldIgnoreNonTransient", func() { - var ( - instance *v1.ServiceInstance - logger = logf.Log.WithName("test-logger") - ) - - BeforeEach(func() { - instance = &v1.ServiceInstance{} - }) - - It("should return false if no ignore annotation", func() { - instance.SetAnnotations(nil) - Expect(shouldIgnoreNonTransient(logger, instance, time.Hour)).To(BeFalse()) - }) - - It("should return false if time exceeded", func() { - annotation := map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "true", - api.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Truncate(48 * time.Hour).Format(time.RFC3339), - } - instance.SetAnnotations(annotation) - Expect(shouldIgnoreNonTransient(logger, instance, time.Hour)).To(BeFalse()) - }) - - It("should return true if time not exceeded", func() { - annotation := map[string]string{ - api.IgnoreNonTransientErrorAnnotation: "true", - api.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Format(time.RFC3339), - } - instance.SetAnnotations(annotation) - Expect(shouldIgnoreNonTransient(logger, instance, time.Hour)).To(BeTrue()) - }) - }) -}) diff --git a/controllers/servicebinding_controller.go b/controllers/servicebinding_controller.go index 6dbbecc9..1e8b8806 100644 --- a/controllers/servicebinding_controller.go +++ b/controllers/servicebinding_controller.go @@ -21,6 +21,13 @@ import ( "encoding/json" "time" + "github.com/SAP/sap-btp-service-operator/api/common" + "github.com/SAP/sap-btp-service-operator/internal/config" + "github.com/SAP/sap-btp-service-operator/internal/utils" + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + "fmt" "k8s.io/client-go/util/workqueue" @@ -28,8 +35,6 @@ import ( servicesv1 "github.com/SAP/sap-btp-service-operator/api/v1" - "github.com/SAP/sap-btp-service-operator/api" - "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime/schema" @@ -55,7 +60,13 @@ const ( // ServiceBindingReconciler reconciles a ServiceBinding object type ServiceBindingReconciler struct { - *BaseReconciler + 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 } // +kubebuilder:rbac:groups=services.cloud.sap.com,resources=servicebindings,verbs=get;list;watch;create;update;patch;delete @@ -65,10 +76,9 @@ type ServiceBindingReconciler struct { // +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=core,resources=events,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;create;update - func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := r.Log.WithValues("servicebinding", req.NamespacedName).WithValues("correlation_id", uuid.New().String(), req.Name, req.Namespace) - ctx = context.WithValue(ctx, LogKey{}, log) + ctx = context.WithValue(ctx, utils.LogKey{}, log) serviceBinding := &servicesv1.ServiceBinding{} if err := r.Client.Get(ctx, req.NamespacedName, serviceBinding); err != nil { @@ -81,7 +91,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque serviceBinding.SetObservedGeneration(serviceBinding.Generation) if len(serviceBinding.GetConditions()) == 0 { - if err := r.init(ctx, serviceBinding); err != nil { + if err := utils.InitConditions(ctx, r.Client, serviceBinding); err != nil { return ctrl.Result{}, err } } @@ -91,22 +101,22 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque if !apierrors.IsNotFound(instanceErr) { log.Error(instanceErr, "failed to get service instance for binding") return ctrl.Result{}, instanceErr - } else if !isMarkedForDeletion(serviceBinding.ObjectMeta) { + } else if !utils.IsMarkedForDeletion(serviceBinding.ObjectMeta) { //instance is not found and binding is not marked for deletion instanceNamespace := serviceBinding.Namespace if len(serviceBinding.Spec.ServiceInstanceNamespace) > 0 { instanceNamespace = serviceBinding.Spec.ServiceInstanceNamespace } errMsg := fmt.Sprintf("couldn't find the service instance '%s' in namespace '%s'", serviceBinding.Spec.ServiceInstanceName, instanceNamespace) - setBlockedCondition(ctx, errMsg, serviceBinding) - if updateErr := r.updateStatus(ctx, serviceBinding); updateErr != nil { + utils.SetBlockedCondition(ctx, errMsg, serviceBinding) + if updateErr := utils.UpdateStatus(ctx, r.Client, serviceBinding); updateErr != nil { return ctrl.Result{}, updateErr } return ctrl.Result{}, instanceErr } } - if isMarkedForDeletion(serviceBinding.ObjectMeta) { + if utils.IsMarkedForDeletion(serviceBinding.ObjectMeta) { return r.delete(ctx, serviceBinding, serviceInstance.Spec.BTPAccessCredentialsSecret) } @@ -115,26 +125,26 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque return r.poll(ctx, serviceBinding, serviceInstance.Spec.BTPAccessCredentialsSecret) } - if !controllerutil.ContainsFinalizer(serviceBinding, api.FinalizerName) { - controllerutil.AddFinalizer(serviceBinding, api.FinalizerName) - log.Info(fmt.Sprintf("added finalizer '%s' to service binding", api.FinalizerName)) + if !controllerutil.ContainsFinalizer(serviceBinding, common.FinalizerName) { + controllerutil.AddFinalizer(serviceBinding, common.FinalizerName) + log.Info(fmt.Sprintf("added finalizer '%s' to service binding", common.FinalizerName)) if err := r.Client.Update(ctx, serviceBinding); err != nil { return ctrl.Result{}, err } } - isBindingReady := meta.IsStatusConditionPresentAndEqual(serviceBinding.Status.Conditions, api.ConditionReady, metav1.ConditionTrue) + isBindingReady := meta.IsStatusConditionPresentAndEqual(serviceBinding.Status.Conditions, common.ConditionReady, metav1.ConditionTrue) if isBindingReady { if isStaleServiceBinding(serviceBinding) { return r.handleStaleServiceBinding(ctx, serviceBinding) } if initCredRotationIfRequired(serviceBinding) { - return ctrl.Result{}, r.updateStatus(ctx, serviceBinding) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding) } } - if meta.IsStatusConditionTrue(serviceBinding.Status.Conditions, api.ConditionCredRotationInProgress) { + if meta.IsStatusConditionTrue(serviceBinding.Status.Conditions, common.ConditionCredRotationInProgress) { if err := r.rotateCredentials(ctx, serviceBinding, serviceInstance.Spec.BTPAccessCredentialsSecret); err != nil { return ctrl.Result{}, err } @@ -150,23 +160,23 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque if serviceNotUsable(serviceInstance) { instanceErr := fmt.Errorf("service instance '%s' is not usable", serviceBinding.Spec.ServiceInstanceName) - setBlockedCondition(ctx, instanceErr.Error(), serviceBinding) - if err := r.updateStatus(ctx, serviceBinding); err != nil { + utils.SetBlockedCondition(ctx, instanceErr.Error(), serviceBinding) + if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil { return ctrl.Result{}, err } return ctrl.Result{}, instanceErr } - if isInProgress(serviceInstance) { + if utils.IsInProgress(serviceInstance) { log.Info(fmt.Sprintf("Service instance with k8s name %s is not ready for binding yet", serviceInstance.Name)) - setInProgressConditions(ctx, smClientTypes.CREATE, fmt.Sprintf("creation in progress, waiting for service instance '%s' to be ready", serviceBinding.Spec.ServiceInstanceName), serviceBinding) + utils.SetInProgressConditions(ctx, smClientTypes.CREATE, fmt.Sprintf("creation in progress, waiting for service instance '%s' to be ready", serviceBinding.Spec.ServiceInstanceName), serviceBinding) - return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, r.updateStatus(ctx, serviceBinding) + return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, utils.UpdateStatus(ctx, r.Client, serviceBinding) } //set owner instance only for original bindings (not rotated) - if serviceBinding.Labels == nil || len(serviceBinding.Labels[api.StaleBindingIDLabel]) == 0 { + if serviceBinding.Labels == nil || len(serviceBinding.Labels[common.StaleBindingIDLabel]) == 0 { if !bindingAlreadyOwnedByInstance(serviceInstance, serviceBinding) && serviceInstance.Namespace == serviceBinding.Namespace { //cross namespace reference not allowed if err := r.setOwner(ctx, serviceInstance, serviceBinding); err != nil { @@ -178,19 +188,19 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque if serviceBinding.Status.BindingID == "" { if err := r.validateSecretNameIsAvailable(ctx, serviceBinding); err != nil { - setBlockedCondition(ctx, err.Error(), serviceBinding) - return ctrl.Result{}, r.updateStatus(ctx, serviceBinding) + utils.SetBlockedCondition(ctx, err.Error(), serviceBinding) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding) } - smClient, err := r.getSMClient(ctx, serviceBinding, serviceInstance.Spec.BTPAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, r.SecretResolver, serviceBinding.Namespace, serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { - return r.markAsTransientError(ctx, Unknown, err, serviceBinding) + return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceBinding) } smBinding, err := r.getBindingForRecovery(ctx, smClient, serviceBinding) if err != nil { log.Error(err, "failed to check binding recovery") - return r.markAsTransientError(ctx, smClientTypes.CREATE, err, serviceBinding) + return utils.MarkAsTransientError(ctx, r.Client, smClientTypes.CREATE, err, serviceBinding) } if smBinding != nil { return r.recover(ctx, serviceBinding, smBinding) @@ -211,43 +221,43 @@ func (r *ServiceBindingReconciler) SetupWithManager(mgr ctrl.Manager) error { } func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient sm.Client, serviceInstance *servicesv1.ServiceInstance, serviceBinding *servicesv1.ServiceBinding) (ctrl.Result, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) log.Info("Creating smBinding in SM") serviceBinding.Status.InstanceID = serviceInstance.Status.InstanceID - _, bindingParameters, err := buildParameters(r.Client, serviceBinding.Namespace, serviceBinding.Spec.ParametersFrom, serviceBinding.Spec.Parameters) + _, bindingParameters, err := utils.BuildSMRequestParameters(r.Client, serviceBinding.Namespace, serviceBinding.Spec.ParametersFrom, serviceBinding.Spec.Parameters) if err != nil { log.Error(err, "failed to parse smBinding parameters") - return r.markAsNonTransientError(ctx, smClientTypes.CREATE, err.Error(), serviceBinding) + return utils.MarkAsNonTransientError(ctx, r.Client, smClientTypes.CREATE, err.Error(), serviceBinding) } smBinding, operationURL, bindErr := smClient.Bind(&smClientTypes.ServiceBinding{ Name: serviceBinding.Spec.ExternalName, Labels: smClientTypes.Labels{ - namespaceLabel: []string{serviceBinding.Namespace}, - k8sNameLabel: []string{serviceBinding.Name}, - clusterIDLabel: []string{r.Config.ClusterID}, + common.NamespaceLabel: []string{serviceBinding.Namespace}, + common.K8sNameLabel: []string{serviceBinding.Name}, + common.ClusterIDLabel: []string{r.Config.ClusterID}, }, ServiceInstanceID: serviceInstance.Status.InstanceID, Parameters: bindingParameters, - }, nil, buildUserInfo(ctx, serviceBinding.Spec.UserInfo)) + }, nil, utils.BuildUserInfo(ctx, serviceBinding.Spec.UserInfo)) if bindErr != nil { log.Error(err, "failed to create service binding", "serviceInstanceID", serviceInstance.Status.InstanceID) - return r.handleError(ctx, smClientTypes.CREATE, bindErr, serviceBinding) + return utils.HandleError(ctx, r.Client, smClientTypes.CREATE, bindErr, serviceBinding, false) } if operationURL != "" { var bindingID string if bindingID = sm.ExtractBindingID(operationURL); len(bindingID) == 0 { - return r.markAsNonTransientError(ctx, smClientTypes.CREATE, fmt.Sprintf("failed to extract smBinding ID from operation URL %s", operationURL), serviceBinding) + return utils.MarkAsNonTransientError(ctx, r.Client, smClientTypes.CREATE, fmt.Sprintf("failed to extract smBinding ID from operation URL %s", operationURL), serviceBinding) } serviceBinding.Status.BindingID = bindingID log.Info("Create smBinding request is async") serviceBinding.Status.OperationURL = operationURL serviceBinding.Status.OperationType = smClientTypes.CREATE - setInProgressConditions(ctx, smClientTypes.CREATE, "", serviceBinding) - if err := r.updateStatus(ctx, serviceBinding); err != nil { + utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "", serviceBinding) + if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil { log.Error(err, "unable to update ServiceBinding status") return ctrl.Result{}, err } @@ -268,18 +278,18 @@ func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient s serviceBinding.Status.BindingID = smBinding.ID serviceBinding.Status.SubaccountID = subaccountID serviceBinding.Status.Ready = metav1.ConditionTrue - setSuccessConditions(smClientTypes.CREATE, serviceBinding) + utils.SetSuccessConditions(smClientTypes.CREATE, serviceBinding) log.Info("Updating binding", "bindingID", smBinding.ID) - return ctrl.Result{}, r.updateStatus(ctx, serviceBinding) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding) } func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *servicesv1.ServiceBinding, btpAccessCredentialsSecret string) (ctrl.Result, error) { - log := GetLogger(ctx) - if controllerutil.ContainsFinalizer(serviceBinding, api.FinalizerName) { - smClient, err := r.getSMClient(ctx, serviceBinding, btpAccessCredentialsSecret) + log := utils.GetLogger(ctx) + if controllerutil.ContainsFinalizer(serviceBinding, common.FinalizerName) { + smClient, err := r.GetSMClient(ctx, r.SecretResolver, serviceBinding.Namespace, btpAccessCredentialsSecret) if err != nil { - return r.markAsTransientError(ctx, Unknown, err, serviceBinding) + return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceBinding) } if len(serviceBinding.Status.BindingID) == 0 { @@ -291,8 +301,8 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *s if smBinding != nil { log.Info("binding exists in SM continue with deletion") serviceBinding.Status.BindingID = smBinding.ID - setInProgressConditions(ctx, smClientTypes.DELETE, "delete after recovery", serviceBinding) - return ctrl.Result{}, r.updateStatus(ctx, serviceBinding) + utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "delete after recovery", serviceBinding) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding) } // make sure there's no secret stored for the binding @@ -301,7 +311,7 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *s } log.Info("Binding does not exists in SM, removing finalizer") - if err := r.removeFinalizer(ctx, serviceBinding, api.FinalizerName); err != nil { + if err := utils.RemoveFinalizer(ctx, r.Client, serviceBinding, common.FinalizerName); err != nil { return ctrl.Result{}, err } return ctrl.Result{}, nil @@ -313,18 +323,18 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *s } log.Info(fmt.Sprintf("Deleting binding with id %v from SM", serviceBinding.Status.BindingID)) - operationURL, unbindErr := smClient.Unbind(serviceBinding.Status.BindingID, nil, buildUserInfo(ctx, serviceBinding.Spec.UserInfo)) + operationURL, unbindErr := smClient.Unbind(serviceBinding.Status.BindingID, nil, utils.BuildUserInfo(ctx, serviceBinding.Spec.UserInfo)) if unbindErr != nil { // delete will proceed anyway - return r.markAsNonTransientError(ctx, smClientTypes.DELETE, unbindErr.Error(), serviceBinding) + return utils.MarkAsNonTransientError(ctx, r.Client, smClientTypes.DELETE, unbindErr.Error(), serviceBinding) } if operationURL != "" { log.Info("Deleting binding async") serviceBinding.Status.OperationURL = operationURL serviceBinding.Status.OperationType = smClientTypes.DELETE - setInProgressConditions(ctx, smClientTypes.DELETE, "", serviceBinding) - if err := r.updateStatus(ctx, serviceBinding); err != nil { + utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "", serviceBinding) + if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil { return ctrl.Result{}, err } return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, nil @@ -337,41 +347,41 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *s } func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *servicesv1.ServiceBinding, btpAccessCredentialsSecret string) (ctrl.Result, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) log.Info(fmt.Sprintf("resource is in progress, found operation url %s", serviceBinding.Status.OperationURL)) - smClient, err := r.getSMClient(ctx, serviceBinding, btpAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, r.SecretResolver, serviceBinding.Namespace, btpAccessCredentialsSecret) if err != nil { - return r.markAsTransientError(ctx, Unknown, err, serviceBinding) + return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceBinding) } status, statusErr := smClient.Status(serviceBinding.Status.OperationURL, nil) if statusErr != nil { log.Info(fmt.Sprintf("failed to fetch operation, got error from SM: %s", statusErr.Error()), "operationURL", serviceBinding.Status.OperationURL) - setFailureConditions(serviceBinding.Status.OperationType, statusErr.Error(), serviceBinding) + utils.SetFailureConditions(serviceBinding.Status.OperationType, statusErr.Error(), serviceBinding) freshStatus := servicesv1.ServiceBindingStatus{ Conditions: serviceBinding.GetConditions(), } - if isMarkedForDeletion(serviceBinding.ObjectMeta) { + if utils.IsMarkedForDeletion(serviceBinding.ObjectMeta) { freshStatus.BindingID = serviceBinding.Status.BindingID } serviceBinding.Status = freshStatus - if err := r.updateStatus(ctx, serviceBinding); err != nil { + if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil { log.Error(err, "failed to update status during polling") } return ctrl.Result{}, statusErr } if status == nil { - return r.markAsTransientError(ctx, serviceBinding.Status.OperationType, fmt.Errorf("failed to get last operation status of %s", serviceBinding.Name), serviceBinding) + return utils.MarkAsTransientError(ctx, r.Client, serviceBinding.Status.OperationType, fmt.Errorf("failed to get last operation status of %s", serviceBinding.Name), serviceBinding) } switch status.State { case smClientTypes.INPROGRESS: fallthrough case smClientTypes.PENDING: if len(status.Description) != 0 { - setInProgressConditions(ctx, status.Type, status.Description, serviceBinding) - if err := r.updateStatus(ctx, serviceBinding); err != nil { + utils.SetInProgressConditions(ctx, status.Type, status.Description, serviceBinding) + if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil { log.Error(err, "unable to update ServiceBinding polling description") return ctrl.Result{}, err } @@ -379,11 +389,11 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *ser return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, nil case smClientTypes.FAILED: // non transient error - should not retry - setFailureConditions(status.Type, status.Description, serviceBinding) + utils.SetFailureConditions(status.Type, status.Description, serviceBinding) if serviceBinding.Status.OperationType == smClientTypes.DELETE { serviceBinding.Status.OperationURL = "" serviceBinding.Status.OperationType = "" - if err := r.updateStatus(ctx, serviceBinding); err != nil { + if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil { log.Error(err, "unable to update ServiceBinding status") return ctrl.Result{}, err } @@ -394,7 +404,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *ser return ctrl.Result{}, fmt.Errorf(errMsg) } case smClientTypes.SUCCEEDED: - setSuccessConditions(status.Type, serviceBinding) + utils.SetSuccessConditions(status.Type, serviceBinding) switch serviceBinding.Status.OperationType { case smClientTypes.CREATE: smBinding, err := smClient.GetBindingByID(serviceBinding.Status.BindingID, nil) @@ -409,7 +419,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *ser if err := r.storeBindingSecret(ctx, serviceBinding, smBinding); err != nil { return r.handleSecretError(ctx, smClientTypes.CREATE, err, serviceBinding) } - setSuccessConditions(status.Type, serviceBinding) + utils.SetSuccessConditions(status.Type, serviceBinding) case smClientTypes.DELETE: return r.deleteSecretAndRemoveFinalizer(ctx, serviceBinding) } @@ -418,15 +428,15 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *ser serviceBinding.Status.OperationURL = "" serviceBinding.Status.OperationType = "" - return ctrl.Result{}, r.updateStatus(ctx, serviceBinding) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding) } func (r *ServiceBindingReconciler) getBindingForRecovery(ctx context.Context, smClient sm.Client, serviceBinding *servicesv1.ServiceBinding) (*smClientTypes.ServiceBinding, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) nameQuery := fmt.Sprintf("name eq '%s'", serviceBinding.Spec.ExternalName) clusterIDQuery := fmt.Sprintf("context/clusterid eq '%s'", r.Config.ClusterID) namespaceQuery := fmt.Sprintf("context/namespace eq '%s'", serviceBinding.Namespace) - k8sNameQuery := fmt.Sprintf("%s eq '%s'", k8sNameLabel, serviceBinding.Name) + k8sNameQuery := fmt.Sprintf("%s eq '%s'", common.K8sNameLabel, serviceBinding.Name) parameters := sm.Parameters{ FieldQuery: []string{nameQuery, clusterIDQuery, namespaceQuery}, LabelQuery: []string{k8sNameQuery}, @@ -449,20 +459,20 @@ func (r *ServiceBindingReconciler) getBindingForRecovery(ctx context.Context, sm } func (r *ServiceBindingReconciler) maintain(ctx context.Context, binding *servicesv1.ServiceBinding) (ctrl.Result, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) shouldUpdateStatus := false if binding.Generation != binding.Status.ObservedGeneration { binding.SetObservedGeneration(binding.Generation) shouldUpdateStatus = true } - if !isFailed(binding) { + if !utils.IsFailed(binding) { if _, err := r.getSecret(ctx, binding.Namespace, binding.Spec.SecretName); err != nil { - if apierrors.IsNotFound(err) && !isMarkedForDeletion(binding.ObjectMeta) { + if apierrors.IsNotFound(err) && !utils.IsMarkedForDeletion(binding.ObjectMeta) { log.Info(fmt.Sprintf("secret not found recovering binding %s", binding.Name)) binding.Status.BindingID = "" binding.Status.Ready = metav1.ConditionFalse - setInProgressConditions(ctx, smClientTypes.CREATE, "recreating deleted secret", binding) + utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "recreating deleted secret", binding) shouldUpdateStatus = true r.Recorder.Event(binding, corev1.EventTypeWarning, "SecretDeleted", "SecretDeleted") } else { @@ -473,14 +483,14 @@ func (r *ServiceBindingReconciler) maintain(ctx context.Context, binding *servic if shouldUpdateStatus { log.Info(fmt.Sprintf("maintanance required for binding %s", binding.Name)) - return ctrl.Result{}, r.updateStatus(ctx, binding) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, binding) } return ctrl.Result{}, nil } func (r *ServiceBindingReconciler) getServiceInstanceForBinding(ctx context.Context, binding *servicesv1.ServiceBinding) (*servicesv1.ServiceInstance, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) serviceInstance := &servicesv1.ServiceInstance{} namespace := binding.Namespace if len(binding.Spec.ServiceInstanceNamespace) > 0 { @@ -495,7 +505,7 @@ func (r *ServiceBindingReconciler) getServiceInstanceForBinding(ctx context.Cont } func (r *ServiceBindingReconciler) setOwner(ctx context.Context, serviceInstance *servicesv1.ServiceInstance, serviceBinding *servicesv1.ServiceBinding) error { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) log.Info("Binding instance as owner of binding", "bindingName", serviceBinding.Name, "instanceName", serviceInstance.Name) if err := controllerutil.SetControllerReference(serviceInstance, serviceBinding, r.Scheme); err != nil { log.Error(err, fmt.Sprintf("Could not update the smBinding %s owner instance reference", serviceBinding.Name)) @@ -531,20 +541,20 @@ func (r *ServiceBindingReconciler) resyncBindingStatus(ctx context.Context, k8sB case smClientTypes.INPROGRESS: k8sBinding.Status.OperationURL = sm.BuildOperationURL(smBinding.LastOperation.ID, smBinding.ID, smClientTypes.ServiceBindingsURL) k8sBinding.Status.OperationType = smBinding.LastOperation.Type - setInProgressConditions(ctx, smBinding.LastOperation.Type, smBinding.LastOperation.Description, k8sBinding) + utils.SetInProgressConditions(ctx, smBinding.LastOperation.Type, smBinding.LastOperation.Description, k8sBinding) case smClientTypes.SUCCEEDED: - setSuccessConditions(operationType, k8sBinding) + utils.SetSuccessConditions(operationType, k8sBinding) case smClientTypes.FAILED: - setFailureConditions(operationType, description, k8sBinding) + utils.SetFailureConditions(operationType, description, k8sBinding) } } func (r *ServiceBindingReconciler) storeBindingSecret(ctx context.Context, k8sBinding *servicesv1.ServiceBinding, smBinding *smClientTypes.ServiceBinding) error { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) logger := log.WithValues("bindingName", k8sBinding.Name, "secretName", k8sBinding.Spec.SecretName) var credentialsMap map[string][]byte - var credentialProperties []SecretMetadataProperty + var credentialProperties []utils.SecretMetadataProperty if len(smBinding.Credentials) == 0 { log.Info("Binding credentials are empty") @@ -553,16 +563,16 @@ func (r *ServiceBindingReconciler) storeBindingSecret(ctx context.Context, k8sBi credentialsMap = map[string][]byte{ *k8sBinding.Spec.SecretKey: smBinding.Credentials, } - credentialProperties = []SecretMetadataProperty{ + credentialProperties = []utils.SecretMetadataProperty{ { Name: *k8sBinding.Spec.SecretKey, - Format: string(JSON), + Format: string(utils.JSON), Container: true, }, } } else { var err error - credentialsMap, credentialProperties, err = normalizeCredentials(smBinding.Credentials) + credentialsMap, credentialProperties, err = utils.NormalizeCredentials(smBinding.Credentials) if err != nil { logger.Error(err, "Failed to store binding secret") return fmt.Errorf("failed to create secret. Error: %v", err.Error()) @@ -581,7 +591,7 @@ func (r *ServiceBindingReconciler) storeBindingSecret(ctx context.Context, k8sBi return err } } else { - metadata := map[string][]SecretMetadataProperty{ + metadata := map[string][]utils.SecretMetadataProperty{ "metaDataProperties": metaDataProperties, "credentialProperties": credentialProperties, } @@ -610,7 +620,7 @@ func (r *ServiceBindingReconciler) storeBindingSecret(ctx context.Context, k8sBi } func (r *ServiceBindingReconciler) createOrUpdateBindingSecret(ctx context.Context, binding *servicesv1.ServiceBinding, secret *corev1.Secret) error { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) dbSecret := &corev1.Secret{} create := false if err := r.Client.Get(ctx, types.NamespacedName{Name: binding.Spec.SecretName, Namespace: binding.Namespace}, dbSecret); err != nil { @@ -639,7 +649,7 @@ func (r *ServiceBindingReconciler) createOrUpdateBindingSecret(ctx context.Conte } func (r *ServiceBindingReconciler) deleteBindingSecret(ctx context.Context, binding *servicesv1.ServiceBinding) error { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) log.Info("Deleting binding secret") bindingSecret := &corev1.Secret{} if err := r.Client.Get(ctx, types.NamespacedName{ @@ -672,7 +682,7 @@ func (r *ServiceBindingReconciler) deleteSecretAndRemoveFinalizer(ctx context.Co return ctrl.Result{}, err } - return ctrl.Result{}, r.removeFinalizer(ctx, serviceBinding, api.FinalizerName) + return ctrl.Result{}, utils.RemoveFinalizer(ctx, r.Client, serviceBinding, common.FinalizerName) } func (r *ServiceBindingReconciler) getSecret(ctx context.Context, namespace string, name string) (*corev1.Secret, error) { @@ -707,15 +717,15 @@ func (r *ServiceBindingReconciler) validateSecretNameIsAvailable(ctx context.Con } func (r *ServiceBindingReconciler) handleSecretError(ctx context.Context, op smClientTypes.OperationCategory, err error, binding *servicesv1.ServiceBinding) (ctrl.Result, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) log.Error(err, fmt.Sprintf("failed to store secret %s for binding %s", binding.Spec.SecretName, binding.Name)) if apierrors.ReasonForError(err) == metav1.StatusReasonUnknown { - return r.markAsNonTransientError(ctx, op, err.Error(), binding) + return utils.MarkAsNonTransientError(ctx, r.Client, op, err.Error(), binding) } - return r.markAsTransientError(ctx, op, err, binding) + return utils.MarkAsTransientError(ctx, r.Client, op, err, binding) } -func (r *ServiceBindingReconciler) addInstanceInfo(ctx context.Context, binding *servicesv1.ServiceBinding, credentialsMap map[string][]byte) ([]SecretMetadataProperty, error) { +func (r *ServiceBindingReconciler) addInstanceInfo(ctx context.Context, binding *servicesv1.ServiceBinding, credentialsMap map[string][]byte) ([]utils.SecretMetadataProperty, error) { instance, err := r.getServiceInstanceForBinding(ctx, binding) if err != nil { return nil, err @@ -734,42 +744,42 @@ func (r *ServiceBindingReconciler) addInstanceInfo(ctx context.Context, binding credentialsMap["tags"] = tagsBytes } - metadata := []SecretMetadataProperty{ + metadata := []utils.SecretMetadataProperty{ { Name: "instance_name", - Format: string(TEXT), + Format: string(utils.TEXT), }, { Name: "instance_guid", - Format: string(TEXT), + Format: string(utils.TEXT), }, { Name: "plan", - Format: string(TEXT), + Format: string(utils.TEXT), }, { Name: "label", - Format: string(TEXT), + Format: string(utils.TEXT), }, { Name: "type", - Format: string(TEXT), + Format: string(utils.TEXT), }, } if _, ok := credentialsMap["tags"]; ok { - metadata = append(metadata, SecretMetadataProperty{Name: "tags", Format: string(JSON)}) + metadata = append(metadata, utils.SecretMetadataProperty{Name: "tags", Format: string(utils.JSON)}) } return metadata, nil } func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, binding *servicesv1.ServiceBinding, btpAccessCredentialsSecret string) error { - suffix := "-" + RandStringRunes(6) - log := GetLogger(ctx) + suffix := "-" + utils.RandStringRunes(6) + log := utils.GetLogger(ctx) if binding.Annotations != nil { - if _, ok := binding.Annotations[api.ForceRotateAnnotation]; ok { + if _, ok := binding.Annotations[common.ForceRotateAnnotation]; ok { log.Info("Credentials rotation - deleting force rotate annotation") - delete(binding.Annotations, api.ForceRotateAnnotation) + delete(binding.Annotations, common.ForceRotateAnnotation) if err := r.Client.Update(ctx, binding); err != nil { log.Info("Credentials rotation - failed to delete force rotate annotation") return err @@ -777,14 +787,14 @@ func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, bindin } } - credInProgressCondition := meta.FindStatusCondition(binding.GetConditions(), api.ConditionCredRotationInProgress) - if credInProgressCondition.Reason == CredRotating { + credInProgressCondition := meta.FindStatusCondition(binding.GetConditions(), common.ConditionCredRotationInProgress) + if credInProgressCondition.Reason == common.CredRotating { if len(binding.Status.BindingID) > 0 && binding.Status.Ready == metav1.ConditionTrue { log.Info("Credentials rotation - finished successfully") now := metav1.NewTime(time.Now()) binding.Status.LastCredentialsRotationTime = &now return r.stopRotation(ctx, binding) - } else if isFailed(binding) { + } else if utils.IsFailed(binding) { log.Info("Credentials rotation - binding failed stopping rotation") return r.stopRotation(ctx, binding) } @@ -798,13 +808,13 @@ func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, bindin } bindings := &servicesv1.ServiceBindingList{} - err := r.Client.List(ctx, bindings, client.MatchingLabels{api.StaleBindingIDLabel: binding.Status.BindingID}, client.InNamespace(binding.Namespace)) + err := r.Client.List(ctx, bindings, client.MatchingLabels{common.StaleBindingIDLabel: binding.Status.BindingID}, client.InNamespace(binding.Namespace)) if err != nil { return err } if len(bindings.Items) == 0 { - smClient, err := r.getSMClient(ctx, binding, btpAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, r.SecretResolver, binding.Namespace, btpAccessCredentialsSecret) if err != nil { return err } @@ -813,8 +823,8 @@ func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, bindin log.Info("Credentials rotation - renaming binding to old in SM", "current", binding.Spec.ExternalName) if _, errRenaming := smClient.RenameBinding(binding.Status.BindingID, binding.Spec.ExternalName+suffix, binding.Name+suffix); errRenaming != nil { log.Error(errRenaming, "Credentials rotation - failed renaming binding to old in SM", "binding", binding.Spec.ExternalName) - setCredRotationInProgressConditions(CredPreparing, errRenaming.Error(), binding) - if errStatus := r.updateStatus(ctx, binding); errStatus != nil { + utils.SetCredRotationInProgressConditions(common.CredPreparing, errRenaming.Error(), binding) + if errStatus := utils.UpdateStatus(ctx, r.Client, binding); errStatus != nil { return errStatus } return errRenaming @@ -824,8 +834,8 @@ func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, bindin if err := r.createOldBinding(ctx, suffix, binding); err != nil { log.Error(err, "Credentials rotation - failed to back up old binding in K8S") - setCredRotationInProgressConditions(CredPreparing, err.Error(), binding) - if errStatus := r.updateStatus(ctx, binding); errStatus != nil { + utils.SetCredRotationInProgressConditions(common.CredPreparing, err.Error(), binding) + if errStatus := utils.UpdateStatus(ctx, r.Client, binding); errStatus != nil { return errStatus } return err @@ -834,16 +844,16 @@ func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, bindin binding.Status.BindingID = "" binding.Status.Ready = metav1.ConditionFalse - setInProgressConditions(ctx, smClientTypes.CREATE, "rotating binding credentials", binding) - setCredRotationInProgressConditions(CredRotating, "", binding) - return r.updateStatus(ctx, binding) + utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "rotating binding credentials", binding) + utils.SetCredRotationInProgressConditions(common.CredRotating, "", binding) + return utils.UpdateStatus(ctx, r.Client, binding) } func (r *ServiceBindingReconciler) stopRotation(ctx context.Context, binding *servicesv1.ServiceBinding) error { conditions := binding.GetConditions() - meta.RemoveStatusCondition(&conditions, api.ConditionCredRotationInProgress) + meta.RemoveStatusCondition(&conditions, common.ConditionCredRotationInProgress) binding.Status.Conditions = conditions - return r.updateStatus(ctx, binding) + return utils.UpdateStatus(ctx, r.Client, binding) } func (r *ServiceBindingReconciler) createOldBinding(ctx context.Context, suffix string, binding *servicesv1.ServiceBinding) error { @@ -853,8 +863,8 @@ func (r *ServiceBindingReconciler) createOldBinding(ctx context.Context, suffix return err } oldBinding.Labels = map[string]string{ - api.StaleBindingIDLabel: binding.Status.BindingID, - api.StaleBindingRotationOfLabel: binding.Name, + common.StaleBindingIDLabel: binding.Status.BindingID, + common.StaleBindingRotationOfLabel: binding.Name, } spec := binding.Spec.DeepCopy() spec.CredRotationPolicy.Enabled = false @@ -865,8 +875,8 @@ func (r *ServiceBindingReconciler) createOldBinding(ctx context.Context, suffix } func (r *ServiceBindingReconciler) handleStaleServiceBinding(ctx context.Context, serviceBinding *servicesv1.ServiceBinding) (ctrl.Result, error) { - log := GetLogger(ctx) - originalBindingName, ok := serviceBinding.Labels[api.StaleBindingRotationOfLabel] + log := utils.GetLogger(ctx) + originalBindingName, ok := serviceBinding.Labels[common.StaleBindingRotationOfLabel] if !ok { //if the user removed the "rotationOf" label the stale binding should be deleted otherwise it will remain forever log.Info("missing rotationOf label, unable to fetch original binding, deleting stale") @@ -880,27 +890,27 @@ func (r *ServiceBindingReconciler) handleStaleServiceBinding(ctx context.Context } return ctrl.Result{}, err } - if meta.IsStatusConditionTrue(origBinding.Status.Conditions, api.ConditionReady) { + if meta.IsStatusConditionTrue(origBinding.Status.Conditions, common.ConditionReady) { return ctrl.Result{}, r.Client.Delete(ctx, serviceBinding) } log.Info("not deleting stale binding since original binding is not ready") - if !meta.IsStatusConditionPresentAndEqual(serviceBinding.Status.Conditions, api.ConditionPendingTermination, metav1.ConditionTrue) { + if !meta.IsStatusConditionPresentAndEqual(serviceBinding.Status.Conditions, common.ConditionPendingTermination, metav1.ConditionTrue) { pendingTerminationCondition := metav1.Condition{ - Type: api.ConditionPendingTermination, + Type: common.ConditionPendingTermination, Status: metav1.ConditionTrue, - Reason: api.ConditionPendingTermination, + Reason: common.ConditionPendingTermination, Message: "waiting for new credentials to be ready", ObservedGeneration: serviceBinding.GetGeneration(), } meta.SetStatusCondition(&serviceBinding.Status.Conditions, pendingTerminationCondition) - return ctrl.Result{}, r.updateStatus(ctx, serviceBinding) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding) } return ctrl.Result{}, nil } func (r *ServiceBindingReconciler) recover(ctx context.Context, serviceBinding *servicesv1.ServiceBinding, smBinding *smClientTypes.ServiceBinding) (ctrl.Result, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) log.Info(fmt.Sprintf("found existing smBinding in SM with id %s, updating status", smBinding.ID)) if smBinding.Credentials != nil { @@ -914,16 +924,16 @@ func (r *ServiceBindingReconciler) recover(ctx context.Context, serviceBinding * } r.resyncBindingStatus(ctx, serviceBinding, smBinding) - return ctrl.Result{}, r.updateStatus(ctx, serviceBinding) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding) } func isStaleServiceBinding(binding *servicesv1.ServiceBinding) bool { - if isMarkedForDeletion(binding.ObjectMeta) { + if utils.IsMarkedForDeletion(binding.ObjectMeta) { return false } if binding.Labels != nil { - if _, ok := binding.Labels[api.StaleBindingIDLabel]; ok { + if _, ok := binding.Labels[common.StaleBindingIDLabel]; ok { if binding.Spec.CredRotationPolicy != nil { keepFor, _ := time.ParseDuration(binding.Spec.CredRotationPolicy.RotatedBindingTTL) if time.Since(binding.CreationTimestamp.Time) > keepFor { @@ -936,10 +946,10 @@ func isStaleServiceBinding(binding *servicesv1.ServiceBinding) bool { } func initCredRotationIfRequired(binding *servicesv1.ServiceBinding) bool { - if isFailed(binding) || !credRotationEnabled(binding) || meta.IsStatusConditionTrue(binding.Status.Conditions, api.ConditionCredRotationInProgress) { + if utils.IsFailed(binding) || !credRotationEnabled(binding) || meta.IsStatusConditionTrue(binding.Status.Conditions, common.ConditionCredRotationInProgress) { return false } - _, forceRotate := binding.Annotations[api.ForceRotateAnnotation] + _, forceRotate := binding.Annotations[common.ForceRotateAnnotation] lastCredentialRotationTime := binding.Status.LastCredentialsRotationTime if lastCredentialRotationTime == nil { @@ -949,7 +959,7 @@ func initCredRotationIfRequired(binding *servicesv1.ServiceBinding) bool { rotationInterval, _ := time.ParseDuration(binding.Spec.CredRotationPolicy.RotationFrequency) if time.Since(lastCredentialRotationTime.Time) > rotationInterval || forceRotate { - setCredRotationInProgressConditions(CredPreparing, "", binding) + utils.SetCredRotationInProgressConditions(common.CredPreparing, "", binding) return true } @@ -964,7 +974,7 @@ func mergeInstanceTags(offeringTags, customTags []string) []string { var tags []string for _, tag := range append(offeringTags, customTags...) { - if !contains(tags, tag) { + if !utils.SliceContains(tags, tag) { tags = append(tags, tag) } } @@ -1002,17 +1012,17 @@ func bindingAlreadyOwnedByInstance(instance *servicesv1.ServiceInstance, binding } func serviceNotUsable(instance *servicesv1.ServiceInstance) bool { - if isMarkedForDeletion(instance.ObjectMeta) { + if utils.IsMarkedForDeletion(instance.ObjectMeta) { return true } if len(instance.Status.Conditions) != 0 { - return instance.Status.Conditions[0].Reason == getConditionReason(smClientTypes.CREATE, smClientTypes.FAILED) + return instance.Status.Conditions[0].Reason == utils.GetConditionReason(smClientTypes.CREATE, smClientTypes.FAILED) } return false } func getInstanceNameForSecretCredentials(instance *servicesv1.ServiceInstance) []byte { - if useMetaName, ok := instance.Annotations[api.UseInstanceMetadataNameInSecret]; ok && useMetaName == "true" { + if useMetaName, ok := instance.Annotations[common.UseInstanceMetadataNameInSecret]; ok && useMetaName == "true" { return []byte(instance.Name) } return []byte(instance.Spec.ExternalName) diff --git a/controllers/servicebinding_controller_test.go b/controllers/servicebinding_controller_test.go index 11d2fa82..b7635557 100644 --- a/controllers/servicebinding_controller_test.go +++ b/controllers/servicebinding_controller_test.go @@ -4,12 +4,13 @@ import ( "context" "encoding/json" "errors" + "github.com/SAP/sap-btp-service-operator/api/common" + "github.com/SAP/sap-btp-service-operator/internal/utils" "k8s.io/utils/pointer" "net/http" ctrl "sigs.k8s.io/controller-runtime" "strings" - "github.com/SAP/sap-btp-service-operator/api" v1 "github.com/SAP/sap-btp-service-operator/api/v1" "github.com/SAP/sap-btp-service-operator/client/sm" "github.com/SAP/sap-btp-service-operator/client/sm/smfakes" @@ -61,7 +62,7 @@ var _ = Describe("ServiceBinding controller", func() { } if wait { - return isResourceReady(createdBinding) || isFailed(createdBinding) + return isResourceReady(createdBinding) || utils.IsFailed(createdBinding) } else { return len(createdBinding.Status.Conditions) > 0 && createdBinding.Status.Conditions[0].Message != "Pending" } @@ -78,7 +79,7 @@ var _ = Describe("ServiceBinding controller", func() { if err != nil { Expect(err.Error()).To(ContainSubstring(failureMessage)) } else { - waitForResourceCondition(ctx, binding, api.ConditionFailed, metav1.ConditionTrue, "", failureMessage) + waitForResourceCondition(ctx, binding, common.ConditionFailed, metav1.ConditionTrue, "", failureMessage) } } @@ -87,7 +88,7 @@ var _ = Describe("ServiceBinding controller", func() { if err != nil { Expect(err.Error()).To(ContainSubstring(failureMessage)) } else { - waitForResourceCondition(ctx, binding, api.ConditionSucceeded, metav1.ConditionFalse, "", failureMessage) + waitForResourceCondition(ctx, binding, common.ConditionSucceeded, metav1.ConditionFalse, "", failureMessage) } return binding } @@ -139,24 +140,24 @@ var _ = Describe("ServiceBinding controller", func() { Expect(bindingSecret.Data).To(HaveKey("instance_guid")) } - validateSecretMetadata := func(bindingSecret *corev1.Secret, credentialProperties []SecretMetadataProperty) { - metadata := make(map[string][]SecretMetadataProperty) + validateSecretMetadata := func(bindingSecret *corev1.Secret, credentialProperties []utils.SecretMetadataProperty) { + metadata := make(map[string][]utils.SecretMetadataProperty) Expect(json.Unmarshal(bindingSecret.Data[".metadata"], &metadata)).To(Succeed()) if credentialProperties != nil { Expect(metadata["credentialProperties"]).To(ContainElements(credentialProperties)) } - Expect(metadata["metaDataProperties"]).To(ContainElement(SecretMetadataProperty{Name: "instance_name", Format: string(TEXT)})) - Expect(metadata["metaDataProperties"]).To(ContainElement(SecretMetadataProperty{Name: "instance_guid", Format: string(TEXT)})) - Expect(metadata["metaDataProperties"]).To(ContainElement(SecretMetadataProperty{Name: "plan", Format: string(TEXT)})) - Expect(metadata["metaDataProperties"]).To(ContainElement(SecretMetadataProperty{Name: "label", Format: string(TEXT)})) - Expect(metadata["metaDataProperties"]).To(ContainElement(SecretMetadataProperty{Name: "type", Format: string(TEXT)})) - Expect(metadata["metaDataProperties"]).To(ContainElement(SecretMetadataProperty{Name: "tags", Format: string(JSON)})) + Expect(metadata["metaDataProperties"]).To(ContainElement(utils.SecretMetadataProperty{Name: "instance_name", Format: string(utils.TEXT)})) + Expect(metadata["metaDataProperties"]).To(ContainElement(utils.SecretMetadataProperty{Name: "instance_guid", Format: string(utils.TEXT)})) + Expect(metadata["metaDataProperties"]).To(ContainElement(utils.SecretMetadataProperty{Name: "plan", Format: string(utils.TEXT)})) + Expect(metadata["metaDataProperties"]).To(ContainElement(utils.SecretMetadataProperty{Name: "label", Format: string(utils.TEXT)})) + Expect(metadata["metaDataProperties"]).To(ContainElement(utils.SecretMetadataProperty{Name: "type", Format: string(utils.TEXT)})) + Expect(metadata["metaDataProperties"]).To(ContainElement(utils.SecretMetadataProperty{Name: "tags", Format: string(utils.JSON)})) } BeforeEach(func() { ctx = context.Background() log := ctrl.Log.WithName("bindingTest") - ctx = context.WithValue(ctx, LogKey{}, log) + ctx = context.WithValue(ctx, utils.LogKey{}, log) testUUID = uuid.New().String() instanceName = "test-instance-" + testUUID bindingName = "test-binding-" + testUUID @@ -232,7 +233,7 @@ var _ = Describe("ServiceBinding controller", func() { binding.Spec.ServiceInstanceName = instanceName binding.Spec.SecretName = secretName Expect(k8sClient.Create(ctx, binding)).To(Succeed()) - waitForResourceCondition(ctx, binding, api.ConditionSucceeded, metav1.ConditionFalse, Blocked, fmt.Sprintf(secretNameTakenErrorFormat, secretName)) + waitForResourceCondition(ctx, binding, common.ConditionSucceeded, metav1.ConditionFalse, common.Blocked, fmt.Sprintf(secretNameTakenErrorFormat, secretName)) }) }) @@ -255,7 +256,7 @@ var _ = Describe("ServiceBinding controller", func() { binding.Spec.SecretName = secretName Expect(k8sClient.Create(ctx, binding)).To(Succeed()) - waitForResourceCondition(ctx, binding, api.ConditionSucceeded, metav1.ConditionFalse, Blocked, fmt.Sprintf(secretAlreadyOwnedErrorFormat, secretName, owningBindingName)) + waitForResourceCondition(ctx, binding, common.ConditionSucceeded, metav1.ConditionFalse, common.Blocked, fmt.Sprintf(secretAlreadyOwnedErrorFormat, secretName, owningBindingName)) bindingLookupKey := getResourceNamespacedName(binding) binding.Spec.SecretName = secretName + "-new" @@ -281,14 +282,14 @@ var _ = Describe("ServiceBinding controller", func() { validateSecretData(bindingSecret, "secret_key", "secret_value") validateSecretData(bindingSecret, "escaped", `{"escaped_key":"escaped_val"}`) validateInstanceInfo(bindingSecret, instanceExternalName) - credentialProperties := []SecretMetadataProperty{ + credentialProperties := []utils.SecretMetadataProperty{ { Name: "secret_key", - Format: string(TEXT), + Format: string(utils.TEXT), }, { Name: "escaped", - Format: string(TEXT), + Format: string(utils.TEXT), }, } validateSecretMetadata(bindingSecret, credentialProperties) @@ -307,10 +308,10 @@ var _ = Describe("ServiceBinding controller", func() { bindingSecret := getSecret(ctx, binding.Spec.SecretName, bindingTestNamespace, true) validateSecretData(bindingSecret, secretKey, `{"secret_key": "secret_value", "escaped": "{\"escaped_key\":\"escaped_val\"}"}`) validateInstanceInfo(bindingSecret, instanceExternalName) - credentialProperties := []SecretMetadataProperty{ + credentialProperties := []utils.SecretMetadataProperty{ { Name: "mycredentials", - Format: string(JSON), + Format: string(utils.JSON), Container: true, }, } @@ -435,7 +436,7 @@ var _ = Describe("ServiceBinding controller", func() { false, ) - cond := meta.FindStatusCondition(createdBinding.GetConditions(), api.ConditionSucceeded) + cond := meta.FindStatusCondition(createdBinding.GetConditions(), common.ConditionSucceeded) Expect(cond).To(Not(BeNil())) Expect(cond.Message).To(ContainSubstring(errorMessage)) Expect(cond.Status).To(Equal(metav1.ConditionFalse)) @@ -466,7 +467,7 @@ var _ = Describe("ServiceBinding controller", func() { It("creation will fail with appropriate message", func() { createdBinding, _ = createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "") - waitForResourceCondition(ctx, createdBinding, api.ConditionFailed, metav1.ConditionTrue, "CreateFailed", "failed to create secret") + waitForResourceCondition(ctx, createdBinding, common.ConditionFailed, metav1.ConditionTrue, "CreateFailed", "failed to create secret") }) }) }) @@ -505,7 +506,7 @@ var _ = Describe("ServiceBinding controller", func() { It("should eventually succeed", func() { binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "") Expect(err).ToNot(HaveOccurred()) - waitForResourceCondition(ctx, binding, api.ConditionFailed, metav1.ConditionTrue, "", "no polling for you") + waitForResourceCondition(ctx, binding, common.ConditionFailed, metav1.ConditionTrue, "", "no polling for you") fakeClient.ListBindingsReturns(&smClientTypes.ServiceBindings{ ServiceBindings: []smClientTypes.ServiceBinding{ { @@ -528,7 +529,7 @@ var _ = Describe("ServiceBinding controller", func() { Context("useMetaName annotation is provided", func() { It("should put in the secret.instance_name the instance meta.name", func() { createdInstance.Annotations = map[string]string{ - api.UseInstanceMetadataNameInSecret: "true", + common.UseInstanceMetadataNameInSecret: "true", } updateInstance(ctx, createdInstance) createdBinding = createBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "") @@ -556,10 +557,10 @@ var _ = Describe("ServiceBinding controller", func() { When("referenced service instance is failed", func() { It("should retry and succeed once the instance is ready", func() { - setFailureConditions(smClientTypes.CREATE, "Failed to create instance (test)", createdInstance) + utils.SetFailureConditions(smClientTypes.CREATE, "Failed to create instance (test)", createdInstance) updateInstanceStatus(ctx, createdInstance) binding := createBindingWithBlockedError(ctx, bindingName, bindingTestNamespace, instanceName, "binding-external-name", "is not usable") - setSuccessConditions(smClientTypes.CREATE, createdInstance) + utils.SetSuccessConditions(smClientTypes.CREATE, createdInstance) updateInstanceStatus(ctx, createdInstance) waitForResourceToBeReady(ctx, binding) }) @@ -568,16 +569,16 @@ var _ = Describe("ServiceBinding controller", func() { When("referenced service instance is not ready", func() { It("should retry and succeed once the instance is ready", func() { fakeClient.StatusReturns(&smClientTypes.Operation{ResourceID: fakeInstanceID, State: smClientTypes.INPROGRESS}, nil) - setInProgressConditions(ctx, smClientTypes.CREATE, "", createdInstance) + utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "", createdInstance) createdInstance.Status.OperationURL = "/1234" createdInstance.Status.OperationType = smClientTypes.CREATE updateInstanceStatus(ctx, createdInstance) createdBinding, err := createBindingWithoutAssertionsAndWait(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", false) Expect(err).ToNot(HaveOccurred()) - Expect(isInProgress(createdBinding)).To(BeTrue()) + Expect(utils.IsInProgress(createdBinding)).To(BeTrue()) - setSuccessConditions(smClientTypes.CREATE, createdInstance) + utils.SetSuccessConditions(smClientTypes.CREATE, createdInstance) createdInstance.Status.OperationType = "" createdInstance.Status.OperationURL = "" updateInstanceStatus(ctx, createdInstance) @@ -696,7 +697,7 @@ var _ = Describe("ServiceBinding controller", func() { if err != nil { return false } - failedCond := meta.FindStatusCondition(createdBinding.GetConditions(), api.ConditionFailed) + failedCond := meta.FindStatusCondition(createdBinding.GetConditions(), common.ConditionFailed) return failedCond != nil && strings.Contains(failedCond.Message, errorMessage) }, timeout, interval).Should(BeTrue()) Expect(len(createdBinding.Finalizers)).To(Equal(1)) @@ -722,15 +723,15 @@ var _ = Describe("ServiceBinding controller", func() { It("should succeed", func() { createdBinding, err := createBindingWithoutAssertions(ctx, bindingName+"-new", bindingTestNamespace, "non-exist-instance", "", "binding-external-name") Expect(err).ToNot(HaveOccurred()) - createdBinding.Finalizers = []string{api.FinalizerName} + createdBinding.Finalizers = []string{common.FinalizerName} Expect(k8sClient.Update(ctx, createdBinding)) Eventually(func() bool { err := k8sClient.Get(ctx, getResourceNamespacedName(createdBinding), createdBinding) if err != nil { return false } - cond := meta.FindStatusCondition(createdBinding.GetConditions(), api.ConditionSucceeded) - return cond != nil && cond.Reason == Blocked + cond := meta.FindStatusCondition(createdBinding.GetConditions(), common.ConditionSucceeded) + return cond != nil && cond.Reason == common.Blocked }, timeout, interval).Should(BeTrue()) deleteAndValidate(createdBinding) }) @@ -770,7 +771,7 @@ var _ = Describe("ServiceBinding controller", func() { if err != nil { return false } - failedCond := meta.FindStatusCondition(createdBinding.GetConditions(), api.ConditionFailed) + failedCond := meta.FindStatusCondition(createdBinding.GetConditions(), common.ConditionFailed) return failedCond != nil && strings.Contains(failedCond.Message, errorMessage) }, timeout, interval).Should(BeTrue()) fakeClient.UnbindReturns("", nil) @@ -792,7 +793,7 @@ var _ = Describe("ServiceBinding controller", func() { if err != nil { return false } - failedCond := meta.FindStatusCondition(createdBinding.GetConditions(), api.ConditionFailed) + failedCond := meta.FindStatusCondition(createdBinding.GetConditions(), common.ConditionFailed) return failedCond != nil && strings.Contains(failedCond.Message, "no polling for you") }, timeout, interval).Should(BeTrue()) fakeClient.UnbindReturns("", nil) @@ -850,13 +851,13 @@ var _ = Describe("ServiceBinding controller", func() { Expect(smCallArgs.FieldQuery[1]).To(ContainSubstring("context/clusterid")) Expect(smCallArgs.FieldQuery[2]).To(ContainSubstring("context/namespace")) - waitForResourceCondition(ctx, createdBinding, api.ConditionSucceeded, testCase.expectedConditionSucceededStatus, getConditionReason(testCase.lastOpType, testCase.lastOpState), "") + waitForResourceCondition(ctx, createdBinding, common.ConditionSucceeded, testCase.expectedConditionSucceededStatus, utils.GetConditionReason(testCase.lastOpType, testCase.lastOpState), "") switch testCase.lastOpState { case smClientTypes.FAILED: - Expect(isFailed(createdBinding)) + Expect(utils.IsFailed(createdBinding)) case smClientTypes.INPROGRESS: - Expect(isInProgress(createdBinding)) + Expect(utils.IsInProgress(createdBinding)) case smClientTypes.SUCCEEDED: Expect(isResourceReady(createdBinding)) } @@ -955,7 +956,7 @@ var _ = Describe("ServiceBinding controller", func() { bindingList := &v1.ServiceBindingList{} Eventually(func() bool { - Expect(k8sClient.List(ctx, bindingList, client.MatchingLabels{api.StaleBindingIDLabel: myBinding.Status.BindingID}, client.InNamespace(bindingTestNamespace))).To(Succeed()) + Expect(k8sClient.List(ctx, bindingList, client.MatchingLabels{common.StaleBindingIDLabel: myBinding.Status.BindingID}, client.InNamespace(bindingTestNamespace))).To(Succeed()) return len(bindingList.Items) > 0 }, timeout, interval).Should(BeTrue()) @@ -975,7 +976,7 @@ var _ = Describe("ServiceBinding controller", func() { RotatedBindingTTL: "1h", } createdBinding.Annotations = map[string]string{ - api.ForceRotateAnnotation: "true", + common.ForceRotateAnnotation: "true", } updateBinding(ctx, defaultLookupKey, createdBinding) @@ -985,7 +986,7 @@ var _ = Describe("ServiceBinding controller", func() { return err == nil && myBinding.Status.LastCredentialsRotationTime != nil }, timeout, interval).Should(BeTrue()) - _, ok := myBinding.Annotations[api.ForceRotateAnnotation] + _, ok := myBinding.Annotations[common.ForceRotateAnnotation] Expect(ok).To(BeFalse()) }) @@ -994,8 +995,8 @@ var _ = Describe("ServiceBinding controller", func() { Expect(k8sClient.Get(ctx, types.NamespacedName{Name: createdBinding.Name, Namespace: bindingTestNamespace}, createdBinding)).To(Succeed()) staleBinding := generateBasicStaleBinding(createdBinding) staleBinding.Labels = map[string]string{ - api.StaleBindingIDLabel: createdBinding.Status.BindingID, - api.StaleBindingRotationOfLabel: createdBinding.Name, + common.StaleBindingIDLabel: createdBinding.Status.BindingID, + common.StaleBindingRotationOfLabel: createdBinding.Name, } staleBinding.Spec.CredRotationPolicy = &v1.CredentialsRotationPolicy{ Enabled: false, @@ -1013,15 +1014,15 @@ var _ = Describe("ServiceBinding controller", func() { failedBinding = newBindingObject("failedbinding", bindingTestNamespace) failedBinding.Spec.ServiceInstanceName = "notexistinstance" Expect(k8sClient.Create(ctx, failedBinding)).To(Succeed()) - waitForResourceCondition(ctx, failedBinding, api.ConditionSucceeded, metav1.ConditionFalse, Blocked, "") + waitForResourceCondition(ctx, failedBinding, common.ConditionSucceeded, metav1.ConditionFalse, common.Blocked, "") }) It("should not delete old binding when stale", func() { Expect(k8sClient.Get(ctx, types.NamespacedName{Name: createdBinding.Name, Namespace: bindingTestNamespace}, createdBinding)).To(Succeed()) staleBinding := generateBasicStaleBinding(createdBinding) staleBinding.Labels = map[string]string{ - api.StaleBindingIDLabel: "1234", - api.StaleBindingRotationOfLabel: failedBinding.Name, + common.StaleBindingIDLabel: "1234", + common.StaleBindingRotationOfLabel: failedBinding.Name, } staleBinding.Spec.CredRotationPolicy = &v1.CredentialsRotationPolicy{ Enabled: false, @@ -1029,7 +1030,7 @@ var _ = Describe("ServiceBinding controller", func() { RotationFrequency: "0ns", } Expect(k8sClient.Create(ctx, staleBinding)).To(Succeed()) - waitForResourceCondition(ctx, staleBinding, api.ConditionPendingTermination, metav1.ConditionTrue, api.ConditionPendingTermination, "") + waitForResourceCondition(ctx, staleBinding, common.ConditionPendingTermination, metav1.ConditionTrue, common.ConditionPendingTermination, "") }) }) @@ -1038,7 +1039,7 @@ var _ = Describe("ServiceBinding controller", func() { Expect(k8sClient.Get(ctx, types.NamespacedName{Name: createdBinding.Name, Namespace: bindingTestNamespace}, createdBinding)).To(Succeed()) staleBinding := generateBasicStaleBinding(createdBinding) staleBinding.Labels = map[string]string{ - api.StaleBindingIDLabel: createdBinding.Status.BindingID, + common.StaleBindingIDLabel: createdBinding.Status.BindingID, } staleBinding.Spec.CredRotationPolicy = &v1.CredentialsRotationPolicy{ Enabled: false, @@ -1147,7 +1148,7 @@ var _ = Describe("ServiceBinding controller", func() { bindingList := &v1.ServiceBindingList{} Eventually(func() bool { - Expect(k8sClient.List(ctx, bindingList, client.MatchingLabels{api.StaleBindingIDLabel: myBinding.Status.BindingID}, client.InNamespace(testNamespace))).To(Succeed()) + Expect(k8sClient.List(ctx, bindingList, client.MatchingLabels{common.StaleBindingIDLabel: myBinding.Status.BindingID}, client.InNamespace(testNamespace))).To(Succeed()) return len(bindingList.Items) > 0 }, timeout, interval).Should(BeTrue()) oldBinding := bindingList.Items[0] diff --git a/controllers/serviceinstance_controller.go b/controllers/serviceinstance_controller.go index 1a791481..ed8aae6c 100644 --- a/controllers/serviceinstance_controller.go +++ b/controllers/serviceinstance_controller.go @@ -24,6 +24,13 @@ import ( "fmt" "net/http" + "github.com/SAP/sap-btp-service-operator/api/common" + "github.com/SAP/sap-btp-service-operator/internal/config" + "github.com/SAP/sap-btp-service-operator/internal/utils" + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -31,8 +38,6 @@ import ( "k8s.io/apimachinery/pkg/api/meta" "k8s.io/utils/pointer" - "github.com/SAP/sap-btp-service-operator/api" - "github.com/google/uuid" "github.com/SAP/sap-btp-service-operator/client/sm" @@ -46,7 +51,13 @@ import ( // ServiceInstanceReconciler reconciles a ServiceInstance object type ServiceInstanceReconciler struct { - *BaseReconciler + 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 } // +kubebuilder:rbac:groups=services.cloud.sap.com,resources=serviceinstances,verbs=get;list;watch;create;update;patch;delete @@ -56,7 +67,7 @@ type ServiceInstanceReconciler struct { func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := r.Log.WithValues("serviceinstance", req.NamespacedName).WithValues("correlation_id", uuid.New().String()) - ctx = context.WithValue(ctx, LogKey{}, log) + ctx = context.WithValue(ctx, utils.LogKey{}, log) serviceInstance := &servicesv1.ServiceInstance{} if err := r.Client.Get(ctx, req.NamespacedName, serviceInstance); err != nil { @@ -71,14 +82,14 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ serviceInstance = serviceInstance.DeepCopy() if len(serviceInstance.GetConditions()) == 0 { - err := r.init(ctx, serviceInstance) + err := utils.InitConditions(ctx, r.Client, serviceInstance) if err != nil { return ctrl.Result{}, err } } - if meta.IsStatusConditionPresentAndEqual(serviceInstance.GetConditions(), api.ConditionFailed, metav1.ConditionTrue) && - shouldIgnoreNonTransient(log, serviceInstance, r.Config.IgnoreNonTransientTimeout) { + if meta.IsStatusConditionPresentAndEqual(serviceInstance.GetConditions(), common.ConditionFailed, metav1.ConditionTrue) && + utils.ShouldIgnoreNonTransient(log, serviceInstance, r.Config.IgnoreNonTransientTimeout) { markNonTransientStatusAsTransient(serviceInstance) if err := r.Status().Update(ctx, serviceInstance); err != nil { return ctrl.Result{}, err @@ -94,10 +105,10 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ } } - return ctrl.Result{}, r.removeAnnotation(ctx, serviceInstance, api.IgnoreNonTransientErrorAnnotation, api.IgnoreNonTransientErrorTimestampAnnotation) + return ctrl.Result{}, utils.RemoveAnnotations(ctx, r.Client, serviceInstance, common.IgnoreNonTransientErrorAnnotation, common.IgnoreNonTransientErrorTimestampAnnotation) } - if isMarkedForDeletion(serviceInstance.ObjectMeta) { + if utils.IsMarkedForDeletion(serviceInstance.ObjectMeta) { // delete updates the generation serviceInstance.SetObservedGeneration(serviceInstance.Generation) return r.deleteInstance(ctx, serviceInstance) @@ -108,9 +119,9 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ return r.poll(ctx, serviceInstance) } - if !controllerutil.ContainsFinalizer(serviceInstance, api.FinalizerName) { - controllerutil.AddFinalizer(serviceInstance, api.FinalizerName) - log.Info(fmt.Sprintf("added finalizer '%s' to service instance", api.FinalizerName)) + if !controllerutil.ContainsFinalizer(serviceInstance, common.FinalizerName) { + controllerutil.AddFinalizer(serviceInstance, common.FinalizerName) + log.Info(fmt.Sprintf("added finalizer '%s' to service instance", common.FinalizerName)) if err := r.Client.Update(ctx, serviceInstance); err != nil { return ctrl.Result{}, err } @@ -119,10 +130,10 @@ 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, serviceInstance, serviceInstance.Spec.BTPAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, r.SecretResolver, serviceInstance.Namespace, serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { log.Error(err, "failed to get sm client") - return r.markAsTransientError(ctx, Unknown, err, serviceInstance) + return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceInstance) } if serviceInstance.Status.InstanceID == "" { @@ -130,7 +141,7 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ smInstance, err := r.getInstanceForRecovery(ctx, smClient, serviceInstance) if err != nil { log.Error(err, "failed to check instance recovery") - return r.markAsTransientError(ctx, Unknown, err, serviceInstance) + return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceInstance) } if smInstance != nil { return r.recover(ctx, smClient, serviceInstance, smInstance) @@ -166,14 +177,14 @@ func (r *ServiceInstanceReconciler) SetupWithManager(mgr ctrl.Manager) error { } func (r *ServiceInstanceReconciler) createInstance(ctx context.Context, smClient sm.Client, serviceInstance *servicesv1.ServiceInstance) (ctrl.Result, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) log.Info("Creating instance in SM") updateHashedSpecValue(serviceInstance) - _, instanceParameters, err := buildParameters(r.Client, serviceInstance.Namespace, serviceInstance.Spec.ParametersFrom, serviceInstance.Spec.Parameters) + _, instanceParameters, err := utils.BuildSMRequestParameters(r.Client, 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") - return r.markAsNonTransientError(ctx, smClientTypes.CREATE, err.Error(), serviceInstance) + return utils.MarkAsNonTransientError(ctx, r.Client, smClientTypes.CREATE, err.Error(), serviceInstance) } provision, provisionErr := smClient.Provision(&smClientTypes.ServiceInstance{ @@ -181,16 +192,16 @@ func (r *ServiceInstanceReconciler) createInstance(ctx context.Context, smClient ServicePlanID: serviceInstance.Spec.ServicePlanID, Parameters: instanceParameters, Labels: smClientTypes.Labels{ - namespaceLabel: []string{serviceInstance.Namespace}, - k8sNameLabel: []string{serviceInstance.Name}, - clusterIDLabel: []string{r.Config.ClusterID}, + common.NamespaceLabel: []string{serviceInstance.Namespace}, + common.K8sNameLabel: []string{serviceInstance.Name}, + common.ClusterIDLabel: []string{r.Config.ClusterID}, }, - }, serviceInstance.Spec.ServiceOfferingName, serviceInstance.Spec.ServicePlanName, nil, buildUserInfo(ctx, serviceInstance.Spec.UserInfo), serviceInstance.Spec.DataCenter) + }, serviceInstance.Spec.ServiceOfferingName, serviceInstance.Spec.ServicePlanName, nil, utils.BuildUserInfo(ctx, serviceInstance.Spec.UserInfo), serviceInstance.Spec.DataCenter) if provisionErr != nil { log.Error(provisionErr, "failed to create service instance", "serviceOfferingName", serviceInstance.Spec.ServiceOfferingName, "servicePlanName", serviceInstance.Spec.ServicePlanName) - return r.handleError(ctx, smClientTypes.CREATE, provisionErr, serviceInstance) + return utils.HandleError(ctx, r.Client, smClientTypes.CREATE, provisionErr, serviceInstance, utils.ShouldIgnoreNonTransient(log, serviceInstance, r.Config.IgnoreNonTransientTimeout)) } serviceInstance.Status.InstanceID = provision.InstanceID @@ -208,65 +219,65 @@ func (r *ServiceInstanceReconciler) createInstance(ctx context.Context, smClient log.Info("Provision request is in progress (async)") serviceInstance.Status.OperationURL = provision.Location serviceInstance.Status.OperationType = smClientTypes.CREATE - setInProgressConditions(ctx, smClientTypes.CREATE, "", serviceInstance) + utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "", serviceInstance) - return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, r.updateStatus(ctx, serviceInstance) + return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, utils.UpdateStatus(ctx, r.Client, serviceInstance) } log.Info(fmt.Sprintf("Instance provisioned successfully, instanceID: %s, subaccountID: %s", serviceInstance.Status.InstanceID, serviceInstance.Status.SubaccountID)) - setSuccessConditions(smClientTypes.CREATE, serviceInstance) - return ctrl.Result{}, r.updateStatus(ctx, serviceInstance) + utils.SetSuccessConditions(smClientTypes.CREATE, serviceInstance) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceInstance) } func (r *ServiceInstanceReconciler) updateInstance(ctx context.Context, smClient sm.Client, serviceInstance *servicesv1.ServiceInstance) (ctrl.Result, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) log.Info(fmt.Sprintf("updating instance %s in SM", serviceInstance.Status.InstanceID)) updateHashedSpecValue(serviceInstance) - _, instanceParameters, err := buildParameters(r.Client, serviceInstance.Namespace, serviceInstance.Spec.ParametersFrom, serviceInstance.Spec.Parameters) + _, instanceParameters, err := utils.BuildSMRequestParameters(r.Client, serviceInstance.Namespace, serviceInstance.Spec.ParametersFrom, serviceInstance.Spec.Parameters) if err != nil { log.Error(err, "failed to parse instance parameters") - return r.markAsNonTransientError(ctx, smClientTypes.UPDATE, fmt.Sprintf("failed to parse parameters: %v", err.Error()), serviceInstance) + return utils.MarkAsNonTransientError(ctx, r.Client, smClientTypes.UPDATE, fmt.Sprintf("failed to parse parameters: %v", err.Error()), serviceInstance) } _, operationURL, err := smClient.UpdateInstance(serviceInstance.Status.InstanceID, &smClientTypes.ServiceInstance{ Name: serviceInstance.Spec.ExternalName, ServicePlanID: serviceInstance.Spec.ServicePlanID, Parameters: instanceParameters, - }, serviceInstance.Spec.ServiceOfferingName, serviceInstance.Spec.ServicePlanName, nil, buildUserInfo(ctx, serviceInstance.Spec.UserInfo), serviceInstance.Spec.DataCenter) + }, serviceInstance.Spec.ServiceOfferingName, serviceInstance.Spec.ServicePlanName, nil, utils.BuildUserInfo(ctx, serviceInstance.Spec.UserInfo), serviceInstance.Spec.DataCenter) if err != nil { log.Error(err, fmt.Sprintf("failed to update service instance with ID %s", serviceInstance.Status.InstanceID)) - return r.handleError(ctx, smClientTypes.UPDATE, err, serviceInstance) + return utils.HandleError(ctx, r.Client, smClientTypes.UPDATE, err, serviceInstance, utils.ShouldIgnoreNonTransient(log, serviceInstance, r.Config.IgnoreNonTransientTimeout)) } if operationURL != "" { log.Info(fmt.Sprintf("Update request accepted, operation URL: %s", operationURL)) serviceInstance.Status.OperationURL = operationURL serviceInstance.Status.OperationType = smClientTypes.UPDATE - setInProgressConditions(ctx, smClientTypes.UPDATE, "", serviceInstance) + utils.SetInProgressConditions(ctx, smClientTypes.UPDATE, "", serviceInstance) - if err := r.updateStatus(ctx, serviceInstance); err != nil { + if err := utils.UpdateStatus(ctx, r.Client, serviceInstance); err != nil { return ctrl.Result{}, err } return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, nil } log.Info("Instance updated successfully") - setSuccessConditions(smClientTypes.UPDATE, serviceInstance) - return ctrl.Result{}, r.updateStatus(ctx, serviceInstance) + utils.SetSuccessConditions(smClientTypes.UPDATE, serviceInstance) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceInstance) } func (r *ServiceInstanceReconciler) deleteInstance(ctx context.Context, serviceInstance *servicesv1.ServiceInstance) (ctrl.Result, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) - if controllerutil.ContainsFinalizer(serviceInstance, api.FinalizerName) { - smClient, err := r.getSMClient(ctx, serviceInstance, serviceInstance.Spec.BTPAccessCredentialsSecret) + if controllerutil.ContainsFinalizer(serviceInstance, common.FinalizerName) { + smClient, err := r.GetSMClient(ctx, r.SecretResolver, serviceInstance.Namespace, serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { log.Error(err, "failed to get sm client") - return r.markAsTransientError(ctx, Unknown, err, serviceInstance) + return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceInstance) } if len(serviceInstance.Status.InstanceID) == 0 { log.Info("No instance id found validating instance does not exists in SM before removing finalizer") @@ -277,11 +288,11 @@ func (r *ServiceInstanceReconciler) deleteInstance(ctx context.Context, serviceI if smInstance != nil { log.Info("instance exists in SM continue with deletion") serviceInstance.Status.InstanceID = smInstance.ID - setInProgressConditions(ctx, smClientTypes.DELETE, "delete after recovery", serviceInstance) - return ctrl.Result{}, r.updateStatus(ctx, serviceInstance) + utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "delete after recovery", serviceInstance) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceInstance) } log.Info("instance does not exists in SM, removing finalizer") - return ctrl.Result{}, r.removeFinalizer(ctx, serviceInstance, api.FinalizerName) + return ctrl.Result{}, utils.RemoveFinalizer(ctx, r.Client, serviceInstance, common.FinalizerName) } if len(serviceInstance.Status.OperationURL) > 0 && serviceInstance.Status.OperationType == smClientTypes.DELETE { @@ -290,10 +301,10 @@ func (r *ServiceInstanceReconciler) deleteInstance(ctx context.Context, serviceI } log.Info(fmt.Sprintf("Deleting instance with id %v from SM", serviceInstance.Status.InstanceID)) - operationURL, deprovisionErr := smClient.Deprovision(serviceInstance.Status.InstanceID, nil, buildUserInfo(ctx, serviceInstance.Spec.UserInfo)) + operationURL, deprovisionErr := smClient.Deprovision(serviceInstance.Status.InstanceID, nil, utils.BuildUserInfo(ctx, serviceInstance.Spec.UserInfo)) if deprovisionErr != nil { // delete will proceed anyway - return r.markAsNonTransientError(ctx, smClientTypes.DELETE, deprovisionErr.Error(), serviceInstance) + return utils.MarkAsNonTransientError(ctx, r.Client, smClientTypes.DELETE, deprovisionErr.Error(), serviceInstance) } if operationURL != "" { @@ -303,65 +314,65 @@ func (r *ServiceInstanceReconciler) deleteInstance(ctx context.Context, serviceI log.Info("Instance was deleted successfully, removing finalizer") // remove our finalizer from the list and update it. - return ctrl.Result{}, r.removeFinalizer(ctx, serviceInstance, api.FinalizerName) + return ctrl.Result{}, utils.RemoveFinalizer(ctx, r.Client, serviceInstance, common.FinalizerName) } return ctrl.Result{}, nil } func (r *ServiceInstanceReconciler) handleInstanceSharing(ctx context.Context, serviceInstance *servicesv1.ServiceInstance, smClient sm.Client) (ctrl.Result, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) log.Info("Handling change in instance sharing") if serviceInstance.ShouldBeShared() { log.Info("Service instance is shouldBeShared, sharing the instance") - err := smClient.ShareInstance(serviceInstance.Status.InstanceID, buildUserInfo(ctx, serviceInstance.Spec.UserInfo)) + err := smClient.ShareInstance(serviceInstance.Status.InstanceID, utils.BuildUserInfo(ctx, serviceInstance.Spec.UserInfo)) if err != nil { log.Error(err, "failed to share instance") - return r.handleInstanceSharingError(ctx, serviceInstance, metav1.ConditionFalse, ShareFailed, err) + return r.handleInstanceSharingError(ctx, serviceInstance, metav1.ConditionFalse, common.ShareFailed, err) } log.Info("instance shared successfully") - setSharedCondition(serviceInstance, metav1.ConditionTrue, ShareSucceeded, "instance shared successfully") + setSharedCondition(serviceInstance, metav1.ConditionTrue, common.ShareSucceeded, "instance shared successfully") } else { //un-share log.Info("Service instance is un-shouldBeShared, un-sharing the instance") - err := smClient.UnShareInstance(serviceInstance.Status.InstanceID, buildUserInfo(ctx, serviceInstance.Spec.UserInfo)) + err := smClient.UnShareInstance(serviceInstance.Status.InstanceID, utils.BuildUserInfo(ctx, serviceInstance.Spec.UserInfo)) if err != nil { log.Error(err, "failed to un-share instance") - return r.handleInstanceSharingError(ctx, serviceInstance, metav1.ConditionTrue, UnShareFailed, err) + return r.handleInstanceSharingError(ctx, serviceInstance, metav1.ConditionTrue, common.UnShareFailed, err) } log.Info("instance un-shared successfully") if serviceInstance.Spec.Shared != nil { - setSharedCondition(serviceInstance, metav1.ConditionFalse, UnShareSucceeded, "instance un-shared successfully") + setSharedCondition(serviceInstance, metav1.ConditionFalse, common.UnShareSucceeded, "instance un-shared successfully") } else { log.Info("removing Shared condition since shared is undefined in instance") conditions := serviceInstance.GetConditions() - meta.RemoveStatusCondition(&conditions, api.ConditionShared) + meta.RemoveStatusCondition(&conditions, common.ConditionShared) serviceInstance.SetConditions(conditions) } } - return ctrl.Result{}, r.updateStatus(ctx, serviceInstance) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceInstance) } func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *servicesv1.ServiceInstance) (ctrl.Result, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) log.Info(fmt.Sprintf("resource is in progress, found operation url %s", serviceInstance.Status.OperationURL)) - smClient, err := r.getSMClient(ctx, serviceInstance, serviceInstance.Spec.BTPAccessCredentialsSecret) + smClient, err := r.GetSMClient(ctx, r.SecretResolver, serviceInstance.Namespace, serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { log.Error(err, "failed to get sm client") - return r.markAsTransientError(ctx, Unknown, err, serviceInstance) + return utils.MarkAsTransientError(ctx, r.Client, common.Unknown, err, serviceInstance) } status, statusErr := smClient.Status(serviceInstance.Status.OperationURL, nil) if statusErr != nil { log.Info(fmt.Sprintf("failed to fetch operation, got error from SM: %s", statusErr.Error()), "operationURL", serviceInstance.Status.OperationURL) - setInProgressConditions(ctx, serviceInstance.Status.OperationType, statusErr.Error(), serviceInstance) + utils.SetInProgressConditions(ctx, serviceInstance.Status.OperationType, statusErr.Error(), serviceInstance) // if failed to read operation status we cleanup the status to trigger re-sync from SM freshStatus := servicesv1.ServiceInstanceStatus{Conditions: serviceInstance.GetConditions(), ObservedGeneration: serviceInstance.Generation} - if isMarkedForDeletion(serviceInstance.ObjectMeta) { + if utils.IsMarkedForDeletion(serviceInstance.ObjectMeta) { freshStatus.InstanceID = serviceInstance.Status.InstanceID } serviceInstance.Status = freshStatus - if err := r.updateStatus(ctx, serviceInstance); err != nil { + if err := utils.UpdateStatus(ctx, r.Client, serviceInstance); err != nil { log.Error(err, "failed to update status during polling") } return ctrl.Result{}, statusErr @@ -377,8 +388,8 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *s case smClientTypes.PENDING: if len(status.Description) > 0 { log.Info(fmt.Sprintf("last operation description is '%s'", status.Description)) - setInProgressConditions(ctx, status.Type, status.Description, serviceInstance) - if err := r.updateStatus(ctx, serviceInstance); err != nil { + utils.SetInProgressConditions(ctx, status.Type, status.Description, serviceInstance) + if err := utils.UpdateStatus(ctx, r.Client, serviceInstance); err != nil { log.Error(err, "unable to update ServiceInstance polling description") return ctrl.Result{}, err } @@ -386,12 +397,12 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *s return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, nil case smClientTypes.FAILED: errMsg := getErrorMsgFromLastOperation(status) - setFailureConditions(status.Type, errMsg, serviceInstance) + utils.SetFailureConditions(status.Type, errMsg, serviceInstance) // in order to delete eventually the object we need return with error if serviceInstance.Status.OperationType == smClientTypes.DELETE { serviceInstance.Status.OperationURL = "" serviceInstance.Status.OperationType = "" - if err := r.updateStatus(ctx, serviceInstance); err != nil { + if err := utils.UpdateStatus(ctx, r.Client, serviceInstance); err != nil { return ctrl.Result{}, err } return ctrl.Result{}, fmt.Errorf(errMsg) @@ -409,25 +420,25 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *s serviceInstance.Status.Ready = metav1.ConditionTrue } else if serviceInstance.Status.OperationType == smClientTypes.DELETE { // delete was successful - remove our finalizer from the list and update it. - if err := r.removeFinalizer(ctx, serviceInstance, api.FinalizerName); err != nil { + if err := utils.RemoveFinalizer(ctx, r.Client, serviceInstance, common.FinalizerName); err != nil { return ctrl.Result{}, err } } - setSuccessConditions(status.Type, serviceInstance) + utils.SetSuccessConditions(status.Type, serviceInstance) } serviceInstance.Status.OperationURL = "" serviceInstance.Status.OperationType = "" - return ctrl.Result{}, r.updateStatus(ctx, serviceInstance) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceInstance) } func (r *ServiceInstanceReconciler) handleAsyncDelete(ctx context.Context, serviceInstance *servicesv1.ServiceInstance, opURL string) (ctrl.Result, error) { serviceInstance.Status.OperationURL = opURL serviceInstance.Status.OperationType = smClientTypes.DELETE - setInProgressConditions(ctx, smClientTypes.DELETE, "", serviceInstance) + utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "", serviceInstance) - if err := r.updateStatus(ctx, serviceInstance); err != nil { + if err := utils.UpdateStatus(ctx, r.Client, serviceInstance); err != nil { return ctrl.Result{}, err } @@ -435,14 +446,14 @@ func (r *ServiceInstanceReconciler) handleAsyncDelete(ctx context.Context, servi } func (r *ServiceInstanceReconciler) getInstanceForRecovery(ctx context.Context, smClient sm.Client, serviceInstance *servicesv1.ServiceInstance) (*smClientTypes.ServiceInstance, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) parameters := sm.Parameters{ FieldQuery: []string{ fmt.Sprintf("name eq '%s'", serviceInstance.Spec.ExternalName), fmt.Sprintf("context/clusterid eq '%s'", r.Config.ClusterID), fmt.Sprintf("context/namespace eq '%s'", serviceInstance.Namespace)}, LabelQuery: []string{ - fmt.Sprintf("%s eq '%s'", k8sNameLabel, serviceInstance.Name)}, + fmt.Sprintf("%s eq '%s'", common.K8sNameLabel, serviceInstance.Name)}, GeneralParams: []string{"attach_last_operations=true"}, } @@ -460,7 +471,7 @@ func (r *ServiceInstanceReconciler) getInstanceForRecovery(ctx context.Context, } func (r *ServiceInstanceReconciler) recover(ctx context.Context, smClient sm.Client, k8sInstance *servicesv1.ServiceInstance, smInstance *smClientTypes.ServiceInstance) (ctrl.Result, error) { - log := GetLogger(ctx) + log := utils.GetLogger(ctx) log.Info(fmt.Sprintf("found existing instance in SM with id %s, updating status", smInstance.ID)) updateHashedSpecValue(k8sInstance) @@ -476,7 +487,7 @@ func (r *ServiceInstanceReconciler) recover(ctx context.Context, smClient sm.Cli k8sInstance.Status.Ready = metav1.ConditionTrue } if smInstance.Shared { - setSharedCondition(k8sInstance, metav1.ConditionTrue, ShareSucceeded, "Instance shared successfully") + setSharedCondition(k8sInstance, metav1.ConditionTrue, common.ShareSucceeded, "Instance shared successfully") } k8sInstance.Status.InstanceID = smInstance.ID k8sInstance.Status.OperationURL = "" @@ -506,46 +517,46 @@ func (r *ServiceInstanceReconciler) recover(ctx context.Context, smClient sm.Cli case smClientTypes.INPROGRESS: k8sInstance.Status.OperationURL = sm.BuildOperationURL(smInstance.LastOperation.ID, smInstance.ID, smClientTypes.ServiceInstancesURL) k8sInstance.Status.OperationType = smInstance.LastOperation.Type - setInProgressConditions(ctx, smInstance.LastOperation.Type, smInstance.LastOperation.Description, k8sInstance) + utils.SetInProgressConditions(ctx, smInstance.LastOperation.Type, smInstance.LastOperation.Description, k8sInstance) case smClientTypes.SUCCEEDED: - setSuccessConditions(operationType, k8sInstance) + utils.SetSuccessConditions(operationType, k8sInstance) case smClientTypes.FAILED: - setFailureConditions(operationType, description, k8sInstance) + utils.SetFailureConditions(operationType, description, k8sInstance) } - return ctrl.Result{}, r.updateStatus(ctx, k8sInstance) + return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, k8sInstance) } -func (r *ServiceInstanceReconciler) handleInstanceSharingError(ctx context.Context, object api.SAPBTPResource, status metav1.ConditionStatus, reason string, err error) (ctrl.Result, error) { - log := GetLogger(ctx) +func (r *ServiceInstanceReconciler) handleInstanceSharingError(ctx context.Context, object common.SAPBTPResource, status metav1.ConditionStatus, reason string, err error) (ctrl.Result, error) { + log := utils.GetLogger(ctx) errMsg := err.Error() isTransient := false if smError, ok := err.(*sm.ServiceManagerError); ok { log.Info(fmt.Sprintf("SM returned error with status code %d", smError.StatusCode)) - isTransient = r.isTransientError(smError, log) + isTransient = utils.IsTransientError(smError, log) errMsg = smError.Error() if smError.StatusCode == http.StatusTooManyRequests { errMsg = "in progress" - reason = InProgress - } else if reason == ShareFailed && + reason = common.InProgress + } else if reason == common.ShareFailed && (smError.StatusCode == http.StatusBadRequest || smError.StatusCode == http.StatusInternalServerError) { /* non-transient error may occur only when sharing SM return 400 when plan is not sharable SM returns 500 when TOGGLES_ENABLE_INSTANCE_SHARE_FROM_OPERATOR feature toggle is off */ - reason = ShareNotSupported + reason = common.ShareNotSupported } } setSharedCondition(object, status, reason, errMsg) - return ctrl.Result{Requeue: isTransient}, r.updateStatus(ctx, object) + return ctrl.Result{Requeue: isTransient}, utils.UpdateStatus(ctx, r.Client, object) } func isFinalState(ctx context.Context, serviceInstance *servicesv1.ServiceInstance) bool { - log := GetLogger(ctx) - if isMarkedForDeletion(serviceInstance.ObjectMeta) { + log := utils.GetLogger(ctx) + if utils.IsMarkedForDeletion(serviceInstance.ObjectMeta) { log.Info("instance is not in final state, it is marked for deletion") return false } @@ -559,7 +570,7 @@ func isFinalState(ctx context.Context, serviceInstance *servicesv1.ServiceInstan } // succeeded=false for current generation, and without failed=true --> transient error retry - if isInProgress(serviceInstance) { + if utils.IsInProgress(serviceInstance) { log.Info("instance is not in final state, sync operation is in progress") return false } @@ -582,8 +593,8 @@ func updateRequired(serviceInstance *servicesv1.ServiceInstance) bool { return false } - cond := meta.FindStatusCondition(serviceInstance.Status.Conditions, api.ConditionSucceeded) - if cond != nil && cond.Reason == UpdateInProgress { //in case of transient error occurred + cond := meta.FindStatusCondition(serviceInstance.Status.Conditions, common.ConditionSucceeded) + if cond != nil && cond.Reason == common.UpdateInProgress { //in case of transient error occurred return true } @@ -596,18 +607,18 @@ func sharingUpdateRequired(serviceInstance *servicesv1.ServiceInstance) bool { return false } - sharedCondition := meta.FindStatusCondition(serviceInstance.GetConditions(), api.ConditionShared) + sharedCondition := meta.FindStatusCondition(serviceInstance.GetConditions(), common.ConditionShared) shouldBeShared := serviceInstance.ShouldBeShared() if sharedCondition == nil { return shouldBeShared } - if sharedCondition.Reason == ShareNotSupported { + if sharedCondition.Reason == common.ShareNotSupported { return false } - if sharedCondition.Reason == InProgress || sharedCondition.Reason == ShareFailed || sharedCondition.Reason == UnShareFailed { + if sharedCondition.Reason == common.InProgress || sharedCondition.Reason == common.ShareFailed || sharedCondition.Reason == common.UnShareFailed { return true } @@ -671,18 +682,18 @@ func generateEncodedMD5Hash(str string) string { return hex.EncodeToString(hash[:]) } -func setSharedCondition(object api.SAPBTPResource, status metav1.ConditionStatus, reason, msg string) { +func setSharedCondition(object common.SAPBTPResource, status metav1.ConditionStatus, reason, msg string) { conditions := object.GetConditions() // align all conditions to latest generation for _, cond := range object.GetConditions() { - if cond.Type != api.ConditionShared { + if cond.Type != common.ConditionShared { cond.ObservedGeneration = object.GetGeneration() meta.SetStatusCondition(&conditions, cond) } } shareCondition := metav1.Condition{ - Type: api.ConditionShared, + Type: common.ConditionShared, Status: status, Reason: reason, Message: msg, @@ -690,7 +701,7 @@ func setSharedCondition(object api.SAPBTPResource, status metav1.ConditionStatus } // remove shared condition and add it as new (in case it has observed generation) - meta.RemoveStatusCondition(&conditions, api.ConditionShared) + meta.RemoveStatusCondition(&conditions, common.ConditionShared) meta.SetStatusCondition(&conditions, shareCondition) object.SetConditions(conditions) @@ -721,15 +732,15 @@ func getErrorMsgFromLastOperation(status *smClientTypes.Operation) string { func markNonTransientStatusAsTransient(serviceInstance *servicesv1.ServiceInstance) { conditions := serviceInstance.GetConditions() - lastOpCondition := meta.FindStatusCondition(conditions, api.ConditionSucceeded) + lastOpCondition := meta.FindStatusCondition(conditions, common.ConditionSucceeded) operation := smClientTypes.CREATE if len(serviceInstance.Status.InstanceID) > 0 { operation = smClientTypes.UPDATE } - lastOpCondition.Reason = getConditionReason(operation, smClientTypes.INPROGRESS) + lastOpCondition.Reason = utils.GetConditionReason(operation, smClientTypes.INPROGRESS) if len(conditions) > 0 { - meta.RemoveStatusCondition(&conditions, api.ConditionFailed) + meta.RemoveStatusCondition(&conditions, common.ConditionFailed) } meta.SetStatusCondition(&conditions, *lastOpCondition) serviceInstance.SetConditions(conditions) diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index d38b8762..9068aa0a 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -3,12 +3,13 @@ package controllers import ( "context" "fmt" - "github.com/SAP/sap-btp-service-operator/api" + "github.com/SAP/sap-btp-service-operator/api/common" v1 "github.com/SAP/sap-btp-service-operator/api/v1" "github.com/SAP/sap-btp-service-operator/client/sm" "github.com/SAP/sap-btp-service-operator/client/sm/smfakes" smClientTypes "github.com/SAP/sap-btp-service-operator/client/sm/types" smclientTypes "github.com/SAP/sap-btp-service-operator/client/sm/types" + "github.com/SAP/sap-btp-service-operator/internal/utils" "github.com/google/uuid" . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" @@ -126,7 +127,7 @@ var _ = Describe("ServiceInstance controller", func() { BeforeEach(func() { ctx = context.Background() log := ctrl.Log.WithName("instanceTest") - ctx = context.WithValue(ctx, LogKey{}, log) + ctx = context.WithValue(ctx, utils.LogKey{}, log) fakeInstanceName = "ic-test-" + uuid.New().String() defaultLookupKey = types.NamespacedName{Name: fakeInstanceName, Namespace: testNamespace} @@ -202,7 +203,7 @@ var _ = Describe("ServiceInstance controller", func() { It("provisioning should fail", func() { serviceInstance = createInstance(ctx, instanceSpec, nil, false) - waitForInstanceConditionAndMessage(ctx, defaultLookupKey, api.ConditionSucceeded, "provided plan id does not match") + waitForInstanceConditionAndMessage(ctx, defaultLookupKey, common.ConditionSucceeded, "provided plan id does not match") }) }) }) @@ -240,7 +241,7 @@ var _ = Describe("ServiceInstance controller", func() { It("should have failure condition", func() { serviceInstance = createInstance(ctx, instanceSpec, nil, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionFailed, metav1.ConditionTrue, CreateFailed, errMessage) + waitForResourceCondition(ctx, serviceInstance, common.ConditionFailed, metav1.ConditionTrue, common.CreateFailed, errMessage) }) }) @@ -252,9 +253,9 @@ var _ = Describe("ServiceInstance controller", func() { Description: errMessage, }) fakeClient.ProvisionReturnsOnCall(1, &sm.ProvisionResponse{InstanceID: fakeInstanceID}, nil) - serviceInstance = createInstance(ctx, instanceSpec, map[string]string{api.IgnoreNonTransientErrorAnnotation: "true"}, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionTrue, Created, "") - waitForResourceAnnotationRemove(ctx, serviceInstance, api.IgnoreNonTransientErrorAnnotation, api.IgnoreNonTransientErrorTimestampAnnotation) + serviceInstance = createInstance(ctx, instanceSpec, map[string]string{common.IgnoreNonTransientErrorAnnotation: "true"}, false) + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionTrue, common.Created, "") + waitForResourceAnnotationRemove(ctx, serviceInstance, common.IgnoreNonTransientErrorAnnotation, common.IgnoreNonTransientErrorTimestampAnnotation) }) }) @@ -264,10 +265,10 @@ var _ = Describe("ServiceInstance controller", func() { StatusCode: http.StatusBadRequest, Description: errMessage, }) - serviceInstance = createInstance(ctx, instanceSpec, map[string]string{api.IgnoreNonTransientErrorAnnotation: "true"}, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionFailed, metav1.ConditionTrue, CreateFailed, errMessage) - waitForResourceAnnotationRemove(ctx, serviceInstance, api.IgnoreNonTransientErrorAnnotation, api.IgnoreNonTransientErrorTimestampAnnotation) - waitForResourceCondition(ctx, serviceInstance, api.ConditionFailed, metav1.ConditionTrue, CreateFailed, errMessage) + serviceInstance = createInstance(ctx, instanceSpec, map[string]string{common.IgnoreNonTransientErrorAnnotation: "true"}, false) + waitForResourceCondition(ctx, serviceInstance, common.ConditionFailed, metav1.ConditionTrue, common.CreateFailed, errMessage) + waitForResourceAnnotationRemove(ctx, serviceInstance, common.IgnoreNonTransientErrorAnnotation, common.IgnoreNonTransientErrorTimestampAnnotation) + waitForResourceCondition(ctx, serviceInstance, common.ConditionFailed, metav1.ConditionTrue, common.CreateFailed, errMessage) sinceCreate := time.Since(serviceInstance.GetCreationTimestamp().Time) Expect(sinceCreate > ignoreNonTransientTimeout) }) @@ -285,7 +286,7 @@ var _ = Describe("ServiceInstance controller", func() { It("should retry until success", func() { serviceInstance = createInstance(ctx, instanceSpec, nil, true) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionTrue, Created, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionTrue, common.Created, "") }) }) @@ -297,7 +298,7 @@ var _ = Describe("ServiceInstance controller", func() { It("should be transient error and eventually succeed", func() { serviceInstance = createInstance(ctx, instanceSpec, nil, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionFalse, CreateInProgress, tooManyRequestsError) + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateInProgress, tooManyRequestsError) fakeClient.ProvisionReturns(&sm.ProvisionResponse{InstanceID: fakeInstanceID}, nil) waitForResourceToBeReady(ctx, serviceInstance) }) @@ -311,14 +312,14 @@ var _ = Describe("ServiceInstance controller", func() { It("should have failure condition - non transient error", func() { serviceInstance = createInstance(ctx, instanceSpec, nil, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionFailed, metav1.ConditionTrue, CreateFailed, errMessage) + waitForResourceCondition(ctx, serviceInstance, common.ConditionFailed, metav1.ConditionTrue, common.CreateFailed, errMessage) }) When("ignoreNonTransientErrorAnnotation exists", func() { It("should have failure conditions and remove the annotation after timeout", func() { - serviceInstance = createInstance(ctx, instanceSpec, map[string]string{api.IgnoreNonTransientErrorAnnotation: "true"}, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionTrue, Created, "") - waitForResourceAnnotationRemove(ctx, serviceInstance, api.IgnoreNonTransientErrorAnnotation, api.IgnoreNonTransientErrorTimestampAnnotation) + serviceInstance = createInstance(ctx, instanceSpec, map[string]string{common.IgnoreNonTransientErrorAnnotation: "true"}, false) + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionTrue, common.Created, "") + waitForResourceAnnotationRemove(ctx, serviceInstance, common.IgnoreNonTransientErrorAnnotation, common.IgnoreNonTransientErrorTimestampAnnotation) Expect(fakeClient.ProvisionCallCount()).To(BeNumerically(">", 1)) }) }) @@ -347,7 +348,7 @@ var _ = Describe("ServiceInstance controller", func() { Type: smClientTypes.CREATE, State: smClientTypes.SUCCEEDED, }, nil) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionTrue, Created, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionTrue, common.Created, "") Expect(serviceInstance.Status.SubaccountID).To(Equal(fakeSubaccountID)) }) }) @@ -361,14 +362,14 @@ var _ = Describe("ServiceInstance controller", func() { State: smClientTypes.FAILED, Errors: []byte(`{"error": "brokerError","description":"broker-failure"}`), }, nil) - waitForInstanceConditionAndMessage(ctx, defaultLookupKey, api.ConditionFailed, "broker-failure") + waitForInstanceConditionAndMessage(ctx, defaultLookupKey, common.ConditionFailed, "broker-failure") }) }) When("updating during create", func() { It("should update the instance after created successfully", func() { serviceInstance = createInstance(ctx, instanceSpec, nil, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionFalse, CreateInProgress, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateInProgress, "") newName := "new-name" + uuid.New().String() Eventually(func() bool { @@ -387,7 +388,7 @@ var _ = Describe("ServiceInstance controller", func() { Type: smClientTypes.CREATE, State: smClientTypes.SUCCEEDED, }, nil) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionTrue, Updated, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionTrue, common.Updated, "") Expect(fakeClient.UpdateInstanceCallCount()).To(BeNumerically(">", 0)) Expect(fakeClient.ProvisionCallCount()).To(BeNumerically(">", 0)) }) @@ -398,7 +399,7 @@ var _ = Describe("ServiceInstance controller", func() { serviceInstance = createInstance(ctx, instanceSpec, nil, false) By("waiting for instance to be CreateInProgress") - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionFalse, CreateInProgress, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateInProgress, "") fakeClient.DeprovisionReturns("/v1/service_instances/id/operations/1234", nil) fakeClient.StatusReturns(&smclientTypes.Operation{ @@ -408,7 +409,7 @@ var _ = Describe("ServiceInstance controller", func() { }, nil) deleteInstance(ctx, serviceInstance, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionFalse, DeleteInProgress, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.DeleteInProgress, "") fakeClient.StatusReturns(&smclientTypes.Operation{ ID: "1234", @@ -453,7 +454,7 @@ var _ = Describe("ServiceInstance controller", func() { serviceInstance = updateInstance(ctx, serviceInstance) Expect(serviceInstance.Spec.ExternalName).To(Equal(newExternalName)) Expect(serviceInstance.Spec.UserInfo).NotTo(BeNil()) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionTrue, Updated, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionTrue, common.Updated, "") }) }) }) @@ -473,7 +474,7 @@ var _ = Describe("ServiceInstance controller", func() { newExternalName := "my-new-external-name" + uuid.New().String() serviceInstance.Spec.ExternalName = newExternalName updateInstance(ctx, serviceInstance) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionFalse, UpdateInProgress, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.UpdateInProgress, "") fakeClient.StatusReturns(&smclientTypes.Operation{ ID: "1234", Type: smClientTypes.UPDATE, @@ -537,7 +538,7 @@ var _ = Describe("ServiceInstance controller", func() { newExternalName := "my-new-external-name" + uuid.New().String() serviceInstance.Spec.ExternalName = newExternalName updateInstance(ctx, serviceInstance) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionFalse, UpdateFailed, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.UpdateFailed, "") }) }) }) @@ -553,7 +554,7 @@ var _ = Describe("ServiceInstance controller", func() { newExternalName := "my-new-external-name" + uuid.New().String() serviceInstance.Spec.ExternalName = newExternalName updateInstance(ctx, serviceInstance) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionFalse, UpdateInProgress, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.UpdateInProgress, "") fakeClient.UpdateInstanceReturns(nil, "", nil) updateInstance(ctx, serviceInstance) waitForResourceToBeReady(ctx, serviceInstance) @@ -569,14 +570,14 @@ var _ = Describe("ServiceInstance controller", func() { newExternalName := "my-new-external-name" + uuid.New().String() serviceInstance.Spec.ExternalName = newExternalName updateInstance(ctx, serviceInstance) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionFalse, UpdateFailed, "") - serviceInstance.Annotations = map[string]string{api.IgnoreNonTransientErrorAnnotation: "true"} + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.UpdateFailed, "") + serviceInstance.Annotations = map[string]string{common.IgnoreNonTransientErrorAnnotation: "true"} updateInstance(ctx, serviceInstance) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionFalse, UpdateInProgress, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.UpdateInProgress, "") fakeClient.UpdateInstanceReturns(nil, "", nil) waitForResourceToBeReady(ctx, serviceInstance) - waitForResourceAnnotationRemove(ctx, serviceInstance, api.IgnoreNonTransientErrorAnnotation, api.IgnoreNonTransientErrorTimestampAnnotation) + waitForResourceAnnotationRemove(ctx, serviceInstance, common.IgnoreNonTransientErrorAnnotation, common.IgnoreNonTransientErrorTimestampAnnotation) }) }) @@ -597,7 +598,7 @@ var _ = Describe("ServiceInstance controller", func() { newExternalName := "my-new-external-name" + uuid.New().String() serviceInstance.Spec.ExternalName = newExternalName updateInstance(ctx, serviceInstance) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionFalse, UpdateInProgress, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.UpdateInProgress, "") fakeClient.StatusReturns(&smclientTypes.Operation{ ID: "1234", Type: smClientTypes.UPDATE, @@ -663,7 +664,7 @@ var _ = Describe("ServiceInstance controller", func() { }) It("should fail deleting the instance because of the webhook delete validation", func() { serviceInstance.Annotations = map[string]string{ - api.PreventDeletion: "true", + common.PreventDeletion: "true", } updateInstance(ctx, serviceInstance) err := k8sClient.Delete(ctx, serviceInstance) @@ -701,7 +702,7 @@ var _ = Describe("ServiceInstance controller", func() { errMsg := "failed to delete instance" fakeClient.DeprovisionReturns("", fmt.Errorf(errMsg)) deleteInstance(ctx, serviceInstance, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionFailed, metav1.ConditionTrue, DeleteFailed, errMsg) + waitForResourceCondition(ctx, serviceInstance, common.ConditionFailed, metav1.ConditionTrue, common.DeleteFailed, errMsg) }) }) }) @@ -715,7 +716,7 @@ var _ = Describe("ServiceInstance controller", func() { State: smClientTypes.INPROGRESS, }, nil) deleteInstance(ctx, serviceInstance, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionFalse, DeleteInProgress, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.DeleteInProgress, "") }) When("polling ends with success", func() { BeforeEach(func() { @@ -742,7 +743,7 @@ var _ = Describe("ServiceInstance controller", func() { It("should not delete the k8s instance and condition is updated with failure", func() { deleteInstance(ctx, serviceInstance, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionFailed, metav1.ConditionTrue, DeleteFailed, "broker-failure") + waitForResourceCondition(ctx, serviceInstance, common.ConditionFailed, metav1.ConditionTrue, common.DeleteFailed, "broker-failure") }) }) }) @@ -804,8 +805,8 @@ var _ = Describe("ServiceInstance controller", func() { if err != nil { return false } - cond := meta.FindStatusCondition(serviceInstance.GetConditions(), api.ConditionSucceeded) - return serviceInstance.Status.HashedSpec == hashed && cond != nil && cond.Reason == Created + cond := meta.FindStatusCondition(serviceInstance.GetConditions(), common.ConditionSucceeded) + return serviceInstance.Status.HashedSpec == hashed && cond != nil && cond.Reason == common.Created }, timeout, interval).Should(BeTrue()) }) }) @@ -868,7 +869,7 @@ var _ = Describe("ServiceInstance controller", func() { It("should recover the existing instance and update condition failure", func() { serviceInstance = createInstance(ctx, instanceSpec, nil, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionFalse, CreateFailed, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateFailed, "") Expect(serviceInstance.Status.InstanceID).To(Equal(fakeInstanceID)) Expect(fakeClient.ProvisionCallCount()).To(Equal(0)) }) @@ -897,7 +898,7 @@ var _ = Describe("ServiceInstance controller", func() { }) It("should recover the instance with status Ready=false", func() { serviceInstance = createInstance(ctx, instanceSpec, nil, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionFailed, metav1.ConditionTrue, CreateFailed, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionFailed, metav1.ConditionTrue, common.CreateFailed, "") Expect(fakeClient.ProvisionCallCount()).To(Equal(0)) Expect(serviceInstance.Status.InstanceID).To(Equal(fakeInstanceID)) }) @@ -962,7 +963,7 @@ var _ = Describe("ServiceInstance controller", func() { Description: "errMessage", }) serviceInstance = createInstance(ctx, sharedInstanceSpec, nil, false) - waitForResourceCondition(ctx, serviceInstance, api.ConditionFailed, metav1.ConditionTrue, CreateFailed, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionFailed, metav1.ConditionTrue, common.CreateFailed, "") Expect(fakeClient.ShareInstanceCallCount()).To(BeZero()) }) }) @@ -980,7 +981,7 @@ var _ = Describe("ServiceInstance controller", func() { }) serviceInstance.Spec.Shared = pointer.Bool(true) updateInstance(ctx, serviceInstance) - waitForResourceCondition(ctx, serviceInstance, api.ConditionShared, metav1.ConditionFalse, InProgress, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionShared, metav1.ConditionFalse, common.InProgress, "") fakeClient.ShareInstanceReturns(nil) waitForInstanceToBeShared(ctx, serviceInstance) }) @@ -994,7 +995,7 @@ var _ = Describe("ServiceInstance controller", func() { }) serviceInstance.Spec.Shared = pointer.Bool(true) updateInstance(ctx, serviceInstance) - waitForResourceCondition(ctx, serviceInstance, api.ConditionShared, metav1.ConditionFalse, ShareFailed, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionShared, metav1.ConditionFalse, common.ShareFailed, "") fakeClient.ShareInstanceReturns(nil) waitForInstanceToBeShared(ctx, serviceInstance) @@ -1009,7 +1010,7 @@ var _ = Describe("ServiceInstance controller", func() { }) serviceInstance.Spec.Shared = pointer.Bool(true) updateInstance(ctx, serviceInstance) - waitForResourceCondition(ctx, serviceInstance, api.ConditionShared, metav1.ConditionFalse, ShareNotSupported, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionShared, metav1.ConditionFalse, common.ShareNotSupported, "") }) }) @@ -1021,7 +1022,7 @@ var _ = Describe("ServiceInstance controller", func() { }) serviceInstance.Spec.Shared = pointer.Bool(true) updateInstance(ctx, serviceInstance) - waitForResourceCondition(ctx, serviceInstance, api.ConditionShared, metav1.ConditionFalse, ShareNotSupported, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionShared, metav1.ConditionFalse, common.ShareNotSupported, "") }) }) }) @@ -1050,7 +1051,7 @@ var _ = Describe("ServiceInstance controller", func() { updateInstance(ctx, serviceInstance) Eventually(func() bool { _ = k8sClient.Get(ctx, defaultLookupKey, serviceInstance) - return meta.FindStatusCondition(serviceInstance.GetConditions(), api.ConditionShared) == nil + return meta.FindStatusCondition(serviceInstance.GetConditions(), common.ConditionShared) == nil }, timeout, interval).Should(BeTrue()) Expect(len(serviceInstance.Status.Conditions)).To(Equal(2)) }) @@ -1081,7 +1082,7 @@ var _ = Describe("ServiceInstance controller", func() { It("should have a reason un-shared failed", func() { serviceInstance.Spec.Shared = pointer.Bool(false) updateInstance(ctx, serviceInstance) - waitForResourceCondition(ctx, serviceInstance, api.ConditionShared, metav1.ConditionTrue, UnShareFailed, "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionShared, metav1.ConditionTrue, common.UnShareFailed, "") }) }) }) @@ -1095,12 +1096,12 @@ var _ = Describe("ServiceInstance controller", func() { Status: v1.ServiceInstanceStatus{ Conditions: []metav1.Condition{ { - Type: api.ConditionReady, + Type: common.ConditionReady, Status: metav1.ConditionTrue, ObservedGeneration: 0, }, { - Type: api.ConditionSucceeded, + Type: common.ConditionSucceeded, Status: metav1.ConditionTrue, ObservedGeneration: 1, }, @@ -1120,12 +1121,12 @@ var _ = Describe("ServiceInstance controller", func() { Status: v1.ServiceInstanceStatus{ Conditions: []metav1.Condition{ { - Type: api.ConditionReady, + Type: common.ConditionReady, Status: metav1.ConditionTrue, ObservedGeneration: 0, }, { - Type: api.ConditionSucceeded, + Type: common.ConditionSucceeded, Status: metav1.ConditionFalse, ObservedGeneration: 1, }, @@ -1148,7 +1149,7 @@ var _ = Describe("ServiceInstance controller", func() { Status: v1.ServiceInstanceStatus{ Conditions: []metav1.Condition{ { - Type: api.ConditionReady, + Type: common.ConditionReady, Status: metav1.ConditionTrue, ObservedGeneration: 1, }, @@ -1171,12 +1172,12 @@ var _ = Describe("ServiceInstance controller", func() { Status: v1.ServiceInstanceStatus{ Conditions: []metav1.Condition{ { - Type: api.ConditionReady, + Type: common.ConditionReady, Status: metav1.ConditionTrue, ObservedGeneration: 1, }, { - Type: api.ConditionSucceeded, + Type: common.ConditionSucceeded, Status: metav1.ConditionFalse, ObservedGeneration: 2, }, @@ -1197,12 +1198,12 @@ var _ = Describe("ServiceInstance controller", func() { Status: v1.ServiceInstanceStatus{ Conditions: []metav1.Condition{ { - Type: api.ConditionReady, + Type: common.ConditionReady, Status: metav1.ConditionTrue, ObservedGeneration: 1, }, { - Type: api.ConditionSucceeded, + Type: common.ConditionSucceeded, Status: metav1.ConditionTrue, ObservedGeneration: 2, }, @@ -1223,17 +1224,17 @@ var _ = Describe("ServiceInstance controller", func() { Status: v1.ServiceInstanceStatus{ Conditions: []metav1.Condition{ { - Type: api.ConditionReady, + Type: common.ConditionReady, Status: metav1.ConditionTrue, ObservedGeneration: 1, }, { - Type: api.ConditionSucceeded, + Type: common.ConditionSucceeded, Status: metav1.ConditionTrue, ObservedGeneration: 2, }, { - Type: api.ConditionShared, + Type: common.ConditionShared, Status: metav1.ConditionFalse, }, }, @@ -1255,17 +1256,17 @@ var _ = Describe("ServiceInstance controller", func() { Status: v1.ServiceInstanceStatus{ Conditions: []metav1.Condition{ { - Type: api.ConditionReady, + Type: common.ConditionReady, Status: metav1.ConditionTrue, ObservedGeneration: 1, }, { - Type: api.ConditionSucceeded, + Type: common.ConditionSucceeded, Status: metav1.ConditionTrue, ObservedGeneration: 2, }, { - Type: api.ConditionShared, + Type: common.ConditionShared, Status: metav1.ConditionTrue, }, }, @@ -1296,12 +1297,12 @@ func waitForInstanceConditionAndMessage(ctx context.Context, key types.Namespace } func waitForInstanceToBeShared(ctx context.Context, serviceInstance *v1.ServiceInstance) { - waitForResourceCondition(ctx, serviceInstance, api.ConditionShared, metav1.ConditionTrue, "", "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionShared, metav1.ConditionTrue, "", "") Expect(len(serviceInstance.Status.Conditions)).To(Equal(3)) } func waitForInstanceToBeUnShared(ctx context.Context, serviceInstance *v1.ServiceInstance) { - waitForResourceCondition(ctx, serviceInstance, api.ConditionShared, metav1.ConditionFalse, "", "") + waitForResourceCondition(ctx, serviceInstance, common.ConditionShared, metav1.ConditionFalse, "", "") Expect(len(serviceInstance.Status.Conditions)).To(Equal(3)) } diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 3cfdddcc..8523c6be 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -21,6 +21,8 @@ import ( "context" "crypto/tls" "fmt" + "github.com/SAP/sap-btp-service-operator/api/common" + "github.com/SAP/sap-btp-service-operator/internal/utils" "net" "net/http" "path/filepath" @@ -33,7 +35,6 @@ import ( ginkgo_config "github.com/onsi/ginkgo/config" . "github.com/onsi/gomega" - "github.com/SAP/sap-btp-service-operator/api" v1 "github.com/SAP/sap-btp-service-operator/api/v1" "github.com/SAP/sap-btp-service-operator/api/v1/webhooks" "github.com/SAP/sap-btp-service-operator/client/sm" @@ -143,26 +144,26 @@ var _ = BeforeSuite(func(done Done) { By("registering controllers") err = (&ServiceInstanceReconciler{ - BaseReconciler: &BaseReconciler{ - Client: k8sManager.GetClient(), - Scheme: k8sManager.GetScheme(), - Log: ctrl.Log.WithName("controllers").WithName("ServiceInstance"), - SMClient: func() sm.Client { return fakeClient }, - Config: testConfig, - Recorder: k8sManager.GetEventRecorderFor("ServiceInstance"), + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + Log: ctrl.Log.WithName("controllers").WithName("ServiceInstance"), + GetSMClient: func(_ context.Context, _ *utils.SecretResolver, _, _ string) (sm.Client, error) { + return fakeClient, nil }, + Config: testConfig, + Recorder: k8sManager.GetEventRecorderFor("ServiceInstance"), }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) err = (&ServiceBindingReconciler{ - BaseReconciler: &BaseReconciler{ - Client: k8sManager.GetClient(), - Scheme: k8sManager.GetScheme(), - Log: ctrl.Log.WithName("controllers").WithName("ServiceBinding"), - SMClient: func() sm.Client { return fakeClient }, - Config: testConfig, - Recorder: k8sManager.GetEventRecorderFor("ServiceBinding"), + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + Log: ctrl.Log.WithName("controllers").WithName("ServiceBinding"), + GetSMClient: func(_ context.Context, _ *utils.SecretResolver, _, _ string) (sm.Client, error) { + return fakeClient, nil }, + Config: testConfig, + Recorder: k8sManager.GetEventRecorderFor("ServiceBinding"), }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) @@ -214,16 +215,16 @@ var _ = AfterSuite(func() { printSection("Finished AfterSuite") }) -func isResourceReady(resource api.SAPBTPResource) bool { +func isResourceReady(resource common.SAPBTPResource) bool { return resource.GetObservedGeneration() == resource.GetGeneration() && - meta.IsStatusConditionPresentAndEqual(resource.GetConditions(), api.ConditionReady, metav1.ConditionTrue) + meta.IsStatusConditionPresentAndEqual(resource.GetConditions(), common.ConditionReady, metav1.ConditionTrue) } -func waitForResourceToBeReady(ctx context.Context, resource api.SAPBTPResource) { - waitForResourceCondition(ctx, resource, api.ConditionReady, metav1.ConditionTrue, "", "") +func waitForResourceToBeReady(ctx context.Context, resource common.SAPBTPResource) { + waitForResourceCondition(ctx, resource, common.ConditionReady, metav1.ConditionTrue, "", "") } -func waitForResourceCondition(ctx context.Context, resource api.SAPBTPResource, conditionType string, status metav1.ConditionStatus, reason, message string) { +func waitForResourceCondition(ctx context.Context, resource common.SAPBTPResource, conditionType string, status metav1.ConditionStatus, reason, message string) { key := getResourceNamespacedName(resource) Eventually(func() bool { if err := k8sClient.Get(ctx, key, resource); err != nil { @@ -259,7 +260,7 @@ func waitForResourceCondition(ctx context.Context, resource api.SAPBTPResource, resource), ) } -func waitForResourceAnnotationRemove(ctx context.Context, resource api.SAPBTPResource, annotationsKey ...string) { +func waitForResourceAnnotationRemove(ctx context.Context, resource common.SAPBTPResource, annotationsKey ...string) { key := getResourceNamespacedName(resource) Eventually(func() bool { if err := k8sClient.Get(ctx, key, resource); err != nil { @@ -347,7 +348,7 @@ func getNonTransientBrokerError(errMessage string) error { return &sm.ServiceManagerError{ StatusCode: http.StatusBadRequest, Description: "smErrMessage", - BrokerError: &api.HTTPStatusCodeError{ + BrokerError: &common.HTTPStatusCodeError{ StatusCode: 400, ErrorMessage: &errMessage, }} @@ -357,7 +358,7 @@ func getTransientBrokerError(errorMessage string) error { return &sm.ServiceManagerError{ StatusCode: http.StatusBadGateway, Description: "smErrMessage", - BrokerError: &api.HTTPStatusCodeError{ + BrokerError: &common.HTTPStatusCodeError{ StatusCode: http.StatusTooManyRequests, ErrorMessage: &errorMessage, }, diff --git a/internal/config/config.go b/internal/config/config.go index ca2f1ee7..359d1256 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -22,8 +22,8 @@ type Config struct { AllowedNamespaces []string `envconfig:"allowed_namespaces"` EnableNamespaceSecrets bool `envconfig:"enable_namespace_secrets"` ClusterID string `envconfig:"cluster_id"` - RetryBaseDelay time.Duration `envconfig:"retry_base_delay"` - RetryMaxDelay time.Duration `envconfig:"retry_max_delay"` + RetryBaseDelay time.Duration + RetryMaxDelay time.Duration IgnoreNonTransientTimeout time.Duration } diff --git a/internal/utils/condition_utils.go b/internal/utils/condition_utils.go new file mode 100644 index 00000000..ae43ec31 --- /dev/null +++ b/internal/utils/condition_utils.go @@ -0,0 +1,244 @@ +package utils + +import ( + "context" + "fmt" + "net/http" + + "github.com/SAP/sap-btp-service-operator/api/common" + "github.com/SAP/sap-btp-service-operator/client/sm" + smClientTypes "github.com/SAP/sap-btp-service-operator/client/sm/types" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func InitConditions(ctx context.Context, k8sClient client.Client, obj common.SAPBTPResource) error { + obj.SetReady(metav1.ConditionFalse) + SetInProgressConditions(ctx, smClientTypes.CREATE, "Pending", obj) + return UpdateStatus(ctx, k8sClient, obj) +} + +func GetConditionReason(opType smClientTypes.OperationCategory, state smClientTypes.OperationState) string { + switch state { + case smClientTypes.SUCCEEDED: + if opType == smClientTypes.CREATE { + return common.Created + } else if opType == smClientTypes.UPDATE { + return common.Updated + } else if opType == smClientTypes.DELETE { + return common.Deleted + } else { + return common.Finished + } + case smClientTypes.INPROGRESS, smClientTypes.PENDING: + if opType == smClientTypes.CREATE { + return common.CreateInProgress + } else if opType == smClientTypes.UPDATE { + return common.UpdateInProgress + } else if opType == smClientTypes.DELETE { + return common.DeleteInProgress + } else { + return common.InProgress + } + case smClientTypes.FAILED: + if opType == smClientTypes.CREATE { + return common.CreateFailed + } else if opType == smClientTypes.UPDATE { + return common.UpdateFailed + } else if opType == smClientTypes.DELETE { + return common.DeleteFailed + } else { + return common.Failed + } + } + + return common.Unknown +} + +func SetInProgressConditions(ctx context.Context, operationType smClientTypes.OperationCategory, message string, object common.SAPBTPResource) { + log := GetLogger(ctx) + if len(message) == 0 { + if operationType == smClientTypes.CREATE { + message = fmt.Sprintf("%s is being created", object.GetControllerName()) + } else if operationType == smClientTypes.UPDATE { + message = fmt.Sprintf("%s is being updated", object.GetControllerName()) + } else if operationType == smClientTypes.DELETE { + message = fmt.Sprintf("%s is being deleted", object.GetControllerName()) + } + } + + conditions := object.GetConditions() + if len(conditions) > 0 { + meta.RemoveStatusCondition(&conditions, common.ConditionFailed) + } + lastOpCondition := metav1.Condition{ + Type: common.ConditionSucceeded, + Status: metav1.ConditionFalse, + Reason: GetConditionReason(operationType, smClientTypes.INPROGRESS), + Message: message, + ObservedGeneration: object.GetObservedGeneration(), + } + meta.SetStatusCondition(&conditions, lastOpCondition) + meta.SetStatusCondition(&conditions, getReadyCondition(object)) + + object.SetConditions(conditions) + log.Info(fmt.Sprintf("setting inProgress conditions: reason: %s, message:%s, generation: %d", lastOpCondition.Reason, message, object.GetGeneration())) +} + +func SetSuccessConditions(operationType smClientTypes.OperationCategory, object common.SAPBTPResource) { + var message string + if operationType == smClientTypes.CREATE { + message = fmt.Sprintf("%s provisioned successfully", object.GetControllerName()) + } else if operationType == smClientTypes.UPDATE { + message = fmt.Sprintf("%s updated successfully", object.GetControllerName()) + } else if operationType == smClientTypes.DELETE { + message = fmt.Sprintf("%s deleted successfully", object.GetControllerName()) + } + + conditions := object.GetConditions() + if len(conditions) > 0 { + meta.RemoveStatusCondition(&conditions, common.ConditionFailed) + } + lastOpCondition := metav1.Condition{ + Type: common.ConditionSucceeded, + Status: metav1.ConditionTrue, + Reason: GetConditionReason(operationType, smClientTypes.SUCCEEDED), + Message: message, + ObservedGeneration: object.GetObservedGeneration(), + } + readyCondition := metav1.Condition{ + Type: common.ConditionReady, + Status: metav1.ConditionTrue, + Reason: common.Provisioned, + Message: message, + ObservedGeneration: object.GetObservedGeneration(), + } + meta.SetStatusCondition(&conditions, lastOpCondition) + meta.SetStatusCondition(&conditions, readyCondition) + + object.SetConditions(conditions) + object.SetReady(metav1.ConditionTrue) +} + +func SetCredRotationInProgressConditions(reason, message string, object common.SAPBTPResource) { + if len(message) == 0 { + message = reason + } + conditions := object.GetConditions() + credRotCondition := metav1.Condition{ + Type: common.ConditionCredRotationInProgress, + Status: metav1.ConditionTrue, + Reason: reason, + Message: message, + ObservedGeneration: object.GetObservedGeneration(), + } + meta.SetStatusCondition(&conditions, credRotCondition) + object.SetConditions(conditions) +} + +func SetFailureConditions(operationType smClientTypes.OperationCategory, errorMessage string, object common.SAPBTPResource) { + var message string + if operationType == smClientTypes.CREATE { + message = fmt.Sprintf("%s create failed: %s", object.GetControllerName(), errorMessage) + } else if operationType == smClientTypes.UPDATE { + message = fmt.Sprintf("%s update failed: %s", object.GetControllerName(), errorMessage) + } else if operationType == smClientTypes.DELETE { + message = fmt.Sprintf("%s deletion failed: %s", object.GetControllerName(), errorMessage) + } + + var reason string + if operationType != common.Unknown { + reason = GetConditionReason(operationType, smClientTypes.FAILED) + } else { + reason = object.GetConditions()[0].Reason + } + + conditions := object.GetConditions() + lastOpCondition := metav1.Condition{ + Type: common.ConditionSucceeded, + Status: metav1.ConditionFalse, + Reason: reason, + Message: message, + ObservedGeneration: object.GetObservedGeneration(), + } + meta.SetStatusCondition(&conditions, lastOpCondition) + + failedCondition := metav1.Condition{ + Type: common.ConditionFailed, + Status: metav1.ConditionTrue, + Reason: reason, + Message: message, + ObservedGeneration: object.GetObservedGeneration(), + } + meta.SetStatusCondition(&conditions, failedCondition) + meta.SetStatusCondition(&conditions, getReadyCondition(object)) + + object.SetConditions(conditions) +} + +func MarkAsNonTransientError(ctx context.Context, k8sClient client.Client, operationType smClientTypes.OperationCategory, errMsg string, object common.SAPBTPResource) (ctrl.Result, error) { + log := GetLogger(ctx) + SetFailureConditions(operationType, errMsg, object) + if operationType != smClientTypes.DELETE { + log.Info(fmt.Sprintf("operation %s of %s encountered a non transient error %s, giving up operation :(", operationType, object.GetControllerName(), errMsg)) + } + object.SetObservedGeneration(object.GetGeneration()) + err := UpdateStatus(ctx, k8sClient, object) + if err != nil { + return ctrl.Result{}, err + } + if operationType == smClientTypes.DELETE { + return ctrl.Result{}, fmt.Errorf(errMsg) + } + return ctrl.Result{}, nil +} + +func MarkAsTransientError(ctx context.Context, k8sClient client.Client, operationType smClientTypes.OperationCategory, err error, object common.SAPBTPResource) (ctrl.Result, error) { + log := GetLogger(ctx) + //DO NOT REMOVE - 429 error is not reflected to the status + if smError, ok := err.(*sm.ServiceManagerError); !ok || smError.StatusCode != http.StatusTooManyRequests { + SetInProgressConditions(ctx, operationType, err.Error(), object) + log.Info(fmt.Sprintf("operation %s of %s encountered a transient error %s, retrying operation :)", operationType, object.GetControllerName(), err.Error())) + if updateErr := UpdateStatus(ctx, k8sClient, object); updateErr != nil { + return ctrl.Result{}, updateErr + } + } + + return ctrl.Result{}, err +} + +// blocked condition marks to the user that action from his side is required, this is considered as in progress operation +func SetBlockedCondition(ctx context.Context, message string, object common.SAPBTPResource) { + SetInProgressConditions(ctx, common.Unknown, message, object) + lastOpCondition := meta.FindStatusCondition(object.GetConditions(), common.ConditionSucceeded) + lastOpCondition.Reason = common.Blocked +} + +func IsInProgress(object common.SAPBTPResource) bool { + conditions := object.GetConditions() + return meta.IsStatusConditionPresentAndEqual(conditions, common.ConditionSucceeded, metav1.ConditionFalse) && + !meta.IsStatusConditionPresentAndEqual(conditions, common.ConditionFailed, metav1.ConditionTrue) +} + +func IsFailed(resource common.SAPBTPResource) bool { + if len(resource.GetConditions()) == 0 { + return false + } + return meta.IsStatusConditionPresentAndEqual(resource.GetConditions(), common.ConditionFailed, metav1.ConditionTrue) || + (resource.GetConditions()[0].Status == metav1.ConditionFalse && + resource.GetConditions()[0].Type == common.ConditionSucceeded && + resource.GetConditions()[0].Reason == common.Blocked) +} + +func getReadyCondition(object common.SAPBTPResource) metav1.Condition { + status := metav1.ConditionFalse + reason := common.NotProvisioned + if object.GetReady() == metav1.ConditionTrue { + status = metav1.ConditionTrue + reason = common.Provisioned + } + + return metav1.Condition{Type: common.ConditionReady, Status: status, Reason: reason} +} diff --git a/internal/utils/condition_utils_test.go b/internal/utils/condition_utils_test.go new file mode 100644 index 00000000..30f5469c --- /dev/null +++ b/internal/utils/condition_utils_test.go @@ -0,0 +1,339 @@ +package utils + +import ( + "github.com/SAP/sap-btp-service-operator/api/common" + v1 "github.com/SAP/sap-btp-service-operator/api/v1" + "github.com/SAP/sap-btp-service-operator/client/sm" + smClientTypes "github.com/SAP/sap-btp-service-operator/client/sm/types" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "net/http" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Condition Utils", func() { + var resource *v1.ServiceBinding + BeforeEach(func() { + resource = getBinding() + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + }) + AfterEach(func() { + err := k8sClient.Delete(ctx, resource) + Expect(client.IgnoreNotFound(err)).ToNot(HaveOccurred()) + }) + Context("InitConditions", func() { + It("should initialize conditions and update status", func() { + err := InitConditions(ctx, k8sClient, resource) + Expect(err).ToNot(HaveOccurred()) + Expect(meta.IsStatusConditionPresentAndEqual(resource.GetConditions(), common.ConditionReady, metav1.ConditionFalse)).To(BeTrue()) + }) + }) + + Context("GetConditionReason", func() { + When("given operation type CREATE and state SUCCEEDED", func() { + It("returns expected condition reason", func() { + Expect(GetConditionReason(smClientTypes.CREATE, smClientTypes.SUCCEEDED)).To(Equal(common.Created)) + }) + }) + + When("given operation type UPDATE and state SUCCEEDED", func() { + It("returns expected condition reason", func() { + Expect(GetConditionReason(smClientTypes.UPDATE, smClientTypes.SUCCEEDED)).To(Equal(common.Updated)) + }) + }) + + When("given operation type DELETE and state SUCCEEDED", func() { + It("returns expected condition reason", func() { + expected := common.Deleted + Expect(GetConditionReason(smClientTypes.DELETE, smClientTypes.SUCCEEDED)).To(Equal(expected)) + }) + }) + + When("given operation type CREATE and state INPROGRESS", func() { + It("returns expected condition reason", func() { + Expect(GetConditionReason(smClientTypes.CREATE, smClientTypes.INPROGRESS)).To(Equal(common.CreateInProgress)) + }) + }) + + When("given operation type UPDATE and state INPROGRESS", func() { + It("returns expected condition reason", func() { + Expect(GetConditionReason(smClientTypes.UPDATE, smClientTypes.INPROGRESS)).To(Equal(common.UpdateInProgress)) + }) + }) + + When("given operation type DELETE and state INPROGRESS", func() { + It("returns expected condition reason", func() { + Expect(GetConditionReason(smClientTypes.DELETE, smClientTypes.INPROGRESS)).To(Equal(common.DeleteInProgress)) + }) + }) + + When("given operation type CREATE and state FAILED", func() { + It("returns expected condition reason", func() { + Expect(GetConditionReason(smClientTypes.CREATE, smClientTypes.FAILED)).To(Equal(common.CreateFailed)) + }) + }) + + When("given operation type UPDATE and state FAILED", func() { + It("returns expected condition reason", func() { + Expect(GetConditionReason(smClientTypes.UPDATE, smClientTypes.FAILED)).To(Equal(common.UpdateFailed)) + }) + }) + + When("given operation type DELETE and state FAILED", func() { + It("returns expected condition reason", func() { + Expect(GetConditionReason(smClientTypes.DELETE, smClientTypes.FAILED)).To(Equal(common.DeleteFailed)) + }) + }) + + When("given an unknown operation type and state SUCCEEDED", func() { + It("returns finished condition reason", func() { + Expect(GetConditionReason("unknown", smClientTypes.SUCCEEDED)).To(Equal(common.Finished)) + }) + }) + + When("given an unknown operation type and state INPROGRESS", func() { + It("returns in progress condition reason", func() { + Expect(GetConditionReason("unknown", smClientTypes.INPROGRESS)).To(Equal(common.InProgress)) + }) + }) + + When("given an unknown operation type and state FAILED", func() { + It("returns failed condition reason", func() { + Expect(GetConditionReason("unknown", smClientTypes.FAILED)).To(Equal(common.Failed)) + }) + }) + + When("given operation type CREATE and unknown state", func() { + It("returns unknown condition reason", func() { + Expect(GetConditionReason(smClientTypes.CREATE, "unknown")).To(Equal(common.Unknown)) + }) + }) + }) + + Context("SetInProgressConditions", func() { + It("should set in-progress conditions", func() { + resource = getBinding() + + SetInProgressConditions(ctx, smClientTypes.CREATE, "Pending", resource) + + // Add assertions to check the state of the resource after calling SetInProgressConditions + Expect(resource.GetConditions()).ToNot(BeEmpty()) + // Add more assertions based on your expected behavior + }) + }) + + Context("SetSuccessConditions", func() { + It("should set success conditions", func() { + operationType := smClientTypes.CREATE + resource = getBinding() + + SetSuccessConditions(operationType, resource) + + // Add assertions to check the state of the resource after calling SetSuccessConditions + Expect(resource.GetConditions()).ToNot(BeEmpty()) + Expect(resource.GetReady()).To(Equal(metav1.ConditionTrue)) + // Add more assertions based on your expected behavior + }) + }) + + Context("SetCredRotationInProgressConditions", func() { + It("should set credentials rotation in-progress conditions", func() { + reason := "RotationReason" + message := "RotationMessage" + resource = getBinding() + + SetCredRotationInProgressConditions(reason, message, resource) + + // Add assertions to check the state of the resource after calling SetCredRotationInProgressConditions + Expect(resource.GetConditions()).ToNot(BeEmpty()) + // Add more assertions based on your expected behavior + }) + }) + + Context("SetFailureConditions", func() { + It("should set failure conditions", func() { + operationType := smClientTypes.CREATE + errorMessage := "Operation failed" + SetFailureConditions(operationType, errorMessage, resource) + Expect(resource.GetConditions()).ToNot(BeEmpty()) + Expect(meta.IsStatusConditionPresentAndEqual(resource.GetConditions(), common.ConditionReady, metav1.ConditionFalse)).To(BeTrue()) + }) + }) + + Context("MarkAsNonTransientError", func() { + It("should mark as non-transient error and update status", func() { + operationType := smClientTypes.CREATE + errorMessage := "Non-transient error" + + result, err := MarkAsNonTransientError(ctx, k8sClient, operationType, errorMessage, resource) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + }) + + Context("MarkAsTransientError", func() { + It("should handle TooManyRequests error correctly", func() { + resource.SetConditions([]metav1.Condition{{Message: "not TooManyRequests"}}) + serviceManagerError := &sm.ServiceManagerError{StatusCode: http.StatusTooManyRequests} + result, err := MarkAsTransientError(ctx, k8sClient, smClientTypes.UPDATE, serviceManagerError, resource) + Expect(err).ToNot(BeNil()) + Expect(resource.GetConditions()[0].Message).To(ContainSubstring("not TooManyRequests")) //TooManyRequests is not reflected to status + Expect(result).To(BeEquivalentTo(ctrl.Result{})) + }) + }) + + Context("SetBlockedCondition", func() { + It("Blocked Condition Set on ServiceBinding", func() { + sb := &v1.ServiceBinding{ + Status: v1.ServiceBindingStatus{ + Conditions: []metav1.Condition{}, + }, + } + + SetBlockedCondition(ctx, "Test message", sb) + Expect(meta.FindStatusCondition(sb.Status.Conditions, common.ConditionSucceeded).Reason).To(Equal(common.Blocked)) + }) + }) + + Context("IsInProgress", func() { + It("should return true for in progress condition", func() { + resource := &v1.ServiceBinding{ + Status: v1.ServiceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: common.ConditionSucceeded, + Status: metav1.ConditionFalse, + }, + { + Type: common.ConditionFailed, + Status: metav1.ConditionFalse, + }, + }, + }, + } + + Expect(IsInProgress(resource)).To(BeTrue()) + }) + + It("should return false for failed condition", func() { + resource := &v1.ServiceBinding{ + Status: v1.ServiceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: common.ConditionSucceeded, + Status: metav1.ConditionFalse, + }, + { + Type: common.ConditionFailed, + Status: metav1.ConditionTrue, + }, + }, + }, + } + + Expect(IsInProgress(resource)).To(BeFalse()) + }) + + It("should return false for succeeded condition", func() { + resource := &v1.ServiceBinding{ + Status: v1.ServiceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: common.ConditionSucceeded, + Status: metav1.ConditionTrue, + }, + { + Type: common.ConditionFailed, + Status: metav1.ConditionFalse, + }, + }, + }, + } + + Expect(IsInProgress(resource)).To(BeFalse()) + }) + + It("should return false for empty conditions", func() { + resource := &v1.ServiceBinding{ + Status: v1.ServiceBindingStatus{ + Conditions: []metav1.Condition{}, + }, + } + + Expect(IsInProgress(resource)).To(BeFalse()) + }) + }) + + Context("IsFailed", func() { + It("Should return false when no conditions available", func() { + sb := &v1.ServiceBinding{Status: v1.ServiceBindingStatus{Conditions: []metav1.Condition{}}} + result := IsFailed(sb) + Expect(result).Should(BeFalse()) + }) + + It("Should return true when ConditionFailed is true", func() { + sb := &v1.ServiceBinding{Status: v1.ServiceBindingStatus{Conditions: []metav1.Condition{{Type: common.ConditionFailed, Status: metav1.ConditionTrue}}}} + result := IsFailed(sb) + Expect(result).Should(BeTrue()) + }) + + It("Should return false when ConditionFailed is false", func() { + sb := &v1.ServiceBinding{Status: v1.ServiceBindingStatus{Conditions: []metav1.Condition{{Type: common.ConditionFailed, Status: metav1.ConditionFalse}}}} + result := IsFailed(sb) + Expect(result).Should(BeFalse()) + }) + + It("Should return true when ConditionSucceeded is false and reason is Blocked", func() { + sb := &v1.ServiceBinding{ + Status: v1.ServiceBindingStatus{Conditions: []metav1.Condition{{Type: common.ConditionSucceeded, Status: metav1.ConditionFalse, Reason: common.Blocked}}}, + } + result := IsFailed(sb) + Expect(result).Should(BeTrue()) + }) + + It("Should return false when ConditionSucceeded is true and reason is Blocked", func() { + sb := &v1.ServiceBinding{ + Status: v1.ServiceBindingStatus{Conditions: []metav1.Condition{{Type: common.ConditionSucceeded, Status: metav1.ConditionTrue, Reason: common.Blocked}}}, + } + result := IsFailed(sb) + Expect(result).Should(BeFalse()) + }) + }) +}) + +func getBinding() *v1.ServiceBinding { + return &v1.ServiceBinding{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "services.cloud.sap.com/v1", + Kind: "ServiceBinding", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "service-binding-1", + Namespace: testNamespace, + }, + Spec: v1.ServiceBindingSpec{ + ServiceInstanceName: "service-instance-1", + ExternalName: "my-service-binding-1", + Parameters: &runtime.RawExtension{Raw: []byte(`{"key":"val"}`)}, + ParametersFrom: []v1.ParametersFromSource{ + { + SecretKeyRef: &v1.SecretKeyReference{ + Name: "param-secret", + Key: "secret-parameter", + }, + }, + }, + CredRotationPolicy: &v1.CredentialsRotationPolicy{ + Enabled: true, + RotationFrequency: "1s", + RotatedBindingTTL: "1s", + }, + }, + + Status: v1.ServiceBindingStatus{}, + } +} diff --git a/internal/utils/controller_util.go b/internal/utils/controller_util.go new file mode 100644 index 00000000..6dbc4f10 --- /dev/null +++ b/internal/utils/controller_util.go @@ -0,0 +1,222 @@ +package utils + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "strings" + "time" + + "github.com/SAP/sap-btp-service-operator/api/common" + "github.com/SAP/sap-btp-service-operator/client/sm" + smClientTypes "github.com/SAP/sap-btp-service-operator/client/sm/types" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apimachinerytypes "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + "github.com/go-logr/logr" + + "k8s.io/apimachinery/pkg/util/rand" + + v1 "k8s.io/api/authentication/v1" +) + +const ( + TEXT format = "text" + JSON format = "json" + UNKNOWN format = "unknown" +) + +type SecretMetadataProperty struct { + Name string `json:"name"` + Container bool `json:"container,omitempty"` + Format string `json:"format"` +} + +type format string + +type LogKey struct { +} + +func RemoveFinalizer(ctx context.Context, k8sClient client.Client, object common.SAPBTPResource, finalizerName string) error { + log := GetLogger(ctx) + if controllerutil.ContainsFinalizer(object, finalizerName) { + log.Info(fmt.Sprintf("removing finalizer %s", finalizerName)) + controllerutil.RemoveFinalizer(object, finalizerName) + if err := k8sClient.Update(ctx, object); err != nil { + if err := k8sClient.Get(ctx, apimachinerytypes.NamespacedName{Name: object.GetName(), Namespace: object.GetNamespace()}, object); err != nil { + return client.IgnoreNotFound(err) + } + controllerutil.RemoveFinalizer(object, finalizerName) + if err := k8sClient.Update(ctx, object); err != nil { + return fmt.Errorf("failed to remove the finalizer '%s'. Error: %v", finalizerName, err) + } + } + log.Info(fmt.Sprintf("removed finalizer %s from %s", finalizerName, object.GetControllerName())) + return nil + } + return nil +} + +func UpdateStatus(ctx context.Context, k8sClient client.Client, object common.SAPBTPResource) error { + log := GetLogger(ctx) + log.Info(fmt.Sprintf("updating %s status", object.GetObjectKind().GroupVersionKind().Kind)) + return k8sClient.Status().Update(ctx, object) +} + +func ShouldIgnoreNonTransient(log logr.Logger, resource common.SAPBTPResource, timeout time.Duration) bool { + annotations := resource.GetAnnotations() + if len(annotations) == 0 || len(annotations[common.IgnoreNonTransientErrorAnnotation]) == 0 { + return false + } + + // we ignore the error + // for service instances, the value is validated in webhook + // for service bindings, the annotation is not allowed + annotationTime, _ := time.Parse(time.RFC3339, annotations[common.IgnoreNonTransientErrorTimestampAnnotation]) + sinceAnnotation := time.Since(annotationTime) + if sinceAnnotation > timeout { + log.Info(fmt.Sprintf("timeout of %s reached - error is considered to be non transient. time passed since annotation timestamp %s", timeout, sinceAnnotation)) + return false + } + log.Info(fmt.Sprintf("timeout of %s was not reached - error is considered to be transient. ime passed since annotation timestamp %s", timeout, sinceAnnotation)) + return true +} + +func NormalizeCredentials(credentialsJSON json.RawMessage) (map[string][]byte, []SecretMetadataProperty, error) { + var credentialsMap map[string]interface{} + err := json.Unmarshal(credentialsJSON, &credentialsMap) + if err != nil { + return nil, nil, err + } + + normalized := make(map[string][]byte) + metadata := make([]SecretMetadataProperty, 0) + for propertyName, value := range credentialsMap { + keyString := strings.Replace(propertyName, " ", "_", -1) + normalizedValue, typpe, err := serialize(value) + if err != nil { + return nil, nil, err + } + metadata = append(metadata, SecretMetadataProperty{ + Name: keyString, + Format: string(typpe), + }) + normalized[keyString] = normalizedValue + } + return normalized, metadata, nil +} + +func BuildUserInfo(ctx context.Context, userInfo *v1.UserInfo) string { + log := GetLogger(ctx) + if userInfo == nil { + return "" + } + userInfoStr, err := json.Marshal(userInfo) + if err != nil { + log.Error(err, "failed to prepare user info") + return "" + } + + return string(userInfoStr) +} + +func SliceContains(slice []string, i string) bool { + for _, s := range slice { + if s == i { + return true + } + } + + return false +} + +func RandStringRunes(n int) string { + var letterRunes = []rune("abcdefghijklmnopqrstuvwxyz1234567890") + b := make([]rune, n) + for i := range b { + b[i] = letterRunes[rand.Intn(len(letterRunes))] + } + return string(b) +} + +func GetLogger(ctx context.Context) logr.Logger { + return ctx.Value(LogKey{}).(logr.Logger) +} + +func HandleError(ctx context.Context, k8sClient client.Client, operationType smClientTypes.OperationCategory, err error, resource common.SAPBTPResource, ignoreNonTransient bool) (ctrl.Result, error) { + log := GetLogger(ctx) + var smError *sm.ServiceManagerError + ok := errors.As(err, &smError) + if !ok { + log.Info("unable to cast error to SM error, will be treated as non transient") + return MarkAsNonTransientError(ctx, k8sClient, operationType, err.Error(), resource) + } + if ignoreNonTransient || IsTransientError(smError, log) { + return MarkAsTransientError(ctx, k8sClient, operationType, smError, resource) + } + + return MarkAsNonTransientError(ctx, k8sClient, operationType, smError.Error(), resource) +} + +func IsTransientError(smError *sm.ServiceManagerError, log logr.Logger) bool { + statusCode := smError.GetStatusCode() + log.Info(fmt.Sprintf("SM returned error with status code %d", statusCode)) + return isTransientStatusCode(statusCode) || isConcurrentOperationError(smError) +} + +func IsMarkedForDeletion(object metav1.ObjectMeta) bool { + return !object.DeletionTimestamp.IsZero() +} + +func RemoveAnnotations(ctx context.Context, k8sClient client.Client, object common.SAPBTPResource, keys ...string) error { + log := GetLogger(ctx) + annotations := object.GetAnnotations() + shouldUpdate := false + if annotations != nil { + for _, key := range keys { + if _, ok := annotations[key]; ok { + log.Info(fmt.Sprintf("deleting annotation with key %s", key)) + delete(annotations, key) + shouldUpdate = true + } + } + if shouldUpdate { + object.SetAnnotations(annotations) + return k8sClient.Update(ctx, object) + } + } + return nil +} + +func isConcurrentOperationError(smError *sm.ServiceManagerError) bool { + // service manager returns 422 for resources that have another operation in progress + // in this case 422 status code is transient + return smError.StatusCode == http.StatusUnprocessableEntity && smError.ErrorType == "ConcurrentOperationInProgress" +} + +func isTransientStatusCode(StatusCode int) bool { + return StatusCode == http.StatusTooManyRequests || + StatusCode == http.StatusServiceUnavailable || + StatusCode == http.StatusGatewayTimeout || + StatusCode == http.StatusBadGateway || + StatusCode == http.StatusNotFound +} + +func serialize(value interface{}) ([]byte, format, error) { + if byteArrayVal, ok := value.([]byte); ok { + return byteArrayVal, JSON, nil + } + if strVal, ok := value.(string); ok { + return []byte(strVal), TEXT, nil + } + data, err := json.Marshal(value) + if err != nil { + return nil, UNKNOWN, err + } + return data, JSON, nil +} diff --git a/internal/utils/controller_util_test.go b/internal/utils/controller_util_test.go new file mode 100644 index 00000000..bcc7b4a3 --- /dev/null +++ b/internal/utils/controller_util_test.go @@ -0,0 +1,218 @@ +package utils + +import ( + "encoding/json" + "github.com/SAP/sap-btp-service-operator/api/common" + v1 "github.com/SAP/sap-btp-service-operator/api/v1" + "github.com/SAP/sap-btp-service-operator/client/sm" + "github.com/go-logr/logr" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + authv1 "k8s.io/api/authentication/v1" + "net/http" + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "time" +) + +var _ = Describe("Controller Util", func() { + + Context("normalize credentials", func() { + var credentialsJSON json.RawMessage + + BeforeEach(func() { + credentialsJSON = []byte(`{"keyStr":"val", "keyBool":true,"keyNum":8,"keyJSON":{"a":"b"}}`) + }) + + It("should normalize correctly", func() { + res, metadata, err := NormalizeCredentials(credentialsJSON) + str := SecretMetadataProperty{ + Name: "keyStr", + Format: string(TEXT), + } + boolean := SecretMetadataProperty{ + Name: "keyBool", + Format: string(JSON), + } + num := SecretMetadataProperty{ + Name: "keyNum", + Format: string(JSON), + } + json := SecretMetadataProperty{ + Name: "keyJSON", + Format: string(JSON), + } + Expect(err).ToNot(HaveOccurred()) + Expect(len(res)).To(Equal(4)) + Expect(metadata).To(ContainElements(str, boolean, num, json)) + }) + + }) + + Context("ShouldIgnoreNonTransient", func() { + var ( + instance *v1.ServiceInstance + logger = logf.Log.WithName("test-logger") + ) + + BeforeEach(func() { + instance = &v1.ServiceInstance{} + }) + + It("should return false if no ignore annotation", func() { + instance.SetAnnotations(nil) + Expect(ShouldIgnoreNonTransient(logger, instance, time.Hour)).To(BeFalse()) + }) + + It("should return false if time exceeded", func() { + annotation := map[string]string{ + common.IgnoreNonTransientErrorAnnotation: "true", + common.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Truncate(48 * time.Hour).Format(time.RFC3339), + } + instance.SetAnnotations(annotation) + Expect(ShouldIgnoreNonTransient(logger, instance, time.Hour)).To(BeFalse()) + }) + + It("should return true if time not exceeded", func() { + annotation := map[string]string{ + common.IgnoreNonTransientErrorAnnotation: "true", + common.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Format(time.RFC3339), + } + instance.SetAnnotations(annotation) + Expect(ShouldIgnoreNonTransient(logger, instance, time.Hour)).To(BeTrue()) + }) + }) + + Context("SliceContains", func() { + It("slice contains", func() { + slice := []string{"element1", "element2", "element3"} + Expect(SliceContains(slice, "element2")).To(BeTrue()) + }) + + It("slice doesn't contain", func() { + slice := []string{"element1", "element2", "element3"} + Expect(SliceContains(slice, "element4")).To(BeFalse()) + }) + + It("empty slice", func() { + slice := []string{} + Expect(SliceContains(slice, "element1")).To(BeFalse()) + }) + }) + + Context("IsTransientError", func() { + var instance *sm.ServiceManagerError + var log logr.Logger + BeforeEach(func() { + log = GetLogger(ctx) + }) + When("400 status code", func() { + BeforeEach(func() { + instance = &sm.ServiceManagerError{ + StatusCode: 400, + } + }) + + It("should not be transient error", func() { + Expect(IsTransientError(instance, log)).To(BeFalse()) + }) + }) + + When("internal server error status code", func() { + BeforeEach(func() { + instance = &sm.ServiceManagerError{ + StatusCode: 500, + } + }) + + It("should be non transient error", func() { + Expect(IsTransientError(instance, log)).To(BeFalse()) + }) + }) + + When("concurrent operation error", func() { + BeforeEach(func() { + instance = &sm.ServiceManagerError{ + StatusCode: http.StatusUnprocessableEntity, + ErrorType: "ConcurrentOperationInProgress", + } + }) + + It("should be transient error", func() { + Expect(IsTransientError(instance, log)).To(BeTrue()) + }) + }) + }) + + Context("RemoveAnnotations tests", func() { + var resource *v1.ServiceBinding + BeforeEach(func() { + resource = getBinding() + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + }) + AfterEach(func() { + err := k8sClient.Delete(ctx, resource) + Expect(client.IgnoreNotFound(err)).ToNot(HaveOccurred()) + }) + When("a single key is removed", func() { + BeforeEach(func() { + resource.Annotations = map[string]string{"key1": "value1", "key2": "value2"} + }) + + It("should not return an error and remove the annotation", func() { + err := RemoveAnnotations(ctx, k8sClient, resource, "key1") + Expect(err).NotTo(HaveOccurred()) + Expect(resource.GetAnnotations()).To(Equal(map[string]string{"key2": "value2"})) + }) + }) + + When("multiple keys are removed", func() { + BeforeEach(func() { + resource.Annotations = map[string]string{"key1": "value1", "key2": "value2"} + }) + + It("should not return an error and remove annotations", func() { + err := RemoveAnnotations(ctx, k8sClient, resource, "key1", "key2") + Expect(err).NotTo(HaveOccurred()) + Expect(resource.GetAnnotations()).To(BeEmpty()) + }) + }) + + When("non-existent key is removed", func() { + BeforeEach(func() { + resource.Annotations = map[string]string{"key1": "value1", "key2": "value2"} + }) + + It("should not return an error", func() { + err := RemoveAnnotations(ctx, k8sClient, resource, "key3") + Expect(err).NotTo(HaveOccurred()) + Expect(resource.GetAnnotations()).To(Equal(map[string]string{"key1": "value1", "key2": "value2"})) + }) + }) + + When("annotations are empty", func() { + BeforeEach(func() { + resource.Annotations = map[string]string{} + }) + + It("should not return an error", func() { + err := RemoveAnnotations(ctx, k8sClient, resource, "key1") + Expect(err).NotTo(HaveOccurred()) + Expect(resource.GetAnnotations()).To(BeEmpty()) + }) + }) + }) + + Context("build user info test ", func() { + + It("should return empty with nil UserInfo", func() { + Expect(BuildUserInfo(ctx, nil)).To(Equal("")) + }) + + It("should return correct UserInfo string with valid UserInfo", func() { + got := BuildUserInfo(ctx, &authv1.UserInfo{Username: "user1", UID: "1"}) + expected := `{"username":"user1","uid":"1"}` + Expect(got).To(Equal(expected)) + }) + }) +}) diff --git a/controllers/parameters.go b/internal/utils/parameters.go similarity index 94% rename from controllers/parameters.go rename to internal/utils/parameters.go index e90d7244..f5b146df 100644 --- a/controllers/parameters.go +++ b/internal/utils/parameters.go @@ -1,4 +1,4 @@ -package controllers +package utils import ( "context" @@ -22,7 +22,7 @@ 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 buildParameters(kubeClient client.Client, namespace string, parametersFrom []servicesv1.ParametersFromSource, parameters *runtime.RawExtension) (map[string]interface{}, []byte, error) { +func BuildSMRequestParameters(kubeClient client.Client, 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 { @@ -67,25 +67,6 @@ func buildParameters(kubeClient client.Client, namespace string, parametersFrom return params, parametersRaw, nil } -// 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) { - var params map[string]interface{} - if parametersFrom.SecretKeyRef != nil { - data, err := fetchSecretKeyValue(kubeClient, namespace, parametersFrom.SecretKeyRef) - if err != nil { - return nil, err - } - p, err := unmarshalJSON(data) - if err != nil { - return nil, err - } - params = p - - } - return params, nil -} - // UnmarshalRawParameters produces a map structure from a given raw YAML/JSON input func UnmarshalRawParameters(in []byte) (map[string]interface{}, error) { parameters := make(map[string]interface{}) @@ -124,3 +105,22 @@ func fetchSecretKeyValue(kubeClient client.Client, namespace string, secretKeyRe } return secret.Data[secretKeyRef.Key], nil } + +// 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) { + var params map[string]interface{} + if parametersFrom.SecretKeyRef != nil { + data, err := fetchSecretKeyValue(kubeClient, namespace, parametersFrom.SecretKeyRef) + if err != nil { + return nil, err + } + p, err := unmarshalJSON(data) + if err != nil { + return nil, err + } + params = p + + } + return params, nil +} diff --git a/internal/utils/parameters_test.go b/internal/utils/parameters_test.go new file mode 100644 index 00000000..c6dd32af --- /dev/null +++ b/internal/utils/parameters_test.go @@ -0,0 +1,23 @@ +package utils + +import ( + v1 "github.com/SAP/sap-btp-service-operator/api/v1" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/runtime" +) + +var _ = Describe("Parameters", func() { + Describe("BuildSMRequestParameters", func() { + It("handles empty parameters", func() { + parametersFrom := []v1.ParametersFromSource{} + parameters := (*runtime.RawExtension)(nil) + + params, rawParam, err := BuildSMRequestParameters(nil, "", parametersFrom, parameters) + + Expect(err).To(BeNil()) + Expect(params).To(BeNil()) + Expect(rawParam).To(BeNil()) + }) + }) +}) diff --git a/internal/secrets/resolver.go b/internal/utils/secret_resolver.go similarity index 80% rename from internal/secrets/resolver.go rename to internal/utils/secret_resolver.go index e643d89b..f0bf58ee 100644 --- a/internal/secrets/resolver.go +++ b/internal/utils/secret_resolver.go @@ -1,4 +1,4 @@ -package secrets +package utils import ( "context" @@ -26,19 +26,21 @@ type SecretResolver struct { Log logr.Logger } -func (sr *SecretResolver) GetSecretForResource(ctx context.Context, namespace, name, btpAccessSecret string) (*v1.Secret, error) { +func (sr *SecretResolver) GetSecretFromManagementNamespace(ctx context.Context, name string) (*v1.Secret, error) { secretForResource := &v1.Secret{} - if len(btpAccessSecret) > 0 { - sr.Log.Info(fmt.Sprintf("Searching for secret name %s in namespace %s", - btpAccessSecret, sr.ManagementNamespace)) - err := sr.Client.Get(ctx, types.NamespacedName{Name: btpAccessSecret, Namespace: sr.ManagementNamespace}, secretForResource) - if err != nil { - sr.Log.Error(err, fmt.Sprintf("Could not fetch secret named %s", btpAccessSecret)) - return nil, err - } - return secretForResource, nil + 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) + if err != nil { + sr.Log.Error(err, fmt.Sprintf("Could not fetch secret named %s", name)) + return nil, err } + return secretForResource, nil +} + +func (sr *SecretResolver) GetSecretForResource(ctx context.Context, namespace, name string) (*v1.Secret, error) { + secretForResource := &v1.Secret{} // search namespace secret if sr.EnableNamespaceSecrets { diff --git a/internal/secrets/resolver_test.go b/internal/utils/secret_resolver_test.go similarity index 87% rename from internal/secrets/resolver_test.go rename to internal/utils/secret_resolver_test.go index f47af200..feab4a40 100644 --- a/internal/secrets/resolver_test.go +++ b/internal/utils/secret_resolver_test.go @@ -1,15 +1,15 @@ -package secrets_test +package utils import ( "context" - "github.com/SAP/sap-btp-service-operator/internal/secrets" "github.com/google/uuid" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -18,24 +18,19 @@ import ( // +kubebuilder:docs-gen:collapse=Imports -const ( - managementNamespace = "test-management-namespace" - testNamespace = "test-namespace" -) - var _ = Describe("Secrets Resolver", func() { var ctx context.Context - var resolver *secrets.SecretResolver + var resolver *SecretResolver var expectedClientID string var secret *corev1.Secret createSecret := func(namePrefix string, namespace string) *corev1.Secret { var name string if namePrefix == "" { - name = secrets.SAPBTPOperatorSecretName + name = SAPBTPOperatorSecretName } else { - name = fmt.Sprintf("%s-%s", namePrefix, secrets.SAPBTPOperatorSecretName) + name = fmt.Sprintf("%s-%s", namePrefix, SAPBTPOperatorSecretName) } By(fmt.Sprintf("Creating secret with name %s", name)) @@ -68,21 +63,21 @@ var _ = Describe("Secrets Resolver", func() { } validateSecretResolved := func() { - resolvedSecret, err := resolver.GetSecretForResource(ctx, testNamespace, secrets.SAPBTPOperatorSecretName, "") + resolvedSecret, err := resolver.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, secrets.SAPBTPOperatorSecretName, "") + _, err := resolver.GetSecretForResource(ctx, testNamespace, SAPBTPOperatorSecretName) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("not found")) } BeforeEach(func() { ctx = context.Background() - resolver = &secrets.SecretResolver{ + resolver = &SecretResolver{ ManagementNamespace: managementNamespace, ReleaseNamespace: managementNamespace, Log: logf.Log.WithName("SecretResolver"), @@ -185,7 +180,7 @@ var _ = Describe("Secrets Resolver", func() { }) It("should resolve the secret", func() { - resolvedSecret, err := resolver.GetSecretForResource(ctx, testNamespace, secrets.SAPBTPOperatorSecretName, secret.Name) + resolvedSecret, err := resolver.GetSecretFromManagementNamespace(ctx, secret.Name) Expect(err).ToNot(HaveOccurred()) Expect(resolvedSecret).ToNot(BeNil()) Expect(string(resolvedSecret.Data["clientid"])).To(Equal(expectedClientID)) diff --git a/internal/utils/sm_utils.go b/internal/utils/sm_utils.go new file mode 100644 index 00000000..1bbdbc94 --- /dev/null +++ b/internal/utils/sm_utils.go @@ -0,0 +1,81 @@ +package utils + +import ( + "context" + "fmt" + + "github.com/SAP/sap-btp-service-operator/client/sm" + v1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func GetSMClient(ctx context.Context, secretResolver *SecretResolver, resourceNamespace, btpAccessSecretName string) (sm.Client, error) { + log := GetLogger(ctx) + + if len(btpAccessSecretName) > 0 { + return getBTPAccessClient(ctx, secretResolver, btpAccessSecretName) + } + + secret, err := secretResolver.GetSecretForResource(ctx, resourceNamespace, SAPBTPOperatorSecretName) + if err != nil { + return nil, err + } + + clientConfig := &sm.ClientConfig{ + ClientID: string(secret.Data["clientid"]), + ClientSecret: string(secret.Data["clientsecret"]), + URL: string(secret.Data["sm_url"]), + TokenURL: string(secret.Data["tokenurl"]), + TokenURLSuffix: string(secret.Data["tokenurlsuffix"]), + SSLDisabled: false, + } + + if len(clientConfig.ClientID) == 0 || len(clientConfig.URL) == 0 || len(clientConfig.TokenURL) == 0 { + log.Info("credentials secret found but did not contain all the required data") + return nil, fmt.Errorf("invalid Service-Manager credentials, contact your cluster administrator") + } + + if len(clientConfig.ClientSecret) == 0 { + tlsSecret, err := secretResolver.GetSecretForResource(ctx, resourceNamespace, SAPBTPOperatorTLSSecretName) + if client.IgnoreNotFound(err) != nil { + return nil, err + } + + if tlsSecret == nil || len(tlsSecret.Data) == 0 || len(tlsSecret.Data[v1.TLSCertKey]) == 0 || len(tlsSecret.Data[v1.TLSPrivateKeyKey]) == 0 { + log.Info("clientsecret not found in SM credentials, and tls secret is invalid") + return nil, fmt.Errorf("invalid Service-Manager credentials, contact your cluster administrator") + } + + log.Info("found tls configuration") + clientConfig.TLSCertKey = string(tlsSecret.Data[v1.TLSCertKey]) + clientConfig.TLSPrivateKey = string(tlsSecret.Data[v1.TLSPrivateKeyKey]) + } + + return sm.NewClient(ctx, clientConfig, nil) +} + +func getBTPAccessClient(ctx context.Context, secretResolver *SecretResolver, secretName string) (sm.Client, error) { + log := GetLogger(ctx) + secret, err := secretResolver.GetSecretFromManagementNamespace(ctx, secretName) + if err != nil { + return nil, err + } + + clientConfig := &sm.ClientConfig{ + ClientID: string(secret.Data["clientid"]), + ClientSecret: string(secret.Data["clientsecret"]), + URL: string(secret.Data["sm_url"]), + TokenURL: string(secret.Data["tokenurl"]), + TokenURLSuffix: string(secret.Data["tokenurlsuffix"]), + TLSPrivateKey: string(secret.Data[v1.TLSPrivateKeyKey]), + TLSCertKey: string(secret.Data[v1.TLSCertKey]), + SSLDisabled: false, + } + + if !clientConfig.IsValid() { + log.Info("btpAccess secret found but did not contain all the required data") + return nil, fmt.Errorf("invalid Service-Manager credentials, contact your cluster administrator") + } + + return sm.NewClient(ctx, clientConfig, nil) +} diff --git a/internal/utils/sm_utils_test.go b/internal/utils/sm_utils_test.go new file mode 100644 index 00000000..10771ecb --- /dev/null +++ b/internal/utils/sm_utils_test.go @@ -0,0 +1,392 @@ +package utils + +import ( + . "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 + + BeforeEach(func() { + resolver = &SecretResolver{ + ManagementNamespace: managementNamespace, + ReleaseNamespace: managementNamespace, + Log: logf.Log.WithName("SecretResolver"), + Client: k8sClient, + } + }) + + Context("GetSMClient", func() { + + AfterEach(func() { + if secret != nil { + err := k8sClient.Delete(ctx, secret) + Expect(client.IgnoreNotFound(err)).ToNot(HaveOccurred()) + } + + if tlsSecret != nil { + err := k8sClient.Delete(ctx, tlsSecret) + Expect(client.IgnoreNotFound(err)).ToNot(HaveOccurred()) + } + }) + + Context("SAPBTPOperatorSecret", func() { + When("secret is valid", func() { + BeforeEach(func() { + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SAPBTPOperatorSecretName, + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "clientid": []byte("12345"), + "clientsecret": []byte("client-secret"), + "sm_url": []byte("https://some.url"), + "tokenurl": []byte("https://token.url"), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + }) + It("should succeed", func() { + client, err := GetSMClient(ctx, resolver, testNamespace, "") + Expect(err).ToNot(HaveOccurred()) + Expect(client).ToNot(BeNil()) + }) + }) + When("secret is missing client secret and there is no tls secret", func() { + BeforeEach(func() { + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SAPBTPOperatorSecretName, + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "clientid": []byte("12345"), + "clientsecret": []byte(""), + "sm_url": []byte("https://some.url"), + "tokenurl": []byte("https://token.url"), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + }) + It("should return error", func() { + client, err := GetSMClient(ctx, resolver, testNamespace, "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) + Expect(client).To(BeNil()) + }) + }) + When("secret is missing token url", func() { + BeforeEach(func() { + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SAPBTPOperatorSecretName, + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "clientid": []byte("12345"), + "clientsecret": []byte("clientsecret"), + "sm_url": []byte("https://some.url"), + "tokenurl": []byte(""), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + }) + It("should return error", func() { + client, err := GetSMClient(ctx, resolver, testNamespace, "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) + Expect(client).To(BeNil()) + }) + }) + When("secret is missing sm url", func() { + BeforeEach(func() { + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SAPBTPOperatorSecretName, + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "clientid": []byte("12345"), + "clientsecret": []byte("clientsecret"), + "tokenurl": []byte("http://tokenurl"), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + }) + It("should return error", func() { + client, err := GetSMClient(ctx, resolver, testNamespace, "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) + Expect(client).To(BeNil()) + }) + }) + }) + + Context("SAPBTPOperatorTLSSecret", func() { + BeforeEach(func() { + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SAPBTPOperatorSecretName, + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "clientid": []byte("12345"), + "sm_url": []byte("https://some.url"), + "tokenurl": []byte("https://token.url"), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + }) + When("valid", func() { + BeforeEach(func() { + tlsSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SAPBTPOperatorTLSSecretName, + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "tls.key": []byte(tlskey), + "tls.crt": []byte(tlscrt), + }, + } + Expect(k8sClient.Create(ctx, tlsSecret)).To(Succeed()) + }) + It("should succeed", func() { + client, err := GetSMClient(ctx, resolver, 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, "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) + Expect(client).To(BeNil()) + }) + }) + When("tls secret is missing required values", func() { + BeforeEach(func() { + tlsSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: SAPBTPOperatorTLSSecretName, + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "tls.key": []byte("12345key"), + }, + } + Expect(k8sClient.Create(ctx, tlsSecret)).To(Succeed()) + }) + It("should return error", func() { + client, err := GetSMClient(ctx, resolver, testNamespace, "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid Service-Manager credentials, contact your cluster administrator")) + Expect(client).To(BeNil()) + }) + }) + }) + + Context("btpAccessSecret", func() { + Context("client credentials", func() { + When("secret is valid", func() { + BeforeEach(func() { + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-btp-access-secret", + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "clientid": []byte("12345"), + "clientsecret": []byte("client-secret"), + "sm_url": []byte("https://some.url"), + "tokenurl": []byte("https://token.url"), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + }) + It("should succeed", func() { + client, err := GetSMClient(ctx, resolver, testNamespace, "my-btp-access-secret") + Expect(err).ToNot(HaveOccurred()) + Expect(client).ToNot(BeNil()) + }) + }) + + When("secret is missing client secret and there is no tls secret", func() { + BeforeEach(func() { + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-btp-access-secret", + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "clientid": []byte("12345"), + "clientsecret": []byte(""), + "sm_url": []byte("https://some.url"), + "tokenurl": []byte("https://token.url"), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + }) + It("should return error", func() { + client, err := GetSMClient(ctx, resolver, 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()) + }) + }) + When("secret is missing token url", func() { + BeforeEach(func() { + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-btp-access-secret", + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "clientid": []byte("12345"), + "clientsecret": []byte("clientsecret"), + "sm_url": []byte("https://some.url"), + "tokenurl": []byte(""), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + }) + It("should return error", func() { + client, err := GetSMClient(ctx, resolver, 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()) + }) + }) + When("secret is missing sm url", func() { + BeforeEach(func() { + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-btp-access-secret", + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "clientid": []byte("12345"), + "clientsecret": []byte("clientsecret"), + "tokenurl": []byte("http://tokenurl"), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + }) + It("should return error", func() { + client, err := GetSMClient(ctx, resolver, 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()) + }) + }) + }) + + Context("tls credentials", func() { + When("secret is valid", func() { + BeforeEach(func() { + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-btp-access-secret", + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "clientid": []byte("12345"), + "sm_url": []byte("https://some.url"), + "tokenurl": []byte("https://token.url"), + "tls.key": []byte(tlskey), + "tls.crt": []byte(tlscrt), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + }) + It("should succeed", func() { + client, err := GetSMClient(ctx, resolver, testNamespace, "my-btp-access-secret") + Expect(err).ToNot(HaveOccurred()) + Expect(client).ToNot(BeNil()) + }) + }) + + When("tls secret is missing required values", func() { + BeforeEach(func() { + tlsSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-btp-access-secret", + Namespace: managementNamespace, + }, + Data: map[string][]byte{ + "tls.key": []byte("12345key"), + }, + } + Expect(k8sClient.Create(ctx, tlsSecret)).To(Succeed()) + }) + It("should return error", func() { + client, err := GetSMClient(ctx, resolver, 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()) + }) + }) + }) + }) + }) +}) + +const tlscrt = `-----BEGIN CERTIFICATE----- +MIIDjDCCAnSgAwIBAgIJAKqmq2MCqIgGMA0GCSqGSIb3DQEBCwUAMCcxCzAJBgNV +BAYTAlVTMRgwFgYDVQQDDA9FeGFtcGxlLVJvb3QtQ0EwIBcNMjIxMDI1MDkzNTI3 +WhgPMjA1MDExMDcwOTM1MjdaMGcxCzAJBgNVBAYTAlVTMRIwEAYDVQQIDAlZb3Vy +U3RhdGUxETAPBgNVBAcMCFlvdXJDaXR5MR0wGwYDVQQKDBRFeGFtcGxlLUNlcnRp +ZmljYXRlczESMBAGA1UEAwwJbG9jYWxob3N0MIIBIjANBgkqhkiG9w0BAQEFAAOC +AQ8AMIIBCgKCAQEA6pbBoAKsnVVO0e9ihC7AXpMJmW4v8TMEDcuDHVPHn7ZT8aUI +v87yT6Mfy+Qb5/XKohTvnLmQvirf4dnqzDxk/S4//8uu0j5zK7iDqfqPVXlwSGXq +l7uavEnCSRQSp8SbaQaUymEZ7nbjjycfJp/uNLJGFShft4wWyHABAsIYg3FqVjfm +UgacoasTyBzjvogBAsZAd9jpIVUQFvb389IacQKk0p6tJ/r7CWlkscvVV+ToyNTx +0538zBwksEzUnGepEJS9rVHKBVTC7Kz/TltUVxoNIZM7UIrCReJEHkOtpHqGcaHs +6S6FVgId+B4YcoZDoc/RE/XiwOPldSXgWOrnvwIDAQABo3kwdzBBBgNVHSMEOjA4 +oSukKTAnMQswCQYDVQQGEwJVUzEYMBYGA1UEAwwPRXhhbXBsZS1Sb290LUNBggkA +v5wjvJ5SVsIwCQYDVR0TBAIwADALBgNVHQ8EBAMCBPAwGgYDVR0RBBMwEYIJbG9j +YWxob3N0hwR/AAABMA0GCSqGSIb3DQEBCwUAA4IBAQCT60QRqid/IDQCZ1x5LVfN +KltSBT+oogZtEM15yL+at0XshiG+UjA7VuLJXRrLcLWya8dzTRombx52v3gFpTGG +YEKxMNXme3KnbVQOWPO1voTiOM8TmJC+7kdUWwv0ghGvudjKTJ51B7kJvph475IZ +Y2SzAPU3ZKeRkNRDMBTl85Ua6NPDq+5dj9NxNhylyhKwP4qf1SocgoB2NVNe9cVU +HQfkmCLS06+y3lrb9C86+SlMmtEouoWymiKZv9pUkSTDUL/Cpk9AdMBWU93aNN8y +DGtUtVWQd2nofkg+l9Yoonsh/QZENSTIL5OA+HPHOlpeZZ2D3vvJXqpuGVUt+A1K +-----END CERTIFICATE-----` + +const tlskey = `-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDqlsGgAqydVU7R +72KELsBekwmZbi/xMwQNy4MdU8eftlPxpQi/zvJPox/L5Bvn9cqiFO+cuZC+Kt/h +2erMPGT9Lj//y67SPnMruIOp+o9VeXBIZeqXu5q8ScJJFBKnxJtpBpTKYRnuduOP +Jx8mn+40skYVKF+3jBbIcAECwhiDcWpWN+ZSBpyhqxPIHOO+iAECxkB32OkhVRAW +9vfz0hpxAqTSnq0n+vsJaWSxy9VX5OjI1PHTnfzMHCSwTNScZ6kQlL2tUcoFVMLs +rP9OW1RXGg0hkztQisJF4kQeQ62keoZxoezpLoVWAh34HhhyhkOhz9ET9eLA4+V1 +JeBY6ue/AgMBAAECggEANGkuJUOzsQsIKxsilYmkbPzI3kCh8W+GblaTmo/HP8WK +h6hphgEEXgqB5hm2qmJdvUyUJB3JWtNVZa48KRktLuuQXOPy0QIm1RPKRsW2FFCn +Z2Vtviyp63tHLvCPInBokFRqFbUQCBkDyk3hRc3heGCEC+ITUHy58lojv6wBsgvN +Qfy5LxWF1gtgCPLC6JNgnnBj/0tb3u+nVftc27QCBjA5PSU3HJHA9CSbraiguH1Y +M6Y0a+o8RTxxyW5+ffsuzSaxAOxIwwoonE931AArkvkxFgg50emkPOWbUnhZJeDq +9MJRJz7ADLgttI376At2RVC5baRMKa4Z9AL4oM23oQKBgQD5+eEBIjyT6HUzeUyx +7mDH88egcKLQ+1LlJyY6Sthmf4BnIScF5rvCf1HBjxZ4Zi0fEBCmSZeCD2cgrpG1 +t7LzhFGh4kiF7x0k331l4N57meiCJx6NGcIR2GRF3dTUAjmuQk8fn5FOGUG/k7L9 +hZrlKMIJKZwp9aYvsxaiQvHr+QKBgQDwPfPhcs9wIZGGkxd2L9r402RE6EamE/kt +HHUKU3a3yVOIrOnjlAkrN+zo215bNhQ7I+a8umjOUdUrEW02qcppGiu2W6QTCeR4 +RylNfJgGLWRimp7soRiErGbyC+q/gkGrSq5ZclGFyNQwJuFbyljiCyHf3K7NWv1O +N1NiPx+vdwKBgFIKjas2llUg1N5Y8C/xgXf+bUUd0oHuCi3FJIm7KLyzGew++DS6 +nmLeMHHrST+ooSRxvFUnD/+SmJEkWhQevy+m/Le5sX2rlZAVfW1jWQGN6L5WonNC +wevjbj1z6bbPKCkmABvr3d+Y8Hg0vGjyYXzWXKBvNJ6czbcX+tS0TfvZAoGBALor +KCyS7dE1EjK5FbtOhl/AYLlNTkIwxC2DGeewmhT9/K+zX2QuOZS2N+6S4GHKXI8f +2RRzV/haTdicHof3t5UO5MTh6xmd1uCmNImJfb17u4j1zSYOCJP3jacQOQ/C/uSg +cM972VTVNilCV+zrt0kj21JBD2yvkA/mq8U8qW8tAoGBAImuOUDuk6C9VlWZp/5f +LNk67JSJiTqe6bDYB6FcDUWw7j2EnhegjcxE205T1vc4BqNEJ4Ilruz7T0w+T5N9 +VsxDsSwDp033fe6+XBSPEf879UZgcrq7eSqCfk+NGf2rcjcsdD8z8wd3IkqPCtKW +ICwycby2nLYd40HJv2+G3mdR +-----END PRIVATE KEY-----` diff --git a/internal/secrets/suite_test.go b/internal/utils/suite_test.go similarity index 67% rename from internal/secrets/suite_test.go rename to internal/utils/suite_test.go index 57647ac2..aa54a034 100644 --- a/internal/secrets/suite_test.go +++ b/internal/utils/suite_test.go @@ -14,14 +14,17 @@ See the License for the specific language governing permissions and limitations under the License. */ -package secrets_test +package utils import ( "context" + v1 "github.com/SAP/sap-btp-service-operator/api/v1" + "k8s.io/client-go/rest" + "path/filepath" "testing" "time" - v1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -40,29 +43,50 @@ import ( // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. const ( - timeout = time.Second * 10 - interval = time.Millisecond * 250 + timeout = time.Second * 10 + interval = time.Millisecond * 250 + managementNamespace = "test-management-namespace" + testNamespace = "test-namespace" ) -var k8sClient client.Client -var testEnv *envtest.Environment +var ( + cfg *rest.Config + ctx context.Context + k8sClient client.Client + testEnv *envtest.Environment +) func TestAPIs(t *testing.T) { RegisterFailHandler(Fail) - RunSpecs(t, "Secret Resolver Suite") + RunSpecs(t, "Utils Suite") } var _ = BeforeSuite(func(done Done) { + + ctx = context.Background() + log := ctrl.Log.WithName("utils_tests") + ctx = context.WithValue(ctx, LogKey{}, log) logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) By("bootstrapping test environment") - testEnv = &envtest.Environment{} + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{filepath.Join("../..", "config", "crd", "bases")}, + } var err error - cfg, err := testEnv.Start() + cfg, err = testEnv.Start() Expect(err).ToNot(HaveOccurred()) Expect(cfg).ToNot(BeNil()) + err = v1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) + + // +kubebuilder:scaffold:scheme + + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + Expect(err).ToNot(HaveOccurred()) + Expect(k8sClient).ToNot(BeNil()) + // +kubebuilder:scaffold:scheme webhookInstallOptions := &testEnv.WebhookInstallOptions @@ -86,14 +110,14 @@ var _ = BeforeSuite(func(done Done) { }() k8sManager.GetCache().WaitForCacheSync(context.Background()) - k8sClient = k8sManager.GetClient() - Expect(k8sClient).ToNot(BeNil()) + //k8sClient = k8sManager.GetClient() + //Expect(k8sClient).ToNot(BeNil()) - nsSpec := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: testNamespace}} + nsSpec := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: testNamespace}} err = k8sClient.Create(context.Background(), nsSpec) Expect(err).ToNot(HaveOccurred()) - nsSpec = &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: managementNamespace}} + nsSpec = &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: managementNamespace}} err = k8sClient.Create(context.Background(), nsSpec) Expect(err).ToNot(HaveOccurred()) diff --git a/main.go b/main.go index ea50ffd9..922c3e45 100644 --- a/main.go +++ b/main.go @@ -20,6 +20,8 @@ import ( "flag" "os" + "github.com/SAP/sap-btp-service-operator/internal/utils" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "k8s.io/client-go/rest" @@ -32,7 +34,6 @@ import ( "github.com/SAP/sap-btp-service-operator/api/v1/webhooks" "sigs.k8s.io/controller-runtime/pkg/webhook" - "github.com/SAP/sap-btp-service-operator/internal/secrets" logf "sigs.k8s.io/controller-runtime/pkg/log" "github.com/SAP/sap-btp-service-operator/internal/config" @@ -97,7 +98,7 @@ func main() { os.Exit(1) } - secretResolver := &secrets.SecretResolver{ + secretResolver := &utils.SecretResolver{ ManagementNamespace: config.Get().ManagementNamespace, ReleaseNamespace: config.Get().ReleaseNamespace, EnableNamespaceSecrets: config.Get().EnableNamespaceSecrets, @@ -106,27 +107,25 @@ func main() { } if err = (&controllers.ServiceInstanceReconciler{ - BaseReconciler: &controllers.BaseReconciler{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("controllers").WithName("ServiceInstance"), - Scheme: mgr.GetScheme(), - Config: config.Get(), - SecretResolver: secretResolver, - Recorder: mgr.GetEventRecorderFor("ServiceInstance"), - }, + 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, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ServiceInstance") os.Exit(1) } if err = (&controllers.ServiceBindingReconciler{ - BaseReconciler: &controllers.BaseReconciler{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("controllers").WithName("ServiceBinding"), - Scheme: mgr.GetScheme(), - Config: config.Get(), - SecretResolver: secretResolver, - Recorder: mgr.GetEventRecorderFor("ServiceBinding"), - }, + 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, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ServiceBinding") os.Exit(1)