Merge pull request #1387 from vmware-tanzu/jtc/bump-to-golang-1.19-semantics

Bump to golang 1.18 semantics

cmd/pinniped-server/main.go

@@ -12,12 +12,13 @@ import (
 	"os"
 	"path/filepath"
 
+	"k8s.io/apimachinery/pkg/util/sets"
+
 	concierge "go.pinniped.dev/internal/concierge/server"
 	// this side effect import ensures that we use fipsonly crypto in fips_strict mode.
 	_ "go.pinniped.dev/internal/crypto/ptls"
 	lua "go.pinniped.dev/internal/localuserauthenticator"
 	"go.pinniped.dev/internal/plog"
-	"go.pinniped.dev/internal/psets"
 	supervisor "go.pinniped.dev/internal/supervisor/server"
 )
 
@@ -37,7 +38,7 @@ func main() {
 	}
 	binary := filepath.Base(os.Args[0])
 	if subcommands[binary] == nil {
-		fail(fmt.Errorf("must be invoked as one of %v, not %q", psets.StringKeySet(subcommands).List(), binary))
+		fail(fmt.Errorf("must be invoked as one of %v, not %q", sets.StringKeySet(subcommands).List(), binary))
 	}
 	subcommands[binary]()
 }

cmd/pinniped/cmd/kubeconfig.go

@@ -720,6 +720,7 @@ func validateKubeconfig(ctx context.Context, flags getKubeconfigParams, kubeconf
 func countCACerts(pemData []byte) int {
 	pool := x509.NewCertPool()
 	pool.AppendCertsFromPEM(pemData)
+	//nolint:staticcheck // since we're not using .Subjects() to access the system pool
 	return len(pool.Subjects())
 }
 

go.mod

@@ -1,6 +1,6 @@
 module go.pinniped.dev
 
-go 1.17
+go 1.18
 
 require (
 	github.com/MakeNowJust/heredoc/v2 v2.0.1
@@ -30,7 +30,6 @@ require (
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.8.1
 	github.com/tdewolff/minify/v2 v2.12.4
-	go.uber.org/atomic v1.10.0
 	go.uber.org/zap v1.24.0
 	golang.org/x/crypto v0.5.0
 	golang.org/x/net v0.5.0
@@ -139,6 +138,7 @@ require (
 	go.opentelemetry.io/otel/sdk v1.10.0 // indirect
 	go.opentelemetry.io/otel/trace v1.10.0 // indirect
 	go.opentelemetry.io/proto/otlp v0.19.0 // indirect
+	go.uber.org/atomic v1.10.0 // indirect
 	go.uber.org/multierr v1.8.0 // indirect
 	golang.org/x/mod v0.7.0 // indirect
 	golang.org/x/sys v0.4.0 // indirect

hack/module.sh

@@ -9,7 +9,7 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
 
 function tidy_cmd() {
   local version="$(cat "${ROOT}/go.mod" | grep '^go ' | cut -f 2 -d ' ')"
-  echo "go mod tidy -v -go=${version} -compat=1.17"
+  echo "go mod tidy -v -go=${version} -compat=${version}"
 }
 
 function lint_cmd() {

internal/certauthority/certauthority_test.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package certauthority
@@ -206,6 +206,7 @@ func TestPool(t *testing.T) {
 	require.NoError(t, err)
 
 	pool := ca.Pool()
+	//nolint:staticcheck // since we're not using .Subjects() to access the system pool
 	require.Len(t, pool.Subjects(), 1)
 }
 

internal/dynamiccert/provider_test.go

@@ -1,4 +1,4 @@
-// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package dynamiccert
@@ -41,6 +41,7 @@ func TestProviderWithDynamicServingCertificateController(t *testing.T) {
 				cert, err := tls.X509KeyPair(certPEM, keyPEM)
 				require.NoError(t, err)
 
+				//nolint:staticcheck // since we're not using .Subjects() to access the system pool
 				return pool.Subjects(), []tls.Certificate{cert}
 			},
 		},
@@ -69,6 +70,7 @@ func TestProviderWithDynamicServingCertificateController(t *testing.T) {
 
 				certKey.UnsetCertKeyContent()
 
+				//nolint:staticcheck // since we're not using .Subjects() to access the system pool
 				return pool.Subjects(), []tls.Certificate{cert}
 			},
 		},
@@ -87,6 +89,7 @@ func TestProviderWithDynamicServingCertificateController(t *testing.T) {
 				cert, err := tls.X509KeyPair(certPEM, keyPEM)
 				require.NoError(t, err)
 
+				//nolint:staticcheck // since we're not using .Subjects() to access the system pool
 				return newCA.Pool().Subjects(), []tls.Certificate{cert}
 			},
 		},
@@ -110,6 +113,7 @@ func TestProviderWithDynamicServingCertificateController(t *testing.T) {
 				ok := pool.AppendCertsFromPEM(ca.CurrentCABundleContent())
 				require.True(t, ok, "should have valid non-empty CA bundle")
 
+				//nolint:staticcheck // since we're not using .Subjects() to access the system pool
 				return pool.Subjects(), []tls.Certificate{cert}
 			},
 		},
@@ -137,6 +141,7 @@ func TestProviderWithDynamicServingCertificateController(t *testing.T) {
 				err = ca.SetCertKeyContent(newOtherCA.Bundle(), caKey)
 				require.NoError(t, err)
 
+				//nolint:staticcheck // since we're not using .Subjects() to access the system pool
 				return newOtherCA.Pool().Subjects(), []tls.Certificate{cert}
 			},
 		},
