From d95221d00245d98c61e585f8f705c869dd475d85 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= <mitr@redhat.com>
Date: Tue, 15 Mar 2022 20:31:32 +0100
Subject: [PATCH] Try to make (make lint) pass with Go 1.18
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is an attempt to fix (make lint), which complains:
> pkg/tlsclientconfig/tlsclientconfig_test.go:37:20: SA1019: tlsc.RootCAs.Subjects is deprecated: if s was returned by SystemCertPool, Subjects will not include the system roots. (staticcheck)
>       for _, s := range tlsc.RootCAs.Subjects() {
>                         ^
> pkg/tlsclientconfig/tlsclientconfig_test.go:43:20: SA1019: systemCertPool.Subjects is deprecated: if s was returned by SystemCertPool, Subjects will not include the system roots. (staticcheck)
>       for _, s := range systemCertPool.Subjects() {
>                         ^
> pkg/tlsclientconfig/tlsclientconfig_test.go:53:20: SA1019: tlsc.RootCAs.Subjects is deprecated: if s was returned by SystemCertPool, Subjects will not include the system roots. (staticcheck)
>       for _, s := range tlsc.RootCAs.Subjects() {

... but the same staticcheck linter, for some reason, does NOT
complain about these deprecated fields; the correct //lint:ignore
comments are ineffective and actually cause extra warnings.

So, silence all of staticcheck via //nolint , hopefully temporarily.

(Also, note that golangci-lint itself, with this update,
crashes with https://github.com/golangci/golangci-lint/issues/2374 ;
a local rebuild does not crash, but still fails per the above.)

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 Makefile                                    |  2 +-
 pkg/tlsclientconfig/tlsclientconfig_test.go | 35 ++++++++++++++++++---
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 800331dcc..c7d8eebfd 100644
--- a/Makefile
+++ b/Makefile
@@ -68,7 +68,7 @@ tools: .install.gitvalidation .install.golangci-lint .install.golint
 
 .install.golangci-lint:
 	if [ ! -x "$(GOBIN)/golangci-lint" ]; then \
-		curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(GOBIN) v1.35.2; \
+		curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(GOBIN) v1.44.2; \
 	fi
 
 .install.golint:
diff --git a/pkg/tlsclientconfig/tlsclientconfig_test.go b/pkg/tlsclientconfig/tlsclientconfig_test.go
index ac64cf647..979eb91ae 100644
--- a/pkg/tlsclientconfig/tlsclientconfig_test.go
+++ b/pkg/tlsclientconfig/tlsclientconfig_test.go
@@ -20,20 +20,45 @@ func TestSetupCertificates(t *testing.T) {
 	err := SetupCertificates("testdata/full", &tlsc)
 	require.NoError(t, err)
 	require.NotNil(t, tlsc.RootCAs)
-	// RootCAs include SystemCertPool
+
+	// SystemCertPool is implemented natively, and .Subjects() does not
+	// return raw certificates, on some systems (as of Go 1.18,
+	// Windows, macOS, iOS); so, .Subjects() is deprecated.
+	// We still use .Subjects() in these tests, because they work
+	// acceptably even in the native case, and they work fine on Linux,
+	// which we care about the most.
+
+	// For an unknown reason, with Go 1.18, as of Mar 15 2022,
+	// (golangci-lint) reports staticcheck SA1019 about using
+	// the deprecated .Subjects() function, but the //lint:ignore
+	// directives are ineffective (and cause extra warnings about
+	// pointless lint:ignore directives). So, use the big hammer
+	// of silencing staticcheck entirely; that should be removed
+	// as soon as practical.
+
+	// On systems where SystemCertPool is not special-cased, RootCAs include SystemCertPool;
+	// On systems where SystemCertPool is special cased, this compares two empty sets
+	// and succeeds.
+	// There isn’t a plausible alternative to calling .Subjects() here.
 	loadedSubjectBytes := map[string]struct{}{}
-	for _, s := range tlsc.RootCAs.Subjects() {
+	// lint:ignore SA1019 Receiving no data for system roots is acceptable.
+	for _, s := range tlsc.RootCAs.Subjects() { //nolint staticcheck: the lint:ignore directive is somehow not recognized (and causes an extra warning!)
 		loadedSubjectBytes[string(s)] = struct{}{}
 	}
 	systemCertPool, err := x509.SystemCertPool()
 	require.NoError(t, err)
-	for _, s := range systemCertPool.Subjects() {
+	// lint:ignore SA1019 Receiving no data for system roots is acceptable.
+	for _, s := range systemCertPool.Subjects() { //nolint staticcheck: the lint:ignore directive is somehow not recognized (and causes an extra warning!)
 		_, ok := loadedSubjectBytes[string(s)]
 		assert.True(t, ok)
 	}
-	// RootCAs include our certificates
+
+	// RootCAs include our certificates.
+	// We could possibly test without .Subjects() this by validating certificates
+	// signed by our test CAs.
 	loadedSubjectCNs := map[string]struct{}{}
-	for _, s := range tlsc.RootCAs.Subjects() {
+	// lint:ignore SA1019 We only care about non-system roots here.
+	for _, s := range tlsc.RootCAs.Subjects() { //nolint staticcheck: the lint:ignore directive is somehow not recognized (and causes an extra warning!)
 		subjectRDN := pkix.RDNSequence{}
 		rest, err := asn1.Unmarshal(s, &subjectRDN)
 		require.NoError(t, err)