Uh oh!
There was an error while loading. Please reload this page.
Add draft ADR on S3 object definition - #177
Conversation
sbernauer
left a comment
There was a problem hiding this comment.
Hi Sönke, thanks for writing this down, looks very good!
One important question: Are S3Bucket and S3Connection objects namespaced?
The other thing remaining for me is to talk about the way to handle the credentials.
I think it is sufficient for the first revision to only support anonymus and access + secretkey.
I don't know if the secretClass is an overkill for this. On the other hand it would give us the following benefits
- Support stuff like automagically creating STS session tokens or some possible AWS concepts i don't know of
- [Only if the S3Connection has cluster scope]: Resolve the missmatch S3Connection cluster scope <-> namespaced Secret. The SecretClass has cluster scope.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
lfrancke
left a comment
There was a problem hiding this comment.
Can you maybe add (for option 5) how generated names come into play and/or how we are compatible with ObjectBucketClaim (OBC)?
As far as I understand it we do would now need to have a controller that converts from the OBC ConfigMap & Secret to our own S3Bucket and/or S3Connection, is that correct?
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
soenkeliebau
commented
Apr 14, 2022
I think they should be namespaced, there is a likelihood that for example the S3Connection could refer to a minio that is running inside of a namespace for example. |
Uh oh!
There was an error while loading. Please reload this page.
razvan
commented
Apr 14, 2022
IMO there are some open questions on the operator side of things. How are multiple S3 connections used in product CRDs ? In addition to the connection details, operators need to know the purpose of each connection (what to do with them). Is there an implicit purpose based on the role/group that contains the connection definition, or are connection labeled/named with a "purpose" label. Are S3 connections defined "globally" in the CRD and referenced from respective role/groups or are they inlined in the role/group configuration? How do product CRDs refer to multiple S3 connections for the same "purpose" (ex. reading data) ? |
Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de> Co-authored-by: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Co-authored-by: Lars Francke <github@lars-francke.de>
All good questions, but I'd argue that some of these will need to be evaluated per operator. What this ADR is trying to solve is mostly the structure of shared S3 objects. I'll try answering inline.
Operator CRDs should choose appropriate naming for field referencing S3Buckets so that the user knows what is meant. For example a fictional product that simply offers backup of data from one S3 bucket into a second S3 bucket the CRD could look like this: ---
apiVersion: s3backupinator.stackable.tech/v1alpha1kind: S3BackupinatorClustermetadata:
name: simple-s3backupinatorspec:
version: "2.57.14"sourceBucket: simple-buckettargetBucket: simple-bucket-backup
---
apiVersion: v1kind: S3Bucketmetadata:
name: simple-bucketdata:
bucketName: myveryimportantdatas3Connection:
inline:
host: test-minioport: 9000secretClass: minio-credentialstls:
verification:
server:
caCert:
secretClass: my-s3-ca
---
apiVersion: v1kind: S3Bucketmetadata:
name: simple-bucket-backupdata:
bucketName: myveryimportantdatas3Connection:
inline:
host: backup-minioport: 9023secretClass: minio-credentialstls:
verification:
server:
caCert:
secretClass: my-s3-ca
I would expect this to normally be a global setting. At least I cannot currently come up with a scenario in which it would make sense to refer to different buckets from different roles/groups. But this is something that we'll need to decide per operator.
Sorry to keep giving the same answer, but this will again be operator specific. ---
apiVersion: s3querynator.stackable.tech/v1alpha1kind: S3QuerynatorClustermetadata:
name: simple-s3querynatorspec:
version: "1.13.11"queryableBuckets:
- simple-bucket
- simple-bucket2
---
apiVersion: v1kind: S3Bucketmetadata:
name: simple-bucketdata:
bucketName: myveryimportantdatas3Connection:
inline:
host: test-minioport: 9000secretClass: minio-credentialstls:
verification:
server:
caCert:
secretClass: my-s3-ca
---
apiVersion: v1kind: S3Bucketmetadata:
name: simple-bucket2data:
bucketName: mynotsoimportantdatas3Connection:
inline:
host: aws-s3-endpointport: 9000secretClass: aws-credentialstls:
verification:
server:
caCert:
secretClass: my-s3-ca |
soenkeliebau
commented
Apr 14, 2022
That is correct. I intentionally left that out of this ADR, as it is not really related to the options outlined here, but instead an unrelated decision to take. Whatever we decide here will have to be implemented by that operator. I have added some more detail around this to the ADR. |
fhennig
left a comment
There was a problem hiding this comment.
looks good to me, thanks for writing it all down!
Uh oh!
There was an error while loading. Please reload this page.
soenkeliebau
commented
Apr 29, 2022
Sorry, addressed one more comment that I had missed. |
Adds ADR on how we want to represent S3 buckets in our CRD structure.