Feature/add s3 sub user (#16)

* fix assert step in step 6 * add subUsers creation and remove * add teststep for adding subUsers to e2e * add ensure subUsersSecret and fix subUser recreation * add subUsers to status * fix subUser spell * fix create subuser bug * add initSpecBasedVars * complete test step for subuser creation and deletion * use functions instead of map for generating subuser fullIds * add e2e-test-local to Makefile * remove redundant subusers in testing steps * add subUser validator to s3userclaim webhook * add bucket-policy test step * add s3subuserbinding type * add untested bucket policy logic * revise bucket test * fix testing problem * user pointer for s3userclaim webhook * add update bucket status for success and failure deletion * add some comments * update e2e test doc * fix make test * remove ensure readonly user * remove redundant comment in s3userclaim webhook * add optional tag marker to Access filed of SubUserBinding * rename functions to follow the verb pattern * break down syncSubusersList to smaller functions * add comment on syncSubusersList function * use marker validation instead of webhook for subusers name in bucket * remove map suffix * implement bucketAccessAction as a function instead of an attribute * fix linting * place the remove subUser log before its action * fix capital case letter of BucketAccessAction * convert subUser form to subuser
snapp-incubator · Nov 6, 2023 · d599a76 · d599a76
1 parent 6e34033
commit d599a76
Show file tree

Hide file tree

Showing 38 changed files with 609 additions and 108 deletions.
diff --git a/Makefile b/Makefile
@@ -114,8 +114,16 @@ test: manifests generate fmt vet envtest setup-dev-env ## Run tests.
 .PHONY: e2e-test
 e2e-test: docker-build # Run e2e tests
 	kubectl kuttl test
-##@ Build
 
+.PHONY: e2e-test-local
+e2e-test-local: docker-build # Run e2e tests for local development purposes
+	kind load docker-image $(IMG)
+	kubectl delete pod -n s3-operator-system -l control-plane=controller-manager
+	kubectl delete s3b --all -A
+	kubectl delete s3u --all -A
+	kubectl kuttl test --start-kind=false
+
+##@ Build
 .PHONY: build
 build: manifests generate fmt vet ## Build manager binary.
 	go build -o bin/manager main.go

diff --git a/api/v1alpha1/s3bucket_types.go b/api/v1alpha1/s3bucket_types.go
@@ -29,13 +29,22 @@ type S3BucketSpec struct {
 	// +kubebuilder:validation:Enum=delete;retain
 	// +kubebuilder:default=delete
 	S3DeletionPolicy string `json:"s3DeletionPolicy,omitempty"`
+
+	// +kubebuilder:validation:Optional
+	S3SubuserBinding []SubuserBinding `json:"s3SubuserBinding,omitempty"`
 }
 
 // S3BucketStatus defines the observed state of S3Bucket
 type S3BucketStatus struct {
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=false
 	Ready bool `json:"ready,omitempty"`
+
+	// +kubebuilder:validation:Optional
+	Reason string `json:"reason,omitempty"`
+
+	// +kubebuilder:validation:Optional
+	S3SubuserBinding []SubuserBinding `json:"s3SubuserBinding,omitempty"`
 }
 
 //+kubebuilder:object:root=true

diff --git a/api/v1alpha1/s3userclaim_types.go b/api/v1alpha1/s3userclaim_types.go
@@ -34,6 +34,9 @@ type S3UserClaimSpec struct {
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default:={"maxSize":"5368709120", "maxObjects":"1000", "maxBuckets": 2}
 	Quota *UserQuota `json:"quota,omitempty"`
+
+	// +kubebuilder:validation:Optional
+	Subusers []Subuser `json:"subusers,omitempty"`
 }
 
 // S3UserClaimStatus defines the observed state of S3UserClaim
@@ -43,6 +46,9 @@ type S3UserClaimStatus struct {
 
 	// +kubebuilder:validation:Optional
 	S3UserName string `json:"s3UserName,omitempty"`
+
+	// +kubebuilder:validation:Optional
+	Subusers []Subuser `json:"subusers,omitempty"`
 }
 
 // +kubebuilder:object:root=true

diff --git a/api/v1alpha1/s3userclaim_webhook.go b/api/v1alpha1/s3userclaim_webhook.go
@@ -68,14 +68,15 @@ var _ webhook.Validator = &S3UserClaim{}
 
 func (suc *S3UserClaim) ValidateCreate() error {
 	s3userclaimlog.Info("validate create", "name", suc.Name)
+	allErrs := field.ErrorList{}
+
+	allErrs = validateQuota(suc, allErrs)
 
-	// Validate Quota
-	errorList := validateQuota(suc)
-	if len(errorList) == 0 {
+	if len(allErrs) == 0 {
 		return nil
 	}
 
-	return apierrors.NewInvalid(suc.GroupVersionKind().GroupKind(), suc.Name, errorList)
+	return apierrors.NewInvalid(suc.GroupVersionKind().GroupKind(), suc.Name, allErrs)
 }
 
 func (suc *S3UserClaim) ValidateUpdate(old runtime.Object) error {
@@ -95,9 +96,7 @@ func (suc *S3UserClaim) ValidateUpdate(old runtime.Object) error {
 		)
 	}
 
-	// Validate Quota
-	errorList := validateQuota(suc)
-	allErrs = append(allErrs, errorList...)
+	allErrs = validateQuota(suc, allErrs)
 
 	if len(allErrs) == 0 {
 		return nil
@@ -128,35 +127,32 @@ func (suc *S3UserClaim) ValidateDelete() error {
 	return nil
 }
 
-func validateQuota(suc *S3UserClaim) field.ErrorList {
+func validateQuota(suc *S3UserClaim, allErrs field.ErrorList) field.ErrorList {
 	ctx, cancel := context.WithTimeout(context.Background(), ValidationTimeout)
 	defer cancel()
 
-	errorList := field.ErrorList{}
-
 	quotaFieldPath := field.NewPath("spec").Child("quota")
 
 	// TODO(therealak12): refactor the code as there are similarities between two quota validator functions
 
 	switch err := validateAgainstNamespaceQuota(ctx, suc); {
 	case err == consts.ErrExceededNamespaceQuota:
-		errorList = append(errorList, field.Forbidden(quotaFieldPath, err.Error()))
+		allErrs = append(allErrs, field.Forbidden(quotaFieldPath, err.Error()))
 	case err != nil:
 		s3userclaimlog.Error(err, "failed to validate against cluster quota")
-		errorList = append(errorList, field.InternalError(quotaFieldPath, fmt.Errorf(consts.ContactCloudTeamErrMessage)))
+		allErrs = append(allErrs, field.InternalError(quotaFieldPath, fmt.Errorf(consts.ContactCloudTeamErrMessage)))
 	}
 
 	switch err := validateAgainstClusterQuota(ctx, suc); {
 	case err == consts.ErrExceededClusterQuota:
-		errorList = append(errorList, field.Forbidden(quotaFieldPath, err.Error()))
+		allErrs = append(allErrs, field.Forbidden(quotaFieldPath, err.Error()))
 	case goerrors.Is(err, consts.ErrClusterQuotaNotDefined):
-		errorList = append(errorList, field.Forbidden(quotaFieldPath, err.Error()))
+		allErrs = append(allErrs, field.Forbidden(quotaFieldPath, err.Error()))
 	case err != nil:
 		s3userclaimlog.Error(err, "failed to validate against cluster quota")
-		errorList = append(errorList, field.InternalError(quotaFieldPath, fmt.Errorf(consts.ContactCloudTeamErrMessage)))
+		allErrs = append(allErrs, field.InternalError(quotaFieldPath, fmt.Errorf(consts.ContactCloudTeamErrMessage)))
 	}
-
-	return errorList
+	return allErrs
 }
 
 func validateAgainstNamespaceQuota(ctx context.Context, suc *S3UserClaim) error {
@@ -280,11 +276,6 @@ func findTeam(ctx context.Context, suc *S3UserClaim) (string, error) {
 		return "", fmt.Errorf("failed to get namespace, %w", err)
 	}
 
-	labels := ns.ObjectMeta.Labels
-	if labels == nil {
-		labels = map[string]string{}
-	}
-
 	team, ok := ns.ObjectMeta.Labels[consts.LabelTeam]
 	if !ok {
 		return "", fmt.Errorf("namespace %s doesn't have team label", ns.ObjectMeta.Name)

diff --git a/api/v1alpha1/types.go b/api/v1alpha1/types.go
@@ -11,3 +11,16 @@ type UserQuota struct {
 	// max number of buckets the user can create
 	MaxBuckets int `json:"maxBuckets,omitempty"`
 }
+
+// +kubebuilder:validation:Pattern=^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
+type Subuser string
+type SubuserBinding struct {
+	// name of the subuser
+	// +kubebuilder:validation:Required
+	Name string `json:"name"`
+	// access of the subuser which can be read or write
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:default=read
+	// +kubebuilder:validation:Enum=read;write
+	Access string `json:"access,omitempty"`
+}
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/s3.snappcloud.io_s3buckets.yaml b/config/crd/bases/s3.snappcloud.io_s3buckets.yaml
@@ -47,6 +47,23 @@ spec:
                 - delete
                 - retain
                 type: string
+              s3SubuserBinding:
+                items:
+                  properties:
+                    access:
+                      default: read
+                      description: access of the subuser which can be read or write
+                      enum:
+                      - read
+                      - write
+                      type: string
+                    name:
+                      description: name of the subuser
+                      type: string
+                  required:
+                  - name
+                  type: object
+                type: array
               s3UserRef:
                 type: string
             required:
@@ -58,6 +75,25 @@ spec:
               ready:
                 default: false
                 type: boolean
+              reason:
+                type: string
+              s3SubuserBinding:
+                items:
+                  properties:
+                    access:
+                      default: read
+                      description: access of the subuser which can be read or write
+                      enum:
+                      - read
+                      - write
+                      type: string
+                    name:
+                      description: name of the subuser
+                      type: string
+                  required:
+                  - name
+                  type: object
+                type: array
             type: object
         type: object
     served: true

diff --git a/config/crd/bases/s3.snappcloud.io_s3userclaims.yaml b/config/crd/bases/s3.snappcloud.io_s3userclaims.yaml
@@ -86,6 +86,11 @@ spec:
                 type: string
               s3UserClass:
                 type: string
+              subusers:
+                items:
+                  pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
+                  type: string
+                type: array
             required:
             - adminSecret
             - readonlySecret
@@ -116,6 +121,11 @@ spec:
                 type: object
               s3UserName:
                 type: string
+              subusers:
+                items:
+                  pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
+                  type: string
+                type: array
             type: object
         type: object
     served: true

diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml
@@ -6,4 +6,7 @@ kind: Kustomization
 images:
 - name: controller
   newName: controller
-  newTag: latest
+  newTag: latest
+- name: ghcr.io/snapp-incubator/s3-operator
+  newName: s3-operator
+  newTag: latest
diff --git a/docs/E2E-TEST-WORKFLOW.md b/docs/E2E-TEST-WORKFLOW.md
@@ -3,7 +3,7 @@
 The e2e tests are performed via [Kuttle](https://kuttl.dev/). Use the bellow command to run the tests:
 
 ```bash
-kubectl-kuttl test
+make e2e-test
 ```
 
 ## Test Workflow
@@ -12,15 +12,18 @@ Here is the test workflow:
 
 | Step | Action                                                       | Assertion                                                    |
 | ---- | ------------------------------------------------------------ | ------------------------------------------------------------ |
-| 0    | Install external-crds                                        | No assertion                                                 |
-| 0    | Install Ceph cluster                                         | No assertion                                                 |
-| 0    | Run the operator locally                                     | No assertion                                                 |
-| 1    | Create S3UserClaims                                          | S3UserClaim CR<br />S3User CR<br />Created user on ceph-rgw  |
-| 2    | Create S3Buckets: One with retain and one with delete DeletionPolicy mode. | S3Bucket CR<br />S3Bucket on ceph-rgw via aws-cli<br />aws-cli with user credentials **can** create or delete objects on the bucket |
-| 3    | Webhook validation: Update S3Bucket S3UserRef                | Must be **denied** by the bucket validation update webhook.  |
-| 3    | Webhook validation: Create S3bucket with wrong S3UserRef     | Must be **denied** by the bucket validation create webhook.  |
-| 3    | Webhook validation: Delete S3UserClaim                       | Must be **denied** by the user validation delete Webhook.    |
-| 4    | Delete S3Buckets                                             | DeletionPolicy on delete: bucket must **be** deleted.<br />DeletionPolicy on retain: bucket must **not be** deleted. |
-| 5    | Delete S3UserClaim for Sample user                           | S3UserClaim and S3User CRs are deleted.                      |
-| 6    | Delete S3bucket and S3UserClaim for Extra user               | S3bucket, S3UserClaim and S3User CRs are deleted.            |
-|      |                                                              |                                                              |
+| 0    | Install Cert Manager                                         | check if cert manager pods are ready                         |
+| 1    | Install Ceph cluster                                         | Check if ceph cluster pod is ready                           |
+| 1    | Install resource quota CRD                                   | No assertion                                                 |
+| 1    | Install the operator                                         | Check if operator pod is ready                               |
+| 2    | Create S3UserClaims                                          | S3UserClaim CR<br />S3User CR<br />Created user on ceph-rgw  |
+| 3    | Create S3Buckets: One with retain and one with delete DeletionPolicy mode. | S3Bucket CR<br />S3Bucket on ceph-rgw via aws-cli<br />aws-cli with user credentials **can** create or delete objects on the bucket |
+| 4    | Webhook validation: Update S3Bucket S3UserRef                | Must be **denied** by the bucket validation update webhook.  |
+| 4    | Webhook validation: Create S3bucket with wrong S3UserRef     | Must be **denied** by the bucket validation create webhook.  |
+| 4    | Webhook validation: Delete S3UserClaim                       | Must be **denied** by the user validation delete Webhook.    |
+| 5    | Add subuser                                                  | Check if subuser is added to the s3Userclaim status.<br />Check if subusers secrets are created. |
+| 6    | Add Bucket Access                                            | Check if subusers have correct access to the bucket.         |
+| 7    | Delete subuser                                               | Check if deleted subuser doesn't have access to the bucket.  |
+| 8    | Delete S3Buckets                                             | DeletionPolicy on delete: bucket must **be** deleted.<br />DeletionPolicy on retain: bucket must **not be** deleted. |
+| 9    | Delete S3UserClaim for Sample user                           | S3UserClaim and S3User CRs are deleted.                      |
+| 10   | Delete S3bucket and S3UserClaim for Extra user               | S3bucket, S3UserClaim and S3User CRs are deleted.            |
diff --git a/internal/controllers/s3bucket/cleaner.go b/internal/controllers/s3bucket/cleaner.go
@@ -44,6 +44,8 @@ func (r *Reconciler) removeOrRetainBucket(ctx context.Context) (*ctrl.Result, er
 			}
 		}
 		r.logger.Error(err, "failed to remove the bucket")
+		// update bucket status with failure reason; e.g. Bucket is not empty
+		r.updateBucketStatus(ctx, false, err.Error())
 		return subreconciler.Requeue()
 	}
 	return subreconciler.ContinueReconciling()