diff --git a/Makefile b/Makefile index a1263dbc..7269922f 100644 --- a/Makefile +++ b/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 diff --git a/README.md b/README.md index 4a6833df..d3cff052 100644 --- a/README.md +++ b/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//sapbtp-operator-.tgz \ + helm upgrade --install sap-btp-operator https://github.com/SAP/sap-btp-service-operator/releases/download//sapbtp-operator-.tgz \ --create-namespace \ - --namespace=sapbtp-operator \ + --namespace=sap-btp-operator \ --set manager.secret.clientid= \ --set manager.secret.clientsecret= \ --set manager.secret.url= \ diff --git a/api/v1alpha1/servicebinding_webhook.go b/api/v1alpha1/servicebinding_webhook.go index 7b0f2399..02ca80e5 100644 --- a/api/v1alpha1/servicebinding_webhook.go +++ b/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 diff --git a/client/sm/client.go b/client/sm/client.go index f1a221d8..399ba528 100644 --- a/client/sm/client.go +++ b/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 diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 02c1d98e..ee1c0420 100644 --- a/config/default/kustomization.yaml +++ b/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: diff --git a/controllers/base_controller.go b/controllers/base_controller.go index 83a4680a..2ab7e24b 100644 --- a/controllers/base_controller.go +++ b/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 diff --git a/controllers/servicebinding_controller.go b/controllers/servicebinding_controller.go index 6ac70e0b..70f54f74 100644 --- a/controllers/servicebinding_controller.go +++ b/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 } diff --git a/controllers/servicebinding_controller_test.go b/controllers/servicebinding_controller_test.go index d33439a6..a5dd2569 100644 --- a/controllers/servicebinding_controller_test.go +++ b/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")) }) }) diff --git a/controllers/serviceinstance_controller.go b/controllers/serviceinstance_controller.go index ab951c76..4f44f058 100644 --- a/controllers/serviceinstance_controller.go +++ b/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) } diff --git a/hack/kubectl-sapbtp b/hack/kubectl-sapbtp index 1e6fc7f8..227feac7 100644 --- a/hack/kubectl-sapbtp +++ b/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") diff --git a/hack/setup_operator_env.sh b/hack/setup_operator_env.sh index 06ced35c..10baef15 100755 --- a/hack/setup_operator_env.sh +++ b/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: diff --git a/internal/secrets/resolver.go b/internal/secrets/resolver.go index cc33e503..826bd7b8 100644 --- a/internal/secrets/resolver.go +++ b/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) } } diff --git a/internal/secrets/resolver_test.go b/internal/secrets/resolver_test.go index 46c8fb31..5eda70b6 100644 --- a/internal/secrets/resolver_test.go +++ b/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() { diff --git a/sapbtp-operator-charts/Chart.yaml b/sapbtp-operator-charts/Chart.yaml index 6534b36b..7e6915cd 100644 --- a/sapbtp-operator-charts/Chart.yaml +++ b/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 diff --git a/sapbtp-operator-charts/templates/configmap.yml b/sapbtp-operator-charts/templates/configmap.yml index 4ee344cd..0a5f6179 100644 --- a/sapbtp-operator-charts/templates/configmap.yml +++ b/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}} \ No newline at end of file diff --git a/sapbtp-operator-charts/templates/deployment.yml b/sapbtp-operator-charts/templates/deployment.yml index a86b9408..dfb4f2f0 100644 --- a/sapbtp-operator-charts/templates/deployment.yml +++ b/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 diff --git a/sapbtp-operator-charts/templates/secret.yml b/sapbtp-operator-charts/templates/secret.yml index b88cd368..0dcbc2b1 100644 --- a/sapbtp-operator-charts/templates/secret.yml +++ b/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: