Merge pull request #136700 from Jefftree/cra-fix

simplify cluster role aggregation and remove update path
This commit is contained in:
Kubernetes Prow Robot
2026-03-12 00:45:35 +05:30
committed by GitHub
2 changed files with 48 additions and 87 deletions

View File

@@ -129,15 +129,7 @@ func (c *ClusterRoleAggregationController) syncClusterRole(ctx context.Context,
return nil
}
err = c.applyClusterRoles(ctx, sharedClusterRole.Name, newPolicyRules)
if errors.IsUnsupportedMediaType(err) { // TODO: Remove this fallback at least one release after ServerSideApply GA
// When Server Side Apply is not enabled, fallback to Update. This is required when running
// 1.21 since api-server can be 1.20 during the upgrade/downgrade.
// Since Server Side Apply is enabled by default in Beta, this fallback only kicks in
// if the feature has been disabled using its feature flag.
err = c.updateClusterRoles(ctx, sharedClusterRole, newPolicyRules)
}
return err
return c.applyClusterRoles(ctx, sharedClusterRole.Name, newPolicyRules)
}
func (c *ClusterRoleAggregationController) applyClusterRoles(ctx context.Context, name string, newPolicyRules []rbacv1.PolicyRule) error {
@@ -149,16 +141,6 @@ func (c *ClusterRoleAggregationController) applyClusterRoles(ctx context.Context
return err
}
func (c *ClusterRoleAggregationController) updateClusterRoles(ctx context.Context, sharedClusterRole *rbacv1.ClusterRole, newPolicyRules []rbacv1.PolicyRule) error {
clusterRole := sharedClusterRole.DeepCopy()
clusterRole.Rules = nil
for _, rule := range newPolicyRules {
clusterRole.Rules = append(clusterRole.Rules, *rule.DeepCopy())
}
_, err := c.clusterRoleClient.ClusterRoles().Update(ctx, clusterRole, metav1.UpdateOptions{})
return err
}
func toApplyPolicyRules(rules []rbacv1.PolicyRule) []*rbacv1ac.PolicyRuleApplyConfiguration {
var result []*rbacv1ac.PolicyRuleApplyConfiguration
for _, rule := range rules {

View File

@@ -23,7 +23,6 @@ import (
"github.com/google/go-cmp/cmp"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/yaml"
@@ -159,78 +158,58 @@ func TestSyncClusterRole(t *testing.T) {
expectedClusterRoleApply: nil,
}}
for _, serverSideApplyEnabled := range []bool{true, false} {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
indexer := cache.NewIndexer(controller.KeyFunc, cache.Indexers{})
objs := []runtime.Object{}
for _, obj := range test.startingClusterRoles {
objs = append(objs, obj)
indexer.Add(obj)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
indexer := cache.NewIndexer(controller.KeyFunc, cache.Indexers{})
objs := []runtime.Object{}
for _, obj := range test.startingClusterRoles {
objs = append(objs, obj)
if err := indexer.Add(obj); err != nil {
t.Fatalf("failed to add object to indexer: %v", err)
}
fakeClient := fakeclient.NewSimpleClientset(objs...)
}
fakeClient := fakeclient.NewClientset(objs...)
// The default reactor doesn't support apply, so we need our own (trivial) reactor
fakeClient.PrependReactor("patch", "clusterroles", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) {
if serverSideApplyEnabled == false {
// UnsupportedMediaType
return true, nil, errors.NewGenericServerResponse(415, "get", action.GetResource().GroupResource(), "test", "Apply not supported", 0, true)
}
return true, nil, nil // clusterroleaggregator drops returned objects so no point in constructing them
})
c := ClusterRoleAggregationController{
clusterRoleClient: fakeClient.RbacV1(),
clusterRoleLister: rbaclisters.NewClusterRoleLister(indexer),
}
err := c.syncClusterRole(context.TODO(), test.clusterRoleToSync)
if err != nil {
t.Fatal(err)
}
// The default reactor doesn't support apply, so we need our own (trivial) reactor
fakeClient.PrependReactor("patch", "clusterroles", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, nil // clusterroleaggregator drops returned objects so no point in constructing them
})
c := ClusterRoleAggregationController{
clusterRoleClient: fakeClient.RbacV1(),
clusterRoleLister: rbaclisters.NewClusterRoleLister(indexer),
}
err := c.syncClusterRole(context.TODO(), test.clusterRoleToSync)
if err != nil {
t.Fatal(err)
}
if test.expectedClusterRoleApply == nil {
if len(fakeClient.Actions()) != 0 {
t.Fatalf("unexpected actions %#v", fakeClient.Actions())
}
return
}
expectedActions := 1
if !serverSideApplyEnabled {
expectedActions = 2
}
if len(fakeClient.Actions()) != expectedActions {
if test.expectedClusterRoleApply == nil {
if len(fakeClient.Actions()) != 0 {
t.Fatalf("unexpected actions %#v", fakeClient.Actions())
}
return
}
action := fakeClient.Actions()[0]
if !action.Matches("patch", "clusterroles") {
t.Fatalf("unexpected action %#v", action)
}
applyAction, ok := action.(clienttesting.PatchAction)
if !ok {
t.Fatalf("unexpected action %#v", action)
}
ac := &rbacv1ac.ClusterRoleApplyConfiguration{}
if err := yaml.Unmarshal(applyAction.GetPatch(), ac); err != nil {
t.Fatalf("error unmarshalling apply request: %v", err)
}
if !equality.Semantic.DeepEqual(ac, test.expectedClusterRoleApply) {
t.Fatalf("%v", cmp.Diff(test.expectedClusterRoleApply, ac))
}
if expectedActions == 2 {
action := fakeClient.Actions()[1]
if !action.Matches("update", "clusterroles") {
t.Fatalf("unexpected action %#v", action)
}
updateAction, ok := action.(clienttesting.UpdateAction)
if !ok {
t.Fatalf("unexpected action %#v", action)
}
if !equality.Semantic.DeepEqual(updateAction.GetObject().(*rbacv1.ClusterRole), test.expectedClusterRole) {
t.Fatalf("%v", cmp.Diff(test.expectedClusterRole, updateAction.GetObject().(*rbacv1.ClusterRole)))
}
}
})
}
expectedActions := 1
if len(fakeClient.Actions()) != expectedActions {
t.Fatalf("unexpected actions %#v", fakeClient.Actions())
}
action := fakeClient.Actions()[0]
if !action.Matches("patch", "clusterroles") {
t.Fatalf("unexpected action %#v", action)
}
applyAction, ok := action.(clienttesting.PatchAction)
if !ok {
t.Fatalf("unexpected action %#v", action)
}
ac := &rbacv1ac.ClusterRoleApplyConfiguration{}
if err := yaml.Unmarshal(applyAction.GetPatch(), ac); err != nil {
t.Fatalf("error unmarshalling apply request: %v", err)
}
if !equality.Semantic.DeepEqual(ac, test.expectedClusterRoleApply) {
t.Fatalf("%v", cmp.Diff(test.expectedClusterRoleApply, ac))
}
})
}
}