From b0a186f2761b7e2cd622587e766f35cceb42fff5 Mon Sep 17 00:00:00 2001 From: everettraven Date: Tue, 17 Sep 2024 17:17:59 -0400 Subject: [PATCH 1/3] wip: poc: permissions preflight check first pass Signed-off-by: everettraven --- cmd/manager/main.go | 2 ++ go.mod | 2 ++ go.sum | 4 ++++ internal/applier/helm.go | 8 ++++---- .../preflights/crdupgradesafety/crdupgradesafety.go | 5 +++-- .../preflights/crdupgradesafety/crdupgradesafety_test.go | 4 ++-- 6 files changed, 17 insertions(+), 8 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index db25c3ad0..dcf42cef7 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -57,6 +57,7 @@ import ( "github.com/operator-framework/operator-controller/internal/labels" "github.com/operator-framework/operator-controller/internal/resolve" "github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety" + "github.com/operator-framework/operator-controller/internal/rukpak/preflights/permissions" "github.com/operator-framework/operator-controller/internal/rukpak/source" "github.com/operator-framework/operator-controller/internal/scheme" "github.com/operator-framework/operator-controller/internal/version" @@ -247,6 +248,7 @@ func main() { preflights := []applier.Preflight{ crdupgradesafety.NewPreflight(aeClient.CustomResourceDefinitions()), + permissions.NewPreflight(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper()), } applier := &applier.Helm{ diff --git a/go.mod b/go.mod index 259b4f4d1..fc66015ce 100644 --- a/go.mod +++ b/go.mod @@ -245,9 +245,11 @@ require ( gopkg.in/warnings.v0 v0.1.2 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/apiserver v0.31.1 // indirect + k8s.io/component-helpers v0.31.0 // indirect k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect k8s.io/kubectl v0.31.0 // indirect + k8s.io/kubernetes v1.30.2 // indirect oras.land/oras-go v1.2.5 // indirect sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect diff --git a/go.sum b/go.sum index 21504d954..10c04d23d 100644 --- a/go.sum +++ b/go.sum @@ -1005,12 +1005,16 @@ k8s.io/client-go v0.31.1 h1:f0ugtWSbWpxHR7sjVpQwuvw9a3ZKLXX0u0itkFXufb0= k8s.io/client-go v0.31.1/go.mod h1:sKI8871MJN2OyeqRlmA4W4KM9KBdBUpDLu/43eGemCg= k8s.io/component-base v0.31.1 h1:UpOepcrX3rQ3ab5NB6g5iP0tvsgJWzxTyAo20sgYSy8= k8s.io/component-base v0.31.1/go.mod h1:WGeaw7t/kTsqpVTaCoVEtillbqAhF2/JgvO0LDOMa0w= +k8s.io/component-helpers v0.31.0 h1:jyRUKA+GX+q19o81k4x94imjNICn+e6Gzi6T89va1/A= +k8s.io/component-helpers v0.31.0/go.mod h1:MrNIvT4iB7wXIseYSWfHUJB/aNUiFvbilp4qDfBQi6s= k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk= k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7FjZpUb45WallggurYhKGag= k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98= k8s.io/kubectl v0.31.0 h1:kANwAAPVY02r4U4jARP/C+Q1sssCcN/1p9Nk+7BQKVg= k8s.io/kubectl v0.31.0/go.mod h1:pB47hhFypGsaHAPjlwrNbvhXgmuAr01ZBvAIIUaI8d4= +k8s.io/kubernetes v1.30.2 h1:11WhS78OYX/lnSy6TXxPO6Hk+E5K9ZNrEsk9JgMSX8I= +k8s.io/kubernetes v1.30.2/go.mod h1:yPbIk3MhmhGigX62FLJm+CphNtjxqCvAIFQXup6RKS0= k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 h1:pUdcCO1Lk/tbT5ztQWOBi5HBgbBP1J8+AsQnQCKsi8A= k8s.io/utils v0.0.0-20240711033017-18e509b52bc8/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= oras.land/oras-go v1.2.5 h1:XpYuAwAb0DfQsunIyMfeET92emK8km3W4yEzZvUbsTo= diff --git a/internal/applier/helm.go b/internal/applier/helm.go index ac43726f1..fc76db700 100644 --- a/internal/applier/helm.go +++ b/internal/applier/helm.go @@ -42,13 +42,13 @@ type Preflight interface { // to installing the Helm release. It is provided // a Helm release and returns an error if the // check is unsuccessful - Install(context.Context, *release.Release) error + Install(context.Context, *release.Release, *ocv1alpha1.ClusterExtension) error // Upgrade runs checks that should be successful prior // to upgrading the Helm release. It is provided // a Helm release and returns an error if the // check is unsuccessful - Upgrade(context.Context, *release.Release) error + Upgrade(context.Context, *release.Release, *ocv1alpha1.ClusterExtension) error } type Helm struct { @@ -87,12 +87,12 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1alpha1.Clust } switch state { case StateNeedsInstall: - err := preflight.Install(ctx, desiredRel) + err := preflight.Install(ctx, desiredRel, ext) if err != nil { return nil, state, err } case StateNeedsUpgrade: - err := preflight.Upgrade(ctx, desiredRel) + err := preflight.Upgrade(ctx, desiredRel, ext) if err != nil { return nil, state, err } diff --git a/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety.go b/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety.go index 3f91c8c2b..d6adf946d 100644 --- a/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety.go +++ b/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety.go @@ -14,6 +14,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/rukpak/util" ) @@ -65,11 +66,11 @@ func NewPreflight(crdCli apiextensionsv1client.CustomResourceDefinitionInterface return p } -func (p *Preflight) Install(ctx context.Context, rel *release.Release) error { +func (p *Preflight) Install(ctx context.Context, rel *release.Release, _ *ocv1alpha1.ClusterExtension) error { return p.runPreflight(ctx, rel) } -func (p *Preflight) Upgrade(ctx context.Context, rel *release.Release) error { +func (p *Preflight) Upgrade(ctx context.Context, rel *release.Release, _ *ocv1alpha1.ClusterExtension) error { return p.runPreflight(ctx, rel) } diff --git a/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go b/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go index 39e0a0fe9..74cc3a0b7 100644 --- a/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go +++ b/internal/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go @@ -197,7 +197,7 @@ func TestInstall(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { preflight := newMockPreflight(getCrdFromManifestFile(t, tc.oldCrdPath), tc.wantCrdGetErr, tc.validator) - err := preflight.Install(context.Background(), tc.release) + err := preflight.Install(context.Background(), tc.release, nil) if len(tc.wantErrMsgs) != 0 { for _, expectedErrMsg := range tc.wantErrMsgs { require.ErrorContainsf(t, err, expectedErrMsg, "") @@ -353,7 +353,7 @@ func TestUpgrade(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { preflight := newMockPreflight(getCrdFromManifestFile(t, tc.oldCrdPath), tc.wantCrdGetErr, tc.validator) - err := preflight.Upgrade(context.Background(), tc.release) + err := preflight.Upgrade(context.Background(), tc.release, nil) if len(tc.wantErrMsgs) != 0 { for _, expectedErrMsg := range tc.wantErrMsgs { require.ErrorContainsf(t, err, expectedErrMsg, "") From 54ec992f3dd96ca832a4b9f66e00822a223efff5 Mon Sep 17 00:00:00 2001 From: everettraven Date: Tue, 17 Sep 2024 17:18:05 -0400 Subject: [PATCH 2/3] wip: poc: permissions preflight check first pass Signed-off-by: everettraven --- .../preflights/permissions/permissions.go | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 internal/rukpak/preflights/permissions/permissions.go diff --git a/internal/rukpak/preflights/permissions/permissions.go b/internal/rukpak/preflights/permissions/permissions.go new file mode 100644 index 000000000..bf599c008 --- /dev/null +++ b/internal/rukpak/preflights/permissions/permissions.go @@ -0,0 +1,106 @@ +package permissions + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strings" + + kappperms "carvel.dev/kapp/pkg/kapp/permissions" + kappres "carvel.dev/kapp/pkg/kapp/resources" + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/rukpak/util" + "helm.sh/helm/v3/pkg/release" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type RestConfigMapper func(context.Context, client.Object, *rest.Config) (*rest.Config, error) + +type Preflight struct { + rcm RestConfigMapper + baseCfg *rest.Config + mapper meta.RESTMapper +} + +func NewPreflight(rcm RestConfigMapper, baseCfg *rest.Config, mapper meta.RESTMapper) *Preflight { + return &Preflight{ + rcm: rcm, + baseCfg: baseCfg, + mapper: mapper, + } +} + +func (p *Preflight) Install(ctx context.Context, rel *release.Release, ext *ocv1alpha1.ClusterExtension) error { + return p.runPreflight(ctx, rel, ext) +} + +func (p *Preflight) Upgrade(ctx context.Context, rel *release.Release, ext *ocv1alpha1.ClusterExtension) error { + return p.runPreflight(ctx, rel, ext) +} + +func (p *Preflight) runPreflight(ctx context.Context, rel *release.Release, ext *ocv1alpha1.ClusterExtension) error { + if rel == nil { + return nil + } + + cfg, err := p.rcm(ctx, ext, p.baseCfg) + if err != nil { + return fmt.Errorf("getting config: %w", err) + } + + kClient, err := kubernetes.NewForConfig(cfg) + if err != nil { + return fmt.Errorf("getting kubernetes client: %w", err) + } + + permissionValidator := kappperms.NewSelfSubjectRulesReviewValidator(kClient.AuthorizationV1().SelfSubjectRulesReviews()) + roleValidator := kappperms.NewRoleValidator(permissionValidator, p.mapper) + bindingValidator := kappperms.NewBindingValidator(permissionValidator, kClient.RbacV1(), p.mapper) + basicValidator := kappperms.NewBasicValidator(permissionValidator, p.mapper) + + validator := kappperms.NewCompositeValidator(basicValidator, map[schema.GroupVersionKind]kappperms.Validator{ + rbacv1.SchemeGroupVersion.WithKind("Role"): roleValidator, + rbacv1.SchemeGroupVersion.WithKind("ClusterRole"): roleValidator, + rbacv1.SchemeGroupVersion.WithKind("RoleBinding"): bindingValidator, + rbacv1.SchemeGroupVersion.WithKind("ClusterRoleBinding"): bindingValidator, + }) + + relObjects, err := util.ManifestObjects(strings.NewReader(rel.Manifest), fmt.Sprintf("%s-release-manifest", rel.Name)) + if err != nil { + return fmt.Errorf("parsing release %q objects: %w", rel.Name, err) + } + + // TODO: Update this to be more efficient + + resources := []kappres.Resource{} + for _, obj := range relObjects { + bytes, err := json.Marshal(obj) + if err != nil { + return fmt.Errorf("marshalling object %v: %w", obj, err) + } + resource, err := kappres.NewResourceFromBytes(bytes) + if err != nil { + return fmt.Errorf("converting bytes to resource: %w", err) + } + resources = append(resources, resource) + } + + verbsToCheck := []string{"create", "update", "patch", "delete", "get", "list", "watch"} + errs := []error{} + for _, resource := range resources { + for _, verb := range verbsToCheck { + err := validator.Validate(ctx, resource, verb) + if err != nil { + errs = append(errs, err) + } + } + } + + return errors.Join(errs...) +} From d9c105c7c60e6f5f24ef4cbcd5d346b4e47dbdad Mon Sep 17 00:00:00 2001 From: everettraven Date: Wed, 18 Sep 2024 11:57:15 -0400 Subject: [PATCH 3/3] wip: poc: permissions preflight check first pass Signed-off-by: everettraven --- cmd/manager/main.go | 2 +- .../preflights/permissions/permissions.go | 31 +++++++++---------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index dcf42cef7..55bd5311d 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -248,7 +248,7 @@ func main() { preflights := []applier.Preflight{ crdupgradesafety.NewPreflight(aeClient.CustomResourceDefinitions()), - permissions.NewPreflight(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper()), + permissions.NewPreflight(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper()), } applier := &applier.Helm{ diff --git a/internal/rukpak/preflights/permissions/permissions.go b/internal/rukpak/preflights/permissions/permissions.go index bf599c008..5966f8832 100644 --- a/internal/rukpak/preflights/permissions/permissions.go +++ b/internal/rukpak/preflights/permissions/permissions.go @@ -9,8 +9,6 @@ import ( kappperms "carvel.dev/kapp/pkg/kapp/permissions" kappres "carvel.dev/kapp/pkg/kapp/resources" - ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - "github.com/operator-framework/operator-controller/internal/rukpak/util" "helm.sh/helm/v3/pkg/release" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -18,6 +16,9 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" + + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/rukpak/util" ) type RestConfigMapper func(context.Context, client.Object, *rest.Config) (*rest.Config, error) @@ -76,9 +77,8 @@ func (p *Preflight) runPreflight(ctx context.Context, rel *release.Release, ext return fmt.Errorf("parsing release %q objects: %w", rel.Name, err) } - // TODO: Update this to be more efficient - - resources := []kappres.Resource{} + verbsToCheck := []string{"create", "update", "patch", "delete", "get", "list", "watch"} + errs := []error{} for _, obj := range relObjects { bytes, err := json.Marshal(obj) if err != nil { @@ -88,19 +88,18 @@ func (p *Preflight) runPreflight(ctx context.Context, rel *release.Release, ext if err != nil { return fmt.Errorf("converting bytes to resource: %w", err) } - resources = append(resources, resource) + + for _, verb := range verbsToCheck { + err := validator.Validate(ctx, resource, verb) + if err != nil { + errs = append(errs, err) + } + } } - verbsToCheck := []string{"create", "update", "patch", "delete", "get", "list", "watch"} - errs := []error{} - for _, resource := range resources { - for _, verb := range verbsToCheck { - err := validator.Validate(ctx, resource, verb) - if err != nil { - errs = append(errs, err) - } - } - } + if len(errs) > 0 { + errs = append([]error{errors.New("validating permissions to install and manage resources")}, errs...) + } return errors.Join(errs...) }