feat: renamed csr approval functions and introduced better error handling

pkg/frontend/admin_openshiftcluster_approvecsr.go

@@ -54,8 +54,8 @@ func (f *frontend) _postAdminOpenShiftClusterApproveCSR(ctx context.Context, r *
 	}
 
 	if csrName != "" {
-		return k.RunCertificateApprove(ctx, csrName)
+		return k.ApproveCsr(ctx, csrName)
 	}
 
-	return k.RunCertificateMassApprove(ctx)
+	return k.ApproveAllCsrs(ctx)
 }

pkg/frontend/admin_openshiftcluster_approvecsr_test.go

@@ -44,7 +44,7 @@ func TestAdminApproveCSR(t *testing.T) {
 			csrName:    "aro-csr",
 			mocks: func(tt *test, k *mock_adminactions.MockKubeActions) {
 				k.EXPECT().
-					RunCertificateApprove(gomock.Any(), tt.csrName).
+					ApproveCsr(gomock.Any(), tt.csrName).
 					Return(nil)
 			},
 			wantStatusCode: http.StatusOK,
@@ -55,7 +55,7 @@ func TestAdminApproveCSR(t *testing.T) {
 			resourceID: fmt.Sprintf("/subscriptions/%s/resourcegroups/resourceGroup/providers/Microsoft.RedHatOpenShift/openShiftClusters/resourceName", mockSubID),
 			mocks: func(tt *test, k *mock_adminactions.MockKubeActions) {
 				k.EXPECT().
-					RunCertificateMassApprove(gomock.Any()).
+					ApproveAllCsrs(gomock.Any()).
 					Return(nil)
 			},
 			wantStatusCode: http.StatusOK,

pkg/frontend/adminactions/certificatesigningrequests.go

@@ -15,22 +15,25 @@ import (
 	"github.com/Azure/ARO-RP/pkg/api"
 )
 
-func (k *kubeActions) RunCertificateApprove(ctx context.Context, csrName string) error {
+func (k *kubeActions) ApproveCsr(ctx context.Context, csrName string) error {
 	csr, err := k.kubecli.CertificatesV1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
+	if err != nil {
 		if kerrors.IsNotFound(err) {
 			return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeResourceNotFound, "", "certificate signing request '%s' was not found.", csrName)
 		}
-
-	return k.RunCertificateApprovalUpdate(ctx, csr)
+		return err
 	}
 
-func (k *kubeActions) RunCertificateMassApprove(ctx context.Context) error {
+	return k.updateCsr(ctx, csr)
+}
+
+func (k *kubeActions) ApproveAllCsrs(ctx context.Context) error {
 	csrs, err := k.kubecli.CertificatesV1().CertificateSigningRequests().List(ctx, metav1.ListOptions{})
 	if err != nil {
 		return err
 	}
 	for _, csr := range csrs.Items {
-		err = k.RunCertificateApprovalUpdate(ctx, &csr)
+		err = k.updateCsr(ctx, &csr)
 		if err != nil {
 			return err
 		}
@@ -38,7 +41,7 @@ func (k *kubeActions) RunCertificateMassApprove(ctx context.Context) error {
 	return nil
 }
 
-func (k *kubeActions) RunCertificateApprovalUpdate(ctx context.Context, csr *certificatesv1.CertificateSigningRequest) error {
+func (k *kubeActions) updateCsr(ctx context.Context, csr *certificatesv1.CertificateSigningRequest) error {
 	modifiedCSR, hasCondition, err := addConditionIfNeeded(csr, string(certificatesv1.CertificateDenied), string(certificatesv1.CertificateApproved), "AROSupportApprove", "This CSR was approved by ARO support personnel.")
 
 	if err != nil {

pkg/frontend/adminactions/kubeactions.go

@@ -10,7 +10,6 @@ import (
 	"github.com/Azure/go-autorest/autorest/to"
 	configclient "github.com/openshift/client-go/config/clientset/versioned"
 	"github.com/sirupsen/logrus"
-	certificatesv1 "k8s.io/api/certificates/v1"
 	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -32,9 +31,8 @@ type KubeActions interface {
 	KubeDelete(ctx context.Context, groupKind, namespace, name string, force bool) error
 	CordonNode(ctx context.Context, nodeName string, unschedulable bool) error
 	DrainNode(ctx context.Context, nodeName string) error
-	RunCertificateApprove(ctx context.Context, csrName string) error
-	RunCertificateMassApprove(ctx context.Context) error
-	RunCertificateApprovalUpdate(ctx context.Context, csr *certificatesv1.CertificateSigningRequest) error
+	ApproveCsr(ctx context.Context, csrName string) error
+	ApproveAllCsrs(ctx context.Context) error
 	Upgrade(ctx context.Context, upgradeY bool) error
 	KubeGetPodLogs(ctx context.Context, namespace, name, containerName string) ([]byte, error)
 }

pkg/util/mocks/adminactions/adminactions.go

@@ -12,7 +12,6 @@ import (
 	compute "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-06-01/compute"
 	gomock "github.com/golang/mock/gomock"
 	logrus "github.com/sirupsen/logrus"
-	v1 "k8s.io/api/certificates/v1"
 	unstructured "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
@@ -39,6 +38,34 @@ func (m *MockKubeActions) EXPECT() *MockKubeActionsMockRecorder {
 	return m.recorder
 }
 
+// ApproveAllCsrs mocks base method.
+func (m *MockKubeActions) ApproveAllCsrs(arg0 context.Context) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "ApproveAllCsrs", arg0)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// ApproveAllCsrs indicates an expected call of ApproveAllCsrs.
+func (mr *MockKubeActionsMockRecorder) ApproveAllCsrs(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ApproveAllCsrs", reflect.TypeOf((*MockKubeActions)(nil).ApproveAllCsrs), arg0)
+}
+
+// ApproveCsr mocks base method.
+func (m *MockKubeActions) ApproveCsr(arg0 context.Context, arg1 string) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "ApproveCsr", arg0, arg1)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// ApproveCsr indicates an expected call of ApproveCsr.
+func (mr *MockKubeActionsMockRecorder) ApproveCsr(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ApproveCsr", reflect.TypeOf((*MockKubeActions)(nil).ApproveCsr), arg0, arg1)
+}
+
 // CordonNode mocks base method.
 func (m *MockKubeActions) CordonNode(arg0 context.Context, arg1 string, arg2 bool) error {
 	m.ctrl.T.Helper()
@@ -140,48 +167,6 @@ func (mr *MockKubeActionsMockRecorder) KubeList(arg0, arg1, arg2 interface{}) *g
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "KubeList", reflect.TypeOf((*MockKubeActions)(nil).KubeList), arg0, arg1, arg2)
 }
 
-// RunCertificateApprovalUpdate mocks base method.
-func (m *MockKubeActions) RunCertificateApprovalUpdate(arg0 context.Context, arg1 *v1.CertificateSigningRequest) error {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "RunCertificateApprovalUpdate", arg0, arg1)
-	ret0, _ := ret[0].(error)
-	return ret0
-}
-
-// RunCertificateApprovalUpdate indicates an expected call of RunCertificateApprovalUpdate.
-func (mr *MockKubeActionsMockRecorder) RunCertificateApprovalUpdate(arg0, arg1 interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RunCertificateApprovalUpdate", reflect.TypeOf((*MockKubeActions)(nil).RunCertificateApprovalUpdate), arg0, arg1)
-}
-
-// RunCertificateApprove mocks base method.
-func (m *MockKubeActions) RunCertificateApprove(arg0 context.Context, arg1 string) error {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "RunCertificateApprove", arg0, arg1)
-	ret0, _ := ret[0].(error)
-	return ret0
-}
-
-// RunCertificateApprove indicates an expected call of RunCertificateApprove.
-func (mr *MockKubeActionsMockRecorder) RunCertificateApprove(arg0, arg1 interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RunCertificateApprove", reflect.TypeOf((*MockKubeActions)(nil).RunCertificateApprove), arg0, arg1)
-}
-
-// RunCertificateMassApprove mocks base method.
-func (m *MockKubeActions) RunCertificateMassApprove(arg0 context.Context) error {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "RunCertificateMassApprove", arg0)
-	ret0, _ := ret[0].(error)
-	return ret0
-}
-
-// RunCertificateMassApprove indicates an expected call of RunCertificateMassApprove.
-func (mr *MockKubeActionsMockRecorder) RunCertificateMassApprove(arg0 interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RunCertificateMassApprove", reflect.TypeOf((*MockKubeActions)(nil).RunCertificateMassApprove), arg0)
-}
-
 // Upgrade mocks base method.
 func (m *MockKubeActions) Upgrade(arg0 context.Context, arg1 bool) error {
 	m.ctrl.T.Helper()