Fix/ensuring existing secrets resulting in panic(#23)

* fix subreconciler requeue handling for setting controller reference

* add webhook validation for existing secrets

* add validation webhook test phase for existing secrets

* revise forbidden error in validate secrets webhook

Author: hoptical

.gitignore

@@ -24,8 +24,9 @@ Dockerfile.cross
 *.swp
 *.swo
 *~
-
+.vscode
 vendor/
+
 # Downloaded go release for building testing Dockerfile
 testing/go*.tar.gz
 # Container id files of dev env containers

api/v1alpha1/s3userclaim_webhook.go

@@ -71,6 +71,9 @@ func (suc *S3UserClaim) ValidateCreate() error {
 
 	allErrs = validateQuota(suc, allErrs)
 
+	secretNames := []string{suc.Spec.AdminSecret, suc.Spec.ReadonlySecret}
+	allErrs = validateSecrets(secretNames, suc.Namespace, allErrs)
+
 	if len(allErrs) == 0 {
 		return nil
 	}
@@ -97,6 +100,17 @@ func (suc *S3UserClaim) ValidateUpdate(old runtime.Object) error {
 
 	allErrs = validateQuota(suc, allErrs)
 
+	// validate against updated secret names
+	var secretNames []string
+	if oldS3UserClaim.Spec.AdminSecret != suc.Spec.AdminSecret {
+		secretNames = append(secretNames, suc.Spec.AdminSecret)
+	}
+	if oldS3UserClaim.Spec.ReadonlySecret != suc.Spec.ReadonlySecret {
+		secretNames = append(secretNames, suc.Spec.ReadonlySecret)
+	}
+
+	allErrs = validateSecrets(secretNames, suc.Namespace, allErrs)
+
 	if len(allErrs) == 0 {
 		return nil
 	}
@@ -224,3 +238,23 @@ func validateAgainstClusterQuota(ctx context.Context, suc *S3UserClaim) error {
 
 	return nil
 }
+
+func validateSecrets(secretNames []string, namespace string, allErrs field.ErrorList) field.ErrorList {
+	ctx, cancel := context.WithTimeout(context.Background(), ValidationTimeout)
+	defer cancel()
+	for _, secretName := range secretNames {
+		existingSecret := &v1.Secret{}
+		switch err := runtimeClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: secretName}, existingSecret); {
+		case apierrors.IsNotFound(err):
+			continue
+		case err != nil:
+			allErrs = append(allErrs,
+				field.InternalError(field.NewPath("spec"), fmt.Errorf("error with getting secret '%s': %w", secretName, err)))
+		default:
+			allErrs = append(allErrs,
+				field.Forbidden(field.NewPath("spec"), fmt.Sprintf("secret %s exists", secretName)))
+		}
+	}
+	return allErrs
+
+}

internal/controllers/s3userclaim/provisioner.go

@@ -327,7 +327,8 @@ func (r *Reconciler) ensureSecret(ctx context.Context, secret *corev1.Secret) (*
 			!metav1.IsControlledBy(existingSecret, r.s3UserClaim) {
 			existingSecret.Data = secret.Data
 			if err := ctrl.SetControllerReference(r.s3UserClaim, existingSecret, r.scheme); err != nil {
-				return nil, err
+				r.logger.Error(err, "failed to set controller reference", "secret name", secret.Name)
+				return subreconciler.Requeue()
 			}
 			if err := r.Update(ctx, existingSecret); err != nil {
 				r.logger.Error(err, "failed to update secret", "name", secret.Name)

pkg/consts/consts.go

@@ -25,7 +25,7 @@ const (
 	ErrClusterQuotaNotDefined      = CustomError("cluster quota is not defined")
 	S3UserClassImmutableErrMessage = "s3UserClass is immutable"
 	S3UserRefImmutableErrMessage   = "s3UserRef is immutable"
-	S3UserRefNotFoundErrMessage    = "There is no s3UserClaim regarding the defined s3UserRef"
+	S3UserRefNotFoundErrMessage    = "there is no s3UserClaim regarding the defined s3UserRef"
 	ContactCloudTeamErrMessage     = "please contact the cloud team"
 
 	FinalizerPrefix             = "s3.snappcloud.io/"

testing/e2e/04-validation-webhook.yaml

@@ -5,10 +5,13 @@ commands:
     ignoreFailure: true
   - command: kubectl apply -f s3bucket-wrong-s3userref.yaml
     ignoreFailure: true
+  - command: kubectl apply -f create-s3user-with-existing-secret.yaml
+    ignoreFailure: true
   - command: kubectl delete s3userclaim s3userclaim-sample -n s3-test
     ignoreFailure: true
 assert:
   - s3bucket-ok.yaml
   - 02-assert.yaml
 error:
   - s3bucket-wrong-s3userref.yaml
+  - create-s3user-with-existing-secret.yaml

testing/e2e/create-s3user-with-existing-secret.yaml

@@ -0,0 +1,16 @@
+# Creating s3userclaim with existing secrets must be deined
+apiVersion: s3.snappcloud.io/v1alpha1
+kind: S3UserClaim
+metadata:
+  name: s3userclaim-sample2
+  namespace: s3-test
+spec:
+  s3UserClass: ceph-default
+  # existing secrets
+  readonlySecret: s3-sample-readonly-secret
+  adminSecret: s3-sample-admin-secret
+  quota:
+    maxSize: 100
+    maxObjects: 50
+    maxBuckets: 5
+