Skip to content

Commit

Permalink
fix: check RBAC to watched resources on startup
Browse files Browse the repository at this point in the history
  • Loading branch information
erikgb committed Jun 10, 2024
1 parent dcb4795 commit 5128787
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 5 deletions.
1 change: 0 additions & 1 deletion charts/accurate/MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ controller:
- list
- watch
- create
- update
- patch
- delete
<snip>
Expand Down
1 change: 0 additions & 1 deletion charts/accurate/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ controller:
- list
- watch
- create
- update
- patch
- delete
# controller.additionalRBAC.clusterRoles -- Specify additional ClusterRoles to be granted
Expand Down
6 changes: 5 additions & 1 deletion cmd/accurate-controller/sub/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -104,7 +109,6 @@ func subMain(ns, addr string, port int) error {
})
}

ctx := ctrl.SetupSignalHandler()
dec := admission.NewDecoder(scheme)

// Namespace reconciler & webhook
Expand Down
1 change: 0 additions & 1 deletion docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ controller:
- list
- watch
- create
- update
- patch
- delete
<snip>
Expand Down
1 change: 0 additions & 1 deletion e2e/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ controller:
- list
- watch
- create
- update
- patch
- delete
clusterRoles:
Expand Down
10 changes: 10 additions & 0 deletions pkg/config/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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" {
Expand All @@ -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())
Expand Down
34 changes: 34 additions & 0 deletions pkg/config/types.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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)
Expand Down
25 changes: 25 additions & 0 deletions pkg/config/types_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package config

import (
"context"
_ "embed"
"testing"

Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 5128787

Please sign in to comment.