Merge pull request #63762 from liggitt/automated-cherry-pick-of-#63761-upstream-release-1.10

pkg/registry/rbac/reconciliation/reconcile_role.go

@@ -214,6 +214,11 @@ func computeReconciledRole(existing, expected RuleOwner, removeExtraPermissions
 	_, result.MissingAggregationRuleSelectors = aggregationRuleCovers(existing.GetAggregationRule(), expected.GetAggregationRule())
 
 	switch {
+	case expected.GetAggregationRule() == nil && existing.GetAggregationRule() != nil:
+		// we didn't expect this to be an aggregated role at all, remove the existing aggregation
+		result.Role.SetAggregationRule(nil)
+		result.Operation = ReconcileUpdate
+
 	case !removeExtraPermissions && len(result.MissingAggregationRuleSelectors) > 0:
 		// add missing rules in the union case
 		aggregationRule := result.Role.GetAggregationRule()

pkg/registry/rbac/reconciliation/reconcile_role_test.go

@@ -350,6 +350,32 @@ func TestComputeReconciledRoleAggregationRules(t *testing.T) {
 			expectedReconciledRole:       aggregatedRole(aggregationrule([]map[string]string{{"alpha": "bravo"}, {"foo": "bar"}})),
 			expectedReconciliationNeeded: true,
 		},
+		"unexpected aggregation": {
+			// desired role is not aggregated
+			expectedRole: role(rules("pods", "nodes", "secrets"), nil, nil),
+			// existing role is aggregated
+			actualRole:             aggregatedRole(aggregationrule([]map[string]string{{"alpha": "bravo"}})),
+			removeExtraPermissions: false,
+
+			// reconciled role should have desired permissions and not be aggregated
+			expectedReconciledRole:       role(rules("pods", "nodes", "secrets"), nil, nil),
+			expectedReconciliationNeeded: true,
+		},
+		"unexpected aggregation with differing permissions": {
+			// desired role is not aggregated
+			expectedRole: role(rules("pods", "nodes", "secrets"), nil, nil),
+			// existing role is aggregated and has other permissions
+			actualRole: func() *rbac.ClusterRole {
+				r := aggregatedRole(aggregationrule([]map[string]string{{"alpha": "bravo"}}))
+				r.Rules = rules("deployments")
+				return r
+			}(),
+			removeExtraPermissions: false,
+
+			// reconciled role should have aggregation removed, preserve differing permissions, and include desired permissions
+			expectedReconciledRole:       role(rules("deployments", "pods", "nodes", "secrets"), nil, nil),
+			expectedReconciliationNeeded: true,
+		},
 	}
 
 	for k, tc := range tests {

pkg/registry/rbac/rest/storage_rbac.go

@@ -323,6 +323,10 @@ func primeAggregatedClusterRoles(clusterRolesToAggregate map[string]string, clus
 		if err != nil {
 			return err
 		}
+		if existingRole.AggregationRule != nil {
+			// the old role already moved to an aggregated role, so there are no custom rules to migrate at this point
+			return nil
+		}
 		glog.V(1).Infof("migrating %v to %v", existingRole.Name, newName)
 		existingRole.Name = newName
 		existingRole.ResourceVersion = "" // clear this so the object can be created.