Adding TLS configuration for fn-runner image registries #139
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Catalin-Stratulat-Ericsson The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test presubmit-nephio-go-test |
Catalin-Stratulat-Ericsson
requested review from
kispaljr,
efiacor,
liamfallon,
kushnaidu and
JamesMcDermott
| @@ -28,6 +31,7 @@ import ( | |||
|
|
|||
| "github.com/google/go-containerregistry/pkg/authn" | |||
| "github.com/google/go-containerregistry/pkg/name" | |||
| v1 "github.com/google/go-containerregistry/pkg/v1" | |||
There was a problem hiding this comment.
Could this have a more distinctive alias? Something like
| v1 "github.com/google/go-containerregistry/pkg/v1" | |
| containerregistry "github.com/google/go-containerregistry/pkg/v1" |
| @@ -597,9 +619,70 @@ func (pm *podManager) getCustomAuth(ref name.Reference, registryAuthSecretPath s | |||
| return authn.FromConfig(dockerConfig.Auths[ref.Context().RegistryStr()]), nil | |||
| } | |||
|
|
|||
| func loadTLSConfig(caCertPath string) (*tls.Config, error) { | |||
There was a problem hiding this comment.
Consider moving these new methods to be more in the order they're used in - so:
getImageMetadatagetImageloadTLSConfigcreateTransport
| // Append the CA certificate to the system pool | ||
| caCertPool := x509.NewCertPool() | ||
| if !caCertPool.AppendCertsFromPEM(caCert) { | ||
| return nil, err |
There was a problem hiding this comment.
Won't err still be nil in this case? Should we create a new error to return instead?
| return nil, err | ||
| } | ||
| if _, err := os.Stat(filepath.Join(tlsSecretPath, "ca.crt")); os.IsNotExist(err) { | ||
| klog.Error(err) |
There was a problem hiding this comment.
Should this log be inside the check-for-ca.pem if block, to indicate that we couldn't find a ca file with either extension?
(or perhaps make this one a Warn and add an Error in the .pem check?)
| _, err := os.Stat(tlsSecretPath) | ||
| if os.IsNotExist(err) { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Combined check syntax?
| _, err := os.Stat(tlsSecretPath) | |
| if os.IsNotExist(err) { | |
| return nil, err | |
| } | |
| if _, err := os.Stat(tlsSecretPath); os.IsNotExist(err) { | |
| return nil, err | |
| } |
Tackles issue #819.
Adds the ability to utilize a user's TLS certificates in the function runner for interacting with private TLS registries.
Documentation for how to configure the function runner in this way can be found PR#183.