Cosmetics - names and strings change (#17)

naming and messages review

Makefile

@@ -45,15 +45,15 @@ uninstall: manifests
 
 # Deploy controller in the configured Kubernetes cluster in ~/.kube/config
 deploy: manifests helm-charts
-	helm upgrade --install sapbtp-operator ./sapbtp-operator-charts \
+	helm upgrade --install sap-btp-operator ./sapbtp-operator-charts \
         --create-namespace \
-        --namespace=sapbtp-operator \
+        --namespace=sap-btp-operator \
         --values=hack/override_values.yaml \
 		--set manager.image.repository=controller \
 		--set manager.image.tag=latest
 
 undeploy:
-	helm uninstall sapbtp-operator -n sapbtp-operator
+	helm uninstall sap-btp-operator -n sap-btp-operator
 
 # Generate manifests e.g. CRD, RBAC etc.
 manifests: controller-gen

README.md

@@ -64,9 +64,9 @@ This feature is still under development, review, and testing.
 
 3. Deploy the the SAP BTP service operator in the cluster using the obtained access credentials:
     ```bash
-    helm upgrade --install sapbtp-operator https://github.com/SAP/sap-btp-service-operator/releases/download/<release>/sapbtp-operator-<release>.tgz \
+    helm upgrade --install sap-btp-operator https://github.com/SAP/sap-btp-service-operator/releases/download/<release>/sapbtp-operator-<release>.tgz \
         --create-namespace \
-        --namespace=sapbtp-operator \
+        --namespace=sap-btp-operator \
         --set manager.secret.clientid=<clientid> \
         --set manager.secret.clientsecret=<clientsecret> \
         --set manager.secret.url=<sm_url> \

api/v1alpha1/servicebinding_webhook.go

@@ -70,7 +70,7 @@ func (r *ServiceBinding) ValidateUpdate(old runtime.Object) error {
 	servicebindinglog.Info("validate update", "name", r.Name)
 
 	if r.specChanged(old) && r.Status.BindingID != "" {
-		return fmt.Errorf("service binding spec cannot be modified after creation")
+		return fmt.Errorf("updating service bindings is not supported")
 	}
 
 	return nil

client/sm/client.go

@@ -97,7 +97,7 @@ func (client *serviceManagerClient) Provision(instance *types.ServiceInstance, s
 	var newInstance *types.ServiceInstance
 	var instanceID string
 	if len(serviceName) == 0 || len(planName) == 0 {
-		return "", "", fmt.Errorf("service name and plan name must be not empty for instance %s", instance.Name)
+		return "", "", fmt.Errorf("missing field values. Specify service name and plan name for the instance '%s'", instance.Name)
 	}
 
 	planID, err := client.getAndValidatePlanID(instance.ServicePlanID, serviceName, planName)
@@ -114,7 +114,7 @@ func (client *serviceManagerClient) Provision(instance *types.ServiceInstance, s
 	if len(location) > 0 {
 		instanceID = ExtractInstanceID(location)
 		if len(instanceID) == 0 {
-			return "", "", fmt.Errorf("failed to extract instance ID from operation URL %s", location)
+			return "", "", fmt.Errorf("failed to extract the service instance ID from the async operation URL: %s", location)
 		}
 	} else if newInstance != nil {
 		instanceID = newInstance.ID
@@ -324,7 +324,7 @@ func (client *serviceManagerClient) getAndValidatePlanID(planID string, serviceN
 
 	var commaSepOfferingIds string
 	if len(offerings.ServiceOfferings) == 0 {
-		return "", fmt.Errorf("service offering with name %s not found", serviceName)
+		return "", fmt.Errorf("couldn't find the service offering '%s'", serviceName)
 	}
 
 	serviceOfferingIds := make([]string, 0, len(offerings.ServiceOfferings))
@@ -342,7 +342,7 @@ func (client *serviceManagerClient) getAndValidatePlanID(planID string, serviceN
 		return "", err
 	}
 	if len(plans.ServicePlans) == 0 {
-		return "", fmt.Errorf("service plan %s not found for service offering %s", planName, serviceName)
+		return "", fmt.Errorf("couldn't find the service plan '%s' for the service offering '%s'", planName, serviceName)
 	} else if len(plans.ServicePlans) == 1 && len(planID) == 0 {
 		return plans.ServicePlans[0].ID, nil
 	} else {
@@ -354,9 +354,9 @@ func (client *serviceManagerClient) getAndValidatePlanID(planID string, serviceN
 	}
 
 	if len(planID) > 0 {
-		err = fmt.Errorf("provided plan ID %s does not match the provided offering name %s and plan name %s", planID, serviceName, planName)
+		err = fmt.Errorf("the provided plan ID '%s' doesn't match the provided offering name '%s' and plan name '%s", planID, serviceName, planName)
 	} else {
-		err = fmt.Errorf("ambiguity error, found more than one resource matching the provided offering name %s and plan name %s, provide the desired servicePlanID", serviceName, planName)
+		err = fmt.Errorf("ambiguity error: found more than one resource that matches the provided offering name '%s' and plan name '%s'. Please provide servicePlanID", serviceName, planName)
 	}
 	return "", err
 

config/default/kustomization.yaml

@@ -6,7 +6,7 @@ namespace: releasenamespace
 # "wordpress" becomes "alices-wordpress".
 # Note that it should also match with the prefix (text before '-') of the namespace
 # field above.
-namePrefix: sapbtp-operator-
+namePrefix: sap-btp-operator-
 
 # Labels to add to all resources and selectors.
 #commonLabels:

controllers/base_controller.go

@@ -73,18 +73,13 @@ func (r *BaseReconciler) getSMClient(ctx context.Context, log logr.Logger, objec
 	}
 
 	secret, err := r.SecretResolver.GetSecretForResource(ctx, object.GetNamespace())
-	if err != nil || secret == nil {
-		setBlockedCondition("Secret not found", object)
+	if err != nil {
+		setBlockedCondition("secret not found", object)
 		if err := r.updateStatusWithRetries(ctx, object, log); err != nil {
 			return nil, err
 		}
-		var secretResolveErr error
-		if err != nil {
-			secretResolveErr = fmt.Errorf("could not resolve SM secret: %s", err.Error())
-		} else {
-			secretResolveErr = fmt.Errorf("SM secret not found")
-		}
-		return nil, secretResolveErr
+
+		return nil, err
 	}
 
 	secretData := secret.Data
@@ -108,7 +103,7 @@ func (r *BaseReconciler) removeFinalizer(ctx context.Context, object servicesv1a
 			}
 			controllerutil.RemoveFinalizer(object, finalizerName)
 			if err := r.Update(ctx, object); err != nil {
-				return fmt.Errorf("failed to remove finalizer %s : %v", finalizerName, err)
+				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()))
@@ -126,10 +121,10 @@ func (r *BaseReconciler) addFinalizer(ctx context.Context, object servicesv1alph
 			}
 			controllerutil.AddFinalizer(object, finalizerName)
 			if err := r.Update(ctx, object); err != nil {
-				return fmt.Errorf("failed to add finalizer %s : %v", finalizerName, err)
+				return fmt.Errorf("failed to add the finalizer '%s'. Error: %v", finalizerName, err)
 			}
 		}
-		log.Info(fmt.Sprintf("added finalizer %s to %s", finalizerName, object.GetControllerName()))
+		log.Info(fmt.Sprintf("added finalizer '%s' to %s", finalizerName, object.GetControllerName()))
 		return nil
 	}
 	return nil

controllers/servicebinding_controller.go

@@ -105,9 +105,9 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	if err != nil || serviceNotUsable(serviceInstance) {
 		var instanceErr error
 		if err != nil {
-			instanceErr = fmt.Errorf("unable to find service instance %s: %s", serviceBinding.Spec.ServiceInstanceName, err.Error())
+			instanceErr = fmt.Errorf("couldn't find the service instance '%s'. Error: %v", serviceBinding.Spec.ServiceInstanceName, err.Error())
 		} else {
-			instanceErr = fmt.Errorf("service instance %s is not usable, unable to create binding %s", serviceBinding.Spec.ServiceInstanceName, serviceBinding.Name)
+			instanceErr = fmt.Errorf("service instance '%s' is not usable", serviceBinding.Spec.ServiceInstanceName)
 		}
 
 		setBlockedCondition(instanceErr.Error(), serviceBinding)
@@ -121,7 +121,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	if isInProgress(serviceInstance) {
 		log.Info(fmt.Sprintf("Service instance with k8s name %s is not ready for binding yet", serviceInstance.Name))
 
-		setInProgressCondition(smTypes.CREATE, fmt.Sprintf("Referenced service instance with k8s name %s is not ready, cannot create binding yet", serviceBinding.Spec.ServiceInstanceName),
+		setInProgressCondition(smTypes.CREATE, fmt.Sprintf("creation in progress, waiting for service instance '%s' to be ready", serviceBinding.Spec.ServiceInstanceName),
 			serviceBinding)
 		if err := r.updateStatusWithRetries(ctx, serviceBinding, log); err != nil {
 			return ctrl.Result{}, err
@@ -461,7 +461,7 @@ func (r *ServiceBindingReconciler) storeBindingSecret(ctx context.Context, k8sBi
 		credentialsMap, err = normalizeCredentials(smBinding.Credentials)
 		if err != nil {
 			logger.Error(err, "Failed to store binding secret")
-			return fmt.Errorf("failed to store binding secret: %s", err.Error())
+			return fmt.Errorf("failed to create secret. Error: %v", err.Error())
 		}
 	}
 
@@ -573,7 +573,7 @@ func (r *ServiceBindingReconciler) validateSecretNameIsAvailable(ctx context.Con
 		if len(otherBindingName) > 0 {
 			return fmt.Errorf("secret %s belongs to another binding %s, choose a differnet name", binding.Spec.SecretName, otherBindingName)
 		}
-		return fmt.Errorf("the specified secret name '%s' is already taken, choose a differnet name", binding.Spec.SecretName)
+		return fmt.Errorf("the specified secret name '%s' is already taken. Choose another name and try again", binding.Spec.SecretName)
 	}
 	return nil
 }

controllers/servicebinding_controller_test.go

@@ -218,7 +218,7 @@ var _ = Describe("ServiceBinding controller", func() {
 			When("referenced service instance does not exist", func() {
 				It("should fail", func() {
 					createBindingWithBlockedError(context.Background(), bindingName, bindingTestNamespace, "no-such-instance", "",
-						"unable to find service instance")
+						"couldn't find the service instance")
 				})
 			})
 
@@ -231,7 +231,7 @@ var _ = Describe("ServiceBinding controller", func() {
 				})
 				It("should fail", func() {
 					createBindingWithBlockedError(context.Background(), bindingName, otherNamespace, instanceName, "",
-						fmt.Sprintf("unable to find service instance"))
+						fmt.Sprintf("couldn't find the service instance"))
 				})
 			})
 
@@ -357,7 +357,7 @@ var _ = Describe("ServiceBinding controller", func() {
 							if err != nil {
 								return false
 							}
-							return isFailed(createdBinding) && strings.Contains(createdBinding.Status.Conditions[0].Message, "failed to store binding secret")
+							return isFailed(createdBinding) && strings.Contains(createdBinding.Status.Conditions[0].Message, "failed to create secret")
 						}, timeout, interval).Should(BeTrue())
 					})
 				})
@@ -510,7 +510,7 @@ var _ = Describe("ServiceBinding controller", func() {
 				createdBinding.Spec.ExternalName = "new-external-name"
 				err := k8sClient.Update(context.Background(), createdBinding)
 				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring("service binding spec cannot be modified after creation"))
+				Expect(err.Error()).To(ContainSubstring("updating service bindings is not supported"))
 			})
 		})
 
@@ -519,7 +519,7 @@ var _ = Describe("ServiceBinding controller", func() {
 				createdBinding.Spec.ServiceInstanceName = "new-instance-name"
 				err := k8sClient.Update(context.Background(), createdBinding)
 				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring("service binding spec cannot be modified after creation"))
+				Expect(err.Error()).To(ContainSubstring("updating service bindings is not supported"))
 			})
 		})
 
@@ -530,7 +530,7 @@ var _ = Describe("ServiceBinding controller", func() {
 				}
 				err := k8sClient.Update(context.Background(), createdBinding)
 				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring("service binding spec cannot be modified after creation"))
+				Expect(err.Error()).To(ContainSubstring("updating service bindings is not supported"))
 			})
 		})
 

controllers/serviceinstance_controller.go

@@ -151,9 +151,9 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, smClient sm.Client
 			if err := r.updateStatusWithRetries(ctx, serviceInstance, log); err != nil {
 				return ctrl.Result{}, err
 			}
-			errMsg := "Async deprovision operation failed"
+			errMsg := "async deprovisioning operation failed"
 			if status.Errors != nil {
-				errMsg = fmt.Sprintf("Async deprovision operation failed, errors: %s", string(status.Errors))
+				errMsg = fmt.Sprintf("%s. Errors: %s", errMsg, string(status.Errors))
 			}
 			return ctrl.Result{}, fmt.Errorf(errMsg)
 		}

hack/kubectl-sapbtp

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 marketplace() {
-  local secret=$(kubectl get secret sapbtp-operator-secret  -n "$1" -o json)
+  local secret=$(kubectl get secret sap-btp-service-operator  -n "$1" -o json)
   local url=$(jq -r .data.url <<< "$secret" | base64 --decode)
   local token=$(_token "$secret")
 
@@ -28,7 +28,7 @@ marketplace() {
 
 
 plans() {
-  local secret=$(kubectl get secret sapbtp-operator-secret  -n "$1" -o json)
+  local secret=$(kubectl get secret sap-btp-service-operator  -n "$1" -o json)
   local url=$(jq -r .data.url <<< "$secret" | base64 --decode)
   local token=$(_token "$secret")
   local plans=$(curl -f -Ss -L  -H "Authorization: Bearer ${token}" "$url/v1/service_plans")
@@ -40,7 +40,7 @@ plans() {
 }
 
 services() {
-  local secret=$(kubectl get secret sapbtp-operator-secret  -n "$1" -o json)
+  local secret=$(kubectl get secret sap-btp-service-operator  -n "$1" -o json)
   local url=$(jq -r .data.url <<< "$secret" | base64 --decode)
   local token=$(_token "$secret")
   local services=$(curl -f -Ss -L  -H "Authorization: Bearer ${token}" "$url/v1/service_offerings")

hack/setup_operator_env.sh

@@ -42,7 +42,7 @@ data:
   CLUSTER_ID: "$clusterid"
   MANAGEMENT_NAMESPACE: "$management_namespace"
 metadata:
-  name: sapbtp-operator-config
+  name: sap-btp-operator-config
   namespace: operators
 EOT
 }
@@ -59,7 +59,7 @@ store_secret() {
 apiVersion: v1
 kind: Secret
 metadata:
-  name: sapbtp-operator-secret
+  name: sap-btp-service-operator
   namespace: $management_namespace
 type: Opaque
 data:

internal/secrets/resolver.go

@@ -14,7 +14,7 @@ import (
 //TODO + revisit the name based approach for managed secret, replace with label based mechanism + admission webhook for secrets to avoid duplications
 
 const (
-	SAPBTPOperatorSecretName = "sapbtp-operator-secret"
+	SAPBTPOperatorSecretName = "sap-btp-service-operator"
 )
 
 type SecretResolver struct {
@@ -63,7 +63,8 @@ func (sr *SecretResolver) GetSecretForResource(ctx context.Context, namespace st
 
 	if !found {
 		// secret not found anywhere
-		return nil, fmt.Errorf("cannot find sapbtp operator secret")
+		sr.Log.Info("secret not found")
+		return nil, fmt.Errorf("secret not found")
 	}
 
 	if err := validateSAPBTPOperatorSecret(secretForResource); err != nil {
@@ -77,12 +78,12 @@ func (sr *SecretResolver) GetSecretForResource(ctx context.Context, namespace st
 func validateSAPBTPOperatorSecret(secret *v1.Secret) error {
 	secretData := secret.Data
 	if secretData == nil {
-		return fmt.Errorf("invalid secret - secret data is missing")
+		return fmt.Errorf("invalid secret: data is missing. Check the fields and try again")
 	}
 
 	for _, field := range []string{"clientid", "clientsecret", "url", "tokenurl"} {
 		if len(secretData[field]) == 0 {
-			return fmt.Errorf("invalid secret - %s is missing", field)
+			return fmt.Errorf("invalid secret: '%s' field is missing", field)
 		}
 	}
 

internal/secrets/resolver_test.go

@@ -74,7 +74,7 @@ var _ = Describe("Secrets Resolver", func() {
 	validateSecretNotResolved := func() {
 		_, err := resolver.GetSecretForResource(ctx, testNamespace)
 		Expect(err).To(HaveOccurred())
-		Expect(err.Error()).To(ContainSubstring("cannot find sapbtp operator secret"))
+		Expect(err.Error()).To(ContainSubstring("secret not found"))
 	}
 
 	BeforeEach(func() {

sapbtp-operator-charts/Chart.yaml

@@ -1,5 +1,5 @@
 apiVersion: v2
 appVersion: "master"
-name: sapbtp-operator
+name: sap-btp-operator
 description: A Helm chart for SAP BTP Operator for Kubernetes
 version: 0.1.0

sapbtp-operator-charts/templates/configmap.yml

@@ -4,5 +4,5 @@ data:
   MANAGEMENT_NAMESPACE: {{.Release.Namespace}}
 kind: ConfigMap
 metadata:
-  name: sapbtp-operator-config
+  name: sap-btp-operator-config
   namespace: {{.Release.Namespace}}

sapbtp-operator-charts/templates/deployment.yml

@@ -3,7 +3,7 @@ kind: Deployment
 metadata:
   labels:
     control-plane: controller-manager
-  name: sapbtp-operator-controller-manager
+  name: sap-btp-operator-controller-manager
   namespace: {{.Release.Namespace}}
 spec:
   replicas: 1
@@ -33,7 +33,7 @@ spec:
             - /manager
           envFrom:
             - configMapRef:
-                name: sapbtp-operator-config
+                name: sap-btp-operator-config
           image: {{.Values.manager.image.repository}}:{{.Values.manager.image.tag}}
           imagePullPolicy: IfNotPresent
           name: manager

sapbtp-operator-charts/templates/secret.yml

@@ -1,7 +1,7 @@
 apiVersion: v1
 kind: Secret
 metadata:
-  name: sapbtp-operator-secret
+  name: sap-btp-service-operator
   namespace: {{ .Release.Namespace }}
 type: Opaque
 data: