diff --git a/cmd/build.go b/cmd/build.go index ad403aeb7..cccada77d 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -3,6 +3,7 @@ package cmd import ( "errors" "fmt" + "strings" "github.com/AlecAivazis/survey/v2" "github.com/AlecAivazis/survey/v2/terminal" @@ -103,7 +104,7 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro // pertinent values from the config. // // NOTE: the checks for .Changed and altered conditionals for defaults will - // be removed when Global Config is integreated, because the config object + // be removed when Global Config is integrated, because the config object // will at that time contain the final value for the attribute, taking into // account whether or not the value was altered via flags or env variables. // This condition is also only necessary for config members whose default @@ -119,13 +120,26 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro // Sets default AND accepts any user-provided overrides f.Registry = config.Registry } + if config.Image != "" { + f.Image = config.Image + } + // Checks if there is a difference between defined registry and its value used as a prefix in the image tag + // In case of a mismatch a new image tag is created and used for build + // Do not react if image tag has been changed outside configuration + if f.Registry != "" && !cmd.Flags().Changed("image") && strings.Index(f.Image, "/") > 0 && !strings.HasPrefix(f.Image, f.Registry) { + prfx := f.Registry + if prfx[len(prfx)-1:] != "/" { + prfx = prfx + "/" + } + sps := strings.Split(f.Image, "/") + updImg := prfx + sps[len(sps)-1] + fmt.Fprintf(cmd.ErrOrStderr(), "Warning: image tag '%s' contains different registry than the specified '%s'. It will be overwritten with value '%s'\n", f.Image, f.Registry, updImg) + f.Image = updImg + } if f.Build.Builder == "" || cmd.Flags().Changed("builder") { // Sets default AND accepts any user-provided overrides f.Build.Builder = config.Builder } - if config.Image != "" { - f.Image = config.Image - } if config.Builder != "" { f.Build.Builder = config.Builder } diff --git a/cmd/build_test.go b/cmd/build_test.go index b0d6e3ea7..a53ddecfb 100644 --- a/cmd/build_test.go +++ b/cmd/build_test.go @@ -2,8 +2,11 @@ package cmd import ( "errors" + "fmt" "testing" + "gotest.tools/v3/assert" + fn "knative.dev/kn-plugin-func" "knative.dev/kn-plugin-func/mock" ) @@ -62,7 +65,7 @@ func TestBuild_RegistryOrImageRequired(t *testing.T) { } } - // earlire test covers the --registry only case, test here that --image + // earlier test covers the --registry only case, test here that --image // also succeeds. cmd.SetArgs([]string{"--image=example.com/alice/myfunc"}) if err := cmd.Execute(); err != nil { @@ -70,7 +73,7 @@ func TestBuild_RegistryOrImageRequired(t *testing.T) { } } -// TestBuild_InvalidRegistry ensures that providing an invalid resitry +// TestBuild_InvalidRegistry ensures that providing an invalid registry // fails with the expected error. func TestBuild_InvalidRegistry(t *testing.T) { testInvalidRegistry(NewBuildCmd, t) @@ -140,7 +143,7 @@ func TestBuild_Push(t *testing.T) { t.Fatal("push should be invoked when requested and a successful build") } - // Exeute with push enabled but with a failed build + // Execute with push enabled but with a failed build builder.BuildFn = func(f fn.Function) error { return errors.New("mock error") } @@ -152,3 +155,75 @@ func TestBuild_Push(t *testing.T) { t.Fatal("push should not be invoked on a failed build") } } + +type buildWithRegistryTestCase struct { + desc string + testFn fn.Function + testFnArgs []string + expRegistry string + expImage string +} + +func TestBuild_RegistryHandling(t *testing.T) { + root := fromTempDirectory(t) + for i, tc := range []buildWithRegistryTestCase{ + + { + desc: "should update func.yaml's image tag if mismatch with func.yaml's registry", + testFn: fn.Function{ + Runtime: "go", + Root: root + "/1", + Registry: TestRegistry, // defined as "example.com/alice" + Image: "docker.io/tigerteam/foo", // image uses different registry in its tag, so it has to be updated + }, + testFnArgs: []string{"--path", root + "/1"}, + expRegistry: TestRegistry, + expImage: TestRegistry + "/foo", + }, + { + desc: "should update func.yaml's image tag and registry if mismatch with --registry flag", + testFn: fn.Function{ + Runtime: "go", + Root: root + "/2", + Registry: TestRegistry, + Image: "docker.io/tigerteam/foo", + }, + testFnArgs: []string{"--path", root + "/2", "--registry", "example.com/test"}, // registry flag should overwrite func.yaml's image and registry + expRegistry: "example.com/test", + expImage: "example.com/test/foo", + }, + + { + desc: "should NOT update func.yaml's registry if --image flag provided", + testFn: fn.Function{ + Runtime: "go", + Root: root + "/3", + Registry: TestRegistry, + Image: "docker.io/tigerteam/foo", + }, + testFnArgs: []string{"--path", root + "/3", "--image", "example.com/test/boo"}, // image flag should NOT overwrite func.yaml's registry + expRegistry: TestRegistry, + expImage: "example.com/test/boo", + }, + } { + var builder = mock.NewBuilder() + cmd := NewBuildCmd(NewClientFactory(func() *fn.Client { + return fn.New(fn.WithBuilder(builder)) + })) + tci := i + 1 + t.Logf("Test case %d: %s", tci, tc.desc) + + err := fn.New().Create(tc.testFn) + assert.Assert(t, err == nil) + + cmd.SetArgs(tc.testFnArgs) + err = cmd.Execute() + assert.Assert(t, err == nil) + + f, err := fn.NewFunction(tc.testFn.Root) + assert.Assert(t, err == nil) + + assert.Assert(t, f.Registry == tc.expRegistry, fmt.Sprintf("Test case %d: expected registry to be '"+tc.expRegistry+"', but got '%s'", tci, f.Registry)) + assert.Assert(t, f.Image == tc.expImage, fmt.Sprintf("Test case %d: expected image to be '"+tc.expImage+"', but got '%s'", tci, f.Image)) + } +} diff --git a/cmd/root_test.go b/cmd/root_test.go index 05619d66d..11790554c 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -299,7 +299,7 @@ func piped(t *testing.T) func() string { // - creates a temp directory and changes to it as the new working directory // - creates a temp directory and uses it for XDG_CONFIG_HOME // - removes temp directories on cleanup -// - resets viper on cleanum (the reason this is "cli-specific") +// - resets viper on cleanup (the reason this is "cli-specific") func fromTempDirectory(t *testing.T) string { t.Helper() t.Setenv("XDG_CONFIG_HOME", t.TempDir()) diff --git a/function.go b/function.go index 6c57e0b30..ecd4348fb 100644 --- a/function.go +++ b/function.go @@ -167,11 +167,10 @@ func NewFunctionWith(defaults Function) Function { // Functions which are syntactically valid are also then logically validated. // Functions from earlier versions are brought up to current using migrations. // Migrations are run prior to validators such that validation can omit -// concerning itself with backwards compatibility. Migrators must therefore +// concerning itself with backwards compatibility. Migrators must therefore // selectively consider the minimal validation necessary to enable migration. func NewFunction(path string) (f Function, err error) { f.Root = path // path is not persisted, as this is the purview of the FS itself - f.Root = path // path is not persisted, as this is the purvew of the FS itself f.Build.BuilderImages = make(map[string]string) f.Deploy.Annotations = make(map[string]string) var filename = filepath.Join(path, FunctionFile)