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

minio: declarative creation of buckets/groups/policies/users (#89559) #164235

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pinpox
Copy link
Member

@pinpox pinpox commented Mar 15, 2022

Description of changes

I'd like to add the functionality as requested in #89559 to allow creating buckets, groups, users and policies declaratively in minio. The units added are based on the work of @expipiplus1 and @JosephLucas.

Pinging @bachp and @hamishmack for review and test. I'm going to need help testing and implementing this, let me know where to start and what is still missing. There might be more elegant ways of doing this.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Mar 15, 2022
@aanderse
Copy link
Member

Oh no... more ensure* options... This leads to configuration files which don't match the actual configuration of a machine, doesn't it? NixOS machines which are difficult to reason about aren't very good NixOS machines...

@pinpox
Copy link
Member Author

pinpox commented Mar 18, 2022

@aanderse Hm, you might be right on that. The solution is not really declarative, do you have any proposal on how we could have s3 stuff like buckets, policies and users scripted in minio with nix? I have looked at the config files that get generated when you create these entities via the GUI, but I'm not sure how to create those from nix directly.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 21, 2022
@RaitoBezarius
Copy link
Member

Oh no... more ensure* options... This leads to configuration files which don't match the actual configuration of a machine, doesn't it? NixOS machines which are difficult to reason about aren't very good NixOS machines...

While I agree with you, I feel like this should be end-user responsibility to choose or not, this compromise.

Bootstrapping many MinIO resources is very annoying and require to use their ugly GUI, I am very interested into having something like this.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 1, 2022
@aanderse
Copy link
Member

aanderse commented Dec 1, 2022

@RaitoBezarius it sounds like you're looking for a solution more like terraform, not NixOS... The two can go well together. Let's not mix them up.

@RaitoBezarius
Copy link
Member

@RaitoBezarius it sounds like you're looking for a solution more like terraform, not NixOS... The two can go well together. Let's not mix them up.

While it's true that Terraform providers or NixOps providers might do a better job at this, there is no reason we cannot have weaker solutions in NixOS as we already have ensure* options for PostgreSQL and all.

The only reason for why I would say this PR would be a bad idea is, if it rots and never get maintained, breaking things at some point.
When this happens, we can remove / deprecate this, in my opinion.

@aanderse
Copy link
Member

aanderse commented Dec 2, 2022

Terraform providers ... might do a better job at thi

Terraform, by design, manages state. NixOS, by design, does not. "might" was the wrong choice of wording.

as we already have ensure* options for PostgreSQL and all.

Which I authored and deeply regret. There are a ton of problems with the ensure* options in the postgres module.

The only reason for why I would say this PR would be a bad idea is, if it rots and never get maintained, breaking things at some point.

I'll continue my point above about the ensure* options in the postgres module: Other modules start using ensure* options and become dependant on them. Module authors may not realize the implications. We had a case some time back where another module ended up breaking user setups because it depended on ensure* options which weren't reproducible. And of course we do the damage of promoting a bad solution that most people don't end up understanding and never realize it is a problem.

I'll summarize my position by quoting myself from above:

This leads to configuration files which don't match the actual configuration of a machine, doesn't it? NixOS machines which are difficult to reason about aren't very good NixOS machines...

That being said... we're both members of the community and respect each other's decisions and positions. I have stated my case and would be happy to discuss further if you think there would be value in that. If not I will at least feel satisfied that I made the argument against when someone hits the merge button. Thanks for hearing me out ❤️

@RaitoBezarius
Copy link
Member

Terraform providers ... might do a better job at thi

Terraform, by design, manages state. NixOS, by design, does not. "might" was the wrong choice of wording.

Well, I disagree because you can clearly manage state using NixOS, e.g. by managing in-tree of your NixOS configuration as simple files.
Of course, this is the poor's man state management and I do agree with the semantics of what you are saying.

Nevertheless, this situation is not necessarily one that will never change, and I do think there is a need for a spectrum of solutions, even if they are less than desirable and introduces a lot of caveats. Tools are always abused by their end-users and (system) designers have to take this possibility in account, therefore, I am not surprised to offer escape hatches.

But, this bring me to my next point.

as we already have ensure* options for PostgreSQL and all.

Which I authored and deeply regret. There are a ton of problems with the ensure* options in the postgres module.

The only reason for why I would say this PR would be a bad idea is, if it rots and never get maintained, breaking things at some point.

I'll continue my point above about the ensure* options in the postgres module: Other modules start using ensure* options and become dependant on them. Module authors may not realize the implications. We had a case some time back where another module ended up breaking user setups because it depended on ensure* options which weren't reproducible. And of course we do the damage of promoting a bad solution that most people don't end up understanding and never realize it is a problem.

I definitely see how ensure* options are very fragile and dangerous in fact.
At the same time, I really appreciate that the community (including you) have gone to great extents to test this, exercise it and ensure (no pun intended) it had the right properties to have a "good enough" level of reliability.

Therefore, as long as we can get the ~same level of reliability for a new option, I would not say "no" to another attempt.

Of course, at some point, this proliferation of ensure* has to stop, and we need "actual" and "proper" ways to bake state or declarative information in these modules.

But there is a long way until upstream software will support this and a lot of people including me cannot buy into Terraform-like things for many other reliability reasons and KISS principles (and I say this as someone who started a NixOps backend and was bummed by the difficulty of maintaining the world's view in sync with my Nix expression view).

I'll summarize my position by quoting myself from above:

This leads to configuration files which don't match the actual configuration of a machine, doesn't it? NixOS machines which are difficult to reason about aren't very good NixOS machines...

That's true and understandable, but in that case, I think we have a dire need to introduce a clear-cut separation between "well behaved NixOS machines" and "clunky NixOS machines that could be well behaved as long as this anti feature X is not broken" and let people adopt their compromise and propagate it through the whole NixOS module.

But I do agree I would like to keep these features to their bare minimum and necessary and on a very specific perimeter to avoid having them growing uncontrollably.
So we should definitely do a more meta thing about this kind of things. :)

That being said... we're both members of the community and respect each other's decisions and positions. I have stated my case and would be happy to discuss further if you think there would be value in that. If not I will at least feel satisfied that I made the argument against when someone hits the merge button. Thanks for hearing me out heart

Thank you for clarifying and having this discussion! :) I hope we did not annoy @pinpox too much, though. Apologies for this meta discussion.

