diff --git a/.github/workflows/push_pr.yaml b/.github/workflows/push_pr.yaml index 8e800940..42fa173b 100644 --- a/.github/workflows/push_pr.yaml +++ b/.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 diff --git a/pkg/utils/rbacutils.go b/pkg/utils/rbacutils.go index 905b4c2b..5a061903 100644 --- a/pkg/utils/rbacutils.go +++ b/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), diff --git a/pkg/utils/rbacutils_test.go b/pkg/utils/rbacutils_test.go new file mode 100644 index 00000000..26b3434e --- /dev/null +++ b/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, + }, + } +}