Merge pull request #71564 from liggitt/reconcile-details

Improve reconcile output to explain what changes are being made

pkg/kubectl/cmd/auth/BUILD

@@ -25,6 +25,7 @@ go_library(
         "//staging/src/k8s.io/api/rbac/v1alpha1:go_default_library",
         "//staging/src/k8s.io/api/rbac/v1beta1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/cli-runtime/pkg/genericclioptions:go_default_library",
         "//staging/src/k8s.io/cli-runtime/pkg/genericclioptions/printers:go_default_library",

pkg/kubectl/cmd/auth/reconcile.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 
 	"github.com/spf13/cobra"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog"
 
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -205,7 +206,7 @@ func (o *ReconcileOptions) RunReconcile() error {
 			if err != nil {
 				return err
 			}
-			o.PrintObject(result.Role.GetObject(), o.Out)
+			o.printResults(result.Role.GetObject(), nil, nil, result.MissingRules, result.ExtraRules, result.Operation, result.Protected)
 
 		case *rbacv1.ClusterRole:
 			reconcileOptions := reconciliation.ReconcileRoleOptions{
@@ -220,7 +221,7 @@ func (o *ReconcileOptions) RunReconcile() error {
 			if err != nil {
 				return err
 			}
-			o.PrintObject(result.Role.GetObject(), o.Out)
+			o.printResults(result.Role.GetObject(), nil, nil, result.MissingRules, result.ExtraRules, result.Operation, result.Protected)
 
 		case *rbacv1.RoleBinding:
 			reconcileOptions := reconciliation.ReconcileRoleBindingOptions{
@@ -236,7 +237,7 @@ func (o *ReconcileOptions) RunReconcile() error {
 			if err != nil {
 				return err
 			}
-			o.PrintObject(result.RoleBinding.GetObject(), o.Out)
+			o.printResults(result.RoleBinding.GetObject(), result.MissingSubjects, result.ExtraSubjects, nil, nil, result.Operation, result.Protected)
 
 		case *rbacv1.ClusterRoleBinding:
 			reconcileOptions := reconciliation.ReconcileRoleBindingOptions{
@@ -251,7 +252,7 @@ func (o *ReconcileOptions) RunReconcile() error {
 			if err != nil {
 				return err
 			}
-			o.PrintObject(result.RoleBinding.GetObject(), o.Out)
+			o.printResults(result.RoleBinding.GetObject(), result.MissingSubjects, result.ExtraSubjects, nil, nil, result.Operation, result.Protected)
 
 		case *rbacv1beta1.Role,
 			*rbacv1beta1.RoleBinding,
@@ -271,3 +272,56 @@ func (o *ReconcileOptions) RunReconcile() error {
 		return nil
 	})
 }
+
+func (o *ReconcileOptions) printResults(object runtime.Object,
+	missingSubjects, extraSubjects []rbacv1.Subject,
+	missingRules, extraRules []rbacv1.PolicyRule,
+	operation reconciliation.ReconcileOperation,
+	protected bool) {
+
+	o.PrintObject(object, o.Out)
+
+	caveat := ""
+	if protected {
+		caveat = ", but object opted out (rbac.authorization.kubernetes.io/autoupdate: false)"
+	}
+	switch operation {
+	case reconciliation.ReconcileNone:
+		return
+	case reconciliation.ReconcileCreate:
+		fmt.Fprintf(o.ErrOut, "\treconciliation required create%s\n", caveat)
+	case reconciliation.ReconcileUpdate:
+		fmt.Fprintf(o.ErrOut, "\treconciliation required update%s\n", caveat)
+	case reconciliation.ReconcileRecreate:
+		fmt.Fprintf(o.ErrOut, "\treconciliation required recreate%s\n", caveat)
+	}
+
+	if len(missingSubjects) > 0 {
+		fmt.Fprintf(o.ErrOut, "\tmissing subjects added:\n")
+		for _, s := range missingSubjects {
+			fmt.Fprintf(o.ErrOut, "\t\t%+v\n", s)
+		}
+	}
+	if o.RemoveExtraSubjects {
+		if len(extraSubjects) > 0 {
+			fmt.Fprintf(o.ErrOut, "\textra subjects removed:\n")
+			for _, s := range extraSubjects {
+				fmt.Fprintf(o.ErrOut, "\t\t%+v\n", s)
+			}
+		}
+	}
+	if len(missingRules) > 0 {
+		fmt.Fprintf(o.ErrOut, "\tmissing rules added:\n")
+		for _, r := range missingRules {
+			fmt.Fprintf(o.ErrOut, "\t\t%+v\n", r)
+		}
+	}
+	if o.RemoveExtraPermissions {
+		if len(extraRules) > 0 {
+			fmt.Fprintf(o.ErrOut, "\textra rules removed:\n")
+			for _, r := range extraRules {
+				fmt.Fprintf(o.ErrOut, "\t\t%+v\n", r)
+			}
+		}
+	}
+}

pkg/registry/rbac/reconciliation/reconcile_role.go

@@ -194,7 +194,10 @@ func computeReconciledRole(existing, expected RuleOwner, removeExtraPermissions
 	}
 
 	// Compute extra and missing rules
-	_, result.ExtraRules = validation.Covers(expected.GetRules(), existing.GetRules())
+	// Don't compute extra permissions if expected and existing roles are both aggregated
+	if expected.GetAggregationRule() == nil || existing.GetAggregationRule() == nil {
+		_, result.ExtraRules = validation.Covers(expected.GetRules(), existing.GetRules())
+	}
 	_, result.MissingRules = validation.Covers(existing.GetRules(), expected.GetRules())
 
 	switch {