operator: invalidate AIStore API client with failed auth token

Signed-off-by: Aaron Wilson <aawilson@nvidia.com>

Author: aaronnw

operator/CHANGELOG.md

@@ -13,6 +13,7 @@ We structure this changelog in accordance with [Keep a Changelog](https://keepac
 ### Changed
 
 - Fixed syncPodTemplate not syncing env var removals when the desired env slice was a prefix of the current, causing rollout loops on config changes such as clearing `authNSecretName`.
+- Invalidate the AIStore API client on authentication failure, not just on failure to acquire initial token. 
 
 ## v2.15.0
 

operator/pkg/services/aisapi.go

@@ -47,14 +47,45 @@ type (
 		mode          string
 		tlsCfg        *tls.Config
 		tokenExpireAt time.Time
+		authFailed    bool
 	}
 )
 
+// IsAuthError returns true if the error is an HTTP 401 or 403 from the AIS API,
+// indicating the token is invalid or has been revoked.
+func IsAuthError(err error) bool {
+	if err == nil {
+		return false
+	}
+	herr, ok := err.(*cmn.ErrHTTP)
+	if !ok {
+		return false
+	}
+	return herr.Status == http.StatusUnauthorized || herr.Status == http.StatusForbidden
+}
+
+// checkAuthErr inspects an error from an AIS API call and marks the client's
+// token as failed if the cluster responded with 401/403. This ensures the next
+// call to GetClient will discard the cached client and fetch a fresh token.
+func (c *AIStoreClient) checkAuthErr(err error) {
+	if IsAuthError(err) {
+		c.authFailed = true
+		logf.FromContext(c.ctx).Info("AIS API returned auth error, token will be refreshed on next reconcile")
+	}
+}
+
 // HasValidBaseParams checks if the client has valid params for the given AIS cluster configuration
 func (c *AIStoreClient) HasValidBaseParams(ctx context.Context, ais *aisv1.AIStore, expectedURL string) bool {
 	if c.params == nil {
 		return false
 	}
+
+	// If a previous API call returned 401/403, force token refresh
+	if c.authFailed {
+		logf.FromContext(ctx).Info("Token previously rejected by AIS (401/403), recreating client")
+		return false
+	}
+
 	// Check if the URL has changed
 	if c.params.URL != expectedURL {
 		return false
@@ -114,35 +145,51 @@ func (c *AIStoreClient) refreshToken(tokenInfo *TokenInfo) {
 }
 
 func (c *AIStoreClient) DecommissionCluster(rmUserData bool) error {
-	return api.DecommissionCluster(*c.params, rmUserData)
+	err := api.DecommissionCluster(*c.params, rmUserData)
+	c.checkAuthErr(err)
+	return err
 }
 
 func (c *AIStoreClient) DecommissionNode(actValue *apc.ActValRmNode) (string, error) {
-	return api.DecommissionNode(*c.params, actValue)
+	xid, err := api.DecommissionNode(*c.params, actValue)
+	c.checkAuthErr(err)
+	return xid, err
 }
 
 func (c *AIStoreClient) GetClusterMap() (smap *meta.Smap, err error) {
-	return api.GetClusterMap(*c.params)
+	smap, err = api.GetClusterMap(*c.params)
+	c.checkAuthErr(err)
+	return
 }
 
 func (c *AIStoreClient) Health(readyToRebalance bool) error {
-	return api.Health(*c.params, readyToRebalance)
+	err := api.Health(*c.params, readyToRebalance)
+	c.checkAuthErr(err)
+	return err
 }
 
 func (c *AIStoreClient) SetClusterConfigUsingMsg(config *cmn.ConfigToSet, transient bool) error {
-	return api.SetClusterConfigUsingMsg(*c.params, config, transient)
+	err := api.SetClusterConfigUsingMsg(*c.params, config, transient)
+	c.checkAuthErr(err)
+	return err
 }
 
 func (c *AIStoreClient) SetPrimaryProxy(newPrimaryID, newPrimaryURL string, force bool) error {
-	return api.SetPrimary(*c.params, newPrimaryID, newPrimaryURL, force)
+	err := api.SetPrimary(*c.params, newPrimaryID, newPrimaryURL, force)
+	c.checkAuthErr(err)
+	return err
 }
 
 func (c *AIStoreClient) ShutdownCluster() error {
-	return api.ShutdownCluster(*c.params)
+	err := api.ShutdownCluster(*c.params)
+	c.checkAuthErr(err)
+	return err
 }
 
 func (c *AIStoreClient) StartMaintenance(actValue *apc.ActValRmNode) (string, error) {
-	return api.StartMaintenance(*c.params, actValue)
+	xid, err := api.StartMaintenance(*c.params, actValue)
+	c.checkAuthErr(err)
+	return xid, err
 }
 
 func NewAIStoreClient(ctx context.Context, url string, tokenInfo *TokenInfo, mode string, tlsCfg *tls.Config) *AIStoreClient {

operator/pkg/services/token_expiration_test.go

@@ -6,9 +6,12 @@ package services
 
 import (
 	"context"
+	"errors"
+	"net/http"
 	"testing"
 	"time"
 
+	"github.com/NVIDIA/aistore/cmn"
 	aisv1 "github.com/ais-operator/api/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -118,3 +121,152 @@ func TestTokenInfoStructure(t *testing.T) {
 		}
 	})
 }
