From 12281c71c7d33ff9fe32a000b82f26c071e914c2 Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Tue, 12 Nov 2024 15:40:56 +0000 Subject: [PATCH 1/8] added tls configuration for fn-runner image registries --- func/internal/podevaluator.go | 106 ++++++++++++++++-- func/internal/podevaluator_podmanager_test.go | 2 +- func/server/server.go | 4 +- 3 files changed, 99 insertions(+), 13 deletions(-) diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index 43a8d773..80939dce 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -16,9 +16,12 @@ package internal import ( "context" + "crypto/tls" + "crypto/x509" "encoding/json" "fmt" "net" + "net/http" "os" "path/filepath" "strconv" @@ -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" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/nephio-project/porch/func/evaluator" util "github.com/nephio-project/porch/pkg/util" @@ -71,7 +75,21 @@ type podEvaluator struct { var _ Evaluator = &podEvaluator{} -func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Duration, podTTLConfig string, functionPodTemplateName string, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string) (Evaluator, error) { +func NewPodEvaluator( + namespace, + wrapperServerImage string, + interval, + ttl time.Duration, + podTTLConfig string, + functionPodTemplateName string, + enablePrivateRegistries bool, + registryAuthSecretPath string, + registryAuthSecretName string, + enableTlsRegistries bool, + tlsSecretPath string, + // tlsSecretName string +) (Evaluator, error) { + restCfg, err := config.GetConfig() if err != nil { return nil, fmt.Errorf("failed to get rest config: %w", err) @@ -105,6 +123,8 @@ func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Du enablePrivateRegistries: enablePrivateRegistries, registryAuthSecretPath: registryAuthSecretPath, registryAuthSecretName: registryAuthSecretName, + enableTlsRegistries: enableTlsRegistries, + tlsSecretPath: tlsSecretPath, requestCh: reqCh, podReadyCh: readyCh, cache: map[string]*podAndGRPCClient{}, @@ -177,6 +197,9 @@ type podCacheManager struct { registryAuthSecretPath string registryAuthSecretName string + enableTlsRegistries bool + tlsSecretPath string + // requestCh is a receive-only channel to receive requestCh <-chan *clientConnRequest // podReadyCh is a channel to receive the information when a pod is ready. @@ -245,7 +268,7 @@ func (pcm *podCacheManager) warmupCache(podTTLConfig string) error { // We invoke the function with useGenerateName=false so that the pod name is fixed, // since we want to ensure only one pod is created for each function. - pcm.podManager.getFuncEvalPodClient(ctx, fnImage, ttl, false, pcm.enablePrivateRegistries, pcm.registryAuthSecretPath, pcm.registryAuthSecretName) + pcm.podManager.getFuncEvalPodClient(ctx, fnImage, ttl, false, pcm.enablePrivateRegistries, pcm.registryAuthSecretPath, pcm.registryAuthSecretName, pcm.enableTlsRegistries, pcm.tlsSecretPath) klog.Infof("preloaded pod cache for function %v", fnImage) }) @@ -313,7 +336,7 @@ func (pcm *podCacheManager) podCacheManager() { pcm.waitlists[req.image] = append(list, req.grpcClientCh) // We invoke the function with useGenerateName=true to avoid potential name collision, since if pod foo is // being deleted and we can't use the same name. - go pcm.podManager.getFuncEvalPodClient(context.Background(), req.image, pcm.podTTL, true, pcm.enablePrivateRegistries, pcm.registryAuthSecretPath, pcm.registryAuthSecretName) + go pcm.podManager.getFuncEvalPodClient(context.Background(), req.image, pcm.podTTL, true, pcm.enablePrivateRegistries, pcm.registryAuthSecretPath, pcm.registryAuthSecretName, pcm.enableTlsRegistries, pcm.tlsSecretPath) case resp := <-pcm.podReadyCh: if resp.err != nil { klog.Warningf("received error from the pod manager: %v", resp.err) @@ -445,9 +468,9 @@ type digestAndEntrypoint struct { // time-to-live period for the pod. If useGenerateName is false, it will try to // create a pod with a fixed name. Otherwise, it will create a pod and let the // apiserver to generate the name from a template. -func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string) { +func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string, enableTlsRegistries bool, tlsSecretPath string) { c, err := func() (*podAndGRPCClient, error) { - podKey, err := pm.retrieveOrCreatePod(ctx, image, ttl, useGenerateName, enablePrivateRegistries, registryAuthSecretPath, registryAuthSecretName) + podKey, err := pm.retrieveOrCreatePod(ctx, image, ttl, useGenerateName, enablePrivateRegistries, registryAuthSecretPath, registryAuthSecretName, enableTlsRegistries, tlsSecretPath) if err != nil { return nil, err } @@ -539,7 +562,7 @@ type DockerConfig struct { } // imageDigestAndEntrypoint gets the entrypoint of a container image by looking at its metadata. -func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string) (*digestAndEntrypoint, error) { +func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string, enableTlsRegistries bool, tlsSecretPath string) (*digestAndEntrypoint, error) { start := time.Now() defer func() { klog.Infof("getting image metadata for %v took %v", image, time.Since(start)) @@ -569,7 +592,7 @@ func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string } } - return pm.getImageMetadata(ctx, ref, auth, image) + return pm.getImageMetadata(ctx, ref, auth, image, enablePrivateRegistries, enableTlsRegistries, tlsSecretPath) } // ensureCustomAuthSecret ensures that, if an image from a custom registry is requested, the appropriate credentials are passed into a secret for function pods to use when pulling. If the secret does not already exist, it is created. @@ -597,9 +620,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) { + // Read the CA certificate file + caCert, err := os.ReadFile(caCertPath) + if err != nil { + return nil, err + } + // Append the CA certificate to the system pool + caCertPool := x509.NewCertPool() + if !caCertPool.AppendCertsFromPEM(caCert) { + return nil, err + } + // Create a tls.Config with the CA pool + tlsConfig := &tls.Config{ + RootCAs: caCertPool, + } + return tlsConfig, nil +} + +func createTransport(tlsConfig *tls.Config) *http.Transport { + return &http.Transport{ + TLSClientConfig: tlsConfig, + } +} + +func getImage(ctx context.Context, ref name.Reference, auth authn.Authenticator, image string, enablePrivateRegistries bool, enableTlsRegistries bool, tlsSecretPath string) (v1.Image, error) { + if enablePrivateRegistries && !strings.HasPrefix(image, defaultRegistry) && enableTlsRegistries { + tlsFile := "ca.crt" + // Check if mounted secret location contains CA file. + _, err := os.Stat(tlsSecretPath) + if os.IsNotExist(err) { + return nil, err + } + if _, err := os.Stat(filepath.Join(tlsSecretPath, "ca.crt")); os.IsNotExist(err) { + klog.Error(err) + if _, err := os.Stat(filepath.Join(tlsSecretPath, "ca.pem")); os.IsNotExist(err) { + return nil, err + } + tlsFile = "ca.pem" + } + // Load the custom TLS configuration + tlsConfig, err := loadTLSConfig(filepath.Join(tlsSecretPath, tlsFile)) + if err != nil { + return nil, err + } + // Create a custom HTTP transport + transport := createTransport(tlsConfig) + + // Attempt image pull with TLS + img, tlsErr := remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx), remote.WithTransport(transport)) + if tlsErr != nil { + // Attempt without TLS + klog.Errorf("Pulling image %s with the provided TLS Cert has failed with error %v", image, tlsErr) + klog.Infof("Attempting image pull without TLS") + return remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx)) + } + return img, tlsErr + } else { + return remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx)) + } +} + // getImageMetadata retrieves the image digest and entrypoint. -func (pm *podManager) getImageMetadata(ctx context.Context, ref name.Reference, auth authn.Authenticator, image string) (*digestAndEntrypoint, error) { - img, err := remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx)) +func (pm *podManager) getImageMetadata(ctx context.Context, ref name.Reference, auth authn.Authenticator, image string, enablePrivateRegistries bool, enableTlsRegistries bool, tlsSecretPath string) (*digestAndEntrypoint, error) { + img, err := getImage(ctx, ref, auth, image, enablePrivateRegistries, enableTlsRegistries, tlsSecretPath) if err != nil { return nil, err } @@ -626,14 +710,14 @@ func (pm *podManager) getImageMetadata(ctx context.Context, ref name.Reference, } // retrieveOrCreatePod retrieves or creates a pod for an image. -func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string) (client.ObjectKey, error) { +func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string, enableTlsRegistries bool, tlsSecretPath string) (client.ObjectKey, error) { var de *digestAndEntrypoint var replacePod bool var currentPod *corev1.Pod var err error val, found := pm.imageMetadataCache.Load(image) if !found { - de, err = pm.imageDigestAndEntrypoint(ctx, image, enablePrivateRegistries, registryAuthSecretPath, registryAuthSecretName) + de, err = pm.imageDigestAndEntrypoint(ctx, image, enablePrivateRegistries, registryAuthSecretPath, registryAuthSecretName, enableTlsRegistries, tlsSecretPath) if err != nil { return client.ObjectKey{}, fmt.Errorf("unable to get the entrypoint for %v: %w", image, err) } diff --git a/func/internal/podevaluator_podmanager_test.go b/func/internal/podevaluator_podmanager_test.go index e51116ca..f85724b2 100644 --- a/func/internal/podevaluator_podmanager_test.go +++ b/func/internal/podevaluator_podmanager_test.go @@ -644,7 +644,7 @@ func TestPodManager(t *testing.T) { fakeServer.evalFunc = tt.evalFunc //Execute the function under test - go pm.getFuncEvalPodClient(ctx, tt.functionImage, time.Hour, tt.useGenerateName, false, "", "auth-secret") + go pm.getFuncEvalPodClient(ctx, tt.functionImage, time.Hour, tt.useGenerateName, false, "/var/tmp/config-secret/.dockerconfigjson", "auth-secret", false, "/var/tmp/tls-secret/") if tt.podPatch != nil { go func() { diff --git a/func/server/server.go b/func/server/server.go index a0a199c0..eebaf4f7 100644 --- a/func/server/server.go +++ b/func/server/server.go @@ -44,6 +44,8 @@ var ( enablePrivateRegistries = flag.Bool("enable-private-registry", false, "if true enables the use of private registries and their authentication") registryAuthSecretPath = flag.String("registry-auth-secret-path", "/var/tmp/config-secret/.dockerconfigjson", "The path of the secret used in custom registry authentication") registryAuthSecretName = flag.String("registry-auth-secret-name", "auth-secret", "The name of the secret used in custom registry authentication") + enableTlsRegistries = flag.Bool("enable-tls-registry", false, "if true enables tls configuration of registries") + tlsSecretPath = flag.String("tls-secret-path", "/var/tmp/tls-secret/", "The path of the secret used in tls configuration") podCacheConfig = flag.String("pod-cache-config", "/pod-cache-config/pod-cache-config.yaml", "Path to the pod cache config file. The file is map of function name to TTL.") podNamespace = flag.String("pod-namespace", "porch-fn-system", "Namespace to run KRM functions pods.") podTTL = flag.Duration("pod-ttl", 30*time.Minute, "TTL for pods before GC.") @@ -92,7 +94,7 @@ func run() error { if wrapperServerImage == "" { return fmt.Errorf("environment variable %v must be set to use pod function evaluator runtime", wrapperServerImageEnv) } - podEval, err := internal.NewPodEvaluator(*podNamespace, wrapperServerImage, *scanInterval, *podTTL, *podCacheConfig, *functionPodTemplateName, *enablePrivateRegistries, *registryAuthSecretPath, *registryAuthSecretName) + podEval, err := internal.NewPodEvaluator(*podNamespace, wrapperServerImage, *scanInterval, *podTTL, *podCacheConfig, *functionPodTemplateName, *enablePrivateRegistries, *registryAuthSecretPath, *registryAuthSecretName, *enableTlsRegistries, *tlsSecretPath) if err != nil { return fmt.Errorf("failed to initialize pod evaluator: %w", err) } From 60c74a026c0c98a6533baeca8adb1d97d28a5cf4 Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Wed, 13 Nov 2024 10:47:29 +0000 Subject: [PATCH 2/8] removed commented out variable --- func/internal/podevaluator.go | 1 - 1 file changed, 1 deletion(-) diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index 80939dce..c91d0078 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -87,7 +87,6 @@ func NewPodEvaluator( registryAuthSecretName string, enableTlsRegistries bool, tlsSecretPath string, - // tlsSecretName string ) (Evaluator, error) { restCfg, err := config.GetConfig() From 5005e1f6b8cb3075a1641d5b6350babe22864120 Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Mon, 18 Nov 2024 15:18:46 +0000 Subject: [PATCH 3/8] ammended comments regarding clarity & errors --- func/internal/podevaluator.go | 83 +++++++++++++++++------------------ 1 file changed, 41 insertions(+), 42 deletions(-) diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index c91d0078..58e34d9a 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -19,6 +19,7 @@ import ( "crypto/tls" "crypto/x509" "encoding/json" + goerrors "errors" "fmt" "net" "net/http" @@ -31,7 +32,7 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" - v1 "github.com/google/go-containerregistry/pkg/v1" + containerregistry "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/nephio-project/porch/func/evaluator" util "github.com/nephio-project/porch/pkg/util" @@ -619,42 +620,44 @@ 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) { - // Read the CA certificate file - caCert, err := os.ReadFile(caCertPath) +// getImageMetadata retrieves the image digest and entrypoint. +func (pm *podManager) getImageMetadata(ctx context.Context, ref name.Reference, auth authn.Authenticator, image string, enablePrivateRegistries bool, enableTlsRegistries bool, tlsSecretPath string) (*digestAndEntrypoint, error) { + img, err := getImage(ctx, ref, auth, image, enablePrivateRegistries, enableTlsRegistries, tlsSecretPath) if err != nil { return nil, err } - // Append the CA certificate to the system pool - caCertPool := x509.NewCertPool() - if !caCertPool.AppendCertsFromPEM(caCert) { + hash, err := img.Digest() + if err != nil { return nil, err } - // Create a tls.Config with the CA pool - tlsConfig := &tls.Config{ - RootCAs: caCertPool, + configFile, err := img.ConfigFile() + if err != nil { + return nil, err } - return tlsConfig, nil -} - -func createTransport(tlsConfig *tls.Config) *http.Transport { - return &http.Transport{ - TLSClientConfig: tlsConfig, + // TODO: to handle all scenario, we should follow https://docs.docker.com/engine/reference/builder/#understand-how-cmd-and-entrypoint-interact. + cfg := configFile.Config + entrypoint := cfg.Entrypoint + if len(entrypoint) == 0 { + entrypoint = cfg.Cmd + } + de := &digestAndEntrypoint{ + digest: hash.Hex, + entrypoint: entrypoint, } + pm.imageMetadataCache.Store(image, de) + return de, nil } -func getImage(ctx context.Context, ref name.Reference, auth authn.Authenticator, image string, enablePrivateRegistries bool, enableTlsRegistries bool, tlsSecretPath string) (v1.Image, error) { +func getImage(ctx context.Context, ref name.Reference, auth authn.Authenticator, image string, enablePrivateRegistries bool, enableTlsRegistries bool, tlsSecretPath string) (containerregistry.Image, error) { if enablePrivateRegistries && !strings.HasPrefix(image, defaultRegistry) && enableTlsRegistries { tlsFile := "ca.crt" // Check if mounted secret location contains CA file. - _, err := os.Stat(tlsSecretPath) - if os.IsNotExist(err) { + if _, err := os.Stat(tlsSecretPath); os.IsNotExist(err) { return nil, err } if _, err := os.Stat(filepath.Join(tlsSecretPath, "ca.crt")); os.IsNotExist(err) { - klog.Error(err) if _, err := os.Stat(filepath.Join(tlsSecretPath, "ca.pem")); os.IsNotExist(err) { - return nil, err + return nil, goerrors.New("failed to find TLS files ca.crt or ca.pem") } tlsFile = "ca.pem" } @@ -663,7 +666,7 @@ func getImage(ctx context.Context, ref name.Reference, auth authn.Authenticator, if err != nil { return nil, err } - // Create a custom HTTP transport + // Create a custom HTTPS transport transport := createTransport(tlsConfig) // Attempt image pull with TLS @@ -680,32 +683,28 @@ func getImage(ctx context.Context, ref name.Reference, auth authn.Authenticator, } } -// getImageMetadata retrieves the image digest and entrypoint. -func (pm *podManager) getImageMetadata(ctx context.Context, ref name.Reference, auth authn.Authenticator, image string, enablePrivateRegistries bool, enableTlsRegistries bool, tlsSecretPath string) (*digestAndEntrypoint, error) { - img, err := getImage(ctx, ref, auth, image, enablePrivateRegistries, enableTlsRegistries, tlsSecretPath) - if err != nil { - return nil, err - } - hash, err := img.Digest() +func loadTLSConfig(caCertPath string) (*tls.Config, error) { + // Read the CA certificate file + caCert, err := os.ReadFile(caCertPath) if err != nil { return nil, err } - configFile, err := img.ConfigFile() - if err != nil { - return nil, err + // Append the CA certificate to the system pool + caCertPool := x509.NewCertPool() + if !caCertPool.AppendCertsFromPEM(caCert) { + return nil, goerrors.New("failed to append certificates from PEM") } - // TODO: to handle all scenario, we should follow https://docs.docker.com/engine/reference/builder/#understand-how-cmd-and-entrypoint-interact. - cfg := configFile.Config - entrypoint := cfg.Entrypoint - if len(entrypoint) == 0 { - entrypoint = cfg.Cmd + // Create a tls.Config with the CA pool + tlsConfig := &tls.Config{ + RootCAs: caCertPool, } - de := &digestAndEntrypoint{ - digest: hash.Hex, - entrypoint: entrypoint, + return tlsConfig, nil +} + +func createTransport(tlsConfig *tls.Config) *http.Transport { + return &http.Transport{ + TLSClientConfig: tlsConfig, } - pm.imageMetadataCache.Store(image, de) - return de, nil } // retrieveOrCreatePod retrieves or creates a pod for an image. From e12bafadbd66983913d46e2a75f595bb659bdaef Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Mon, 18 Nov 2024 15:41:08 +0000 Subject: [PATCH 4/8] changed argument name and description to clarify logic --- func/internal/podevaluator.go | 52 +++++++++++++++++------------------ func/server/server.go | 30 ++++++++++---------- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index 58e34d9a..dee99c46 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -86,7 +86,7 @@ func NewPodEvaluator( enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string, - enableTlsRegistries bool, + enablePrivateRegistriesTls bool, tlsSecretPath string, ) (Evaluator, error) { @@ -118,17 +118,17 @@ func NewPodEvaluator( pe := &podEvaluator{ requestCh: reqCh, podCacheManager: &podCacheManager{ - gcScanInternal: interval, - podTTL: ttl, - enablePrivateRegistries: enablePrivateRegistries, - registryAuthSecretPath: registryAuthSecretPath, - registryAuthSecretName: registryAuthSecretName, - enableTlsRegistries: enableTlsRegistries, - tlsSecretPath: tlsSecretPath, - requestCh: reqCh, - podReadyCh: readyCh, - cache: map[string]*podAndGRPCClient{}, - waitlists: map[string][]chan<- *clientConnAndError{}, + gcScanInternal: interval, + podTTL: ttl, + enablePrivateRegistries: enablePrivateRegistries, + registryAuthSecretPath: registryAuthSecretPath, + registryAuthSecretName: registryAuthSecretName, + enablePrivateRegistriesTls: enablePrivateRegistriesTls, + tlsSecretPath: tlsSecretPath, + requestCh: reqCh, + podReadyCh: readyCh, + cache: map[string]*podAndGRPCClient{}, + waitlists: map[string][]chan<- *clientConnAndError{}, podManager: &podManager{ kubeClient: cl, @@ -197,8 +197,8 @@ type podCacheManager struct { registryAuthSecretPath string registryAuthSecretName string - enableTlsRegistries bool - tlsSecretPath string + enablePrivateRegistriesTls bool + tlsSecretPath string // requestCh is a receive-only channel to receive requestCh <-chan *clientConnRequest @@ -268,7 +268,7 @@ func (pcm *podCacheManager) warmupCache(podTTLConfig string) error { // We invoke the function with useGenerateName=false so that the pod name is fixed, // since we want to ensure only one pod is created for each function. - pcm.podManager.getFuncEvalPodClient(ctx, fnImage, ttl, false, pcm.enablePrivateRegistries, pcm.registryAuthSecretPath, pcm.registryAuthSecretName, pcm.enableTlsRegistries, pcm.tlsSecretPath) + pcm.podManager.getFuncEvalPodClient(ctx, fnImage, ttl, false, pcm.enablePrivateRegistries, pcm.registryAuthSecretPath, pcm.registryAuthSecretName, pcm.enablePrivateRegistriesTls, pcm.tlsSecretPath) klog.Infof("preloaded pod cache for function %v", fnImage) }) @@ -336,7 +336,7 @@ func (pcm *podCacheManager) podCacheManager() { pcm.waitlists[req.image] = append(list, req.grpcClientCh) // We invoke the function with useGenerateName=true to avoid potential name collision, since if pod foo is // being deleted and we can't use the same name. - go pcm.podManager.getFuncEvalPodClient(context.Background(), req.image, pcm.podTTL, true, pcm.enablePrivateRegistries, pcm.registryAuthSecretPath, pcm.registryAuthSecretName, pcm.enableTlsRegistries, pcm.tlsSecretPath) + go pcm.podManager.getFuncEvalPodClient(context.Background(), req.image, pcm.podTTL, true, pcm.enablePrivateRegistries, pcm.registryAuthSecretPath, pcm.registryAuthSecretName, pcm.enablePrivateRegistriesTls, pcm.tlsSecretPath) case resp := <-pcm.podReadyCh: if resp.err != nil { klog.Warningf("received error from the pod manager: %v", resp.err) @@ -468,9 +468,9 @@ type digestAndEntrypoint struct { // time-to-live period for the pod. If useGenerateName is false, it will try to // create a pod with a fixed name. Otherwise, it will create a pod and let the // apiserver to generate the name from a template. -func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string, enableTlsRegistries bool, tlsSecretPath string) { +func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string, enablePrivateRegistriesTls bool, tlsSecretPath string) { c, err := func() (*podAndGRPCClient, error) { - podKey, err := pm.retrieveOrCreatePod(ctx, image, ttl, useGenerateName, enablePrivateRegistries, registryAuthSecretPath, registryAuthSecretName, enableTlsRegistries, tlsSecretPath) + podKey, err := pm.retrieveOrCreatePod(ctx, image, ttl, useGenerateName, enablePrivateRegistries, registryAuthSecretPath, registryAuthSecretName, enablePrivateRegistriesTls, tlsSecretPath) if err != nil { return nil, err } @@ -562,7 +562,7 @@ type DockerConfig struct { } // imageDigestAndEntrypoint gets the entrypoint of a container image by looking at its metadata. -func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string, enableTlsRegistries bool, tlsSecretPath string) (*digestAndEntrypoint, error) { +func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string, enablePrivateRegistriesTls bool, tlsSecretPath string) (*digestAndEntrypoint, error) { start := time.Now() defer func() { klog.Infof("getting image metadata for %v took %v", image, time.Since(start)) @@ -592,7 +592,7 @@ func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string } } - return pm.getImageMetadata(ctx, ref, auth, image, enablePrivateRegistries, enableTlsRegistries, tlsSecretPath) + return pm.getImageMetadata(ctx, ref, auth, image, enablePrivateRegistries, enablePrivateRegistriesTls, tlsSecretPath) } // ensureCustomAuthSecret ensures that, if an image from a custom registry is requested, the appropriate credentials are passed into a secret for function pods to use when pulling. If the secret does not already exist, it is created. @@ -621,8 +621,8 @@ func (pm *podManager) getCustomAuth(ref name.Reference, registryAuthSecretPath s } // getImageMetadata retrieves the image digest and entrypoint. -func (pm *podManager) getImageMetadata(ctx context.Context, ref name.Reference, auth authn.Authenticator, image string, enablePrivateRegistries bool, enableTlsRegistries bool, tlsSecretPath string) (*digestAndEntrypoint, error) { - img, err := getImage(ctx, ref, auth, image, enablePrivateRegistries, enableTlsRegistries, tlsSecretPath) +func (pm *podManager) getImageMetadata(ctx context.Context, ref name.Reference, auth authn.Authenticator, image string, enablePrivateRegistries bool, enablePrivateRegistriesTls bool, tlsSecretPath string) (*digestAndEntrypoint, error) { + img, err := getImage(ctx, ref, auth, image, enablePrivateRegistries, enablePrivateRegistriesTls, tlsSecretPath) if err != nil { return nil, err } @@ -648,8 +648,8 @@ func (pm *podManager) getImageMetadata(ctx context.Context, ref name.Reference, return de, nil } -func getImage(ctx context.Context, ref name.Reference, auth authn.Authenticator, image string, enablePrivateRegistries bool, enableTlsRegistries bool, tlsSecretPath string) (containerregistry.Image, error) { - if enablePrivateRegistries && !strings.HasPrefix(image, defaultRegistry) && enableTlsRegistries { +func getImage(ctx context.Context, ref name.Reference, auth authn.Authenticator, image string, enablePrivateRegistries bool, enablePrivateRegistriesTls bool, tlsSecretPath string) (containerregistry.Image, error) { + if enablePrivateRegistries && !strings.HasPrefix(image, defaultRegistry) && enablePrivateRegistriesTls { tlsFile := "ca.crt" // Check if mounted secret location contains CA file. if _, err := os.Stat(tlsSecretPath); os.IsNotExist(err) { @@ -708,14 +708,14 @@ func createTransport(tlsConfig *tls.Config) *http.Transport { } // retrieveOrCreatePod retrieves or creates a pod for an image. -func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string, enableTlsRegistries bool, tlsSecretPath string) (client.ObjectKey, error) { +func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string, enablePrivateRegistriesTls bool, tlsSecretPath string) (client.ObjectKey, error) { var de *digestAndEntrypoint var replacePod bool var currentPod *corev1.Pod var err error val, found := pm.imageMetadataCache.Load(image) if !found { - de, err = pm.imageDigestAndEntrypoint(ctx, image, enablePrivateRegistries, registryAuthSecretPath, registryAuthSecretName, enableTlsRegistries, tlsSecretPath) + de, err = pm.imageDigestAndEntrypoint(ctx, image, enablePrivateRegistries, registryAuthSecretPath, registryAuthSecretName, enablePrivateRegistriesTls, tlsSecretPath) if err != nil { return client.ObjectKey{}, fmt.Errorf("unable to get the entrypoint for %v: %w", image, err) } diff --git a/func/server/server.go b/func/server/server.go index eebaf4f7..54374b04 100644 --- a/func/server/server.go +++ b/func/server/server.go @@ -38,20 +38,20 @@ const ( ) var ( - port = flag.Int("port", 9445, "The server port") - functions = flag.String("functions", "./functions", "Path to cached functions.") - config = flag.String("config", "./config.yaml", "Path to the config file.") - enablePrivateRegistries = flag.Bool("enable-private-registry", false, "if true enables the use of private registries and their authentication") - registryAuthSecretPath = flag.String("registry-auth-secret-path", "/var/tmp/config-secret/.dockerconfigjson", "The path of the secret used in custom registry authentication") - registryAuthSecretName = flag.String("registry-auth-secret-name", "auth-secret", "The name of the secret used in custom registry authentication") - enableTlsRegistries = flag.Bool("enable-tls-registry", false, "if true enables tls configuration of registries") - tlsSecretPath = flag.String("tls-secret-path", "/var/tmp/tls-secret/", "The path of the secret used in tls configuration") - podCacheConfig = flag.String("pod-cache-config", "/pod-cache-config/pod-cache-config.yaml", "Path to the pod cache config file. The file is map of function name to TTL.") - podNamespace = flag.String("pod-namespace", "porch-fn-system", "Namespace to run KRM functions pods.") - podTTL = flag.Duration("pod-ttl", 30*time.Minute, "TTL for pods before GC.") - scanInterval = flag.Duration("scan-interval", time.Minute, "The interval of GC between scans.") - disableRuntimes = flag.String("disable-runtimes", "", fmt.Sprintf("The runtime(s) to disable. Multiple runtimes should separated by `,`. Available runtimes: `%v`, `%v`.", execRuntime, podRuntime)) - functionPodTemplateName = flag.String("function-pod-template", "", "Configmap that contains a pod specification") + port = flag.Int("port", 9445, "The server port") + functions = flag.String("functions", "./functions", "Path to cached functions.") + config = flag.String("config", "./config.yaml", "Path to the config file.") + enablePrivateRegistries = flag.Bool("enable-private-registry", false, "if true enables the use of private registries and their authentication") + registryAuthSecretPath = flag.String("registry-auth-secret-path", "/var/tmp/config-secret/.dockerconfigjson", "The path of the secret used in custom registry authentication") + registryAuthSecretName = flag.String("registry-auth-secret-name", "auth-secret", "The name of the secret used in custom registry authentication") + enablePrivateRegistriesTls = flag.Bool("enable-private-registry-tls", false, "if enabled, will prioritize use of user provided TLS secret when accessing registries") + tlsSecretPath = flag.String("tls-secret-path", "/var/tmp/tls-secret/", "The path of the secret used in tls configuration") + podCacheConfig = flag.String("pod-cache-config", "/pod-cache-config/pod-cache-config.yaml", "Path to the pod cache config file. The file is map of function name to TTL.") + podNamespace = flag.String("pod-namespace", "porch-fn-system", "Namespace to run KRM functions pods.") + podTTL = flag.Duration("pod-ttl", 30*time.Minute, "TTL for pods before GC.") + scanInterval = flag.Duration("scan-interval", time.Minute, "The interval of GC between scans.") + disableRuntimes = flag.String("disable-runtimes", "", fmt.Sprintf("The runtime(s) to disable. Multiple runtimes should separated by `,`. Available runtimes: `%v`, `%v`.", execRuntime, podRuntime)) + functionPodTemplateName = flag.String("function-pod-template", "", "Configmap that contains a pod specification") ) func main() { @@ -94,7 +94,7 @@ func run() error { if wrapperServerImage == "" { return fmt.Errorf("environment variable %v must be set to use pod function evaluator runtime", wrapperServerImageEnv) } - podEval, err := internal.NewPodEvaluator(*podNamespace, wrapperServerImage, *scanInterval, *podTTL, *podCacheConfig, *functionPodTemplateName, *enablePrivateRegistries, *registryAuthSecretPath, *registryAuthSecretName, *enableTlsRegistries, *tlsSecretPath) + podEval, err := internal.NewPodEvaluator(*podNamespace, wrapperServerImage, *scanInterval, *podTTL, *podCacheConfig, *functionPodTemplateName, *enablePrivateRegistries, *registryAuthSecretPath, *registryAuthSecretName, *enablePrivateRegistriesTls, *tlsSecretPath) if err != nil { return fmt.Errorf("failed to initialize pod evaluator: %w", err) } From 58be7fd3021da62225d7ab4e2f84cb66dddb6d59 Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Mon, 18 Nov 2024 16:01:14 +0000 Subject: [PATCH 5/8] more argument renames for clarity --- func/server/server.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/func/server/server.go b/func/server/server.go index 54374b04..b722d8ea 100644 --- a/func/server/server.go +++ b/func/server/server.go @@ -41,10 +41,10 @@ var ( port = flag.Int("port", 9445, "The server port") functions = flag.String("functions", "./functions", "Path to cached functions.") config = flag.String("config", "./config.yaml", "Path to the config file.") - enablePrivateRegistries = flag.Bool("enable-private-registry", false, "if true enables the use of private registries and their authentication") - registryAuthSecretPath = flag.String("registry-auth-secret-path", "/var/tmp/config-secret/.dockerconfigjson", "The path of the secret used in custom registry authentication") - registryAuthSecretName = flag.String("registry-auth-secret-name", "auth-secret", "The name of the secret used in custom registry authentication") - enablePrivateRegistriesTls = flag.Bool("enable-private-registry-tls", false, "if enabled, will prioritize use of user provided TLS secret when accessing registries") + enablePrivateRegistries = flag.Bool("enable-private-registries", false, "if true enables the use of private registries and their authentication") + registryAuthSecretPath = flag.String("registry-auth-secret-path", "/var/tmp/config-secret/.dockerconfigjson", "The path of the secret used for authenticating to custom registries") + registryAuthSecretName = flag.String("registry-auth-secret-name", "auth-secret", "The name of the secret used for authenticating to custom registries") + enablePrivateRegistriesTls = flag.Bool("enable-private-registries-tls", false, "if enabled, will prioritize use of user provided TLS secret when accessing registries") tlsSecretPath = flag.String("tls-secret-path", "/var/tmp/tls-secret/", "The path of the secret used in tls configuration") podCacheConfig = flag.String("pod-cache-config", "/pod-cache-config/pod-cache-config.yaml", "Path to the pod cache config file. The file is map of function name to TTL.") podNamespace = flag.String("pod-namespace", "porch-fn-system", "Namespace to run KRM functions pods.") From 4651b6f29ddfba98eacff3b1e73409dd2e61f181 Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Mon, 18 Nov 2024 16:25:44 +0000 Subject: [PATCH 6/8] clear TLS cert file missing error formatting --- func/internal/podevaluator.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index dee99c46..aab0c21c 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -19,7 +19,6 @@ import ( "crypto/tls" "crypto/x509" "encoding/json" - goerrors "errors" "fmt" "net" "net/http" @@ -655,9 +654,9 @@ func getImage(ctx context.Context, ref name.Reference, auth authn.Authenticator, if _, err := os.Stat(tlsSecretPath); os.IsNotExist(err) { return nil, err } - if _, err := os.Stat(filepath.Join(tlsSecretPath, "ca.crt")); os.IsNotExist(err) { - if _, err := os.Stat(filepath.Join(tlsSecretPath, "ca.pem")); os.IsNotExist(err) { - return nil, goerrors.New("failed to find TLS files ca.crt or ca.pem") + if _, errCRT := os.Stat(filepath.Join(tlsSecretPath, "ca.crt")); os.IsNotExist(errCRT) { + if _, errPEM := os.Stat(filepath.Join(tlsSecretPath, "ca.pem")); os.IsNotExist(errPEM) { + return nil, fmt.Errorf("ca.crt not found: %v, and ca.pem also not found: %v", errCRT, errPEM) } tlsFile = "ca.pem" } @@ -692,7 +691,7 @@ func loadTLSConfig(caCertPath string) (*tls.Config, error) { // Append the CA certificate to the system pool caCertPool := x509.NewCertPool() if !caCertPool.AppendCertsFromPEM(caCert) { - return nil, goerrors.New("failed to append certificates from PEM") + return nil, fmt.Errorf("failed to append certificates from PEM") } // Create a tls.Config with the CA pool tlsConfig := &tls.Config{ From 41ba0e39a98217aa0f75623016b3e1aa27c80646 Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Wed, 20 Nov 2024 15:29:11 +0000 Subject: [PATCH 7/8] changed order of getImage function to prioritize default operation first --- func/internal/podevaluator.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index aab0c21c..3e0feced 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -612,7 +612,7 @@ func (pm *podManager) getCustomAuth(ref name.Reference, registryAuthSecretPath s var dockerConfig DockerConfig if err := json.Unmarshal(dockerConfigBytes, &dockerConfig); err != nil { - klog.Errorf("error unmarshalling authentication file %v", err) + klog.Errorf("error unmarshaling authentication file %v", err) return nil, err } @@ -648,7 +648,10 @@ func (pm *podManager) getImageMetadata(ctx context.Context, ref name.Reference, } func getImage(ctx context.Context, ref name.Reference, auth authn.Authenticator, image string, enablePrivateRegistries bool, enablePrivateRegistriesTls bool, tlsSecretPath string) (containerregistry.Image, error) { - if enablePrivateRegistries && !strings.HasPrefix(image, defaultRegistry) && enablePrivateRegistriesTls { + // if private registries or their appropriate tls configuration are disabled in the config we pull image with default operation otherwise try and use their tls cert's + if !enablePrivateRegistries || strings.HasPrefix(image, defaultRegistry) || !enablePrivateRegistriesTls { + return remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx)) + } else { tlsFile := "ca.crt" // Check if mounted secret location contains CA file. if _, err := os.Stat(tlsSecretPath); os.IsNotExist(err) { @@ -668,17 +671,15 @@ func getImage(ctx context.Context, ref name.Reference, auth authn.Authenticator, // Create a custom HTTPS transport transport := createTransport(tlsConfig) - // Attempt image pull with TLS + // Attempt image pull with given custom TLS cert img, tlsErr := remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx), remote.WithTransport(transport)) if tlsErr != nil { - // Attempt without TLS + // Attempt without given custom TLS cert but with default keychain klog.Errorf("Pulling image %s with the provided TLS Cert has failed with error %v", image, tlsErr) - klog.Infof("Attempting image pull without TLS") + klog.Infof("Attempting image pull with default keychain instead of provided TLS Cert") return remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx)) } return img, tlsErr - } else { - return remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx)) } } From 77aec114ac4a032b07849027ece2e6bcd121bcc1 Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Thu, 21 Nov 2024 10:55:42 +0000 Subject: [PATCH 8/8] removed retundant else. as noted by Liam --- func/internal/podevaluator.go | 55 +++++++++++++++++------------------ 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index 3e0feced..e4b7368b 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -651,36 +651,35 @@ func getImage(ctx context.Context, ref name.Reference, auth authn.Authenticator, // if private registries or their appropriate tls configuration are disabled in the config we pull image with default operation otherwise try and use their tls cert's if !enablePrivateRegistries || strings.HasPrefix(image, defaultRegistry) || !enablePrivateRegistriesTls { return remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx)) - } else { - tlsFile := "ca.crt" - // Check if mounted secret location contains CA file. - if _, err := os.Stat(tlsSecretPath); os.IsNotExist(err) { - return nil, err - } - if _, errCRT := os.Stat(filepath.Join(tlsSecretPath, "ca.crt")); os.IsNotExist(errCRT) { - if _, errPEM := os.Stat(filepath.Join(tlsSecretPath, "ca.pem")); os.IsNotExist(errPEM) { - return nil, fmt.Errorf("ca.crt not found: %v, and ca.pem also not found: %v", errCRT, errPEM) - } - tlsFile = "ca.pem" - } - // Load the custom TLS configuration - tlsConfig, err := loadTLSConfig(filepath.Join(tlsSecretPath, tlsFile)) - if err != nil { - return nil, err - } - // Create a custom HTTPS transport - transport := createTransport(tlsConfig) - - // Attempt image pull with given custom TLS cert - img, tlsErr := remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx), remote.WithTransport(transport)) - if tlsErr != nil { - // Attempt without given custom TLS cert but with default keychain - klog.Errorf("Pulling image %s with the provided TLS Cert has failed with error %v", image, tlsErr) - klog.Infof("Attempting image pull with default keychain instead of provided TLS Cert") - return remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx)) + } + tlsFile := "ca.crt" + // Check if mounted secret location contains CA file. + if _, err := os.Stat(tlsSecretPath); os.IsNotExist(err) { + return nil, err + } + if _, errCRT := os.Stat(filepath.Join(tlsSecretPath, "ca.crt")); os.IsNotExist(errCRT) { + if _, errPEM := os.Stat(filepath.Join(tlsSecretPath, "ca.pem")); os.IsNotExist(errPEM) { + return nil, fmt.Errorf("ca.crt not found: %v, and ca.pem also not found: %v", errCRT, errPEM) } - return img, tlsErr + tlsFile = "ca.pem" + } + // Load the custom TLS configuration + tlsConfig, err := loadTLSConfig(filepath.Join(tlsSecretPath, tlsFile)) + if err != nil { + return nil, err + } + // Create a custom HTTPS transport + transport := createTransport(tlsConfig) + + // Attempt image pull with given custom TLS cert + img, tlsErr := remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx), remote.WithTransport(transport)) + if tlsErr != nil { + // Attempt without given custom TLS cert but with default keychain + klog.Errorf("Pulling image %s with the provided TLS Cert has failed with error %v", image, tlsErr) + klog.Infof("Attempting image pull with default keychain instead of provided TLS Cert") + return remote.Image(ref, remote.WithAuth(auth), remote.WithContext(ctx)) } + return img, tlsErr } func loadTLSConfig(caCertPath string) (*tls.Config, error) {