Skip to content

Commit

Permalink
chore: small RBAC improvements
Browse files Browse the repository at this point in the history
* `talosctl config new` now sets endpoints in the generated config.
* Avoid duplication of roles in metadata.
* Remove method name prefix handling. All methods should be set explicitly.
* Add tests.

Closes siderolabs#3421.

Signed-off-by: Alexey Palazhchenko <alexey.palazhchenko@gmail.com>
(cherry picked from commit ad047a7)
  • Loading branch information
AlekSi authored and smira committed Jun 28, 2021
1 parent 829e54f commit 550dfe5
Show file tree
Hide file tree
Showing 21 changed files with 199 additions and 160 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ PKGS ?= v0.6.0
EXTRAS ?= v0.4.0
GO_VERSION ?= 1.16
GOFUMPT_VERSION ?= v0.1.0
STRINGER_VERSION ?= v0.1.0
STRINGER_VERSION ?= v0.1.3
IMPORTVET ?= autonomy/importvet:f6b07d9
OPERATING_SYSTEM := $(shell uname -s | tr "[:upper:]" "[:lower:]")
TALOSCTL_DEFAULT_TARGET := talosctl-$(OPERATING_SYSTEM)
Expand Down
3 changes: 3 additions & 0 deletions cmd/talosctl/cmd/talos/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,9 @@ var configNewCmd = &cobra.Command{
return err
}

// make the new config immediately useful
config.Contexts[config.Context].Endpoints = c.GetEndpoints()

