Configure BTP access secret in instance spec (#363)

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"`
 }
 

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

api/v1/serviceinstance_validating_webhook.go

@@ -41,27 +41,17 @@ 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
 }
 
 func (si *ServiceInstance) ValidateUpdate(old runtime.Object) (warnings admission.Warnings, err error) {
 	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
 }

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"))
 			})
 		})
 	})

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
-}

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)

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

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

config/samples/subaccountSecret.yaml → config/samples/btp_access_credentials_secret.yaml

@@ -1,7 +1,7 @@
 apiVersion: v1
 kind: Secret
 metadata:
-  name: <subaccountID>-sap-btp-service-operator
+  name: <name>
   namespace: sap-btp-operator
 type: Opaque
 data:

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
+

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

config/samples/services_v1_serviceinstance2.yaml → 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

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:

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,14 +390,17 @@ 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)
 			if err != nil {
 				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
 		}