+
+func TestIsAuthError(t *testing.T) {
+	tests := []struct {
+		name     string
+		err      error
+		expected bool
+	}{
+		{
+			name:     "nil error",
+			err:      nil,
+			expected: false,
+		},
+		{
+			name:     "non-HTTP error",
+			err:      errors.New("connection refused"),
+			expected: false,
+		},
+		{
+			name: "401 Unauthorized",
+			err: &cmn.ErrHTTP{
+				Status: http.StatusUnauthorized,
+			},
+			expected: true,
+		},
+		{
+			name: "403 Forbidden",
+			err: &cmn.ErrHTTP{
+				Status: http.StatusForbidden,
+			},
+			expected: true,
+		},
+		{
+			name: "404 Not Found",
+			err: &cmn.ErrHTTP{
+				Status: http.StatusNotFound,
+			},
+			expected: false,
+		},
+		{
+			name: "503 Service Unavailable",
+			err: &cmn.ErrHTTP{
+				Status: http.StatusServiceUnavailable,
+			},
+			expected: false,
+		},
+		{
+			name: "500 Internal Server Error",
+			err: &cmn.ErrHTTP{
+				Status: http.StatusInternalServerError,
+			},
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := IsAuthError(tt.err)
+			if result != tt.expected {
+				t.Errorf("IsAuthError(%v) = %v, want %v", tt.err, result, tt.expected)
+			}
+		})
+	}
+}
+
+func TestAuthFailedInvalidatesClient(t *testing.T) {
+	ctx := context.Background()
+	testURL := "http://test:8080"
+
+	ais := &aisv1.AIStore{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-cluster",
+			Namespace: "default",
+		},
+		Spec: aisv1.AIStoreSpec{},
+	}
+
+	t.Run("client valid before auth failure", func(t *testing.T) {
+		client := &AIStoreClient{
+			ctx:    ctx,
+			params: buildBaseParams(testURL, "", nil),
+			mode:   "",
+		}
+
+		if !client.HasValidBaseParams(ctx, ais, testURL) {
+			t.Error("Expected client to be valid before any auth failure")
+		}
+	})
+
+	t.Run("client invalid after auth failure", func(t *testing.T) {
+		client := &AIStoreClient{
+			ctx:    ctx,
+			params: buildBaseParams(testURL, "", nil),
+			mode:   "",
+		}
+
+		// Simulate receiving a 401 error
+		authErr := &cmn.ErrHTTP{Status: http.StatusUnauthorized}
+		client.checkAuthErr(authErr)
+
+		if client.HasValidBaseParams(ctx, ais, testURL) {
+			t.Error("Expected client to be invalid after 401 error")
+		}
+	})
+
+	t.Run("non-auth error does not invalidate client", func(t *testing.T) {
+		client := &AIStoreClient{
+			ctx:    ctx,
+			params: buildBaseParams(testURL, "", nil),
+			mode:   "",
+		}
+
+		// Simulate receiving a 500 error
+		serverErr := &cmn.ErrHTTP{Status: http.StatusInternalServerError}
+		client.checkAuthErr(serverErr)
+
+		if !client.HasValidBaseParams(ctx, ais, testURL) {
+			t.Error("Expected client to remain valid after non-auth error")
+		}
+	})
+
+	t.Run("nil error does not invalidate client", func(t *testing.T) {
+		client := &AIStoreClient{
+			ctx:    ctx,
+			params: buildBaseParams(testURL, "", nil),
+			mode:   "",
+		}
+
+		client.checkAuthErr(nil)
+
+		if !client.HasValidBaseParams(ctx, ais, testURL) {
+			t.Error("Expected client to remain valid after nil error")
+		}
+	})
+
+	t.Run("403 Forbidden invalidates client", func(t *testing.T) {
+		client := &AIStoreClient{
+			ctx:    ctx,
+			params: buildBaseParams(testURL, "", nil),
+			mode:   "",
+		}
+
+		authErr := &cmn.ErrHTTP{Status: http.StatusForbidden}
+		client.checkAuthErr(authErr)
+
+		if client.HasValidBaseParams(ctx, ais, testURL) {
+			t.Error("Expected client to be invalid after 403 error")
+		}
+	})
+}