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

Allow --no-rbac flag that allows users to not pass rbac config #9972

Merged
merged 5 commits into from
Mar 25, 2022

Conversation

notfelineit
Copy link
Contributor

Signed-off-by: notfelineit notfelineit@gmail.com

Description

Breaking Change
This PR adds a flag --no-rbac that allows users to bypass passing the flag --rbac-config. In doing so, a default rbac configuration is used:

rules:
  - resource: "*"
    actions:
    - "get"
    - "create"
    - "delete"
    - "put"
    - "ping"
    subjects: ["*"]
    clusters: ["*"]

It also makes it so that a user must explicitly pass in either --no-rbac or --rbac, otherwise vtadmin will fail to come up:
Screen Shot 2022-03-23 at 4 59 44 PM

This is a breaking change since users who were using --rbac-config must now explicitly pass in --rbac flag as well.

Cases

--no-rbac:
Screen Shot 2022-03-23 at 5 01 08 PM

--rbac:
Screen Shot 2022-03-23 at 4 59 24 PM

Failing to pass in either flag:
Screen Shot 2022-03-23 at 4 59 44 PM

Related Issue(s)

This addresses this issue: #9419

Checklist

  • Should this PR be backported? No
  • Tests were added or are not required
  • Documentation was added or is not required Documentation should be added to release notes

Deployment Notes

N/A

Signed-off-by: notfelineit <notfelineit@gmail.com>
Copy link
Contributor

@ajm188 ajm188 left a comment

Choose a reason for hiding this comment

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

all i've got are style commentary!

go/cmd/vtadmin/main.go Outdated Show resolved Hide resolved
go/vt/vtadmin/rbac/config.go Outdated Show resolved Hide resolved
go/vt/vtadmin/rbac/config.go Show resolved Hide resolved
go/cmd/vtadmin/main.go Outdated Show resolved Hide resolved
Signed-off-by: notfelineit <notfelineit@gmail.com>
Copy link
Contributor

@ajm188 ajm188 left a comment

Choose a reason for hiding this comment

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

one more round 😅

go/cmd/vtadmin/main.go Outdated Show resolved Hide resolved
go/cmd/vtadmin/main.go Outdated Show resolved Hide resolved
Co-authored-by: Andrew Mason <andrew@planetscale.com>
go/cmd/vtadmin/main.go Outdated Show resolved Hide resolved
go/cmd/vtadmin/main.go Outdated Show resolved Hide resolved
Signed-off-by: notfelineit <notfelineit@gmail.com>
Signed-off-by: notfelineit <notfelineit@gmail.com>
@ajm188 ajm188 merged commit c82d09a into main Mar 25, 2022
@ajm188 ajm188 deleted the frances-no-rbac branch March 25, 2022 18:59
ajm188 pushed a commit to planetscale/vitess that referenced this pull request Apr 5, 2022
This was introduced and made a required flag (with mutually-exclusive `--no-rbac`)
in vitessio#9972.

Signed-off-by: Andrew Mason <andrew@planetscale.com>
ajm188 pushed a commit that referenced this pull request Apr 5, 2022
* Add `--rbac` to example vtadmin-up

This was introduced and made a required flag (with mutually-exclusive `--no-rbac`)
in #9972.

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* Cleanup `Options.Services`, and inspect the ServiceInfo() directly

Instead of requiring users to know in advance (and put into their startup
scripts), which services will run, we can (correctly) inspect the service
names that have been registered on the underlying grpc.Server and mark
them as SERVING in the health service.

_Alsooooo_, this was never bound to a flag, so users couldn't use it
anyway! Whoops.

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* Provide option to enable channelz service

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* Add flag to enable channelz

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* Tidy up flags; label all sections

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* Add missing flag for AllowTracing

Signed-off-by: Andrew Mason <andrew@planetscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants