Skip to content

Implement multitenancy #50

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.

Sign up for GitHub

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

Jump to bottom
Closed
Donatien26 wants to merge 3 commits into from
Closed

Implement multitenancy #50

Donatien26 wants to merge 3 commits into from

Conversation

@Donatien26
Copy link
Collaborator

@Donatien26 Donatien26 commented Aug 26, 2024 •
edited
Loading

This PR allow to:

  • Hability to add labelSelector to filter which resource a S3Operator instance must manage
  • Hability to manage multiple s3 Instance

@Donatien26 Donatien26 force-pushed the implement-multitenancy branch from 444d4f4 to 34b85b3 Compare August 26, 2024 14:16
@sathieu
Copy link
Collaborator

sathieu commented Sep 13, 2024 •
edited
Loading

This PR allow to:

* Hability to add labelSelector to filter which resource a S3Operator instance must manage

* Hability to manage multiple s3 Instance

I think those two features are orthogonal. I would go for 2️⃣ only.

EDIT:

Proposed roadmap:

  • add s3InstanceRef, if empty the controller args are used (for backward compat) (this PR)
  • deprecate args to configure default instance
  • BREAKING: s3InstanceRef mandatory, and remove args to configure default instance

sathieu
sathieu reviewed Sep 13, 2024
View reviewed changes
@Donatien26 Donatien26 force-pushed the implement-multitenancy branch 3 times, most recently from c4bda93 to bc9818a Compare September 19, 2024 13:34
sathieu
sathieu requested changes Sep 26, 2024
View reviewed changes
Copy link
Collaborator

@sathieu sathieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is another set of reviews. Sorry for the added work.

I think the default instance stuff should be removed, the default instance could be added to the chart easily.

@Donatien26 Donatien26 force-pushed the implement-multitenancy branch 13 times, most recently from ae58bdd to f7549e0 Compare October 18, 2024 13:39
sathieu
sathieu requested changes Oct 18, 2024
View reviewed changes
Copy link
Collaborator

@sathieu sathieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few remaining comments. This is almost done 👍

@Donatien26 Donatien26 force-pushed the implement-multitenancy branch 5 times, most recently from 6362c60 to de342d8 Compare November 5, 2024 13:05
@Donatien26
Copy link
Collaborator Author

Donatien26 commented Nov 5, 2024

TODO don't encode base64 cert

sathieu
sathieu reviewed Nov 5, 2024
View reviewed changes
sathieu
sathieu reviewed Nov 5, 2024
View reviewed changes
sathieu
sathieu reviewed Nov 5, 2024
View reviewed changes
sathieu
sathieu reviewed Nov 5, 2024
View reviewed changes
sathieu
sathieu reviewed Nov 5, 2024
View reviewed changes
sathieu
sathieu reviewed Nov 5, 2024
View reviewed changes
phlg
phlg reviewed Nov 5, 2024
View reviewed changes
@Donatien26 Donatien26 force-pushed the implement-multitenancy branch from de342d8 to 4a29bd1 Compare November 7, 2024 16:03
@Donatien26 Donatien26 closed this Feb 14, 2025
@Donatien26
Copy link
Collaborator Author

Donatien26 commented Feb 14, 2025

close in favor of #57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sathieu sathieu sathieu requested changes

@phlg phlg phlg left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Donatien26 @sathieu @phlg