Skip to content

Commit 4e958f0

Browse files
authored
fix: access revocation maxcompute (#327)
1 parent c3db7ca commit 4e958f0

2 files changed

Lines changed: 54 additions & 0 deletions

File tree

plugins/providers/maxcompute/external.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,13 @@ func (p *provider) revokeTableRolesFromMember(ctx context.Context, pc *domain.Pr
571571
roleQuery := strings.Join(slices.GenericsStandardizeSlice(roles), ", ")
572572
var query = fmt.Sprintf("REVOKE %s ON TABLE `%s`.`%s`.`%s` FROM USER `%s`", roleQuery, project, schema, table, ramAccountId)
573573
if _, err = odpsExecuteQueryOnSecurityManager(ctx, invoker, query); err != nil {
574+
// If the table no longer exists in MaxCompute there is nothing left to revoke,
575+
// so treat it as a successful no-op. Otherwise a dropped table would permanently
576+
// block stale-grant cleanup (e.g. auto-access-restoration). This mirrors the
577+
// NoSuchObject handling in revokeProjectRolesFromMember.
578+
if isTableNotFoundErr(err) {
579+
return nil
580+
}
574581
var restErr restclient.HttpError
575582
if errors.As(err, &restErr) && restErr.ErrorMessage != nil {
576583
return fmt.Errorf("fail to revoke table role from '%s.%s.%s': %s", project, schema, table, restErr.ErrorMessage.Message)
@@ -580,6 +587,16 @@ func (p *provider) revokeTableRolesFromMember(ctx context.Context, pc *domain.Pr
580587
return nil
581588
}
582589

590+
// isTableNotFoundErr reports whether err is a MaxCompute "table not found" error
591+
// (ODPS-0130131). When revoking a role on a table that has already been dropped,
592+
// the grant is effectively gone, so callers can treat this as a no-op.
593+
func isTableNotFoundErr(err error) bool {
594+
if err == nil {
595+
return false
596+
}
597+
return strings.Contains(err.Error(), "ODPS-0130131")
598+
}
599+
583600
// ---------------------------------------------------------------------------------------------------------------------
584601
// External Client
585602
// ---------------------------------------------------------------------------------------------------------------------

plugins/providers/maxcompute/external_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package maxcompute
33
import (
44
"context"
55
"errors"
6+
"fmt"
67
"reflect"
78
"testing"
89

@@ -64,6 +65,42 @@ func TestODPSShouldRetry(t *testing.T) {
6465
}
6566
}
6667

68+
func TestIsTableNotFoundErr(t *testing.T) {
69+
tests := []struct {
70+
name string
71+
err error
72+
want bool
73+
}{
74+
{
75+
name: "nil error",
76+
err: nil,
77+
want: false,
78+
},
79+
{
80+
name: "odps table not found error",
81+
err: errors.New("ODPS-0130131:Table not found - 'haryo_poc_playground.a_table' table not found"),
82+
want: true,
83+
},
84+
{
85+
name: "wrapped odps table not found error",
86+
err: fmt.Errorf("fail to revoke: %w", errors.New("ODPS-0130131:Table not found")),
87+
want: true,
88+
},
89+
{
90+
name: "unrelated error",
91+
err: errors.New("read: connection reset by peer"),
92+
want: false,
93+
},
94+
}
95+
for _, tt := range tests {
96+
t.Run(tt.name, func(t *testing.T) {
97+
if got := isTableNotFoundErr(tt.err); got != tt.want {
98+
t.Errorf("isTableNotFoundErr() = %v, want %v", got, tt.want)
99+
}
100+
})
101+
}
102+
}
103+
67104
func TestBatchLoadTablesSkippingFailuresSplitsOnlyFailedBatch(t *testing.T) {
68105
p := &provider{logger: log.NewNoop()}
69106
input := []string{"a", "b", "bad", "c", "d", "e", "f", "g"}

0 commit comments

Comments
 (0)