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

[vtadmin-api] Allow RBAC config file to be optional #9419

Closed
doeg opened this issue Dec 17, 2021 · 7 comments
Closed

[vtadmin-api] Allow RBAC config file to be optional #9419

doeg opened this issue Dec 17, 2021 · 7 comments
Assignees
Projects

Comments

@doeg
Copy link
Contributor

doeg commented Dec 17, 2021

The --rbac-config flag is required when running vtadmin-api. If omitted:

F1217 12:14:01.251523   31578 main.go:72] open rbac.yaml: no such file or directory

The local example provides a default rbac.yaml file here: https://github.com/vitessio/vitess/blob/main/examples/local/vtadmin/rbac.yaml

However, it might be more ergonomic + explicit if Vitess operators could either:

  • Provide an --rbac flag and an --rbac-config file, or
  • Provide a --no-rbac (or --rbac=false) flag to explicitly opt out of using RBAC.
@doeg doeg created this issue from a note in VTAdmin (Inbox (unplanned/untriaged)) Dec 17, 2021
@setassociative
Copy link
Contributor

I was reading the related backscroll with jakon and had the thought that defaulting allow-all when no rbac config flag is convenient but allows failure in an unsafe way: I can imagine fucking up a chef flag -> not passing an rbac flag -> opening write actions to the world.

Still extremely into easy allow-all option but we should make it opt-in under --no-rbac or similar.

@notfelineit
Copy link
Contributor

A clarifying q, does that mean we're either saying 1) must pass in rbac config or 2) must explicitly pass --no-rbac to default to all?

@jakon89
Copy link

jakon89 commented Dec 17, 2021

@setassociative +1 for --no-rbac
Anyway, unknown flags aren't allowed so its unlikely that scenario is possible:

adam@local ~ % docker run vtadmin:latest --rbac-confi /non-exisiting-file
ERROR: logging before flag.Parse: E1217 18:05:40.620713       1 syslogger.go:149] can't connect to syslog
Error: unknown flag: --rbac-confi
Usage:
  vtadmin [flags]

@setassociative
Copy link
Contributor

setassociative commented Dec 17, 2021

@notfelineit sorry, to be more explicit: I'm suggesting you must pass in --rbac flag + config file or --no-rbac. Basically the caller needs to be explicit about how they want rbac to work in either case

@jakon89 so when constructing args to set up vtadmin there is the error where you typo the arg and it's safe as you point out. I'm more worried about a scenario where it gets omitted entirely for whatever reason and I'd rather fail hard and fast in that situation

@doeg
Copy link
Contributor Author

doeg commented Dec 17, 2021

This sounds great. I updated the issue description to specify an explicit --rbac or --no-rbac flag (instead of "failing open" if an RBAC config file is not provided).

@notfelineit
Copy link
Contributor

This issue is being addressed here: #9972

@notfelineit
Copy link
Contributor

Merged #9972! Closing this issue.

VTAdmin automation moved this from Inbox (unplanned/untriaged) to Done Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

5 participants