Merge pull request #73595 from liggitt/authn-message

Preserve authentication webhook error message, deduplicate aggregated errors when printing

staging/src/k8s.io/apimachinery/pkg/util/errors/BUILD

@@ -20,6 +20,7 @@ go_library(
     ],
     importmap = "k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/errors",
     importpath = "k8s.io/apimachinery/pkg/util/errors",
+    deps = ["//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library"],
 )
 
 filegroup(

staging/src/k8s.io/apimachinery/pkg/util/errors/errors.go

@@ -19,6 +19,8 @@ package errors
 import (
 	"errors"
 	"fmt"
+
+	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 // MessageCountMap contains occurrence for each error message.
@@ -67,12 +69,38 @@ func (agg aggregate) Error() string {
 	if len(agg) == 1 {
 		return agg[0].Error()
 	}
-	result := fmt.Sprintf("[%s", agg[0].Error())
-	for i := 1; i < len(agg); i++ {
-		result += fmt.Sprintf(", %s", agg[i].Error())
+	seenerrs := sets.NewString()
+	result := ""
+	agg.visit(func(err error) {
+		msg := err.Error()
+		if seenerrs.Has(msg) {
+			return
+		}
+		seenerrs.Insert(msg)
+		if len(seenerrs) > 1 {
+			result += ", "
+		}
+		result += msg
+	})
+	if len(seenerrs) == 1 {
+		return result
+	}
+	return "[" + result + "]"
+}
+
+func (agg aggregate) visit(f func(err error)) {
+	for _, err := range agg {
+		switch err := err.(type) {
+		case aggregate:
+			err.visit(f)
+		case Aggregate:
+			for _, nestedErr := range err.Errors() {
+				f(nestedErr)
+			}
+		default:
+			f(err)
+		}
 	}
-	result += "]"
-	return result
 }
 
 // Errors is part of the Aggregate interface.

staging/src/k8s.io/apimachinery/pkg/util/errors/errors_test.go

@@ -147,6 +147,70 @@ func TestPluralAggregate(t *testing.T) {
 	}
 }
 
+func TestDedupeAggregate(t *testing.T) {
+	var slice []error = []error{fmt.Errorf("abc"), fmt.Errorf("abc")}
+	var agg Aggregate
+
+	agg = NewAggregate(slice)
+	if agg == nil {
+		t.Errorf("expected non-nil")
+	}
+	if s := agg.Error(); s != "abc" {
+		t.Errorf("expected 'abc', got %q", s)
+	}
+	if s := agg.Errors(); len(s) != 2 {
+		t.Errorf("expected two-elements slice, got %#v", s)
+	}
+}
+
+func TestDedupePluralAggregate(t *testing.T) {
+	var slice []error = []error{fmt.Errorf("abc"), fmt.Errorf("abc"), fmt.Errorf("123")}
+	var agg Aggregate
+
+	agg = NewAggregate(slice)
+	if agg == nil {
+		t.Errorf("expected non-nil")
+	}
+	if s := agg.Error(); s != "[abc, 123]" {
+		t.Errorf("expected '[abc, 123]', got %q", s)
+	}
+	if s := agg.Errors(); len(s) != 3 {
+		t.Errorf("expected three-elements slice, got %#v", s)
+	}
+}
+
+func TestFlattenAndDedupeAggregate(t *testing.T) {
+	var slice []error = []error{fmt.Errorf("abc"), fmt.Errorf("abc"), NewAggregate([]error{fmt.Errorf("abc")})}
+	var agg Aggregate
+
+	agg = NewAggregate(slice)
+	if agg == nil {
+		t.Errorf("expected non-nil")
+	}
+	if s := agg.Error(); s != "abc" {
+		t.Errorf("expected 'abc', got %q", s)
+	}
+	if s := agg.Errors(); len(s) != 3 {
+		t.Errorf("expected three-elements slice, got %#v", s)
+	}
+}
+
+func TestFlattenAggregate(t *testing.T) {
+	var slice []error = []error{fmt.Errorf("abc"), fmt.Errorf("abc"), NewAggregate([]error{fmt.Errorf("abc"), fmt.Errorf("def"), NewAggregate([]error{fmt.Errorf("def"), fmt.Errorf("ghi")})})}
+	var agg Aggregate
+
+	agg = NewAggregate(slice)
+	if agg == nil {
+		t.Errorf("expected non-nil")
+	}
+	if s := agg.Error(); s != "[abc, def, ghi]" {
+		t.Errorf("expected '[abc, def, ghi]', got %q", s)
+	}
+	if s := agg.Errors(); len(s) != 3 {
+		t.Errorf("expected three-elements slice, got %#v", s)
+	}
+}
+
 func TestFilterOut(t *testing.T) {
 	testCases := []struct {
 		err      error

staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/webhook.go

@@ -19,6 +19,7 @@ package webhook
 
 import (
 	"context"
+	"errors"
 	"time"
 
 	authentication "k8s.io/api/authentication/v1beta1"
@@ -120,7 +121,11 @@ func (w *WebhookTokenAuthenticator) AuthenticateToken(ctx context.Context, token
 
 	r.Status = result.Status
 	if !r.Status.Authenticated {
-		return nil, false, nil
+		var err error
+		if len(r.Status.Error) != 0 {
+			err = errors.New(r.Status.Error)
+		}
+		return nil, false, err
 	}
 
 	var extra map[string][]string