diff --git a/api/v1/servicebinding_types.go b/api/v1/servicebinding_types.go index a3605dc5..86c3e638 100644 --- a/api/v1/servicebinding_types.go +++ b/api/v1/servicebinding_types.go @@ -121,7 +121,7 @@ type ServiceBindingStatus struct { // Indicates when binding secret was rotated LastCredentialsRotationTime *metav1.Time `json:"lastCredentialsRotationTime,omitempty"` - // The subaccount id of the service instance + // The subaccount id of the service binding SubaccountID string `json:"subaccountID,omitempty"` } diff --git a/api/v1/serviceinstance_types.go b/api/v1/serviceinstance_types.go index 81bd98fb..02f2eb38 100644 --- a/api/v1/serviceinstance_types.go +++ b/api/v1/serviceinstance_types.go @@ -83,8 +83,8 @@ type ServiceInstanceSpec struct { // +optional UserInfo *v1.UserInfo `json:"userInfo,omitempty"` - // The desired subaccount id for the service instance - SubaccountID string `json:"subaccountID,omitempty"` + // The name of the btp access credentials secret + BTPAccessCredentialsSecret string `json:"btpAccessCredentialsSecret,omitempty"` } // ServiceInstanceStatus defines the observed state of ServiceInstance diff --git a/api/v1/serviceinstance_validating_webhook.go b/api/v1/serviceinstance_validating_webhook.go index 92f02723..d55d74be 100644 --- a/api/v1/serviceinstance_validating_webhook.go +++ b/api/v1/serviceinstance_validating_webhook.go @@ -41,18 +41,8 @@ var _ webhook.Validator = &ServiceInstance{} // log is for logging in this package. var serviceinstancelog = logf.Log.WithName("serviceinstance-resource") -var allowMultipleTenants bool - -func SetAllowMultipleTenants(isAllowed bool) { - allowMultipleTenants = isAllowed -} func (si *ServiceInstance) ValidateCreate() (warnings admission.Warnings, err error) { - serviceinstancelog.Info("validate create", "name", si.Name) - if !allowMultipleTenants && len(si.Spec.SubaccountID) > 0 { - serviceinstancelog.Error(fmt.Errorf("invalid subaccountID property"), "the operator installation does not allow multiple subaccounts") - return nil, fmt.Errorf("setting the subaccountID property is not allowed") - } return nil, nil } @@ -60,8 +50,8 @@ func (si *ServiceInstance) ValidateUpdate(old runtime.Object) (warnings admissio serviceinstancelog.Info("validate update", "name", si.Name) oldInstance := old.(*ServiceInstance) - if oldInstance.Spec.SubaccountID != si.Spec.SubaccountID { - return nil, fmt.Errorf("changing the subaccountID for an existing instance is not allowed") + if oldInstance.Spec.BTPAccessCredentialsSecret != si.Spec.BTPAccessCredentialsSecret { + return nil, fmt.Errorf("changing the btpAccessCredentialsSecret for an existing instance is not allowed") } return nil, nil } diff --git a/api/v1/serviceinstance_validating_webhook_test.go b/api/v1/serviceinstance_validating_webhook_test.go index d7530982..3323830d 100644 --- a/api/v1/serviceinstance_validating_webhook_test.go +++ b/api/v1/serviceinstance_validating_webhook_test.go @@ -12,27 +12,16 @@ var _ = Describe("Service Instance Webhook Test", func() { instance = getInstance() }) - Context("Validate Create", func() { - When("multiple subaccounts is not allowed and subaccountID exists", func() { - It("should fail", func() { - instance := getInstanceWithSubaccountID() - SetAllowMultipleTenants(false) - _, err := instance.ValidateCreate() - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("setting the subaccountID property is not allowed")) - }) - }) - }) - Context("Validate Update", func() { - When("multiple subaccounts is not allowed and subaccountID changed", func() { + When("btpAccessCredentialsSecret changed", func() { It("should fail", func() { - instance := getInstanceWithSubaccountID() - newInstance := getInstanceWithSubaccountID() - newInstance.Spec.SubaccountID = "12345" + instance := getInstance() + instance.Spec.BTPAccessCredentialsSecret = "" + newInstance := getInstance() + newInstance.Spec.BTPAccessCredentialsSecret = "new-secret" _, err := newInstance.ValidateUpdate(instance) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("changing the subaccountID for an existing instance is not allowed")) + Expect(err.Error()).To(ContainSubstring("changing the btpAccessCredentialsSecret for an existing instance is not allowed")) }) }) }) diff --git a/api/v1/suite_test.go b/api/v1/suite_test.go index 879881f8..39dfb954 100644 --- a/api/v1/suite_test.go +++ b/api/v1/suite_test.go @@ -88,9 +88,3 @@ func getInstance() *ServiceInstance { Status: ServiceInstanceStatus{}, } } - -func getInstanceWithSubaccountID() *ServiceInstance { - si := getInstance() - si.Spec.SubaccountID = "testsubaccountid" - return si -} diff --git a/client/sm/client.go b/client/sm/client.go index 62c5ec57..26c21130 100644 --- a/client/sm/client.go +++ b/client/sm/client.go @@ -139,6 +139,8 @@ func NewClient(ctx context.Context, config *ClientConfig, httpClient auth.HTTPCl func (client *serviceManagerClient) Provision(instance *types.ServiceInstance, serviceName string, planName string, q *Parameters, user string, dataCenter string) (*ProvisionResponse, error) { var newInstance *types.ServiceInstance var instanceID string + var subaccountID string + if len(serviceName) == 0 || len(planName) == 0 { return nil, fmt.Errorf("missing field values. Specify service name and plan name for the instance '%s'", instance.Name) } @@ -161,13 +163,14 @@ func (client *serviceManagerClient) Provision(instance *types.ServiceInstance, s } } else if newInstance != nil { instanceID = newInstance.ID + subaccountID = getSubaccountID(newInstance) } res := &ProvisionResponse{ InstanceID: instanceID, Location: location, PlanID: planInfo.planID, - SubaccountID: getSubaccountIDFromContext(newInstance), + SubaccountID: subaccountID, } if planInfo.serviceOffering != nil { @@ -177,23 +180,6 @@ func (client *serviceManagerClient) Provision(instance *types.ServiceInstance, s return res, nil } -func getSubaccountIDFromContext(instance *types.ServiceInstance) string { - subaccountID := "" - if instance == nil || len(instance.Context) == 0 { - return subaccountID - } - - var contextMap map[string]interface{} - if err := json.Unmarshal(instance.Context, &contextMap); err != nil { - return "" - } - - if saID, ok := contextMap["subaccount_id"]; ok { - subaccountID, _ = saID.(string) - } - return subaccountID -} - // Bind creates binding to an instance in service manager func (client *serviceManagerClient) Bind(binding *types.ServiceBinding, q *Parameters, user string) (*types.ServiceBinding, string, error) { var newBinding *types.ServiceBinding @@ -563,6 +549,26 @@ func ExtractInstanceID(operationURL string) string { return "" } +func getSubaccountID(instance *types.ServiceInstance) string { + if len(instance.Labels["subaccount_id"]) > 0 { + return instance.Labels["subaccount_id"][0] + } + subaccountID := "" + if instance == nil || len(instance.Context) == 0 { + return subaccountID + } + + var contextMap map[string]interface{} + if err := json.Unmarshal(instance.Context, &contextMap); err != nil { + return "" + } + + if saID, ok := contextMap["subaccount_id"]; ok { + subaccountID, _ = saID.(string) + } + return subaccountID +} + func ExtractBindingID(operationURL string) string { r := regexp.MustCompile(`^/v1/service_bindings/(.*)/operations/.*$`) matches := r.FindStringSubmatch(operationURL) diff --git a/config/crd/bases/services.cloud.sap.com_servicebindings.yaml b/config/crd/bases/services.cloud.sap.com_servicebindings.yaml index 5883c996..7b1110bd 100644 --- a/config/crd/bases/services.cloud.sap.com_servicebindings.yaml +++ b/config/crd/bases/services.cloud.sap.com_servicebindings.yaml @@ -264,7 +264,7 @@ spec: description: Indicates whether binding is ready for usage type: string subaccountID: - description: The subaccount id of the service instance + description: The subaccount id of the service binding type: string required: - conditions diff --git a/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml b/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml index 43f5f941..6790474d 100644 --- a/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml +++ b/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml @@ -65,6 +65,9 @@ spec: spec: description: ServiceInstanceSpec defines the desired state of ServiceInstance properties: + btpAccessCredentialsSecret: + description: The name of the btp access credentials secret + type: string customTags: description: List of custom tags describing the ServiceInstance, will be copied to `ServiceBinding` secret in the key called `tags`. @@ -128,9 +131,6 @@ spec: shared: description: Indicates the desired shared state type: boolean - subaccountID: - description: The desired subaccount id for the service instance - type: string userInfo: description: UserInfo contains information about the user that last modified this instance. This field is set by the API server and diff --git a/config/samples/subaccountSecret.yaml b/config/samples/btp_access_credentials_secret.yaml similarity index 78% rename from config/samples/subaccountSecret.yaml rename to config/samples/btp_access_credentials_secret.yaml index 9ddeecda..bd04f167 100644 --- a/config/samples/subaccountSecret.yaml +++ b/config/samples/btp_access_credentials_secret.yaml @@ -1,7 +1,7 @@ apiVersion: v1 kind: Secret metadata: - name: -sap-btp-service-operator + name: namespace: sap-btp-operator type: Opaque data: diff --git a/config/samples/cross_namespaces.yaml b/config/samples/cross_namespaces.yaml new file mode 100644 index 00000000..6c8ec40d --- /dev/null +++ b/config/samples/cross_namespaces.yaml @@ -0,0 +1,20 @@ +--- +apiVersion: services.cloud.sap.com/v1 +kind: ServiceInstance +metadata: + name: sample-instance-1 + namespace: dev-instances +spec: + serviceOfferingName: service-manager + servicePlanName: subaccount-audit + +--- +apiVersion: services.cloud.sap.com/v1 +kind: ServiceBinding +metadata: + name: sample-binding-1 + namespace: dev-bindings +spec: + serviceInstanceName: sample-instance-1 + serviceInstanceNamespace: dev-instances + diff --git a/config/samples/services_v1_servicebinding2.yaml b/config/samples/services_v1_servicebinding2.yaml deleted file mode 100644 index d5eaa298..00000000 --- a/config/samples/services_v1_servicebinding2.yaml +++ /dev/null @@ -1,6 +0,0 @@ -apiVersion: services.cloud.sap.com/v1 -kind: ServiceBinding -metadata: - name: sample-binding-2 -spec: - serviceInstanceName: sample-instance-1 diff --git a/config/samples/services_v1_servicebinding3.yaml b/config/samples/services_v1_servicebinding3.yaml deleted file mode 100644 index 76c0e05d..00000000 --- a/config/samples/services_v1_servicebinding3.yaml +++ /dev/null @@ -1,6 +0,0 @@ -apiVersion: services.cloud.sap.com/v1 -kind: ServiceBinding -metadata: - name: multi-data-center-binding -spec: - serviceInstanceName: multi-data-center-instance diff --git a/config/samples/services_v1_servicebinding_cred_rotation.yaml b/config/samples/services_v1_servicebinding_cred_rotation.yaml new file mode 100644 index 00000000..711da86a --- /dev/null +++ b/config/samples/services_v1_servicebinding_cred_rotation.yaml @@ -0,0 +1,10 @@ +apiVersion: services.cloud.sap.com/v1 +kind: ServiceBinding +metadata: + name: sample-binding-rotate +spec: + serviceInstanceName: sample-instance-1 + credentialsRotationPolicy: + enabled: true + rotatedBindingTTL: 48h + rotationFrequency: 168h diff --git a/config/samples/services_v1_serviceinstance2.yaml b/config/samples/services_v1_serviceinstance_btp_access.yaml similarity index 52% rename from config/samples/services_v1_serviceinstance2.yaml rename to config/samples/services_v1_serviceinstance_btp_access.yaml index 1fa3f54d..3e2ccb36 100644 --- a/config/samples/services_v1_serviceinstance2.yaml +++ b/config/samples/services_v1_serviceinstance_btp_access.yaml @@ -1,7 +1,8 @@ apiVersion: services.cloud.sap.com/v1 kind: ServiceInstance metadata: - name: sample-instance-2 + name: sample-instance-1 spec: serviceOfferingName: service-manager - servicePlanName: subaccount-admin + servicePlanName: subaccount-audit + btpAccessCredentialsSecret: mybtpsecret \ No newline at end of file diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index b158a1cb..0d991b93 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -2,6 +2,7 @@ apiVersion: admissionregistration.k8s.io/v1 kind: MutatingWebhookConfiguration metadata: + creationTimestamp: null name: mutating-webhook-configuration webhooks: - admissionReviewVersions: @@ -50,6 +51,7 @@ webhooks: apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: + creationTimestamp: null name: validating-webhook-configuration webhooks: - admissionReviewVersions: diff --git a/controllers/servicebinding_controller.go b/controllers/servicebinding_controller.go index f0800c27..76e583bb 100644 --- a/controllers/servicebinding_controller.go +++ b/controllers/servicebinding_controller.go @@ -93,7 +93,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque } if isMarkedForDeletion(serviceBinding.ObjectMeta) { - return r.delete(ctx, serviceBinding, serviceInstance.Spec.SubaccountID) + return r.delete(ctx, serviceBinding, serviceInstance.Spec.BTPAccessCredentialsSecret) } if err != nil { // instance not found @@ -108,7 +108,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque if len(serviceBinding.Status.OperationURL) > 0 { // ongoing operation - poll status from SM - return r.poll(ctx, serviceBinding, serviceInstance.Spec.SubaccountID) + return r.poll(ctx, serviceBinding, serviceInstance.Spec.BTPAccessCredentialsSecret) } if !controllerutil.ContainsFinalizer(serviceBinding, api.FinalizerName) { @@ -131,7 +131,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque } if meta.IsStatusConditionTrue(serviceBinding.Status.Conditions, api.ConditionCredRotationInProgress) { - if err := r.rotateCredentials(ctx, serviceBinding, serviceInstance.Spec.SubaccountID); err != nil { + if err := r.rotateCredentials(ctx, serviceBinding, serviceInstance.Spec.BTPAccessCredentialsSecret); err != nil { return ctrl.Result{}, err } } @@ -178,7 +178,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque return ctrl.Result{}, r.updateStatus(ctx, serviceBinding) } - smClient, err := r.getSMClient(ctx, serviceBinding, serviceInstance.Spec.SubaccountID) + smClient, err := r.getSMClient(ctx, serviceBinding, serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { return r.markAsTransientError(ctx, Unknown, err.Error(), serviceBinding) } @@ -270,10 +270,10 @@ func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient s return ctrl.Result{}, r.updateStatus(ctx, serviceBinding) } -func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *servicesv1.ServiceBinding, subaccountID string) (ctrl.Result, error) { +func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *servicesv1.ServiceBinding, btpAccessCredentialsSecret string) (ctrl.Result, error) { log := GetLogger(ctx) if controllerutil.ContainsFinalizer(serviceBinding, api.FinalizerName) { - smClient, err := r.getSMClient(ctx, serviceBinding, subaccountID) + smClient, err := r.getSMClient(ctx, serviceBinding, btpAccessCredentialsSecret) if err != nil { return r.markAsTransientError(ctx, Unknown, err.Error(), serviceBinding) } @@ -305,7 +305,7 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *s if len(serviceBinding.Status.OperationURL) > 0 && serviceBinding.Status.OperationType == smClientTypes.DELETE { // ongoing delete operation - poll status from SM - return r.poll(ctx, serviceBinding, subaccountID) + return r.poll(ctx, serviceBinding, btpAccessCredentialsSecret) } log.Info(fmt.Sprintf("Deleting binding with id %v from SM", serviceBinding.Status.BindingID)) @@ -332,11 +332,11 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *s return ctrl.Result{}, nil } -func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *servicesv1.ServiceBinding, subaccountID string) (ctrl.Result, error) { +func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *servicesv1.ServiceBinding, btpAccessCredentialsSecret string) (ctrl.Result, error) { log := GetLogger(ctx) log.Info(fmt.Sprintf("resource is in progress, found operation url %s", serviceBinding.Status.OperationURL)) - smClient, err := r.getSMClient(ctx, serviceBinding, subaccountID) + smClient, err := r.getSMClient(ctx, serviceBinding, btpAccessCredentialsSecret) if err != nil { return r.markAsTransientError(ctx, Unknown, err.Error(), serviceBinding) } @@ -390,7 +390,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *ser return ctrl.Result{}, fmt.Errorf(errMsg) } case smClientTypes.SUCCEEDED: - setSuccessConditions(smClientTypes.OperationCategory(status.Type), serviceBinding) + setSuccessConditions(status.Type, serviceBinding) switch serviceBinding.Status.OperationType { case smClientTypes.CREATE: smBinding, err := smClient.GetBindingByID(serviceBinding.Status.BindingID, nil) @@ -398,6 +398,9 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *ser log.Error(err, fmt.Sprintf("binding %s succeeded but could not fetch it from SM", serviceBinding.Status.BindingID)) return ctrl.Result{}, err } + if len(smBinding.Labels["subaccount_id"]) > 0 { + serviceBinding.Status.SubaccountID = smBinding.Labels["subaccount_id"][0] + } if err := r.storeBindingSecret(ctx, serviceBinding, smBinding); err != nil { return r.handleSecretError(ctx, smClientTypes.CREATE, err, serviceBinding) @@ -756,7 +759,7 @@ func (r *ServiceBindingReconciler) addInstanceInfo(ctx context.Context, binding return metadata, nil } -func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, binding *servicesv1.ServiceBinding, subaccountID string) error { +func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, binding *servicesv1.ServiceBinding, btpAccessCredentialsSecret string) error { suffix := "-" + RandStringRunes(6) log := GetLogger(ctx) if binding.Annotations != nil { @@ -797,7 +800,7 @@ func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, bindin } if len(bindings.Items) == 0 { - smClient, err := r.getSMClient(ctx, binding, subaccountID) + smClient, err := r.getSMClient(ctx, binding, btpAccessCredentialsSecret) if err != nil { return err } diff --git a/controllers/serviceinstance_controller.go b/controllers/serviceinstance_controller.go index 2e4a7401..41936e76 100644 --- a/controllers/serviceinstance_controller.go +++ b/controllers/serviceinstance_controller.go @@ -107,7 +107,7 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ log.Info(fmt.Sprintf("instance is not in final state, handling... (generation: %d, observedGen: %d", serviceInstance.Generation, serviceInstance.Status.ObservedGeneration)) serviceInstance.SetObservedGeneration(serviceInstance.Generation) - smClient, err := r.getSMClient(ctx, serviceInstance, serviceInstance.Spec.SubaccountID) + smClient, err := r.getSMClient(ctx, serviceInstance, serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { log.Error(err, "failed to get sm client") return r.markAsTransientError(ctx, Unknown, err.Error(), serviceInstance) @@ -251,7 +251,7 @@ func (r *ServiceInstanceReconciler) deleteInstance(ctx context.Context, serviceI log := GetLogger(ctx) if controllerutil.ContainsFinalizer(serviceInstance, api.FinalizerName) { - smClient, err := r.getSMClient(ctx, serviceInstance, serviceInstance.Spec.SubaccountID) + smClient, err := r.getSMClient(ctx, serviceInstance, serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { log.Error(err, "failed to get sm client") return r.markAsTransientError(ctx, Unknown, err.Error(), serviceInstance) @@ -333,7 +333,7 @@ func (r *ServiceInstanceReconciler) handleInstanceSharing(ctx context.Context, s func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *servicesv1.ServiceInstance) (ctrl.Result, error) { log := GetLogger(ctx) log.Info(fmt.Sprintf("resource is in progress, found operation url %s", serviceInstance.Status.OperationURL)) - smClient, err := r.getSMClient(ctx, serviceInstance, serviceInstance.Spec.SubaccountID) + smClient, err := r.getSMClient(ctx, serviceInstance, serviceInstance.Spec.BTPAccessCredentialsSecret) if err != nil { log.Error(err, "failed to get sm client") return r.markAsTransientError(ctx, Unknown, err.Error(), serviceInstance) @@ -377,15 +377,23 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *s return ctrl.Result{}, fmt.Errorf(errMsg) } case smClientTypes.SUCCEEDED: - setSuccessConditions(status.Type, serviceInstance) - if serviceInstance.Status.OperationType == smClientTypes.DELETE { + if serviceInstance.Status.OperationType == smClientTypes.CREATE { + smInstance, err := smClient.GetInstanceByID(serviceInstance.Status.InstanceID, nil) + if err != nil { + log.Error(err, fmt.Sprintf("instance %s succeeded but could not fetch it from SM", serviceInstance.Status.InstanceID)) + return ctrl.Result{}, err + } + if len(smInstance.Labels["subaccount_id"]) > 0 { + serviceInstance.Status.SubaccountID = smInstance.Labels["subaccount_id"][0] + } + serviceInstance.Status.Ready = metav1.ConditionTrue + } else if serviceInstance.Status.OperationType == smClientTypes.DELETE { // delete was successful - remove our finalizer from the list and update it. if err := r.removeFinalizer(ctx, serviceInstance, api.FinalizerName); err != nil { return ctrl.Result{}, err } - } else if serviceInstance.Status.OperationType == smClientTypes.CREATE { - serviceInstance.Status.Ready = metav1.ConditionTrue } + setSuccessConditions(status.Type, serviceInstance) } serviceInstance.Status.OperationURL = "" diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index e062aa10..4a11bcb2 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -204,24 +204,6 @@ var _ = Describe("ServiceInstance controller", func() { }) }) }) - - When("multiple subaccounts is disabled", func() { - BeforeEach(func() { - v1.SetAllowMultipleTenants(false) - }) - AfterEach(func() { - v1.SetAllowMultipleTenants(true) - }) - It("should fail if instance contains subaccount id", func() { - instance := &v1.ServiceInstance{Spec: instanceSpec} - instance.Name = fakeInstanceName - instance.Namespace = testNamespace - instance.Spec.SubaccountID = "someID" - err := k8sClient.Create(ctx, instance) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("setting the subaccountID property is not allowed")) - }) - }) }) Context("Sync", func() { @@ -315,6 +297,9 @@ var _ = Describe("ServiceInstance controller", func() { }) When("polling ends with success", func() { + BeforeEach(func() { + fakeClient.GetInstanceByIDReturns(&smclientTypes.ServiceInstance{Labels: map[string][]string{"subaccount_id": []string{fakeSubaccountID}}}, nil) + }) It("should update in progress condition and provision the instance successfully", func() { serviceInstance = createInstance(ctx, instanceSpec, false) fakeClient.StatusReturns(&smclientTypes.Operation{ @@ -323,6 +308,7 @@ var _ = Describe("ServiceInstance controller", func() { State: smClientTypes.SUCCEEDED, }, nil) waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionTrue, Created, "") + Expect(serviceInstance.Status.SubaccountID).To(Equal(fakeSubaccountID)) }) }) @@ -584,10 +570,10 @@ var _ = Describe("ServiceInstance controller", func() { It("should fail", func() { deleteInstance(ctx, serviceInstance, true) serviceInstance = createInstance(ctx, instanceSpec, true) - serviceInstance.Spec.SubaccountID = "12345" + serviceInstance.Spec.BTPAccessCredentialsSecret = "12345" err := k8sClient.Update(ctx, serviceInstance) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("changing the subaccountID for an existing instance is not allowed")) + Expect(err.Error()).To(ContainSubstring("changing the btpAccessCredentialsSecret for an existing instance is not allowed")) }) }) }) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 65de6363..8ba241cf 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -136,7 +136,6 @@ var _ = BeforeSuite(func(done Done) { err = (&v1.ServiceBinding{}).SetupWebhookWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) - v1.SetAllowMultipleTenants(true) err = (&v1.ServiceInstance{}).SetupWebhookWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) diff --git a/internal/config/config.go b/internal/config/config.go index 4b13cc44..f162bc8c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -13,32 +13,30 @@ var ( ) type Config struct { - SyncPeriod time.Duration `envconfig:"sync_period"` - PollInterval time.Duration `envconfig:"poll_interval"` - LongPollInterval time.Duration `envconfig:"long_poll_interval"` - ManagementNamespace string `envconfig:"management_namespace"` - EnableMultipleSubaccounts bool `envconfig:"enable_multiple_subaccounts"` - ReleaseNamespace string `envconfig:"release_namespace"` - AllowClusterAccess bool `envconfig:"allow_cluster_access"` - AllowedNamespaces []string `envconfig:"allowed_namespaces"` - EnableNamespaceSecrets bool `envconfig:"enable_namespace_secrets"` - ClusterID string `envconfig:"cluster_id"` - RetryBaseDelay time.Duration `envconfig:"retry_base_delay"` - RetryMaxDelay time.Duration `envconfig:"retry_max_delay"` + SyncPeriod time.Duration `envconfig:"sync_period"` + PollInterval time.Duration `envconfig:"poll_interval"` + LongPollInterval time.Duration `envconfig:"long_poll_interval"` + ManagementNamespace string `envconfig:"management_namespace"` + ReleaseNamespace string `envconfig:"release_namespace"` + AllowClusterAccess bool `envconfig:"allow_cluster_access"` + AllowedNamespaces []string `envconfig:"allowed_namespaces"` + EnableNamespaceSecrets bool `envconfig:"enable_namespace_secrets"` + ClusterID string `envconfig:"cluster_id"` + RetryBaseDelay time.Duration `envconfig:"retry_base_delay"` + RetryMaxDelay time.Duration `envconfig:"retry_max_delay"` } func Get() Config { loadOnce.Do(func() { config = Config{ // default values - SyncPeriod: 60 * time.Second, - PollInterval: 10 * time.Second, - LongPollInterval: 5 * time.Minute, - EnableNamespaceSecrets: true, - EnableMultipleSubaccounts: true, - AllowedNamespaces: []string{}, - AllowClusterAccess: true, - RetryBaseDelay: 10 * time.Second, - RetryMaxDelay: time.Hour, + SyncPeriod: 60 * time.Second, + PollInterval: 10 * time.Second, + LongPollInterval: 5 * time.Minute, + EnableNamespaceSecrets: true, + AllowedNamespaces: []string{}, + AllowClusterAccess: true, + RetryBaseDelay: 10 * time.Second, + RetryMaxDelay: time.Hour, } envconfig.MustProcess("", &config) }) diff --git a/internal/secrets/resolver.go b/internal/secrets/resolver.go index 075166af..e643d89b 100644 --- a/internal/secrets/resolver.go +++ b/internal/secrets/resolver.go @@ -19,30 +19,22 @@ const ( ) type SecretResolver struct { - EnableMultipleSubaccounts bool - ManagementNamespace string - ReleaseNamespace string - EnableNamespaceSecrets bool - Client client.Client - Log logr.Logger + ManagementNamespace string + ReleaseNamespace string + EnableNamespaceSecrets bool + Client client.Client + Log logr.Logger } -func (sr *SecretResolver) GetSecretForResource(ctx context.Context, namespace, name, subaccountID string) (*v1.Secret, error) { +func (sr *SecretResolver) GetSecretForResource(ctx context.Context, namespace, name, btpAccessSecret string) (*v1.Secret, error) { secretForResource := &v1.Secret{} - if !sr.EnableMultipleSubaccounts { - sr.Log.Info("enableMultipleSubaccounts set to false - using default cluster secret") - return sr.getDefaultSecret(ctx, name) - } - - // search subaccount secret - if len(subaccountID) > 0 { - secretName := fmt.Sprintf("%s-%s", subaccountID, name) - sr.Log.Info(fmt.Sprintf("Searching for secret name %s in namespace %s for subaccountID %s", - secretName, sr.ManagementNamespace, subaccountID)) - err := sr.Client.Get(ctx, types.NamespacedName{Name: secretName, Namespace: sr.ManagementNamespace}, secretForResource) + if len(btpAccessSecret) > 0 { + sr.Log.Info(fmt.Sprintf("Searching for secret name %s in namespace %s", + btpAccessSecret, sr.ManagementNamespace)) + err := sr.Client.Get(ctx, types.NamespacedName{Name: btpAccessSecret, Namespace: sr.ManagementNamespace}, secretForResource) if err != nil { - sr.Log.Error(err, "Could not fetch subaccount secret") + sr.Log.Error(err, fmt.Sprintf("Could not fetch secret named %s", btpAccessSecret)) return nil, err } return secretForResource, nil diff --git a/internal/secrets/resolver_test.go b/internal/secrets/resolver_test.go index b299f313..f47af200 100644 --- a/internal/secrets/resolver_test.go +++ b/internal/secrets/resolver_test.go @@ -83,11 +83,10 @@ var _ = Describe("Secrets Resolver", func() { BeforeEach(func() { ctx = context.Background() resolver = &secrets.SecretResolver{ - EnableMultipleSubaccounts: true, - ManagementNamespace: managementNamespace, - ReleaseNamespace: managementNamespace, - Log: logf.Log.WithName("SecretResolver"), - Client: k8sClient, + ManagementNamespace: managementNamespace, + ReleaseNamespace: managementNamespace, + Log: logf.Log.WithName("SecretResolver"), + Client: k8sClient, } }) @@ -179,25 +178,17 @@ var _ = Describe("Secrets Resolver", func() { }) }) - Context("Subaccount secret in management namespace", func() { + Context("btp access secret in management namespace", func() { subaccountID := "12345" BeforeEach(func() { secret = createSecret(subaccountID, managementNamespace) }) It("should resolve the secret", func() { - resolvedSecret, err := resolver.GetSecretForResource(ctx, testNamespace, secrets.SAPBTPOperatorSecretName, subaccountID) + resolvedSecret, err := resolver.GetSecretForResource(ctx, testNamespace, secrets.SAPBTPOperatorSecretName, secret.Name) Expect(err).ToNot(HaveOccurred()) Expect(resolvedSecret).ToNot(BeNil()) Expect(string(resolvedSecret.Data["clientid"])).To(Equal(expectedClientID)) }) - - It("should not resolve the secret if EnableMultipleSubaccount is false", func() { - resolver.EnableMultipleSubaccounts = false - _, err := resolver.GetSecretForResource(ctx, testNamespace, secrets.SAPBTPOperatorSecretName, subaccountID) - Expect(err).To(HaveOccurred()) - resolver.EnableMultipleSubaccounts = true - }) }) - }) diff --git a/main.go b/main.go index 638ab011..ea50ffd9 100644 --- a/main.go +++ b/main.go @@ -98,12 +98,11 @@ func main() { } secretResolver := &secrets.SecretResolver{ - EnableMultipleSubaccounts: config.Get().EnableMultipleSubaccounts, - ManagementNamespace: config.Get().ManagementNamespace, - ReleaseNamespace: config.Get().ReleaseNamespace, - EnableNamespaceSecrets: config.Get().EnableNamespaceSecrets, - Client: mgr.GetClient(), - Log: logf.Log.WithName("secret-resolver"), + ManagementNamespace: config.Get().ManagementNamespace, + ReleaseNamespace: config.Get().ReleaseNamespace, + EnableNamespaceSecrets: config.Get().EnableNamespaceSecrets, + Client: mgr.GetClient(), + Log: logf.Log.WithName("secret-resolver"), } if err = (&controllers.ServiceInstanceReconciler{ @@ -139,7 +138,6 @@ func main() { setupLog.Error(err, "unable to create webhook", "webhook", "ServiceBinding") os.Exit(1) } - servicesv1.SetAllowMultipleTenants(config.Get().EnableMultipleSubaccounts) if err = (&servicesv1.ServiceInstance{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "ServiceInstance") os.Exit(1) diff --git a/sapbtp-operator-charts/templates/configmap.yml b/sapbtp-operator-charts/templates/configmap.yml index f6ac57a8..e40487a0 100644 --- a/sapbtp-operator-charts/templates/configmap.yml +++ b/sapbtp-operator-charts/templates/configmap.yml @@ -22,7 +22,6 @@ data: ALLOWED_NAMESPACES: {{ join "," .Values.manager.allowed_namespaces }} {{- end }} {{- end }} - ENABLE_MULTIPLE_SUBACCOUNTS: {{ .Values.manager.enable_multiple_subaccounts | quote }} kind: ConfigMap metadata: name: sap-btp-operator-config diff --git a/sapbtp-operator-charts/templates/crd.yml b/sapbtp-operator-charts/templates/crd.yml index 365cc691..6f79e1f4 100644 --- a/sapbtp-operator-charts/templates/crd.yml +++ b/sapbtp-operator-charts/templates/crd.yml @@ -263,7 +263,7 @@ spec: description: Indicates whether binding is ready for usage type: string subaccountID: - description: The subaccount id of the service instance + description: The subaccount id of the service binding type: string required: - conditions @@ -592,6 +592,9 @@ spec: spec: description: ServiceInstanceSpec defines the desired state of ServiceInstance properties: + btpAccessCredentialsSecret: + description: The name of the btp access credentials secret + type: string customTags: description: List of custom tags describing the ServiceInstance, will be copied to `ServiceBinding` secret in the key called `tags`. @@ -655,9 +658,6 @@ spec: shared: description: Indicates the desired shared state type: boolean - subaccountID: - description: The desired subaccount id for the service instance - type: string userInfo: description: UserInfo contains information about the user that last modified this instance. This field is set by the API server and diff --git a/sapbtp-operator-charts/values.yaml b/sapbtp-operator-charts/values.yaml index b5323b3e..a2b79594 100644 --- a/sapbtp-operator-charts/values.yaml +++ b/sapbtp-operator-charts/values.yaml @@ -8,7 +8,6 @@ manager: req_memory_limit: 20Mi req_cpu_limit: 100m allow_cluster_access: true - enable_multiple_subaccounts: true allowed_namespaces: [] replica_count: 2 enable_leader_election: true