From 66fd1cdf068f9e36f15d47017e20dc76a6424419 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?M=C3=B3zes=20L=C3=A1szl=C3=B3=20M=C3=A1t=C3=A9?=
 <laszlo.mozes@nokia.com>
Date: Thu, 5 Dec 2024 16:46:22 +0100
Subject: [PATCH 1/2] pass default-image-prefix to function runners

---
 .gitignore                            |  5 ++++-
 internal/kpt/fnruntime/container.go   | 21 ++++++++++++------
 internal/kpt/fnruntime/runner.go      |  5 +++--
 internal/kpt/fnruntime/runner_test.go | 31 +++++++++++++++++++++++++++
 internal/kpt/util/get/get.go          | 10 ++++++++-
 pkg/apiserver/apiserver.go            |  2 +-
 pkg/cmd/server/start.go               |  3 ++-
 pkg/kpt/fs_test.go                    |  4 ++--
 pkg/kpt/render_test.go                |  3 ++-
 pkg/task/render_test.go               |  2 +-
 10 files changed, 70 insertions(+), 16 deletions(-)

diff --git a/.gitignore b/.gitignore
index 26a28f3c..e26a85f5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -28,4 +28,7 @@ __debug*
 
 ### VisualStudioCode Patch ###
 # Ignore all local history of files
