(iceberg-go) branch main updated: fix(rest): surface failures refreshing expired credentials (#1579)

[atserakhau]

This is an automated email from the ASF dual-hosted git repository.

laskoviymishka pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git


The following commit(s) were added to refs/heads/main by this push:
     new c2e7ec00 fix(rest): surface failures refreshing expired credentials 
(#1579)
c2e7ec00 is described below

commit c2e7ec004d09a3fccdcb9b61a221dc1f8d0f3004
Author: Minh Vu <vuhoangmin...@gmail.com>
AuthorDate: Wed Aug 5 18:57:26 2026 +0200

    fix(rest): surface failures refreshing expired credentials (#1579)
    
    * fix(rest): surface failures refreshing expired credentials
    
    Signed-off-by: Minh Vu <vuhoangmin...@gmail.com>
    
    * fix(rest): distinguish initial and refreshed IO failures
    
    Signed-off-by: Minh Vu <vuhoangmin...@gmail.com>
    
    * test(rest): preserve cached IO before credential expiry
    
    * fix(rest): fail expired credential refreshes
    
    ---------
    
    Signed-off-by: Minh Vu <vuhoangmin...@gmail.com>
---
 catalog/rest/vended_creds.go      |  8 ++++----
 catalog/rest/vended_creds_test.go | 29 +++++++++++++++++++++++++----
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/catalog/rest/vended_creds.go b/catalog/rest/vended_creds.go
index 44ec4882..66b8b9be 100644
--- a/catalog/rest/vended_creds.go
+++ b/catalog/rest/vended_creds.go
@@ -136,7 +136,7 @@ func (v *vendedCredentialRefresher) loadFS(ctx 
context.Context) (iceio.IO, error
        default:
                freshCreds, err := v.fetchCreds(ctx, v.identifier)
                if err != nil {
-                       return v.cachedIO, nil
+                       return nil, fmt.Errorf("refresh vended credentials for 
%s: %w", v.location, err)
                }
 
                config = maps.Clone(v.props)
@@ -145,11 +145,11 @@ func (v *vendedCredentialRefresher) loadFS(ctx 
context.Context) (iceio.IO, error
 
        newIO, err := iceio.LoadFS(ctx, config, v.location)
        if err != nil {
-               if v.cachedIO != nil {
-                       return v.cachedIO, nil
+               if v.cachedIO == nil {
+                       return nil, err
                }
 
-               return nil, err
+               return nil, fmt.Errorf("load filesystem with refreshed 
credentials for %s: %w", v.location, err)
        }
 
        v.cachedIO = newIO
diff --git a/catalog/rest/vended_creds_test.go 
b/catalog/rest/vended_creds_test.go
index 059a149e..f9a65c62 100644
--- a/catalog/rest/vended_creds_test.go
+++ b/catalog/rest/vended_creds_test.go
@@ -46,12 +46,13 @@ func newTestRefresher(fetchCreds func(ctx context.Context, 
ident []string) (iceb
 func TestVendedCredsCachedIOReturnedWhenNotExpired(t *testing.T) {
        t.Parallel()
 
+       refreshErr := errors.New("refresh unavailable")
        var callCount atomic.Int32
 
        r := newTestRefresher(func(ctx context.Context, ident []string) 
(iceberg.Properties, error) {
                callCount.Add(1)
 
-               return iceberg.Properties{}, nil
+               return nil, refreshErr
        })
 
        // Seed cached IO and set expiry in the future.
@@ -153,7 +154,7 @@ func TestVendedCredsConcurrentAccess(t *testing.T) {
        assert.NotNil(t, r.cachedIO, "cachedIO should be set after initial 
load")
 }
 
-func TestVendedCredsGracefulDegradation(t *testing.T) {
+func TestVendedCredsReturnsRefreshFailureForExpiredCredentials(t *testing.T) {
        t.Parallel()
 
        fetchErr := errors.New("network error")
@@ -170,8 +171,27 @@ func TestVendedCredsGracefulDegradation(t *testing.T) {
        r.expiresAt = now.Add(-time.Second)
 
        got, err := r.loadFS(context.Background())
-       require.NoError(t, err, "should not return error when cached IO exists 
and refresh fails")
-       assert.Equal(t, existingIO, got, "should return cached IO on refresh 
failure")
+       require.ErrorIs(t, err, fetchErr)
+       require.ErrorContains(t, err, "refresh vended credentials for 
file:///tmp/test")
+       assert.Nil(t, got)
+}
+
+func TestVendedCredsReturnsLoadFailureForExpiredCredentials(t *testing.T) {
+       t.Parallel()
+
+       now := time.Now()
+       r := newTestRefresher(func(ctx context.Context, ident []string) 
(iceberg.Properties, error) {
+               return iceberg.Properties{}, nil
+       })
+       r.nowFunc = func() time.Time { return now }
+       r.location = "notascheme://bucket/path"
+       r.cachedIO = iceio.LocalFS{}
+       r.expiresAt = now.Add(-time.Second)
+
+       got, err := r.loadFS(context.Background())
+       require.Error(t, err)
+       require.ErrorContains(t, err, "load filesystem with refreshed 
credentials for notascheme://bucket/path")
+       assert.Nil(t, got)
 }
 
 func TestVendedCredsErrorWhenInitialLoadFails(t *testing.T) {
@@ -185,6 +205,7 @@ func TestVendedCredsErrorWhenInitialLoadFails(t *testing.T) 
{
 
        got, err := r.loadFS(context.Background())
        require.Error(t, err)
+       assert.NotContains(t, err.Error(), "refreshed credentials")
        assert.Nil(t, got)
 }