chore: small RBAC improvements

* `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 <[email protected]>

Makefile

@@ -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)

cmd/talosctl/cmd/talos/config.go

@@ -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)
 		})
 	},

internal/app/apid/pkg/backend/apid.go

@@ -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...)

internal/app/machined/internal/server/v1alpha1/v1alpha1_cluster.go

@@ -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 {

internal/app/machined/pkg/system/services/machined.go

@@ -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),
 

internal/app/machined/pkg/system/services/machined_test.go

@@ -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"
@@ -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)
@@ -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)
 		}
 	})
 
@@ -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)
 		}
 	})
 }

internal/integration/base/cli.go

@@ -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...)

internal/integration/base/run.go

@@ -8,7 +8,6 @@ package base
 
 import (
 	"bytes"
-	"io"
 	"os"
 	"os/exec"
 	"regexp"
@@ -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
 	}
 }
@@ -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) {
@@ -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)
@@ -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
@@ -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()
@@ -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()
@@ -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 {