Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/s3 sub user #16

Merged
merged 36 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
d1916ce
fix assert step in step 6
hoptical Oct 21, 2023
cf0cffb
add subUsers creation and remove
hoptical Oct 23, 2023
5113b10
add teststep for adding subUsers to e2e
hoptical Oct 23, 2023
0088200
add ensure subUsersSecret and fix subUser recreation
hoptical Oct 23, 2023
b248d4e
add subUsers to status
hoptical Oct 23, 2023
de5251b
fix subUser spell
hoptical Oct 23, 2023
5c2c252
fix create subuser bug
hoptical Oct 23, 2023
333de3e
add initSpecBasedVars
hoptical Oct 28, 2023
6fc674c
complete test step for subuser creation and deletion
hoptical Oct 28, 2023
2eb728b
use functions instead of map for generating subuser fullIds
hoptical Oct 29, 2023
476d5b4
add e2e-test-local to Makefile
hoptical Oct 29, 2023
2fd5b13
remove redundant subusers in testing steps
hoptical Oct 29, 2023
6323355
add subUser validator to s3userclaim webhook
hoptical Oct 29, 2023
04d8a31
add bucket-policy test step
hoptical Oct 29, 2023
765e74e
add s3subuserbinding type
hoptical Oct 29, 2023
85d6b50
add untested bucket policy logic
hoptical Oct 30, 2023
033e286
revise bucket test
hoptical Oct 30, 2023
199a023
fix testing problem
hoptical Oct 31, 2023
5eb0228
user pointer for s3userclaim webhook
hoptical Nov 3, 2023
8d71e84
add update bucket status for success and failure deletion
hoptical Nov 3, 2023
6818e0d
add some comments
hoptical Nov 3, 2023
dc97591
update e2e test doc
hoptical Nov 3, 2023
497c3aa
fix make test
hoptical Nov 4, 2023
11c8cd3
remove ensure readonly user
hoptical Nov 4, 2023
5c2e0fd
remove redundant comment in s3userclaim webhook
hoptical Nov 4, 2023
ed903cf
add optional tag marker to Access filed of SubUserBinding
hoptical Nov 4, 2023
bc70bad
rename functions to follow the verb pattern
hoptical Nov 4, 2023
4b272d0
break down syncSubusersList to smaller functions
hoptical Nov 4, 2023
9f4b069
add comment on syncSubusersList function
hoptical Nov 4, 2023
ff41616
use marker validation instead of webhook for subusers name in bucket
hoptical Nov 4, 2023
252d8da
remove map suffix
hoptical Nov 4, 2023
36842fc
implement bucketAccessAction as a function instead of an attribute
hoptical Nov 4, 2023
26deb51
fix linting
hoptical Nov 4, 2023
3a6c1a5
place the remove subUser log before its action
hoptical Nov 4, 2023
7aa4f6e
fix capital case letter of BucketAccessAction
hoptical Nov 4, 2023
9b648c7
convert subUser form to subuser
hoptical Nov 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions api/v1alpha1/s3bucket_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions api/v1alpha1/s3userclaim_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
35 changes: 13 additions & 22 deletions api/v1alpha1/s3userclaim_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 13 additions & 0 deletions api/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
hoptical marked this conversation as resolved.
Show resolved Hide resolved
Access string `json:"access,omitempty"`
}
39 changes: 37 additions & 2 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 36 additions & 0 deletions config/crd/bases/s3.snappcloud.io_s3buckets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down
10 changes: 10 additions & 0 deletions config/crd/bases/s3.snappcloud.io_s3userclaims.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion config/manager/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
29 changes: 16 additions & 13 deletions docs/E2E-TEST-WORKFLOW.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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. |
2 changes: 2 additions & 0 deletions internal/controllers/s3bucket/cleaner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading