Adjust to new K8s 1.30 API

internal/controller/authenticator/cachecleaner/cachecleaner_test.go

@@ -8,16 +8,16 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"go.uber.org/mock/gomock"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/authentication/authenticator"
 
 	authv1alpha "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1"
 	pinnipedfake "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned/fake"
 	pinnipedinformers "go.pinniped.dev/generated/latest/client/concierge/informers/externalversions"
+	controllerAuthenticator "go.pinniped.dev/internal/controller/authenticator"
 	"go.pinniped.dev/internal/controller/authenticator/authncache"
 	"go.pinniped.dev/internal/controllerlib"
-	"go.pinniped.dev/internal/mocks/mocktokenauthenticatorcloser"
 	"go.pinniped.dev/internal/testutil/testlogger"
 )
 
@@ -109,8 +109,8 @@ func TestController(t *testing.T) {
 			initialCache: func(t *testing.T, cache *authncache.Cache) {
 				cache.Store(testWebhookKey1, nil)
 				cache.Store(testWebhookKey2, nil)
-				cache.Store(testJWTAuthenticatorKey1, newClosableCacheValue(t, 0))
-				cache.Store(testJWTAuthenticatorKey2, newClosableCacheValue(t, 1))
+				cache.Store(testJWTAuthenticatorKey1, newClosableCacheValue(t, false))
+				cache.Store(testJWTAuthenticatorKey2, newClosableCacheValue(t, true))
 				cache.Store(testKeyUnknownType, nil)
 			},
 			objects: []runtime.Object{
@@ -174,10 +174,28 @@ func TestController(t *testing.T) {
 	}
 }
 
-func newClosableCacheValue(t *testing.T, wantCloses int) authncache.Value {
-	ctrl := gomock.NewController(t)
-	t.Cleanup(ctrl.Finish)
-	tac := mocktokenauthenticatorcloser.NewMockTokenAuthenticatorCloser(ctrl)
-	tac.EXPECT().Close().Times(wantCloses)
-	return tac
+type mockValue struct {
+	wasClosed bool
+}
+
+func (m *mockValue) Close() {
+	m.wasClosed = true
+}
+
+func (m *mockValue) AuthenticateToken(_ context.Context, _ string) (*authenticator.Response, bool, error) {
+	panic("implement me")
+}
+
+var _ authncache.Value = (*mockValue)(nil)
+var _ controllerAuthenticator.Closer = (*mockValue)(nil)
+
+func newClosableCacheValue(t *testing.T, wantClose bool) authncache.Value {
+	t.Helper()
+	mock := &mockValue{}
+
+	t.Cleanup(func() {
+		require.Equal(t, wantClose, mock.wasClosed)
+	})
+
+	return mock
 }

internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go

@@ -102,10 +102,17 @@ type tokenAuthenticatorCloser interface {
 }
 
 type cachedJWTAuthenticator struct {
-	tokenAuthenticatorCloser
-	spec *auth1alpha1.JWTAuthenticatorSpec
+	authenticator.Token
+	spec   *auth1alpha1.JWTAuthenticatorSpec
+	cancel context.CancelFunc
 }
 
+func (c *cachedJWTAuthenticator) Close() {
+	c.cancel()
+}
+
+var _ tokenAuthenticatorCloser = (*cachedJWTAuthenticator)(nil)
+
 // New instantiates a new controllerlib.Controller which will populate the provided authncache.Cache.
 func New(
 	cache *authncache.Cache,
@@ -162,7 +169,7 @@ func (c *jwtCacheFillerController) Sync(ctx controllerlib.Context) error {
 	// If this authenticator already exists, then only recreate it if is different from the desired
 	// authenticator. We don't want to be creating a new authenticator for every resync period.
 	//
-	// If we do need to recreate the authenticator, then make sure we close the old one to avoid
+	// If we do need to recreate the authenticator, then make sure cancel the old one to avoid
 	// goroutine leaks.
 	if value := c.cache.Get(cacheKey); value != nil {
 		jwtAuthenticator := c.extractValueAsJWTAuthenticator(value)
@@ -517,7 +524,8 @@ func (c *jwtCacheFillerController) newCachedJWTAuthenticator(client *http.Client
 		groupsClaim = defaultGroupsClaim
 	}
 
-	oidcAuthenticator, err := oidc.New(oidc.Options{
+	cancelCtx, cancel := context.WithCancel(context.Background())
+	oidcAuthenticator, err := oidc.New(cancelCtx, oidc.Options{
 		JWTAuthenticator: apiserver.JWTAuthenticator{
 			Issuer: apiserver.Issuer{
 				URL:       spec.Issuer,
@@ -552,6 +560,7 @@ func (c *jwtCacheFillerController) newCachedJWTAuthenticator(client *http.Client
 			Reason:  reasonInvalidAuthenticator,
 			Message: msg,
 		})
+		cancel()
 		// resync err, lots of possible issues that may or may not be machine related
 		return nil, conditions, fmt.Errorf("%s: %w", errText, err)
 	}
@@ -563,8 +572,9 @@ func (c *jwtCacheFillerController) newCachedJWTAuthenticator(client *http.Client
 		Message: msg,
 	})
 	return &cachedJWTAuthenticator{
-		tokenAuthenticatorCloser: oidcAuthenticator,
-		spec:                     spec,
+		Token:  oidcAuthenticator,
+		spec:   spec,
+		cancel: cancel,
 	}, conditions, nil
 }
 

internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go

@@ -24,7 +24,6 @@ import (
 	fositejwt "github.com/ory/fosite/token/jwt"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"go.uber.org/mock/gomock"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -40,7 +39,6 @@ import (
 	"go.pinniped.dev/internal/controller/authenticator/authncache"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/crypto/ptls"
-	"go.pinniped.dev/internal/mocks/mocktokenauthenticatorcloser"
 	"go.pinniped.dev/internal/plog"
 	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/internal/testutil/conciergetestutil"
@@ -1749,11 +1747,12 @@ func TestController(t *testing.T) {
 				Kind:     "JWTAuthenticator",
 				Name:     syncCtx.Key.Name,
 			}
-			cachedAuthenticator := cache.Get(expectedCacheKey)
+			cachedAuthenticator, ok := cache.Get(expectedCacheKey).(tokenAuthenticatorCloser)
+			require.True(t, ok)
 			require.NotNil(t, cachedAuthenticator)
 
 			// Schedule it to be closed at the end of the test.
-			t.Cleanup(cachedAuthenticator.(*cachedJWTAuthenticator).Close)
+			t.Cleanup(cachedAuthenticator.Close)
 
 			const (
 				goodSubject  = "some-subject"
@@ -2087,18 +2086,17 @@ func createJWT(
 }
 
 func newCacheValue(t *testing.T, spec auth1alpha1.JWTAuthenticatorSpec, wantClose bool) authncache.Value {
-	ctrl := gomock.NewController(t)
-	t.Cleanup(ctrl.Finish)
-	tokenAuthenticatorCloser := mocktokenauthenticatorcloser.NewMockTokenAuthenticatorCloser(ctrl)
+	t.Helper()
+	wasClosed := false
 
-	wantCloses := 0
-	if wantClose {
-		wantCloses++
-	}
-	tokenAuthenticatorCloser.EXPECT().Close().Times(wantCloses)
+	t.Cleanup(func() {
+		require.Equal(t, wantClose, wasClosed)
+	})
 
 	return &cachedJWTAuthenticator{
-		tokenAuthenticatorCloser: tokenAuthenticatorCloser,
-		spec:                     &spec,
+		spec: &spec,
+		cancel: func() {
+			wasClosed = true
+		},
 	}
 }

internal/controller/impersonatorconfig/impersonator_config.go

@@ -1,4 +1,4 @@
-// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package impersonatorconfig
@@ -27,6 +27,7 @@ import (
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	corev1informers "k8s.io/client-go/informers/core/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
@@ -1205,7 +1206,7 @@ func validateCredentialIssuerSpec(spec *v1alpha1.ImpersonationProxySpec) error {
 	}
 
 	// If specified, validate that the LoadBalancerIP is a valid IPv4 or IPv6 address.
-	if ip := spec.Service.LoadBalancerIP; ip != "" && len(validation.IsValidIP(ip)) > 0 {
+	if ip := spec.Service.LoadBalancerIP; ip != "" && len(validation.IsValidIP(field.NewPath("spec", "service", "loadBalancerIP"), ip)) > 0 {
 		return fmt.Errorf("invalid LoadBalancerIP %q", spec.Service.LoadBalancerIP)
 	}
 

internal/endpointaddr/endpointaddr.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 
 	"k8s.io/apimachinery/pkg/util/validation"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
 type HostPort struct {
@@ -41,7 +42,7 @@ func (h *HostPort) Endpoint() string {
 // - "[<IPv6>]:<port>"   (IPv6 address with port, brackets are required)
 //
 // If the input does not specify a port number, then defaultPort will be used.
-func Parse(endpoint string, defaultPort uint16) (HostPort, error) {
+func Parse(endpoint string, defaultPort uint16, paths ...string) (HostPort, error) {
 	// Try parsing it both with and without an implicit port 443 at the end.
 	host, port, err := net.SplitHostPort(endpoint)
 
@@ -61,9 +62,13 @@ func Parse(endpoint string, defaultPort uint16) (HostPort, error) {
 		return HostPort{}, fmt.Errorf("invalid port %q", port)
 	}
 
+	if len(paths) < 1 {
+		paths = []string{"UNKNOWN_PATH"}
+	}
+
 	// Check if the host part is a IPv4 or IPv6 address or a valid hostname according to RFC 1123.
 	switch {
-	case len(validation.IsValidIP(host)) == 0:
+	case len(validation.IsValidIP(field.NewPath(paths[0], paths[1:]...), host)) == 0:
 	case len(validation.IsDNS1123Subdomain(host)) == 0:
 	default:
 		return HostPort{}, fmt.Errorf("host %q is not a valid hostname or IP address", host)

internal/endpointaddr/endpointaddr_test.go

@@ -17,6 +17,7 @@ func TestParse(t *testing.T) {
 		name           string
 		input          string
 		defaultPort    uint16
+		paths          []string
 		expectErr      string
 		expect         HostPort
 		expectEndpoint string
@@ -42,6 +43,13 @@ func TestParse(t *testing.T) {
 			expect:         HostPort{Host: "127.0.0.1", Port: 8443},
 			expectEndpoint: "127.0.0.1:8443",
 		},
+		{
+			name:        "invalid IPv4",
+			input:       "1.1.1.",
+			defaultPort: 443,
+			paths:       []string{"does", "not", "matter", "because", "this", "error", "is", "ignored"},
+			expectErr:   `host "1.1.1." is not a valid hostname or IP address`,
+		},
 		{
 			name:           "IPv4 as IPv6 in brackets with port",
 			input:          "[::127.0.0.1]:8443",
@@ -170,7 +178,7 @@ func TestParse(t *testing.T) {
 	} {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := Parse(tt.input, tt.defaultPort)
+			got, err := Parse(tt.input, tt.defaultPort, tt.paths...)
 			if tt.expectErr == "" {
 				assert.NoError(t, err)
 				assert.Equal(t, tt.expect, got)

internal/mocks/mocktokenauthenticatorcloser/generate.go

@@ -1,21 +0,0 @@
-// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package mocktokenauthenticatorcloser
-
-import (
-	"k8s.io/apiserver/pkg/authentication/authenticator"
-
-	pinnipedauthenticator "go.pinniped.dev/internal/controller/authenticator"
-)
-
-//go:generate go run -v go.uber.org/mock/mockgen  -destination=mocktokenauthenticatorcloser.go -package=mocktokenauthenticatorcloser -copyright_file=../../../hack/header.txt . TokenAuthenticatorCloser
-
-// TokenAuthenticatorCloser is a type that can authenticate tokens and be closed idempotently.
-//
-// This type is slightly different from io.Closer, because io.Closer can return an error and is not
-// necessarily idempotent.
-type TokenAuthenticatorCloser interface {
-	authenticator.Token
-	pinnipedauthenticator.Closer
-}

internal/mocks/mocktokenauthenticatorcloser/mocktokenauthenticatorcloser.go

@@ -1,73 +0,0 @@
-// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-//
-
-// Code generated by MockGen. DO NOT EDIT.
-// Source: go.pinniped.dev/internal/mocks/mocktokenauthenticatorcloser (interfaces: TokenAuthenticatorCloser)
-//
-// Generated by this command:
-//
-//	mockgen -destination=mocktokenauthenticatorcloser.go -package=mocktokenauthenticatorcloser -copyright_file=../../../hack/header.txt . TokenAuthenticatorCloser
-//
-
-// Package mocktokenauthenticatorcloser is a generated GoMock package.
-package mocktokenauthenticatorcloser
-
-import (
-	context "context"
-	reflect "reflect"
-
-	gomock "go.uber.org/mock/gomock"
-	authenticator "k8s.io/apiserver/pkg/authentication/authenticator"
-)
-
-// MockTokenAuthenticatorCloser is a mock of TokenAuthenticatorCloser interface.
-type MockTokenAuthenticatorCloser struct {
-	ctrl     *gomock.Controller
-	recorder *MockTokenAuthenticatorCloserMockRecorder
-}
-
-// MockTokenAuthenticatorCloserMockRecorder is the mock recorder for MockTokenAuthenticatorCloser.
-type MockTokenAuthenticatorCloserMockRecorder struct {
-	mock *MockTokenAuthenticatorCloser
-}
-
-// NewMockTokenAuthenticatorCloser creates a new mock instance.
-func NewMockTokenAuthenticatorCloser(ctrl *gomock.Controller) *MockTokenAuthenticatorCloser {
-	mock := &MockTokenAuthenticatorCloser{ctrl: ctrl}
-	mock.recorder = &MockTokenAuthenticatorCloserMockRecorder{mock}
-	return mock
-}
-
-// EXPECT returns an object that allows the caller to indicate expected use.
-func (m *MockTokenAuthenticatorCloser) EXPECT() *MockTokenAuthenticatorCloserMockRecorder {
-	return m.recorder
-}
-
-// AuthenticateToken mocks base method.
-func (m *MockTokenAuthenticatorCloser) AuthenticateToken(arg0 context.Context, arg1 string) (*authenticator.Response, bool, error) {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "AuthenticateToken", arg0, arg1)
-	ret0, _ := ret[0].(*authenticator.Response)
-	ret1, _ := ret[1].(bool)
-	ret2, _ := ret[2].(error)
-	return ret0, ret1, ret2
-}
-
-// AuthenticateToken indicates an expected call of AuthenticateToken.
-func (mr *MockTokenAuthenticatorCloserMockRecorder) AuthenticateToken(arg0, arg1 any) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthenticateToken", reflect.TypeOf((*MockTokenAuthenticatorCloser)(nil).AuthenticateToken), arg0, arg1)
-}
-
-// Close mocks base method.
-func (m *MockTokenAuthenticatorCloser) Close() {
-	m.ctrl.T.Helper()
-	m.ctrl.Call(m, "Close")
-}
-
-// Close indicates an expected call of Close.
-func (mr *MockTokenAuthenticatorCloserMockRecorder) Close() *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockTokenAuthenticatorCloser)(nil).Close))
-}

internal/upstreamldap/upstreamldap.go

@@ -326,7 +326,7 @@ func (p *Provider) dialTLS(ctx context.Context, addr endpointaddr.HostPort) (Con
 	}
 
 	conn := ldap.NewConn(c, true)
-	conn.Start() //nolint:staticcheck // will need a different approach soon
+	conn.Start()
 	return conn, nil
 }
 
@@ -348,7 +348,7 @@ func (p *Provider) dialStartTLS(ctx context.Context, addr endpointaddr.HostPort)
 	}
 
 	conn := ldap.NewConn(c, false)
-	conn.Start() //nolint:staticcheck // will need a different approach soon
+	conn.Start()
 	err = conn.StartTLS(tlsConfig)
 	if err != nil {
 		return nil, err

test/testlib/activedirectory.go

@@ -199,7 +199,7 @@ func dialTLS(t *testing.T, env *TestEnv) *ldap.Conn {
 	c, err := dialer.DialContext(context.Background(), "tcp", env.SupervisorUpstreamActiveDirectory.Host)
 	require.NoError(t, err)
 	conn := ldap.NewConn(c, true)
-	conn.Start() //nolint:staticcheck // will need a different approach soon
+	conn.Start()
 	return conn
 }