Skip to content

Commit 1af98d0

Browse files
authored
wrap service account not found error (#1698)
1 parent f6b1130 commit 1af98d0

File tree

5 files changed

+87
-2
lines changed

5 files changed

+87
-2
lines changed

internal/authentication/tokengetter.go

+20
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ package authentication
22

33
import (
44
"context"
5+
"fmt"
56
"sync"
67
"time"
78

89
authenticationv1 "k8s.io/api/authentication/v1"
10+
"k8s.io/apimachinery/pkg/api/errors"
911
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1012
"k8s.io/apimachinery/pkg/types"
1113
corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
@@ -19,6 +21,21 @@ type TokenGetter struct {
1921
mu sync.RWMutex
2022
}
2123

24+
type ServiceAccountNotFoundError struct {
25+
ServiceAccountName string
26+
ServiceAccountNamespace string
27+
Err error
28+
}
29+
30+
func (e *ServiceAccountNotFoundError) Unwrap() error {
31+
return e.Err
32+
}
33+
34+
// Error implements the error interface for ServiceAccountNotFoundError.
35+
func (e *ServiceAccountNotFoundError) Error() string {
36+
return fmt.Sprintf("service account \"%s\" not found in namespace \"%s\": unable to authenticate with the Kubernetes cluster.", e.ServiceAccountName, e.ServiceAccountNamespace)
37+
}
38+
2239
type TokenGetterOption func(*TokenGetter)
2340

2441
const (
@@ -86,6 +103,9 @@ func (t *TokenGetter) getToken(ctx context.Context, key types.NamespacedName) (*
86103
Spec: authenticationv1.TokenRequestSpec{ExpirationSeconds: ptr.To(int64(t.expirationDuration / time.Second))},
87104
}, metav1.CreateOptions{})
88105
if err != nil {
106+
if errors.IsNotFound(err) {
107+
return nil, &ServiceAccountNotFoundError{ServiceAccountName: key.Name, ServiceAccountNamespace: key.Namespace}
108+
}
89109
return nil, err
90110
}
91111
return &req.Status, nil

internal/authentication/tokengetter_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,15 @@ func TestTokenGetterGet(t *testing.T) {
7272
"test-namespace-3", "test-token-3", "failed to get token"},
7373
{"Testing error when getting token from fake client", "test-service-account-4",
7474
"test-namespace-4", "error when fetching token", "error when fetching token"},
75+
{"Testing service account not found", "missing-sa",
76+
"test-namespace-5", "", "service account \"missing-sa\" not found in namespace \"test-namespace-5\": unable to authenticate with the Kubernetes cluster."},
7577
}
7678

7779
for _, tc := range tests {
7880
got, err := tg.Get(context.Background(), types.NamespacedName{Namespace: tc.namespace, Name: tc.serviceAccountName})
7981
if err != nil {
8082
t.Logf("%s: expected: %v, got: %v", tc.testName, tc.want, err)
81-
assert.EqualError(t, err, tc.errorMsg)
83+
assert.EqualError(t, err, tc.errorMsg, "Error message should match expected output")
8284
} else {
8385
t.Logf("%s: expected: %v, got: %v", tc.testName, tc.want, got)
8486
assert.Equal(t, tc.want, got, tc.errorMsg)

internal/controllers/clusterextension_controller.go

+7
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import (
5050

5151
ocv1 "github.com/operator-framework/operator-controller/api/v1"
5252
catalogd "github.com/operator-framework/operator-controller/catalogd/api/v1"
53+
"github.com/operator-framework/operator-controller/internal/authentication"
5354
"github.com/operator-framework/operator-controller/internal/bundleutil"
5455
"github.com/operator-framework/operator-controller/internal/conditionsets"
5556
"github.com/operator-framework/operator-controller/internal/contentmanager"
@@ -206,6 +207,12 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.Cl
206207
installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, ext)
207208
if err != nil {
208209
setInstallStatus(ext, nil)
210+
var saerr *authentication.ServiceAccountNotFoundError
211+
if errors.As(err, &saerr) {
212+
setInstalledStatusConditionUnknown(ext, saerr.Error())
213+
setStatusProgressing(ext, errors.New("installation cannot proceed due to missing ServiceAccount"))
214+
return ctrl.Result{}, err
215+
}
209216
setInstalledStatusConditionUnknown(ext, err.Error())
210217
setStatusProgressing(ext, errors.New("retrying to get installed bundle"))
211218
return ctrl.Result{}, err

internal/controllers/clusterextension_controller_test.go

+55
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/operator-framework/operator-registry/alpha/declcfg"
2929

3030
ocv1 "github.com/operator-framework/operator-controller/api/v1"
31+
"github.com/operator-framework/operator-controller/internal/authentication"
3132
"github.com/operator-framework/operator-controller/internal/conditionsets"
3233
"github.com/operator-framework/operator-controller/internal/controllers"
3334
"github.com/operator-framework/operator-controller/internal/finalizers"
@@ -332,6 +333,60 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T)
332333
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
333334
}
334335

336+
func TestClusterExtensionServiceAccountNotFound(t *testing.T) {
337+
cl, reconciler := newClientAndReconciler(t)
338+
reconciler.InstalledBundleGetter = &MockInstalledBundleGetter{
339+
err: &authentication.ServiceAccountNotFoundError{
340+
ServiceAccountName: "missing-sa",
341+
ServiceAccountNamespace: "default",
342+
}}
343+
344+
ctx := context.Background()
345+
extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
346+
347+
t.Log("Given a cluster extension with a missing service account")
348+
clusterExtension := &ocv1.ClusterExtension{
349+
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
350+
Spec: ocv1.ClusterExtensionSpec{
351+
Source: ocv1.SourceConfig{
352+
SourceType: "Catalog",
353+
Catalog: &ocv1.CatalogSource{
354+
PackageName: "test-package",
355+
},
356+
},
357+
Namespace: "default",
358+
ServiceAccount: ocv1.ServiceAccountReference{
359+
Name: "missing-sa",
360+
},
361+
},
362+
}
363+
364+
require.NoError(t, cl.Create(ctx, clusterExtension))
365+
366+
t.Log("When reconciling the cluster extension")
367+
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
368+
369+
require.Equal(t, ctrl.Result{}, res)
370+
require.Error(t, err)
371+
require.IsType(t, &authentication.ServiceAccountNotFoundError{}, err)
372+
t.Log("By fetching updated cluster extension after reconcile")
373+
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
374+
375+
t.Log("By checking the status conditions")
376+
installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled)
377+
require.NotNil(t, installedCond)
378+
require.Equal(t, metav1.ConditionUnknown, installedCond.Status)
379+
require.Contains(t, installedCond.Message, fmt.Sprintf("service account %q not found in namespace %q: unable to authenticate with the Kubernetes cluster.",
380+
"missing-sa", "default"))
381+
382+
progressingCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing)
383+
require.NotNil(t, progressingCond)
384+
require.Equal(t, metav1.ConditionTrue, progressingCond.Status)
385+
require.Equal(t, ocv1.ReasonRetrying, progressingCond.Reason)
386+
require.Contains(t, progressingCond.Message, "installation cannot proceed due to missing ServiceAccount")
387+
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
388+
}
389+
335390
func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) {
336391
cl, reconciler := newClientAndReconciler(t)
337392
reconciler.Unpacker = &MockUnpacker{

internal/controllers/suite_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,15 @@ func newClient(t *testing.T) client.Client {
7474

7575
type MockInstalledBundleGetter struct {
7676
bundle *controllers.InstalledBundle
77+
err error
7778
}
7879

7980
func (m *MockInstalledBundleGetter) SetBundle(bundle *controllers.InstalledBundle) {
8081
m.bundle = bundle
8182
}
8283

8384
func (m *MockInstalledBundleGetter) GetInstalledBundle(ctx context.Context, ext *ocv1.ClusterExtension) (*controllers.InstalledBundle, error) {
84-
return m.bundle, nil
85+
return m.bundle, m.err
8586
}
8687

8788
var _ controllers.Applier = (*MockApplier)(nil)

0 commit comments

Comments
 (0)