diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index 1a27fd821a..d0bc46640c 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -1198,7 +1198,7 @@ func testRegistryLoads(cmdFn commandConstructor, t *testing.T) { f := fn.Function{ Root: root, - Name: "myFunc", + Name: "my-func", Runtime: "go", Registry: "example.com/alice", } @@ -1219,7 +1219,7 @@ func testRegistryLoads(cmdFn commandConstructor, t *testing.T) { t.Fatal(err) } - expected := "example.com/alice/myFunc:latest" + expected := "example.com/alice/my-func:latest" if f.Image != expected { t.Fatalf("expected image name '%v'. got %v", expected, f.Image) } diff --git a/pkg/functions/client.go b/pkg/functions/client.go index 16f5230750..3fb5c37419 100644 --- a/pkg/functions/client.go +++ b/pkg/functions/client.go @@ -24,7 +24,7 @@ import ( const ( // DefaultRegistry through which containers of functions will be shuttled. - DefaultRegistry = "docker.io" + DefaultRegistry = "index.docker.io" // DefaultTemplate is the default function signature / environmental context // of the resultant function. All runtimes are expected to have at least diff --git a/pkg/functions/client_test.go b/pkg/functions/client_test.go index 85e256569a..db8c0a65be 100644 --- a/pkg/functions/client_test.go +++ b/pkg/functions/client_test.go @@ -50,7 +50,7 @@ var ( // thus implicitly tests Create, Build and Deploy, which are exposed // by the client API for those who prefer manual transmissions. func TestClient_New(t *testing.T) { - root := "testdata/example.com/testNew" + root := "testdata/example.com/test-new" defer Using(t, root)() client := fn.New(fn.WithRegistry(TestRegistry), fn.WithVerbose(true)) @@ -210,7 +210,7 @@ func TestClient_New_RuntimeRequired(t *testing.T) { // TestClient_New_NameDefaults ensures that a newly created function has its name defaulted // to a name which can be derived from the last part of the given root path. func TestClient_New_NameDefaults(t *testing.T) { - root := "testdata/example.com/testNameDefaults" + root := "testdata/example.com/test-name-defaults" defer Using(t, root)() client := fn.New(fn.WithRegistry(TestRegistry)) @@ -230,7 +230,7 @@ func TestClient_New_NameDefaults(t *testing.T) { t.Fatal(err) } - expected := "testNameDefaults" + expected := "test-name-defaults" if f.Name != expected { t.Fatalf("name was not defaulted. expected '%v' got '%v'", expected, f.Name) } @@ -239,7 +239,7 @@ func TestClient_New_NameDefaults(t *testing.T) { // TestClient_New_WritesTemplate ensures the config file and files from the template // are written on new. func TestClient_New_WritesTemplate(t *testing.T) { - root := "testdata/example.com/testWritesTemplate" + root := "testdata/example.com/test-writes-template" defer Using(t, root)() client := fn.New(fn.WithRegistry(TestRegistry)) @@ -262,7 +262,7 @@ func TestClient_New_WritesTemplate(t *testing.T) { // TestClient_New_ExtantAborts ensures that a directory which contains an extant // function does not reinitialize. func TestClient_New_ExtantAborts(t *testing.T) { - root := "testdata/example.com/testExtantAborts" + root := "testdata/example.com/test-extant-aborts" defer Using(t, root)() client := fn.New(fn.WithRegistry(TestRegistry)) @@ -305,7 +305,7 @@ func TestClient_New_NonemptyAborts(t *testing.T) { // conjunction with other tools (.envrc, etc) func TestClient_New_HiddenFilesIgnored(t *testing.T) { // Create a directory for the function - root := "testdata/example.com/testHiddenFilesIgnored" + root := "testdata/example.com/test-hidden-files-ignored" defer Using(t, root)() client := fn.New(fn.WithRegistry(TestRegistry)) @@ -332,7 +332,7 @@ func TestClient_New_HiddenFilesIgnored(t *testing.T) { // See the CLI for full details, but a standard default location is // $HOME/.config/func/repositories/boson/go/json func TestClient_New_RepositoriesExtensible(t *testing.T) { - root := "testdata/example.com/testRepositoriesExtensible" + root := "testdata/example.com/test-repositories-extensible" defer Using(t, root)() client := fn.New( @@ -479,7 +479,7 @@ func TestClient_New_RegistryRequired(t *testing.T) { // resultant OCI container is populated. func TestClient_New_ImageNamePopulated(t *testing.T) { // Create the root function directory - root := "testdata/example.com/testDeriveImage" + root := "testdata/example.com/test-derive-image" defer Using(t, root)() // Create the function which calculates fields such as name and image. @@ -508,7 +508,7 @@ func TestClient_New_ImageNamePopulated(t *testing.T) { // For example "alice" becomes "docker.io/alice" func TestClient_New_ImageRegistryDefaults(t *testing.T) { // Create the root function directory - root := "testdata/example.com/testDeriveImageDefaultRegistry" + root := "testdata/example.com/test-derive-image-default-registry" defer Using(t, root)() // Create the function which calculates fields such as name and image. @@ -533,9 +533,9 @@ func TestClient_New_ImageRegistryDefaults(t *testing.T) { // Deploy (and confirms expected fields calculated). func TestClient_New_Delegation(t *testing.T) { var ( - root = "testdata/example.com/testNewDelegates" // .. in which to initialize - expectedName = "testNewDelegates" // expected to be derived - expectedImage = "example.com/alice/testNewDelegates:latest" + root = "testdata/example.com/test-new-delegates" // .. in which to initialize + expectedName = "test-new-delegates" // expected to be derived + expectedImage = "example.com/alice/test-new-delegates:latest" builder = mock.NewBuilder() pusher = mock.NewPusher() deployer = mock.NewDeployer() @@ -607,7 +607,7 @@ func TestClient_New_Delegation(t *testing.T) { // See TestClient_Runner for the test of the default runner implementation. func TestClient_Run(t *testing.T) { // Create the root function directory - root := "testdata/example.com/testRun" + root := "testdata/example.com/test-run" defer Using(t, root)() // client with the mock runner and the new test function @@ -693,7 +693,7 @@ func TestClient_Runner(t *testing.T) { // .func to .gitignore is an important externally visible "feature", an explicit // test is warranted. func TestClient_Run_DataDir(t *testing.T) { - root := "testdata/example.com/testRunDataDir" + root := "testdata/example.com/test-run-data-dir" defer Using(t, root)() // Create a function at root @@ -795,9 +795,9 @@ func TestClient_RunTimeout(t *testing.T) { // process, erroring if run on a directory uncreated. func TestClient_Update(t *testing.T) { var ( - root = "testdata/example.com/testUpdate" - expectedName = "testUpdate" - expectedImage = "example.com/alice/testUpdate:latest" + root = "testdata/example.com/test-update" + expectedName = "test-update" + expectedImage = "example.com/alice/test-update:latest" builder = mock.NewBuilder() pusher = mock.NewPusher() deployer = mock.NewDeployerWithResult(fn.DeploymentResult{ @@ -954,8 +954,8 @@ func TestClient_Deploy_RegistryUpdate(t *testing.T) { // the function with the name of the function at the provided root. func TestClient_Remove_ByPath(t *testing.T) { var ( - root = "testdata/example.com/testRemoveByPath" - expectedName = "testRemoveByPath" + root = "testdata/example.com/test-remove-by-path" + expectedName = "test-remove-by-path" remover = mock.NewRemover() ) @@ -993,8 +993,8 @@ func TestClient_Remove_ByPath(t *testing.T) { // the function with the name of the function at the provided root. func TestClient_Remove_DeleteAll(t *testing.T) { var ( - root = "testdata/example.com/testRemoveDeleteAll" - expectedName = "testRemoveDeleteAll" + root = "testdata/example.com/test-remove-delete-all" + expectedName = "test-remove-delete-all" remover = mock.NewRemover() pipelinesProvider = mock.NewPipelinesProvider() deleteAll = true @@ -1039,8 +1039,8 @@ func TestClient_Remove_DeleteAll(t *testing.T) { // the function with the name of the function at the provided root. func TestClient_Remove_Dont_DeleteAll(t *testing.T) { var ( - root = "testdata/example.com/testRemoveDontDeleteAll" - expectedName = "testRemoveDontDeleteAll" + root = "testdata/example.com/test-remove-dont-delete-all" + expectedName = "test-remove-dont-delete-all" remover = mock.NewRemover() pipelinesProvider = mock.NewPipelinesProvider() deleteAll = false @@ -1351,7 +1351,7 @@ func TestClient_Deploy_UnbuiltErrors(t *testing.T) { // TestClient_New_BuilderImagesPersisted Asserts that the client preserves user- // provided Builder Images func TestClient_New_BuildersPersisted(t *testing.T) { - root := "testdata/example.com/testConfiguredBuilders" // Root from which to run the test + root := "testdata/example.com/test-configured-builders" // Root from which to run the test defer Using(t, root)() client := fn.New(fn.WithRegistry(TestRegistry)) @@ -1388,7 +1388,7 @@ func TestClient_New_BuildersPersisted(t *testing.T) { // TestClient_New_BuildpacksPersisted ensures that provided buildpacks are // persisted on new functions. func TestClient_New_BuildpacksPersisted(t *testing.T) { - root := "testdata/example.com/testConfiguredBuildpacks" // Root from which to run the test + root := "testdata/example.com/test-configured-buildpacks" // Root from which to run the test defer Using(t, root)() buildpacks := []string{ @@ -1504,7 +1504,7 @@ func TestClient_Runtimes(t *testing.T) { // TestClient_New_Timestamp ensures that the creation timestamp is set on functions // which are successfully initialized using the client library. func TestClient_New_Timestamp(t *testing.T) { - root := "testdata/example.com/testCreateStamp" + root := "testdata/example.com/test-create-stamp" defer Using(t, root)() start := time.Now() @@ -1526,7 +1526,7 @@ func TestClient_New_Timestamp(t *testing.T) { // function using a simple HTTP POST method with the invoke message as form // field values (as though a simple form were posted). func TestClient_Invoke_HTTP(t *testing.T) { - root := "testdata/example.com/testInvokeHTTP" + root := "testdata/example.com/test-invoke-http" defer Using(t, root)() // Flag indicating the function was invoked @@ -1639,7 +1639,7 @@ func TestClient_Invoke_HTTP(t *testing.T) { // the invoker is sending the invocation message as a CloudEvent rather than // a standard HTTP form POST. func TestClient_Invoke_CloudEvent(t *testing.T) { - root := "testdata/example.com/testInvokeCloudEvent" + root := "testdata/example.com/test-invoke-cloud-event" defer Using(t, root)() var ( @@ -1730,7 +1730,7 @@ func TestClient_Invoke_CloudEvent(t *testing.T) { // TestClient_Instances ensures that when a function is run (locally) its metadata // is available to other clients inspecting the same function using .Instances func TestClient_Instances(t *testing.T) { - root := "testdata/example.com/testInstances" + root := "testdata/example.com/test-instances" defer Using(t, root)() // A mock runner diff --git a/pkg/functions/function.go b/pkg/functions/function.go index 455ba819c8..d7a09a7304 100644 --- a/pkg/functions/function.go +++ b/pkg/functions/function.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/google/go-containerregistry/pkg/name" "gopkg.in/yaml.v2" fnlabels "knative.dev/func/pkg/k8s/labels" "knative.dev/pkg/ptr" @@ -567,24 +568,19 @@ func (f Function) ImageName() (image string, err error) { f.Registry = strings.Trim(f.Registry, "/") // too defensive? - registryTokens := strings.Split(f.Registry, "/") - if len(registryTokens) == 1 { // only namespace provided: ex. 'alice' - image = DefaultRegistry + "/" + f.Registry + "/" + f.Name - } else if len(registryTokens) == 2 || len(registryTokens) == 3 { - // registry/namespace ('quay.io/alice') or - // registry/parent-namespace/namespace ('quay.io/project/alice') provided - image = f.Registry + "/" + f.Name - } else if len(registryTokens) > 3 { // the name of the image is also provided `quay.io/alice/my.function.name` - return "", fmt.Errorf("registry should be either 'namespace', 'registry/namespace' or 'registry/parent/namespace', the name of the image will be derived from the function name") - } - // Explicitly append :latest tag. We expect source control to drive // versioning, rather than rely on image tags with explicitly pinned version // numbers, as is seen in many serverless solutions. This will be updated // to branch name when we add source-driven canary/ bluegreen deployments. - // For pinning to an exact container image, see ImageWithDigest - return image + ":latest", nil + refStr := f.Registry + "/" + f.Name + ":latest" + + ref, err := name.ParseReference(refStr) + if err != nil { + return "", fmt.Errorf("cannot determine function image: %w", err) + } + + return ref.Name(), nil } // Format yaml unmarshall error to be more human friendly. diff --git a/pkg/functions/function_unit_test.go b/pkg/functions/function_unit_test.go index b0fc6fd18e..d890d73d8b 100644 --- a/pkg/functions/function_unit_test.go +++ b/pkg/functions/function_unit_test.go @@ -140,29 +140,38 @@ func TestFunction_ImageName(t *testing.T) { err error ) tests := []struct { - registry string name string + registry string + funcName string expectedImage string expectError bool }{ - {"alice", "myfunc", DefaultRegistry + "/alice/myfunc:latest", false}, - {"quay.io/alice", "myfunc", "quay.io/alice/myfunc:latest", false}, - {"docker.io/alice", "myfunc", "docker.io/alice/myfunc:latest", false}, - {"docker.io/alice/sub", "myfunc", "docker.io/alice/sub/myfunc:latest", false}, - {"alice", "", "", true}, - {"", "myfunc", "", true}, + {"short-name", "alice", "myfunc", DefaultRegistry + "/alice/myfunc:latest", false}, + {"short-name-trailing-slash", "alice/", "myfunc", DefaultRegistry + "/alice/myfunc:latest", false}, + {"full-name-quay-io", "quay.io/alice", "myfunc", "quay.io/alice/myfunc:latest", false}, + {"full-name-docker-io", "docker.io/alice", "myfunc", DefaultRegistry + "/alice/myfunc:latest", false}, + {"full-name-with-sub-path", "docker.io/alice/sub", "myfunc", DefaultRegistry + "/alice/sub/myfunc:latest", false}, + {"localhost-direct", "localhost:5000", "myfunc", "localhost:5000/myfunc:latest", false}, + {"full-name-with-sub-sub-path", "us-central1-docker.pkg.dev/my-gcpproject/team/user", "myfunc", "us-central1-docker.pkg.dev/my-gcpproject/team/user/myfunc:latest", false}, + {"missing-func-name", "alice", "", "", true}, + {"missing-registry", "", "myfunc", "", true}, } for _, test := range tests { - f = Function{Registry: test.registry, Name: test.name} - got, err = f.ImageName() - if test.expectError && err == nil { - t.Errorf("registry '%v' and name '%v' did not yield the expected error", - test.registry, test.name) - } - if got != test.expectedImage { - t.Errorf("expected registry '%v' name '%v' to yield image '%v', got '%v'", - test.registry, test.name, test.expectedImage, got) - } + t.Run(test.name, func(t *testing.T) { + f = Function{Registry: test.registry, Name: test.funcName} + got, err = f.ImageName() + if test.expectError && err == nil { + t.Errorf("registry '%v' and name '%v' did not yield the expected error", + test.registry, test.funcName) + } + if !test.expectError && err != nil { + t.Errorf("unexpected error: %v", err) + } + if got != test.expectedImage { + t.Errorf("expected registry '%v' name '%v' to yield image '%v', got '%v'", + test.registry, test.funcName, test.expectedImage, got) + } + }) } }