From 866e36fee2b0347a73d8ad156042f42d6118ebfb Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Mon, 21 Oct 2024 11:51:44 +0100 Subject: [PATCH 1/7] added functionality to pull and use images from private authenticated repositories --- deployments/porch/5-rbac.yaml | 3 + func/internal/podevaluator.go | 148 +++++++++++++++--- func/internal/podevaluator_podmanager_test.go | 2 +- func/server/server.go | 3 +- 4 files changed, 134 insertions(+), 22 deletions(-) diff --git a/deployments/porch/5-rbac.yaml b/deployments/porch/5-rbac.yaml index 8346c83c..f7560f12 100644 --- a/deployments/porch/5-rbac.yaml +++ b/deployments/porch/5-rbac.yaml @@ -68,3 +68,6 @@ rules: - apiGroups: [""] resources: ["pods"] verbs: ["create", "delete", "patch", "get", "watch", "list"] + - apiGroups: [""] + resources: ["secrets"] + verbs: ["get", "create", "delete"] diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index 92a4d89b..e56aba07 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -16,6 +16,7 @@ package internal import ( "context" + "encoding/json" "fmt" "net" "os" @@ -25,7 +26,7 @@ import ( "sync" "time" - "github.com/google/go-containerregistry/pkg/gcrane" + "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/nephio-project/porch/func/evaluator" @@ -57,6 +58,9 @@ const ( fieldManagerName = "krm-function-runner" functionContainerName = "function" defaultManagerNamespace = "porch-system" + defaultRepository = "gcr.io/kpt-fn/" + // perhaps should try and get the name of the dockerconfig secret given by user and match this secret name to that to avoid hard coded value? + customRepoImgPullSecret = "auth-secret" channelBufferSize = 128 ) @@ -69,7 +73,7 @@ type podEvaluator struct { var _ Evaluator = &podEvaluator{} -func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Duration, podTTLConfig string, functionPodTemplateName string) (Evaluator, error) { +func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Duration, podTTLConfig string, functionPodTemplateName string, customRepoAuth string) (Evaluator, error) { restCfg, err := config.GetConfig() if err != nil { return nil, fmt.Errorf("failed to get rest config: %w", err) @@ -100,6 +104,7 @@ func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Du podCacheManager: &podCacheManager{ gcScanInternal: interval, podTTL: ttl, + customRepoAuth: customRepoAuth, requestCh: reqCh, podReadyCh: readyCh, cache: map[string]*podAndGRPCClient{}, @@ -168,6 +173,8 @@ type podCacheManager struct { gcScanInternal time.Duration podTTL time.Duration + customRepoAuth 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. @@ -236,7 +243,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.podManager.getFuncEvalPodClient(ctx, fnImage, ttl, false, pcm.customRepoAuth) klog.Infof("preloaded pod cache for function %v", fnImage) }) @@ -304,7 +311,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) + go pcm.podManager.getFuncEvalPodClient(context.Background(), req.image, pcm.podTTL, true, pcm.customRepoAuth) case resp := <-pcm.podReadyCh: if resp.err != nil { klog.Warningf("received error from the pod manager: %v", resp.err) @@ -436,9 +443,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) { +func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, customRepoAuth string) { c, err := func() (*podAndGRPCClient, error) { - podKey, err := pm.retrieveOrCreatePod(ctx, image, ttl, useGenerateName) + podKey, err := pm.retrieveOrCreatePod(ctx, image, ttl, useGenerateName, customRepoAuth) if err != nil { return nil, err } @@ -466,18 +473,120 @@ func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, tt } } +func (pm *podManager) InspectOrCreateSecret(ctx context.Context, customRepoAuth string) error { + secret := &corev1.Secret{} + // using pod manager client since this secret is only related to these pods and nothing else + err := pm.kubeClient.Get(context.Background(), client.ObjectKey{ + Name: "auth-secret", + Namespace: pm.namespace, + }, secret) + if err != nil { + if client.IgnoreNotFound(err) != nil { + // Error other than "not found" occurred + return err + } + klog.Infof("Secret for private repo pods does not exist and is required.\nGenerating Secret Now") + dockerConfigBytes, err := os.ReadFile(customRepoAuth) + if err != nil { + return err + } + // Secret does not exist, create it + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-secret", + Namespace: pm.namespace, + }, + Data: map[string][]byte{ + ".dockerconfigjson": dockerConfigBytes, + }, + Type: corev1.SecretTypeDockerConfigJson, + } + err = pm.kubeClient.Create(ctx, secret) + if err != nil { + return err + } + + klog.Infof("Secret created successfully") + } else { + klog.Infof("Secret already exists") + } + return nil +} + +// DockerConfig represents the structure of Docker config.json +type DockerConfig struct { + Auths map[string]authn.AuthConfig `json:"auths"` +} + // imageDigestAndEntrypoint gets the entrypoint of a container image by looking at its metadata. -func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string) (*digestAndEntrypoint, error) { +func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string, customRepoAuth string) (*digestAndEntrypoint, error) { start := time.Now() - defer func() { - klog.Infof("getting image metadata for %v took %v", image, time.Since(start)) - }() - var entrypoint []string + defer klog.Infof("getting image metadata for %v took %v", image, time.Since(start)) + ref, err := name.ParseReference(image) if err != nil { + klog.Errorf("we got an error parsing the ref %v", err) return nil, err } - img, err := remote.Image(ref, remote.WithAuthFromKeychain(gcrane.Keychain), remote.WithContext(ctx)) + + var auth authn.Authenticator + if customRepoAuth != "" && !strings.HasPrefix(image, defaultRepository) { + if err := pm.handleCustomAuth(ctx, customRepoAuth); err != nil { + return nil, err + } + + auth, err = pm.getCustomAuth(ref, customRepoAuth) + if err != nil { + return nil, err + } + } else { + auth, err = authn.DefaultKeychain.Resolve(ref.Context()) + if err != nil { + klog.Errorf("error resolving default keychain: %v", err) + return nil, err + } + } + + return pm.getImageMetadata(ctx, ref, auth, image) +} + +// handleCustomAuth ensures if images from custom repo's are requested their appropriate credentials are passed onto a secret for fn pods to use when pulling if it doesnt already exist +func (pm *podManager) handleCustomAuth(ctx context.Context, customRepoAuth string) error { + if err := pm.InspectOrCreateSecret(ctx, customRepoAuth); err != nil { + return err + } + return nil +} + +// if a custom image is requested use the secret provided to authenticate +func (pm *podManager) appendImagePullSecret(image string, customRepoAuth string, podTemplate *corev1.Pod) { + if customRepoAuth != "" && !strings.HasPrefix(image, defaultRepository) { + podTemplate.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ + {Name: "auth-secret"}, + } + } +} + +// getCustomAuth reads and parses the custom repo auth file from the mounted secret. +func (pm *podManager) getCustomAuth(ref name.Reference, customRepoAuth string) (authn.Authenticator, error) { + dockerConfigBytes, err := os.ReadFile(customRepoAuth) + if err != nil { + klog.Errorf("error reading authentication file %v", err) + return nil, err + } + + var dockerConfig DockerConfig + if err := json.Unmarshal(dockerConfigBytes, &dockerConfig); err != nil { + klog.Errorf("error unmarshalling authentication file %v", err) + return nil, err + } + + return authn.FromConfig(dockerConfig.Auths[ref.Context().RegistryStr()]), nil +} + +// 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)) if err != nil { return nil, err } @@ -485,16 +594,14 @@ func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string if err != nil { return nil, err } - cf, err := img.ConfigFile() + configFile, err := img.ConfigFile() if err != nil { return nil, err } - - cfg := cf.Config // TODO: to handle all scenario, we should follow https://docs.docker.com/engine/reference/builder/#understand-how-cmd-and-entrypoint-interact. - if len(cfg.Entrypoint) != 0 { - entrypoint = cfg.Entrypoint - } else { + cfg := configFile.Config + entrypoint := cfg.Entrypoint + if len(entrypoint) == 0 { entrypoint = cfg.Cmd } de := &digestAndEntrypoint{ @@ -506,14 +613,14 @@ func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string } // retrieveOrCreatePod retrieves or creates a pod for an image. -func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl time.Duration, useGenerateName bool) (client.ObjectKey, error) { +func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, customRepoAuth 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) + de, err = pm.imageDigestAndEntrypoint(ctx, image, customRepoAuth) if err != nil { return client.ObjectKey{}, fmt.Errorf("unable to get the entrypoint for %v: %w", image, err) } @@ -532,6 +639,7 @@ func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl // TODO: It's possible to set up a Watch in the fn runner namespace, and always try to maintain a up-to-date local cache. podList := &corev1.PodList{} podTemplate, templateVersion, err := pm.getBasePodTemplate(ctx) + pm.appendImagePullSecret(image, customRepoAuth, podTemplate) if err != nil { klog.Errorf("failed to generate a base pod template: %v", err) return client.ObjectKey{}, fmt.Errorf("failed to generate a base pod template: %w", err) diff --git a/func/internal/podevaluator_podmanager_test.go b/func/internal/podevaluator_podmanager_test.go index 86ed8b4f..2d847092 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) + go pm.getFuncEvalPodClient(ctx, tt.functionImage, time.Hour, tt.useGenerateName, "") if tt.podPatch != nil { go func() { diff --git a/func/server/server.go b/func/server/server.go index cb7e9661..0425f96f 100644 --- a/func/server/server.go +++ b/func/server/server.go @@ -41,6 +41,7 @@ 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.") + customRepoAuth = flag.String("custom-repo-secret-path", "", "Path to means of authentication for using images from custom repositories e.g. docker config file") 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.") @@ -89,7 +90,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) + podEval, err := internal.NewPodEvaluator(*podNamespace, wrapperServerImage, *scanInterval, *podTTL, *podCacheConfig, *functionPodTemplateName, *customRepoAuth) if err != nil { return fmt.Errorf("failed to initialize pod evaluator: %w", err) } From c7b999980e3a98e697c4879baa87add76894e447 Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Tue, 22 Oct 2024 09:51:10 +0100 Subject: [PATCH 2/7] changed variable nameing from repo/repository to registries --- func/internal/podevaluator.go | 76 ++++++++++++++++++----------------- func/server/server.go | 4 +- 2 files changed, 41 insertions(+), 39 deletions(-) diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index e56aba07..1f1b671d 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -58,9 +58,9 @@ const ( fieldManagerName = "krm-function-runner" functionContainerName = "function" defaultManagerNamespace = "porch-system" - defaultRepository = "gcr.io/kpt-fn/" + defaultRegistry = "gcr.io/kpt-fn/" // perhaps should try and get the name of the dockerconfig secret given by user and match this secret name to that to avoid hard coded value? - customRepoImgPullSecret = "auth-secret" + customRegistryImgPullSecret = "auth-secret" channelBufferSize = 128 ) @@ -73,7 +73,7 @@ type podEvaluator struct { var _ Evaluator = &podEvaluator{} -func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Duration, podTTLConfig string, functionPodTemplateName string, customRepoAuth string) (Evaluator, error) { +func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Duration, podTTLConfig string, functionPodTemplateName string, registryAuthSecretPath string) (Evaluator, error) { restCfg, err := config.GetConfig() if err != nil { return nil, fmt.Errorf("failed to get rest config: %w", err) @@ -102,13 +102,13 @@ func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Du pe := &podEvaluator{ requestCh: reqCh, podCacheManager: &podCacheManager{ - gcScanInternal: interval, - podTTL: ttl, - customRepoAuth: customRepoAuth, - requestCh: reqCh, - podReadyCh: readyCh, - cache: map[string]*podAndGRPCClient{}, - waitlists: map[string][]chan<- *clientConnAndError{}, + gcScanInternal: interval, + podTTL: ttl, + registryAuthSecretPath: registryAuthSecretPath, + requestCh: reqCh, + podReadyCh: readyCh, + cache: map[string]*podAndGRPCClient{}, + waitlists: map[string][]chan<- *clientConnAndError{}, podManager: &podManager{ kubeClient: cl, @@ -173,7 +173,7 @@ type podCacheManager struct { gcScanInternal time.Duration podTTL time.Duration - customRepoAuth string + registryAuthSecretPath string // requestCh is a receive-only channel to receive requestCh <-chan *clientConnRequest @@ -243,7 +243,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.customRepoAuth) + pcm.podManager.getFuncEvalPodClient(ctx, fnImage, ttl, false, pcm.registryAuthSecretPath) klog.Infof("preloaded pod cache for function %v", fnImage) }) @@ -311,7 +311,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.customRepoAuth) + go pcm.podManager.getFuncEvalPodClient(context.Background(), req.image, pcm.podTTL, true, pcm.registryAuthSecretPath) case resp := <-pcm.podReadyCh: if resp.err != nil { klog.Warningf("received error from the pod manager: %v", resp.err) @@ -443,9 +443,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, customRepoAuth string) { +func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, registryAuthSecretPath string) { c, err := func() (*podAndGRPCClient, error) { - podKey, err := pm.retrieveOrCreatePod(ctx, image, ttl, useGenerateName, customRepoAuth) + podKey, err := pm.retrieveOrCreatePod(ctx, image, ttl, useGenerateName, registryAuthSecretPath) if err != nil { return nil, err } @@ -473,7 +473,7 @@ func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, tt } } -func (pm *podManager) InspectOrCreateSecret(ctx context.Context, customRepoAuth string) error { +func (pm *podManager) InspectOrCreateSecret(ctx context.Context, registryAuthSecretPath string) error { secret := &corev1.Secret{} // using pod manager client since this secret is only related to these pods and nothing else err := pm.kubeClient.Get(context.Background(), client.ObjectKey{ @@ -485,8 +485,8 @@ func (pm *podManager) InspectOrCreateSecret(ctx context.Context, customRepoAuth // Error other than "not found" occurred return err } - klog.Infof("Secret for private repo pods does not exist and is required.\nGenerating Secret Now") - dockerConfigBytes, err := os.ReadFile(customRepoAuth) + klog.Infof("Secret for private registry pods does not exist and is required.\nGenerating Secret Now") + dockerConfigBytes, err := os.ReadFile(registryAuthSecretPath) if err != nil { return err } @@ -519,9 +519,11 @@ 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, customRepoAuth string) (*digestAndEntrypoint, error) { +func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string, registryAuthSecretPath string) (*digestAndEntrypoint, error) { start := time.Now() - defer klog.Infof("getting image metadata for %v took %v", image, time.Since(start)) + defer func() { + klog.Infof("getting image metadata for %v took %v", image, time.Since(start)) + }() ref, err := name.ParseReference(image) if err != nil { @@ -530,12 +532,12 @@ func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string } var auth authn.Authenticator - if customRepoAuth != "" && !strings.HasPrefix(image, defaultRepository) { - if err := pm.handleCustomAuth(ctx, customRepoAuth); err != nil { + if registryAuthSecretPath != "" && !strings.HasPrefix(image, defaultRegistry) { + if err := pm.handleCustomAuth(ctx, registryAuthSecretPath); err != nil { return nil, err } - auth, err = pm.getCustomAuth(ref, customRepoAuth) + auth, err = pm.getCustomAuth(ref, registryAuthSecretPath) if err != nil { return nil, err } @@ -550,26 +552,26 @@ func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string return pm.getImageMetadata(ctx, ref, auth, image) } -// handleCustomAuth ensures if images from custom repo's are requested their appropriate credentials are passed onto a secret for fn pods to use when pulling if it doesnt already exist -func (pm *podManager) handleCustomAuth(ctx context.Context, customRepoAuth string) error { - if err := pm.InspectOrCreateSecret(ctx, customRepoAuth); err != nil { +// handleCustomAuth ensures if images from custom registry's are requested their appropriate credentials are passed onto a secret for fn pods to use when pulling if it doesnt already exist +func (pm *podManager) handleCustomAuth(ctx context.Context, registryAuthSecretPath string) error { + if err := pm.InspectOrCreateSecret(ctx, registryAuthSecretPath); err != nil { return err } return nil } // if a custom image is requested use the secret provided to authenticate -func (pm *podManager) appendImagePullSecret(image string, customRepoAuth string, podTemplate *corev1.Pod) { - if customRepoAuth != "" && !strings.HasPrefix(image, defaultRepository) { +func (pm *podManager) appendImagePullSecret(image string, registryAuthSecretPath string, podTemplate *corev1.Pod) { + if registryAuthSecretPath != "" && !strings.HasPrefix(image, defaultRegistry) { podTemplate.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ {Name: "auth-secret"}, } } } -// getCustomAuth reads and parses the custom repo auth file from the mounted secret. -func (pm *podManager) getCustomAuth(ref name.Reference, customRepoAuth string) (authn.Authenticator, error) { - dockerConfigBytes, err := os.ReadFile(customRepoAuth) +// getCustomAuth reads and parses the custom registry auth file from the mounted secret. +func (pm *podManager) getCustomAuth(ref name.Reference, registryAuthSecretPath string) (authn.Authenticator, error) { + dockerConfigBytes, err := os.ReadFile(registryAuthSecretPath) if err != nil { klog.Errorf("error reading authentication file %v", err) return nil, err @@ -613,14 +615,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, customRepoAuth string) (client.ObjectKey, error) { +func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, registryAuthSecretPath 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, customRepoAuth) + de, err = pm.imageDigestAndEntrypoint(ctx, image, registryAuthSecretPath) if err != nil { return client.ObjectKey{}, fmt.Errorf("unable to get the entrypoint for %v: %w", image, err) } @@ -639,7 +641,7 @@ func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl // TODO: It's possible to set up a Watch in the fn runner namespace, and always try to maintain a up-to-date local cache. podList := &corev1.PodList{} podTemplate, templateVersion, err := pm.getBasePodTemplate(ctx) - pm.appendImagePullSecret(image, customRepoAuth, podTemplate) + pm.appendImagePullSecret(image, registryAuthSecretPath, podTemplate) if err != nil { klog.Errorf("failed to generate a base pod template: %v", err) return client.ObjectKey{}, fmt.Errorf("failed to generate a base pod template: %w", err) @@ -873,9 +875,9 @@ func podID(image, hash string) (string, error) { return "", fmt.Errorf("unable to parse image reference %v: %w", image, err) } - // repoName will be something like gcr.io/kpt-fn/set-namespace - repoName := ref.Context().Name() - parts := strings.Split(repoName, "/") + // registryName will be something like gcr.io/kpt-fn/set-namespace + registryName := ref.Context().Name() + parts := strings.Split(registryName, "/") name := strings.ReplaceAll(parts[len(parts)-1], "_", "-") return fmt.Sprintf("%v-%v", name, hash[:8]), nil } diff --git a/func/server/server.go b/func/server/server.go index 0425f96f..12065f23 100644 --- a/func/server/server.go +++ b/func/server/server.go @@ -41,7 +41,7 @@ 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.") - customRepoAuth = flag.String("custom-repo-secret-path", "", "Path to means of authentication for using images from custom repositories e.g. docker config file") + registryAuthSecretPath = flag.String("registry-auth-secret-path", "", "Path to means of authentication for using images from custom registries e.g. docker config file") 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.") @@ -90,7 +90,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, *customRepoAuth) + podEval, err := internal.NewPodEvaluator(*podNamespace, wrapperServerImage, *scanInterval, *podTTL, *podCacheConfig, *functionPodTemplateName, *registryAuthSecretPath) if err != nil { return fmt.Errorf("failed to initialize pod evaluator: %w", err) } From 643f8f68d9173c247b530370c9f61121bb2c2a35 Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Wed, 30 Oct 2024 09:13:33 +0000 Subject: [PATCH 3/7] language changes + secret content update if changed during runtime --- deployments/porch/5-rbac.yaml | 2 +- func/internal/podevaluator.go | 61 ++++++++++++++++++++++++----------- 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/deployments/porch/5-rbac.yaml b/deployments/porch/5-rbac.yaml index f7560f12..e18cca7b 100644 --- a/deployments/porch/5-rbac.yaml +++ b/deployments/porch/5-rbac.yaml @@ -70,4 +70,4 @@ rules: verbs: ["create", "delete", "patch", "get", "watch", "list"] - apiGroups: [""] resources: ["secrets"] - verbs: ["get", "create", "delete"] + verbs: ["create", "delete", "patch", "get"] diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index 1f1b671d..c61587af 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -477,7 +477,7 @@ func (pm *podManager) InspectOrCreateSecret(ctx context.Context, registryAuthSec secret := &corev1.Secret{} // using pod manager client since this secret is only related to these pods and nothing else err := pm.kubeClient.Get(context.Background(), client.ObjectKey{ - Name: "auth-secret", + Name: customRegistryImgPullSecret, Namespace: pm.namespace, }, secret) if err != nil { @@ -493,7 +493,7 @@ func (pm *podManager) InspectOrCreateSecret(ctx context.Context, registryAuthSec // Secret does not exist, create it secret = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "auth-secret", + Name: customRegistryImgPullSecret, Namespace: pm.namespace, }, Data: map[string][]byte{ @@ -506,9 +506,32 @@ func (pm *podManager) InspectOrCreateSecret(ctx context.Context, registryAuthSec return err } - klog.Infof("Secret created successfully") + klog.Infof("Private registry secret created successfully") } else { - klog.Infof("Secret already exists") + klog.Infof("Private registry secret already exists") + // Fetch "customRegistryImgPullSecret" from "porch-fn-runner" namespace + podAuthSecret := &corev1.Secret{} + err = pm.kubeClient.Get(ctx, client.ObjectKey{ + Name: customRegistryImgPullSecret, + Namespace: pm.namespace, + }, podAuthSecret) + + if err != nil { + return err + } + // Compare the data of the two secrets + if string(secret.Data[".dockerconfigjson"]) == string(podAuthSecret.Data[".dockerconfigjson"]) { + klog.Infof("The data content of the user given secret matches the private registry secret.") + } else { + klog.Infof("The data content of the private registry secret does not match given secret") + // Patch "secret-1" with the data from the existing secret + podAuthSecret.Data[".dockerconfigjson"] = secret.Data[".dockerconfigjson"] + err = pm.kubeClient.Update(ctx, podAuthSecret) + if err != nil { + return err + } + klog.Infof("Private registry secret patched successfully.") + } } return nil } @@ -533,7 +556,7 @@ func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string var auth authn.Authenticator if registryAuthSecretPath != "" && !strings.HasPrefix(image, defaultRegistry) { - if err := pm.handleCustomAuth(ctx, registryAuthSecretPath); err != nil { + if err := pm.ensureCustomAuthSecret(ctx, registryAuthSecretPath); err != nil { return nil, err } @@ -552,23 +575,14 @@ func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string return pm.getImageMetadata(ctx, ref, auth, image) } -// handleCustomAuth ensures if images from custom registry's are requested their appropriate credentials are passed onto a secret for fn pods to use when pulling if it doesnt already exist -func (pm *podManager) handleCustomAuth(ctx context.Context, registryAuthSecretPath string) error { +// 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. +func (pm *podManager) ensureCustomAuthSecret(ctx context.Context, registryAuthSecretPath string) error { if err := pm.InspectOrCreateSecret(ctx, registryAuthSecretPath); err != nil { return err } return nil } -// if a custom image is requested use the secret provided to authenticate -func (pm *podManager) appendImagePullSecret(image string, registryAuthSecretPath string, podTemplate *corev1.Pod) { - if registryAuthSecretPath != "" && !strings.HasPrefix(image, defaultRegistry) { - podTemplate.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ - {Name: "auth-secret"}, - } - } -} - // getCustomAuth reads and parses the custom registry auth file from the mounted secret. func (pm *podManager) getCustomAuth(ref name.Reference, registryAuthSecretPath string) (authn.Authenticator, error) { dockerConfigBytes, err := os.ReadFile(registryAuthSecretPath) @@ -793,6 +807,15 @@ func (pm *podManager) getBasePodTemplate(ctx context.Context) (*corev1.Pod, stri } } +// if a custom image is requested, use the secret provided to authenticate +func (pm *podManager) appendImagePullSecret(image string, registryAuthSecretPath string, podTemplate *corev1.Pod) { + if registryAuthSecretPath != "" && !strings.HasPrefix(image, defaultRegistry) { + podTemplate.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ + {Name: customRegistryImgPullSecret}, + } + } +} + // Patches the expected port, and the original entrypoint and image of the kpt function into the function container func (pm *podManager) patchNewPodContainer(pod *corev1.Pod, de digestAndEntrypoint, image string) error { var patchedContainer bool @@ -875,9 +898,9 @@ func podID(image, hash string) (string, error) { return "", fmt.Errorf("unable to parse image reference %v: %w", image, err) } - // registryName will be something like gcr.io/kpt-fn/set-namespace - registryName := ref.Context().Name() - parts := strings.Split(registryName, "/") + // repoName will be something like gcr.io/kpt-fn/set-namespace + repoName := ref.Context().Name() + parts := strings.Split(repoName, "/") name := strings.ReplaceAll(parts[len(parts)-1], "_", "-") return fmt.Sprintf("%v-%v", name, hash[:8]), nil } From 419b898c7a94ac3404c52c0f0ca67a8074aa3a76 Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Fri, 1 Nov 2024 11:25:02 +0000 Subject: [PATCH 4/7] fixed checking inspectorcreatesecret function checking same secret + should have used update instead of patch for secret rbac --- deployments/porch/5-rbac.yaml | 2 +- func/internal/podevaluator.go | 27 +++++++++++---------------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/deployments/porch/5-rbac.yaml b/deployments/porch/5-rbac.yaml index e18cca7b..0b42f821 100644 --- a/deployments/porch/5-rbac.yaml +++ b/deployments/porch/5-rbac.yaml @@ -70,4 +70,4 @@ rules: verbs: ["create", "delete", "patch", "get", "watch", "list"] - apiGroups: [""] resources: ["secrets"] - verbs: ["create", "delete", "patch", "get"] + verbs: ["create", "delete", "update", "get"] diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index c61587af..2965c7bc 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -474,24 +474,24 @@ func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, tt } func (pm *podManager) InspectOrCreateSecret(ctx context.Context, registryAuthSecretPath string) error { - secret := &corev1.Secret{} + podSecret := &corev1.Secret{} // using pod manager client since this secret is only related to these pods and nothing else err := pm.kubeClient.Get(context.Background(), client.ObjectKey{ Name: customRegistryImgPullSecret, Namespace: pm.namespace, - }, secret) + }, podSecret) if err != nil { if client.IgnoreNotFound(err) != nil { // Error other than "not found" occurred return err } - klog.Infof("Secret for private registry pods does not exist and is required.\nGenerating Secret Now") + klog.Infof("Secret for private registry pods does not exist and is required. Generating Secret Now") dockerConfigBytes, err := os.ReadFile(registryAuthSecretPath) if err != nil { return err } // Secret does not exist, create it - secret = &corev1.Secret{ + podSecret = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: customRegistryImgPullSecret, Namespace: pm.namespace, @@ -501,7 +501,7 @@ func (pm *podManager) InspectOrCreateSecret(ctx context.Context, registryAuthSec }, Type: corev1.SecretTypeDockerConfigJson, } - err = pm.kubeClient.Create(ctx, secret) + err = pm.kubeClient.Create(ctx, podSecret) if err != nil { return err } @@ -509,24 +509,19 @@ func (pm *podManager) InspectOrCreateSecret(ctx context.Context, registryAuthSec klog.Infof("Private registry secret created successfully") } else { klog.Infof("Private registry secret already exists") - // Fetch "customRegistryImgPullSecret" from "porch-fn-runner" namespace - podAuthSecret := &corev1.Secret{} - err = pm.kubeClient.Get(ctx, client.ObjectKey{ - Name: customRegistryImgPullSecret, - Namespace: pm.namespace, - }, podAuthSecret) - + // use the bytes Data of the user secret and compare it to the data of the pod secret + dockerConfigBytes, err := os.ReadFile(registryAuthSecretPath) if err != nil { return err } // Compare the data of the two secrets - if string(secret.Data[".dockerconfigjson"]) == string(podAuthSecret.Data[".dockerconfigjson"]) { + if string(podSecret.Data[".dockerconfigjson"]) == string(dockerConfigBytes) { klog.Infof("The data content of the user given secret matches the private registry secret.") } else { klog.Infof("The data content of the private registry secret does not match given secret") - // Patch "secret-1" with the data from the existing secret - podAuthSecret.Data[".dockerconfigjson"] = secret.Data[".dockerconfigjson"] - err = pm.kubeClient.Update(ctx, podAuthSecret) + // Patch the secret on the pods with the data from the user secret + podSecret.Data[".dockerconfigjson"] = dockerConfigBytes + err = pm.kubeClient.Update(ctx, podSecret) if err != nil { return err } From f36b6fdab5694b7b0805691605cffb57fa0f5c9a Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Tue, 5 Nov 2024 12:23:16 +0000 Subject: [PATCH 5/7] removed hardcoded authentication secret name on porch-fn-system namespace and added argument to allow its configuration --- func/internal/podevaluator.go | 38 +++++++++---------- func/internal/podevaluator_podmanager_test.go | 2 +- func/server/server.go | 5 ++- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index 2965c7bc..3f020ab4 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -59,8 +59,6 @@ const ( functionContainerName = "function" defaultManagerNamespace = "porch-system" defaultRegistry = "gcr.io/kpt-fn/" - // perhaps should try and get the name of the dockerconfig secret given by user and match this secret name to that to avoid hard coded value? - customRegistryImgPullSecret = "auth-secret" channelBufferSize = 128 ) @@ -73,7 +71,7 @@ type podEvaluator struct { var _ Evaluator = &podEvaluator{} -func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Duration, podTTLConfig string, functionPodTemplateName string, registryAuthSecretPath string) (Evaluator, error) { +func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Duration, podTTLConfig string, functionPodTemplateName string, registryAuthSecretPath string, registryAuthSecretName string) (Evaluator, error) { restCfg, err := config.GetConfig() if err != nil { return nil, fmt.Errorf("failed to get rest config: %w", err) @@ -105,6 +103,7 @@ func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Du gcScanInternal: interval, podTTL: ttl, registryAuthSecretPath: registryAuthSecretPath, + registryAuthSecretName: registryAuthSecretName, requestCh: reqCh, podReadyCh: readyCh, cache: map[string]*podAndGRPCClient{}, @@ -174,6 +173,7 @@ type podCacheManager struct { podTTL time.Duration registryAuthSecretPath string + registryAuthSecretName string // requestCh is a receive-only channel to receive requestCh <-chan *clientConnRequest @@ -243,7 +243,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.registryAuthSecretPath) + pcm.podManager.getFuncEvalPodClient(ctx, fnImage, ttl, false, pcm.registryAuthSecretPath, pcm.registryAuthSecretName) klog.Infof("preloaded pod cache for function %v", fnImage) }) @@ -311,7 +311,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.registryAuthSecretPath) + go pcm.podManager.getFuncEvalPodClient(context.Background(), req.image, pcm.podTTL, true, pcm.registryAuthSecretPath, pcm.registryAuthSecretName) case resp := <-pcm.podReadyCh: if resp.err != nil { klog.Warningf("received error from the pod manager: %v", resp.err) @@ -443,9 +443,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, registryAuthSecretPath string) { +func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, registryAuthSecretPath string, registryAuthSecretName string) { c, err := func() (*podAndGRPCClient, error) { - podKey, err := pm.retrieveOrCreatePod(ctx, image, ttl, useGenerateName, registryAuthSecretPath) + podKey, err := pm.retrieveOrCreatePod(ctx, image, ttl, useGenerateName, registryAuthSecretPath, registryAuthSecretName) if err != nil { return nil, err } @@ -473,11 +473,11 @@ func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, tt } } -func (pm *podManager) InspectOrCreateSecret(ctx context.Context, registryAuthSecretPath string) error { +func (pm *podManager) InspectOrCreateSecret(ctx context.Context, registryAuthSecretPath string, registryAuthSecretName string) error { podSecret := &corev1.Secret{} // using pod manager client since this secret is only related to these pods and nothing else err := pm.kubeClient.Get(context.Background(), client.ObjectKey{ - Name: customRegistryImgPullSecret, + Name: registryAuthSecretName, Namespace: pm.namespace, }, podSecret) if err != nil { @@ -493,7 +493,7 @@ func (pm *podManager) InspectOrCreateSecret(ctx context.Context, registryAuthSec // Secret does not exist, create it podSecret = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: customRegistryImgPullSecret, + Name: registryAuthSecretName, Namespace: pm.namespace, }, Data: map[string][]byte{ @@ -537,7 +537,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, registryAuthSecretPath string) (*digestAndEntrypoint, error) { +func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string, registryAuthSecretPath string, registryAuthSecretName string) (*digestAndEntrypoint, error) { start := time.Now() defer func() { klog.Infof("getting image metadata for %v took %v", image, time.Since(start)) @@ -551,7 +551,7 @@ func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string var auth authn.Authenticator if registryAuthSecretPath != "" && !strings.HasPrefix(image, defaultRegistry) { - if err := pm.ensureCustomAuthSecret(ctx, registryAuthSecretPath); err != nil { + if err := pm.ensureCustomAuthSecret(ctx, registryAuthSecretPath, registryAuthSecretName); err != nil { return nil, err } @@ -571,8 +571,8 @@ func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string } // 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. -func (pm *podManager) ensureCustomAuthSecret(ctx context.Context, registryAuthSecretPath string) error { - if err := pm.InspectOrCreateSecret(ctx, registryAuthSecretPath); err != nil { +func (pm *podManager) ensureCustomAuthSecret(ctx context.Context, registryAuthSecretPath string, registryAuthSecretName string) error { + if err := pm.InspectOrCreateSecret(ctx, registryAuthSecretPath, registryAuthSecretName); err != nil { return err } return nil @@ -624,14 +624,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, registryAuthSecretPath string) (client.ObjectKey, error) { +func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, registryAuthSecretPath string, registryAuthSecretName 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, registryAuthSecretPath) + de, err = pm.imageDigestAndEntrypoint(ctx, image, registryAuthSecretPath, registryAuthSecretName) if err != nil { return client.ObjectKey{}, fmt.Errorf("unable to get the entrypoint for %v: %w", image, err) } @@ -650,7 +650,7 @@ func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl // TODO: It's possible to set up a Watch in the fn runner namespace, and always try to maintain a up-to-date local cache. podList := &corev1.PodList{} podTemplate, templateVersion, err := pm.getBasePodTemplate(ctx) - pm.appendImagePullSecret(image, registryAuthSecretPath, podTemplate) + pm.appendImagePullSecret(image, registryAuthSecretPath, registryAuthSecretName, podTemplate) if err != nil { klog.Errorf("failed to generate a base pod template: %v", err) return client.ObjectKey{}, fmt.Errorf("failed to generate a base pod template: %w", err) @@ -803,10 +803,10 @@ func (pm *podManager) getBasePodTemplate(ctx context.Context) (*corev1.Pod, stri } // if a custom image is requested, use the secret provided to authenticate -func (pm *podManager) appendImagePullSecret(image string, registryAuthSecretPath string, podTemplate *corev1.Pod) { +func (pm *podManager) appendImagePullSecret(image string, registryAuthSecretPath string, registryAuthSecretName string, podTemplate *corev1.Pod) { if registryAuthSecretPath != "" && !strings.HasPrefix(image, defaultRegistry) { podTemplate.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ - {Name: customRegistryImgPullSecret}, + {Name: registryAuthSecretName}, } } } diff --git a/func/internal/podevaluator_podmanager_test.go b/func/internal/podevaluator_podmanager_test.go index 2d847092..b16439ec 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, "") + go pm.getFuncEvalPodClient(ctx, tt.functionImage, time.Hour, tt.useGenerateName, "", "auth-secret") if tt.podPatch != nil { go func() { diff --git a/func/server/server.go b/func/server/server.go index 12065f23..e3ec4e1d 100644 --- a/func/server/server.go +++ b/func/server/server.go @@ -41,7 +41,8 @@ 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.") - registryAuthSecretPath = flag.String("registry-auth-secret-path", "", "Path to means of authentication for using images from custom registries e.g. docker config file") + registryAuthSecretPath = flag.String("registry-auth-secret-path", "", "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") 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.") @@ -90,7 +91,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, *registryAuthSecretPath) + podEval, err := internal.NewPodEvaluator(*podNamespace, wrapperServerImage, *scanInterval, *podTTL, *podCacheConfig, *functionPodTemplateName, *registryAuthSecretPath, *registryAuthSecretName) if err != nil { return fmt.Errorf("failed to initialize pod evaluator: %w", err) } From e53d7c4bdf1e40bf7c3df363a25e77b63f86bfa3 Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Wed, 6 Nov 2024 09:36:56 +0000 Subject: [PATCH 6/7] added default value for argument & changed secret addition to pod until after pod template is successful --- func/internal/podevaluator.go | 6 +++--- func/server/server.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index 3f020ab4..e8c4bf00 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -550,7 +550,7 @@ func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string } var auth authn.Authenticator - if registryAuthSecretPath != "" && !strings.HasPrefix(image, defaultRegistry) { + if !strings.HasPrefix(image, defaultRegistry) { if err := pm.ensureCustomAuthSecret(ctx, registryAuthSecretPath, registryAuthSecretName); err != nil { return nil, err } @@ -650,11 +650,11 @@ func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl // TODO: It's possible to set up a Watch in the fn runner namespace, and always try to maintain a up-to-date local cache. podList := &corev1.PodList{} podTemplate, templateVersion, err := pm.getBasePodTemplate(ctx) - pm.appendImagePullSecret(image, registryAuthSecretPath, registryAuthSecretName, podTemplate) if err != nil { klog.Errorf("failed to generate a base pod template: %v", err) return client.ObjectKey{}, fmt.Errorf("failed to generate a base pod template: %w", err) } + pm.appendImagePullSecret(image, registryAuthSecretPath, registryAuthSecretName, podTemplate) err = pm.kubeClient.List(ctx, podList, client.InNamespace(pm.namespace), client.MatchingLabels(map[string]string{krmFunctionLabel: podId})) if err != nil { klog.Warningf("error when listing pods for %q: %v", image, err) @@ -804,7 +804,7 @@ func (pm *podManager) getBasePodTemplate(ctx context.Context) (*corev1.Pod, stri // if a custom image is requested, use the secret provided to authenticate func (pm *podManager) appendImagePullSecret(image string, registryAuthSecretPath string, registryAuthSecretName string, podTemplate *corev1.Pod) { - if registryAuthSecretPath != "" && !strings.HasPrefix(image, defaultRegistry) { + if !strings.HasPrefix(image, defaultRegistry) { podTemplate.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ {Name: registryAuthSecretName}, } diff --git a/func/server/server.go b/func/server/server.go index e3ec4e1d..0a4c8079 100644 --- a/func/server/server.go +++ b/func/server/server.go @@ -41,7 +41,7 @@ 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.") - registryAuthSecretPath = flag.String("registry-auth-secret-path", "", "The path of the secret used in custom registry 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") 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 7e2569ba8046f565eedeb31e18b9cdf929343687 Mon Sep 17 00:00:00 2001 From: Catalin Stratulat Date: Wed, 6 Nov 2024 11:26:54 +0000 Subject: [PATCH 7/7] added enable variable for private registry feature default=false --- func/internal/podevaluator.go | 46 ++++++++++--------- func/internal/podevaluator_podmanager_test.go | 2 +- func/server/server.go | 3 +- 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/func/internal/podevaluator.go b/func/internal/podevaluator.go index e8c4bf00..43a8d773 100644 --- a/func/internal/podevaluator.go +++ b/func/internal/podevaluator.go @@ -71,7 +71,7 @@ type podEvaluator struct { var _ Evaluator = &podEvaluator{} -func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Duration, podTTLConfig string, functionPodTemplateName string, 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) (Evaluator, error) { restCfg, err := config.GetConfig() if err != nil { return nil, fmt.Errorf("failed to get rest config: %w", err) @@ -100,14 +100,15 @@ func NewPodEvaluator(namespace, wrapperServerImage string, interval, ttl time.Du pe := &podEvaluator{ requestCh: reqCh, podCacheManager: &podCacheManager{ - gcScanInternal: interval, - podTTL: ttl, - registryAuthSecretPath: registryAuthSecretPath, - registryAuthSecretName: registryAuthSecretName, - requestCh: reqCh, - podReadyCh: readyCh, - cache: map[string]*podAndGRPCClient{}, - waitlists: map[string][]chan<- *clientConnAndError{}, + gcScanInternal: interval, + podTTL: ttl, + enablePrivateRegistries: enablePrivateRegistries, + registryAuthSecretPath: registryAuthSecretPath, + registryAuthSecretName: registryAuthSecretName, + requestCh: reqCh, + podReadyCh: readyCh, + cache: map[string]*podAndGRPCClient{}, + waitlists: map[string][]chan<- *clientConnAndError{}, podManager: &podManager{ kubeClient: cl, @@ -172,8 +173,9 @@ type podCacheManager struct { gcScanInternal time.Duration podTTL time.Duration - registryAuthSecretPath string - registryAuthSecretName string + enablePrivateRegistries bool + registryAuthSecretPath string + registryAuthSecretName string // requestCh is a receive-only channel to receive requestCh <-chan *clientConnRequest @@ -243,7 +245,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.registryAuthSecretPath, pcm.registryAuthSecretName) + pcm.podManager.getFuncEvalPodClient(ctx, fnImage, ttl, false, pcm.enablePrivateRegistries, pcm.registryAuthSecretPath, pcm.registryAuthSecretName) klog.Infof("preloaded pod cache for function %v", fnImage) }) @@ -311,7 +313,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.registryAuthSecretPath, pcm.registryAuthSecretName) + go pcm.podManager.getFuncEvalPodClient(context.Background(), req.image, pcm.podTTL, true, pcm.enablePrivateRegistries, pcm.registryAuthSecretPath, pcm.registryAuthSecretName) case resp := <-pcm.podReadyCh: if resp.err != nil { klog.Warningf("received error from the pod manager: %v", resp.err) @@ -443,9 +445,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, registryAuthSecretPath string, registryAuthSecretName string) { +func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, ttl time.Duration, useGenerateName bool, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string) { c, err := func() (*podAndGRPCClient, error) { - podKey, err := pm.retrieveOrCreatePod(ctx, image, ttl, useGenerateName, registryAuthSecretPath, registryAuthSecretName) + podKey, err := pm.retrieveOrCreatePod(ctx, image, ttl, useGenerateName, enablePrivateRegistries, registryAuthSecretPath, registryAuthSecretName) if err != nil { return nil, err } @@ -537,7 +539,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, registryAuthSecretPath string, registryAuthSecretName string) (*digestAndEntrypoint, error) { +func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string, enablePrivateRegistries bool, registryAuthSecretPath string, registryAuthSecretName string) (*digestAndEntrypoint, error) { start := time.Now() defer func() { klog.Infof("getting image metadata for %v took %v", image, time.Since(start)) @@ -550,7 +552,7 @@ func (pm *podManager) imageDigestAndEntrypoint(ctx context.Context, image string } var auth authn.Authenticator - if !strings.HasPrefix(image, defaultRegistry) { + if enablePrivateRegistries && !strings.HasPrefix(image, defaultRegistry) { if err := pm.ensureCustomAuthSecret(ctx, registryAuthSecretPath, registryAuthSecretName); err != nil { return nil, err } @@ -624,14 +626,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, 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) (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, registryAuthSecretPath, registryAuthSecretName) + de, err = pm.imageDigestAndEntrypoint(ctx, image, enablePrivateRegistries, registryAuthSecretPath, registryAuthSecretName) if err != nil { return client.ObjectKey{}, fmt.Errorf("unable to get the entrypoint for %v: %w", image, err) } @@ -654,7 +656,7 @@ func (pm *podManager) retrieveOrCreatePod(ctx context.Context, image string, ttl klog.Errorf("failed to generate a base pod template: %v", err) return client.ObjectKey{}, fmt.Errorf("failed to generate a base pod template: %w", err) } - pm.appendImagePullSecret(image, registryAuthSecretPath, registryAuthSecretName, podTemplate) + pm.appendImagePullSecret(image, enablePrivateRegistries, registryAuthSecretName, podTemplate) err = pm.kubeClient.List(ctx, podList, client.InNamespace(pm.namespace), client.MatchingLabels(map[string]string{krmFunctionLabel: podId})) if err != nil { klog.Warningf("error when listing pods for %q: %v", image, err) @@ -803,8 +805,8 @@ func (pm *podManager) getBasePodTemplate(ctx context.Context) (*corev1.Pod, stri } // if a custom image is requested, use the secret provided to authenticate -func (pm *podManager) appendImagePullSecret(image string, registryAuthSecretPath string, registryAuthSecretName string, podTemplate *corev1.Pod) { - if !strings.HasPrefix(image, defaultRegistry) { +func (pm *podManager) appendImagePullSecret(image string, enablePrivateRegistries bool, registryAuthSecretName string, podTemplate *corev1.Pod) { + if enablePrivateRegistries && !strings.HasPrefix(image, defaultRegistry) { podTemplate.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ {Name: registryAuthSecretName}, } diff --git a/func/internal/podevaluator_podmanager_test.go b/func/internal/podevaluator_podmanager_test.go index b16439ec..e51116ca 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, "", "auth-secret") + go pm.getFuncEvalPodClient(ctx, tt.functionImage, time.Hour, tt.useGenerateName, false, "", "auth-secret") if tt.podPatch != nil { go func() { diff --git a/func/server/server.go b/func/server/server.go index 0a4c8079..a0a199c0 100644 --- a/func/server/server.go +++ b/func/server/server.go @@ -41,6 +41,7 @@ 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") 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.") @@ -91,7 +92,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, *registryAuthSecretPath, *registryAuthSecretName) + podEval, err := internal.NewPodEvaluator(*podNamespace, wrapperServerImage, *scanInterval, *podTTL, *podCacheConfig, *functionPodTemplateName, *enablePrivateRegistries, *registryAuthSecretPath, *registryAuthSecretName) if err != nil { return fmt.Errorf("failed to initialize pod evaluator: %w", err) }