Error if rolebinding exists with different role reference in namespace (#2579)

* added unit test for setup role binding method
* changes to throw an error in case of mismatch in cluster role name
* changes to add a check for role kind

.github/workflows/push_pr.yaml

@@ -78,7 +78,7 @@ jobs:
 
       - name: Install Skaffold
         run: |
-          curl -Lo skaffold https://storage.googleapis.com/skaffold/releases/latest/skaffold-linux-amd64
+          curl -Lo skaffold https://storage.googleapis.com/skaffold/releases/v1.39.2/skaffold-linux-amd64
           sudo install skaffold /usr/local/bin/
           skaffold version
 

pkg/utils/rbacutils.go

@@ -252,6 +252,16 @@ func SetupRoleBinding(ctx context.Context, logger *zap.Logger, k8sClient kuberne
 				zap.String("role_binding_namespace", roleBindingNs))
 			return AddSaToRoleBindingWithRetries(ctx, logger, k8sClient, roleBinding, roleBindingNs, sa, saNamespace, role, roleKind)
 		}
+		if rbObj.RoleRef.Name != role || rbObj.RoleRef.Kind != roleKind {
+			logger.Error("rolebinding with different role references exists",
+				zap.String("role_binding", rbObj.Name),
+				zap.String("role_binding_namespace", roleBindingNs),
+				zap.String("roleref", role),
+				zap.String("roleref_kind", roleKind),
+				zap.String("old_roleref", rbObj.RoleRef.Name),
+				zap.String("old_roleref_kind", rbObj.RoleRef.Kind))
+			return fmt.Errorf("rolebinding %s in namespace %s exists with different roleref, retry by deleting existing rolebinding", rbObj.Name, roleBindingNs)
+		}
 		logger.Debug("service account already present in rolebinding so nothing to add",
 			zap.String("service_account_name", sa),
 			zap.String("service_account_namespace", saNamespace),

pkg/utils/rbacutils_test.go

@@ -0,0 +1,81 @@
+package utils
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/rbac/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes/fake"
+
+	fv1 "github.com/fission/fission/pkg/apis/core/v1"
+	"github.com/fission/fission/pkg/utils/loggerfactory"
+)
+
+const (
+	namespace      string = "testns"
+	serviceAccount string = "testSA"
+	clusterRole    string = "testClusterRole"
+	rolebinding    string = "testRolebinding"
+)
+
+func TestSetupRoleBinding(t *testing.T) {
+	ctx := context.Background()
+	logger := loggerfactory.GetLogger()
+	kubernetesClient := fake.NewSimpleClientset()
+
+	//case 1 => when role binding doesn't exists
+	_, err := createServiceAccount(ctx, kubernetesClient)
+	if err != nil {
+		t.Fatalf("Error creating service account: %s", err.Error())
+	}
+	_, err = createClusterRole(ctx, clusterRole, kubernetesClient)
+	if err != nil {
+		t.Fatalf("Error creating cluster role: %s", err.Error())
+	}
+	err = SetupRoleBinding(ctx, logger, kubernetesClient, rolebinding, namespace, clusterRole, fv1.ClusterRole, serviceAccount, namespace)
+	assert.Nil(t, err, "error should be nil and new role binding will get created")
+
+	//case 2 => rolebinding exists but service account doesn't exists
+	err = kubernetesClient.CoreV1().ServiceAccounts(namespace).Delete(ctx, serviceAccount, metav1.DeleteOptions{})
+	if err != nil {
+		t.Fatalf("Error deleting service account: %s", err.Error())
+	}
+	err = SetupRoleBinding(ctx, logger, kubernetesClient, rolebinding, namespace, clusterRole, fv1.ClusterRole, serviceAccount, namespace)
+	assert.Nil(t, err, "error should be nil and service account should add in rolebinding")
+
+	//case 3 => rolebinding, cluster role and service account, all exists
+	err = SetupRoleBinding(ctx, logger, kubernetesClient, rolebinding, namespace, clusterRole, fv1.ClusterRole, serviceAccount, namespace)
+	assert.Nil(t, err, "error should be nil and nothing to add")
+
+	//case 4 => This must fail, if there is change in cluster-role-name
+	err = SetupRoleBinding(ctx, logger, kubernetesClient, rolebinding, namespace, "invalid-cluster-name", fv1.ClusterRole, serviceAccount, namespace)
+	assert.NotNil(t, err)
+	assert.Equal(t, err.Error(), fmt.Sprintf("rolebinding %s in namespace %s exists with different roleref, retry by deleting existing rolebinding", rolebinding, namespace))
+}
+
+func createClusterRole(ctx context.Context, clusterRole string, kubernetesClient *fake.Clientset) (*v1.ClusterRole, error) {
+	objRole := MakeClusterRoleObj(clusterRole)
+	var err error
+	objRole, err = kubernetesClient.RbacV1().ClusterRoles().Create(ctx, objRole, metav1.CreateOptions{})
+	return objRole, err
+}
+
+func createServiceAccount(ctx context.Context, kubernetesClient *fake.Clientset) (*corev1.ServiceAccount, error) {
+	objSA := MakeSAObj(serviceAccount, namespace)
+	var err error
+	objSA, err = kubernetesClient.CoreV1().ServiceAccounts(namespace).Create(ctx, objSA, metav1.CreateOptions{})
+	return objSA, err
+}
+
+// MakeClusterRoleObj returns a ClusterRole object
+func MakeClusterRoleObj(clusterRoleName string) *v1.ClusterRole {
+	return &v1.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: clusterRoleName,
+		},
+	}
+}