public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

IAM: Improve team binding hooks error handling and validation (#113393)

Browse Source 
small team hook fixes
This commit is contained in:
Jo
2025-11-04 11:54:38 +01:00
committed by GitHub
parent b4312a220f
commit 769787ea40
2 changed files with 235 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/registry/apis/iam/team_binding_hooks.go
+18 -3
View File
@@ -86,6 +86,7 @@ func (b *IdentityAccessManagementAPIBuilder) AfterTeamBindingCreate(obj runtime.
"name", tb.Name,
"subject", tb.Spec.Subject.Name,
"teamRef", tb.Spec.TeamRef.Name,
"permission", tb.Spec.Permission,
"err", err,
)
status = "failure"
@@ -117,6 +118,7 @@ func (b *IdentityAccessManagementAPIBuilder) AfterTeamBindingCreate(obj runtime.
"name", tb.Name,
"subject", tb.Spec.Subject.Name,
"teamRef", tb.Spec.TeamRef.Name,
"permission", tb.Spec.Permission,
)
} else {
// Record successful tuple write
@@ -143,6 +145,20 @@ func (b *IdentityAccessManagementAPIBuilder) BeginTeamBindingUpdate(ctx context.
return nil, nil
}
if oldTB.Spec.Subject.Name == newTB.Spec.Subject.Name && oldTB.Spec.TeamRef.Name == newTB.Spec.TeamRef.Name && oldTB.Spec.Permission == newTB.Spec.Permission {
return nil, nil // No changes to the team binding
}
if newTB.Spec.Subject.Name == "" || newTB.Spec.TeamRef.Name == "" {
b.logger.Error("invalid team binding",
"namespace", newTB.Namespace,
"name", newTB.Name,
"subject", newTB.Spec.Subject.Name,
"teamRef", newTB.Spec.TeamRef.Name,
)
return nil, nil
}
// Convert old team binding to tuple for deletion
var oldTuple *v1.TupleKey
var oldErr error
@@ -154,6 +170,7 @@ func (b *IdentityAccessManagementAPIBuilder) BeginTeamBindingUpdate(ctx context.
"name", oldTB.Name,
"err", oldErr,
)
return nil, nil
}
}
@@ -168,18 +185,16 @@ func (b *IdentityAccessManagementAPIBuilder) BeginTeamBindingUpdate(ctx context.
"name", newTB.Name,
"err", newErr,
)
return nil, nil
}
}
// Return a finish function that performs the zanzana write only on success
return func(ctx context.Context, success bool) {
if !success {
// Update failed, don't write to zanzana
return
}
// Grab a ticket to write to Zanzana
// This limits the amount of concurrent connections to Zanzana
wait := time.Now()
b.zTickets <- true
hooksWaitHistogram.WithLabelValues("teambinding", "update").Observe(time.Since(wait).Seconds())
pkg/registry/apis/iam/team_binding_hooks_test.go
+217
View File
@@ -487,6 +487,223 @@ func TestBeginTeamBindingUpdate(t *testing.T) {
require.NoError(t, err)
require.Nil(t, finishFunc) // Should return nil when zClient is nil
})
t.Run("should handle empty old binding subject name gracefully", func(t *testing.T) {
wg.Add(1)
oldBinding := iamv0.TeamBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "binding-6",
Namespace: "org-6",
},
Spec: iamv0.TeamBindingSpec{
Subject: iamv0.TeamBindingspecSubject{
Name: "", // Empty name - conversion will be skipped
},
TeamRef: iamv0.TeamBindingTeamRef{
Name: "team-1",
},
Permission: iamv0.TeamBindingTeamPermissionMember,
},
}
newBinding := iamv0.TeamBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "binding-6",
Namespace: "org-6",
},
Spec: iamv0.TeamBindingSpec{
Subject: iamv0.TeamBindingspecSubject{
Name: "user-2",
},
TeamRef: iamv0.TeamBindingTeamRef{
Name: "team-1",
},
Permission: iamv0.TeamBindingTeamPermissionMember,
},
}
testEmptyOldBinding := func(ctx context.Context, req *v1.WriteRequest) error {
defer wg.Done()
require.NotNil(t, req)
require.Equal(t, "org-6", req.Namespace)
// Should not delete old binding (it was skipped due to empty name)
require.Nil(t, req.Deletes)
// Should write new binding
require.NotNil(t, req.Writes)
require.Len(t, req.Writes.TupleKeys, 1)
require.Equal(
t,
req.Writes.TupleKeys[0],
&v1.TupleKey{User: "user:user-2", Relation: "member", Object: "team:team-1"},
)
return nil
}
b.zClient = &FakeZanzanaClient{writeCallback: testEmptyOldBinding}
finishFunc, err := b.BeginTeamBindingUpdate(context.Background(), &newBinding, &oldBinding, nil)
require.NoError(t, err)
require.NotNil(t, finishFunc) // Should still return finish function
finishFunc(context.Background(), true)
wg.Wait()
})
t.Run("should return nil finish func when bindings are identical", func(t *testing.T) {
oldBinding := iamv0.TeamBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "binding-7",
Namespace: "org-7",
},
Spec: iamv0.TeamBindingSpec{
Subject: iamv0.TeamBindingspecSubject{
Name: "user-1",
},
TeamRef: iamv0.TeamBindingTeamRef{
Name: "team-1",
},
Permission: iamv0.TeamBindingTeamPermissionMember,
},
}
newBinding := iamv0.TeamBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "binding-7",
Namespace: "org-7",
},
Spec: iamv0.TeamBindingSpec{
Subject: iamv0.TeamBindingspecSubject{
Name: "user-1",
},
TeamRef: iamv0.TeamBindingTeamRef{
Name: "team-1",
},
Permission: iamv0.TeamBindingTeamPermissionMember,
},
}
writeCalled := false
testNoWriteOnNoChange := func(ctx context.Context, req *v1.WriteRequest) error {
writeCalled = true
require.Fail(t, "Write should not be called when bindings are identical")
return nil
}
b.zClient = &FakeZanzanaClient{writeCallback: testNoWriteOnNoChange}
finishFunc, err := b.BeginTeamBindingUpdate(context.Background(), &newBinding, &oldBinding, nil)
require.NoError(t, err)
require.Nil(t, finishFunc) // Should return nil when bindings are identical
// Verify write was never called
time.Sleep(100 * time.Millisecond)
require.False(t, writeCalled, "Write callback should not be called when bindings are identical")
})
t.Run("should return nil finish func when new binding has empty subject name", func(t *testing.T) {
oldBinding := iamv0.TeamBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "binding-8",
Namespace: "org-8",
},
Spec: iamv0.TeamBindingSpec{
Subject: iamv0.TeamBindingspecSubject{
Name: "user-1",
},
TeamRef: iamv0.TeamBindingTeamRef{
Name: "team-1",
},
Permission: iamv0.TeamBindingTeamPermissionMember,
},
}
newBinding := iamv0.TeamBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "binding-8",
Namespace: "org-8",
},
Spec: iamv0.TeamBindingSpec{
Subject: iamv0.TeamBindingspecSubject{
Name: "", // Empty name - should cause early return
},
TeamRef: iamv0.TeamBindingTeamRef{
Name: "team-1",
},
Permission: iamv0.TeamBindingTeamPermissionMember,
},
}
writeCalled := false
testNoWriteOnInvalidBinding := func(ctx context.Context, req *v1.WriteRequest) error {
writeCalled = true
require.Fail(t, "Write should not be called when new binding has empty subject name")
return nil
}
b.zClient = &FakeZanzanaClient{writeCallback: testNoWriteOnInvalidBinding}
finishFunc, err := b.BeginTeamBindingUpdate(context.Background(), &newBinding, &oldBinding, nil)
require.NoError(t, err)
require.Nil(t, finishFunc) // Should return nil when new binding has empty subject name
// Verify write was never called
time.Sleep(100 * time.Millisecond)
require.False(t, writeCalled, "Write callback should not be called when new binding has empty subject name")
})
t.Run("should return nil finish func when new binding has empty team ref name", func(t *testing.T) {
oldBinding := iamv0.TeamBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "binding-9",
Namespace: "org-9",
},
Spec: iamv0.TeamBindingSpec{
Subject: iamv0.TeamBindingspecSubject{
Name: "user-1",
},
TeamRef: iamv0.TeamBindingTeamRef{
Name: "team-1",
},
Permission: iamv0.TeamBindingTeamPermissionMember,
},
}
newBinding := iamv0.TeamBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "binding-9",
Namespace: "org-9",
},
Spec: iamv0.TeamBindingSpec{
Subject: iamv0.TeamBindingspecSubject{
Name: "user-2",
},
TeamRef: iamv0.TeamBindingTeamRef{
Name: "", // Empty name - should cause early return
},
Permission: iamv0.TeamBindingTeamPermissionMember,
},
}
writeCalled := false
testNoWriteOnInvalidBinding := func(ctx context.Context, req *v1.WriteRequest) error {
writeCalled = true
require.Fail(t, "Write should not be called when new binding has empty team ref name")
return nil
}
b.zClient = &FakeZanzanaClient{writeCallback: testNoWriteOnInvalidBinding}
finishFunc, err := b.BeginTeamBindingUpdate(context.Background(), &newBinding, &oldBinding, nil)
require.NoError(t, err)
require.Nil(t, finishFunc) // Should return nil when new binding has empty team ref name
// Verify write was never called
time.Sleep(100 * time.Millisecond)
require.False(t, writeCalled, "Write callback should not be called when new binding has empty team ref name")
})
}
func TestAfterTeamBindingDelete(t *testing.T) {