Skip to content

Commit

Permalink
Try to make (make lint) pass with Go 1.18
Browse files Browse the repository at this point in the history
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 golangci/golangci-lint#2374 ;
a local rebuild does not crash, but still fails per the above.)

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
  • Loading branch information
mtrmac committed Mar 15, 2022
1 parent 9d9f162 commit d95221d
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 6 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
35 changes: 30 additions & 5 deletions pkg/tlsclientconfig/tlsclientconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit d95221d

Please sign in to comment.