From d215725136312ec652dd7ac6b0b2868d70887f0b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 8 Oct 2022 15:53:22 +0200 Subject: [PATCH] pkg/cri/(server|sbserver): criService.getTLSConfig() add TODO to verify nolint This `//nolint` was added in https://github.com/containerd/cri/commit/f5c7ac92724405806eb4e330ecab8f4350601089 to suppress warnings about the `NameToCertificate` function being deprecated: // Deprecated: NameToCertificate only allows associating a single certificate // with a given name. Leave that field nil to let the library select the first // compatible chain from Certificates. Looking at that, it was deprecated in Go 1.14 through https://github.com/golang/go/commit/eb93c684d40de4924fc0664d7d9e98a84d5a100b (https://go-review.googlesource.com/c/go/+/205059), which describes: crypto/tls: select only compatible chains from Certificates Now that we have a full implementation of the logic to check certificate compatibility, we can let applications just list multiple chains in Certificates (for example, an RSA and an ECDSA one) and choose the most appropriate automatically. NameToCertificate only maps each name to one chain, so simply deprecate it, and while at it simplify its implementation by not stripping trailing dots from the SNI (which is specified not to have any, see RFC 6066, Section 3) and by not supporting multi-level wildcards, which are not a thing in the WebPKI (and in crypto/x509). We should at least have a comment describing why we are ignoring this, but preferably review whether we should still use it. Signed-off-by: Sebastiaan van Stijn --- pkg/cri/sbserver/image_pull.go | 2 +- pkg/cri/server/image_pull.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cri/sbserver/image_pull.go b/pkg/cri/sbserver/image_pull.go index 148ba3487b35..b3579b421fd1 100644 --- a/pkg/cri/sbserver/image_pull.go +++ b/pkg/cri/sbserver/image_pull.go @@ -318,7 +318,7 @@ func (c *criService) getTLSConfig(registryTLSConfig criconfig.TLSConfig) (*tls.C if len(cert.Certificate) != 0 { tlsConfig.Certificates = []tls.Certificate{cert} } - tlsConfig.BuildNameToCertificate() // nolint:staticcheck + tlsConfig.BuildNameToCertificate() //nolint:staticcheck // TODO(thaJeztah): verify if we should ignore the deprecation; see https://github.com/containerd/containerd/pull/7349/files#r990644833 } if registryTLSConfig.CAFile != "" { diff --git a/pkg/cri/server/image_pull.go b/pkg/cri/server/image_pull.go index e6af29e8a5b3..fcda1ef33e2e 100644 --- a/pkg/cri/server/image_pull.go +++ b/pkg/cri/server/image_pull.go @@ -318,7 +318,7 @@ func (c *criService) getTLSConfig(registryTLSConfig criconfig.TLSConfig) (*tls.C if len(cert.Certificate) != 0 { tlsConfig.Certificates = []tls.Certificate{cert} } - tlsConfig.BuildNameToCertificate() // nolint:staticcheck + tlsConfig.BuildNameToCertificate() //nolint:staticcheck // TODO(thaJeztah): verify if we should ignore the deprecation; see https://github.com/containerd/containerd/pull/7349/files#r990644833 } if registryTLSConfig.CAFile != "" {