My proposal would be the following: we should discuss more of this in another place, it could be RFC or it could be an issue in nixpkgs architecture team, etc.

I think I will ask for more members of the community to weigh in on that. I do have an usecase for this PR and would really love to get it, but I get your points.

@aanderse
Copy link
Member

aanderse commented Dec 2, 2022

My proposal would be the following: we should discuss more of this in another place, it could be RFC or it could be an issue in nixpkgs architecture team, etc.

I think I will ask for more members of the community to weigh in on that. I do have an usecase for this PR and would really love to get it, but I get your points.

Sounds good. Thanks so much @RaitoBezarius!

@pinpox
Copy link
Member Author

pinpox commented Dec 2, 2022

Question regarding this meta-discussion about state: Where does "configuration" end and "state" start? I'm having trouble pinpointing the difference between a S3 bucket that is declaratively configured to "be there" as part of a setup and something like e.g. a virtual host in nginx, which is considered part of the configuration.

Staying with minio as an example, would you consider a policy to be state or configuration? It could be defined in a json configuration document and generated from nix code. While I haven't seen a way to do this for buckets, where is the difference?

@aanderse
Copy link
Member

aanderse commented Dec 4, 2022

Unfortunately some applications don't differentiate between state and configuration. Some applications use configuration to initially define state. Not always easy to reconcile this with nix philosophy.

I'm having trouble pinpointing the difference between a S3 bucket that is declaratively configured to "be there" as part of a setup and something like e.g. a virtual host in nginx, which is considered part of the configuration.

For the purpose of this conversation let's ask a simple questions for each scenario: Can your S3 bucket or virtual host ever deviate from what is described in nix?

@pinpox
Copy link
Member Author

pinpox commented Dec 6, 2022

For the purpose of this conversation let's ask a simple questions for each scenario: Can your S3 bucket or virtual host ever deviate from what is described in nix?

The virtual host can not. The S3 bucket (currently) can. But that is exactly what I'm trying to change: I would like the buckets to be in an inmutable, declared state. Of course the content should be mutable, but the bucket itself should not.

@aanderse
Copy link
Member

The bucket will remain mutable, though. Since you use imperative commands to create buckets this implies that you can later issue more commands imperatively to undo/alter these buckets and make your configuration deviate from reality.

It boils down to minio storing state every time you issue a command to create a bucket. nginx has no stored state for each virtual host... just configuration.

@whentze
Copy link
Contributor

whentze commented May 31, 2023

The virtual host can not. The S3 bucket (currently) can. But that is exactly what I'm trying to change: I would like the buckets to be in an inmutable, declared state. Of course the content should be mutable, but the bucket itself should not.

Right, so the idea is that some piece of state (the set of buckets that exist) is managed declaratively, while another part (the content of the buckets) is not. That by itself is not a problem, it's how all of NixOS works. What makes this tricky in this case is that the dynamic part of the state depends on the static part of the state.

We need to ask: What should NixOS, when applying a configuration, do if it finds a bucket that's not declared.
You essentially have two options here:

  1. Leave the bucket alone. This is what the existing ensure* options in NixOS do, and it's the "safe" option. But it also means that the set of buckets is not fully managed declaratively. With this option, if you ever remove declaratively managed buckets, you will end up leaking them and having to manually remove them.
  2. Delete the bucket. This would be more thorough and not leak buckets, but it bears the danger of losing data irrecoverably. In any case, it means that you can not fearlessly try out and roll back a NixOS configuration.

FWIW, I don't think that this is a blocker for this PR. As others have noted, ensure-style options are already a thing. Just wanted to clarify why this is a bit spicier than e.g. nginx virtualhosts.

@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants