From 2f9d96b6bae902e33b481c969b7a56751f82bca3 Mon Sep 17 00:00:00 2001 From: rashmi_kh Date: Wed, 5 Feb 2025 17:20:09 +0530 Subject: [PATCH] wrap service account not found error --- internal/authentication/tokengetter.go | 20 +++++++ internal/authentication/tokengetter_test.go | 4 +- .../clusterextension_controller.go | 7 +++ .../clusterextension_controller_test.go | 52 +++++++++++++++++++ internal/controllers/suite_test.go | 4 ++ 5 files changed, 86 insertions(+), 1 deletion(-) diff --git a/internal/authentication/tokengetter.go b/internal/authentication/tokengetter.go index 3fec56f37..9e563dc39 100644 --- a/internal/authentication/tokengetter.go +++ b/internal/authentication/tokengetter.go @@ -2,10 +2,12 @@ package authentication import ( "context" + "fmt" "sync" "time" authenticationv1 "k8s.io/api/authentication/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" corev1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -19,6 +21,21 @@ type TokenGetter struct { mu sync.RWMutex } +type ServiceAccountNotFoundError struct { + ServiceAccountName string + ServiceAccountNamespace string + Err error +} + +func (e *ServiceAccountNotFoundError) Unwrap() error { + return e.Err +} + +// Error implements the error interface for ServiceAccountNotFoundError. +func (e *ServiceAccountNotFoundError) Error() string { + return fmt.Sprintf("service account %q not found in namespace %q: unable to authenticate with the Kubernetes cluster.", e.ServiceAccountName, e.ServiceAccountNamespace) +} + type TokenGetterOption func(*TokenGetter) const ( @@ -86,6 +103,9 @@ func (t *TokenGetter) getToken(ctx context.Context, key types.NamespacedName) (* Spec: authenticationv1.TokenRequestSpec{ExpirationSeconds: ptr.To(int64(t.expirationDuration / time.Second))}, }, metav1.CreateOptions{}) if err != nil { + if errors.IsNotFound(err) { + return nil, &ServiceAccountNotFoundError{ServiceAccountName: key.Name, ServiceAccountNamespace: key.Namespace} + } return nil, err } return &req.Status, nil diff --git a/internal/authentication/tokengetter_test.go b/internal/authentication/tokengetter_test.go index b9553cac3..663e95f1b 100644 --- a/internal/authentication/tokengetter_test.go +++ b/internal/authentication/tokengetter_test.go @@ -72,13 +72,15 @@ func TestTokenGetterGet(t *testing.T) { "test-namespace-3", "test-token-3", "failed to get token"}, {"Testing error when getting token from fake client", "test-service-account-4", "test-namespace-4", "error when fetching token", "error when fetching token"}, + {"Testing service account not found", "missing-sa", + "test-namespace-5", "", "service account \"missing-sa\" not found in namespace \"test-namespace-5\": unable to authenticate with the Kubernetes cluster."}, } for _, tc := range tests { got, err := tg.Get(context.Background(), types.NamespacedName{Namespace: tc.namespace, Name: tc.serviceAccountName}) if err != nil { t.Logf("%s: expected: %v, got: %v", tc.testName, tc.want, err) - assert.EqualError(t, err, tc.errorMsg) + assert.EqualError(t, err, tc.errorMsg, "Error message should match expected output") } else { t.Logf("%s: expected: %v, got: %v", tc.testName, tc.want, got) assert.Equal(t, tc.want, got, tc.errorMsg) diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go index 66c61de6f..9353790db 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -50,6 +50,7 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" catalogd "github.com/operator-framework/operator-controller/catalogd/api/v1" + "github.com/operator-framework/operator-controller/internal/authentication" "github.com/operator-framework/operator-controller/internal/bundleutil" "github.com/operator-framework/operator-controller/internal/conditionsets" "github.com/operator-framework/operator-controller/internal/contentmanager" @@ -206,6 +207,12 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.Cl installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, ext) if err != nil { setInstallStatus(ext, nil) + var saerr *authentication.ServiceAccountNotFoundError + if errors.As(err, &saerr) { + setInstalledStatusConditionUnknown(ext, saerr.Error()) + setStatusProgressing(ext, errors.New("installation cannot proceed due to missing ServiceAccount")) + return ctrl.Result{}, err + } setInstalledStatusConditionUnknown(ext, err.Error()) setStatusProgressing(ext, errors.New("retrying to get installed bundle")) return ctrl.Result{}, err diff --git a/internal/controllers/clusterextension_controller_test.go b/internal/controllers/clusterextension_controller_test.go index ab4dc5e18..af65df0e2 100644 --- a/internal/controllers/clusterextension_controller_test.go +++ b/internal/controllers/clusterextension_controller_test.go @@ -28,6 +28,7 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/authentication" "github.com/operator-framework/operator-controller/internal/conditionsets" "github.com/operator-framework/operator-controller/internal/controllers" "github.com/operator-framework/operator-controller/internal/finalizers" @@ -332,6 +333,57 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T) require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{})) } +func TestClusterExtensionServiceAccountNotFound(t *testing.T) { + cl, reconciler := newClientAndReconciler(t) + reconciler.InstalledBundleGetter = &MockInstalledBundleGetter{ + err: &authentication.ServiceAccountNotFoundError{}, // Simulate missing SA + } + + ctx := context.Background() + extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} + + t.Log("Given a cluster extension with a missing service account") + clusterExtension := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, + Spec: ocv1.ClusterExtensionSpec{ + Source: ocv1.SourceConfig{ + SourceType: "Catalog", + Catalog: &ocv1.CatalogSource{ + PackageName: "test-package", + }, + }, + Namespace: "default", + ServiceAccount: ocv1.ServiceAccountReference{ + Name: "missing-sa", + }, + }, + } + + require.NoError(t, cl.Create(ctx, clusterExtension)) + + t.Log("When reconciling the cluster extension") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) + + require.Equal(t, ctrl.Result{}, res) + require.Error(t, err) + require.IsType(t, &authentication.ServiceAccountNotFoundError{}, err) + t.Log("By fetching updated cluster extension after reconcile") + require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) + + t.Log("By checking the status conditions") + installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled) + require.NotNil(t, installedCond) + require.Equal(t, metav1.ConditionUnknown, installedCond.Status) + require.Contains(t, installedCond.Message, "unable to authenticate with the Kubernetes cluster") + + progressingCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing) + require.NotNil(t, progressingCond) + require.Equal(t, metav1.ConditionTrue, progressingCond.Status) + require.Equal(t, ocv1.ReasonRetrying, progressingCond.Reason) + require.Contains(t, progressingCond.Message, "installation cannot proceed due to missing ServiceAccount") + require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{})) +} + func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) { cl, reconciler := newClientAndReconciler(t) reconciler.Unpacker = &MockUnpacker{ diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index 5803d2089..398dd5204 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -74,6 +74,7 @@ func newClient(t *testing.T) client.Client { type MockInstalledBundleGetter struct { bundle *controllers.InstalledBundle + err error } func (m *MockInstalledBundleGetter) SetBundle(bundle *controllers.InstalledBundle) { @@ -81,6 +82,9 @@ func (m *MockInstalledBundleGetter) SetBundle(bundle *controllers.InstalledBundl } func (m *MockInstalledBundleGetter) GetInstalledBundle(ctx context.Context, ext *ocv1.ClusterExtension) (*controllers.InstalledBundle, error) { + if m.err != nil { + return m.bundle, m.err + } return m.bundle, nil }