From 5128787dfd3e657dabd9fb0a7bf05db346c57d62 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sat, 8 Jun 2024 20:24:20 +0200 Subject: [PATCH] fix: check RBAC to watched resources on startup --- charts/accurate/MIGRATION.md | 1 - charts/accurate/values.yaml | 1 - cmd/accurate-controller/sub/run.go | 6 +++++- docs/config.md | 1 - e2e/values.yaml | 1 - pkg/config/suite_test.go | 10 +++++++++ pkg/config/types.go | 34 ++++++++++++++++++++++++++++++ pkg/config/types_test.go | 25 ++++++++++++++++++++++ 8 files changed, 74 insertions(+), 5 deletions(-) diff --git a/charts/accurate/MIGRATION.md b/charts/accurate/MIGRATION.md index edc761b..739df82 100644 --- a/charts/accurate/MIGRATION.md +++ b/charts/accurate/MIGRATION.md @@ -114,7 +114,6 @@ controller: - list - watch - create - - update - patch - delete diff --git a/charts/accurate/values.yaml b/charts/accurate/values.yaml index 45e57a9..1429a24 100644 --- a/charts/accurate/values.yaml +++ b/charts/accurate/values.yaml @@ -101,7 +101,6 @@ controller: - list - watch - create - - update - patch - delete # controller.additionalRBAC.clusterRoles -- Specify additional ClusterRoles to be granted diff --git a/cmd/accurate-controller/sub/run.go b/cmd/accurate-controller/sub/run.go index 7acccae..1f0d1d3 100644 --- a/cmd/accurate-controller/sub/run.go +++ b/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 diff --git a/docs/config.md b/docs/config.md index 72a9da0..e9413c3 100644 --- a/docs/config.md +++ b/docs/config.md @@ -162,7 +162,6 @@ controller: - list - watch - create - - update - patch - delete diff --git a/e2e/values.yaml b/e2e/values.yaml index fb6ee32..3503c4e 100644 --- a/e2e/values.yaml +++ b/e2e/values.yaml @@ -54,7 +54,6 @@ controller: - list - watch - create - - update - patch - delete clusterRoles: diff --git a/pkg/config/suite_test.go b/pkg/config/suite_test.go index 5a00ba3..307ea54 100644 --- a/pkg/config/suite_test.go +++ b/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()) diff --git a/pkg/config/types.go b/pkg/config/types.go index 781f562..8c06358 100644 --- a/pkg/config/types.go +++ b/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) diff --git a/pkg/config/types_test.go b/pkg/config/types_test.go index acea454..0ef294c 100644 --- a/pkg/config/types_test.go +++ b/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