From 823bf183ccaf42a989cb1b109cab2f26e8c2089b Mon Sep 17 00:00:00 2001 From: Seshachalam Yerasala Venkata Date: Mon, 27 Feb 2023 17:50:13 +0530 Subject: [PATCH] Add role component - Remove Role from template yaml from chart - Add utils function `GetRoleName` - Removed old `reconcileRole` - Add unit tests for Role component --- api/v1alpha1/types_etcd.go | 5 + charts/etcd/templates/etcd-role.yaml | 47 ------ controllers/etcd/reconciler.go | 45 ++---- pkg/component/etcd/role/role.go | 85 ++++++++++ pkg/component/etcd/role/role_suite_test.go | 27 ++++ pkg/component/etcd/role/role_test.go | 146 ++++++++++++++++++ pkg/component/etcd/role/value_helper_test.go | 63 ++++++++ pkg/component/etcd/role/values.go | 29 ++++ pkg/component/etcd/role/values_helper.go | 32 ++++ .../controllers/etcd/reconciler_test.go | 2 +- 10 files changed, 399 insertions(+), 82 deletions(-) delete mode 100644 charts/etcd/templates/etcd-role.yaml create mode 100644 pkg/component/etcd/role/role.go create mode 100644 pkg/component/etcd/role/role_suite_test.go create mode 100644 pkg/component/etcd/role/role_test.go create mode 100644 pkg/component/etcd/role/value_helper_test.go create mode 100644 pkg/component/etcd/role/values.go create mode 100644 pkg/component/etcd/role/values_helper.go diff --git a/api/v1alpha1/types_etcd.go b/api/v1alpha1/types_etcd.go index 21dfc3c66..8c2bf2e97 100644 --- a/api/v1alpha1/types_etcd.go +++ b/api/v1alpha1/types_etcd.go @@ -470,3 +470,8 @@ func (e *Etcd) GetAsOwnerReference() metav1.OwnerReference { BlockOwnerDeletion: pointer.Bool(true), } } + +// GetRoleName returns the role name for the Etcd +func (e *Etcd) GetRoleName() string { + return fmt.Sprintf("druid.gardener.cloud:etcd:%s", e.Name) +} diff --git a/charts/etcd/templates/etcd-role.yaml b/charts/etcd/templates/etcd-role.yaml deleted file mode 100644 index 518ba6dac..000000000 --- a/charts/etcd/templates/etcd-role.yaml +++ /dev/null @@ -1,47 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: Role -metadata: - name: {{ .Values.roleName }} - namespace: {{ .Release.Namespace }} - ownerReferences: - - apiVersion: druid.gardener.cloud/v1alpha1 - blockOwnerDeletion: true - controller: true - kind: Etcd - name: {{ .Values.name }} - uid: {{ .Values.uid }} - labels: - name: etcd - instance: {{ .Values.name }} -{{- if .Values.labels }} -{{ toYaml .Values.labels | indent 4 }} -{{- end }} -rules: -- apiGroups: - - coordination.k8s.io - resources: - - leases - verbs: - - list - - get - - update - - patch - - watch -- apiGroups: - - apps - resources: - - statefulsets - verbs: - - get - - list - - patch - - update - - watch -- apiGroups: - - "" - resources: - - pods - verbs: - - get - - list - - watch \ No newline at end of file diff --git a/controllers/etcd/reconciler.go b/controllers/etcd/reconciler.go index 5c37789f1..6c5fa2e81 100644 --- a/controllers/etcd/reconciler.go +++ b/controllers/etcd/reconciler.go @@ -26,6 +26,7 @@ import ( componentconfigmap "github.com/gardener/etcd-druid/pkg/component/etcd/configmap" componentlease "github.com/gardener/etcd-druid/pkg/component/etcd/lease" componentpdb "github.com/gardener/etcd-druid/pkg/component/etcd/poddisruptionbudget" + componentrole "github.com/gardener/etcd-druid/pkg/component/etcd/role" componentservice "github.com/gardener/etcd-druid/pkg/component/etcd/service" componentserviceaccount "github.com/gardener/etcd-druid/pkg/component/etcd/serviceaccount" componentsts "github.com/gardener/etcd-druid/pkg/component/etcd/statefulset" @@ -271,6 +272,14 @@ func (r *Reconciler) delete(ctx context.Context, etcd *druidv1alpha1.Etcd) (ctrl }, err } + roleValues := componentrole.GenerateValues(etcd) + roleDeployer := componentrole.New(r.Client, roleValues) + if err := roleDeployer.Destroy(ctx); err != nil { + return ctrl.Result{ + Requeue: true, + }, err + } + if sets.NewString(etcd.Finalizers...).Has(common.FinalizerName) { logger.Info("Removing finalizer") if err := controllerutils.RemoveFinalizers(ctx, r.Client, etcd, common.FinalizerName); client.IgnoreNotFound(err) != nil { @@ -283,39 +292,6 @@ func (r *Reconciler) delete(ctx context.Context, etcd *druidv1alpha1.Etcd) (ctrl return ctrl.Result{}, nil } -func (r *Reconciler) reconcileRole(ctx context.Context, logger logr.Logger, etcd *druidv1alpha1.Etcd, values map[string]interface{}) error { - logger.Info("Reconciling role") - var err error - - decoded, err := r.chart.decodeRole(etcd.Name, etcd.Namespace, values) - if err != nil { - return err - } - - roleObj := &rbac.Role{} - key := client.ObjectKeyFromObject(decoded) - if err := r.Get(ctx, key, roleObj); err != nil { - if !apierrors.IsNotFound(err) { - return err - } - if err := r.Create(ctx, decoded); err != nil { - return err - } - logger.Info("Creating role", "role", kutil.Key(decoded.Namespace, decoded.Name).String()) - return nil - } - - if !reflect.DeepEqual(decoded.Rules, roleObj.Rules) { - roleObjCopy := roleObj.DeepCopy() - roleObjCopy.Rules = decoded.Rules - if err := r.Patch(ctx, roleObjCopy, client.MergeFrom(roleObj)); err != nil { - return err - } - } - - return nil -} - func (r *Reconciler) reconcileRoleBinding(ctx context.Context, logger logr.Logger, etcd *druidv1alpha1.Etcd, values map[string]interface{}) error { logger.Info("Reconciling rolebinding") var err error @@ -403,7 +379,8 @@ func (r *Reconciler) reconcileEtcd(ctx context.Context, logger logr.Logger, etcd return reconcileResult{err: err} } - err = r.reconcileRole(ctx, logger, etcd, roleValues) + roleDeployer := componentrole.New(r.Client, componentrole.GenerateValues(etcd)) + err = roleDeployer.Deploy(ctx) if err != nil { return reconcileResult{err: err} } diff --git a/pkg/component/etcd/role/role.go b/pkg/component/etcd/role/role.go new file mode 100644 index 000000000..e87d38955 --- /dev/null +++ b/pkg/component/etcd/role/role.go @@ -0,0 +1,85 @@ +// Copyright (c) 2023 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package role + +import ( + "context" + + "github.com/gardener/gardener/pkg/controllerutils" + + gardenercomponent "github.com/gardener/gardener/pkg/operation/botanist/component" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type component struct { + client client.Client + values *Values +} + +func (c component) Deploy(ctx context.Context) error { + role := c.emptyRole() + _, err := controllerutils.GetAndCreateOrStrategicMergePatch(ctx, c.client, role, func() error { + role.Name = c.values.Name + role.Namespace = c.values.Namespace + role.Labels = c.values.Labels + role.OwnerReferences = c.values.OwnerReferences + role.Rules = getRules() + return nil + }) + return err +} + +func (c component) Destroy(ctx context.Context) error { + return client.IgnoreNotFound(c.client.Delete(ctx, c.emptyRole())) +} + +func (c component) emptyRole() *rbacv1.Role { + return &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: c.values.Name, + Namespace: c.values.Namespace, + }, + } +} + +func getRules() []rbacv1.PolicyRule { + return []rbacv1.PolicyRule{ + { + APIGroups: []string{"coordination.k8s.io"}, + Resources: []string{"leases"}, + Verbs: []string{"list", "get", "update", "patch", "watch"}, + }, + { + APIGroups: []string{"apps"}, + Resources: []string{"statefulsets"}, + Verbs: []string{"get", "list", "patch", "update", "watch"}, + }, + { + APIGroups: []string{""}, + Resources: []string{"pods"}, + Verbs: []string{"get", "list", "watch"}, + }, + } +} + +// New creates a new role deployer instance. +func New(c client.Client, values *Values) gardenercomponent.Deployer { + return &component{ + client: c, + values: values, + } +} diff --git a/pkg/component/etcd/role/role_suite_test.go b/pkg/component/etcd/role/role_suite_test.go new file mode 100644 index 000000000..98656d090 --- /dev/null +++ b/pkg/component/etcd/role/role_suite_test.go @@ -0,0 +1,27 @@ +// Copyright (c) 2023 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package role + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestService(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Role Component Suite") +} diff --git a/pkg/component/etcd/role/role_test.go b/pkg/component/etcd/role/role_test.go new file mode 100644 index 000000000..079e198d3 --- /dev/null +++ b/pkg/component/etcd/role/role_test.go @@ -0,0 +1,146 @@ +// Copyright (c) 2023 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package role_test + +import ( + "context" + + "github.com/gardener/etcd-druid/api/v1alpha1" + "github.com/gardener/etcd-druid/pkg/client/kubernetes" + "github.com/gardener/etcd-druid/pkg/component/etcd/role" + + . "github.com/gardener/gardener/pkg/utils/test/matchers" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +var _ = Describe("Role Component", Ordered, func() { + var ( + ctx = context.TODO() + c = fake.NewClientBuilder().WithScheme(kubernetes.Scheme).Build() + values = getTestRoleValues() + roleComponent = role.New(c, values) + ) + + Describe("#Deploy", func() { + It("should create the Role with the expected values", func() { + By("creating a Role") + err := roleComponent.Deploy(ctx) + Expect(err).NotTo(HaveOccurred()) + + By("verifying that the Role is created on the K8s cluster as expected") + created := &rbacv1.Role{} + err = c.Get(ctx, getRoleKeyFromValue(values), created) + Expect(err).NotTo(HaveOccurred()) + verifyRoleValues(created, values) + }) + It("should update the Role with the expected values", func() { + By("updating the Role") + values.Labels["new"] = "label" + err := roleComponent.Deploy(ctx) + Expect(err).NotTo(HaveOccurred()) + + By("verifying that the Role is updated on the K8s cluster as expected") + updated := &rbacv1.Role{} + err = c.Get(ctx, getRoleKeyFromValue(values), updated) + Expect(err).NotTo(HaveOccurred()) + verifyRoleValues(updated, values) + }) + It("should not return an error when there is nothing to update the Role", func() { + err := roleComponent.Deploy(ctx) + Expect(err).NotTo(HaveOccurred()) + updated := &rbacv1.Role{} + err = c.Get(ctx, getRoleKeyFromValue(values), updated) + Expect(err).NotTo(HaveOccurred()) + verifyRoleValues(updated, values) + }) + It("should return an error when the update fails", func() { + values.Name = "" + err := roleComponent.Deploy(ctx) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Required value: name is required")) + }) + }) + + Describe("#Destroy", func() { + It("should delete the Role", func() { + By("deleting the Role") + err := roleComponent.Destroy(ctx) + Expect(err).NotTo(HaveOccurred()) + + By("verifying that the Role is deleted from the K8s cluster as expected") + role := &rbacv1.Role{} + Expect(c.Get(ctx, getRoleKeyFromValue(values), role)).To(BeNotFoundError()) + }) + It("should not return an error when there is nothing to delete", func() { + err := roleComponent.Destroy(ctx) + Expect(err).NotTo(HaveOccurred()) + }) + }) +}) + +func verifyRoleValues(expected *rbacv1.Role, values *role.Values) { + Expect(expected.Name).To(Equal(values.Name)) + Expect(expected.Labels).To(Equal(values.Labels)) + Expect(expected.Namespace).To(Equal(values.Namespace)) + Expect(expected.OwnerReferences).To(Equal(values.OwnerReferences)) + Expect(expected.Rules).To(Equal([]rbacv1.PolicyRule{ + { + APIGroups: []string{"coordination.k8s.io"}, + Resources: []string{"leases"}, + Verbs: []string{"list", "get", "update", "patch", "watch"}, + }, + { + APIGroups: []string{"apps"}, + Resources: []string{"statefulsets"}, + Verbs: []string{"get", "list", "patch", "update", "watch"}, + }, + { + APIGroups: []string{""}, + Resources: []string{"pods"}, + Verbs: []string{"get", "list", "watch"}, + }, + })) +} +func getRoleKeyFromValue(values *role.Values) types.NamespacedName { + return client.ObjectKey{Name: values.Name, Namespace: values.Namespace} +} + +func getTestRoleValues() *role.Values { + return &role.Values{ + Name: "test-role", + Namespace: "test-namespace", + Labels: map[string]string{ + "foo": "bar", + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "etcd", + Name: "test-etcd", + UID: "123-456-789", + Controller: pointer.Bool(true), + BlockOwnerDeletion: pointer.Bool(true), + }, + }, + } +} diff --git a/pkg/component/etcd/role/value_helper_test.go b/pkg/component/etcd/role/value_helper_test.go new file mode 100644 index 000000000..ac2c9fefa --- /dev/null +++ b/pkg/component/etcd/role/value_helper_test.go @@ -0,0 +1,63 @@ +// Copyright (c) 2023 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package role_test + +import ( + "github.com/gardener/etcd-druid/api/v1alpha1" + "github.com/gardener/etcd-druid/pkg/component/etcd/role" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = Describe("Role", func() { + var ( + etcd = &v1alpha1.Etcd{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "Etcd", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "etcd-name", + Namespace: "etcd-namespace", + UID: "etcd-uid", + }, + Spec: v1alpha1.EtcdSpec{ + Labels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + } + expected = &role.Values{ + Name: etcd.GetRoleName(), + Namespace: etcd.Namespace, + Labels: map[string]string{ + "name": "etcd", + "instance": etcd.Name, + }, + OwnerReferences: []metav1.OwnerReference{ + etcd.GetAsOwnerReference(), + }, + } + ) + + Context("Generate Values", func() { + It("should generate correct values", func() { + values := role.GenerateValues(etcd) + Expect(values).To(Equal(expected)) + }) + }) +}) diff --git a/pkg/component/etcd/role/values.go b/pkg/component/etcd/role/values.go new file mode 100644 index 000000000..1fac861b6 --- /dev/null +++ b/pkg/component/etcd/role/values.go @@ -0,0 +1,29 @@ +// Copyright (c) 2023 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package role + +import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + +// Values defines the fields used to create a Role for Etcd. +type Values struct { + // Name is the name of the Role. + Name string + // Namespace is the namespace of the Role. + Namespace string + // OwnerReferences are the OwnerReferences of the Role. + OwnerReferences []metav1.OwnerReference + // Labels are the labels of the Role. + Labels map[string]string +} diff --git a/pkg/component/etcd/role/values_helper.go b/pkg/component/etcd/role/values_helper.go new file mode 100644 index 000000000..f4f87ed14 --- /dev/null +++ b/pkg/component/etcd/role/values_helper.go @@ -0,0 +1,32 @@ +// Copyright (c) 2023 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package role + +import ( + druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// GenerateValues generates `role.Values` for the role component with the given `etcd` object. +func GenerateValues(etcd *druidv1alpha1.Etcd) *Values { + return &Values{ + Name: etcd.GetRoleName(), + Namespace: etcd.Namespace, + Labels: etcd.GetDefaultLabels(), + OwnerReferences: []metav1.OwnerReference{ + etcd.GetAsOwnerReference(), + }, + } +} diff --git a/test/integration/controllers/etcd/reconciler_test.go b/test/integration/controllers/etcd/reconciler_test.go index 31891ff41..9476fdc63 100644 --- a/test/integration/controllers/etcd/reconciler_test.go +++ b/test/integration/controllers/etcd/reconciler_test.go @@ -424,7 +424,7 @@ var _ = Describe("Multinode ETCD", func() { func validateRole(instance *druidv1alpha1.Etcd, role *rbac.Role) { Expect(*role).To(MatchFields(IgnoreExtras, Fields{ "ObjectMeta": MatchFields(IgnoreExtras, Fields{ - "Name": Equal(fmt.Sprintf("druid.gardener.cloud:etcd:%s", instance.Name)), + "Name": Equal(instance.GetRoleName()), "Namespace": Equal(instance.Namespace), "Labels": MatchKeys(IgnoreExtras, Keys{ "name": Equal("etcd"),