-**/.history
\ No newline at end of file
+**/.history
+
+### Jetbrains IDEs ###
+.idea/*
\ No newline at end of file
diff --git a/internal/kpt/fnruntime/container.go b/internal/kpt/fnruntime/container.go
index 74506278..ef35ec98 100644
--- a/internal/kpt/fnruntime/container.go
+++ b/internal/kpt/fnruntime/container.go
@@ -224,13 +224,22 @@ func NewContainerEnvFromStringSlice(envStr []string) *runtimeutil.ContainerEnv {
 	return ce
 }
 
-// ResolveToImageForCLI converts the function short path to the full image url.
-// If the function is Catalog function, it adds "gcr.io/kpt-fn/".e.g. set-namespace:v0.1 --> gcr.io/kpt-fn/set-namespace:v0.1
-func ResolveToImageForCLI(_ context.Context, image string) (string, error) {
-	if !strings.Contains(image, "/") {
-		return fmt.Sprintf("gcr.io/kpt-fn/%s", image), nil
+// ResolveToImageForCLIFunc returns a func that converts the function short path to the full image url.
+// `prefix` must end with a "/".
+// If the function is Catalog function, it adds `prefix` .e.g. "set-namespace:v0.1" --> prefix + "set-namespace:v0.1"
+func ResolveToImageForCLIFunc(prefix string) func(_ context.Context, image string) (string, error) {
+	prefix = strings.TrimSuffix(prefix, "/")
+	if prefix == "" {
+		return func(_ context.Context, image string) (string, error) {
+			return image, nil
+		}
+	}
+	return func(_ context.Context, image string) (string, error) {
+		if !strings.Contains(image, "/") {
+			return fmt.Sprintf("%s/%s", prefix, image), nil
+		}
+		return image, nil
 	}
-	return image, nil
 }
 
 // ContainerImageError is an error type which will be returned when
diff --git a/internal/kpt/fnruntime/runner.go b/internal/kpt/fnruntime/runner.go
index 8fcea9c5..3ea60d13 100644
--- a/internal/kpt/fnruntime/runner.go
+++ b/internal/kpt/fnruntime/runner.go
@@ -43,6 +43,7 @@ import (
 
 const (
 	FuncGenPkgContext = "builtins/gen-pkg-context"
+	GCRImagePrefix    = "gcr.io/kpt-fn/"
 )
 
 type RunnerOptions struct {
@@ -79,9 +80,9 @@ type RunnerOptions struct {
 // ImageResolveFunc is the type for a function that can resolve a partial image to a (more) fully-qualified name
 type ImageResolveFunc func(ctx context.Context, image string) (string, error)
 
-func (o *RunnerOptions) InitDefaults() {
+func (o *RunnerOptions) InitDefaults(defaultImagePrefix string) {
 	o.ImagePullPolicy = IfNotPresentPull
-	o.ResolveToImage = ResolveToImageForCLI
+	o.ResolveToImage = ResolveToImageForCLIFunc(defaultImagePrefix)
 }
 
 // NewRunner returns a FunctionRunner given a specification of a function
diff --git a/internal/kpt/fnruntime/runner_test.go b/internal/kpt/fnruntime/runner_test.go
index 176b86b2..ecbb0e38 100644
--- a/internal/kpt/fnruntime/runner_test.go
+++ b/internal/kpt/fnruntime/runner_test.go
@@ -646,3 +646,34 @@ func TestPrintFnStderr(t *testing.T) {
 		})
 	}
 }
+
+func TestRunnerOptions_InitDefaults(t *testing.T) {
+	tests := map[string]struct {
+		prefix string
+	}{
+		"empty":             {prefix: ""},
+		"trailing_slash":    {prefix: "example.org/kpt-fn/"},
+		"no_trailing_slash": {prefix: "example.org/kpt-fn"},
+	}
+
+	const fnName = "my-krm-function"
+
+	for testName, tc := range tests {
+		t.Run(testName, func(t *testing.T) {
+			opts := &RunnerOptions{}
+			opts.InitDefaults(tc.prefix)
+
+			result, err := opts.ResolveToImage(context.TODO(), fnName)
+
+			assert.NoError(t, err)
+			assert.Equal(t, getExpectedPrefix(tc.prefix)+fnName, result)
+		})
+	}
+}
+
+func getExpectedPrefix(prefix string) string {
+	if prefix != "" && !strings.HasSuffix(prefix, "/") {
+		return prefix + "/"
+	}
+	return prefix
+}
diff --git a/internal/kpt/util/get/get.go b/internal/kpt/util/get/get.go
index 953581d8..1901b4a2 100644
--- a/internal/kpt/util/get/get.go
+++ b/internal/kpt/util/get/get.go
@@ -61,6 +61,10 @@ type Command struct {
 	// Kptfile. This determines how changes will be merged when updating the
 	// package.
 	UpdateStrategy kptfilev1.UpdateStrategyType
+
+	// DefaultKrmFunctionImagePrefix is the prefix to be used with unqualified
+	// KRM function image names. Defaults to "gcr.io/kpt-fn/".
+	DefaultKrmFunctionImagePrefix string
 }
 
 // Run runs the Command.
@@ -127,7 +131,7 @@ func (c Command) Run(ctx context.Context) error {
 		pr := printer.FromContextOrDie(ctx)
 		pr.Printf("\nCustomizing package for deployment.\n")
 		hookCmd := hook.Executor{}
-		hookCmd.RunnerOptions.InitDefaults()
+		hookCmd.RunnerOptions.InitDefaults(c.DefaultKrmFunctionImagePrefix)
 		hookCmd.PkgPath = c.Destination
 
 		builtinHooks := []kptfilev1.Function{
@@ -221,6 +225,10 @@ func (c *Command) DefaultValues() error {
 		c.UpdateStrategy = kptfilev1.ResourceMerge
 	}
 
+	if len(c.DefaultKrmFunctionImagePrefix) == 0 {
+		c.DefaultKrmFunctionImagePrefix = fnruntime.GCRImagePrefix
+	}
+
 	return nil
 }
 
diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go
index e1c38b40..8af03a9c 100644
--- a/pkg/apiserver/apiserver.go
+++ b/pkg/apiserver/apiserver.go
@@ -237,7 +237,7 @@ func (c completedConfig) New() (*PorchServer, error) {
 
 	runnerOptionsResolver := func(namespace string) fnruntime.RunnerOptions {
 		runnerOptions := fnruntime.RunnerOptions{}
-		runnerOptions.InitDefaults()
+		runnerOptions.InitDefaults(c.ExtraConfig.DefaultImagePrefix)
 
 		return runnerOptions
 	}
diff --git a/pkg/cmd/server/start.go b/pkg/cmd/server/start.go
index a7588adb..27ec82b8 100644
--- a/pkg/cmd/server/start.go
+++ b/pkg/cmd/server/start.go
@@ -22,6 +22,7 @@ import (
 	"os"
 	"time"
 
+	"github.com/nephio-project/porch/internal/kpt/fnruntime"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
@@ -244,7 +245,7 @@ func (o *PorchServerOptions) AddFlags(fs *pflag.FlagSet) {
 	}
 
 	fs.StringVar(&o.FunctionRunnerAddress, "function-runner", "", "Address of the function runner gRPC service.")
-	fs.StringVar(&o.DefaultImagePrefix, "default-image-prefix", "gcr.io/kpt-fn/", "Default prefix for unqualified function names")
+	fs.StringVar(&o.DefaultImagePrefix, "default-image-prefix", fnruntime.GCRImagePrefix, "Default prefix for unqualified function names")
 	fs.StringVar(&o.CacheDirectory, "cache-directory", "", "Directory where Porch server stores repository and package caches.")
 	fs.IntVar(&o.MaxRequestBodySize, "max-request-body-size", 6*1024*1024, "Maximum size of the request body in bytes. Keep this in sync with function-runner's corresponding argument.")
 	fs.BoolVar(&o.UseGitCaBundle, "use-git-cabundle", false, "Determine whether to use a user-defined CaBundle for TLS towards git.")
diff --git a/pkg/kpt/fs_test.go b/pkg/kpt/fs_test.go
index 6a3d5dc4..b822ec1e 100644
--- a/pkg/kpt/fs_test.go
+++ b/pkg/kpt/fs_test.go
@@ -106,7 +106,7 @@ spec:
 		FileSystem: fs,
 		Runtime:    &runtime{},
 	}
-	r.RunnerOptions.InitDefaults()
+	r.RunnerOptions.InitDefaults(fnruntime.GCRImagePrefix)
 	r.RunnerOptions.ImagePullPolicy = fnruntime.IfNotPresentPull
 	_, err := r.Execute(fake.CtxWithDefaultPrinter())
 	if err != nil {
@@ -220,7 +220,7 @@ spec:
 		FileSystem: fs,
 		Runtime:    &runtime{},
 	}
-	r.RunnerOptions.InitDefaults()
+	r.RunnerOptions.InitDefaults(fnruntime.GCRImagePrefix)
 	r.RunnerOptions.ImagePullPolicy = fnruntime.IfNotPresentPull
 
 	_, err := r.Execute(fake.CtxWithDefaultPrinter())
diff --git a/pkg/kpt/render_test.go b/pkg/kpt/render_test.go
index f10288fd..0eb77dcd 100644
--- a/pkg/kpt/render_test.go
+++ b/pkg/kpt/render_test.go
@@ -21,6 +21,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/nephio-project/porch/internal/kpt/fnruntime"
 	"github.com/nephio-project/porch/internal/kpt/util/render"
 	"github.com/nephio-project/porch/pkg/kpt/printer/fake"
 	"sigs.k8s.io/kustomize/kyaml/filesys"
@@ -65,7 +66,7 @@ func TestRender(t *testing.T) {
 				FileSystem: filesys.FileSystemOrOnDisk{},
 				Output:     &output,
 			}
-			r.RunnerOptions.InitDefaults()
+			r.RunnerOptions.InitDefaults(fnruntime.GCRImagePrefix)
 
 			if _, err := r.Execute(fake.CtxWithDefaultPrinter()); err != nil {
 				t.Errorf("Render failed: %v", err)
diff --git a/pkg/task/render_test.go b/pkg/task/render_test.go
index 1d1fc17a..748b6260 100644
--- a/pkg/task/render_test.go
+++ b/pkg/task/render_test.go
@@ -31,7 +31,7 @@ import (
 
 func TestRender(t *testing.T) {
 	runnerOptions := fnruntime.RunnerOptions{}
-	runnerOptions.InitDefaults()
+	runnerOptions.InitDefaults(fnruntime.GCRImagePrefix)
 
 	render := &renderPackageMutation{
 		runnerOptions: runnerOptions,

From 3b5e1dddcd6d573431b95bbd611b515a178afaf2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?M=C3=B3zes=20L=C3=A1szl=C3=B3=20M=C3=A1t=C3=A9?=
 <laszlo.mozes@nokia.com>
Date: Mon, 9 Dec 2024 13:28:04 +0100
Subject: [PATCH 2/2] fix doc string

---
 internal/kpt/fnruntime/container.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/internal/kpt/fnruntime/container.go b/internal/kpt/fnruntime/container.go
index ef35ec98..75aa24d6 100644
--- a/internal/kpt/fnruntime/container.go
+++ b/internal/kpt/fnruntime/container.go
@@ -224,9 +224,9 @@ func NewContainerEnvFromStringSlice(envStr []string) *runtimeutil.ContainerEnv {
 	return ce
 }
 
-// ResolveToImageForCLIFunc returns a func that converts the function short path to the full image url.
-// `prefix` must end with a "/".
-// If the function is Catalog function, it adds `prefix` .e.g. "set-namespace:v0.1" --> prefix + "set-namespace:v0.1"
+// ResolveToImageForCLIFunc returns a func that converts the KRM function short path to the full image url.
+// If the function is a catalog function, it prepends `prefix`, e.g. "set-namespace:v0.1" --> prefix + "set-namespace:v0.1".
+// A "/" is appended to `prefix` if it is not an empty string and does not end with a "/".
 func ResolveToImageForCLIFunc(prefix string) func(_ context.Context, image string) (string, error) {
 	prefix = strings.TrimSuffix(prefix, "/")
 	if prefix == "" {