@@ -221,6 +226,7 @@ func poolSubjects(pool *x509.CertPool) [][]byte {
 	if pool == nil {
 		return nil
 	}
+	//nolint:staticcheck // since we're not using .Subjects() to access the system pool
 	return pool.Subjects()
 }
 

internal/kubeclient/kubeclient_test.go

@@ -949,6 +949,7 @@ func TestUnwrap(t *testing.T) {
 
 	server, restConfig := fakekubeapi.Start(t, nil)
 
+	//nolint:staticcheck // since we're not using .Subjects() to access the system pool
 	serverSubjects := server.Client().Transport.(*http.Transport).TLSClientConfig.RootCAs.Subjects()
 
 	t.Run("regular client", func(t *testing.T) {
@@ -1089,6 +1090,7 @@ func testUnwrap(t *testing.T, client *Client, serverSubjects [][]byte) {
 			require.Equal(t, secureTLSConfig.NextProtos, tlsConfig.NextProtos)
 
 			// x509.CertPool has some embedded functions that make it hard to compare so just look at the subjects
+			//nolint:staticcheck // since we're not using .Subjects() to access the system pool
 			require.Equal(t, serverSubjects, tlsConfig.RootCAs.Subjects())
 		})
 	}

internal/leaderelection/leaderelection.go

@@ -1,4 +1,4 @@
-// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package leaderelection
@@ -6,9 +6,9 @@ package leaderelection
 import (
 	"context"
 	"fmt"
+	"sync/atomic"
 	"time"
 
-	"go.uber.org/atomic"
 	appsv1 "k8s.io/api/apps/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -45,7 +45,7 @@ func New(podInfo *downward.PodInfo, deployment *appsv1.Deployment, opts ...kubec
 		return nil, nil, fmt.Errorf("could not create internal client for leader election: %w", err)
 	}
 
-	isLeader := &isLeaderTracker{tracker: atomic.NewBool(false)}
+	isLeader := &isLeaderTracker{tracker: &atomic.Bool{}}
 
 	identity := podInfo.Name
 	leaseName := deployment.Name

internal/leaderelection/leaderelection_test.go

@@ -1,4 +1,4 @@
-// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package leaderelection
@@ -6,11 +6,11 @@ package leaderelection
 import (
 	"context"
 	"errors"
+	"sync/atomic"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
-	"go.uber.org/atomic"
 	coordinationv1 "k8s.io/api/coordination/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kubefake "k8s.io/client-go/kubernetes/fake"
@@ -63,7 +63,7 @@ func Test_releaseLock_Update(t *testing.T) {
 			t.Parallel()
 
 			internalClient := kubefake.NewSimpleClientset()
-			isLeader := &isLeaderTracker{tracker: atomic.NewBool(false)}
+			isLeader := &isLeaderTracker{tracker: &atomic.Bool{}}
 
 			leaderElectorCtx, cancel := context.WithCancel(context.Background())
 

internal/psets/psets.go

@@ -1,70 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package psets
-
-import (
-	"reflect"
-	"sort"
-)
-
-// These were copied from https://github.com/kubernetes/kubernetes/tree/v1.25.5/staging/src/k8s.io/apimachinery/pkg/util/sets
-// which is the last version before they were converted to generic functions which require the use
-// of Go 1.18+ to compile. This is not a full copy of the files from k/k, but rather only copies of the
-// functions that we actually use. When we are ready to require the use of Go 1.18+ to compile Pinniped,
-// then we can go back to using the version of this package from the k8s libraries. Our use
-// of this package was very minimal, so its easy enough to just copy the few functions that we were
-// actually using to keep Go 1.17 compatibility a little longer.
-
-// Empty is public since it is used by some internal API objects for conversions between external
-// string arrays and internal sets, and conversion logic requires public types today.
-type Empty struct{}
-
-// sets.String is a set of strings, implemented via map[string]struct{} for minimal memory consumption.
-type String map[string]Empty
-
-// StringKeySet creates a String from a keys of a map[string](? extends interface{}).
-// If the value passed in is not actually a map, this will panic.
-func StringKeySet(theMap interface{}) String {
-	v := reflect.ValueOf(theMap)
-	ret := String{}
-
-	for _, keyValue := range v.MapKeys() {
-		ret.Insert(keyValue.Interface().(string))
-	}
-	return ret
-}
-
-// Insert adds items to the set.
-func (s String) Insert(items ...string) String {
-	for _, item := range items {
-		s[item] = Empty{}
-	}
-	return s
-}
-
-// Has returns true if and only if item is contained in the set.
-func (s String) Has(item string) bool {
-	_, contained := s[item]
-	return contained
-}
-
-type sortableSliceOfString []string
-
-func (s sortableSliceOfString) Len() int           { return len(s) }
-func (s sortableSliceOfString) Less(i, j int) bool { return lessString(s[i], s[j]) }
-func (s sortableSliceOfString) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
-
-// List returns the contents as a sorted string slice.
-func (s String) List() []string {
-	res := make(sortableSliceOfString, 0, len(s))
-	for key := range s {
-		res = append(res, key)
-	}
-	sort.Sort(res)
-	return []string(res)
-}
-
-func lessString(lhs, rhs string) bool {
-	return lhs < rhs
-}

internal/supervisor/server/bootstrap.go

@@ -1,4 +1,4 @@
-// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package server
@@ -9,9 +9,9 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"sync/atomic"
 	"time"
 
-	"go.uber.org/atomic"
 	"k8s.io/apimachinery/pkg/util/sets"
 
 	"go.pinniped.dev/internal/certauthority"
@@ -23,8 +23,8 @@ type contextKey int
 const bootstrapKey contextKey = iota
 
 func withBootstrapConnCtx(ctx context.Context, _ net.Conn) context.Context {
-	isBootstrap := atomic.NewBool(false) // safe for concurrent access
+	isBootstrap := atomic.Bool{} // safe for concurrent access
-	return context.WithValue(ctx, bootstrapKey, isBootstrap)
+	return context.WithValue(ctx, bootstrapKey, &isBootstrap)
 }
 
 func setIsBootstrapConn(ctx context.Context) {

internal/upstreamoidc/upstreamoidc.go

@@ -18,12 +18,12 @@ import (
 	"golang.org/x/oauth2"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/sets"
 
 	oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc"
 	"go.pinniped.dev/internal/httputil/httperr"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/plog"
-	"go.pinniped.dev/internal/psets"
 	"go.pinniped.dev/pkg/oidcclient/nonce"
 	"go.pinniped.dev/pkg/oidcclient/oidctypes"
 	"go.pinniped.dev/pkg/oidcclient/pkce"
@@ -421,7 +421,7 @@ func maybeLogClaims(msg, name string, claims map[string]interface{}) {
 	}
 
 	if plog.Enabled(plog.LevelDebug) { // log keys at debug level
-		keys := psets.StringKeySet(claims).List() // note: this is only safe because the compiler asserts that claims is a map[string]<anything>
+		keys := sets.StringKeySet(claims).List() // note: this is only safe because the compiler asserts that claims is a map[string]<anything>
 		plog.Debug(msg, "providerName", name, "keys", keys)
 		return
 	}

test/integration/leaderelection_test.go

@@ -17,13 +17,13 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/rand"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/util/retry"
 	"k8s.io/utils/pointer"
 
 	"go.pinniped.dev/internal/downward"
 	"go.pinniped.dev/internal/kubeclient"
 	"go.pinniped.dev/internal/leaderelection"
-	"go.pinniped.dev/internal/psets"
 	"go.pinniped.dev/test/testlib"
 )
 
@@ -175,7 +175,7 @@ func leaderElectionClients(t *testing.T, namespace *corev1.Namespace, leaseName
 		clients[identity], cancels[identity] = leaderElectionClient(t, namespace, leaseName, identity)
 	}
 
-	t.Logf("running leader election client tests with %d clients: %v", len(clients), psets.StringKeySet(clients).List())
+	t.Logf("running leader election client tests with %d clients: %v", len(clients), sets.StringKeySet(clients).List())
 
 	return clients, cancels
 }
@@ -191,7 +191,7 @@ func pickRandomLeaderElectionClient(clients map[string]*kubeclient.Client) *kube
 func waitForIdentity(ctx context.Context, t *testing.T, namespace *corev1.Namespace, leaseName string, clients map[string]*kubeclient.Client) *coordinationv1.Lease {
 	t.Helper()
 
-	identities := psets.StringKeySet(clients)
+	identities := sets.StringKeySet(clients)
 	var out *coordinationv1.Lease
 
 	testlib.RequireEventuallyWithoutError(t, func() (bool, error) {