Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding TLS configuration for fn-runner image registries #139

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+98 −13
Open

Adding TLS configuration for fn-runner image registries #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
12281c7
added tls configuration for fn-runner image registries
Catalin-Stratulat-Ericsson Nov 12, 2024
60c74a0
removed commented out variable
Catalin-Stratulat-Ericsson Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 94 additions & 11 deletions func/internal/podevaluator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ package internal

import (
"context"
"crypto/tls"
"crypto/x509"
"encoding/json"
"fmt"
"net"
"net/http"
"os"
"path/filepath"
"strconv"
Expand All @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this have a more distinctive alias? Something like

Suggested change
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"
Expand Down Expand Up @@ -71,7 +75,20 @@ 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,
) (Evaluator, error) {

restCfg, err := config.GetConfig()
if err != nil {
return nil, fmt.Errorf("failed to get rest config: %w", err)
Expand Down Expand Up @@ -105,6 +122,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{},
Expand Down Expand Up @@ -177,6 +196,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.
Expand Down Expand Up @@ -245,7 +267,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)
})

Expand Down Expand Up @@ -313,7 +335,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)
Expand Down Expand Up @@ -445,9 +467,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
}
Expand Down Expand Up @@ -539,7 +561,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))
Expand Down Expand Up @@ -569,7 +591,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.
Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving these new methods to be more in the order they're used in - so:

  • getImageMetadata
  • getImage
  • loadTLSConfig
  • createTransport

// 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't err still be nil in this case? Should we create a new error to return instead?

}
// 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
}
Comment on lines +650 to +653
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combined check syntax?

Suggested change
_, err := os.Stat(tlsSecretPath)
if os.IsNotExist(err) {
return nil, 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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?)

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
}
Expand All @@ -626,14 +709,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)
}
Expand Down
2 changes: 1 addition & 1 deletion func/internal/podevaluator_podmanager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 3 additions & 1 deletion func/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down Expand Up @@ -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)
}
Expand Down
Loading