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

Gateway mode added with storage options s3 and azure. #27

Merged
merged 4 commits into from
Jan 27, 2022
Merged

Gateway mode added with storage options s3 and azure. #27

merged 4 commits into from
Jan 27, 2022

Conversation

Barteus
Copy link
Contributor

@Barteus Barteus commented Jan 18, 2022

Gateway mode implemented without caching.

@Barteus Barteus requested a review from a team as a code owner January 18, 2022 11:30
@DomFleischmann
Copy link
Contributor

Thank you for the contribution @Barteus can you tell us a bit more on how you have tested this? What storage options besides azure did you try?

Also if you could provide us some documentation to test it ourselves that would be great!

@ca-scribner and @knkski please take a look at this PR.

@Barteus
Copy link
Contributor Author

Barteus commented Jan 18, 2022

I have checked for azure and s3 storage options.

Scenarios checked (both azure and s3 if applicable):

  • run default in server mode
  • green path, proper credentials
  • bad mode - message error in juju status
  • bad storage mode - container failure in juju status, log in container shows why
  • bad credentials - container failure in juju status, log in container shows why

config.yaml Outdated Show resolved Hide resolved
@ca-scribner
Copy link
Contributor

I tested this with S3 and it worked for me as well. Actually it worked easier than I expected and I wasn't sure how/where the access/secret keys were used. @Barteus do you understand how exactly this works? I read through the docs a bit and wasn't certain. Am I right that we must:

  • provide access-key/secret-key that is allowed to access the S3 storage of interest
  • provide the above creds to the charm, which then starts minio with those credentials
  • provide those same creds to anyone connecting to the minio API (eg through mc or through a charm interfacing with this charm via a relation) so they can connect to the minio api with them (I guess minio uses those creds as it's auth as well as uses them to auth with S3?)

We should probably add some documentation in the readme that describes how to deploy and interface with these. We could also add integration tests that connect to a real S3 store somewhere (and could even have a client connect through minio to that store), but that might be overkill

@Barteus
Copy link
Contributor Author

Barteus commented Jan 19, 2022

I fully agree with the first two points.

About providing users access to Minio buckets and console (currently on random port each time). You can share your S3 AWS creds, but for security reasons, it is not advisable. There is another alternative - you can create a user in console and give them proper accesses (ie read-only or read-write or admin). Users will use their "Minio User" access key and secret.

@ca-scribner
Copy link
Contributor

Re creds yeah that seemed odd. So am I right that you're saying instead of using the aws creds both as the login to minio and minio's login to aws, we can:

  1. give minio the aws creds (like done in the current examples) so minio can access S3
  2. create creds in minio that are used by users (/other charms) to log into minio (which then lets them see the S3 storage from point (1))

@Barteus
Copy link
Contributor Author

Barteus commented Jan 19, 2022

Yes, exactly as you described. Creating users works for both the server and gateway mode.

@ca-scribner
Copy link
Contributor

I wonder what a clean integration of this user management would like like in charming terms. An action in this charm to create users (assuming there's a CLI method for that in minio, which I'd guess there is) seems straightforward. The actual relating/using of the minio charm would probably be different though (right now minio provides related charms with creds, whereas if using separate user accounts we'd almost want the reverse).

To start though we could just separate the user facing and aws facing credentials in the config. So the current access/secret key config options are for users, and if you're using gateway you specify the gateway type and creds for the gateway separately

Copy link
Contributor

@DomFleischmann DomFleischmann left a comment

Choose a reason for hiding this comment

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

With the README added, this LGTM

@Barteus
Copy link
Contributor Author

Barteus commented Jan 19, 2022

I have added a new feature request (issue) for user management actions - #28

@Barteus
Copy link
Contributor Author

Barteus commented Jan 19, 2022

About the split, I would rather leave the minio user credentials for minio binary to handle and keep the single key/secret configuration for minio to use. In both modes, you can use key/secret passed in the charm to connect and use minio.

src/charm.py Outdated Show resolved Hide resolved
config.yaml Outdated Show resolved Hide resolved
@ca-scribner
Copy link
Contributor

I added two tiny requests but otherwise it looks great and I'm good to merge it either way

Copy link
Contributor

@DomFleischmann DomFleischmann left a comment

Choose a reason for hiding this comment

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

LGTM

@DomFleischmann DomFleischmann merged commit 65b1ccb into canonical:master Jan 27, 2022
@Barteus Barteus deleted the feature/azure-gateway branch February 2, 2022 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants