diff --git a/api/common/common.go b/api/common/common.go index cec801e3..708fa219 100644 --- a/api/common/common.go +++ b/api/common/common.go @@ -3,6 +3,8 @@ package common import ( "fmt" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -13,6 +15,7 @@ type ControllerName string const ( ServiceInstanceController ControllerName = "ServiceInstance" ServiceBindingController ControllerName = "ServiceBinding" + SecretController ControllerName = "Secret" FinalizerName string = "services.cloud.sap.com/sap-btp-finalizer" StaleBindingIDLabel string = "services.cloud.sap.com/stale" StaleBindingRotationOfLabel string = "services.cloud.sap.com/rotationOf" @@ -74,11 +77,18 @@ type SAPBTPResource interface { GetParameters() *runtime.RawExtension GetStatus() interface{} SetStatus(status interface{}) - GetObservedGeneration() int64 - SetObservedGeneration(int64) DeepClone() SAPBTPResource SetReady(metav1.ConditionStatus) GetReady() metav1.ConditionStatus GetAnnotations() map[string]string SetAnnotations(map[string]string) } + +func GetObservedGeneration(obj SAPBTPResource) int64 { + cond := meta.FindStatusCondition(obj.GetConditions(), ConditionSucceeded) + observedGen := int64(0) + if cond != nil { + observedGen = cond.ObservedGeneration + } + return observedGen +} diff --git a/api/common/consts.go b/api/common/consts.go index dd1a2166..e863ceee 100644 --- a/api/common/consts.go +++ b/api/common/consts.go @@ -3,6 +3,8 @@ package common const ( ManagedByBTPOperatorLabel = "services.cloud.sap.com/managed-by-sap-btp-operator" ClusterSecretLabel = "services.cloud.sap.com/cluster-secret" + InstanceSecretRefLabel = "services.cloud.sap.com/secret-ref_" + WatchSecretLabel = "services.cloud.sap.com/watch-secret" NamespaceLabel = "_namespace" K8sNameLabel = "_k8sname" diff --git a/api/v1/servicebinding_types.go b/api/v1/servicebinding_types.go index db432556..01296125 100644 --- a/api/v1/servicebinding_types.go +++ b/api/v1/servicebinding_types.go @@ -123,9 +123,6 @@ type ServiceBindingStatus struct { // Service binding conditions Conditions []metav1.Condition `json:"conditions"` - // Last generation that was acted on - ObservedGeneration int64 `json:"observedGeneration,omitempty"` - // Indicates whether binding is ready for usage Ready metav1.ConditionStatus `json:"ready,omitempty"` @@ -179,14 +176,6 @@ func (sb *ServiceBinding) SetStatus(status interface{}) { sb.Status = status.(ServiceBindingStatus) } -func (sb *ServiceBinding) GetObservedGeneration() int64 { - return sb.Status.ObservedGeneration -} - -func (sb *ServiceBinding) SetObservedGeneration(newObserved int64) { - sb.Status.ObservedGeneration = newObserved -} - 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 c0a91b2f..c0e713f9 100644 --- a/api/v1/servicebinding_types_test.go +++ b/api/v1/servicebinding_types_test.go @@ -66,18 +66,6 @@ var _ = Describe("Service Binding Type Test", func() { Expect(binding.GetControllerName()).To(Equal(common.ServiceBindingController)) }) - It("should update observed generation", func() { - Expect(binding.Status.ObservedGeneration).To(Equal(int64(0))) - binding.SetObservedGeneration(2) - Expect(binding.GetObservedGeneration()).To(Equal(int64(2))) - }) - - It("should update observed generation", func() { - Expect(binding.Status.ObservedGeneration).To(Equal(int64(0))) - binding.SetObservedGeneration(2) - Expect(binding.Status.ObservedGeneration).To(Equal(int64(2))) - }) - It("should update ready", func() { Expect(binding.Status.Ready).To(Equal(metav1.ConditionStatus(""))) binding.SetReady(metav1.ConditionTrue) diff --git a/api/v1/serviceinstance_types.go b/api/v1/serviceinstance_types.go index e6e967ff..4c391a85 100644 --- a/api/v1/serviceinstance_types.go +++ b/api/v1/serviceinstance_types.go @@ -17,11 +17,16 @@ limitations under the License. package v1 import ( + "crypto/md5" + "encoding/hex" + "encoding/json" + "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" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" ) // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! @@ -72,6 +77,10 @@ type ServiceInstanceSpec struct { // +optional ParametersFrom []ParametersFromSource `json:"parametersFrom,omitempty"` + // indicate instance will update on secrets from parametersFrom change + // +optional + SubscribeToSecretChanges *bool `json:"subscribeToSecretChanges,omitempty"` + // List of custom tags describing the ServiceInstance, will be copied to `ServiceBinding` secret in the key called `tags`. // +optional CustomTags []string `json:"customTags,omitempty"` @@ -107,9 +116,6 @@ type ServiceInstanceStatus struct { // Service instance conditions Conditions []metav1.Condition `json:"conditions"` - // Last generation that was acted on - ObservedGeneration int64 `json:"observedGeneration,omitempty"` - // Indicates whether instance is ready for usage Ready metav1.ConditionStatus `json:"ready,omitempty"` @@ -118,6 +124,9 @@ type ServiceInstanceStatus struct { // The subaccount id of the service instance SubaccountID string `json:"subaccountID,omitempty"` + + // if true need to update instance + ForceReconcile bool `json:"forceReconcile,omitempty"` } // +kubebuilder:object:root=true @@ -165,14 +174,6 @@ func (si *ServiceInstance) SetStatus(status interface{}) { si.Status = status.(ServiceInstanceStatus) } -func (si *ServiceInstance) GetObservedGeneration() int64 { - return si.Status.ObservedGeneration -} - -func (si *ServiceInstance) SetObservedGeneration(newObserved int64) { - si.Status.ObservedGeneration = newObserved -} - func (si *ServiceInstance) DeepClone() common.SAPBTPResource { return si.DeepCopy() } @@ -207,6 +208,19 @@ func init() { func (si *ServiceInstance) Hub() {} -func (si *ServiceInstance) ShouldBeShared() bool { +func (si *ServiceInstance) GetShared() bool { return si.Spec.Shared != nil && *si.Spec.Shared } + +func (si *ServiceInstance) IsSubscribedToSecretChange() bool { + return si.Spec.SubscribeToSecretChanges != nil && *si.Spec.SubscribeToSecretChanges +} + +func (si *ServiceInstance) GetSpecHash() string { + spec := si.Spec + spec.Shared = ptr.To(false) + specBytes, _ := json.Marshal(spec) + s := string(specBytes) + hash := md5.Sum([]byte(s)) + return hex.EncodeToString(hash[:]) +} diff --git a/api/v1/serviceinstance_types_test.go b/api/v1/serviceinstance_types_test.go index 35fa441c..15fb011e 100644 --- a/api/v1/serviceinstance_types_test.go +++ b/api/v1/serviceinstance_types_test.go @@ -1,12 +1,17 @@ package v1 import ( + "crypto/md5" + "encoding/hex" + "encoding/json" + "github.com/SAP/sap-btp-service-operator/api/common" . "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" + "k8s.io/utils/ptr" ) var _ = Describe("Service Instance Type Test", func() { @@ -96,12 +101,6 @@ var _ = Describe("Service Instance Type Test", func() { Expect(instance.GetControllerName()).To(Equal(common.ServiceInstanceController)) }) - It("should update observed generation", func() { - Expect(instance.Status.ObservedGeneration).To(Equal(int64(0))) - instance.SetObservedGeneration(2) - Expect(instance.GetObservedGeneration()).To(Equal(int64(2))) - }) - It("should update ready", func() { Expect(instance.Status.Ready).To(Equal(metav1.ConditionStatus(""))) instance.SetReady(metav1.ConditionTrue) @@ -129,4 +128,85 @@ var _ = Describe("Service Instance Type Test", func() { instance.SetAnnotations(annotation) Expect(instance.GetAnnotations()).To(Equal(annotation)) }) + + It("should update SubscribeToSecretChanges", func() { + instance.Spec.SubscribeToSecretChanges = &[]bool{true}[0] + Expect(instance.IsSubscribedToSecretChange()).To(BeTrue()) + }) + + It("should return correct spec hash", func() { + // Calculate expected hash + spec := instance.Spec + spec.Shared = ptr.To(false) + specBytes, _ := json.Marshal(spec) + hash := md5.Sum(specBytes) + expectedHash := hex.EncodeToString(hash[:]) + + // Get actual hash + actualHash := instance.GetSpecHash() + + // Compare hashes + Expect(actualHash).To(Equal(expectedHash)) + }) + It("should update spec hash when spec changes", func() { + // Calculate initial hash + initialHash := instance.GetSpecHash() + + // Modify the spec + instance.Spec.ServicePlanName = "new-plan" + + // Calculate new hash + newHash := instance.GetSpecHash() + + // Ensure the hash has changed + Expect(initialHash).NotTo(Equal(newHash)) + }) + It("should update spec hash when parametersFrom changes", func() { + // Calculate initial hash + initialHash := instance.GetSpecHash() + + // Modify the parametersFrom field + instance.Spec.ParametersFrom = []ParametersFromSource{ + { + SecretKeyRef: &SecretKeyReference{ + Name: "new-param-secret", + Key: "new-secret-parameter", + }, + }, + } + + // Calculate new hash + newHash := instance.GetSpecHash() + + // Ensure the hash has changed + Expect(initialHash).NotTo(Equal(newHash)) + }) + It("should update spec hash when parametersFrom changes with initial object", func() { + // Initialize ParametersFrom with an object + instance.Spec.ParametersFrom = []ParametersFromSource{ + { + SecretKeyRef: &SecretKeyReference{ + Name: "initial-param-secret", + Key: "initial-secret-parameter", + }, + }, + } + + // Calculate initial hash + initialHash := instance.GetSpecHash() + + // Modify the parametersFrom field + instance.Spec.ParametersFrom = append(instance.Spec.ParametersFrom, ParametersFromSource{ + SecretKeyRef: &SecretKeyReference{ + Name: "new-param-secret", + Key: "new-secret-parameter", + }, + }) + + // Calculate new hash + newHash := instance.GetSpecHash() + + // Ensure the hash has changed + Expect(initialHash).NotTo(Equal(newHash)) + }) }) diff --git a/api/v1/suite_test.go b/api/v1/suite_test.go index 39dfb954..7fa64993 100644 --- a/api/v1/suite_test.go +++ b/api/v1/suite_test.go @@ -77,6 +77,7 @@ func getInstance() *ServiceInstance { }, }, }, + SubscribeToSecretChanges: &[]bool{true}[0], UserInfo: &v1.UserInfo{ Username: "test-user", Groups: []string{"test-group"}, diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 03fd1a7c..ea90a066 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -287,6 +287,11 @@ func (in *ServiceInstanceSpec) DeepCopyInto(out *ServiceInstanceSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.SubscribeToSecretChanges != nil { + in, out := &in.SubscribeToSecretChanges, &out.SubscribeToSecretChanges + *out = new(bool) + **out = **in + } if in.CustomTags != nil { in, out := &in.CustomTags, &out.CustomTags *out = make([]string, len(*in)) diff --git a/config/crd/bases/services.cloud.sap.com_servicebindings.yaml b/config/crd/bases/services.cloud.sap.com_servicebindings.yaml index 9f6384d3..7a0fb9eb 100644 --- a/config/crd/bases/services.cloud.sap.com_servicebindings.yaml +++ b/config/crd/bases/services.cloud.sap.com_servicebindings.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.15.0 + controller-gen.kubebuilder.io/version: v0.16.1 name: servicebindings.services.cloud.sap.com spec: group: services.cloud.sap.com @@ -82,7 +82,6 @@ spec: description: |- Parameters for the binding. - The Parameters field is NOT secret or secured in any way and should NEVER be used to hold sensitive information. To set parameters that contain secret information, you should ALWAYS store that information @@ -199,16 +198,8 @@ spec: conditions: description: Service binding conditions items: - description: "Condition contains details for one aspect of the current - state of this API Resource.\n---\nThis struct is intended for - direct use as an array at the field path .status.conditions. For - example,\n\n\n\ttype FooStatus struct{\n\t // Represents the - observations of a foo's current state.\n\t // Known .status.conditions.type - are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // - +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t - \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" - patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t - \ // other fields\n\t}" + description: Condition contains details for one aspect of the current + state of this API Resource. properties: lastTransitionTime: description: |- @@ -249,12 +240,7 @@ spec: - Unknown type: string type: - description: |- - type of condition in CamelCase or in foo.example.com/CamelCase. - --- - Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be - useful (see .node.status.conditions), the ability to deconflict is important. - The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + description: type of condition in CamelCase or in foo.example.com/CamelCase. maxLength: 316 pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string @@ -273,10 +259,6 @@ spec: description: Indicates when binding secret was rotated format: date-time type: string - observedGeneration: - description: Last generation that was acted on - format: int64 - type: integer operationType: description: The operation type (CREATE/UPDATE/DELETE) for ongoing operation @@ -366,7 +348,6 @@ spec: description: |- Parameters for the binding. - The Parameters field is NOT secret or secured in any way and should NEVER be used to hold sensitive information. To set parameters that contain secret information, you should ALWAYS store that information @@ -468,16 +449,8 @@ spec: conditions: description: Service binding conditions items: - description: "Condition contains details for one aspect of the current - state of this API Resource.\n---\nThis struct is intended for - direct use as an array at the field path .status.conditions. For - example,\n\n\n\ttype FooStatus struct{\n\t // Represents the - observations of a foo's current state.\n\t // Known .status.conditions.type - are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // - +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t - \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" - patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t - \ // other fields\n\t}" + description: Condition contains details for one aspect of the current + state of this API Resource. properties: lastTransitionTime: description: |- @@ -518,12 +491,7 @@ spec: - Unknown type: string type: - description: |- - type of condition in CamelCase or in foo.example.com/CamelCase. - --- - Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be - useful (see .node.status.conditions), the ability to deconflict is important. - The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + description: type of condition in CamelCase or in foo.example.com/CamelCase. maxLength: 316 pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string diff --git a/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml b/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml index 745cfde5..eef2bb72 100644 --- a/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml +++ b/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.15.0 + controller-gen.kubebuilder.io/version: v0.16.1 name: serviceinstances.services.cloud.sap.com spec: group: services.cloud.sap.com @@ -89,7 +89,6 @@ spec: description: |- Provisioning parameters for the instance. - The Parameters field is NOT secret or secured in any way and should NEVER be used to hold sensitive information. To set parameters that contain secret information, you should ALWAYS store that information @@ -140,6 +139,10 @@ spec: shared: description: Indicates the desired shared state type: boolean + subscribeToSecretChanges: + description: indicate instance will update on secrets from parametersFrom + change + type: boolean userInfo: description: |- UserInfo contains information about the user that last modified this @@ -181,16 +184,8 @@ spec: conditions: description: Service instance conditions items: - description: "Condition contains details for one aspect of the current - state of this API Resource.\n---\nThis struct is intended for - direct use as an array at the field path .status.conditions. For - example,\n\n\n\ttype FooStatus struct{\n\t // Represents the - observations of a foo's current state.\n\t // Known .status.conditions.type - are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // - +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t - \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" - patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t - \ // other fields\n\t}" + description: Condition contains details for one aspect of the current + state of this API Resource. properties: lastTransitionTime: description: |- @@ -231,12 +226,7 @@ spec: - Unknown type: string type: - description: |- - type of condition in CamelCase or in foo.example.com/CamelCase. - --- - Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be - useful (see .node.status.conditions), the ability to deconflict is important. - The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + description: type of condition in CamelCase or in foo.example.com/CamelCase. maxLength: 316 pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string @@ -248,6 +238,9 @@ spec: - type type: object type: array + forceReconcile: + description: if true need to update instance + type: boolean hashedSpec: description: HashedSpec is the hashed spec without the shared property type: string @@ -255,10 +248,6 @@ spec: description: The generated ID of the instance, will be automatically filled once the instance is created type: string - observedGeneration: - description: Last generation that was acted on - format: int64 - type: integer operationType: description: The operation type (CREATE/UPDATE/DELETE) for ongoing operation @@ -349,7 +338,6 @@ spec: description: |- Provisioning parameters for the instance. - The Parameters field is NOT secret or secured in any way and should NEVER be used to hold sensitive information. To set parameters that contain secret information, you should ALWAYS store that information @@ -441,16 +429,8 @@ spec: conditions: description: Service instance conditions items: - description: "Condition contains details for one aspect of the current - state of this API Resource.\n---\nThis struct is intended for - direct use as an array at the field path .status.conditions. For - example,\n\n\n\ttype FooStatus struct{\n\t // Represents the - observations of a foo's current state.\n\t // Known .status.conditions.type - are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // - +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t - \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" - patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t - \ // other fields\n\t}" + description: Condition contains details for one aspect of the current + state of this API Resource. properties: lastTransitionTime: description: |- @@ -491,12 +471,7 @@ spec: - Unknown type: string type: - description: |- - type of condition in CamelCase or in foo.example.com/CamelCase. - --- - Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be - useful (see .node.status.conditions), the ability to deconflict is important. - The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + description: type of condition in CamelCase or in foo.example.com/CamelCase. maxLength: 316 pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index de9857dc..74f1babd 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -17,17 +17,6 @@ rules: - "" resources: - events - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - "" - resources: - secrets verbs: - create @@ -41,25 +30,6 @@ rules: - services.cloud.sap.com resources: - servicebindings - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - services.cloud.sap.com - resources: - - servicebindings/status - verbs: - - get - - patch - - update -- apiGroups: - - services.cloud.sap.com - resources: - serviceinstances verbs: - create @@ -72,6 +42,7 @@ rules: - apiGroups: - services.cloud.sap.com resources: + - servicebindings/status - serviceinstances/status verbs: - get diff --git a/config/samples/services_v1_serviceinstance.yaml b/config/samples/services_v1_serviceinstance.yaml index bd19f598..197de5a2 100644 --- a/config/samples/services_v1_serviceinstance.yaml +++ b/config/samples/services_v1_serviceinstance.yaml @@ -4,4 +4,4 @@ metadata: name: sample-instance-1 spec: serviceOfferingName: service-manager - servicePlanName: subaccount-audit + servicePlanName: subaccount-audit \ No newline at end of file diff --git a/controllers/secret_controller.go b/controllers/secret_controller.go new file mode 100644 index 00000000..a0a22011 --- /dev/null +++ b/controllers/secret_controller.go @@ -0,0 +1,133 @@ +package controllers + +import ( + "context" + "fmt" + "reflect" + + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + "k8s.io/apimachinery/pkg/labels" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + "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/internal/utils" + "github.com/go-logr/logr" + "github.com/google/uuid" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +type SecretReconciler struct { + client.Client + Scheme *runtime.Scheme + Log logr.Logger +} + +// +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 *SecretReconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { + log := r.Log.WithValues("secret", req.NamespacedName).WithValues("correlation_id", uuid.New().String()) + ctx = context.WithValue(ctx, utils.LogKey{}, log) + log.Info(fmt.Sprintf("reconciling secret %s", req.NamespacedName)) + // Fetch the Secret + secret := &corev1.Secret{} + if err := r.Get(ctx, req.NamespacedName, secret); err != nil { + if !apierrors.IsNotFound(err) { + log.Error(err, "unable to fetch Secret") + } + // we'll ignore not-found errors, since they can't be fixed by an immediate + // requeue (we'll need to wait for a new notification), and we can get them + // on deleted requests. + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + var instances v1.ServiceInstanceList + labelSelector := client.MatchingLabels{common.InstanceSecretRefLabel + string(secret.GetUID()): secret.Name} + if err := r.Client.List(ctx, &instances, labelSelector); err != nil { + log.Error(err, "failed to list service instances") + return ctrl.Result{}, err + } + + if len(instances.Items) == 0 { + // No instances are using this secret + log.Info(fmt.Sprintf("no instances are using secret %s, removing watch label and finalizer", req.NamespacedName)) + delete(secret.Labels, common.WatchSecretLabel) + return ctrl.Result{}, utils.RemoveFinalizer(ctx, r.Client, secret, common.FinalizerName, common.SecretController) + } + + for _, instance := range instances.Items { + log.Info(fmt.Sprintf("waking up instance %s", instance.Name)) + instance.Status.ForceReconcile = true + err := utils.UpdateStatus(ctx, r.Client, &instance) + if err != nil { + return reconcile.Result{}, err + } + } + + if utils.IsMarkedForDeletion(secret.ObjectMeta) { + return ctrl.Result{}, utils.RemoveFinalizer(ctx, r.Client, secret, common.FinalizerName, common.SecretController) + } + + return reconcile.Result{}, nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *SecretReconciler) SetupWithManager(mgr ctrl.Manager) error { + labelSelector := labels.SelectorFromSet(map[string]string{common.WatchSecretLabel: "true"}) + labelPredicate := predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + return labelSelector.Matches(labels.Set(e.ObjectNew.GetLabels())) && (isSecretDataChanged(e) || isSecretInDelete(e)) + }, + CreateFunc: func(e event.CreateEvent) bool { + return labelSelector.Matches(labels.Set(e.Object.GetLabels())) + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return labelSelector.Matches(labels.Set(e.Object.GetLabels())) + }, + GenericFunc: func(e event.GenericEvent) bool { + return labelSelector.Matches(labels.Set(e.Object.GetLabels())) + }, + } + + return ctrl.NewControllerManagedBy(mgr). + For(&corev1.Secret{}). + WithEventFilter(labelPredicate). + WithOptions(controller.Options{MaxConcurrentReconciles: 1}). + Complete(r) +} + +func isSecretDataChanged(e event.UpdateEvent) bool { + // Type assert to *v1.Secret + oldSecret, okOld := e.ObjectOld.(*corev1.Secret) + newSecret, okNew := e.ObjectNew.(*corev1.Secret) + if !okOld || !okNew { + // If the objects are not Secrets, skip the event + return false + } + + // Compare the Data field (byte slices) + return !reflect.DeepEqual(oldSecret.Data, newSecret.Data) || !reflect.DeepEqual(oldSecret.StringData, newSecret.StringData) +} + +func isSecretInDelete(e event.UpdateEvent) bool { + // Type assert to *v1.Secret + + newSecret, okNew := e.ObjectNew.(*corev1.Secret) + if !okNew { + // If the objects are not Secrets, skip the event + return false + } + + // Compare the Data field (byte slices) + return !newSecret.GetDeletionTimestamp().IsZero() && controllerutil.ContainsFinalizer(newSecret, common.FinalizerName) +} diff --git a/controllers/servicebinding_controller.go b/controllers/servicebinding_controller.go index fef2f309..7c954946 100644 --- a/controllers/servicebinding_controller.go +++ b/controllers/servicebinding_controller.go @@ -91,8 +91,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque return ctrl.Result{}, client.IgnoreNotFound(err) } serviceBinding = serviceBinding.DeepCopy() - log.Info(fmt.Sprintf("Current generation is %v and observed is %v", serviceBinding.Generation, serviceBinding.GetObservedGeneration())) - serviceBinding.SetObservedGeneration(serviceBinding.Generation) + log.Info(fmt.Sprintf("Current generation is %v and observed is %v", serviceBinding.Generation, common.GetObservedGeneration(serviceBinding))) if len(serviceBinding.GetConditions()) == 0 { if err := utils.InitConditions(ctx, r.Client, serviceBinding); err != nil { @@ -182,7 +181,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque if utils.IsInProgress(serviceInstance) { log.Info(fmt.Sprintf("Service instance with k8s name %s is not ready for binding yet", serviceInstance.Name)) - utils.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, false) return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, utils.UpdateStatus(ctx, r.Client, serviceBinding) } @@ -242,7 +241,7 @@ func (r *ServiceBindingReconciler) updateSecret(ctx context.Context, serviceBind return err } log.Info("Updating binding", "bindingID", smBinding.ID) - utils.SetSuccessConditions(smClientTypes.UPDATE, serviceBinding) + utils.SetSuccessConditions(smClientTypes.UPDATE, serviceBinding, false) } } return nil @@ -259,7 +258,7 @@ func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient s log := utils.GetLogger(ctx) log.Info("Creating smBinding in SM") serviceBinding.Status.InstanceID = serviceInstance.Status.InstanceID - _, bindingParameters, err := utils.BuildSMRequestParameters(serviceBinding.Namespace, serviceBinding.Spec.ParametersFrom, serviceBinding.Spec.Parameters) + bindingParameters, _, err := utils.BuildSMRequestParameters(serviceBinding.Namespace, serviceBinding.Spec.Parameters, serviceBinding.Spec.ParametersFrom) if err != nil { log.Error(err, "failed to parse smBinding parameters") return utils.MarkAsNonTransientError(ctx, r.Client, smClientTypes.CREATE, err, serviceBinding) @@ -291,7 +290,7 @@ func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient s log.Info("Create smBinding request is async") serviceBinding.Status.OperationURL = operationURL serviceBinding.Status.OperationType = smClientTypes.CREATE - utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "", serviceBinding) + utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "", serviceBinding, false) if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil { log.Error(err, "unable to update ServiceBinding status") return ctrl.Result{}, err @@ -313,7 +312,7 @@ func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient s serviceBinding.Status.BindingID = smBinding.ID serviceBinding.Status.SubaccountID = subaccountID serviceBinding.Status.Ready = metav1.ConditionTrue - utils.SetSuccessConditions(smClientTypes.CREATE, serviceBinding) + utils.SetSuccessConditions(smClientTypes.CREATE, serviceBinding, false) log.Info("Updating binding", "bindingID", smBinding.ID) return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding) @@ -336,7 +335,7 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *v if smBinding != nil { log.Info("binding exists in SM continue with deletion") serviceBinding.Status.BindingID = smBinding.ID - utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "delete after recovery", serviceBinding) + utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "delete after recovery", serviceBinding, false) return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding) } @@ -346,7 +345,7 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *v } log.Info("Binding does not exists in SM, removing finalizer") - if err := utils.RemoveFinalizer(ctx, r.Client, serviceBinding, common.FinalizerName); err != nil { + if err := utils.RemoveFinalizer(ctx, r.Client, serviceBinding, common.FinalizerName, serviceBinding.GetControllerName()); err != nil { return ctrl.Result{}, err } return ctrl.Result{}, nil @@ -367,7 +366,7 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *v log.Info("Deleting binding async") serviceBinding.Status.OperationURL = operationURL serviceBinding.Status.OperationType = smClientTypes.DELETE - utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "", serviceBinding) + utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "", serviceBinding, false) if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil { return ctrl.Result{}, err } @@ -392,7 +391,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *v1. 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) - utils.SetInProgressConditions(ctx, serviceBinding.Status.OperationType, string(smClientTypes.INPROGRESS), serviceBinding) + utils.SetInProgressConditions(ctx, serviceBinding.Status.OperationType, string(smClientTypes.INPROGRESS), serviceBinding, false) freshStatus := v1.ServiceBindingStatus{ Conditions: serviceBinding.GetConditions(), } @@ -414,7 +413,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *v1. fallthrough case smClientTypes.PENDING: if len(status.Description) != 0 { - utils.SetInProgressConditions(ctx, status.Type, status.Description, serviceBinding) + utils.SetInProgressConditions(ctx, status.Type, status.Description, serviceBinding, true) if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil { log.Error(err, "unable to update ServiceBinding polling description") return ctrl.Result{}, err @@ -423,7 +422,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *v1. return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, nil case smClientTypes.FAILED: // non transient error - should not retry - utils.SetFailureConditions(status.Type, status.Description, serviceBinding) + utils.SetFailureConditions(status.Type, status.Description, serviceBinding, true) if serviceBinding.Status.OperationType == smClientTypes.DELETE { serviceBinding.Status.OperationURL = "" serviceBinding.Status.OperationType = "" @@ -438,7 +437,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *v1. return ctrl.Result{}, fmt.Errorf(errMsg) } case smClientTypes.SUCCEEDED: - utils.SetSuccessConditions(status.Type, serviceBinding) + utils.SetSuccessConditions(status.Type, serviceBinding, true) switch serviceBinding.Status.OperationType { case smClientTypes.CREATE: smBinding, err := smClient.GetBindingByID(serviceBinding.Status.BindingID, nil) @@ -453,7 +452,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *v1. if err := r.storeBindingSecret(ctx, serviceBinding, smBinding); err != nil { return r.handleSecretError(ctx, smClientTypes.CREATE, err, serviceBinding) } - utils.SetSuccessConditions(status.Type, serviceBinding) + utils.SetSuccessConditions(status.Type, serviceBinding, false) case smClientTypes.DELETE: return r.deleteSecretAndRemoveFinalizer(ctx, serviceBinding) } @@ -495,10 +494,6 @@ func (r *ServiceBindingReconciler) getBindingForRecovery(ctx context.Context, sm func (r *ServiceBindingReconciler) maintain(ctx context.Context, binding *v1.ServiceBinding) (ctrl.Result, error) { log := utils.GetLogger(ctx) shouldUpdateStatus := false - if binding.Generation != binding.Status.ObservedGeneration { - binding.SetObservedGeneration(binding.Generation) - shouldUpdateStatus = true - } if !utils.IsFailed(binding) { secret, err := r.getSecret(ctx, binding.Namespace, binding.Spec.SecretName) if err != nil { @@ -507,7 +502,7 @@ func (r *ServiceBindingReconciler) maintain(ctx context.Context, binding *v1.Ser log.Info(fmt.Sprintf("secret not found recovering binding %s", binding.Name)) binding.Status.BindingID = "" binding.Status.Ready = metav1.ConditionFalse - utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "recreating deleted secret", binding) + utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "recreating deleted secret", binding, false) shouldUpdateStatus = true r.Recorder.Event(binding, corev1.EventTypeWarning, "SecretDeleted", "SecretDeleted") } else { @@ -565,7 +560,6 @@ func (r *ServiceBindingReconciler) setOwner(ctx context.Context, serviceInstance } func (r *ServiceBindingReconciler) resyncBindingStatus(ctx context.Context, k8sBinding *v1.ServiceBinding, smBinding *smClientTypes.ServiceBinding) { - k8sBinding.Status.ObservedGeneration = k8sBinding.Generation k8sBinding.Status.BindingID = smBinding.ID k8sBinding.Status.InstanceID = smBinding.ServiceInstanceID k8sBinding.Status.OperationURL = "" @@ -587,11 +581,11 @@ 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 - utils.SetInProgressConditions(ctx, smBinding.LastOperation.Type, smBinding.LastOperation.Description, k8sBinding) + utils.SetInProgressConditions(ctx, smBinding.LastOperation.Type, smBinding.LastOperation.Description, k8sBinding, false) case smClientTypes.SUCCEEDED: - utils.SetSuccessConditions(operationType, k8sBinding) + utils.SetSuccessConditions(operationType, k8sBinding, false) case smClientTypes.FAILED: - utils.SetFailureConditions(operationType, description, k8sBinding) + utils.SetFailureConditions(operationType, description, k8sBinding, false) } } @@ -621,6 +615,11 @@ func (r *ServiceBindingReconciler) storeBindingSecret(ctx context.Context, k8sBi } secret.Labels[common.ManagedByBTPOperatorLabel] = "true" + if secret.Annotations == nil { + secret.Annotations = map[string]string{} + } + secret.Annotations["binding"] = k8sBinding.Name + return r.createOrUpdateBindingSecret(ctx, k8sBinding, secret) } @@ -808,7 +807,7 @@ func (r *ServiceBindingReconciler) deleteSecretAndRemoveFinalizer(ctx context.Co return ctrl.Result{}, err } - return ctrl.Result{}, utils.RemoveFinalizer(ctx, r.Client, serviceBinding, common.FinalizerName) + return ctrl.Result{}, utils.RemoveFinalizer(ctx, r.Client, serviceBinding, common.FinalizerName, serviceBinding.GetControllerName()) } func (r *ServiceBindingReconciler) getSecret(ctx context.Context, namespace string, name string) (*corev1.Secret, error) { @@ -988,7 +987,7 @@ func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, bindin binding.Status.BindingID = "" binding.Status.Ready = metav1.ConditionFalse - utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "rotating binding credentials", binding) + utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "rotating binding credentials", binding, false) utils.SetCredRotationInProgressConditions(common.CredRotating, "", binding) return utils.UpdateStatus(ctx, r.Client, binding) } diff --git a/controllers/servicebinding_controller_test.go b/controllers/servicebinding_controller_test.go index 1d1033e9..398d8a80 100644 --- a/controllers/servicebinding_controller_test.go +++ b/controllers/servicebinding_controller_test.go @@ -4,11 +4,12 @@ import ( "context" "encoding/json" "errors" - "github.com/lithammer/dedent" - authv1 "k8s.io/api/authentication/v1" "net/http" "strings" + "github.com/lithammer/dedent" + authv1 "k8s.io/api/authentication/v1" + "github.com/SAP/sap-btp-service-operator/api/common" "github.com/SAP/sap-btp-service-operator/internal/utils" "k8s.io/utils/pointer" @@ -22,7 +23,6 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -36,8 +36,6 @@ import ( var _ = Describe("ServiceBinding controller", func() { var ( - ctx context.Context - createdInstance *v1.ServiceInstance createdBinding *v1.ServiceBinding @@ -47,9 +45,10 @@ var _ = Describe("ServiceBinding controller", func() { bindingName string instanceName string instanceExternalName string + paramsSecret *corev1.Secret ) - createBindingWithoutAssertionsAndWait := func(ctx context.Context, name, namespace, instanceName, instanceNamespace, externalName string, secretTemplate string, wait bool) (*v1.ServiceBinding, error) { + createBindingWithoutAssertions := func(ctx context.Context, name, namespace, instanceName, instanceNamespace, externalName string, secretTemplate string, wait bool) (*v1.ServiceBinding, error) { binding := generateBasicBindingTemplate(name, namespace, instanceName, instanceNamespace, externalName, secretTemplate) if err := k8sClient.Create(ctx, binding); err != nil { return nil, err @@ -72,45 +71,13 @@ var _ = Describe("ServiceBinding controller", func() { return createdBinding, nil } - createBindingWithoutAssertions := func(ctx context.Context, name, namespace, instanceName, instanceNamespace, externalName string, secretTemplate string) (*v1.ServiceBinding, error) { - return createBindingWithoutAssertionsAndWait(ctx, name, namespace, instanceName, instanceNamespace, externalName, secretTemplate, false) - } - - createBindingWithTransitError := func(ctx context.Context, name, namespace, instanceName, externalName, failureMessage string, secretTemplate string) { - binding, err := createBindingWithoutAssertions(ctx, name, namespace, instanceName, "", externalName, secretTemplate) - if err != nil { - Expect(err.Error()).To(ContainSubstring(failureMessage)) - } else { - waitForResourceCondition(ctx, binding, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateInProgress, failureMessage) - } - } - - createBindingWithError := func(ctx context.Context, name, namespace, instanceName, externalName, failureMessage string, secretTemplate string) { - binding, err := createBindingWithoutAssertions(ctx, name, namespace, instanceName, "", externalName, secretTemplate) - if err != nil { - Expect(err.Error()).To(ContainSubstring(failureMessage)) - } else { - waitForResourceCondition(ctx, binding, common.ConditionFailed, metav1.ConditionTrue, "", failureMessage) - } - } - - createBindingWithBlockedError := func(ctx context.Context, name, namespace, instanceName, externalName, failureMessage string) *v1.ServiceBinding { - binding, err := createBindingWithoutAssertions(ctx, name, namespace, instanceName, "", externalName, "") - if err != nil { - Expect(err.Error()).To(ContainSubstring(failureMessage)) - } else { - waitForResourceCondition(ctx, binding, common.ConditionSucceeded, metav1.ConditionFalse, "", failureMessage) - } - return binding - } - - createBinding := func(ctx context.Context, name, namespace, instanceName, instanceNamespace, externalName string, secretTemplate string) *v1.ServiceBinding { - createdBinding, err := createBindingWithoutAssertions(ctx, name, namespace, instanceName, instanceNamespace, externalName, secretTemplate) + createAndValidateBinding := func(ctx context.Context, name, namespace, instanceName, instanceNamespace, externalName string, secretTemplate string) *v1.ServiceBinding { + createdBinding, err := createBindingWithoutAssertions(ctx, name, namespace, instanceName, instanceNamespace, externalName, secretTemplate, false) Expect(err).ToNot(HaveOccurred()) Expect(createdBinding.Status.InstanceID).ToNot(BeEmpty()) Expect(createdBinding.Status.BindingID).To(Equal(fakeBindingID)) Expect(createdBinding.Spec.SecretName).To(Not(BeEmpty())) - Expect(int(createdBinding.Status.ObservedGeneration)).To(Equal(1)) + Expect(common.GetObservedGeneration(createdBinding)).To(Equal(int64(1))) Expect(string(createdBinding.Spec.Parameters.Raw)).To(ContainSubstring("\"key\":\"value\"")) smBinding, _, _ := fakeClient.BindArgsForCall(0) params := smBinding.Parameters @@ -182,26 +149,23 @@ var _ = Describe("ServiceBinding controller", func() { fakeClient.GetInstanceByIDReturns(smInstance, nil) defaultLookupKey = types.NamespacedName{Namespace: bindingTestNamespace, Name: bindingName} - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: bindingTestNamespace, Name: "param-secret"}, &corev1.Secret{}) - if apierrors.IsNotFound(err) { - createParamsSecret(bindingTestNamespace) - } else { - Expect(err).ToNot(HaveOccurred()) - } - createdInstance = createInstance(ctx, instanceName, bindingTestNamespace, instanceExternalName) + paramsSecret = createParamsSecret(ctx, "binding-params-secret", bindingTestNamespace) + }) AfterEach(func() { if createdBinding != nil { fakeClient.UnbindReturns("", nil) - deleteAndWait(ctx, types.NamespacedName{Name: createdBinding.Name, Namespace: createdBinding.Namespace}, &v1.ServiceBinding{}) + deleteAndWait(ctx, createdBinding) } - if createdInstance != nil { - deleteAndWait(ctx, types.NamespacedName{Name: instanceName, Namespace: bindingTestNamespace}, &v1.ServiceInstance{}) + fakeClient.DeprovisionReturns("", nil) + deleteAndWait(ctx, createdInstance) } + deleteAndWait(ctx, paramsSecret) + createdBinding = nil createdInstance = nil }) @@ -210,15 +174,17 @@ var _ = Describe("ServiceBinding controller", func() { Context("invalid parameters", func() { When("service instance name is not provided", func() { It("should fail", func() { - createBindingWithTransitError(ctx, bindingName, bindingTestNamespace, "", "", - "spec.serviceInstanceName in body should be at least 1 chars long", "") + _, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, "", "", "", "", false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("spec.serviceInstanceName in body should be at least 1 chars long")) }) }) When("referenced service instance does not exist", func() { It("should fail", func() { - createBindingWithBlockedError(ctx, bindingName, bindingTestNamespace, "no-such-instance", "", - "couldn't find the service instance") + binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, "no-such-instance", "", "", "", false) + Expect(err).ToNot(HaveOccurred()) + waitForResourceCondition(ctx, binding, common.ConditionSucceeded, metav1.ConditionFalse, "", "couldn't find the service instance") }) }) @@ -235,7 +201,7 @@ var _ = Describe("ServiceBinding controller", func() { }) AfterEach(func() { - deleteAndWait(ctx, types.NamespacedName{Name: secretName, Namespace: bindingTestNamespace}, &corev1.Secret{}) + deleteAndWait(ctx, secret) }) When("name is already taken", func() { @@ -284,7 +250,7 @@ var _ = Describe("ServiceBinding controller", func() { Context("sync", func() { It("Should create binding and store the binding credentials in a secret", func() { - createdBinding = createBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "") + createdBinding = createAndValidateBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "") Expect(createdBinding.Spec.ExternalName).To(Equal("binding-external-name")) Expect(createdBinding.Spec.UserInfo).NotTo(BeNil()) @@ -354,7 +320,7 @@ var _ = Describe("ServiceBinding controller", func() { When("secret deleted by user", func() { It("should recreate the secret", func() { - createdBinding = createBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "") + createdBinding = createAndValidateBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "") secretLookupKey := types.NamespacedName{Name: createdBinding.Spec.SecretName, Namespace: createdBinding.Namespace} bindingSecret := getSecret(ctx, secretLookupKey.Name, secretLookupKey.Namespace, true) originalSecretUID := bindingSecret.UID @@ -395,7 +361,9 @@ var _ = Describe("ServiceBinding controller", func() { }) It("should fail with the error returned from SM", func() { - createBindingWithError(ctx, bindingName, bindingTestNamespace, instanceName, "binding-external-name", errorMessage, "") + binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "", false) + Expect(err).ToNot(HaveOccurred()) + waitForResourceCondition(ctx, binding, common.ConditionFailed, metav1.ConditionTrue, "", errorMessage) }) }) @@ -410,7 +378,7 @@ var _ = Describe("ServiceBinding controller", func() { }) It("should eventually succeed", func() { - b, err := createBindingWithoutAssertionsAndWait(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "", true) + b, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "", true) Expect(err).ToNot(HaveOccurred()) Expect(isResourceReady(b)).To(BeTrue()) }) @@ -426,7 +394,9 @@ var _ = Describe("ServiceBinding controller", func() { }) It("should fail", func() { - createBindingWithTransitError(ctx, bindingName, bindingTestNamespace, instanceName, "binding-external-name", errorMessage, "") + binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "", false) + Expect(err).ToNot(HaveOccurred()) + waitForResourceCondition(ctx, binding, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateInProgress, errorMessage) }) }) @@ -437,7 +407,7 @@ var _ = Describe("ServiceBinding controller", func() { }) It("should detect the error as transient and eventually succeed", func() { - createdBinding, _ := createBindingWithoutAssertionsAndWait(ctx, + createdBinding, _ := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, @@ -464,7 +434,9 @@ var _ = Describe("ServiceBinding controller", func() { }) It("should detect the error as non-transient and fail", func() { - createBindingWithTransitError(ctx, bindingName, bindingTestNamespace, instanceName, "binding-external-name", errorMessage, "") + binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "", false) + Expect(err).ToNot(HaveOccurred()) + waitForResourceCondition(ctx, binding, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateInProgress, errorMessage) }) }) @@ -476,7 +448,7 @@ var _ = Describe("ServiceBinding controller", func() { }) It("creation will fail with appropriate message", func() { - createdBinding, _ = createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", "") + createdBinding, _ = createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", "", false) waitForResourceCondition(ctx, createdBinding, common.ConditionFailed, metav1.ConditionTrue, "CreateFailed", "failed to create secret") }) }) @@ -490,7 +462,7 @@ var _ = Describe("ServiceBinding controller", func() { When("bind polling returns success", func() { It("Should create binding and store the binding credentials in a secret", func() { fakeClient.StatusReturns(&smClientTypes.Operation{ResourceID: fakeBindingID, State: smClientTypes.SUCCEEDED}, nil) - createdBinding = createBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "", "") + createdBinding = createAndValidateBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "", "") }) }) @@ -502,7 +474,10 @@ var _ = Describe("ServiceBinding controller", func() { State: smClientTypes.FAILED, Description: errorMessage, }, nil) - createBindingWithError(ctx, bindingName, bindingTestNamespace, instanceName, "existing-name", errorMessage, "") + + binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "existing-name", "", false) + Expect(err).ToNot(HaveOccurred()) + waitForResourceCondition(ctx, binding, common.ConditionFailed, metav1.ConditionTrue, "", errorMessage) }) }) @@ -513,7 +488,7 @@ var _ = Describe("ServiceBinding controller", func() { fakeClient.GetBindingByIDReturns(&smClientTypes.ServiceBinding{ID: fakeBindingID, LastOperation: &smClientTypes.Operation{State: smClientTypes.SUCCEEDED, Type: smClientTypes.CREATE}}, nil) }) It("should eventually succeed", func() { - binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", "") + binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", "", false) Expect(err).ToNot(HaveOccurred()) waitForResourceCondition(ctx, binding, common.ConditionFailed, metav1.ConditionTrue, "", "no polling for you") fakeClient.ListBindingsReturns(&smClientTypes.ServiceBindings{ @@ -541,7 +516,7 @@ var _ = Describe("ServiceBinding controller", func() { common.UseInstanceMetadataNameInSecret: "true", } updateInstance(ctx, createdInstance) - createdBinding = createBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "", "") + createdBinding = createAndValidateBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "", "") bindingSecret := getSecret(ctx, createdBinding.Spec.SecretName, createdBinding.Namespace, true) validateInstanceInfo(bindingSecret, instanceName) validateSecretMetadata(bindingSecret, nil) @@ -550,7 +525,7 @@ var _ = Describe("ServiceBinding controller", func() { When("external name is not provided", func() { It("succeeds and uses the k8s name as external name", func() { - createdBinding = createBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "", "") + createdBinding = createAndValidateBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "", "") Expect(createdBinding.Spec.ExternalName).To(Equal(createdBinding.Name)) }) }) @@ -566,10 +541,14 @@ var _ = Describe("ServiceBinding controller", func() { When("referenced service instance is failed", func() { It("should retry and succeed once the instance is ready", func() { - utils.SetFailureConditions(smClientTypes.CREATE, "Failed to create instance (test)", createdInstance) + utils.SetFailureConditions(smClientTypes.CREATE, "Failed to create instance (test)", createdInstance, false) updateInstanceStatus(ctx, createdInstance) - binding := createBindingWithBlockedError(ctx, bindingName, bindingTestNamespace, instanceName, "binding-external-name", "is not usable") - utils.SetSuccessConditions(smClientTypes.CREATE, createdInstance) + + binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "", false) + Expect(err).ToNot(HaveOccurred()) + waitForResourceCondition(ctx, binding, common.ConditionSucceeded, metav1.ConditionFalse, "", "is not usable") + + utils.SetSuccessConditions(smClientTypes.CREATE, createdInstance, false) updateInstanceStatus(ctx, createdInstance) waitForResourceToBeReady(ctx, binding) }) @@ -578,16 +557,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) - utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "", createdInstance) + utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "", createdInstance, false) createdInstance.Status.OperationURL = "/1234" createdInstance.Status.OperationType = smClientTypes.CREATE updateInstanceStatus(ctx, createdInstance) - createdBinding, err := createBindingWithoutAssertionsAndWait(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "", false) + createdBinding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "", false) Expect(err).ToNot(HaveOccurred()) Expect(utils.IsInProgress(createdBinding)).To(BeTrue()) - utils.SetSuccessConditions(smClientTypes.CREATE, createdInstance) + utils.SetSuccessConditions(smClientTypes.CREATE, createdInstance, false) createdInstance.Status.OperationType = "" createdInstance.Status.OperationURL = "" updateInstanceStatus(ctx, createdInstance) @@ -610,7 +589,7 @@ stringData: newKey: {{ .credentials.secret_key }} tags: {{ .instance.tags }}`) - createdBinding, err := createBindingWithoutAssertionsAndWait(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, true) + createdBinding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, true) Expect(err).ToNot(HaveOccurred()) Expect(isResourceReady(createdBinding)).To(BeTrue()) By("Verify binding secret created") @@ -632,7 +611,7 @@ stringData: newKey: {{ .credentials.secret_key }} tags: {{ .instance.tags }}`) - createdBinding, err := createBindingWithoutAssertionsAndWait(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, true) + createdBinding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, true) Expect(err).ToNot(HaveOccurred()) Expect(isResourceReady(createdBinding)).To(BeTrue()) By("Verify binding secret created") @@ -649,7 +628,9 @@ stringData: kind: Secret metadata: name: my-secret-name`) - createBindingWithError(ctx, bindingName, bindingTestNamespace, instanceName, "", "the Secret template is invalid: Secret's metadata field", secretTemplate) + binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, false) + Expect(err).ToNot(HaveOccurred()) + waitForResourceCondition(ctx, binding, common.ConditionFailed, metav1.ConditionTrue, "", "the Secret template is invalid: Secret's metadata field") }) It("should fail to create the secret if wrong template key in the spec.secretTemplate is provided", func() { ctx := context.Background() @@ -659,7 +640,7 @@ stringData: stringData: foo: {{ .non_existing_key }}`) - binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate) + binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, false) Expect(err).To(BeNil()) bindingLookupKey := getResourceNamespacedName(binding) Eventually(func() bool { @@ -675,7 +656,9 @@ stringData: secretTemplate := dedent.Dedent(` apiVersion: v1 kind: Pod`) - createBindingWithError(ctx, bindingName, bindingTestNamespace, instanceName, "", "but needs to be of kind 'Secret'", secretTemplate) + binding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, false) + Expect(err).ToNot(HaveOccurred()) + waitForResourceCondition(ctx, binding, common.ConditionFailed, metav1.ConditionTrue, "", "but needs to be of kind 'Secret'") }) It("should succeed to create the secret- empty data", func() { ctx := context.Background() @@ -689,7 +672,7 @@ metadata: instance_name: {{ .instance.instance_name }} stringData:`) - createdBinding, err := createBindingWithoutAssertionsAndWait(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, true) + createdBinding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, true) Expect(err).ToNot(HaveOccurred()) Expect(isResourceReady(createdBinding)).To(BeTrue()) By("Verify binding secret created") @@ -722,7 +705,7 @@ metadata: annotations: instance_name: {{ .instance.instance_name }}`) - createdBinding, err := createBindingWithoutAssertionsAndWait(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, true) + createdBinding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, true) Expect(err).ToNot(HaveOccurred()) Expect(isResourceReady(createdBinding)).To(BeTrue()) By("Verify binding secret created") @@ -761,7 +744,7 @@ stringData: newKey: {{ .credentials.auth.basic.password }} tags: {{ .instance.tags }}`) - createdBinding, err := createBindingWithoutAssertionsAndWait(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, true) + createdBinding, err := createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "", secretTemplate, true) Expect(err).ToNot(HaveOccurred()) Expect(isResourceReady(createdBinding)).To(BeTrue()) By("Verify binding secret created") @@ -786,7 +769,7 @@ metadata: instance_name: {{ .instance.instance_name }} stringData: newKey: {{ .credentials.secret_key }}`) - createdBinding = createBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", secretTemplate) + createdBinding = createAndValidateBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", secretTemplate) fakeClient.GetBindingByIDReturns(&smClientTypes.ServiceBinding{ID: fakeBindingID, Credentials: json.RawMessage("{\"secret_key\": \"secret_value\"}")}, nil) Expect(isResourceReady(createdBinding)).To(BeTrue()) }) @@ -922,14 +905,9 @@ stringData: }) Context("Delete", func() { - deleteAndValidate := func(binding *v1.ServiceBinding) { - deleteAndWait(ctx, getResourceNamespacedName(createdBinding), &v1.ServiceBinding{}) - err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.Spec.SecretName, Namespace: binding.Namespace}, &corev1.Secret{}) - Expect(apierrors.IsNotFound(err)).To(BeTrue()) - } BeforeEach(func() { - createdBinding = createBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "") + createdBinding = createAndValidateBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "") Expect(isResourceReady(createdBinding)).To(BeTrue()) }) @@ -939,7 +917,7 @@ stringData: fakeClient.UnbindReturns("", nil) }) It("should delete the k8s binding and secret", func() { - deleteAndValidate(createdBinding) + deleteAndWait(ctx, createdBinding) }) }) @@ -964,7 +942,7 @@ stringData: }) It("recovers the binding and delete the k8s binding and secret", func() { - deleteAndValidate(createdBinding) + deleteAndWait(ctx, createdBinding) }) }) @@ -975,7 +953,7 @@ stringData: }) AfterEach(func() { fakeClient.UnbindReturns("", nil) - deleteAndValidate(createdBinding) + deleteAndWait(ctx, createdBinding) }) It("should not remove finalizer and keep the secret", func() { @@ -1000,7 +978,7 @@ stringData: }) It("should eventually succeed", func() { - deleteAndValidate(createdBinding) + deleteAndWait(ctx, createdBinding) }) }) @@ -1009,7 +987,7 @@ stringData: fakeClient.UnbindReturns("", nil) }) It("should succeed", func() { - createdBinding, err := createBindingWithoutAssertions(ctx, bindingName+"-new", bindingTestNamespace, "non-exist-instance", "", "binding-external-name", "") + createdBinding, err := createBindingWithoutAssertions(ctx, bindingName+"-new", bindingTestNamespace, "non-exist-instance", "", "binding-external-name", "", false) Expect(err).ToNot(HaveOccurred()) createdBinding.Finalizers = []string{common.FinalizerName} Expect(k8sClient.Update(ctx, createdBinding)) @@ -1021,7 +999,7 @@ stringData: cond := meta.FindStatusCondition(createdBinding.GetConditions(), common.ConditionSucceeded) return cond != nil && cond.Reason == common.Blocked }, timeout, interval).Should(BeTrue()) - deleteAndValidate(createdBinding) + deleteAndWait(ctx, createdBinding) }) }) }) @@ -1038,7 +1016,7 @@ stringData: It("should delete the k8s binding and secret", func() { Expect(k8sClient.Delete(ctx, createdBinding)).To(Succeed()) - deleteAndValidate(createdBinding) + deleteAndWait(ctx, createdBinding) }) }) @@ -1063,7 +1041,7 @@ stringData: return failedCond != nil && strings.Contains(failedCond.Message, errorMessage) }, timeout, interval).Should(BeTrue()) fakeClient.UnbindReturns("", nil) - deleteAndValidate(createdBinding) + deleteAndWait(ctx, createdBinding) }) }) @@ -1085,7 +1063,7 @@ stringData: return cond != nil && strings.Contains(cond.Message, string(smClientTypes.INPROGRESS)) }, timeout, interval).Should(BeTrue()) fakeClient.UnbindReturns("", nil) - deleteAndValidate(createdBinding) + deleteAndWait(ctx, createdBinding) }) }) }) @@ -1128,7 +1106,7 @@ stringData: When(fmt.Sprintf("last operation is %s %s", testCase.lastOpType, testCase.lastOpState), func() { It("should resync status", func() { var err error - createdBinding, err = createBindingWithoutAssertionsAndWait(ctx, bindingName, bindingTestNamespace, instanceName, "", "fake-binding-external-name", "", false) + createdBinding, err = createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "fake-binding-external-name", "", false) Expect(err).ToNot(HaveOccurred()) smCallArgs := fakeClient.ListBindingsArgsForCall(0) Expect(smCallArgs.LabelQuery).To(HaveLen(1)) @@ -1180,7 +1158,7 @@ stringData: It("should resync successfully", func() { var err error - createdBinding, err = createBindingWithoutAssertionsAndWait(ctx, bindingName, bindingTestNamespace, instanceName, "", "fake-binding-external-name", "", false) + createdBinding, err = createBindingWithoutAssertions(ctx, bindingName, bindingTestNamespace, instanceName, "", "fake-binding-external-name", "", false) Expect(err).ToNot(HaveOccurred()) }) }) @@ -1189,7 +1167,7 @@ stringData: Context("Credential Rotation", func() { BeforeEach(func() { fakeClient.RenameBindingReturns(nil, nil) - createdBinding = createBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "") + createdBinding = createAndValidateBinding(ctx, bindingName, bindingTestNamespace, instanceName, "", "binding-external-name", "") fakeClient.ListBindingsStub = func(params *sm.Parameters) (*smClientTypes.ServiceBindings, error) { if params == nil || params.FieldQuery == nil || len(params.FieldQuery) == 0 { return nil, nil @@ -1343,21 +1321,13 @@ stringData: Context("Cross Namespace", func() { var crossBinding *v1.ServiceBinding - var paramsSecret *corev1.Secret + var serviceInstanceInAnotherNamespace *v1.ServiceInstance BeforeEach(func() { - paramsSecret = &corev1.Secret{} - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: "param-secret"}, paramsSecret) - if apierrors.IsNotFound(err) { - createParamsSecret(testNamespace) - } else { - Expect(err).ToNot(HaveOccurred()) - } + serviceInstanceInAnotherNamespace = createInstance(ctx, instanceName, testNamespace, instanceExternalName) }) - AfterEach(func() { - deleteAndWait(ctx, types.NamespacedName{Namespace: testNamespace, Name: "param-secret"}, &corev1.Secret{}) + deleteAndWait(ctx, serviceInstanceInAnotherNamespace) }) - When("binding is created in a different namespace than the instance", func() { AfterEach(func() { if crossBinding != nil { @@ -1365,7 +1335,7 @@ stringData: } }) It("should succeed", func() { - crossBinding = createBinding(ctx, bindingName, testNamespace, instanceName, bindingTestNamespace, "cross-binding-external-name", "") + crossBinding = createAndValidateBinding(ctx, bindingName, bindingTestNamespace, instanceName, testNamespace, "cross-binding-external-name", "") By("Verify binding secret created") getSecret(ctx, createdBinding.Spec.SecretName, createdBinding.Namespace, true) @@ -1375,7 +1345,7 @@ stringData: Context("cred rotation", func() { BeforeEach(func() { fakeClient.RenameBindingReturns(nil, nil) - crossBinding = createBinding(ctx, bindingName, testNamespace, instanceName, bindingTestNamespace, "cross-binding-external-name", "") + crossBinding = createAndValidateBinding(ctx, bindingName, bindingTestNamespace, instanceName, testNamespace, "cross-binding-external-name", "") fakeClient.ListBindingsStub = func(params *sm.Parameters) (*smClientTypes.ServiceBindings, error) { if params == nil || params.FieldQuery == nil || len(params.FieldQuery) == 0 { return nil, nil @@ -1402,12 +1372,12 @@ stringData: }) AfterEach(func() { if crossBinding != nil { - Expect(k8sClient.Delete(ctx, crossBinding)) + deleteAndWait(ctx, crossBinding) } }) It("should rotate the credentials and create old binding", func() { - key := types.NamespacedName{Name: bindingName, Namespace: testNamespace} + key := types.NamespacedName{Name: bindingName, Namespace: bindingTestNamespace} Expect(k8sClient.Get(ctx, key, crossBinding)).To(Succeed()) crossBinding.Spec.CredRotationPolicy = &v1.CredentialsRotationPolicy{ Enabled: true, @@ -1417,7 +1387,7 @@ stringData: var secret *corev1.Secret Eventually(func() bool { - secret = getSecret(ctx, crossBinding.Spec.SecretName, testNamespace, true) + secret = getSecret(ctx, crossBinding.Spec.SecretName, bindingTestNamespace, true) secret.Data = map[string][]byte{} return k8sClient.Update(ctx, secret) == nil }, timeout, interval).Should(BeTrue()) @@ -1430,19 +1400,19 @@ stringData: return err == nil && myBinding.Status.LastCredentialsRotationTime != nil && len(myBinding.Status.Conditions) == 2 }, timeout, interval).Should(BeTrue()) - secret = getSecret(ctx, myBinding.Spec.SecretName, testNamespace, true) + secret = getSecret(ctx, myBinding.Spec.SecretName, bindingTestNamespace, true) val := secret.Data["secret_key"] Expect(string(val)).To(Equal("secret_value")) bindingList := &v1.ServiceBindingList{} Eventually(func() bool { - Expect(k8sClient.List(ctx, bindingList, client.MatchingLabels{common.StaleBindingIDLabel: myBinding.Status.BindingID}, client.InNamespace(testNamespace))).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()) oldBinding := bindingList.Items[0] Expect(oldBinding.Spec.CredRotationPolicy.Enabled).To(BeFalse()) - secret = getSecret(ctx, oldBinding.Spec.SecretName, testNamespace, true) + secret = getSecret(ctx, oldBinding.Spec.SecretName, bindingTestNamespace, true) val = secret.Data["secret_key2"] Expect(string(val)).To(Equal("secret_value2")) }) @@ -1464,7 +1434,7 @@ func generateBasicBindingTemplate(name, namespace, instanceName, instanceNamespa binding.Spec.ParametersFrom = []v1.ParametersFromSource{ { SecretKeyRef: &v1.SecretKeyReference{ - Name: "param-secret", + Name: "binding-params-secret", Key: "secret-parameter", }, }, diff --git a/controllers/serviceinstance_controller.go b/controllers/serviceinstance_controller.go index 2cb1d798..d207295b 100644 --- a/controllers/serviceinstance_controller.go +++ b/controllers/serviceinstance_controller.go @@ -18,13 +18,12 @@ package controllers import ( "context" - "crypto/md5" - "encoding/hex" "encoding/json" "fmt" "net/http" + "strings" - "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/predicate" "github.com/SAP/sap-btp-service-operator/api/common" "github.com/SAP/sap-btp-service-operator/internal/config" @@ -81,6 +80,9 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ } serviceInstance = serviceInstance.DeepCopy() + if utils.IsMarkedForDeletion(serviceInstance.ObjectMeta) { + return r.deleteInstance(ctx, serviceInstance) + } if len(serviceInstance.GetConditions()) == 0 { err := utils.InitConditions(ctx, r.Client, serviceInstance) if err != nil { @@ -100,12 +102,6 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, nil } - if utils.IsMarkedForDeletion(serviceInstance.ObjectMeta) { - // delete updates the generation - serviceInstance.SetObservedGeneration(serviceInstance.Generation) - return r.deleteInstance(ctx, serviceInstance) - } - if len(serviceInstance.Status.OperationURL) > 0 { // ongoing operation - poll status from SM return r.poll(ctx, serviceInstance) @@ -119,9 +115,6 @@ 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) if err != nil { log.Error(err, "failed to get sm client") @@ -145,38 +138,33 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ // Update if updateRequired(serviceInstance) { - if res, err := r.updateInstance(ctx, smClient, serviceInstance); err != nil { - log.Info("got error while trying to update instance") - return ctrl.Result{}, err - } else if res.Requeue { - return res, nil - } + return r.updateInstance(ctx, smClient, serviceInstance) } - // Handle instance share if needed - if sharingUpdateRequired(serviceInstance) { + // share/unshare + if shareOrUnshareRequired(serviceInstance) { return r.handleInstanceSharing(ctx, serviceInstance, smClient) } + log.Info("No action required") return ctrl.Result{}, nil } func (r *ServiceInstanceReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&v1.ServiceInstance{}). - WithOptions(controller.Options{RateLimiter: workqueue.NewItemExponentialFailureRateLimiter(r.Config.RetryBaseDelay, r.Config.RetryMaxDelay)}). - Complete(r) + WithOptions(controller.Options{RateLimiter: workqueue.NewItemExponentialFailureRateLimiter(r.Config.RetryBaseDelay, r.Config.RetryMaxDelay)}).Complete(r) } func (r *ServiceInstanceReconciler) createInstance(ctx context.Context, smClient sm.Client, serviceInstance *v1.ServiceInstance) (ctrl.Result, error) { log := utils.GetLogger(ctx) log.Info("Creating instance in SM") updateHashedSpecValue(serviceInstance) - _, instanceParameters, err := utils.BuildSMRequestParameters(serviceInstance.Namespace, serviceInstance.Spec.ParametersFrom, serviceInstance.Spec.Parameters) + instanceParameters, err := r.buildSMRequestParameters(ctx, serviceInstance) 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 utils.MarkAsNonTransientError(ctx, r.Client, smClientTypes.CREATE, err, serviceInstance) + return utils.MarkAsTransientError(ctx, r.Client, smClientTypes.CREATE, err, serviceInstance) } provision, provisionErr := smClient.Provision(&smClientTypes.ServiceInstance{ @@ -211,14 +199,14 @@ 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 - utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "", serviceInstance) + utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "", serviceInstance, false) 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)) - utils.SetSuccessConditions(smClientTypes.CREATE, serviceInstance) + utils.SetSuccessConditions(smClientTypes.CREATE, serviceInstance, false) return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceInstance) } @@ -226,14 +214,13 @@ func (r *ServiceInstanceReconciler) updateInstance(ctx context.Context, smClient log := utils.GetLogger(ctx) log.Info(fmt.Sprintf("updating instance %s in SM", serviceInstance.Status.InstanceID)) - updateHashedSpecValue(serviceInstance) - - _, instanceParameters, err := utils.BuildSMRequestParameters(serviceInstance.Namespace, serviceInstance.Spec.ParametersFrom, serviceInstance.Spec.Parameters) + instanceParameters, err := r.buildSMRequestParameters(ctx, serviceInstance) if err != nil { log.Error(err, "failed to parse instance parameters") - return utils.MarkAsNonTransientError(ctx, r.Client, smClientTypes.UPDATE, err, serviceInstance) + return utils.MarkAsTransientError(ctx, r.Client, smClientTypes.UPDATE, err, serviceInstance) } + updateHashedSpecValue(serviceInstance) _, operationURL, err := smClient.UpdateInstance(serviceInstance.Status.InstanceID, &smClientTypes.ServiceInstance{ Name: serviceInstance.Spec.ExternalName, ServicePlanID: serviceInstance.Spec.ServicePlanID, @@ -249,8 +236,8 @@ func (r *ServiceInstanceReconciler) updateInstance(ctx context.Context, smClient log.Info(fmt.Sprintf("Update request accepted, operation URL: %s", operationURL)) serviceInstance.Status.OperationURL = operationURL serviceInstance.Status.OperationType = smClientTypes.UPDATE - utils.SetInProgressConditions(ctx, smClientTypes.UPDATE, "", serviceInstance) - + utils.SetInProgressConditions(ctx, smClientTypes.UPDATE, "", serviceInstance, false) + serviceInstance.Status.ForceReconcile = false if err := utils.UpdateStatus(ctx, r.Client, serviceInstance); err != nil { return ctrl.Result{}, err } @@ -258,13 +245,15 @@ func (r *ServiceInstanceReconciler) updateInstance(ctx context.Context, smClient return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, nil } log.Info("Instance updated successfully") - utils.SetSuccessConditions(smClientTypes.UPDATE, serviceInstance) + utils.SetSuccessConditions(smClientTypes.UPDATE, serviceInstance, false) + serviceInstance.Status.ForceReconcile = false return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceInstance) } func (r *ServiceInstanceReconciler) deleteInstance(ctx context.Context, serviceInstance *v1.ServiceInstance) (ctrl.Result, error) { log := utils.GetLogger(ctx) + log.Info("deleting instance") if controllerutil.ContainsFinalizer(serviceInstance, common.FinalizerName) { smClient, err := r.GetSMClient(ctx, serviceInstance) if err != nil { @@ -280,11 +269,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 - utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "delete after recovery", serviceInstance) + utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "delete after recovery", serviceInstance, false) return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceInstance) } log.Info("instance does not exists in SM, removing finalizer") - return ctrl.Result{}, utils.RemoveFinalizer(ctx, r.Client, serviceInstance, common.FinalizerName) + return ctrl.Result{}, utils.RemoveFinalizer(ctx, r.Client, serviceInstance, common.FinalizerName, serviceInstance.GetControllerName()) } if len(serviceInstance.Status.OperationURL) > 0 && serviceInstance.Status.OperationType == smClientTypes.DELETE { @@ -306,7 +295,7 @@ 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{}, utils.RemoveFinalizer(ctx, r.Client, serviceInstance, common.FinalizerName) + return ctrl.Result{}, utils.RemoveFinalizer(ctx, r.Client, serviceInstance, common.FinalizerName, serviceInstance.GetControllerName()) } return ctrl.Result{}, nil } @@ -315,8 +304,8 @@ func (r *ServiceInstanceReconciler) handleInstanceSharing(ctx context.Context, s log := utils.GetLogger(ctx) log.Info("Handling change in instance sharing") - if serviceInstance.ShouldBeShared() { - log.Info("Service instance is shouldBeShared, sharing the instance") + if serviceInstance.GetShared() { + log.Info("Service instance appears to be unshared, sharing the instance") err := smClient.ShareInstance(serviceInstance.Status.InstanceID, utils.BuildUserInfo(ctx, serviceInstance.Spec.UserInfo)) if err != nil { log.Error(err, "failed to share instance") @@ -325,7 +314,7 @@ func (r *ServiceInstanceReconciler) handleInstanceSharing(ctx context.Context, s log.Info("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") + log.Info("Service instance appears to be shared, un-sharing the instance") err := smClient.UnShareInstance(serviceInstance.Status.InstanceID, utils.BuildUserInfo(ctx, serviceInstance.Spec.UserInfo)) if err != nil { log.Error(err, "failed to un-share instance") @@ -357,9 +346,9 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *v 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) - utils.SetInProgressConditions(ctx, serviceInstance.Status.OperationType, string(smClientTypes.INPROGRESS), serviceInstance) + utils.SetInProgressConditions(ctx, serviceInstance.Status.OperationType, string(smClientTypes.INPROGRESS), serviceInstance, false) // if failed to read operation status we cleanup the status to trigger re-sync from SM - freshStatus := v1.ServiceInstanceStatus{Conditions: serviceInstance.GetConditions(), ObservedGeneration: serviceInstance.Generation} + freshStatus := v1.ServiceInstanceStatus{Conditions: serviceInstance.GetConditions()} if utils.IsMarkedForDeletion(serviceInstance.ObjectMeta) { freshStatus.InstanceID = serviceInstance.Status.InstanceID } @@ -380,7 +369,7 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *v case smClientTypes.PENDING: if len(status.Description) > 0 { log.Info(fmt.Sprintf("last operation description is '%s'", status.Description)) - utils.SetInProgressConditions(ctx, status.Type, status.Description, serviceInstance) + utils.SetInProgressConditions(ctx, status.Type, status.Description, serviceInstance, true) if err := utils.UpdateStatus(ctx, r.Client, serviceInstance); err != nil { log.Error(err, "unable to update ServiceInstance polling description") return ctrl.Result{}, err @@ -389,7 +378,7 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *v return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, nil case smClientTypes.FAILED: errMsg := getErrorMsgFromLastOperation(status) - utils.SetFailureConditions(status.Type, errMsg, serviceInstance) + utils.SetFailureConditions(status.Type, errMsg, serviceInstance, true) // in order to delete eventually the object we need return with error if serviceInstance.Status.OperationType == smClientTypes.DELETE { serviceInstance.Status.OperationURL = "" @@ -412,11 +401,11 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *v 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 := utils.RemoveFinalizer(ctx, r.Client, serviceInstance, common.FinalizerName); err != nil { + if err := utils.RemoveFinalizer(ctx, r.Client, serviceInstance, common.FinalizerName, serviceInstance.GetControllerName()); err != nil { return ctrl.Result{}, err } } - utils.SetSuccessConditions(status.Type, serviceInstance) + utils.SetSuccessConditions(status.Type, serviceInstance, true) } serviceInstance.Status.OperationURL = "" @@ -428,7 +417,7 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *v func (r *ServiceInstanceReconciler) handleAsyncDelete(ctx context.Context, serviceInstance *v1.ServiceInstance, opURL string) (ctrl.Result, error) { serviceInstance.Status.OperationURL = opURL serviceInstance.Status.OperationType = smClientTypes.DELETE - utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "", serviceInstance) + utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "", serviceInstance, false) if err := utils.UpdateStatus(ctx, r.Client, serviceInstance); err != nil { return ctrl.Result{}, err @@ -467,14 +456,6 @@ func (r *ServiceInstanceReconciler) recover(ctx context.Context, smClient sm.Cli log.Info(fmt.Sprintf("found existing instance in SM with id %s, updating status", smInstance.ID)) updateHashedSpecValue(k8sInstance) - // set observed generation to 0 because we dont know which generation the current state in SM represents, - // unless the generation is 1 and SM is in the same state as operator - if k8sInstance.Generation == 1 { - k8sInstance.SetObservedGeneration(1) - } else { - k8sInstance.SetObservedGeneration(0) - } - if smInstance.Ready { k8sInstance.Status.Ready = metav1.ConditionTrue } @@ -509,11 +490,11 @@ 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 - utils.SetInProgressConditions(ctx, smInstance.LastOperation.Type, smInstance.LastOperation.Description, k8sInstance) + utils.SetInProgressConditions(ctx, smInstance.LastOperation.Type, smInstance.LastOperation.Description, k8sInstance, false) case smClientTypes.SUCCEEDED: - utils.SetSuccessConditions(operationType, k8sInstance) + utils.SetSuccessConditions(operationType, k8sInstance, false) case smClientTypes.FAILED: - utils.SetFailureConditions(operationType, description, k8sInstance) + utils.SetFailureConditions(operationType, description, k8sInstance, false) } return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, k8sInstance) @@ -546,18 +527,67 @@ func (r *ServiceInstanceReconciler) handleInstanceSharingError(ctx context.Conte return ctrl.Result{Requeue: isTransient}, utils.UpdateStatus(ctx, r.Client, object) } +func (r *ServiceInstanceReconciler) buildSMRequestParameters(ctx context.Context, serviceInstance *v1.ServiceInstance) ([]byte, error) { + log := utils.GetLogger(ctx) + instanceParameters, paramSecrets, err := utils.BuildSMRequestParameters(serviceInstance.Namespace, serviceInstance.Spec.Parameters, serviceInstance.Spec.ParametersFrom) + if err != nil { + log.Error(err, "failed to build instance parameters") + return nil, err + } + instanceLabelsChanged := false + instanceLabels := make(map[string]string) + if serviceInstance.IsSubscribedToSecretChange() { + // find all new secrets on the instance + for secretUID := range paramSecrets { + secret := paramSecrets[secretUID] + instanceLabels[common.InstanceSecretRefLabel+secretUID] = secret.Name + if _, ok := serviceInstance.Labels[common.InstanceSecretRefLabel+secretUID]; !ok { + log.Info(fmt.Sprintf("adding secret watch for secret %s", secret.Name)) + instanceLabelsChanged = true + if err := utils.LabelSecretForWatch(ctx, r.Client, secret); err != nil { + log.Error(err, fmt.Sprintf("failed to mark secret for watch %s", secretUID)) + return nil, err + } + } + } + } + + //sync instance labels + for key := range serviceInstance.Labels { + if strings.HasPrefix(key, common.InstanceSecretRefLabel) { + if _, ok := instanceLabels[key]; !ok { + instanceLabelsChanged = true + } + } else { + // this label not related to secrets, add it + instanceLabels[key] = serviceInstance.Labels[key] + } + } + if instanceLabelsChanged { + serviceInstance.Labels = instanceLabels + log.Info("updating instance with secret labels") + return instanceParameters, r.Client.Update(ctx, serviceInstance) + } + + return instanceParameters, nil +} + func isFinalState(ctx context.Context, serviceInstance *v1.ServiceInstance) bool { log := utils.GetLogger(ctx) - if utils.IsMarkedForDeletion(serviceInstance.ObjectMeta) { - log.Info("instance is not in final state, it is marked for deletion") + + if serviceInstance.Status.ForceReconcile { + log.Info("instance is not in final state, ForceReconcile is true") return false } + if len(serviceInstance.Status.OperationURL) > 0 { log.Info(fmt.Sprintf("instance is not in final state, async operation is in progress (%s)", serviceInstance.Status.OperationURL)) return false } - if serviceInstance.Generation != serviceInstance.GetObservedGeneration() { - log.Info(fmt.Sprintf("instance is not in final state, generation: %d, observedGen: %d", serviceInstance.Generation, serviceInstance.GetObservedGeneration())) + + observedGen := common.GetObservedGeneration(serviceInstance) + if serviceInstance.Generation != observedGen { + log.Info(fmt.Sprintf("instance is not in final state, generation: %d, observedGen: %d", serviceInstance.Generation, observedGen)) return false } @@ -567,7 +597,7 @@ func isFinalState(ctx context.Context, serviceInstance *v1.ServiceInstance) bool return false } - if sharingUpdateRequired(serviceInstance) { + if shareOrUnshareRequired(serviceInstance) { log.Info("instance is not in final state, need to sync sharing status") if len(serviceInstance.Status.HashedSpec) == 0 { updateHashedSpecValue(serviceInstance) @@ -585,40 +615,40 @@ func updateRequired(serviceInstance *v1.ServiceInstance) bool { return false } + if serviceInstance.Status.ForceReconcile { + return true + } + cond := meta.FindStatusCondition(serviceInstance.Status.Conditions, common.ConditionSucceeded) if cond != nil && cond.Reason == common.UpdateInProgress { //in case of transient error occurred return true } - return getSpecHash(serviceInstance) != serviceInstance.Status.HashedSpec + return serviceInstance.GetSpecHash() != serviceInstance.Status.HashedSpec } -func sharingUpdateRequired(serviceInstance *v1.ServiceInstance) bool { +func shareOrUnshareRequired(serviceInstance *v1.ServiceInstance) bool { //relevant only for non-shared instances - sharing instance is possible only for usable instances if serviceInstance.Status.Ready != metav1.ConditionTrue { return false } sharedCondition := meta.FindStatusCondition(serviceInstance.GetConditions(), common.ConditionShared) - shouldBeShared := serviceInstance.ShouldBeShared() - if sharedCondition == nil { - return shouldBeShared + return serviceInstance.GetShared() } if sharedCondition.Reason == common.ShareNotSupported { return false } - if sharedCondition.Reason == common.InProgress || sharedCondition.Reason == common.ShareFailed || sharedCondition.Reason == common.UnShareFailed { - return true - } - - if shouldBeShared { - return sharedCondition.Status == metav1.ConditionFalse + if sharedCondition.Status == metav1.ConditionFalse { + // instance does not appear to be shared, should share it if shared is requested + return serviceInstance.GetShared() } - return sharedCondition.Status == metav1.ConditionTrue + // instance appears to be shared, should unshare it if shared is not requested + return !serviceInstance.GetShared() } func getOfferingTags(smClient sm.Client, planID string) ([]string, error) { @@ -661,19 +691,6 @@ func getTags(tags []byte) ([]string, error) { return tagsArr, nil } -func getSpecHash(serviceInstance *v1.ServiceInstance) string { - spec := serviceInstance.Spec - spec.Shared = ptr.To(false) - specBytes, _ := json.Marshal(spec) - s := string(specBytes) - return generateEncodedMD5Hash(s) -} - -func generateEncodedMD5Hash(str string) string { - hash := md5.Sum([]byte(str)) - return hex.EncodeToString(hash[:]) -} - func setSharedCondition(object common.SAPBTPResource, status metav1.ConditionStatus, reason, msg string) { conditions := object.GetConditions() // align all conditions to latest generation @@ -700,7 +717,7 @@ func setSharedCondition(object common.SAPBTPResource, status metav1.ConditionSta } func updateHashedSpecValue(serviceInstance *v1.ServiceInstance) { - serviceInstance.Status.HashedSpec = getSpecHash(serviceInstance) + serviceInstance.Status.HashedSpec = serviceInstance.GetSpecHash() } func getErrorMsgFromLastOperation(status *smClientTypes.Operation) string { @@ -721,3 +738,7 @@ func getErrorMsgFromLastOperation(status *smClientTypes.Operation) string { } return errMsg } + +type SecretPredicate struct { + predicate.Funcs +} diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index 01fb7399..3b3d306a 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -6,6 +6,8 @@ import ( "net/http" "strings" + "sigs.k8s.io/controller-runtime/pkg/client" + authv1 "k8s.io/api/authentication/v1" "github.com/SAP/sap-btp-service-operator/api/common" @@ -42,11 +44,10 @@ const ( var _ = Describe("ServiceInstance controller", func() { var ( - ctx context.Context - serviceInstance *v1.ServiceInstance fakeInstanceName string defaultLookupKey types.NamespacedName + paramsSecret *corev1.Secret ) instanceSpec := v1.ServiceInstanceSpec{ @@ -59,7 +60,7 @@ var _ = Describe("ServiceInstance controller", func() { ParametersFrom: []v1.ParametersFromSource{ { SecretKeyRef: &v1.SecretKeyReference{ - Name: "param-secret", + Name: "instance-params-secret", Key: "secret-parameter", }, }, @@ -77,21 +78,21 @@ var _ = Describe("ServiceInstance controller", func() { ParametersFrom: []v1.ParametersFromSource{ { SecretKeyRef: &v1.SecretKeyReference{ - Name: "param-secret", + Name: "instance-params-secret", Key: "secret-parameter", }, }, }, } - createInstance := func(ctx context.Context, instanceSpec v1.ServiceInstanceSpec, annotations map[string]string, waitForReady bool) *v1.ServiceInstance { + createInstance := func(ctx context.Context, instanceName string, instanceSpec v1.ServiceInstanceSpec, annotations map[string]string, waitForReady bool) *v1.ServiceInstance { instance := &v1.ServiceInstance{ TypeMeta: metav1.TypeMeta{ APIVersion: "services.cloud.sap.com/v1", Kind: "ServiceInstance", }, ObjectMeta: metav1.ObjectMeta{ - Name: fakeInstanceName, + Name: instanceName, Namespace: testNamespace, Annotations: annotations, }, @@ -138,18 +139,14 @@ var _ = Describe("ServiceInstance controller", func() { fakeClient.DeprovisionReturns("", nil) fakeClient.GetInstanceByIDReturns(&smclientTypes.ServiceInstance{ID: fakeInstanceID, Ready: true, LastOperation: &smClientTypes.Operation{State: smClientTypes.SUCCEEDED, Type: smClientTypes.CREATE}}, nil) - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: "param-secret"}, &corev1.Secret{}) - if apierrors.IsNotFound(err) { - createParamsSecret(testNamespace) - } else { - Expect(err).ToNot(HaveOccurred()) - } + paramsSecret = createParamsSecret(ctx, "instance-params-secret", testNamespace) }) AfterEach(func() { if serviceInstance != nil { - deleteInstance(ctx, serviceInstance, true) + deleteAndWait(ctx, serviceInstance) } + deleteAndWait(ctx, paramsSecret) }) Describe("Create", func() { @@ -204,7 +201,7 @@ var _ = Describe("ServiceInstance controller", func() { }) It("provisioning should fail", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) waitForInstanceConditionAndMessage(ctx, defaultLookupKey, common.ConditionSucceeded, "provided plan id does not match") }) }) @@ -214,21 +211,20 @@ var _ = Describe("ServiceInstance controller", func() { Context("Sync", func() { When("provision request to SM succeeds", func() { It("should provision instance of the provided offering and plan name successfully", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) Expect(serviceInstance.Status.InstanceID).To(Equal(fakeInstanceID)) Expect(serviceInstance.Status.SubaccountID).To(Equal(fakeSubaccountID)) Expect(serviceInstance.Spec.ExternalName).To(Equal(fakeInstanceExternalName)) Expect(serviceInstance.Name).To(Equal(fakeInstanceName)) Expect(serviceInstance.Status.HashedSpec).To(Not(BeNil())) Expect(string(serviceInstance.Spec.Parameters.Raw)).To(ContainSubstring("\"key\":\"value\"")) - Expect(serviceInstance.Status.HashedSpec).To(Equal(getSpecHash(serviceInstance))) + Expect(serviceInstance.Status.HashedSpec).To(Equal(serviceInstance.GetSpecHash())) smInstance, _, _, _, _, _ := fakeClient.ProvisionArgsForCall(0) params := smInstance.Parameters Expect(params).To(ContainSubstring("\"key\":\"value\"")) Expect(params).To(ContainSubstring("\"secret-key\":\"secret-value\"")) }) }) - When("provision request to SM fails", func() { errMessage := "failed to provision instance" @@ -242,7 +238,7 @@ var _ = Describe("ServiceInstance controller", func() { }) It("should have failure condition", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionTrue, common.Created, "") }) }) @@ -255,7 +251,7 @@ var _ = Describe("ServiceInstance controller", func() { Description: errMessage, }) fakeClient.ProvisionReturnsOnCall(1, &sm.ProvisionResponse{InstanceID: fakeInstanceID}, nil) - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionTrue, common.Created, "") }) It("provision fails until timeout", func() { @@ -263,7 +259,7 @@ var _ = Describe("ServiceInstance controller", func() { StatusCode: http.StatusBadRequest, Description: errMessage, }) - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateInProgress, errMessage) }) }) @@ -278,7 +274,7 @@ var _ = Describe("ServiceInstance controller", func() { }) It("should retry until success", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionTrue, common.Created, "") }) }) @@ -290,7 +286,7 @@ var _ = Describe("ServiceInstance controller", func() { }) It("should be transient error and eventually succeed", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateInProgress, tooManyRequestsError) fakeClient.ProvisionReturns(&sm.ProvisionResponse{InstanceID: fakeInstanceID}, nil) waitForResourceToBeReady(ctx, serviceInstance) @@ -303,7 +299,7 @@ var _ = Describe("ServiceInstance controller", func() { fakeClient.ProvisionReturnsOnCall(2, &sm.ProvisionResponse{InstanceID: fakeInstanceID}, nil) }) It("should fail first and then succeed", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionTrue, common.Created, "") Expect(fakeClient.ProvisionCallCount()).To(BeNumerically(">", 1)) }) @@ -326,7 +322,7 @@ var _ = Describe("ServiceInstance controller", func() { fakeClient.GetInstanceByIDReturns(&smclientTypes.ServiceInstance{Labels: map[string][]string{"subaccount_id": {fakeSubaccountID}}}, nil) }) It("should update in progress condition and provision the instance successfully", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) fakeClient.StatusReturns(&smclientTypes.Operation{ ID: "1234", Type: smClientTypes.CREATE, @@ -339,7 +335,7 @@ var _ = Describe("ServiceInstance controller", func() { When("polling ends with failure", func() { It("should update to failure condition with the broker err description", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) fakeClient.StatusReturns(&smclientTypes.Operation{ ID: "1234", Type: smClientTypes.CREATE, @@ -352,7 +348,7 @@ var _ = Describe("ServiceInstance controller", func() { When("updating during create", func() { It("should update the instance after created successfully", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateInProgress, "") newName := "new-name" + uuid.New().String() @@ -380,7 +376,7 @@ var _ = Describe("ServiceInstance controller", func() { When("deleting while create is in progress", func() { It("should be deleted successfully", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) By("waiting for instance to be CreateInProgress") waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateInProgress, "") @@ -412,7 +408,7 @@ var _ = Describe("ServiceInstance controller", func() { ServicePlanName: "a-plan-name", ServiceOfferingName: "an-offering-name", } - serviceInstance = createInstance(ctx, withoutExternal, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, withoutExternal, nil, true) Expect(serviceInstance.Status.InstanceID).To(Equal(fakeInstanceID)) Expect(serviceInstance.Spec.ExternalName).To(Equal(fakeInstanceName)) Expect(serviceInstance.Name).To(Equal(fakeInstanceName)) @@ -422,7 +418,7 @@ var _ = Describe("ServiceInstance controller", func() { Describe("Update", func() { BeforeEach(func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) Expect(serviceInstance.Spec.ExternalName).To(Equal(fakeInstanceExternalName)) }) @@ -611,7 +607,7 @@ var _ = Describe("ServiceInstance controller", func() { When("subaccount id changed", func() { It("should fail", func() { deleteInstance(ctx, serviceInstance, true) - serviceInstance = createInstance(ctx, instanceSpec, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) serviceInstance.Spec.BTPAccessCredentialsSecret = "12345" err := k8sClient.Update(ctx, serviceInstance) Expect(err).To(HaveOccurred()) @@ -634,7 +630,7 @@ var _ = Describe("ServiceInstance controller", func() { Describe("Delete", func() { BeforeEach(func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) fakeClient.DeprovisionReturns("", nil) }) AfterEach(func() { @@ -783,7 +779,7 @@ var _ = Describe("ServiceInstance controller", func() { Describe("full reconcile", func() { When("instance hashedSpec is not initialized", func() { BeforeEach(func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) }) It("should not send update request and update the hashed spec", func() { hashed := serviceInstance.Status.HashedSpec @@ -814,7 +810,7 @@ var _ = Describe("ServiceInstance controller", func() { }) It("should call correctly to SM and recover the instance", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) Expect(fakeClient.ProvisionCallCount()).To(Equal(0)) Expect(serviceInstance.Status.InstanceID).To(Equal(fakeInstanceID)) smCallArgs := fakeClient.ListInstancesArgsForCall(0) @@ -837,12 +833,12 @@ var _ = Describe("ServiceInstance controller", func() { }) It("should recover the existing instance and poll until instance is ready", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) key := getResourceNamespacedName(serviceInstance) Eventually(func() bool { _ = k8sClient.Get(ctx, key, serviceInstance) return serviceInstance.Status.InstanceID == fakeInstanceID - }, timeout, interval).Should(BeTrue(), eventuallyMsgForResource("service instance id not recovered", key, serviceInstance)) + }, timeout, interval).Should(BeTrue(), eventuallyMsgForResource("service instance id not recovered", serviceInstance)) Expect(fakeClient.ProvisionCallCount()).To(Equal(0)) Expect(fakeClient.ListInstancesCallCount()).To(BeNumerically(">", 0)) fakeClient.StatusReturns(&smclientTypes.Operation{ResourceID: fakeInstanceID, State: smClientTypes.SUCCEEDED, Type: smClientTypes.CREATE}, nil) @@ -858,7 +854,7 @@ var _ = Describe("ServiceInstance controller", func() { }) It("should recover the existing instance and update condition failure", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateFailed, "") Expect(serviceInstance.Status.InstanceID).To(Equal(fakeInstanceID)) Expect(fakeClient.ProvisionCallCount()).To(Equal(0)) @@ -876,7 +872,7 @@ var _ = Describe("ServiceInstance controller", func() { recoveredInstance.Ready = true }) It("should recover the instance with status Ready=true", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) waitForResourceToBeReady(ctx, serviceInstance) Expect(fakeClient.ProvisionCallCount()).To(Equal(0)) Expect(serviceInstance.Status.InstanceID).To(Equal(fakeInstanceID)) @@ -887,7 +883,7 @@ var _ = Describe("ServiceInstance controller", func() { recoveredInstance.Ready = false }) It("should recover the instance with status Ready=false", func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, false) waitForResourceCondition(ctx, serviceInstance, common.ConditionFailed, metav1.ConditionTrue, common.CreateFailed, "") Expect(fakeClient.ProvisionCallCount()).To(Equal(0)) Expect(serviceInstance.Status.InstanceID).To(Equal(fakeInstanceID)) @@ -904,14 +900,14 @@ var _ = Describe("ServiceInstance controller", func() { When("creating instance with shared=true", func() { It("should succeed to provision and sharing the instance", func() { fakeClient.ShareInstanceReturns(nil) - serviceInstance = createInstance(ctx, sharedInstanceSpec, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, sharedInstanceSpec, nil, true) waitForInstanceToBeShared(ctx, serviceInstance) }) }) Context("sharing an existing instance", func() { BeforeEach(func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) }) When("updating existing instance to shared", func() { @@ -952,7 +948,7 @@ var _ = Describe("ServiceInstance controller", func() { StatusCode: http.StatusBadRequest, Description: "errMessage", }) - serviceInstance = createInstance(ctx, sharedInstanceSpec, nil, false) + serviceInstance = createInstance(ctx, fakeInstanceName, sharedInstanceSpec, nil, false) waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateInProgress, "") Expect(fakeClient.ShareInstanceCallCount()).To(BeZero()) }) @@ -960,7 +956,7 @@ var _ = Describe("ServiceInstance controller", func() { When("instance is valid and share failed", func() { BeforeEach(func() { - serviceInstance = createInstance(ctx, instanceSpec, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) }) When("shared failed with rate limit error", func() { @@ -1022,7 +1018,7 @@ var _ = Describe("ServiceInstance controller", func() { Context("un-sharing an existing shared instance", func() { BeforeEach(func() { fakeClient.ShareInstanceReturns(nil) - serviceInstance = createInstance(ctx, sharedInstanceSpec, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, sharedInstanceSpec, nil, true) waitForInstanceToBeShared(ctx, serviceInstance) }) @@ -1061,7 +1057,7 @@ var _ = Describe("ServiceInstance controller", func() { When("instance is valid and un-share failed", func() { BeforeEach(func() { fakeClient.ShareInstanceReturns(nil) - serviceInstance = createInstance(ctx, sharedInstanceSpec, nil, true) + serviceInstance = createInstance(ctx, fakeInstanceName, sharedInstanceSpec, nil, true) waitForInstanceToBeShared(ctx, serviceInstance) fakeClient.UnShareInstanceReturns(&sm.ServiceManagerError{ StatusCode: http.StatusBadRequest, @@ -1136,6 +1132,9 @@ var _ = Describe("ServiceInstance controller", func() { When("async operation in progress", func() { It("should return false", func() { var instance = &v1.ServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 2, + }, Status: v1.ServiceInstanceStatus{ Conditions: []metav1.Condition{ { @@ -1144,9 +1143,8 @@ var _ = Describe("ServiceInstance controller", func() { ObservedGeneration: 1, }, }, - HashedSpec: "929e78f4449f8036ce39da3cc3e7eaea", - OperationURL: "/operations/somepollingurl", - ObservedGeneration: 2, + HashedSpec: "929e78f4449f8036ce39da3cc3e7eaea", + OperationURL: "/operations/somepollingurl", }, Spec: v1.ServiceInstanceSpec{ ExternalName: "name", @@ -1195,7 +1193,7 @@ var _ = Describe("ServiceInstance controller", func() { { Type: common.ConditionSucceeded, Status: metav1.ConditionTrue, - ObservedGeneration: 2, + ObservedGeneration: 1, }, }, HashedSpec: "bla", @@ -1243,6 +1241,9 @@ var _ = Describe("ServiceInstance controller", func() { When("in final state", func() { It("should return true", func() { var instance = &v1.ServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 2, + }, Status: v1.ServiceInstanceStatus{ Conditions: []metav1.Condition{ { @@ -1260,8 +1261,7 @@ var _ = Describe("ServiceInstance controller", func() { Status: metav1.ConditionTrue, }, }, - HashedSpec: "929e78f4449f8036ce39da3cc3e7eaea", - ObservedGeneration: 2, + HashedSpec: "929e78f4449f8036ce39da3cc3e7eaea", }, Spec: v1.ServiceInstanceSpec{ ExternalName: "name", @@ -1273,6 +1273,243 @@ var _ = Describe("ServiceInstance controller", func() { }) }) }) + + Context("secret watcher", func() { + When("secret updated and instance watch secret", func() { + anotherInstanceName := "instance2" + var anotherInstance *v1.ServiceInstance + var anotherSecret *corev1.Secret + BeforeEach(func() { + instanceSpec.SubscribeToSecretChanges = pointer.Bool(true) + }) + AfterEach(func() { + instanceSpec.SubscribeToSecretChanges = pointer.Bool(false) + if anotherInstance != nil { + deleteAndWait(ctx, anotherInstance) + } + if anotherSecret != nil { + deleteAndWait(ctx, anotherSecret) + } + }) + It("should update instance with the secret change", func() { + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) + smInstance, _, _, _, _, _ := fakeClient.ProvisionArgsForCall(0) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"secret-value\""}) + + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{serviceInstance}) + + credentialsMap := make(map[string][]byte) + credentialsMap["secret-parameter"] = []byte("{\"secret-key\":\"new-secret-value\"}") + paramsSecret.Data = credentialsMap + Expect(k8sClient.Update(ctx, paramsSecret)).To(Succeed()) + Eventually(func() bool { + return fakeClient.UpdateInstanceCallCount() == 1 + }, timeout*3, interval).Should(BeTrue(), "expected condition was not met") + + _, smInstance, _, _, _, _, _ = fakeClient.UpdateInstanceArgsForCall(0) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"new-secret-value\""}) + deleteAndWait(ctx, serviceInstance) + + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{}) + }) + It("should update instance with the secret change and secret have labels", func() { + paramsSecret.Labels = map[string]string{"label": "value"} + Expect(k8sClient.Update(ctx, paramsSecret)).To(Succeed()) + + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) + smInstance, _, _, _, _, _ := fakeClient.ProvisionArgsForCall(0) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"secret-value\""}) + + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{serviceInstance}) + + credentialsMap := make(map[string][]byte) + credentialsMap["secret-parameter"] = []byte("{\"secret-key\":\"new-secret-value\"}") + paramsSecret.Data = credentialsMap + Expect(k8sClient.Update(ctx, paramsSecret)).To(Succeed()) + Eventually(func() bool { + return fakeClient.UpdateInstanceCallCount() == 1 + }, timeout*3, interval).Should(BeTrue(), "expected condition was not met") + + _, smInstance, _, _, _, _, _ = fakeClient.UpdateInstanceArgsForCall(0) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"new-secret-value\""}) + deleteAndWait(ctx, serviceInstance) + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{}) + Expect(paramsSecret.Labels["label"]).To(Equal("value")) + + }) + It("create instance before secret", func() { + newInstanceSpec := v1.ServiceInstanceSpec{ + ExternalName: fakeInstanceExternalName, + ServicePlanName: fakePlanName, + ServiceOfferingName: fakeOfferingName, + Parameters: &runtime.RawExtension{ + Raw: []byte(`{"key": "value"}`), + }, + ParametersFrom: []v1.ParametersFromSource{ + { + SecretKeyRef: &v1.SecretKeyReference{ + Name: "instance-params-secret-new", + Key: "secret-parameter", + }, + }, + }, + SubscribeToSecretChanges: pointer.Bool(true), + } + serviceInstance = createInstance(ctx, anotherInstanceName, newInstanceSpec, nil, false) + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.CreateInProgress, "secrets \"instance-params-secret-new\" not found") + Expect(fakeClient.ProvisionCallCount()).To(Equal(0)) + + anotherSecret = createParamsSecret(ctx, "instance-params-secret-new", testNamespace) + waitForResourceToBeReady(ctx, serviceInstance) + smInstance, _, _, _, _, _ := fakeClient.ProvisionArgsForCall(0) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"secret-value\""}) + + checkSecretAnnotationsAndLabels(ctx, k8sClient, anotherSecret, []*v1.ServiceInstance{serviceInstance}) + + credentialsMap := make(map[string][]byte) + credentialsMap["secret-parameter"] = []byte("{\"secret-key\":\"new-secret-value\"}") + anotherSecret.Data = credentialsMap + Expect(k8sClient.Update(ctx, anotherSecret)).To(Succeed()) + Eventually(func() bool { + return fakeClient.UpdateInstanceCallCount() == 1 + }, timeout*3, interval).Should(BeTrue(), "expected condition was not met") + + _, smInstance, _, _, _, _, _ = fakeClient.UpdateInstanceArgsForCall(0) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"new-secret-value\""}) + deleteAndWait(ctx, serviceInstance) + checkSecretAnnotationsAndLabels(ctx, k8sClient, anotherSecret, []*v1.ServiceInstance{}) + }) + It("update instance parameterFrom", func() { + credentialsMap := make(map[string][]byte) + credentialsMap["secret-parameter2"] = []byte("{\"secret-key2\":\"secret-value2\"}") + anotherSecret = createSecret(ctx, "instance-params-secret-new", testNamespace, credentialsMap) + + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) + smInstance, _, _, _, _, _ := fakeClient.ProvisionArgsForCall(0) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"secret-value\""}) + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{serviceInstance}) + + // update instance parametersFrom + serviceInstance.Spec.ParametersFrom = []v1.ParametersFromSource{ + { + SecretKeyRef: &v1.SecretKeyReference{ + Name: "instance-params-secret", + Key: "secret-parameter", + }, + }, + { + SecretKeyRef: &v1.SecretKeyReference{ + Name: "instance-params-secret-new", + Key: "secret-parameter2", + }, + }, + } + serviceInstance = updateInstance(ctx, serviceInstance) + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionTrue, common.Updated, "") + _, smInstance, _, _, _, _, _ = fakeClient.UpdateInstanceArgsForCall(0) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"secret-value\"", "\"secret-key2\":\"secret-value2\""}) + checkSecretAnnotationsAndLabels(ctx, k8sClient, anotherSecret, []*v1.ServiceInstance{serviceInstance}) + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{serviceInstance}) + + // update instance parametersFrom + serviceInstance.Spec.ParametersFrom = []v1.ParametersFromSource{ + { + SecretKeyRef: &v1.SecretKeyReference{ + Name: "instance-params-secret", + Key: "secret-parameter", + }, + }, + } + serviceInstance = updateInstance(ctx, serviceInstance) + Eventually(func() bool { + return fakeClient.UpdateInstanceCallCount() > 1 + }, timeout*3, interval).Should(BeTrue(), + "dkd", + ) + _, smInstance, _, _, _, _, _ = fakeClient.UpdateInstanceArgsForCall(1) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"secret-value\""}) + checkSecretAnnotationsAndLabels(ctx, k8sClient, anotherSecret, []*v1.ServiceInstance{}) + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{serviceInstance}) + + Expect(serviceInstance.Labels[common.InstanceSecretRefLabel+string(anotherSecret.GetUID())]).To(BeEmpty()) + + }) + It("should update two instances with the secret change", func() { + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) + smInstance, _, _, _, _, _ := fakeClient.ProvisionArgsForCall(0) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"secret-value\""}) + + anotherInstance = createInstance(ctx, anotherInstanceName, instanceSpec, nil, true) + smInstance, _, _, _, _, _ = fakeClient.ProvisionArgsForCall(1) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"secret-value\""}) + + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{serviceInstance, anotherInstance}) + + credentialsMap := make(map[string][]byte) + credentialsMap["secret-parameter"] = []byte("{\"secret-key\":\"new-secret-value\"}") + paramsSecret.Data = credentialsMap + Expect(k8sClient.Update(ctx, paramsSecret)).To(Succeed()) + Eventually(func() bool { + return fakeClient.UpdateInstanceCallCount() == 2 + }, timeout*3, interval).Should(BeTrue(), "expected condition was not met") + + _, smInstance, _, _, _, _, _ = fakeClient.UpdateInstanceArgsForCall(0) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"new-secret-value\""}) + + _, smInstance, _, _, _, _, _ = fakeClient.UpdateInstanceArgsForCall(1) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"new-secret-value\""}) + + deleteAndWait(ctx, anotherInstance) + + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{serviceInstance}) + }) + It("delete of secret when secret is watched", func() { + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) + Expect(k8sClient.Get(ctx, getResourceNamespacedName(paramsSecret), paramsSecret)).To(Succeed()) + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{serviceInstance}) + + deleteAndWait(ctx, paramsSecret) + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionFalse, common.UpdateInProgress, "secrets \"instance-params-secret\" not found") + }) + }) + When("secret updated and instance don't watch secret", func() { + AfterEach(func() { + instanceSpec.SubscribeToSecretChanges = pointer.Bool(false) + }) + It("should not update instance with the secret change", func() { + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) + smInstance, _, _, _, _, _ := fakeClient.ProvisionArgsForCall(0) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"secret-value\""}) + + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{}) + + credentialsMap := make(map[string][]byte) + credentialsMap["secret-parameter"] = []byte("{\"secret-key\":\"new-secret-value\"}") + paramsSecret.Data = credentialsMap + Expect(k8sClient.Update(ctx, paramsSecret)).To(Succeed()) + Expect(fakeClient.UpdateInstanceCallCount()).To(Equal(0)) + }) + It("should not update instance with the secret change after removing SubscribeToSecretChanges", func() { + instanceSpec.SubscribeToSecretChanges = pointer.Bool(true) + serviceInstance = createInstance(ctx, fakeInstanceName, instanceSpec, nil, true) + smInstance, _, _, _, _, _ := fakeClient.ProvisionArgsForCall(0) + checkParams(string(smInstance.Parameters), []string{"\"key\":\"value\"", "\"secret-key\":\"secret-value\""}) + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{serviceInstance}) + + serviceInstance.Spec.SubscribeToSecretChanges = pointer.Bool(false) + updateInstance(ctx, serviceInstance) + waitForResourceCondition(ctx, serviceInstance, common.ConditionSucceeded, metav1.ConditionTrue, common.Updated, "") + checkSecretAnnotationsAndLabels(ctx, k8sClient, paramsSecret, []*v1.ServiceInstance{}) + + credentialsMap := make(map[string][]byte) + credentialsMap["secret-parameter"] = []byte("{\"secret-key\":\"new-secret-value\"}") + paramsSecret.Data = credentialsMap + Expect(k8sClient.Update(ctx, paramsSecret)).To(Succeed()) + //Expect(fakeClient.UpdateInstanceCallCount()).To(Equal(1)) + }) + }) + + }) }) func waitForInstanceConditionAndMessage(ctx context.Context, key types.NamespacedName, conditionType, msg string) { @@ -1334,3 +1571,32 @@ func updateInstanceStatus(ctx context.Context, instance *v1.ServiceInstance) *v1 }, timeout, interval).Should(BeTrue()) return si } + +func checkSecretAnnotationsAndLabels(ctx context.Context, k8sClient client.Client, paramsSecret *corev1.Secret, instances []*v1.ServiceInstance) { + + if len(instances) == 0 { + Expect(k8sClient.Get(ctx, getResourceNamespacedName(paramsSecret), paramsSecret)).To(Succeed()) + // Add an data to wake up the secret + paramsSecret.Data["secret-parameter2"] = []byte("{\"secret-key\":\"new-secret-value\"}") + // Update the secret in the Kubernetes cluster + Expect(k8sClient.Update(ctx, paramsSecret)).To(Succeed()) + Eventually(func() bool { + Expect(k8sClient.Get(ctx, getResourceNamespacedName(paramsSecret), paramsSecret)).To(Succeed()) + return len(paramsSecret.Labels[common.WatchSecretLabel]) == 0 && len(paramsSecret.Finalizers) == 0 + }, timeout, interval).Should(BeTrue()) + } else { + Expect(k8sClient.Get(ctx, getResourceNamespacedName(paramsSecret), paramsSecret)).To(Succeed()) + for _, instance := range instances { + Expect(k8sClient.Get(ctx, getResourceNamespacedName(instance), instance)).To(Succeed()) + Expect(instance.Labels[common.InstanceSecretRefLabel+string(paramsSecret.GetUID())]).To(Equal(paramsSecret.Name)) + } + Expect(paramsSecret.Finalizers[0]).To(Equal(common.FinalizerName)) + Expect(paramsSecret.Labels[common.WatchSecretLabel]).To(Equal("true")) + } +} + +func checkParams(params string, substrings []string) { + for _, substring := range substrings { + Expect(params).To(ContainSubstring(substring)) + } +} diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 211eaaad..743648c9 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -178,6 +178,13 @@ var _ = BeforeSuite(func(done Done) { }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) + err = (&SecretReconciler{ + Client: k8sManager.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("Secret"), + Scheme: k8sManager.GetScheme(), + }).SetupWithManager(k8sManager) + Expect(err).ToNot(HaveOccurred()) + // +kubebuilder:scaffold:webhook ctx, cancel = context.WithCancel(context.TODO()) @@ -230,7 +237,7 @@ var _ = AfterSuite(func() { }) func isResourceReady(resource common.SAPBTPResource) bool { - return resource.GetObservedGeneration() == resource.GetGeneration() && + return common.GetObservedGeneration(resource) == resource.GetGeneration() && meta.IsStatusConditionPresentAndEqual(resource.GetConditions(), common.ConditionReady, metav1.ConditionTrue) } @@ -245,7 +252,7 @@ func waitForResourceCondition(ctx context.Context, resource common.SAPBTPResourc return false } - if resource.GetObservedGeneration() != resource.GetGeneration() { + if common.GetObservedGeneration(resource) != resource.GetGeneration() { return false } @@ -268,10 +275,7 @@ func waitForResourceCondition(ctx context.Context, resource common.SAPBTPResourc return true }, timeout*3, interval).Should(BeTrue(), - eventuallyMsgForResource( - fmt.Sprintf("expected condition: {type: %s, status: %s, reason: %s, message: %s} was not met", conditionType, status, reason, message), - key, - resource), + eventuallyMsgForResource(fmt.Sprintf("expected condition: {type: %s, status: %s, reason: %s, message: %s} was not met. %v", conditionType, status, reason, message, resource.GetConditions()), resource), ) } @@ -279,51 +283,48 @@ func getResourceNamespacedName(resource client.Object) types.NamespacedName { return types.NamespacedName{Namespace: resource.GetNamespace(), Name: resource.GetName()} } -func deleteAndWait(ctx context.Context, key types.NamespacedName, resource client.Object) { - wait := true +func deleteAndWait(ctx context.Context, resource client.Object) { Eventually(func() bool { - if err := k8sClient.Get(ctx, key, resource); err != nil { + if err := k8sClient.Get(ctx, getResourceNamespacedName(resource), resource); err != nil { if apierrors.IsNotFound(err) { - wait = false return true } return false } - if err := k8sClient.Delete(ctx, resource); err != nil { - if apierrors.IsNotFound(err) { - wait = false - return true - } - return false - } - - return true - }, timeout, interval).Should(BeTrue(), eventuallyMsgForResource("failed to mark for deletion", key, resource)) - - if wait { - waitForResourceToBeDeleted(ctx, key, resource) - } + err := k8sClient.Delete(ctx, resource) + return apierrors.IsNotFound(err) + }, timeout, interval).Should(BeTrue(), eventuallyMsgForResource("failed to delete resource", resource)) } func waitForResourceToBeDeleted(ctx context.Context, key types.NamespacedName, resource client.Object) { Eventually(func() bool { return apierrors.IsNotFound(k8sClient.Get(ctx, key, resource)) - }, timeout, interval).Should(BeTrue(), eventuallyMsgForResource("resource is not deleted", key, resource)) + }, timeout, interval).Should(BeTrue(), eventuallyMsgForResource("resource is not deleted", resource)) } -func createParamsSecret(namespace string) { +func createParamsSecret(ctx context.Context, secretName, namespace string) *corev1.Secret { credentialsMap := make(map[string][]byte) credentialsMap["secret-parameter"] = []byte("{\"secret-key\":\"secret-value\"}") + return createSecret(ctx, secretName, namespace, credentialsMap) +} + +func createSecret(ctx context.Context, secretName string, namespace string, credentialsMap map[string][]byte) *corev1.Secret { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "param-secret", + Name: secretName, Namespace: namespace, }, Data: credentialsMap, } - Expect(k8sClient.Create(context.Background(), secret)).ToNot(HaveOccurred()) + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: secretName, Namespace: namespace}, secret) + return err == nil + }, timeout, interval).Should(BeTrue()) + return secret } func printSection(str string) { @@ -359,7 +360,7 @@ func getTransientBrokerError(errorMessage string) error { } } -func eventuallyMsgForResource(message string, key types.NamespacedName, resource client.Object) string { +func eventuallyMsgForResource(message string, resource client.Object) string { gvk, _ := apiutil.GVKForObject(resource, scheme.Scheme) - return fmt.Sprintf("eventaully failure for %s %s. message: %s", gvk.Kind, key.String(), message) + return fmt.Sprintf("eventaully failure for %s %s. message: %s", gvk.Kind, getResourceNamespacedName(resource), message) } diff --git a/internal/utils/condition_utils.go b/internal/utils/condition_utils.go index 6d82cc40..21174963 100644 --- a/internal/utils/condition_utils.go +++ b/internal/utils/condition_utils.go @@ -14,7 +14,7 @@ import ( func InitConditions(ctx context.Context, k8sClient client.Client, obj common.SAPBTPResource) error { obj.SetReady(metav1.ConditionFalse) - SetInProgressConditions(ctx, smClientTypes.CREATE, "Pending", obj) + SetInProgressConditions(ctx, smClientTypes.CREATE, "Pending", obj, false) return UpdateStatus(ctx, k8sClient, obj) } @@ -55,7 +55,7 @@ func GetConditionReason(opType smClientTypes.OperationCategory, state smClientTy return common.Unknown } -func SetInProgressConditions(ctx context.Context, operationType smClientTypes.OperationCategory, message string, object common.SAPBTPResource) { +func SetInProgressConditions(ctx context.Context, operationType smClientTypes.OperationCategory, message string, object common.SAPBTPResource, isAsyncOperation bool) { log := GetLogger(ctx) if len(message) == 0 { if operationType == smClientTypes.CREATE { @@ -71,12 +71,16 @@ func SetInProgressConditions(ctx context.Context, operationType smClientTypes.Op if len(conditions) > 0 { meta.RemoveStatusCondition(&conditions, common.ConditionFailed) } + observedGen := object.GetGeneration() + if isAsyncOperation { + observedGen = getLastObservedGen(object) + } lastOpCondition := metav1.Condition{ Type: common.ConditionSucceeded, Status: metav1.ConditionFalse, Reason: GetConditionReason(operationType, smClientTypes.INPROGRESS), Message: message, - ObservedGeneration: object.GetObservedGeneration(), + ObservedGeneration: observedGen, } meta.SetStatusCondition(&conditions, lastOpCondition) meta.SetStatusCondition(&conditions, getReadyCondition(object)) @@ -85,7 +89,7 @@ func SetInProgressConditions(ctx context.Context, operationType smClientTypes.Op 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) { +func SetSuccessConditions(operationType smClientTypes.OperationCategory, object common.SAPBTPResource, isAsyncOperation bool) { var message string if operationType == smClientTypes.CREATE { message = fmt.Sprintf("%s provisioned successfully", object.GetControllerName()) @@ -99,19 +103,23 @@ func SetSuccessConditions(operationType smClientTypes.OperationCategory, object if len(conditions) > 0 { meta.RemoveStatusCondition(&conditions, common.ConditionFailed) } + observedGen := object.GetGeneration() + if isAsyncOperation { + observedGen = getLastObservedGen(object) + } lastOpCondition := metav1.Condition{ Type: common.ConditionSucceeded, Status: metav1.ConditionTrue, Reason: GetConditionReason(operationType, smClientTypes.SUCCEEDED), Message: message, - ObservedGeneration: object.GetObservedGeneration(), + ObservedGeneration: observedGen, } readyCondition := metav1.Condition{ Type: common.ConditionReady, Status: metav1.ConditionTrue, Reason: common.Provisioned, Message: message, - ObservedGeneration: object.GetObservedGeneration(), + ObservedGeneration: observedGen, } meta.SetStatusCondition(&conditions, lastOpCondition) meta.SetStatusCondition(&conditions, readyCondition) @@ -130,13 +138,13 @@ func SetCredRotationInProgressConditions(reason, message string, object common.S Status: metav1.ConditionTrue, Reason: reason, Message: message, - ObservedGeneration: object.GetObservedGeneration(), + ObservedGeneration: object.GetGeneration(), } meta.SetStatusCondition(&conditions, credRotCondition) object.SetConditions(conditions) } -func SetFailureConditions(operationType smClientTypes.OperationCategory, errorMessage string, object common.SAPBTPResource) { +func SetFailureConditions(operationType smClientTypes.OperationCategory, errorMessage string, object common.SAPBTPResource, isAsyncOperation bool) { var message string if operationType == smClientTypes.CREATE { message = fmt.Sprintf("%s create failed: %s", object.GetControllerName(), errorMessage) @@ -153,23 +161,27 @@ func SetFailureConditions(operationType smClientTypes.OperationCategory, errorMe reason = object.GetConditions()[0].Reason } + observedGen := object.GetGeneration() + if isAsyncOperation { + observedGen = getLastObservedGen(object) + } conditions := object.GetConditions() lastOpCondition := metav1.Condition{ Type: common.ConditionSucceeded, Status: metav1.ConditionFalse, Reason: reason, Message: message, - ObservedGeneration: object.GetObservedGeneration(), + ObservedGeneration: observedGen, } - meta.SetStatusCondition(&conditions, lastOpCondition) - failedCondition := metav1.Condition{ Type: common.ConditionFailed, Status: metav1.ConditionTrue, Reason: reason, Message: message, - ObservedGeneration: object.GetObservedGeneration(), + ObservedGeneration: observedGen, } + + meta.SetStatusCondition(&conditions, lastOpCondition) meta.SetStatusCondition(&conditions, failedCondition) meta.SetStatusCondition(&conditions, getReadyCondition(object)) @@ -180,15 +192,14 @@ func MarkAsNonTransientError(ctx context.Context, k8sClient client.Client, opera log := GetLogger(ctx) errMsg := err.Error() log.Info(fmt.Sprintf("operation %s of %s encountered a non transient error %s, setting failure conditions", operationType, object.GetControllerName(), errMsg)) - SetFailureConditions(operationType, errMsg, object) - object.SetObservedGeneration(object.GetGeneration()) + SetFailureConditions(operationType, errMsg, object, false) return ctrl.Result{}, UpdateStatus(ctx, k8sClient, object) } func MarkAsTransientError(ctx context.Context, k8sClient client.Client, operationType smClientTypes.OperationCategory, err error, object common.SAPBTPResource) (ctrl.Result, error) { log := GetLogger(ctx) log.Info(fmt.Sprintf("operation %s of %s encountered a transient error %s, retrying operation :)", operationType, object.GetControllerName(), err.Error())) - SetInProgressConditions(ctx, operationType, err.Error(), object) + SetInProgressConditions(ctx, operationType, err.Error(), object, false) if updateErr := UpdateStatus(ctx, k8sClient, object); updateErr != nil { return ctrl.Result{}, updateErr } @@ -198,7 +209,7 @@ func MarkAsTransientError(ctx context.Context, k8sClient client.Client, operatio // 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) + SetInProgressConditions(ctx, common.Unknown, message, object, false) lastOpCondition := meta.FindStatusCondition(object.GetConditions(), common.ConditionSucceeded) lastOpCondition.Reason = common.Blocked } @@ -229,3 +240,12 @@ func getReadyCondition(object common.SAPBTPResource) metav1.Condition { return metav1.Condition{Type: common.ConditionReady, Status: status, Reason: reason} } + +func getLastObservedGen(object common.SAPBTPResource) int64 { + conditions := object.GetConditions() + cond := meta.FindStatusCondition(conditions, common.ConditionSucceeded) + if cond != nil { + return cond.ObservedGeneration + } + return 0 +} diff --git a/internal/utils/condition_utils_test.go b/internal/utils/condition_utils_test.go index a044eaf7..db278169 100644 --- a/internal/utils/condition_utils_test.go +++ b/internal/utils/condition_utils_test.go @@ -120,7 +120,7 @@ var _ = Describe("Condition Utils", func() { It("should set in-progress conditions", func() { resource = getBinding() - SetInProgressConditions(ctx, smClientTypes.CREATE, "Pending", resource) + SetInProgressConditions(ctx, smClientTypes.CREATE, "Pending", resource, false) // Add assertions to check the state of the resource after calling SetInProgressConditions Expect(resource.GetConditions()).ToNot(BeEmpty()) @@ -133,7 +133,7 @@ var _ = Describe("Condition Utils", func() { operationType := smClientTypes.CREATE resource = getBinding() - SetSuccessConditions(operationType, resource) + SetSuccessConditions(operationType, resource, false) // Add assertions to check the state of the resource after calling SetSuccessConditions Expect(resource.GetConditions()).ToNot(BeEmpty()) @@ -160,7 +160,7 @@ var _ = Describe("Condition Utils", func() { It("should set failure conditions", func() { operationType := smClientTypes.CREATE errorMessage := "Operation failed" - SetFailureConditions(operationType, errorMessage, resource) + SetFailureConditions(operationType, errorMessage, resource, false) Expect(resource.GetConditions()).ToNot(BeEmpty()) Expect(meta.IsStatusConditionPresentAndEqual(resource.GetConditions(), common.ConditionReady, metav1.ConditionFalse)).To(BeTrue()) }) @@ -304,6 +304,34 @@ var _ = Describe("Condition Utils", func() { Expect(result).Should(BeFalse()) }) }) + + Context("getLastObservedGen", func() { + It("should return the last observed generation from the conditions", func() { + resource := &v1.ServiceBinding{ + Status: v1.ServiceBindingStatus{ + Conditions: []metav1.Condition{ + { + Type: common.ConditionSucceeded, + Status: metav1.ConditionTrue, + ObservedGeneration: 5, + }, + }, + }, + } + + Expect(getLastObservedGen(resource)).To(Equal(int64(5))) + }) + + It("should return 0 if the ConditionSucceeded condition is not present", func() { + resource := &v1.ServiceBinding{ + Status: v1.ServiceBindingStatus{ + Conditions: []metav1.Condition{}, + }, + } + + Expect(getLastObservedGen(resource)).To(Equal(int64(0))) + }) + }) }) func getBinding() *v1.ServiceBinding { diff --git a/internal/utils/controller_util.go b/internal/utils/controller_util.go index d368ae0e..8d44d290 100644 --- a/internal/utils/controller_util.go +++ b/internal/utils/controller_util.go @@ -9,6 +9,8 @@ import ( "strings" "time" + corev1 "k8s.io/api/core/v1" + "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" @@ -42,7 +44,7 @@ type format string type LogKey struct { } -func RemoveFinalizer(ctx context.Context, k8sClient client.Client, object common.SAPBTPResource, finalizerName string) error { +func RemoveFinalizer(ctx context.Context, k8sClient client.Client, object client.Object, finalizerName string, controllerName common.ControllerName) error { log := GetLogger(ctx) if controllerutil.ContainsFinalizer(object, finalizerName) { log.Info(fmt.Sprintf("removing finalizer %s", finalizerName)) @@ -56,7 +58,7 @@ func RemoveFinalizer(ctx context.Context, k8sClient client.Client, object common 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())) + log.Info(fmt.Sprintf("removed finalizer %s from %s", finalizerName, controllerName)) return nil } return nil @@ -146,6 +148,15 @@ func HandleError(ctx context.Context, k8sClient client.Client, operationType smC return MarkAsNonTransientError(ctx, k8sClient, operationType, err, resource) } +// ParseNamespacedName converts a "namespace/name" string to a types.NamespacedName object. +func ParseNamespacedName(input string) (apimachinerytypes.NamespacedName, error) { + parts := strings.SplitN(input, "/", 2) + if len(parts) != 2 { + return apimachinerytypes.NamespacedName{}, fmt.Errorf("invalid format: expected 'namespace/name', got '%s'", input) + } + return apimachinerytypes.NamespacedName{Namespace: parts[0], Name: parts[1]}, nil +} + func handleRateLimitError(smError *sm.ServiceManagerError, log logr.Logger) (ctrl.Result, error) { retryAfterStr := smError.ResponseHeaders.Get("Retry-After") if len(retryAfterStr) > 0 { @@ -235,3 +246,14 @@ func serialize(value interface{}) ([]byte, format, error) { } return data, JSON, nil } + +func LabelSecretForWatch(ctx context.Context, k8sClient client.Client, secret *corev1.Secret) error { + if secret.Labels == nil { + secret.Labels = make(map[string]string) + } + secret.Labels[common.WatchSecretLabel] = "true" + if !controllerutil.ContainsFinalizer(secret, common.FinalizerName) { + controllerutil.AddFinalizer(secret, common.FinalizerName) + } + return k8sClient.Update(ctx, secret) +} diff --git a/internal/utils/controller_util_test.go b/internal/utils/controller_util_test.go index 2b4ede86..f80fb6df 100644 --- a/internal/utils/controller_util_test.go +++ b/internal/utils/controller_util_test.go @@ -4,12 +4,17 @@ import ( "encoding/json" "net/http" + "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" + 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" ) @@ -46,6 +51,7 @@ var _ = Describe("Controller Util", func() { }) }) + Context("SliceContains", func() { It("slice contains", func() { slice := []string{"element1", "element2", "element3"} @@ -178,4 +184,52 @@ var _ = Describe("Controller Util", func() { Expect(got).To(Equal(expected)) }) }) + + Context("ParseNamespacedName", func() { + It("should return correct namespace and name", func() { + nsName, err := ParseNamespacedName(types.NamespacedName{ + Namespace: "namespace", + Name: "name", + }.String()) + Expect(err).ToNot(HaveOccurred()) + Expect(nsName.Namespace).To(Equal("namespace")) + Expect(nsName.Name).To(Equal("name")) + }) + + It("should return error if not a valid namespaced name", func() { + _, err := ParseNamespacedName("namespaceName") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid format: expected 'namespace/name")) + }) + }) + + Context("LabelSecretForWatch", func() { + It("should add the watch label to the secret if it is missing", func() { + // Create a fake client + + // Create a secret without the watch label + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "default", + }, + } + err := k8sClient.Create(ctx, secret) + Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Get(ctx, types.NamespacedName{Name: "test-secret", Namespace: "default"}, secret) + Expect(err).ToNot(HaveOccurred()) + + // Call the function + err = LabelSecretForWatch(ctx, k8sClient, secret) + Expect(err).ToNot(HaveOccurred()) + + // Get the updated secret + updatedSecret := &corev1.Secret{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "test-secret", Namespace: "default"}, updatedSecret) + Expect(err).ToNot(HaveOccurred()) + Expect(updatedSecret.Finalizers[0]).To(Equal(common.FinalizerName)) + Expect(updatedSecret.Labels[common.WatchSecretLabel]).To(Equal("true")) + + }) + }) }) diff --git a/internal/utils/parameters.go b/internal/utils/parameters.go index 7d4b18dc..bbce343e 100644 --- a/internal/utils/parameters.go +++ b/internal/utils/parameters.go @@ -3,7 +3,6 @@ package utils import ( "context" "encoding/json" - "fmt" servicesv1 "github.com/SAP/sap-btp-service-operator/api/v1" @@ -20,14 +19,16 @@ import ( // secret values. // The second return value is parameters marshalled to byt array // The third return value is any error that caused the function to fail. -func BuildSMRequestParameters(namespace string, parametersFrom []servicesv1.ParametersFromSource, parameters *runtime.RawExtension) (map[string]interface{}, []byte, error) { +func BuildSMRequestParameters(namespace string, parameters *runtime.RawExtension, parametersFrom []servicesv1.ParametersFromSource) ([]byte, map[string]*corev1.Secret, error) { params := make(map[string]interface{}) + secretsSet := map[string]*corev1.Secret{} if len(parametersFrom) > 0 { for _, p := range parametersFrom { - fps, err := fetchParametersFromSource(namespace, &p) + fps, secret, err := fetchParametersFromSource(namespace, &p) if err != nil { return nil, nil, err } + secretsSet[string(secret.UID)] = secret for k, v := range fps { // we don't want to add shared param because sm api does not support updating // shared param with other params, for sharing we have different function. @@ -62,7 +63,7 @@ func BuildSMRequestParameters(namespace string, parametersFrom []servicesv1.Para if err != nil { return nil, nil, err } - return params, parametersRaw, nil + return parametersRaw, secretsSet, nil } // UnmarshalRawParameters produces a map structure from a given raw YAML/JSON input @@ -94,31 +95,35 @@ func unmarshalJSON(in []byte) (map[string]interface{}, error) { } // fetchSecretKeyValue requests and returns the contents of the given secret key -func fetchSecretKeyValue(namespace string, secretKeyRef *servicesv1.SecretKeyReference) ([]byte, error) { +func fetchSecretKeyValue(namespace string, secretKeyRef *servicesv1.SecretKeyReference) ([]byte, *corev1.Secret, error) { secret := &corev1.Secret{} err := GetSecretWithFallback(context.Background(), types.NamespacedName{Namespace: namespace, Name: secretKeyRef.Name}, secret) if err != nil { - return nil, err + return nil, nil, err } - return secret.Data[secretKeyRef.Key], nil + + return secret.Data[secretKeyRef.Key], secret, nil } // fetchParametersFromSource fetches data from a specified external source and // represents it in the parameters map format -func fetchParametersFromSource(namespace string, parametersFrom *servicesv1.ParametersFromSource) (map[string]interface{}, error) { +func fetchParametersFromSource(namespace string, parametersFrom *servicesv1.ParametersFromSource) (map[string]interface{}, *corev1.Secret, error) { var params map[string]interface{} if parametersFrom.SecretKeyRef != nil { - data, err := fetchSecretKeyValue(namespace, parametersFrom.SecretKeyRef) + data, secret, err := fetchSecretKeyValue(namespace, parametersFrom.SecretKeyRef) if err != nil { - return nil, err + return nil, nil, err + } + if secret.DeletionTimestamp != nil { + return nil, nil, fmt.Errorf("secret %s is marked for deletion", secret.Name) } p, err := unmarshalJSON(data) if err != nil { - return nil, err + return nil, nil, err } params = p - + return params, secret, nil } - return params, nil + return params, nil, nil } diff --git a/internal/utils/parameters_test.go b/internal/utils/parameters_test.go index d5e34ae0..031db4c3 100644 --- a/internal/utils/parameters_test.go +++ b/internal/utils/parameters_test.go @@ -1,10 +1,15 @@ package utils import ( + "github.com/SAP/sap-btp-service-operator/internal/config" + v1 "github.com/SAP/sap-btp-service-operator/api/v1" . "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/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) var _ = Describe("Parameters", func() { @@ -13,11 +18,11 @@ var _ = Describe("Parameters", func() { var parametersFrom []v1.ParametersFromSource parameters := (*runtime.RawExtension)(nil) - params, rawParam, err := BuildSMRequestParameters("", parametersFrom, parameters) + rawParam, secrets, err := BuildSMRequestParameters("", parameters, parametersFrom) Expect(err).To(BeNil()) - Expect(params).To(BeNil()) Expect(rawParam).To(BeNil()) + Expect(len(secrets)).To(BeZero()) }) It("handles parameters from source", func() { var parametersFrom []v1.ParametersFromSource @@ -25,11 +30,58 @@ var _ = Describe("Parameters", func() { Raw: []byte(`{"key":"value"}`), } - params, rawParam, err := BuildSMRequestParameters("", parametersFrom, parameters) + rawParam, secrets, err := BuildSMRequestParameters("", parameters, parametersFrom) Expect(err).To(BeNil()) - Expect(params).To(Equal(map[string]interface{}{"key": "value"})) Expect(rawParam).To(Equal([]byte(`{"key":"value"}`))) + Expect(len(secrets)).To(BeZero()) + }) + It("handles parameters from source with secrets", func() { + // Setup + namespace := "test-namespace" + parameters := &runtime.RawExtension{Raw: []byte(`{"param1":"value1"}`)} + secretData := map[string][]byte{"secret-parameter": []byte(`{"param2":"value2"}`)} + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "param-secret", + Namespace: namespace, + }, + Data: secretData, + } + parametersFrom := []v1.ParametersFromSource{ + { + SecretKeyRef: &v1.SecretKeyReference{ + Name: "param-secret", + Key: "secret-parameter", + }, + }, + } + + // Create a fake client with the secret + k8sClient := fake.NewClientBuilder().WithObjects(secret).Build() + + // Initialize the secrets client + InitializeSecretsClient(k8sClient, k8sClient, config.Config{ + ManagementNamespace: "management-namespace", + ReleaseNamespace: "release-namespace", + EnableNamespaceSecrets: true, + EnableLimitedCache: true, + }) + + // Test + parametersRaw, secretsSet, err := BuildSMRequestParameters(namespace, parameters, parametersFrom) + + // Assertions + Expect(err).To(BeNil()) + expectedParams := map[string]interface{}{ + "param1": "value1", + "param2": "value2", + } + rawParameters, err := MarshalRawParameters(expectedParams) + Expect(err).To(BeNil()) + Expect(parametersRaw).To(Equal(rawParameters)) + Expect(len(secretsSet)).To(Equal(1)) + Expect(secretsSet[string(secret.UID)]).To(Equal(secret)) }) }) }) diff --git a/internal/utils/secret_resolver.go b/internal/utils/secret_resolver.go index a77f97ac..1ffc1d55 100644 --- a/internal/utils/secret_resolver.go +++ b/internal/utils/secret_resolver.go @@ -61,10 +61,8 @@ func GetSecretForResource(ctx context.Context, namespace, name string) (*v1.Secr func (sr *secretClient) getSecretFromManagementNamespace(ctx context.Context, name string) (*v1.Secret, error) { secretForResource := &v1.Secret{} - sr.Log.Info(fmt.Sprintf("Searching for secret %s in management namespace %s", name, sr.ManagementNamespace)) err := sr.getWithClientFallback(ctx, types.NamespacedName{Name: name, Namespace: sr.ManagementNamespace}, secretForResource) if err != nil { - sr.Log.Info(fmt.Sprintf("Could not fetch secret %s from management namespace %s", name, sr.ManagementNamespace)) return nil, err } return secretForResource, nil @@ -75,14 +73,12 @@ func (sr *secretClient) getSecretForResource(ctx context.Context, namespace, nam // search namespace secret if sr.EnableNamespaceSecrets { - sr.Log.Info(fmt.Sprintf("Searching for secret %s in namespace %s", name, namespace)) err := sr.getWithClientFallback(ctx, types.NamespacedName{Name: name, Namespace: namespace}, secretForResource) if err == nil { return secretForResource, nil } if client.IgnoreNotFound(err) != nil { - sr.Log.Error(err, fmt.Sprintf("Could not fetch secret %s from namespace %s", name, namespace)) return nil, err } } @@ -95,17 +91,15 @@ func (sr *secretClient) getSecretForResource(ctx context.Context, namespace, nam } if client.IgnoreNotFound(err) != nil { - sr.Log.Error(err, fmt.Sprintf("Could not fetch secret %s-%s in the management namespace %s", namespace, name, sr.ManagementNamespace)) return nil, err } - // namespace-specific secret not found in management namespace, fallback to central cluster secret return sr.getClusterDefaultSecret(ctx, name) } func (sr *secretClient) getClusterDefaultSecret(ctx context.Context, name string) (*v1.Secret, error) { secretForResource := &v1.Secret{} - sr.Log.Info(fmt.Sprintf("Searching for cluster secret %s in releaseNamespace %s", name, sr.ReleaseNamespace)) + // sr.Log.Info(fmt.Sprintf("Searching for cluster secret %s in releaseNamespace %s", name, sr.ReleaseNamespace)) err := sr.getWithClientFallback(ctx, types.NamespacedName{Namespace: sr.ReleaseNamespace, Name: name}, secretForResource) if err != nil { sr.Log.Error(err, fmt.Sprintf("Could not fetch cluster secret %s from releaseNamespace %s", name, sr.ReleaseNamespace)) @@ -118,12 +112,10 @@ func (sr *secretClient) getWithClientFallback(ctx context.Context, key types.Nam err := sr.Client.Get(ctx, key, secretForResource) if err != nil { if errors.IsNotFound(err) && sr.LimitedCacheEnabled { - sr.Log.Info(fmt.Sprintf("secret %s not found in cache, falling back to non-cached client", key.String())) err = sr.NonCachedClient.Get(ctx, key, secretForResource) if err != nil { return err } - sr.Log.Info(fmt.Sprintf("secret %s found using non-cached client", key.String())) return nil } return err diff --git a/internal/utils/sm_utils.go b/internal/utils/sm_utils.go index 9d30597d..a2bf9e82 100644 --- a/internal/utils/sm_utils.go +++ b/internal/utils/sm_utils.go @@ -33,6 +33,7 @@ func GetSMClient(ctx context.Context, serviceInstance *v1.ServiceInstance) (sm.C log.Error(err, "failed to get secret for instance") return nil, err } + log.Info(fmt.Sprintf("using secret %s in namespace %s", secret.Name, secret.Namespace)) } clientConfig := &sm.ClientConfig{ @@ -62,7 +63,7 @@ func GetSMClient(ctx context.Context, serviceInstance *v1.ServiceInstance) (sm.C if client.IgnoreNotFound(err) != nil { return nil, err } - + log.Info(fmt.Sprintf("using tls secret %s in namespace %s", secret.Name, secret.Namespace)) if tlsSecret == nil || len(tlsSecret.Data) == 0 || len(tlsSecret.Data[corev1.TLSCertKey]) == 0 || len(tlsSecret.Data[corev1.TLSPrivateKeyKey]) == 0 { log.Info("clientsecret not found in SM credentials, and tls secret is invalid") return nil, &InvalidCredentialsError{} diff --git a/main.go b/main.go index 50bd0c47..c7eff406 100644 --- a/main.go +++ b/main.go @@ -174,6 +174,14 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "ServiceBinding") os.Exit(1) } + if err = (&controllers.SecretReconciler{ + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("Secret"), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "Secret") + os.Exit(1) + } if os.Getenv("ENABLE_WEBHOOKS") != "false" { mgr.GetWebhookServer().Register("/mutate-services-cloud-sap-com-v1-serviceinstance", &webhook.Admission{Handler: &webhooks.ServiceInstanceDefaulter{Decoder: admission.NewDecoder(mgr.GetScheme())}}) mgr.GetWebhookServer().Register("/mutate-services-cloud-sap-com-v1-servicebinding", &webhook.Admission{Handler: &webhooks.ServiceBindingDefaulter{Decoder: admission.NewDecoder(mgr.GetScheme())}}) diff --git a/sapbtp-operator-charts/templates/crd.yml b/sapbtp-operator-charts/templates/crd.yml index 551cbd69..899611fe 100644 --- a/sapbtp-operator-charts/templates/crd.yml +++ b/sapbtp-operator-charts/templates/crd.yml @@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.15.0 + controller-gen.kubebuilder.io/version: v0.16.1 name: servicebindings.services.cloud.sap.com spec: group: services.cloud.sap.com @@ -81,7 +81,6 @@ spec: description: |- Parameters for the binding. - The Parameters field is NOT secret or secured in any way and should NEVER be used to hold sensitive information. To set parameters that contain secret information, you should ALWAYS store that information @@ -198,16 +197,8 @@ spec: conditions: description: Service binding conditions items: - description: "Condition contains details for one aspect of the current - state of this API Resource.\n---\nThis struct is intended for - direct use as an array at the field path .status.conditions. For - example,\n\n\n\ttype FooStatus struct{\n\t // Represents the - observations of a foo's current state.\n\t // Known .status.conditions.type - are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // - +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t - \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" - patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t - \ // other fields\n\t}" + description: Condition contains details for one aspect of the current + state of this API Resource. properties: lastTransitionTime: description: |- @@ -248,12 +239,7 @@ spec: - Unknown type: string type: - description: |- - type of condition in CamelCase or in foo.example.com/CamelCase. - --- - Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be - useful (see .node.status.conditions), the ability to deconflict is important. - The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + description: type of condition in CamelCase or in foo.example.com/CamelCase. maxLength: 316 pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string @@ -365,7 +351,6 @@ spec: description: |- Parameters for the binding. - The Parameters field is NOT secret or secured in any way and should NEVER be used to hold sensitive information. To set parameters that contain secret information, you should ALWAYS store that information @@ -467,16 +452,8 @@ spec: conditions: description: Service binding conditions items: - description: "Condition contains details for one aspect of the current - state of this API Resource.\n---\nThis struct is intended for - direct use as an array at the field path .status.conditions. For - example,\n\n\n\ttype FooStatus struct{\n\t // Represents the - observations of a foo's current state.\n\t // Known .status.conditions.type - are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // - +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t - \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" - patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t - \ // other fields\n\t}" + description: Condition contains details for one aspect of the current + state of this API Resource. properties: lastTransitionTime: description: |- @@ -517,12 +494,7 @@ spec: - Unknown type: string type: - description: |- - type of condition in CamelCase or in foo.example.com/CamelCase. - --- - Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be - useful (see .node.status.conditions), the ability to deconflict is important. - The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + description: type of condition in CamelCase or in foo.example.com/CamelCase. maxLength: 316 pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string @@ -568,7 +540,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.15.0 + controller-gen.kubebuilder.io/version: v0.16.1 name: serviceinstances.services.cloud.sap.com spec: group: services.cloud.sap.com @@ -654,7 +626,6 @@ spec: description: |- Provisioning parameters for the instance. - The Parameters field is NOT secret or secured in any way and should NEVER be used to hold sensitive information. To set parameters that contain secret information, you should ALWAYS store that information @@ -705,6 +676,10 @@ spec: shared: description: Indicates the desired shared state type: boolean + subscribeToSecretChanges: + description: indicate instance will update on secrets from parametersFrom + change + type: boolean userInfo: description: |- UserInfo contains information about the user that last modified this @@ -746,16 +721,8 @@ spec: conditions: description: Service instance conditions items: - description: "Condition contains details for one aspect of the current - state of this API Resource.\n---\nThis struct is intended for - direct use as an array at the field path .status.conditions. For - example,\n\n\n\ttype FooStatus struct{\n\t // Represents the - observations of a foo's current state.\n\t // Known .status.conditions.type - are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // - +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t - \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" - patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t - \ // other fields\n\t}" + description: Condition contains details for one aspect of the current + state of this API Resource. properties: lastTransitionTime: description: |- @@ -796,12 +763,7 @@ spec: - Unknown type: string type: - description: |- - type of condition in CamelCase or in foo.example.com/CamelCase. - --- - Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be - useful (see .node.status.conditions), the ability to deconflict is important. - The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + description: type of condition in CamelCase or in foo.example.com/CamelCase. maxLength: 316 pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string @@ -914,7 +876,6 @@ spec: description: |- Provisioning parameters for the instance. - The Parameters field is NOT secret or secured in any way and should NEVER be used to hold sensitive information. To set parameters that contain secret information, you should ALWAYS store that information @@ -1006,16 +967,8 @@ spec: conditions: description: Service instance conditions items: - description: "Condition contains details for one aspect of the current - state of this API Resource.\n---\nThis struct is intended for - direct use as an array at the field path .status.conditions. For - example,\n\n\n\ttype FooStatus struct{\n\t // Represents the - observations of a foo's current state.\n\t // Known .status.conditions.type - are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // - +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t - \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" - patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t - \ // other fields\n\t}" + description: Condition contains details for one aspect of the current + state of this API Resource. properties: lastTransitionTime: description: |- @@ -1056,12 +1009,7 @@ spec: - Unknown type: string type: - description: |- - type of condition in CamelCase or in foo.example.com/CamelCase. - --- - Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be - useful (see .node.status.conditions), the ability to deconflict is important. - The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + description: type of condition in CamelCase or in foo.example.com/CamelCase. maxLength: 316 pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string