Operator doesn't reconcile if the bucket/policybinding already exists

[codestation]

In my case i already had some buckets/policies created, then added manifests for the existing ones and more for the new ones that intended to create.

The new ones are created without a problem, but the existing ones always fail since minio reports that they already exists. For example:

time=2024-11-28T15:16:27.473Z level=ERROR msg="Reconciler error" name=operator controller=miniobucket controllerGroup=bfiola.dev controllerKind=MinioBucket MinioBucket.name=test-bucket MinioBucket.namespace=minio namespace=minio name=test-bucket reconcileID=b788f5c3-c57e-4a15-9d79-c25a755bee32 err="Your previous request to create the named bucket succeeded and you already own it.

time=2024-11-28T15:18:59.855Z level=ERROR msg="Reconciler error" name=operator controller=miniopolicybinding controllerGroup=bfiola.dev controllerKind=MinioPolicyBinding MinioPolicyBinding.name=test-policy MinioPolicyBinding.namespace=minio namespace=minio name=test-policy reconcileID=c6dcdf2d-d2f9-4d5d-9299-e3243e96b593 err="The specified policy change is already in effect. (Specified policy update has no net effect)"

I am not sure if the policies also have this problem because they fail with another error:

time=2024-11-28T15:19:03.343Z level=ERROR msg="Reconciler error" name=operator controller=miniopolicy controllerGroup=bfiola.dev controllerKind=MinioPolicy MinioPolicy.name=test-policy MinioPolicy.namespace=minio namespace=minio name=test-policy reconcileID=30bf723a-0d8c-4ef7-b3f6-9058b35d86ed err="invalid version '2012-10-17T00:00:00Z'"

My manifests are the following:

apiVersion: bfiola.dev/v1
kind: MinioBucket
metadata:
  name: test-bucket
spec:
  deletionPolicy: IfEmpty
  name: test-bucket
  tenantRef:
    namespace: minio
    name: minio
---
apiVersion: bfiola.dev/v1
kind: MinioUser
metadata:
  name: test-user
spec:
  accessKey: test
  secretKeyRef:
    name: test-user
    key: AWS_SECRET_ACCESS_KEY
  tenantRef:
    namespace: minio
    name: minio
---
apiVersion: bfiola.dev/v1
kind: MinioPolicy
metadata:
  name: test-policy
spec:
  name: test-access
  statement:
    - effect: "Allow"
      action:
        - "s3:*"
      resource:
        - "arn:aws:s3:::test-bucket/*"
        - "arn:aws:s3:::test-bucket"
  tenantRef:
    namespace: minio
    name: minio
  version: 2012-10-17
---
apiVersion: bfiola.dev/v1
kind: MinioPolicyBinding
metadata:
  name: test-policy
spec:
  policy: test-access
  tenantRef:
    namespace: minio
    name: minio
  user:
    builtin: test

The operator should handle those cases and expect that some resources could be already in there, else apply the changes needed so they are the same as their manifest.

[benfiola]

Hey there, thanks for checking out the operator! 🎉

While not set in stone, I did this intentionally - I didn’t want the operator to potentially modify resources that it didn’t originally create - so the operator allows minio to fail in these situations.

However, if we think this is needlessly cautious - it will certainly simplify my code and the conditions I test for.

[benfiola]

Thinking a little more about this, an unintended consequence of this is that the operator will allow duplicate resources.

For example, several MinioBucket resources could point to the same bucket. Currently, allowing minio to fail when a bucket exists prevents this from happening. If all MinioBucket resources think they ‘own’ the underlying bucket, deleting any of these resources will potentially delete that underlying bucket.

I personally don’t think this would be desirable behavior as it subverts expectations - I would be surprised to find that one MinioBucket’s data had been deleted because another MinioBucket was deleted.

[codestation]

For example, several MinioBucket resources could point to the same bucket

This shouldn't be a validation error? The bucket name should be unique for the same tenantRef. Same with users (accessKey) and policies.

I opened the issue because i am not clear on how to handle a migration where an user (me) already has buckets/users/policies in place and want to convert those to declarative manifests. I guess i could delete all users and policies and let the operator recreate them (this would cause a brief downtime for affected services), but i cannot do that with buckets because they have multiple TB of data.

Maybe adding a flag like inheritExisting: true to the CRDs to give the OK to the operator to grab the existing resources for itself.

[benfiola]

This shouldn't be a validation error? The bucket name should be unique for the same tenantRef. Same with users (accessKey) and policies.

Yes, however the operator currently doesn't implement admission webhooks and is incapable of performing pre-submission validation. (- though this is something I'd like to do soon!).

Currently, we keep track of whether a kubernetes resource reconciled successfully, and treat errors like 'minio bucket already exists' as reconciliation failures. If I create four identical MinioBucket resources - one will reconcile successfully and create the bucket, meanwhile the three others will fail to reconcile. It's not perfect ... but in the absence of admission webhooks, I think it works reasonably well.

I opened the issue because i am not clear on how to handle a migration where an user (me) already has buckets/users/policies in place and want to convert those to declarative manifests. I guess i could delete all users and policies and let the operator recreate them (this would cause a brief downtime for affected services), but i cannot do that with buckets because they have multiple TB of data.

Yeah, I think that there should be a better story around migration than what currently exists ...

Maybe adding a flag like inheritExisting: true to the CRDs to give the OK to the operator to grab the existing resources for itself.

... and I think this is a great idea! I'll get something going ASAP - thanks for the suggestion!