Merge pull request #134 from erikgb/validate-rbac

fix: check RBAC to watched resources on startup

charts/accurate/MIGRATION.md

@@ -114,7 +114,6 @@ controller:
           - list
           - watch
           - create
-          - update
           - patch
           - delete
 <snip>

charts/accurate/values.yaml

@@ -101,7 +101,6 @@ controller:
           - list
           - watch
           - create
-          - update
           - patch
           - delete
     # controller.additionalRBAC.clusterRoles -- Specify additional ClusterRoles to be granted

cmd/accurate-controller/sub/run.go

@@ -89,9 +89,14 @@ func subMain(ns, addr string, port int) error {
 		return fmt.Errorf("unable to start manager: %w", err)
 	}
 
+	ctx := ctrl.SetupSignalHandler()
+
 	if err := cfg.Validate(mgr.GetRESTMapper()); err != nil {
 		return fmt.Errorf("invalid configurations: %w", err)
 	}
+	if err := cfg.ValidateRBAC(ctx, mgr.GetClient(), mgr.GetRESTMapper()); err != nil {
+		return fmt.Errorf("when validating RBAC to support configuration: %w", err)
+	}
 
 	watched := make([]*unstructured.Unstructured, len(cfg.Watches))
 	for i := range cfg.Watches {
@@ -104,7 +109,6 @@ func subMain(ns, addr string, port int) error {
 		})
 	}
 
-	ctx := ctrl.SetupSignalHandler()
 	dec := admission.NewDecoder(scheme)
 
 	// Namespace reconciler & webhook

docs/config.md

@@ -162,7 +162,6 @@ controller:
           - list
           - watch
           - create
-          - update
           - patch
           - delete
 <snip>

e2e/values.yaml

@@ -54,7 +54,6 @@ controller:
           - list
           - watch
           - create
-          - update
           - patch
           - delete
     clusterRoles:

pkg/config/suite_test.go

@@ -11,6 +11,7 @@ import (
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/discovery/cached/memory"
 	"k8s.io/client-go/restmapper"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
@@ -22,6 +23,8 @@ import (
 
 var testEnv *envtest.Environment
 var mapper meta.RESTMapper
+var fullAccessClient client.Client
+var noAccessClient client.Client
 
 func TestAPIs(t *testing.T) {
 	if os.Getenv("TEST_CONFIG") != "1" {
@@ -41,6 +44,13 @@ var _ = BeforeSuite(func() {
 	cfg, err := testEnv.Start()
 	Expect(err).NotTo(HaveOccurred())
 	Expect(cfg).NotTo(BeNil())
+	fullAccessClient, err = client.New(cfg, client.Options{})
+	Expect(err).NotTo(HaveOccurred())
+
+	noAccessUser, err := testEnv.AddUser(envtest.User{Name: "no-access-user"}, nil)
+	Expect(err).NotTo(HaveOccurred())
+	noAccessClient, err = client.New(noAccessUser.Config(), client.Options{})
+	Expect(err).NotTo(HaveOccurred())
 
 	dc, err := discovery.NewDiscoveryClientForConfig(cfg)
 	Expect(err).NotTo(HaveOccurred())

pkg/config/types.go

@@ -1,14 +1,18 @@
 package config
 
 import (
+	"context"
 	"fmt"
 	"path"
 	"regexp"
 	"strings"
 
+	authv1 "k8s.io/api/authorization/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/errors"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/yaml"
 
 	"github.com/cybozu-go/accurate/pkg/constants"
@@ -99,6 +103,36 @@ func (c *Config) Validate(mapper meta.RESTMapper) error {
 	return nil
 }
 
+// ValidateRBAC validates that the manager has RBAC permissions to support configuration
+func (c *Config) ValidateRBAC(ctx context.Context, client client.Client, mapper meta.RESTMapper) error {
+	var errList []error
+
+	for _, gvk := range c.Watches {
+		mapping, err := mapper.RESTMapping(schema.GroupKind{Group: gvk.Group, Kind: gvk.Kind}, gvk.Version)
+		if err != nil {
+			return fmt.Errorf("error mapping GVK %s: %w", gvk.String(), err)
+		}
+
+		selfCheck := &authv1.SelfSubjectAccessReview{}
+		selfCheck.Spec.ResourceAttributes = &authv1.ResourceAttributes{
+			Group:    mapping.Resource.Group,
+			Version:  mapping.Resource.Version,
+			Resource: mapping.Resource.Resource,
+		}
+		for _, verb := range []string{"get", "list", "watch", "create", "patch", "delete"} {
+			selfCheck.Spec.ResourceAttributes.Verb = verb
+			if err := client.Create(ctx, selfCheck); err != nil {
+				return fmt.Errorf("error creating SelfSubjectAccessReview: %w", err)
+			}
+			if !selfCheck.Status.Allowed {
+				errList = append(errList, fmt.Errorf("missing permission to %s %s", verb, mapping.Resource.String()))
+			}
+		}
+	}
+
+	return errors.NewAggregate(errList)
+}
+
 // Load loads configurations.
 func (c *Config) Load(data []byte) error {
 	return yaml.Unmarshal(data, c, yaml.DisallowUnknownFields)

pkg/config/types_test.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"context"
 	_ "embed"
 	"testing"
 
@@ -88,6 +89,30 @@ var _ = Describe("Validate", func() {
 	})
 })
 
+var _ = Describe("ValidateRBAC", func() {
+	var c *Config
+	var ctx context.Context
+
+	BeforeEach(func() {
+		c = &Config{
+			Watches: []metav1.GroupVersionKind{{
+				Group:   "rbac.authorization.k8s.io",
+				Version: "v1",
+				Kind:    "Role",
+			}},
+		}
+		ctx = context.Background()
+	})
+
+	It("should succeed when RBAC present to watched resources", func() {
+		Expect(c.ValidateRBAC(ctx, fullAccessClient, mapper)).To(Succeed())
+	})
+
+	It("should error when missing RBAC to watched resources", func() {
+		Expect(c.ValidateRBAC(ctx, noAccessClient, mapper)).To(MatchError(ContainSubstring("missing permission to patch rbac.authorization.k8s.io/v1, Resource=roles")))
+	})
+})
+
 //go:embed testdata/config.yaml
 var validData []byte