return config.Save(path)
})
},
Expand Down
4 changes: 3 additions & 1 deletion internal/app/apid/pkg/backend/apid.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ func (a *APID) String() string {
// GetConnection returns a grpc connection to the backend.
func (a *APID) GetConnection(ctx context.Context) (context.Context, *grpc.ClientConn, error) {
md, _ := metadata.FromIncomingContext(ctx)
md = metadata.Join(md, authz.RolesAsMetadata(authz.GetRoles(ctx))) // TODO(rbac): duplicates?
md = md.Copy()

authz.SetMetadata(md, authz.GetRoles(ctx))

if authority := md[":authority"]; len(authority) > 0 {
md.Set("proxyfrom", authority...)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ func (s *Server) HealthCheck(in *clusterapi.HealthCheckRequest, srv clusterapi.C
checkCtx, checkCtxCancel := context.WithTimeout(srv.Context(), in.WaitTimeout.AsDuration())
defer checkCtxCancel()

md := authz.RolesAsMetadata(authz.GetRoles(srv.Context()))
md := metadata.New(nil)
authz.SetMetadata(md, authz.GetRoles(srv.Context()))
checkCtx = metadata.NewOutgoingContext(checkCtx, md)

if err := clusterState.resolve(checkCtx, k8sProvider); err != nil {
Expand Down
4 changes: 3 additions & 1 deletion internal/app/machined/pkg/system/services/machined.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ var rules = map[string]role.Set{
"/network.NetworkService/Routes": role.MakeSet(role.Admin, role.Reader),

// per-type authorization is handled by the service itself
"/resource.ResourceService": role.MakeSet(role.Admin, role.Reader),
"/resource.ResourceService/Get": role.MakeSet(role.Admin, role.Reader),
"/resource.ResourceService/List": role.MakeSet(role.Admin, role.Reader),
"/resource.ResourceService/Watch": role.MakeSet(role.Admin, role.Reader),

"/storage.StorageService/Disks": role.MakeSet(role.Admin, role.Reader),

Expand Down
45 changes: 5 additions & 40 deletions internal/app/machined/pkg/system/services/machined_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/stretchr/testify/require"
"google.golang.org/grpc"

"github.com/talos-systems/talos/pkg/grpc/middleware/authz"
"github.com/talos-systems/talos/pkg/machinery/api/cluster"
"github.com/talos-systems/talos/pkg/machinery/api/inspect"
"github.com/talos-systems/talos/pkg/machinery/api/machine"
Expand Down Expand Up @@ -51,7 +50,7 @@ func collectMethods(t *testing.T) map[string]struct{} {
return methods
}

func TestRules(t *testing.T) { //nolint:gocyclo
func TestRules(t *testing.T) {
t.Parallel()

methods := collectMethods(t)
Expand All @@ -61,25 +60,8 @@ func TestRules(t *testing.T) { //nolint:gocyclo
t.Parallel()

for rule := range rules {
var found bool
for method := range methods {
prefix := method
for prefix != "/" {
if prefix == rule {
found = true

break
}

prefix = authz.NextPrefix(prefix)
}

if found {
break
}
}

assert.True(t, found, "no method for rule %q", rule)
_, ok := methods[rule]
assert.True(t, ok, "no method for rule %q", rule)
}
})

Expand All @@ -88,25 +70,8 @@ func TestRules(t *testing.T) { //nolint:gocyclo
t.Parallel()

for method := range methods {
var found bool
for rule := range rules {
prefix := method
for prefix != "/" {
if prefix == rule {
found = true

break
}

prefix = authz.NextPrefix(prefix)
}

if found {
break
}
}

assert.True(t, found, "no rule for method %q", method)
_, ok := rules[method]
assert.True(t, ok, "no rule for method %q", method)
}
})
}
2 changes: 2 additions & 0 deletions internal/integration/base/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ func (cliSuite *CLISuite) discoverKubectl() cluster.Info {
}
}

// buildCLICmd builds exec.Cmd from TalosSuite and args.
// TalosSuite flags are added at the beginning so they can be overridden by args.
func (cliSuite *CLISuite) buildCLICmd(args []string) *exec.Cmd {
if cliSuite.Endpoint != "" {
args = append([]string{"--endpoints", cliSuite.Endpoint}, args...)
Expand Down
49 changes: 25 additions & 24 deletions internal/integration/base/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package base

import (
"bytes"
"io"
"os"
"os/exec"
"regexp"
Expand All @@ -28,25 +27,19 @@ type runOptions struct {
stdoutEmpty bool
stderrNotEmpty bool
stdoutRegexps []*regexp.Regexp
stderrRegexps []*regexp.Regexp
stdoutNegativeRegexps []*regexp.Regexp
stderrRegexps []*regexp.Regexp
stderrNegativeRegexps []*regexp.Regexp
stdoutMatchers []MatchFunc
stderrMatchers []MatchFunc
}

// ShouldFail tells Run command should fail.
// ShouldFail tells run command should fail (with non-empty stderr).
//
// ShouldFail also sets StdErrNotEmpty.
func ShouldFail() RunOption {
return func(opts *runOptions) {
opts.shouldFail = true
}
}

// StderrNotEmpty tells run that stderr of the command should not be empty.
func StderrNotEmpty() RunOption {
return func(opts *runOptions) {
opts.stderrNotEmpty = true
}
}
Expand All @@ -58,6 +51,13 @@ func StdoutEmpty() RunOption {
}
}

// StderrNotEmpty tells run that stderr of the command should not be empty.
func StderrNotEmpty() RunOption {
return func(opts *runOptions) {
opts.stderrNotEmpty = true
}
}

// StdoutShouldMatch appends to the set of regexps stdout contents should match.
func StdoutShouldMatch(r *regexp.Regexp) RunOption {
return func(opts *runOptions) {
Expand All @@ -72,14 +72,17 @@ func StdoutShouldNotMatch(r *regexp.Regexp) RunOption {
}
}

// StderrShouldMatch appends to the set of regexps sterr contents should match.
// StderrShouldMatch appends to the set of regexps stderr contents should match.
//
// StderrShouldMatch also sets StdErrNotEmpty.
func StderrShouldMatch(r *regexp.Regexp) RunOption {
return func(opts *runOptions) {
opts.stderrRegexps = append(opts.stderrRegexps, r)
opts.stderrNotEmpty = true
}
}

// StderrShouldNotMatch appends to the set of regexps sterr contents should not match.
// StderrShouldNotMatch appends to the set of regexps stderr contents should not match.
func StderrShouldNotMatch(r *regexp.Regexp) RunOption {
return func(opts *runOptions) {
opts.stderrNegativeRegexps = append(opts.stderrNegativeRegexps, r)
Expand Down Expand Up @@ -108,7 +111,7 @@ func runAndWait(suite *suite.Suite, cmd *exec.Cmd) (stdoutBuf, stderrBuf *bytes.

cmd.Stdin = nil
cmd.Stdout = &stdout
cmd.Stderr = io.MultiWriter(os.Stderr, &stderr)
cmd.Stderr = &stderr
cmd.Env = []string{}

// filter environment variables
Expand All @@ -128,6 +131,8 @@ func runAndWait(suite *suite.Suite, cmd *exec.Cmd) (stdoutBuf, stderrBuf *bytes.
}
}

suite.T().Logf("Running %q", strings.Join(cmd.Args, " "))

suite.Require().NoError(cmd.Start())

err = cmd.Wait()
Expand All @@ -146,6 +151,11 @@ func run(suite *suite.Suite, cmd *exec.Cmd, options ...RunOption) (stdout string
}

stdoutBuf, stderrBuf, err := runAndWait(suite, cmd)
if err != nil {
// check that command failed, not something else happened
_, ok := err.(*exec.ExitError)
suite.Require().True(ok, "%s", err)
}

if stdoutBuf != nil {
stdout = stdoutBuf.String()
Expand All @@ -156,19 +166,10 @@ func run(suite *suite.Suite, cmd *exec.Cmd, options ...RunOption) (stdout string
stderr = stderrBuf.String()
}

if err == nil {
if opts.shouldFail {
suite.Assert().NotNil(err, "command should have failed but it exited with zero exit code")
}
if opts.shouldFail {
suite.Assert().Error(err, "command expected to fail, but did not")
} else {
exitErr, ok := err.(*exec.ExitError)
if !ok {
suite.Require().Nil(err, "command failed to be run")
}

if !opts.shouldFail {
suite.Assert().Nil(exitErr, "command failed with exit code: %d", exitErr.ExitCode())
}
suite.Assert().NoError(err, "command failed")
}

if opts.stdoutEmpty {
Expand Down
Loading

0 comments on commit 550dfe5

Please sign in to comment.