Skip to content

Commit

Permalink
Refactor RoleBinding Helm charts into component (#539)
Browse files Browse the repository at this point in the history
* Add rolebinding component
- Add utils function `GetRoleBindingName`
- Add unit tests for RoleBinding component

* Remove RoleBinding from template yaml from chart

* Addressed review feeback

* Apply suggestions from code review

Co-authored-by: Madhav Bhargava <madhav.bhargava@sap.com>

* Revert the changes which are not related to this PR's scope.

* Use Etcd Druid group from types_etcd GroupVersion.Group

---------

Co-authored-by: Madhav Bhargava <madhav.bhargava@sap.com>
  • Loading branch information
seshachalam-yv and unmarshall authored Apr 14, 2023
1 parent ebc8311 commit fe0d4cf
Show file tree
Hide file tree
Showing 10 changed files with 434 additions and 90 deletions.
7 changes: 6 additions & 1 deletion api/v1alpha1/types_etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,5 +473,10 @@ func (e *Etcd) GetAsOwnerReference() metav1.OwnerReference {

// GetRoleName returns the role name for the Etcd
func (e *Etcd) GetRoleName() string {
return fmt.Sprintf("druid.gardener.cloud:etcd:%s", e.Name)
return fmt.Sprintf("%s:etcd:%s", GroupVersion.Group, e.Name)
}

// GetRoleBindingName returns the rolebinding name for the Etcd
func (e *Etcd) GetRoleBindingName() string {
return fmt.Sprintf("%s:etcd:%s", GroupVersion.Group, e.Name)
}
9 changes: 7 additions & 2 deletions api/v1alpha1/types_etcd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,13 @@ var _ = Describe("Etcd", func() {

Context("GetRoleName", func() {
It("should return the role name for the Etcd", func() {
expected := "druid.gardener.cloud:etcd:foo"
Expect(created.GetRoleName()).To(Equal(expected))
Expect(created.GetRoleName()).To(Equal(GroupVersion.Group + ":etcd:foo"))
})
})

Context("GetRoleBindingName", func() {
It("should return the rolebinding name for the Etcd", func() {
Expect(created.GetRoleName()).To(Equal(GroupVersion.Group + ":etcd:foo"))
})
})
})
Expand Down
26 changes: 0 additions & 26 deletions charts/etcd/templates/etcd-rolebinding.yaml

This file was deleted.

73 changes: 12 additions & 61 deletions controllers/etcd/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"
"fmt"
"path/filepath"
"reflect"

druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1"
"github.com/gardener/etcd-druid/controllers/utils"
Expand All @@ -27,6 +26,7 @@ import (
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"
componentrolebinding "github.com/gardener/etcd-druid/pkg/component/etcd/rolebinding"
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"
Expand All @@ -40,7 +40,6 @@ import (
"github.com/go-logr/logr"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
rbac "k8s.io/api/rbac/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -280,6 +279,14 @@ func (r *Reconciler) delete(ctx context.Context, etcd *druidv1alpha1.Etcd) (ctrl
}, err
}

roleBindingValues := componentrolebinding.GenerateValues(etcd)
roleBindingDeployer := componentrolebinding.New(r.Client, roleBindingValues)
if err := roleBindingDeployer.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 {
Expand All @@ -292,40 +299,6 @@ func (r *Reconciler) delete(ctx context.Context, etcd *druidv1alpha1.Etcd) (ctrl
return ctrl.Result{}, 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

decoded, err := r.chart.decodeRoleBinding(etcd.Name, etcd.Namespace, values)
if err != nil {
return err
}

roleBindingObj := &rbac.RoleBinding{}
key := client.ObjectKeyFromObject(decoded)
if err := r.Get(ctx, key, roleBindingObj); err != nil {
if !apierrors.IsNotFound(err) {
return err
}
if err := r.Create(ctx, decoded); err != nil {
return err
}
logger.Info("Creating rolebinding", "rolebinding", kutil.Key(decoded.Namespace, decoded.Name).String())
return nil
}

if !reflect.DeepEqual(decoded.RoleRef, roleBindingObj.RoleRef) || !reflect.DeepEqual(decoded.Subjects, roleBindingObj.Subjects) {
roleBindingObjCopy := roleBindingObj.DeepCopy()
roleBindingObjCopy.RoleRef = decoded.RoleRef
roleBindingObjCopy.Subjects = decoded.Subjects
if err := r.Patch(ctx, roleBindingObjCopy, client.MergeFrom(roleBindingObj)); err != nil {
return err
}
}

return err
}

func (r *Reconciler) reconcileEtcd(ctx context.Context, logger logr.Logger, etcd *druidv1alpha1.Etcd) reconcileResult {
// Check if Spec.Replicas is odd or even.
// TODO(timuthy): The following checks should rather be part of a validation. Also re-enqueuing doesn't make sense in case the values are invalid.
Expand Down Expand Up @@ -381,11 +354,9 @@ func (r *Reconciler) reconcileEtcd(ctx context.Context, logger logr.Logger, etcd
return reconcileResult{err: err}
}

roleBindingValues, err := r.getMapFromEtcd(etcd, r.config.DisableEtcdServiceAccountAutomount)
if err != nil {
return reconcileResult{err: err}
}
err = r.reconcileRoleBinding(ctx, logger, etcd, roleBindingValues)
roleBindingValues := componentrolebinding.GenerateValues(etcd)
roleBindingDeployer := componentrolebinding.New(r.Client, roleBindingValues)
err = roleBindingDeployer.Deploy(ctx)
if err != nil {
return reconcileResult{err: err}
}
Expand Down Expand Up @@ -420,26 +391,6 @@ func (r *Reconciler) reconcileEtcd(ctx context.Context, logger logr.Logger, etcd
return reconcileResult{svcName: &serviceValues.ClientServiceName, sts: sts, err: err}
}

func (r *Reconciler) getMapFromEtcd(etcd *druidv1alpha1.Etcd, disableEtcdServiceAccountAutomount bool) (map[string]interface{}, error) {
pdbMinAvailable := 0
if etcd.Spec.Replicas > 1 {
pdbMinAvailable = int(etcd.Spec.Replicas)
}

values := map[string]interface{}{
"name": etcd.Name,
"uid": etcd.UID,
"labels": etcd.Spec.Labels,
"pdbMinAvailable": pdbMinAvailable,
"serviceAccountName": etcd.GetServiceAccountName(),
"disableEtcdServiceAccountAutomount": disableEtcdServiceAccountAutomount,
"roleName": fmt.Sprintf("druid.gardener.cloud:etcd:%s", etcd.Name),
"roleBindingName": fmt.Sprintf("druid.gardener.cloud:etcd:%s", etcd.Name),
}

return values, nil
}

func isPeerTLSChangedToEnabled(peerTLSEnabledStatusFromMembers bool, configMapValues *componentconfigmap.Values) bool {
if peerTLSEnabledStatusFromMembers {
return false
Expand Down
76 changes: 76 additions & 0 deletions pkg/component/etcd/rolebinding/rolebinding.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// 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 rolebinding

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 {
roleBinding := c.emptyRoleBinding()
_, err := controllerutils.GetAndCreateOrStrategicMergePatch(ctx, c.client, roleBinding, func() error {
roleBinding.Name = c.values.Name
roleBinding.Namespace = c.values.Namespace
roleBinding.Labels = c.values.Labels
roleBinding.OwnerReferences = c.values.OwnerReferences
roleBinding.RoleRef = rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "Role",
Name: c.values.RoleName,
}
roleBinding.Subjects = []rbacv1.Subject{
{
Kind: "ServiceAccount",
Name: c.values.ServiceAccountName,
Namespace: c.values.Namespace,
},
}
return nil
})
return err
}

func (c component) Destroy(ctx context.Context) error {
return client.IgnoreNotFound(c.client.Delete(ctx, c.emptyRoleBinding()))
}

func (c *component) emptyRoleBinding() *rbacv1.RoleBinding {
return &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: c.values.Name,
Namespace: c.values.Namespace,
},
}
}

// New creates a new role binding deployer instance.
func New(c client.Client, value *Values) gardenercomponent.Deployer {
return &component{
client: c,
values: value,
}
}
27 changes: 27 additions & 0 deletions pkg/component/etcd/rolebinding/rolebinding_suite_test.go
Original file line number Diff line number Diff line change
@@ -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 rolebinding

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestRoleBinding(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "RoleBinding Component Suite")
}
Loading

0 comments on commit fe0d4cf

Please sign in to comment.