Skip to content

Commit

Permalink
Merge pull request #151 from nokia/use-fn-image-prefix
Browse files Browse the repository at this point in the history
Pass default-image-prefix to function runners
  • Loading branch information
nephio-prow[bot] authored Dec 12, 2024
2 parents a3db339 + 3b5e1dd commit f252f33
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 16 deletions.
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,7 @@ __debug*

### VisualStudioCode Patch ###
# Ignore all local history of files
**/.history
**/.history

### Jetbrains IDEs ###
.idea/*
21 changes: 15 additions & 6 deletions internal/kpt/fnruntime/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 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 == "" {
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
Expand Down
5 changes: 3 additions & 2 deletions internal/kpt/fnruntime/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (

const (
FuncGenPkgContext = "builtins/gen-pkg-context"
GCRImagePrefix = "gcr.io/kpt-fn/"
)

type RunnerOptions struct {
Expand Down Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions internal/kpt/fnruntime/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
10 changes: 9 additions & 1 deletion internal/kpt/util/get/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -221,6 +225,10 @@ func (c *Command) DefaultValues() error {
c.UpdateStrategy = kptfilev1.ResourceMerge
}

if len(c.DefaultKrmFunctionImagePrefix) == 0 {
c.DefaultKrmFunctionImagePrefix = fnruntime.GCRImagePrefix
}

return nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/cmd/server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"os"
"time"

"github.com/nephio-project/porch/internal/kpt/fnruntime"
"github.com/spf13/cobra"
"github.com/spf13/pflag"

Expand Down Expand Up @@ -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.")
Expand Down
4 changes: 2 additions & 2 deletions pkg/kpt/fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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())
Expand Down
3 changes: 2 additions & 1 deletion pkg/kpt/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/task/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (

func TestRender(t *testing.T) {
runnerOptions := fnruntime.RunnerOptions{}
runnerOptions.InitDefaults()
runnerOptions.InitDefaults(fnruntime.GCRImagePrefix)

render := &renderPackageMutation{
runnerOptions: runnerOptions,
Expand Down

0 comments on commit f252f33

Please sign in to comment.