From b37dc85223ff25adec2db0c42a512e02c8726585 Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Wed, 7 Oct 2020 14:30:23 -0700 Subject: [PATCH 01/14] refactor deployPodSecurityPolicyFromYaml to use k8s style retry Signed-off-by: Brian Downs --- pkg/rke2/psp.go | 63 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 19 deletions(-) diff --git a/pkg/rke2/psp.go b/pkg/rke2/psp.go index bd0b6f7403..bc19e44727 100644 --- a/pkg/rke2/psp.go +++ b/pkg/rke2/psp.go @@ -389,30 +389,50 @@ func decodeYamlResource(data interface{}, yaml string) error { return decoder.Decode(data) } -func retryTo(ctx context.Context, runFunc deployFn, cs *kubernetes.Clientset, resource interface{}, retries, wait int) error { - var err error - if retries <= 0 { - retries = defaultRetries - } - if wait <= 0 { - wait = defaultWaitSeconds - } - for i := 0; i < retries; i++ { - if err = runFunc(ctx, cs, resource); err != nil { - time.Sleep(time.Second * time.Duration(wait)) - continue - } - return nil - } - return err -} +// func retryTo(ctx context.Context, runFunc deployFn, cs *kubernetes.Clientset, resource interface{}, retries, wait int) error { +// var err error +// if retries <= 0 { +// retries = defaultRetries +// } +// if wait <= 0 { +// wait = defaultWaitSeconds +// } +// for i := 0; i < retries; i++ { +// if err = runFunc(ctx, cs, resource); err != nil { +// time.Sleep(time.Second * time.Duration(wait)) +// continue +// } +// return nil +// } +// return err +// } func deployPodSecurityPolicyFromYaml(ctx context.Context, cs *kubernetes.Clientset, pspYaml string) error { var psp v1beta1.PodSecurityPolicy if err := decodeYamlResource(&psp, pspYaml); err != nil { return err } - return retryTo(ctx, deployPodSecurityPolicy, cs, psp, defaultRetries, defaultWaitSeconds) + + // try to create the given PSP (continue documenting the flow...) + if err := retry.OnError(retry.DefaultBackoff, + func(err error) bool { + return !apierrors.IsAlreadyExists(err) + }, func() error { + if _, err := cs.PolicyV1beta1().PodSecurityPolicies().Create(ctx, &psp, metav1.CreateOptions{}); err != nil { + return err + } + return nil + }, + ); err != nil && apierrors.IsAlreadyExists(err) { + return retry.RetryOnConflict(retry.DefaultBackoff, func() error { + _, err := cs.PolicyV1beta1().PodSecurityPolicies().Update(ctx, &psp, metav1.UpdateOptions{}) + return err + }) + } else { + return err + } + + return nil } func deployPodSecurityPolicy(ctx context.Context, cs *kubernetes.Clientset, p interface{}) error { @@ -424,7 +444,12 @@ func deployPodSecurityPolicy(ctx context.Context, cs *kubernetes.Clientset, p in if !apierrors.IsAlreadyExists(err) { return err } - if _, err := cs.PolicyV1beta1().PodSecurityPolicies().Update(ctx, &psp, metav1.UpdateOptions{}); err != nil { + if err := retry.OnError(retry.DefaultBackoff, func(error) bool { return true }, func() error { + if _, err := cs.PolicyV1beta1().PodSecurityPolicies().Update(ctx, &psp, metav1.UpdateOptions{}); err != nil { + return err + } + return nil + }); err != nil { return err } } From f008b942fe528b326c468c4dbcedfe947deb6dc7 Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Wed, 7 Oct 2020 16:08:51 -0700 Subject: [PATCH 02/14] refactor to use k8s provided retry logic. start tests Signed-off-by: Brian Downs --- pkg/rke2/psp.go | 137 +++++++++++++++++++----------------------------- 1 file changed, 53 insertions(+), 84 deletions(-) diff --git a/pkg/rke2/psp.go b/pkg/rke2/psp.go index bc19e44727..8b6c1c9a2c 100644 --- a/pkg/rke2/psp.go +++ b/pkg/rke2/psp.go @@ -389,31 +389,14 @@ func decodeYamlResource(data interface{}, yaml string) error { return decoder.Decode(data) } -// func retryTo(ctx context.Context, runFunc deployFn, cs *kubernetes.Clientset, resource interface{}, retries, wait int) error { -// var err error -// if retries <= 0 { -// retries = defaultRetries -// } -// if wait <= 0 { -// wait = defaultWaitSeconds -// } -// for i := 0; i < retries; i++ { -// if err = runFunc(ctx, cs, resource); err != nil { -// time.Sleep(time.Second * time.Duration(wait)) -// continue -// } -// return nil -// } -// return err -// } - func deployPodSecurityPolicyFromYaml(ctx context.Context, cs *kubernetes.Clientset, pspYaml string) error { var psp v1beta1.PodSecurityPolicy if err := decodeYamlResource(&psp, pspYaml); err != nil { return err } - // try to create the given PSP (continue documenting the flow...) + // try to create the given PSP. If it already exists, we fall through to + // attempting to update the existing PSP. if err := retry.OnError(retry.DefaultBackoff, func(err error) bool { return !apierrors.IsAlreadyExists(err) @@ -431,29 +414,6 @@ func deployPodSecurityPolicyFromYaml(ctx context.Context, cs *kubernetes.Clients } else { return err } - - return nil -} - -func deployPodSecurityPolicy(ctx context.Context, cs *kubernetes.Clientset, p interface{}) error { - psp, ok := p.(v1beta1.PodSecurityPolicy) - if !ok { - return fmt.Errorf("invalid type provided: %T, expected: PodSecurityPolicy", p) - } - if _, err := cs.PolicyV1beta1().PodSecurityPolicies().Create(ctx, &psp, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return err - } - if err := retry.OnError(retry.DefaultBackoff, func(error) bool { return true }, func() error { - if _, err := cs.PolicyV1beta1().PodSecurityPolicies().Update(ctx, &psp, metav1.UpdateOptions{}); err != nil { - return err - } - return nil - }); err != nil { - return err - } - } - return nil } func deployClusterRoleBindingFromYaml(ctx context.Context, cs *kubernetes.Clientset, clusterRoleBindingYaml string) error { @@ -461,23 +421,26 @@ func deployClusterRoleBindingFromYaml(ctx context.Context, cs *kubernetes.Client if err := decodeYamlResource(&clusterRoleBinding, clusterRoleBindingYaml); err != nil { return err } - return retryTo(ctx, deployClusterRoleBinding, cs, clusterRoleBinding, defaultRetries, defaultWaitSeconds) -} -func deployClusterRoleBinding(ctx context.Context, cs *kubernetes.Clientset, crb interface{}) error { - clusterRoleBinding, ok := crb.(rbacv1.ClusterRoleBinding) - if !ok { - return fmt.Errorf("invalid type provided: %T, expected: ClusterRoleBinding", crb) - } - if _, err := cs.RbacV1().ClusterRoleBindings().Create(ctx, &clusterRoleBinding, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return err - } - if _, err := cs.RbacV1().ClusterRoleBindings().Update(ctx, &clusterRoleBinding, metav1.UpdateOptions{}); err != nil { + // try to create the given cluster role binding. If it already exists, we + // fall through to attempting to update the existing cluster role binding. + if err := retry.OnError(retry.DefaultBackoff, + func(err error) bool { + return !apierrors.IsAlreadyExists(err) + }, func() error { + if _, err := cs.RbacV1().ClusterRoleBindings().Create(ctx, &clusterRoleBinding, metav1.CreateOptions{}); err != nil { + return err + } + return nil + }, + ); err != nil && apierrors.IsAlreadyExists(err) { + return retry.RetryOnConflict(retry.DefaultBackoff, func() error { + _, err := cs.RbacV1().ClusterRoleBindings().Update(ctx, &clusterRoleBinding, metav1.UpdateOptions{}) return err - } + }) + } else { + return err } - return nil } func deployClusterRoleFromYaml(ctx context.Context, cs *kubernetes.Clientset, clusterRoleYaml string) error { @@ -485,23 +448,26 @@ func deployClusterRoleFromYaml(ctx context.Context, cs *kubernetes.Clientset, cl if err := decodeYamlResource(&clusterRole, clusterRoleYaml); err != nil { return err } - return retryTo(ctx, deployClusterRole, cs, clusterRole, defaultRetries, defaultWaitSeconds) -} -func deployClusterRole(ctx context.Context, cs *kubernetes.Clientset, cr interface{}) error { - clusterRole, ok := cr.(rbacv1.ClusterRole) - if !ok { - return fmt.Errorf("invalid type provided: %T, expected: ClusterRole", cr) - } - if _, err := cs.RbacV1().ClusterRoles().Create(ctx, &clusterRole, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return err - } - if _, err := cs.RbacV1().ClusterRoles().Update(ctx, &clusterRole, metav1.UpdateOptions{}); err != nil { + // try to create the given cluster role. If it already exists, we + // fall through to attempting to update the existing cluster role. + if err := retry.OnError(retry.DefaultBackoff, + func(err error) bool { + return !apierrors.IsAlreadyExists(err) + }, func() error { + if _, err := cs.RbacV1().ClusterRoles().Create(ctx, &clusterRole, metav1.CreateOptions{}); err != nil { + return err + } + return nil + }, + ); err != nil && apierrors.IsAlreadyExists(err) { + return retry.RetryOnConflict(retry.DefaultBackoff, func() error { + _, err := cs.RbacV1().ClusterRoles().Update(ctx, &clusterRole, metav1.UpdateOptions{}) return err - } + }) + } else { + return err } - return nil } func deployRoleBindingFromYaml(ctx context.Context, cs *kubernetes.Clientset, roleBindingYaml string) error { @@ -509,21 +475,24 @@ func deployRoleBindingFromYaml(ctx context.Context, cs *kubernetes.Clientset, ro if err := decodeYamlResource(&roleBinding, roleBindingYaml); err != nil { return err } - return retryTo(ctx, deployRoleBinding, cs, roleBinding, defaultRetries, defaultWaitSeconds) -} -func deployRoleBinding(ctx context.Context, cs *kubernetes.Clientset, rb interface{}) error { - roleBinding, ok := rb.(rbacv1.RoleBinding) - if !ok { - return fmt.Errorf("invalid type provided: %T, expected: RoleBinding", rb) - } - if _, err := cs.RbacV1().RoleBindings(roleBinding.Namespace).Create(ctx, &roleBinding, metav1.CreateOptions{}); err != nil { - if !apierrors.IsAlreadyExists(err) { - return err - } - if _, err := cs.RbacV1().RoleBindings(roleBinding.Namespace).Update(ctx, &roleBinding, metav1.UpdateOptions{}); err != nil { + // try to create the given role binding. If it already exists, we fall through to + // attempting to update the existing role binding. + if err := retry.OnError(retry.DefaultBackoff, + func(err error) bool { + return !apierrors.IsAlreadyExists(err) + }, func() error { + if _, err := cs.RbacV1().RoleBindings(roleBinding.Namespace).Create(ctx, &roleBinding, metav1.CreateOptions{}); err != nil { + return err + } + return nil + }, + ); err != nil && apierrors.IsAlreadyExists(err) { + return retry.RetryOnConflict(retry.DefaultBackoff, func() error { + _, err := cs.RbacV1().RoleBindings(roleBinding.Namespace).Update(ctx, &roleBinding, metav1.UpdateOptions{}) return err - } + }) + } else { + return err } - return nil } From c3579cf3076a8b5f7141a0e8ebe4fa2c7c736be1 Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Wed, 7 Oct 2020 16:08:55 -0700 Subject: [PATCH 03/14] refactor to use k8s provided retry logic. start tests Signed-off-by: Brian Downs --- pkg/rke2/psp_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 pkg/rke2/psp_test.go diff --git a/pkg/rke2/psp_test.go b/pkg/rke2/psp_test.go new file mode 100644 index 0000000000..c151e7f814 --- /dev/null +++ b/pkg/rke2/psp_test.go @@ -0,0 +1,38 @@ +package rke2 + +import ( + "context" + "testing" + + "k8s.io/client-go/kubernetes" +) + +func Test_deployPodSecurityPolicyFromYaml(t *testing.T) { + type args struct { + ctx context.Context + cs *kubernetes.Clientset + pspYaml string + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "successful", + args: args{ + ctx: context.Background(), + cs: nil, + pspYaml: "", + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := deployPodSecurityPolicyFromYaml(tt.args.ctx, tt.args.cs, tt.args.pspYaml); (err != nil) != tt.wantErr { + t.Errorf("deployPodSecurityPolicyFromYaml() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} From d130417b2a49e4885d2916400f27b78761fc1a69 Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Wed, 7 Oct 2020 19:13:41 -0700 Subject: [PATCH 04/14] intermediate update in process Signed-off-by: Brian Downs --- Makefile | 4 ++++ pkg/rke2/psp.go | 2 +- pkg/rke2/psp_test.go | 38 ++++++++++++++++++++++++++++++++++---- scripts/unit-tests | 8 ++++++++ 4 files changed, 47 insertions(+), 5 deletions(-) create mode 100755 scripts/unit-tests diff --git a/Makefile b/Makefile index f14b8e50e1..b3aff8adc3 100644 --- a/Makefile +++ b/Makefile @@ -123,6 +123,10 @@ package-bundle: build ## Package the tarball bundle test: ./scripts/test +.PHONY: unit-tests +unit-tests: + ./scripts/unit-tests + ./.dapper: @echo Downloading dapper @curl -sL https://releases.rancher.com/dapper/v0.5.0/dapper-$$(uname -s)-$$(uname -m) > .dapper.tmp diff --git a/pkg/rke2/psp.go b/pkg/rke2/psp.go index 8b6c1c9a2c..0dfea093f4 100644 --- a/pkg/rke2/psp.go +++ b/pkg/rke2/psp.go @@ -389,7 +389,7 @@ func decodeYamlResource(data interface{}, yaml string) error { return decoder.Decode(data) } -func deployPodSecurityPolicyFromYaml(ctx context.Context, cs *kubernetes.Clientset, pspYaml string) error { +func deployPodSecurityPolicyFromYaml(ctx context.Context, cs kubernetes.Interface, pspYaml string) error { var psp v1beta1.PodSecurityPolicy if err := decodeYamlResource(&psp, pspYaml); err != nil { return err diff --git a/pkg/rke2/psp_test.go b/pkg/rke2/psp_test.go index c151e7f814..eca56d1ca2 100644 --- a/pkg/rke2/psp_test.go +++ b/pkg/rke2/psp_test.go @@ -2,15 +2,18 @@ package rke2 import ( "context" + "fmt" "testing" + "k8s.io/api/policy/v1beta1" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" ) func Test_deployPodSecurityPolicyFromYaml(t *testing.T) { type args struct { ctx context.Context - cs *kubernetes.Clientset + cs kubernetes.Interface pspYaml string } tests := []struct { @@ -19,11 +22,38 @@ func Test_deployPodSecurityPolicyFromYaml(t *testing.T) { wantErr bool }{ { - name: "successful", + name: "successfully create PSP", args: args{ ctx: context.Background(), - cs: nil, - pspYaml: "", + cs: fake.NewSimpleClientset(nil), + pspYaml: fmt.Sprintf(globalRestrictedPSPTemplate, "test-psp"), + }, + wantErr: false, + }, + { + name: "successfully update PSP", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(&v1beta1.PodSecurityPolicy{}), + pspYaml: fmt.Sprintf(globalRestrictedPSPTemplate, "test-psp"), + }, + wantErr: false, + }, + { + name: "fail to create", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(nil), + pspYaml: fmt.Sprintf(globalRestrictedPSPTemplate, "test-psp"), + }, + wantErr: false, + }, + { + name: "fail to update", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(nil), + pspYaml: fmt.Sprintf(globalRestrictedPSPTemplate, "test-psp"), }, wantErr: false, }, diff --git a/scripts/unit-tests b/scripts/unit-tests new file mode 100755 index 0000000000..1eb3d574e6 --- /dev/null +++ b/scripts/unit-tests @@ -0,0 +1,8 @@ +#!/usr/bin/env bash +set -ex + +source ./scripts/build-binary + +cd ../ + +go test -tags=selinux,osusergo,netgo -v -cover ./... From d50cbec3d30de30f1865eb419b22907f2e698f81 Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Wed, 7 Oct 2020 21:09:09 -0700 Subject: [PATCH 05/14] add unit tests that increases coverage by 15% and accounts for most code paths in refactored code Signed-off-by: Brian Downs --- pkg/rke2/psp.go | 20 +++-- pkg/rke2/psp_test.go | 191 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 188 insertions(+), 23 deletions(-) diff --git a/pkg/rke2/psp.go b/pkg/rke2/psp.go index 0dfea093f4..301565ec73 100644 --- a/pkg/rke2/psp.go +++ b/pkg/rke2/psp.go @@ -369,8 +369,6 @@ func updateNamespaceRef(ctx context.Context, cs *kubernetes.Clientset, ns *v1.Na return nil } -type deployFn func(context.Context, *kubernetes.Clientset, interface{}) error - // newClient create a new Kubernetes client from configuration. func newClient(kubeConfigPath string, k8sWrapTransport transport.WrapperFunc) (*kubernetes.Clientset, error) { config, err := clientcmd.BuildConfigFromFlags("", kubeConfigPath) @@ -411,12 +409,13 @@ func deployPodSecurityPolicyFromYaml(ctx context.Context, cs kubernetes.Interfac _, err := cs.PolicyV1beta1().PodSecurityPolicies().Update(ctx, &psp, metav1.UpdateOptions{}) return err }) - } else { + } else if err != nil { return err } + return nil } -func deployClusterRoleBindingFromYaml(ctx context.Context, cs *kubernetes.Clientset, clusterRoleBindingYaml string) error { +func deployClusterRoleBindingFromYaml(ctx context.Context, cs kubernetes.Interface, clusterRoleBindingYaml string) error { var clusterRoleBinding rbacv1.ClusterRoleBinding if err := decodeYamlResource(&clusterRoleBinding, clusterRoleBindingYaml); err != nil { return err @@ -438,12 +437,13 @@ func deployClusterRoleBindingFromYaml(ctx context.Context, cs *kubernetes.Client _, err := cs.RbacV1().ClusterRoleBindings().Update(ctx, &clusterRoleBinding, metav1.UpdateOptions{}) return err }) - } else { + } else if err != nil { return err } + return nil } -func deployClusterRoleFromYaml(ctx context.Context, cs *kubernetes.Clientset, clusterRoleYaml string) error { +func deployClusterRoleFromYaml(ctx context.Context, cs kubernetes.Interface, clusterRoleYaml string) error { var clusterRole rbacv1.ClusterRole if err := decodeYamlResource(&clusterRole, clusterRoleYaml); err != nil { return err @@ -465,12 +465,13 @@ func deployClusterRoleFromYaml(ctx context.Context, cs *kubernetes.Clientset, cl _, err := cs.RbacV1().ClusterRoles().Update(ctx, &clusterRole, metav1.UpdateOptions{}) return err }) - } else { + } else if err != nil { return err } + return nil } -func deployRoleBindingFromYaml(ctx context.Context, cs *kubernetes.Clientset, roleBindingYaml string) error { +func deployRoleBindingFromYaml(ctx context.Context, cs kubernetes.Interface, roleBindingYaml string) error { var roleBinding rbacv1.RoleBinding if err := decodeYamlResource(&roleBinding, roleBindingYaml); err != nil { return err @@ -492,7 +493,8 @@ func deployRoleBindingFromYaml(ctx context.Context, cs *kubernetes.Clientset, ro _, err := cs.RbacV1().RoleBindings(roleBinding.Namespace).Update(ctx, &roleBinding, metav1.UpdateOptions{}) return err }) - } else { + } else if err != nil { return err } + return nil } diff --git a/pkg/rke2/psp_test.go b/pkg/rke2/psp_test.go index eca56d1ca2..24529483b0 100644 --- a/pkg/rke2/psp_test.go +++ b/pkg/rke2/psp_test.go @@ -6,11 +6,38 @@ import ( "testing" "k8s.io/api/policy/v1beta1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" ) +var testPodSecurityPolicy = &v1beta1.PodSecurityPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-psp", + }, +} + +var testClusterRole = &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-role", + }, +} + +var testClusterRoleBinding = &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-role-binding", + }, +} + +var testRoleBinding = &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-role-binding", + }, +} + func Test_deployPodSecurityPolicyFromYaml(t *testing.T) { + pspYAML := fmt.Sprintf(globalRestrictedPSPTemplate, "test-psp") type args struct { ctx context.Context cs kubernetes.Interface @@ -25,43 +52,179 @@ func Test_deployPodSecurityPolicyFromYaml(t *testing.T) { name: "successfully create PSP", args: args{ ctx: context.Background(), - cs: fake.NewSimpleClientset(nil), - pspYaml: fmt.Sprintf(globalRestrictedPSPTemplate, "test-psp"), + cs: fake.NewSimpleClientset(&v1beta1.PodSecurityPolicy{}), + pspYaml: pspYAML, }, wantErr: false, }, + { + name: "fail to decode YAML", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(testPodSecurityPolicy), + pspYaml: "", + }, + wantErr: true, + }, { name: "successfully update PSP", args: args{ ctx: context.Background(), - cs: fake.NewSimpleClientset(&v1beta1.PodSecurityPolicy{}), - pspYaml: fmt.Sprintf(globalRestrictedPSPTemplate, "test-psp"), + cs: fake.NewSimpleClientset(testPodSecurityPolicy), + pspYaml: pspYAML, }, wantErr: false, }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := deployPodSecurityPolicyFromYaml(tt.args.ctx, tt.args.cs, tt.args.pspYaml); (err != nil) != tt.wantErr { + t.Errorf("deployPodSecurityPolicyFromYaml() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func Test_deployClusterRoleBindingFromYaml(t *testing.T) { + type args struct { + ctx context.Context + cs kubernetes.Interface + clusterRoleBindingYaml string + } + tests := []struct { + name string + args args + wantErr bool + }{ + { - name: "fail to create", + name: "successfully create cluster role binding", args: args{ - ctx: context.Background(), - cs: fake.NewSimpleClientset(nil), - pspYaml: fmt.Sprintf(globalRestrictedPSPTemplate, "test-psp"), + ctx: context.Background(), + cs: fake.NewSimpleClientset(&rbacv1.ClusterRoleBinding{}), + clusterRoleBindingYaml: fmt.Sprintf(kubeletAPIServerRoleBindingTemplate, "test-cluster-role-binding"), }, wantErr: false, }, { - name: "fail to update", + name: "fail to decode YAML", args: args{ - ctx: context.Background(), - cs: fake.NewSimpleClientset(nil), - pspYaml: fmt.Sprintf(globalRestrictedPSPTemplate, "test-psp"), + ctx: context.Background(), + cs: fake.NewSimpleClientset(testClusterRoleBinding), + clusterRoleBindingYaml: "", + }, + wantErr: true, + }, + { + name: "successfully update cluster role binding", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(testClusterRoleBinding), + clusterRoleBindingYaml: fmt.Sprintf(kubeletAPIServerRoleBindingTemplate, "test-cluster-role-binding"), }, wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := deployPodSecurityPolicyFromYaml(tt.args.ctx, tt.args.cs, tt.args.pspYaml); (err != nil) != tt.wantErr { - t.Errorf("deployPodSecurityPolicyFromYaml() error = %v, wantErr %v", err, tt.wantErr) + if err := deployClusterRoleBindingFromYaml(tt.args.ctx, tt.args.cs, tt.args.clusterRoleBindingYaml); (err != nil) != tt.wantErr { + t.Errorf("deployClusterRoleBindingFromYaml() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func Test_deployClusterRoleFromYaml(t *testing.T) { + type args struct { + ctx context.Context + cs kubernetes.Interface + clusterRoleYaml string + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "successfully create cluster role", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(&rbacv1.ClusterRole{}), + clusterRoleYaml: fmt.Sprintf(roleTemplate, "test-cluster-role", "test-resource-name"), + }, + wantErr: false, + }, + { + name: "fail to decode YAML", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(testClusterRole), + clusterRoleYaml: "", + }, + wantErr: true, + }, + { + name: "successfully update cluster role", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(testClusterRole), + clusterRoleYaml: fmt.Sprintf(roleTemplate, "test-cluster-role", "test-resource-name"), + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := deployClusterRoleFromYaml(tt.args.ctx, tt.args.cs, tt.args.clusterRoleYaml); (err != nil) != tt.wantErr { + t.Errorf("deployClusterRoleFromYaml() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func Test_deployRoleBindingFromYaml(t *testing.T) { + type args struct { + ctx context.Context + cs kubernetes.Interface + roleBindingYaml string + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "successfully create role binding", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(&rbacv1.RoleBinding{}), + roleBindingYaml: fmt.Sprintf(tunnelControllerRoleTemplate, "test-role-binding"), + }, + wantErr: false, + }, + { + name: "fail to decode YAML", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(testRoleBinding), + roleBindingYaml: "", + }, + wantErr: true, + }, + { + name: "successfully update role binding", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(testRoleBinding), + roleBindingYaml: fmt.Sprintf(tunnelControllerRoleTemplate, "test-role-binding"), + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := deployRoleBindingFromYaml(tt.args.ctx, tt.args.cs, tt.args.roleBindingYaml); (err != nil) != tt.wantErr { + t.Errorf("deployRoleBindingFromYaml() error = %v, wantErr %v", err, tt.wantErr) } }) } From 6abcd3d1348c24494157e1466d25b2262c4585e5 Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Thu, 8 Oct 2020 11:26:42 -0700 Subject: [PATCH 06/14] update make to run unit tests with the other tests Signed-off-by: Brian Downs --- Makefile | 4 ++-- pkg/rke2/psp_test.go | 28 ++++++++++++++++++---------- scripts/unit-tests | 7 ++----- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index b3aff8adc3..606d7f5d20 100644 --- a/Makefile +++ b/Makefile @@ -116,11 +116,11 @@ package-images: build-images ## Package docker images for airgap environment ./scripts/package-images .PHONY: package-bundle -package-bundle: build ## Package the tarball bundle +package-bundle: build ## Package the tarball bundle ./scripts/package-bundle .PHONY: test -test: +test: unit-tests ./scripts/test .PHONY: unit-tests diff --git a/pkg/rke2/psp_test.go b/pkg/rke2/psp_test.go index 24529483b0..b67b209a13 100644 --- a/pkg/rke2/psp_test.go +++ b/pkg/rke2/psp_test.go @@ -12,27 +12,34 @@ import ( "k8s.io/client-go/kubernetes/fake" ) +const ( + testPSPName = "test-psp" + testClusterRoleName = "test-cluster-role" + testClusterRoleBindingName = "test-cluster-role-binding" + testRoleBindingName = "test-role-binding" +) + var testPodSecurityPolicy = &v1beta1.PodSecurityPolicy{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-psp", + Name: testPSPName, }, } var testClusterRole = &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster-role", + Name: testClusterRoleName, }, } var testClusterRoleBinding = &rbacv1.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster-role-binding", + Name: testClusterRoleBindingName, }, } var testRoleBinding = &rbacv1.RoleBinding{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-role-binding", + Name: testRoleBindingName, }, } @@ -102,7 +109,7 @@ func Test_deployClusterRoleBindingFromYaml(t *testing.T) { args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(&rbacv1.ClusterRoleBinding{}), - clusterRoleBindingYaml: fmt.Sprintf(kubeletAPIServerRoleBindingTemplate, "test-cluster-role-binding"), + clusterRoleBindingYaml: fmt.Sprintf(kubeletAPIServerRoleBindingTemplate, testClusterRoleBindingName), }, wantErr: false, }, @@ -120,7 +127,7 @@ func Test_deployClusterRoleBindingFromYaml(t *testing.T) { args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(testClusterRoleBinding), - clusterRoleBindingYaml: fmt.Sprintf(kubeletAPIServerRoleBindingTemplate, "test-cluster-role-binding"), + clusterRoleBindingYaml: fmt.Sprintf(kubeletAPIServerRoleBindingTemplate, testClusterRoleBindingName), }, wantErr: false, }, @@ -135,6 +142,7 @@ func Test_deployClusterRoleBindingFromYaml(t *testing.T) { } func Test_deployClusterRoleFromYaml(t *testing.T) { + const testResourceName = "test-resource-name" type args struct { ctx context.Context cs kubernetes.Interface @@ -150,7 +158,7 @@ func Test_deployClusterRoleFromYaml(t *testing.T) { args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(&rbacv1.ClusterRole{}), - clusterRoleYaml: fmt.Sprintf(roleTemplate, "test-cluster-role", "test-resource-name"), + clusterRoleYaml: fmt.Sprintf(roleTemplate, "test-cluster-role", testResourceName), }, wantErr: false, }, @@ -168,7 +176,7 @@ func Test_deployClusterRoleFromYaml(t *testing.T) { args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(testClusterRole), - clusterRoleYaml: fmt.Sprintf(roleTemplate, "test-cluster-role", "test-resource-name"), + clusterRoleYaml: fmt.Sprintf(roleTemplate, "test-cluster-role", testResourceName), }, wantErr: false, }, @@ -198,7 +206,7 @@ func Test_deployRoleBindingFromYaml(t *testing.T) { args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(&rbacv1.RoleBinding{}), - roleBindingYaml: fmt.Sprintf(tunnelControllerRoleTemplate, "test-role-binding"), + roleBindingYaml: fmt.Sprintf(tunnelControllerRoleTemplate, testRoleBindingName), }, wantErr: false, }, @@ -216,7 +224,7 @@ func Test_deployRoleBindingFromYaml(t *testing.T) { args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(testRoleBinding), - roleBindingYaml: fmt.Sprintf(tunnelControllerRoleTemplate, "test-role-binding"), + roleBindingYaml: fmt.Sprintf(tunnelControllerRoleTemplate, testRoleBindingName), }, wantErr: false, }, diff --git a/scripts/unit-tests b/scripts/unit-tests index 1eb3d574e6..c19d5b69bc 100755 --- a/scripts/unit-tests +++ b/scripts/unit-tests @@ -1,8 +1,5 @@ #!/usr/bin/env bash -set -ex - -source ./scripts/build-binary -cd ../ +set -ex -go test -tags=selinux,osusergo,netgo -v -cover ./... +go test -v -cover ./... From 08057dab166bd50f97a59c9b484d95c16bbddd63 Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Thu, 8 Oct 2020 11:35:51 -0700 Subject: [PATCH 07/14] individualize makefile test targets Signed-off-by: Brian Downs --- Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 606d7f5d20..7c7b84b48a 100644 --- a/Makefile +++ b/Makefile @@ -120,13 +120,16 @@ package-bundle: build ## Package the tarball bundle ./scripts/package-bundle .PHONY: test -test: unit-tests - ./scripts/test +test: unit-tests integration-tests .PHONY: unit-tests unit-tests: ./scripts/unit-tests +.PHONY: integration-tests +integration-tests: + ./scripts/test + ./.dapper: @echo Downloading dapper @curl -sL https://releases.rancher.com/dapper/v0.5.0/dapper-$$(uname -s)-$$(uname -m) > .dapper.tmp From 2b8722e29544517a6e4050e219a05ff338133be7 Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Fri, 9 Oct 2020 12:12:30 -0700 Subject: [PATCH 08/14] mock calls to the api server with the client-go fake Signed-off-by: Brian Downs --- go.mod | 1 + go.sum | 2 + pkg/rke2/psp.go | 6 ++- pkg/rke2/psp_test.go | 96 +++++++++++++++++++++++++++++++++++++------- 4 files changed, 88 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index d3f373004e..a96281be7d 100644 --- a/go.mod +++ b/go.mod @@ -64,6 +64,7 @@ require ( github.com/sirupsen/logrus v1.6.0 github.com/urfave/cli v1.22.2 google.golang.org/grpc v1.31.1 + gopkg.in/kubernetes/client-go.v2 v2.0.0 k8s.io/api v0.19.0 k8s.io/apimachinery v0.19.0 k8s.io/apiserver v0.19.0 diff --git a/go.sum b/go.sum index 29635077b4..d39805f19d 100644 --- a/go.sum +++ b/go.sum @@ -1130,6 +1130,8 @@ gopkg.in/gcfg.v1 v1.2.0/go.mod h1:yesOnuUOFQAhST5vPY4nbZsb/huCgGGXlipJsBn0b3o= gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2/go.mod h1:Xk6kEKp8OKb+X14hQBKWaSkCsqBpgog8nAV2xsGOxlo= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= +gopkg.in/kubernetes/client-go.v2 v2.0.0 h1:q0rz2mkUAWjUxgr2y291HFsdfIYC2aEOHMfX8CF37NA= +gopkg.in/kubernetes/client-go.v2 v2.0.0/go.mod h1:caCeBjCdEkMVq3Va9BefCQjy9dxw4+zkONO3WEx49b0= gopkg.in/mcuadros/go-syslog.v2 v2.2.1/go.mod h1:l5LPIyOOyIdQquNg+oU6Z3524YwrcqEm0aKH+5zpt2U= gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce h1:xcEWjVhvbDy+nHP67nPDDpbYrY+ILlfndk4bRioVHaU= gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce/go.mod h1:yeKp02qBN3iKW1OzL3MGk2IdtZzaj7SFntXj72NppTA= diff --git a/pkg/rke2/psp.go b/pkg/rke2/psp.go index 301565ec73..0491b1b735 100644 --- a/pkg/rke2/psp.go +++ b/pkg/rke2/psp.go @@ -406,8 +406,10 @@ func deployPodSecurityPolicyFromYaml(ctx context.Context, cs kubernetes.Interfac }, ); err != nil && apierrors.IsAlreadyExists(err) { return retry.RetryOnConflict(retry.DefaultBackoff, func() error { - _, err := cs.PolicyV1beta1().PodSecurityPolicies().Update(ctx, &psp, metav1.UpdateOptions{}) - return err + if _, err := cs.PolicyV1beta1().PodSecurityPolicies().Update(ctx, &psp, metav1.UpdateOptions{}); err != nil { + return err + } + return nil }) } else if err != nil { return err diff --git a/pkg/rke2/psp_test.go b/pkg/rke2/psp_test.go index b67b209a13..d9c72e7d06 100644 --- a/pkg/rke2/psp_test.go +++ b/pkg/rke2/psp_test.go @@ -2,14 +2,20 @@ package rke2 import ( "context" + "errors" "fmt" "testing" "k8s.io/api/policy/v1beta1" rbacv1 "k8s.io/api/rbac/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" + fakepolicyv1beta1 "k8s.io/client-go/kubernetes/typed/policy/v1beta1/fake" + k8stesting "k8s.io/client-go/testing" ) const ( @@ -43,8 +49,38 @@ var testRoleBinding = &rbacv1.RoleBinding{ }, } +// fakeWithNonretriableError creates a new value of fake.Clientset +// pointer and sets a Reactor to return an error that is not retriable. +func fakeWithNonretriableError() *fake.Clientset { + cs := fake.NewSimpleClientset(testPodSecurityPolicy) + cs.PolicyV1beta1().(*fakepolicyv1beta1.FakePolicyV1beta1).PrependReactor("update", "*", + func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, &v1beta1.PodSecurityPolicy{}, errors.New("non retriable error") + }, + ) + return cs +} + +// fakeWithRetriableError creates a new value of fake.Clientset +// pointer and sets a Reactor to return an error that will be +// caught by retry logic. +func fakeWithRetriableError() *fake.Clientset { + cs := fake.NewSimpleClientset(testPodSecurityPolicy) + cs.PolicyV1beta1().(*fakepolicyv1beta1.FakePolicyV1beta1).PrependReactor("update", "*", + func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, &v1beta1.PodSecurityPolicy{}, + k8serrors.NewConflict(schema.GroupResource{ + Resource: "psp", + }, + "psp-update", nil, + ) + }, + ) + return cs +} + func Test_deployPodSecurityPolicyFromYaml(t *testing.T) { - pspYAML := fmt.Sprintf(globalRestrictedPSPTemplate, "test-psp") + pspYAML := fmt.Sprintf(globalRestrictedPSPTemplate, testPSPName) type args struct { ctx context.Context cs kubernetes.Interface @@ -55,44 +91,72 @@ func Test_deployPodSecurityPolicyFromYaml(t *testing.T) { args args wantErr bool }{ + { + name: "fail to decode YAML", + args: args{ + ctx: context.TODO(), + cs: fake.NewSimpleClientset(testPodSecurityPolicy), + pspYaml: "", + }, + wantErr: true, + }, { name: "successfully create PSP", args: args{ - ctx: context.Background(), + ctx: context.TODO(), cs: fake.NewSimpleClientset(&v1beta1.PodSecurityPolicy{}), pspYaml: pspYAML, }, wantErr: false, }, { - name: "fail to decode YAML", + name: "successfully update PSP", args: args{ - ctx: context.Background(), + ctx: context.TODO(), cs: fake.NewSimpleClientset(testPodSecurityPolicy), - pspYaml: "", + pspYaml: pspYAML, + }, + wantErr: false, + }, + { + name: "fail update PSP - nonretriable", + args: args{ + ctx: context.TODO(), + cs: fakeWithNonretriableError(), + pspYaml: pspYAML, }, wantErr: true, }, { - name: "successfully update PSP", + name: "fail update PSP - retriable error", args: args{ - ctx: context.Background(), - cs: fake.NewSimpleClientset(testPodSecurityPolicy), + ctx: context.TODO(), + cs: fakeWithRetriableError(), pspYaml: pspYAML, }, - wantErr: false, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if err := deployPodSecurityPolicyFromYaml(tt.args.ctx, tt.args.cs, tt.args.pspYaml); (err != nil) != tt.wantErr { + t.Log(err) t.Errorf("deployPodSecurityPolicyFromYaml() error = %v, wantErr %v", err, tt.wantErr) } + //verify that the existing PSP has in fact been updated from the given YAML. + if tt.name == "successfully create PSP" || tt.name == "successfully update PSP" { + val, _ := tt.args.cs.PolicyV1beta1().PodSecurityPolicies().Get(context.TODO(), testPSPName, metav1.GetOptions{}) + annocationsLen := len(val.Annotations) + if annocationsLen != 4 { + t.Errorf("expected 4 but got %d", annocationsLen) + } + } }) } } func Test_deployClusterRoleBindingFromYaml(t *testing.T) { + clusterRoleBindingYaml := fmt.Sprintf(kubeletAPIServerRoleBindingTemplate, testClusterRoleBindingName) type args struct { ctx context.Context cs kubernetes.Interface @@ -109,7 +173,7 @@ func Test_deployClusterRoleBindingFromYaml(t *testing.T) { args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(&rbacv1.ClusterRoleBinding{}), - clusterRoleBindingYaml: fmt.Sprintf(kubeletAPIServerRoleBindingTemplate, testClusterRoleBindingName), + clusterRoleBindingYaml: clusterRoleBindingYaml, }, wantErr: false, }, @@ -127,7 +191,7 @@ func Test_deployClusterRoleBindingFromYaml(t *testing.T) { args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(testClusterRoleBinding), - clusterRoleBindingYaml: fmt.Sprintf(kubeletAPIServerRoleBindingTemplate, testClusterRoleBindingName), + clusterRoleBindingYaml: clusterRoleBindingYaml, }, wantErr: false, }, @@ -143,6 +207,7 @@ func Test_deployClusterRoleBindingFromYaml(t *testing.T) { func Test_deployClusterRoleFromYaml(t *testing.T) { const testResourceName = "test-resource-name" + clusterRoleYaml := fmt.Sprintf(roleTemplate, "test-cluster-role", testResourceName) type args struct { ctx context.Context cs kubernetes.Interface @@ -158,7 +223,7 @@ func Test_deployClusterRoleFromYaml(t *testing.T) { args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(&rbacv1.ClusterRole{}), - clusterRoleYaml: fmt.Sprintf(roleTemplate, "test-cluster-role", testResourceName), + clusterRoleYaml: clusterRoleYaml, }, wantErr: false, }, @@ -176,7 +241,7 @@ func Test_deployClusterRoleFromYaml(t *testing.T) { args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(testClusterRole), - clusterRoleYaml: fmt.Sprintf(roleTemplate, "test-cluster-role", testResourceName), + clusterRoleYaml: clusterRoleYaml, }, wantErr: false, }, @@ -191,6 +256,7 @@ func Test_deployClusterRoleFromYaml(t *testing.T) { } func Test_deployRoleBindingFromYaml(t *testing.T) { + roleBindingYaml := fmt.Sprintf(tunnelControllerRoleTemplate, testRoleBindingName) type args struct { ctx context.Context cs kubernetes.Interface @@ -206,7 +272,7 @@ func Test_deployRoleBindingFromYaml(t *testing.T) { args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(&rbacv1.RoleBinding{}), - roleBindingYaml: fmt.Sprintf(tunnelControllerRoleTemplate, testRoleBindingName), + roleBindingYaml: roleBindingYaml, }, wantErr: false, }, @@ -224,7 +290,7 @@ func Test_deployRoleBindingFromYaml(t *testing.T) { args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(testRoleBinding), - roleBindingYaml: fmt.Sprintf(tunnelControllerRoleTemplate, testRoleBindingName), + roleBindingYaml: roleBindingYaml, }, wantErr: false, }, From df17e06ed31cdec8e2769c61b92863b933e8320c Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Fri, 9 Oct 2020 14:25:54 -0700 Subject: [PATCH 09/14] add function for mock setup Signed-off-by: Brian Downs --- pkg/rke2/psp_test.go | 103 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 82 insertions(+), 21 deletions(-) diff --git a/pkg/rke2/psp_test.go b/pkg/rke2/psp_test.go index d9c72e7d06..caa9707206 100644 --- a/pkg/rke2/psp_test.go +++ b/pkg/rke2/psp_test.go @@ -15,6 +15,7 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" fakepolicyv1beta1 "k8s.io/client-go/kubernetes/typed/policy/v1beta1/fake" + fakerbacv1 "k8s.io/client-go/kubernetes/typed/rbac/v1/fake" k8stesting "k8s.io/client-go/testing" ) @@ -49,33 +50,93 @@ var testRoleBinding = &rbacv1.RoleBinding{ }, } -// fakeWithNonretriableError creates a new value of fake.Clientset -// pointer and sets a Reactor to return an error that is not retriable. -func fakeWithNonretriableError() *fake.Clientset { +// fakeWithNonretriableError recieves a value of type runtime.Object, +// determines underlying underlying type, and creates a new value of +// type fake.Clientset pointer and sets a Reactor to return an error +// that is not retriable. +func fakeWithNonretriableError(ro interface{}) *fake.Clientset { cs := fake.NewSimpleClientset(testPodSecurityPolicy) - cs.PolicyV1beta1().(*fakepolicyv1beta1.FakePolicyV1beta1).PrependReactor("update", "*", - func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, &v1beta1.PodSecurityPolicy{}, errors.New("non retriable error") - }, - ) + const errMsg = "non retriable error" + switch ro.(type) { + case *v1beta1.PodSecurityPolicy: + cs.PolicyV1beta1().(*fakepolicyv1beta1.FakePolicyV1beta1).PrependReactor("update", "*", + func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, &v1beta1.PodSecurityPolicy{}, errors.New(errMsg) + }, + ) + case *rbacv1.ClusterRoleBinding: + cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("update", "*", + func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, &rbacv1.ClusterRoleBinding{}, errors.New(errMsg) + }, + ) + case *rbacv1.ClusterRole: + cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("update", "*", + func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, &rbacv1.ClusterRole{}, errors.New(errMsg) + }, + ) + case *rbacv1.RoleBinding: + cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("update", "*", + func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, &rbacv1.RoleBinding{}, errors.New(errMsg) + }, + ) + } return cs } // fakeWithRetriableError creates a new value of fake.Clientset // pointer and sets a Reactor to return an error that will be // caught by retry logic. -func fakeWithRetriableError() *fake.Clientset { +func fakeWithRetriableError(ro interface{}) *fake.Clientset { cs := fake.NewSimpleClientset(testPodSecurityPolicy) - cs.PolicyV1beta1().(*fakepolicyv1beta1.FakePolicyV1beta1).PrependReactor("update", "*", - func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, &v1beta1.PodSecurityPolicy{}, - k8serrors.NewConflict(schema.GroupResource{ - Resource: "psp", - }, - "psp-update", nil, - ) - }, - ) + switch ro.(type) { + case *v1beta1.PodSecurityPolicy: + cs.PolicyV1beta1().(*fakepolicyv1beta1.FakePolicyV1beta1).PrependReactor("update", "*", + func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, &v1beta1.PodSecurityPolicy{}, + k8serrors.NewConflict(schema.GroupResource{ + Resource: "psp", + }, + "psp-update", nil, + ) + }, + ) + case *rbacv1.ClusterRoleBinding: + cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("update", "*", + func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, &v1beta1.PodSecurityPolicy{}, + k8serrors.NewConflict(schema.GroupResource{ + Resource: "clusterolebindings", + }, + "cluster-role-binding-update", nil, + ) + }, + ) + case *rbacv1.ClusterRole: + cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("update", "*", + func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, &rbacv1.ClusterRoleBinding{}, + k8serrors.NewConflict(schema.GroupResource{ + Resource: "clusterrole", + }, + "cluster-role-update", nil, + ) + }, + ) + case *rbacv1.RoleBinding: + cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("update", "*", + func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, &rbacv1.RoleBinding{}, + k8serrors.NewConflict(schema.GroupResource{ + Resource: "rolebindings", + }, + "role-binding-update", nil, + ) + }, + ) + } return cs } @@ -122,7 +183,7 @@ func Test_deployPodSecurityPolicyFromYaml(t *testing.T) { name: "fail update PSP - nonretriable", args: args{ ctx: context.TODO(), - cs: fakeWithNonretriableError(), + cs: fakeWithNonretriableError(&v1beta1.PodSecurityPolicy{}), pspYaml: pspYAML, }, wantErr: true, @@ -131,7 +192,7 @@ func Test_deployPodSecurityPolicyFromYaml(t *testing.T) { name: "fail update PSP - retriable error", args: args{ ctx: context.TODO(), - cs: fakeWithRetriableError(), + cs: fakeWithRetriableError(&v1beta1.PodSecurityPolicy{}), pspYaml: pspYAML, }, wantErr: true, From 7e06cbf268873cc3f8b7826a38b095f23ee380f6 Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Fri, 9 Oct 2020 15:51:55 -0700 Subject: [PATCH 10/14] update tests and cases Signed-off-by: Brian Downs --- pkg/rke2/psp.go | 14 ++++-- pkg/rke2/psp_test.go | 108 ++++++++++++++++++++++++++++++++----------- 2 files changed, 89 insertions(+), 33 deletions(-) diff --git a/pkg/rke2/psp.go b/pkg/rke2/psp.go index 0491b1b735..10de828666 100644 --- a/pkg/rke2/psp.go +++ b/pkg/rke2/psp.go @@ -436,8 +436,10 @@ func deployClusterRoleBindingFromYaml(ctx context.Context, cs kubernetes.Interfa }, ); err != nil && apierrors.IsAlreadyExists(err) { return retry.RetryOnConflict(retry.DefaultBackoff, func() error { - _, err := cs.RbacV1().ClusterRoleBindings().Update(ctx, &clusterRoleBinding, metav1.UpdateOptions{}) - return err + if _, err := cs.RbacV1().ClusterRoleBindings().Update(ctx, &clusterRoleBinding, metav1.UpdateOptions{}); err != nil { + return err + } + return nil }) } else if err != nil { return err @@ -463,9 +465,11 @@ func deployClusterRoleFromYaml(ctx context.Context, cs kubernetes.Interface, clu return nil }, ); err != nil && apierrors.IsAlreadyExists(err) { - return retry.RetryOnConflict(retry.DefaultBackoff, func() error { - _, err := cs.RbacV1().ClusterRoles().Update(ctx, &clusterRole, metav1.UpdateOptions{}) - return err + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + if _, err := cs.RbacV1().ClusterRoles().Update(ctx, &clusterRole, metav1.UpdateOptions{}); err != nil { + return err + } + return nil }) } else if err != nil { return err diff --git a/pkg/rke2/psp_test.go b/pkg/rke2/psp_test.go index caa9707206..8c86ac402c 100644 --- a/pkg/rke2/psp_test.go +++ b/pkg/rke2/psp_test.go @@ -65,19 +65,19 @@ func fakeWithNonretriableError(ro interface{}) *fake.Clientset { }, ) case *rbacv1.ClusterRoleBinding: - cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("update", "*", + cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("*", "*", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { return true, &rbacv1.ClusterRoleBinding{}, errors.New(errMsg) }, ) case *rbacv1.ClusterRole: - cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("update", "*", + cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("*", "*", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { return true, &rbacv1.ClusterRole{}, errors.New(errMsg) }, ) case *rbacv1.RoleBinding: - cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("update", "*", + cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("*", "*", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { return true, &rbacv1.RoleBinding{}, errors.New(errMsg) }, @@ -104,9 +104,9 @@ func fakeWithRetriableError(ro interface{}) *fake.Clientset { }, ) case *rbacv1.ClusterRoleBinding: - cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("update", "*", + cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("*", "*", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, &v1beta1.PodSecurityPolicy{}, + return true, &rbacv1.ClusterRoleBinding{}, k8serrors.NewConflict(schema.GroupResource{ Resource: "clusterolebindings", }, @@ -115,9 +115,9 @@ func fakeWithRetriableError(ro interface{}) *fake.Clientset { }, ) case *rbacv1.ClusterRole: - cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("update", "*", + cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("*", "*", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, &rbacv1.ClusterRoleBinding{}, + return true, &rbacv1.ClusterRole{}, k8serrors.NewConflict(schema.GroupResource{ Resource: "clusterrole", }, @@ -126,7 +126,7 @@ func fakeWithRetriableError(ro interface{}) *fake.Clientset { }, ) case *rbacv1.RoleBinding: - cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("update", "*", + cs.RbacV1().(*fakerbacv1.FakeRbacV1).PrependReactor("*", "*", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { return true, &rbacv1.RoleBinding{}, k8serrors.NewConflict(schema.GroupResource{ @@ -201,7 +201,6 @@ func Test_deployPodSecurityPolicyFromYaml(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if err := deployPodSecurityPolicyFromYaml(tt.args.ctx, tt.args.cs, tt.args.pspYaml); (err != nil) != tt.wantErr { - t.Log(err) t.Errorf("deployPodSecurityPolicyFromYaml() error = %v, wantErr %v", err, tt.wantErr) } //verify that the existing PSP has in fact been updated from the given YAML. @@ -228,7 +227,15 @@ func Test_deployClusterRoleBindingFromYaml(t *testing.T) { args args wantErr bool }{ - + { + name: "fail to decode YAML", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(testClusterRoleBinding), + clusterRoleBindingYaml: "", + }, + wantErr: true, + }, { name: "successfully create cluster role binding", args: args{ @@ -239,22 +246,31 @@ func Test_deployClusterRoleBindingFromYaml(t *testing.T) { wantErr: false, }, { - name: "fail to decode YAML", + name: "successfully update cluster role binding", args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(testClusterRoleBinding), - clusterRoleBindingYaml: "", + clusterRoleBindingYaml: clusterRoleBindingYaml, + }, + wantErr: false, + }, + { + name: "fail update cluster role binding - nonretriable", + args: args{ + ctx: context.TODO(), + cs: fakeWithNonretriableError(&rbacv1.ClusterRoleBinding{}), + clusterRoleBindingYaml: clusterRoleBindingYaml, }, wantErr: true, }, { - name: "successfully update cluster role binding", + name: "fail update cluster role binding - retriable error", args: args{ - ctx: context.Background(), - cs: fake.NewSimpleClientset(testClusterRoleBinding), + ctx: context.TODO(), + cs: fakeWithRetriableError(&rbacv1.ClusterRoleBinding{}), clusterRoleBindingYaml: clusterRoleBindingYaml, }, - wantErr: false, + wantErr: true, }, } for _, tt := range tests { @@ -279,6 +295,15 @@ func Test_deployClusterRoleFromYaml(t *testing.T) { args args wantErr bool }{ + { + name: "fail to decode YAML", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(testClusterRole), + clusterRoleYaml: "", + }, + wantErr: true, + }, { name: "successfully create cluster role", args: args{ @@ -289,22 +314,31 @@ func Test_deployClusterRoleFromYaml(t *testing.T) { wantErr: false, }, { - name: "fail to decode YAML", + name: "successfully update cluster role", args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(testClusterRole), - clusterRoleYaml: "", + clusterRoleYaml: clusterRoleYaml, + }, + wantErr: false, + }, + { + name: "fail update cluster role binding - nonretriable", + args: args{ + ctx: context.TODO(), + cs: fakeWithNonretriableError(&rbacv1.ClusterRole{}), + clusterRoleYaml: clusterRoleYaml, }, wantErr: true, }, { - name: "successfully update cluster role", + name: "fail update cluster role binding - retriable error", args: args{ - ctx: context.Background(), - cs: fake.NewSimpleClientset(testClusterRole), + ctx: context.TODO(), + cs: fakeWithRetriableError(&rbacv1.ClusterRole{}), clusterRoleYaml: clusterRoleYaml, }, - wantErr: false, + wantErr: true, }, } for _, tt := range tests { @@ -328,6 +362,15 @@ func Test_deployRoleBindingFromYaml(t *testing.T) { args args wantErr bool }{ + { + name: "fail to decode YAML", + args: args{ + ctx: context.Background(), + cs: fake.NewSimpleClientset(testRoleBinding), + roleBindingYaml: "", + }, + wantErr: true, + }, { name: "successfully create role binding", args: args{ @@ -338,22 +381,31 @@ func Test_deployRoleBindingFromYaml(t *testing.T) { wantErr: false, }, { - name: "fail to decode YAML", + name: "successfully update role binding", args: args{ ctx: context.Background(), cs: fake.NewSimpleClientset(testRoleBinding), - roleBindingYaml: "", + roleBindingYaml: roleBindingYaml, + }, + wantErr: false, + }, + { + name: "fail update role binding - nonretriable", + args: args{ + ctx: context.TODO(), + cs: fakeWithNonretriableError(&rbacv1.RoleBinding{}), + roleBindingYaml: roleBindingYaml, }, wantErr: true, }, { - name: "successfully update role binding", + name: "fail update role binding - retriable error", args: args{ - ctx: context.Background(), - cs: fake.NewSimpleClientset(testRoleBinding), + ctx: context.TODO(), + cs: fakeWithRetriableError(&rbacv1.RoleBinding{}), roleBindingYaml: roleBindingYaml, }, - wantErr: false, + wantErr: true, }, } for _, tt := range tests { From 1db23568b1690a4fd0246d273230be3e37ab50b2 Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Mon, 12 Oct 2020 09:32:50 -0700 Subject: [PATCH 11/14] revert go mod Signed-off-by: Brian Downs --- go.mod | 1 - go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/go.mod b/go.mod index a96281be7d..d3f373004e 100644 --- a/go.mod +++ b/go.mod @@ -64,7 +64,6 @@ require ( github.com/sirupsen/logrus v1.6.0 github.com/urfave/cli v1.22.2 google.golang.org/grpc v1.31.1 - gopkg.in/kubernetes/client-go.v2 v2.0.0 k8s.io/api v0.19.0 k8s.io/apimachinery v0.19.0 k8s.io/apiserver v0.19.0 diff --git a/go.sum b/go.sum index d39805f19d..29635077b4 100644 --- a/go.sum +++ b/go.sum @@ -1130,8 +1130,6 @@ gopkg.in/gcfg.v1 v1.2.0/go.mod h1:yesOnuUOFQAhST5vPY4nbZsb/huCgGGXlipJsBn0b3o= gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2/go.mod h1:Xk6kEKp8OKb+X14hQBKWaSkCsqBpgog8nAV2xsGOxlo= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= -gopkg.in/kubernetes/client-go.v2 v2.0.0 h1:q0rz2mkUAWjUxgr2y291HFsdfIYC2aEOHMfX8CF37NA= -gopkg.in/kubernetes/client-go.v2 v2.0.0/go.mod h1:caCeBjCdEkMVq3Va9BefCQjy9dxw4+zkONO3WEx49b0= gopkg.in/mcuadros/go-syslog.v2 v2.2.1/go.mod h1:l5LPIyOOyIdQquNg+oU6Z3524YwrcqEm0aKH+5zpt2U= gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce h1:xcEWjVhvbDy+nHP67nPDDpbYrY+ILlfndk4bRioVHaU= gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce/go.mod h1:yeKp02qBN3iKW1OzL3MGk2IdtZzaj7SFntXj72NppTA= From 5c5f567df5e354116c2cea61389810772c87604d Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Mon, 12 Oct 2020 11:24:08 -0700 Subject: [PATCH 12/14] revert if checks from testing Signed-off-by: Brian Downs --- pkg/rke2/psp.go | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/pkg/rke2/psp.go b/pkg/rke2/psp.go index 10de828666..f41f2151d5 100644 --- a/pkg/rke2/psp.go +++ b/pkg/rke2/psp.go @@ -399,17 +399,14 @@ func deployPodSecurityPolicyFromYaml(ctx context.Context, cs kubernetes.Interfac func(err error) bool { return !apierrors.IsAlreadyExists(err) }, func() error { - if _, err := cs.PolicyV1beta1().PodSecurityPolicies().Create(ctx, &psp, metav1.CreateOptions{}); err != nil { - return err - } - return nil + _, err := cs.PolicyV1beta1().PodSecurityPolicies().Create(ctx, &psp, metav1.CreateOptions{}) + return err + }, ); err != nil && apierrors.IsAlreadyExists(err) { return retry.RetryOnConflict(retry.DefaultBackoff, func() error { - if _, err := cs.PolicyV1beta1().PodSecurityPolicies().Update(ctx, &psp, metav1.UpdateOptions{}); err != nil { - return err - } - return nil + _, err := cs.PolicyV1beta1().PodSecurityPolicies().Update(ctx, &psp, metav1.UpdateOptions{}) + return err }) } else if err != nil { return err @@ -436,10 +433,8 @@ func deployClusterRoleBindingFromYaml(ctx context.Context, cs kubernetes.Interfa }, ); err != nil && apierrors.IsAlreadyExists(err) { return retry.RetryOnConflict(retry.DefaultBackoff, func() error { - if _, err := cs.RbacV1().ClusterRoleBindings().Update(ctx, &clusterRoleBinding, metav1.UpdateOptions{}); err != nil { - return err - } - return nil + _, err := cs.RbacV1().ClusterRoleBindings().Update(ctx, &clusterRoleBinding, metav1.UpdateOptions{}) + return err }) } else if err != nil { return err @@ -459,17 +454,13 @@ func deployClusterRoleFromYaml(ctx context.Context, cs kubernetes.Interface, clu func(err error) bool { return !apierrors.IsAlreadyExists(err) }, func() error { - if _, err := cs.RbacV1().ClusterRoles().Create(ctx, &clusterRole, metav1.CreateOptions{}); err != nil { - return err - } - return nil + _, err := cs.RbacV1().ClusterRoles().Create(ctx, &clusterRole, metav1.CreateOptions{}) + return err }, ); err != nil && apierrors.IsAlreadyExists(err) { return retry.RetryOnConflict(retry.DefaultRetry, func() error { - if _, err := cs.RbacV1().ClusterRoles().Update(ctx, &clusterRole, metav1.UpdateOptions{}); err != nil { - return err - } - return nil + _, err := cs.RbacV1().ClusterRoles().Update(ctx, &clusterRole, metav1.UpdateOptions{}) + return err }) } else if err != nil { return err @@ -489,10 +480,8 @@ func deployRoleBindingFromYaml(ctx context.Context, cs kubernetes.Interface, rol func(err error) bool { return !apierrors.IsAlreadyExists(err) }, func() error { - if _, err := cs.RbacV1().RoleBindings(roleBinding.Namespace).Create(ctx, &roleBinding, metav1.CreateOptions{}); err != nil { - return err - } - return nil + _, err := cs.RbacV1().RoleBindings(roleBinding.Namespace).Create(ctx, &roleBinding, metav1.CreateOptions{}) + return err }, ); err != nil && apierrors.IsAlreadyExists(err) { return retry.RetryOnConflict(retry.DefaultBackoff, func() error { From 8a73478ecb6da466d279a17245f9d720e3285055 Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Mon, 12 Oct 2020 12:52:45 -0700 Subject: [PATCH 13/14] update error handling Signed-off-by: Brian Downs --- pkg/rke2/psp.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/rke2/psp.go b/pkg/rke2/psp.go index f41f2151d5..a04d789557 100644 --- a/pkg/rke2/psp.go +++ b/pkg/rke2/psp.go @@ -426,10 +426,8 @@ func deployClusterRoleBindingFromYaml(ctx context.Context, cs kubernetes.Interfa func(err error) bool { return !apierrors.IsAlreadyExists(err) }, func() error { - if _, err := cs.RbacV1().ClusterRoleBindings().Create(ctx, &clusterRoleBinding, metav1.CreateOptions{}); err != nil { - return err - } - return nil + _, err := cs.RbacV1().ClusterRoleBindings().Create(ctx, &clusterRoleBinding, metav1.CreateOptions{}) + return err }, ); err != nil && apierrors.IsAlreadyExists(err) { return retry.RetryOnConflict(retry.DefaultBackoff, func() error { From ca2f78b00a1b6dfbffd8a22abacfce89da37526b Mon Sep 17 00:00:00 2001 From: Brian Downs Date: Wed, 14 Oct 2020 13:09:58 -0700 Subject: [PATCH 14/14] update the update logic to retrieve the latest version of the object being updated Signed-off-by: Brian Downs --- pkg/rke2/psp.go | 55 +++++++++++++++++++++++++++++++++++---- pkg/rke2/psp_templates.go | 3 +-- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/pkg/rke2/psp.go b/pkg/rke2/psp.go index a04d789557..365d8a9f22 100644 --- a/pkg/rke2/psp.go +++ b/pkg/rke2/psp.go @@ -401,11 +401,21 @@ func deployPodSecurityPolicyFromYaml(ctx context.Context, cs kubernetes.Interfac }, func() error { _, err := cs.PolicyV1beta1().PodSecurityPolicies().Create(ctx, &psp, metav1.CreateOptions{}) return err - }, ); err != nil && apierrors.IsAlreadyExists(err) { return retry.RetryOnConflict(retry.DefaultBackoff, func() error { - _, err := cs.PolicyV1beta1().PodSecurityPolicies().Update(ctx, &psp, metav1.UpdateOptions{}) + retrievedPSP, err := cs.PolicyV1beta1().PodSecurityPolicies().Get(ctx, psp.Name, metav1.GetOptions{}) + if err != nil { + return err + } + if retrievedPSP.Annotations == nil { + retrievedPSP.Annotations = make(map[string]string, len(psp.Annotations)) + } + for k, v := range psp.Annotations { + retrievedPSP.Annotations[k] = v + } + retrievedPSP.Spec = psp.Spec + _, err = cs.PolicyV1beta1().PodSecurityPolicies().Update(ctx, retrievedPSP, metav1.UpdateOptions{}) return err }) } else if err != nil { @@ -431,7 +441,19 @@ func deployClusterRoleBindingFromYaml(ctx context.Context, cs kubernetes.Interfa }, ); err != nil && apierrors.IsAlreadyExists(err) { return retry.RetryOnConflict(retry.DefaultBackoff, func() error { - _, err := cs.RbacV1().ClusterRoleBindings().Update(ctx, &clusterRoleBinding, metav1.UpdateOptions{}) + retrievedCRB, err := cs.RbacV1().ClusterRoleBindings().Get(ctx, clusterRoleBinding.Name, metav1.GetOptions{}) + if err != nil { + return err + } + if retrievedCRB.Annotations == nil { + retrievedCRB.Annotations = make(map[string]string, len(clusterRoleBinding.Annotations)) + } + for k, v := range clusterRoleBinding.Annotations { + retrievedCRB.Annotations[k] = v + } + retrievedCRB.Subjects = clusterRoleBinding.Subjects + retrievedCRB.RoleRef = clusterRoleBinding.RoleRef + _, err = cs.RbacV1().ClusterRoleBindings().Update(ctx, retrievedCRB, metav1.UpdateOptions{}) return err }) } else if err != nil { @@ -457,7 +479,18 @@ func deployClusterRoleFromYaml(ctx context.Context, cs kubernetes.Interface, clu }, ); err != nil && apierrors.IsAlreadyExists(err) { return retry.RetryOnConflict(retry.DefaultRetry, func() error { - _, err := cs.RbacV1().ClusterRoles().Update(ctx, &clusterRole, metav1.UpdateOptions{}) + retrievedCR, err := cs.RbacV1().ClusterRoles().Get(ctx, clusterRole.Name, metav1.GetOptions{}) + if err != nil { + return err + } + if retrievedCR.Annotations == nil { + retrievedCR.Annotations = make(map[string]string, len(clusterRole.Annotations)) + } + for k, v := range clusterRole.Annotations { + retrievedCR.Annotations[k] = v + } + retrievedCR.Rules = clusterRole.Rules + _, err = cs.RbacV1().ClusterRoles().Update(ctx, retrievedCR, metav1.UpdateOptions{}) return err }) } else if err != nil { @@ -483,7 +516,19 @@ func deployRoleBindingFromYaml(ctx context.Context, cs kubernetes.Interface, rol }, ); err != nil && apierrors.IsAlreadyExists(err) { return retry.RetryOnConflict(retry.DefaultBackoff, func() error { - _, err := cs.RbacV1().RoleBindings(roleBinding.Namespace).Update(ctx, &roleBinding, metav1.UpdateOptions{}) + retrievedR, err := cs.RbacV1().RoleBindings(roleBinding.Namespace).Get(ctx, roleBinding.Name, metav1.GetOptions{}) + if err != nil { + return err + } + if retrievedR.Annotations == nil { + retrievedR.Annotations = make(map[string]string, len(roleBinding.Annotations)) + } + for k, v := range roleBinding.Annotations { + retrievedR.Annotations[k] = v + } + retrievedR.Subjects = roleBinding.Subjects + retrievedR.RoleRef = roleBinding.RoleRef + _, err = cs.RbacV1().RoleBindings(roleBinding.Namespace).Update(ctx, retrievedR, metav1.UpdateOptions{}) return err }) } else if err != nil { diff --git a/pkg/rke2/psp_templates.go b/pkg/rke2/psp_templates.go index 917061147d..eb7e692a03 100644 --- a/pkg/rke2/psp_templates.go +++ b/pkg/rke2/psp_templates.go @@ -127,8 +127,7 @@ subjects: name: system:authenticated ` -const systemUnrestrictedPSPTemplate = ` -apiVersion: policy/v1beta1 +const systemUnrestrictedPSPTemplate = `apiVersion: policy/v1beta1 kind: PodSecurityPolicy metadata: name: %s