Migrate callers to k8s.io/apimachinery/pkg/util/errors.NewAggregate

Signed-off-by: Monis Khan <mok@vmware.com>
This commit is contained in:
Monis Khan 2021-02-05 12:56:05 -05:00
parent 81d4e50f94
commit 05a471fdf9
No known key found for this signature in database
GPG Key ID: 52C90ADA01B269B8
11 changed files with 42 additions and 49 deletions

View File

@ -288,8 +288,7 @@ func TestGetKubeconfig(t *testing.T) {
},
wantError: true,
wantStderr: here.Doc(`
Error: invalid api group suffix: 1 error(s):
- a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
Error: invalid api group suffix: a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
`),
},
{

View File

@ -132,8 +132,7 @@ func TestLoginOIDCCommand(t *testing.T) {
},
wantError: true,
wantStderr: here.Doc(`
Error: invalid concierge parameters: invalid api group suffix: 1 error(s):
- a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
Error: invalid concierge parameters: invalid api group suffix: a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
`),
},
{

View File

@ -142,8 +142,7 @@ func TestLoginStaticCommand(t *testing.T) {
},
wantError: true,
wantStderr: here.Doc(`
Error: invalid concierge parameters: invalid api group suffix: 1 error(s):
- a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
Error: invalid concierge parameters: invalid api group suffix: a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
`),
},
{

View File

@ -197,7 +197,7 @@ func TestFromPath(t *testing.T) {
credentialIssuer: pinniped-config
apiService: pinniped-api
`),
wantError: "validate apiGroupSuffix: 1 error(s):\n- a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')",
wantError: "validate apiGroupSuffix: a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')",
},
}
for _, test := range tests {

View File

@ -72,7 +72,7 @@ func TestFromPath(t *testing.T) {
names:
defaultTLSCertificateSecret: my-secret-name
`),
wantError: "validate apiGroupSuffix: 1 error(s):\n- a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')",
wantError: "validate apiGroupSuffix: a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')",
},
}
for _, test := range tests {

View File

@ -12,6 +12,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/clock"
"k8s.io/apimachinery/pkg/util/errors"
"k8s.io/client-go/util/retry"
"k8s.io/klog/v2"
@ -20,7 +21,6 @@ import (
configinformers "go.pinniped.dev/generated/1.20/client/supervisor/informers/externalversions/config/v1alpha1"
pinnipedcontroller "go.pinniped.dev/internal/controller"
"go.pinniped.dev/internal/controllerlib"
"go.pinniped.dev/internal/multierror"
"go.pinniped.dev/internal/oidc/provider"
"go.pinniped.dev/internal/plog"
)
@ -107,7 +107,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
}
}
errs := multierror.New()
var errs []error
federationDomainIssuers := make([]*provider.FederationDomainIssuer, 0)
for _, federationDomain := range federationDomains {
@ -123,7 +123,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
configv1alpha1.DuplicateFederationDomainStatusCondition,
"Duplicate issuer: "+federationDomain.Spec.Issuer,
); err != nil {
errs.Add(fmt.Errorf("could not update status: %w", err))
errs = append(errs, fmt.Errorf("could not update status: %w", err))
}
continue
}
@ -138,7 +138,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
configv1alpha1.SameIssuerHostMustUseSameSecretFederationDomainStatusCondition,
"Issuers with the same DNS hostname (address not including port) must use the same secretName: "+issuerURLToHostnameKey(issuerURL),
); err != nil {
errs.Add(fmt.Errorf("could not update status: %w", err))
errs = append(errs, fmt.Errorf("could not update status: %w", err))
}
continue
}
@ -152,7 +152,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
configv1alpha1.InvalidFederationDomainStatusCondition,
"Invalid: "+err.Error(),
); err != nil {
errs.Add(fmt.Errorf("could not update status: %w", err))
errs = append(errs, fmt.Errorf("could not update status: %w", err))
}
continue
}
@ -164,7 +164,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
configv1alpha1.SuccessFederationDomainStatusCondition,
"Provider successfully created",
); err != nil {
errs.Add(fmt.Errorf("could not update status: %w", err))
errs = append(errs, fmt.Errorf("could not update status: %w", err))
continue
}
@ -173,7 +173,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
c.providerSetter.SetProviders(federationDomainIssuers...)
return errs.ErrOrNil()
return errors.NewAggregate(errs)
}
func (c *federationDomainWatcherController) updateStatus(

View File

@ -6,6 +6,7 @@ package supervisorconfig
import (
"context"
"errors"
"fmt"
"net/url"
"reflect"
"sync"
@ -320,7 +321,7 @@ func TestSync(t *testing.T) {
it("sets the provider that it could actually update in the API", func() {
startInformersAndController()
err := controllerlib.TestSync(t, subject, *syncContext)
r.EqualError(err, "1 error(s):\n- could not update status: some update error")
r.EqualError(err, "could not update status: some update error")
provider1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer)
r.NoError(err)
@ -339,7 +340,7 @@ func TestSync(t *testing.T) {
it("returns an error", func() {
startInformersAndController()
err := controllerlib.TestSync(t, subject, *syncContext)
r.EqualError(err, "1 error(s):\n- could not update status: some update error")
r.EqualError(err, "could not update status: some update error")
federationDomain1.Status.Status = v1alpha1.SuccessFederationDomainStatusCondition
federationDomain1.Status.Message = "Provider successfully created"
@ -455,7 +456,7 @@ func TestSync(t *testing.T) {
it("returns an error", func() {
startInformersAndController()
err := controllerlib.TestSync(t, subject, *syncContext)
r.EqualError(err, "1 error(s):\n- could not update status: some update error")
r.EqualError(err, "could not update status: some update error")
federationDomain.Status.Status = v1alpha1.SuccessFederationDomainStatusCondition
federationDomain.Status.Message = "Provider successfully created"
@ -491,7 +492,7 @@ func TestSync(t *testing.T) {
it("returns the get error", func() {
startInformersAndController()
err := controllerlib.TestSync(t, subject, *syncContext)
r.EqualError(err, "1 error(s):\n- could not update status: get failed: some get error")
r.EqualError(err, "could not update status: get failed: some get error")
federationDomain.Status.Status = v1alpha1.SuccessFederationDomainStatusCondition
federationDomain.Status.Message = "Provider successfully created"
@ -606,7 +607,7 @@ func TestSync(t *testing.T) {
it("sets the provider that it could actually update in the API", func() {
startInformersAndController()
err := controllerlib.TestSync(t, subject, *syncContext)
r.EqualError(err, "1 error(s):\n- could not update status: some update error")
r.EqualError(err, "could not update status: some update error")
validProvider, err := provider.NewFederationDomainIssuer(validFederationDomain.Spec.Issuer)
r.NoError(err)
@ -623,7 +624,7 @@ func TestSync(t *testing.T) {
it("returns an error", func() {
startInformersAndController()
err := controllerlib.TestSync(t, subject, *syncContext)
r.EqualError(err, "1 error(s):\n- could not update status: some update error")
r.EqualError(err, "could not update status: some update error")
validFederationDomain.Status.Status = v1alpha1.SuccessFederationDomainStatusCondition
validFederationDomain.Status.Message = "Provider successfully created"
@ -761,22 +762,20 @@ func TestSync(t *testing.T) {
})
when("we cannot talk to the API", func() {
var count int
it.Before(func() {
pinnipedAPIClient.PrependReactor(
"get",
"federationdomains",
func(_ coretesting.Action) (bool, runtime.Object, error) {
return true, nil, errors.New("some get error")
count++
return true, nil, fmt.Errorf("some get error %d", count)
},
)
})
it("returns the get errors", func() {
expectedError := here.Doc(`
3 error(s):
- could not update status: get failed: some get error
- could not update status: get failed: some get error
- could not update status: get failed: some get error`)
expectedError := here.Doc(`[could not update status: get failed: some get error 1, could not update status: get failed: some get error 2, could not update status: get failed: some get error 3]`)
startInformersAndController()
err := controllerlib.TestSync(t, subject, *syncContext)
r.EqualError(err, expectedError)
@ -947,23 +946,20 @@ func TestSync(t *testing.T) {
})
when("we cannot talk to the API", func() {
var count int
it.Before(func() {
pinnipedAPIClient.PrependReactor(
"get",
"federationdomains",
func(_ coretesting.Action) (bool, runtime.Object, error) {
return true, nil, errors.New("some get error")
count++
return true, nil, fmt.Errorf("some get error %d", count)
},
)
})
it("returns the get errors", func() {
expectedError := here.Doc(`
4 error(s):
- could not update status: get failed: some get error
- could not update status: get failed: some get error
- could not update status: get failed: some get error
- could not update status: get failed: some get error`)
expectedError := here.Doc(`[could not update status: get failed: some get error 1, could not update status: get failed: some get error 2, could not update status: get failed: some get error 3, could not update status: get failed: some get error 4]`)
startInformersAndController()
err := controllerlib.TestSync(t, subject, *syncContext)
r.EqualError(err, expectedError)

View File

@ -9,11 +9,11 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/validation"
"go.pinniped.dev/internal/constable"
"go.pinniped.dev/internal/kubeclient"
"go.pinniped.dev/internal/multierror"
)
const (
@ -127,17 +127,17 @@ func Unreplace(baseAPIGroup, apiGroupSuffix string) (string, bool) {
// makes sure that the provided apiGroupSuffix is a valid DNS-1123 subdomain with at least one dot,
// to match Kubernetes behavior.
func Validate(apiGroupSuffix string) error {
err := multierror.New()
var errs []error //nolint: prealloc
if len(strings.Split(apiGroupSuffix, ".")) < 2 {
err.Add(constable.Error("must contain '.'"))
errs = append(errs, constable.Error("must contain '.'"))
}
errorStrings := validation.IsDNS1123Subdomain(apiGroupSuffix)
for _, errorString := range errorStrings {
errorString := errorString
err.Add(constable.Error(errorString))
errs = append(errs, constable.Error(errorString))
}
return err.ErrOrNil()
return errors.NewAggregate(errs)
}

View File

@ -487,19 +487,19 @@ func TestValidate(t *testing.T) {
},
{
apiGroupSuffix: "no-dots",
wantErrorPrefix: "1 error(s):\n- must contain '.'",
wantErrorPrefix: "must contain '.'",
},
{
apiGroupSuffix: ".starts.with.dot",
wantErrorPrefix: "1 error(s):\n- a lowercase RFC 1123 subdomain must consist",
wantErrorPrefix: "a lowercase RFC 1123 subdomain must consist",
},
{
apiGroupSuffix: "ends.with.dot.",
wantErrorPrefix: "1 error(s):\n- a lowercase RFC 1123 subdomain must consist",
wantErrorPrefix: "a lowercase RFC 1123 subdomain must consist",
},
{
apiGroupSuffix: ".multiple-issues.because-this-string-is-longer-than-the-253-character-limit-of-a-dns-1123-identifier-" + chars(253),
wantErrorPrefix: "2 error(s):\n- must be no more than 253 characters\n- a lowercase RFC 1123 subdomain must consist",
wantErrorPrefix: "[must be no more than 253 characters, a lowercase RFC 1123 subdomain must consist",
},
}
for _, test := range tests {

View File

@ -30,6 +30,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/errors"
kubescheme "k8s.io/client-go/kubernetes/scheme"
restclient "k8s.io/client-go/rest"
aggregatorclientscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme"
@ -37,7 +38,6 @@ import (
pinnipedconciergeclientsetscheme "go.pinniped.dev/generated/1.20/client/concierge/clientset/versioned/scheme"
pinnipedsupervisorclientsetscheme "go.pinniped.dev/generated/1.20/client/supervisor/clientset/versioned/scheme"
"go.pinniped.dev/internal/httputil/httperr"
"go.pinniped.dev/internal/multierror"
)
// Start starts an httptest.Server (with TLS) that pretends to be a Kube API server.
@ -107,7 +107,7 @@ func decodeObj(r *http.Request) (runtime.Object, error) {
}
var obj runtime.Object
multiErr := multierror.New()
var errs []error //nolint: prealloc
codecsThatWeUseInOurCode := []runtime.NegotiatedSerializer{
kubescheme.Codecs,
aggregatorclientscheme.Codecs,
@ -119,9 +119,9 @@ func decodeObj(r *http.Request) (runtime.Object, error) {
if err == nil {
return obj, nil
}
multiErr.Add(err)
errs = append(errs, err)
}
return nil, multiErr.ErrOrNil()
return nil, errors.NewAggregate(errs)
}
func tryDecodeObj(

View File

@ -112,7 +112,7 @@ func TestNew(t *testing.T) {
WithEndpoint("https://example.com"),
WithAPIGroupSuffix(""),
},
wantErr: "invalid api group suffix: 2 error(s):\n- must contain '.'\n- a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')",
wantErr: "invalid api group suffix: [must contain '.', a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')]",
},
{
name: "invalid api group suffix",
@ -121,7 +121,7 @@ func TestNew(t *testing.T) {
WithEndpoint("https://example.com"),
WithAPIGroupSuffix(".starts.with.dot"),
},
wantErr: "invalid api group suffix: 1 error(s):\n- a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')",
wantErr: "invalid api group suffix: a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')",
},
{
name: "valid",