pkg/cri/(server|sbserver): criService.getTLSConfig() add TODO to verify nolint

thaJeztah · thaJeztah · commit d21572513631 · 2022-10-12T14:40:11.000+02:00
This `//nolint` was added in containerd/cri@f5c7ac9 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 golang/go@eb93c68 (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 <github@gone.nl>
diff --git 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
@@ -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 != "" {

Original file line number	Diff line number	Diff line change
`@@ -318,7 +318,7 @@ func (c criService) getTLSConfig(registryTLSConfig criconfig.TLSConfig) (tls.C`
`318`	`318`	`if len(cert.Certificate) != 0 {`
`319`	`319`	`tlsConfig.Certificates = []tls.Certificate{cert}`
`320`	`320`	`}`
`321`		`- tlsConfig.BuildNameToCertificate() // nolint:staticcheck`
	`321`	`+ tlsConfig.BuildNameToCertificate() //nolint:staticcheck // TODO(thaJeztah): verify if we should ignore the deprecation; see https://github.com/containerd/containerd/pull/7349/files#r990644833`
`322`	`322`	`}`
`323`	`323`
`324`	`324`	`if registryTLSConfig.CAFile != "" {`