diff --git a/cmd/deploy.go b/cmd/deploy.go index 28451e1a51..177af2553a 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -4,6 +4,8 @@ import ( "errors" "fmt" "io" + "os" + "slices" "strconv" "strings" @@ -11,6 +13,7 @@ import ( "github.com/google/go-containerregistry/pkg/name" "github.com/ory/viper" "github.com/spf13/cobra" + "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/api/resource" "knative.dev/client/pkg/util" "knative.dev/func/cmd/common" @@ -81,6 +84,15 @@ DESCRIPTION eliminating the need for a local container engine. To trigger deployment of a git repository instead of local source, combine with '--source': '{{rootCmdUse}} deploy --remote --source=git.example.com/alice/f.git' + A branch, tag or commit is given with '--revision': + '{{rootCmdUse}} deploy --remote --source=git.example.com/alice/f.git --revision=v1.2.0' + The function is then read from the repository, so no local copy is + needed, and nothing is written locally: to change the function, clone + the repository, edit it and deploy the working tree. Choose the + directory within the repository with '--source-dir'. The function is + deployed as configured by the func.yaml committed there: of the flags + which configure a function, only '--registry' and '--registry-insecure' + apply. Domain When deploying, a function's route is automatically generated using the @@ -268,6 +280,11 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { // Initialize config first cfg = newDeployConfig(cmd) + // A git repository deployed remotely needs no local function + if cfg.Remote && cfg.Source != "" { + return runDeployFromGit(cmd, cfg, newClient) + } + // Create function object to check if initialized if f, err = fn.NewFunction(cfg.Path); err != nil { return @@ -275,16 +292,7 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { // Check if function exists BEFORE prompting for config if !f.Initialized() { - if !cfg.Remote || f.Build.Source.URL == "" { - // Only error if this is not a fully remote build - return NewErrNotInitializedFromPath(f.Root, "deploy") - } else { - // TODO: this case is not supported because the pipeline - // implementation requires the function's name, which is in the - // remote repository. We should inspect the remote repository. - // For now, give a more helpful error. - return errors.New("please ensure the function's source is also available locally") - } + return NewErrNotInitializedFromPath(f.Root, "deploy") } // Now that we know function exists, proceed with prompting @@ -579,7 +587,10 @@ type deployConfig struct { // clusters). If not provided, the currently configured namespace will be // used. For instance, that which would be used by default by `kubectl` // (~/.kube/config) in the case of Kubernetes. - Namespace string + // The survey tag routes the namespace prompt's answer here: by name alone + // survey would pick the embedded config.Global.Namespace, which Configure + // records as the namespace the function is already deployed in. + Namespace string `survey:"namespace"` //Service account to be used in deployed function ServiceAccountName string @@ -969,3 +980,89 @@ func warnExposeIgnore(w io.Writer, expose, deployer string) { "support external exposure via this field.\n", expose) } } + +// gitFlags are the only flags a deployment from a git repository takes: +// those which locate the function, the registry, which depends on where it +// is deployed, and those which are not the function's settings. The function +// is otherwise deployed as configured by the func.yaml committed there. +var gitFlags = []string{"remote", "source", "revision", "source-dir", "registry", + "registry-insecure", "registry-authfile", "username", "password", "token", "verbose"} + +// runDeployFromGit deploys the function committed to a git repository with a +// remote pipeline, which builds the commit the function was read at. The +// function is read into memory and nothing is written locally: to change it, +// clone the repository, edit it and deploy the working tree. Settings the +// function leaves empty default as for any function, never to those of a +// function in the current directory. +func runDeployFromGit(cmd *cobra.Command, cfg deployConfig, newClient ClientFactory) (err error) { + given := func(flag string) bool { + env := "FUNC_" + strings.ToUpper(strings.ReplaceAll(flag, "-", "_")) + return cmd.Flags().Changed(flag) || os.Getenv(env) != "" + } + var extra []string + cmd.Flags().VisitAll(func(fl *pflag.Flag) { + if given(fl.Name) && !slices.Contains(gitFlags, fl.Name) { + extra = append(extra, "--"+fl.Name) + } + }) + if len(extra) > 0 { + return fmt.Errorf("%s cannot be used with --source: the function is deployed as "+ + "configured by the func.yaml in the repository. To change it, clone the "+ + "repository, edit it and deploy the working tree", strings.Join(extra, ", ")) + } + + // The revision may also be given as the URL's fragment (#), + // which then wins, as it does in Configure. + source := fn.Source{URL: cfg.Source, Revision: cfg.Revision, Dir: cfg.SourceDir} + if parts := strings.SplitN(cfg.Source, "#", 2); len(parts) == 2 { + source.URL, source.Revision = parts[0], parts[1] + } + f, err := fn.NewFunctionFromGit(cmd.Context(), source) + if err != nil { + return + } + + global, err := config.NewDefault() + if err != nil { + return + } + if f.Build.Builder == "" { + f.Build.Builder = global.Builder + } + if given("registry") { + f.Registry = cfg.Registry + } + if f.Registry == "" { + f.Registry = global.Registry + } + if given("registry-insecure") { + f.RegistryInsecure = cfg.RegistryInsecure + } + f.Namespace = defaultNamespace(f, cfg.Verbose) + if err = f.Validate(); err != nil { + return + } + + // The client is configured by cfg, whose defaults came from the current + // directory. + cfg.Builder, cfg.Registry, cfg.RegistryInsecure = f.Build.Builder, f.Registry, f.RegistryInsecure + clientOptions, err := cfg.clientOptions() + if err != nil { + return + } + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, InsecureSkipVerify: cfg.RegistryInsecure}, clientOptions...) + defer done() + + url, f, err := client.RunPipeline(cmd.Context(), f) + if errors.Is(err, fn.ErrRegistryRequired) { + return NewErrRegistryRequired(err, "deploy") + } else if err != nil { + return wrapDeploymentError(err) + } + fmt.Fprintf(cmd.OutOrStdout(), "Function Deployed at %v\n", url) + if fn.ExposureRecordMissing(f.Expose, f.Deploy.Expose, f.Deploy.Deployer) { + fmt.Fprintf(cmd.OutOrStderr(), "Warning: expose %q was requested but the cluster's "+ + "func-util image applied no external exposure; the function is running cluster-local\n", f.Expose) + } + return nil +} diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index 2e636d4289..69f4e27e22 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -8,12 +8,15 @@ import ( "os" "path/filepath" "reflect" + "slices" "strings" "testing" "time" + "github.com/AlecAivazis/survey/v2/core" "github.com/ory/viper" "github.com/spf13/cobra" + "github.com/spf13/pflag" "knative.dev/func/pkg/builders" "knative.dev/func/pkg/config" @@ -533,58 +536,288 @@ func testFunctionContext(cmdFn commandConstructor, t *testing.T) { } } -// TestDeploy_RemoteSourcePersists ensures that the source flags, if provided, -// are persisted to the Function for subsequent deployments. -func TestDeploy_RemoteSourcePersists(t *testing.T) { +// funcYAML is the content of a func.yaml for a go function of the given +// name, with extra appended as further top-level keys. +func funcYAML(name, extra string) string { + return "specVersion: " + fn.LastSpecVersion() + "\nname: " + name + + "\nruntime: go\ncreated: 2024-01-01T00:00:00Z\n" + extra +} + +// serveFunction serves a git repository holding, on main, a func.yaml for +// a go function of the given name, and returns the repository's URL. +func serveFunction(t *testing.T, name string) string { + t.Helper() + url, _ := ServeGitRepository(t, map[string]map[string]string{ + "main": {"func.yaml": funcYAML(name, "")}, + }) + return url +} + +// TestDeploy_RemoteGitNoLocalFunction ensures a remote deployment of a git +// repository needs no local copy of the function: the function is read from +// the repository, and nothing is written to the (empty) current directory. +// +// func deploy --remote --source={url} +// +// https://github.com/knative/func/issues/3203 +func TestDeploy_RemoteGitNoLocalFunction(t *testing.T) { root := FromTempDirectory(t) + // Only the requested branch and directory hold a function named + // remote-fn, so deploying it shows both were honoured. + url, heads := ServeGitRepository(t, map[string]map[string]string{ + "main": {"func.yaml": funcYAML("root-fn", "")}, + "feature": {"functions/remote-fn/func.yaml": funcYAML("remote-fn", "")}, + }) - var ( - url = "https://example.com/user/repo" - branch = "main" - dir = "function" - ) + pipeliner := mock.NewPipelinesProvider() + var deployed fn.Function + base := pipeliner.RunFn + pipeliner.RunFn = func(f fn.Function) (string, fn.Function, error) { + deployed = f + return base(f) + } - // Create a new Function in the temp directory - f, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}) + cmd := NewDeployCmd(NewTestClient( + fn.WithPipelinesProvider(pipeliner), + fn.WithRegistry(TestRegistry), + )) + cmd.SetArgs([]string{"--remote", + "--source=" + url, + "--revision=feature", + "--source-dir=functions/remote-fn"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + // The pipeline received the function from the repository and builds the + // commit it was read at + want := fn.Source{URL: url, Revision: "feature", Dir: "functions/remote-fn", Commit: heads["feature"]} + if !pipeliner.RunInvoked { + t.Fatal("pipeline was not invoked") + } + if deployed.Name != "remote-fn" { + t.Errorf("expected the repository's function to be deployed, got %q", deployed.Name) + } + if deployed.Root != "" { + t.Errorf("expected no root, got %q", deployed.Root) + } + if deployed.Build.Source != want { + t.Errorf("expected source %+v, got %+v", want, deployed.Build.Source) + } + // Nothing was written locally + if _, err := os.Stat(filepath.Join(root, fn.FunctionFile)); !os.IsNotExist(err) { + t.Errorf("expected no %s to be written, got err %v", fn.FunctionFile, err) + } +} + +// TestDeploy_RemoteGitWritesNothing ensures a remote deployment of a git +// repository is a deployment by reference: the pipeline is created for the +// function as committed in the repository, and whatever function is in the +// current directory, even a copy of the same one, is neither written to nor +// marked as built. +func TestDeploy_RemoteGitWritesNothing(t *testing.T) { + root := FromTempDirectory(t) + // Only the repository's copy of my-fn has this environment variable + url, _ := ServeGitRepository(t, map[string]map[string]string{ + "main": {"func.yaml": funcYAML("my-fn", `run: + envs: + - name: REPO_ONLY + value: "yes" +`)}, + }) + + if _, err := fn.New().Init(fn.Function{Name: "my-fn", Runtime: "go", Root: root}); err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(filepath.Join(root, fn.FunctionFile)) if err != nil { t.Fatal(err) } - // Deploy the Function specifying all of the source flags + pipeliner := mock.NewPipelinesProvider() + var deployed fn.Function + base := pipeliner.RunFn + pipeliner.RunFn = func(f fn.Function) (string, fn.Function, error) { + deployed = f + return base(f) + } cmd := NewDeployCmd(NewTestClient( - fn.WithPipelinesProvider(mock.NewPipelinesProvider()), + fn.WithPipelinesProvider(pipeliner), fn.WithRegistry(TestRegistry), )) - cmd.SetArgs([]string{"--remote", "--source=" + url, "--revision=" + branch, "--source-dir=" + dir, "."}) + cmd.SetArgs([]string{"--remote", "--source=" + url}) if err := cmd.Execute(); err != nil { t.Fatal(err) } - // Load the Function and ensure the flags were stored. - f, err = fn.NewFunction(root) + if len(deployed.Run.Envs) != 1 { + t.Errorf("expected the repository's function to be deployed, got envs %v", deployed.Run.Envs) + } + after, err := os.ReadFile(filepath.Join(root, fn.FunctionFile)) + if err != nil { + t.Fatal(err) + } + if string(before) != string(after) { + t.Errorf("expected the local function to be untouched, got:\n%s", after) + } + local, err := fn.NewFunction(root) if err != nil { t.Fatal(err) } - if want := (fn.Source{URL: url, Revision: branch, Dir: dir}); f.Build.Source != want { - t.Errorf("expected source %+v persisted, got %+v", want, f.Build.Source) + if local.Built() { + t.Error("expected the local sources, which were not built, not to be stamped as built") } } -// TestDeploy_RemoteSourceEnv ensures the source flags are also read from -// their environment variables, FUNC_SOURCE, FUNC_REVISION and -// FUNC_SOURCE_DIR. -func TestDeploy_RemoteSourceEnv(t *testing.T) { +// TestDeploy_RemoteGitDefaultsFromRepository ensures the function is deployed +// as configured by the repository's func.yaml, except for the registry (and +// whether it is insecure) given as a flag or environment variable, and that +// the settings it leaves empty default as for any function. The function in +// the current directory, from which the command's flag defaults were derived, +// plays no part. +func TestDeploy_RemoteGitDefaultsFromRepository(t *testing.T) { root := FromTempDirectory(t) - - if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}); err != nil { + url, _ := ServeGitRepository(t, map[string]map[string]string{ + "main": {"func.yaml": funcYAML("remote-fn", `registry: example.com/repo +build: + builder: s2i + pvcSize: 2Gi +deploy: + namespace: repo-ns +`)}, + "bare": {"func.yaml": funcYAML("bare-fn", "")}, + }) + if _, err := fn.New().Init(fn.Function{Name: "local-fn", Runtime: "go", Root: root, + Registry: "example.com/local", + Namespace: "local-ns", + Build: fn.BuildSpec{Builder: "s2i", PVCSize: "5Gi"}, + }); err != nil { t.Fatal(err) } - t.Setenv("FUNC_SOURCE", "https://example.com/user/repo") - t.Setenv("FUNC_REVISION", "v1.2.0") + + deploy := func(t *testing.T, args ...string) fn.Function { + t.Helper() + var deployed fn.Function + pipeliner := mock.NewPipelinesProvider() + base := pipeliner.RunFn + pipeliner.RunFn = func(f fn.Function) (string, fn.Function, error) { + deployed = f + return base(f) + } + cmd := NewDeployCmd(NewTestClient(fn.WithPipelinesProvider(pipeliner), fn.WithRegistry(TestRegistry))) + cmd.SetArgs(append([]string{"--remote", "--source=" + url}, args...)) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + return deployed + } + + // The repository's values apply + f := deploy(t, "--revision=main") + if f.Name != "remote-fn" || f.Build.Builder != "s2i" || f.Registry != "example.com/repo" || + f.Build.PVCSize != "2Gi" || f.Namespace != "repo-ns" { + t.Errorf("expected the repository's func.yaml to configure the function, got name=%q builder=%q registry=%q pvc=%q namespace=%q", + f.Name, f.Build.Builder, f.Registry, f.Build.PVCSize, f.Namespace) + } + + // Empty settings default as for any function, not to the local function's + f = deploy(t, "--revision=bare") + if f.Build.Builder != builders.Default || f.Build.PVCSize != "" || f.Registry == "example.com/local" || f.Namespace == "local-ns" { + t.Errorf("expected the defaults of a function, got builder=%q pvc=%q registry=%q namespace=%q", + f.Build.Builder, f.Build.PVCSize, f.Registry, f.Namespace) + } + + // The registry may be given as an environment variable or a flag + t.Setenv("FUNC_REGISTRY", "example.com/env") + if f = deploy(t, "--revision=main"); f.Registry != "example.com/env" { + t.Errorf("expected the registry from the environment, got %q", f.Registry) + } + if f = deploy(t, "--revision=main", "--registry=example.com/flag"); f.Registry != "example.com/flag" { + t.Errorf("expected the registry from the flag, got %q", f.Registry) + } + if f = deploy(t, "--revision=main", "--registry-insecure"); !f.RegistryInsecure { + t.Error("expected the registry to be insecure, as the flag says") + } +} + +// TestDeploy_RemoteGitRejectsFlags ensures that a deployment from a git +// repository takes none of the flags which configure the function, whether +// given as a flag or an environment variable: the function is deployed as +// committed, so they fail the deployment instead of being dropped. +func TestDeploy_RemoteGitRejectsFlags(t *testing.T) { + _ = FromTempDirectory(t) + url := serveFunction(t, "remote-fn") + + var rejected []string + NewDeployCmd(NewTestClient()).Flags().VisitAll(func(fl *pflag.Flag) { + if !slices.Contains(gitFlags, fl.Name) { + rejected = append(rejected, fl.Name) + } + }) + for _, flag := range rejected { + t.Run(flag, func(t *testing.T) { + pipeliner := mock.NewPipelinesProvider() + cmd := NewDeployCmd(NewTestClient(fn.WithPipelinesProvider(pipeliner), fn.WithRegistry(TestRegistry))) + cmd.SetArgs([]string{"--remote", "--source=" + url, "--" + flag + "=true"}) + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "--"+flag+" cannot be used with --source") { + t.Fatalf("expected --%s to be rejected, got %v", flag, err) + } + if pipeliner.RunInvoked { + t.Error("pipeline should not run") + } + }) + } + + t.Setenv("FUNC_NAMESPACE", "env-ns") + cmd := NewDeployCmd(NewTestClient(fn.WithPipelinesProvider(mock.NewPipelinesProvider()), fn.WithRegistry(TestRegistry))) + cmd.SetArgs([]string{"--remote", "--source=" + url}) + if err := cmd.Execute(); err == nil || !strings.Contains(err.Error(), "--namespace cannot be used with --source") { + t.Fatalf("expected FUNC_NAMESPACE to be rejected, got %v", err) + } +} + +// TestDeploy_RemoteGitLoadError ensures a repository the function cannot be +// read from fails the deployment before any pipeline is run. +func TestDeploy_RemoteGitLoadError(t *testing.T) { + _ = FromTempDirectory(t) + url := serveFunction(t, "remote-fn") + + pipeliner := mock.NewPipelinesProvider() + cmd := NewDeployCmd(NewTestClient(fn.WithPipelinesProvider(pipeliner), fn.WithRegistry(TestRegistry))) + cmd.SetArgs([]string{"--remote", "--source=" + url, "--revision=nope"}) + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Fatalf("expected the loader's error, got %v", err) + } + if pipeliner.RunInvoked { + t.Error("pipeline should not run when the function cannot be read") + } +} + +// TestDeploy_RemoteSourceEnv ensures the source flags are also read from +// their environment variables, FUNC_SOURCE, FUNC_REVISION and +// FUNC_SOURCE_DIR: the function is read from the repository they name. +func TestDeploy_RemoteSourceEnv(t *testing.T) { + _ = FromTempDirectory(t) + // Only the named branch and directory hold a function named repo-fn + url, heads := ServeGitRepository(t, map[string]map[string]string{ + "main": {"func.yaml": funcYAML("root-fn", "")}, + "release": {"function/func.yaml": funcYAML("repo-fn", "")}, + }) + t.Setenv("FUNC_SOURCE", url) + t.Setenv("FUNC_REVISION", "release") t.Setenv("FUNC_SOURCE_DIR", "function") + pipeliner := mock.NewPipelinesProvider() + var deployed fn.Function + base := pipeliner.RunFn + pipeliner.RunFn = func(f fn.Function) (string, fn.Function, error) { + deployed = f + return base(f) + } cmd := NewDeployCmd(NewTestClient( - fn.WithPipelinesProvider(mock.NewPipelinesProvider()), + fn.WithPipelinesProvider(pipeliner), fn.WithRegistry(TestRegistry), )) cmd.SetArgs([]string{"--remote"}) @@ -592,26 +825,29 @@ func TestDeploy_RemoteSourceEnv(t *testing.T) { t.Fatal(err) } - f, err := fn.NewFunction(root) - if err != nil { - t.Fatal(err) - } - want := fn.Source{URL: "https://example.com/user/repo", Revision: "v1.2.0", Dir: "function"} - if f.Build.Source != want { - t.Errorf("expected source %+v from the environment, got %+v", want, f.Build.Source) + want := fn.Source{URL: url, Revision: "release", Dir: "function", Commit: heads["release"]} + if deployed.Name != "repo-fn" || deployed.Build.Source != want { + t.Errorf("expected repo-fn read from %+v (the environment), got %q from %+v", want, deployed.Name, deployed.Build.Source) } } // TestDeploy_RemoteSourceUsed ensures that any source values provided as flags are used -// when invoking a remote deployment. +// when invoking a remote deployment: to read the function from the repository +// and as the pipeline's source. func TestDeploy_RemoteSourceUsed(t *testing.T) { root := FromTempDirectory(t) var ( - url = "https://example.com/user/repo" branch = "main" dir = "function" ) + // Only the requested directory holds a function named repo-fn + url, _ := ServeGitRepository(t, map[string]map[string]string{ + "main": { + "func.yaml": funcYAML("root-fn", ""), + "function/func.yaml": funcYAML("repo-fn", ""), + }, + }) // Create a new Function in the temp dir _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}) if err != nil { @@ -621,6 +857,9 @@ func TestDeploy_RemoteSourceUsed(t *testing.T) { // A Pipelines Provider which will validate the expected values were received pipeliner := mock.NewPipelinesProvider() pipeliner.RunFn = func(f fn.Function) (string, fn.Function, error) { + if f.Name != "repo-fn" { + t.Errorf("expected the function read from %s at %s, got %q", dir, branch, f.Name) + } if f.Build.Source.URL != url { t.Errorf("Pipeline Provider expected git URL '%v' got '%v'", url, f.Build.Source.URL) } @@ -644,27 +883,35 @@ func TestDeploy_RemoteSourceUsed(t *testing.T) { if err := cmd.Execute(); err != nil { t.Fatal(err) } + if !pipeliner.RunInvoked { + t.Fatal("pipeline was not invoked") + } } -// TestDeploy_RemoteSourceFragment ensures that a --source which specifies the branch -// in the URL is equivalent to providing --revision +// TestDeploy_RemoteSourceFragment ensures that a --source which specifies the +// branch in the URL is equivalent to providing --revision: the function is +// read from, and the pipeline builds, the repository at that branch. func TestDeploy_RemoteSourceFragment(t *testing.T) { - root := FromTempDirectory(t) - - f, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}) - if err != nil { - t.Fatal(err) - } + _ = FromTempDirectory(t) + // Only the branch named in the fragment holds a function named repo-fn + expectedUrl, _ := ServeGitRepository(t, map[string]map[string]string{ + "main": {"func.yaml": funcYAML("root-fn", "")}, + "branch": {"func.yaml": funcYAML("repo-fn", "")}, + }) var ( - url = "https://example.com/user/repo#branch" - expectedUrl = "https://example.com/user/repo" + url = expectedUrl + "#branch" expectedBranch = "branch" ) + pipeliner := mock.NewPipelinesProvider() + var deployed fn.Function + base := pipeliner.RunFn + pipeliner.RunFn = func(f fn.Function) (string, fn.Function, error) { + deployed = f + return base(f) + } cmd := NewDeployCmd(NewTestClient( - fn.WithDeployer(mock.NewDeployer()), - fn.WithBuilder(mock.NewBuilder()), - fn.WithPipelinesProvider(mock.NewPipelinesProvider()), + fn.WithPipelinesProvider(pipeliner), fn.WithRegistry(TestRegistry), )) cmd.SetArgs([]string{"--remote", "--source=" + url}) @@ -672,16 +919,11 @@ func TestDeploy_RemoteSourceFragment(t *testing.T) { if err := cmd.Execute(); err != nil { t.Fatal(err) } - - f, err = fn.NewFunction(root) - if err != nil { - t.Fatal(err) - } - if f.Build.Source.URL != expectedUrl { - t.Errorf("expected git URL '%v' got '%v'", expectedUrl, f.Build.Source.URL) + if deployed.Name != "repo-fn" { + t.Errorf("expected the function to be read from %q at %q, got %q", expectedUrl, expectedBranch, deployed.Name) } - if f.Build.Source.Revision != expectedBranch { - t.Errorf("expected git branch '%v' got '%v'", expectedBranch, f.Build.Source.Revision) + if deployed.Build.Source.URL != expectedUrl || deployed.Build.Source.Revision != expectedBranch { + t.Errorf("expected the pipeline to build %q at %q, got %+v", expectedUrl, expectedBranch, deployed.Build.Source) } } @@ -1100,6 +1342,28 @@ func TestDeploy_Namespace(t *testing.T) { } } +// TestDeploy_NamespacePromptAnswer ensures the answer to the namespace prompt +// (--confirm) becomes the namespace to deploy to. deployConfig shadows the +// Namespace of its embedded config.Global, and survey matched that one by +// name: the function was deployed to the default namespace and, deployed +// locally, a function of the same name in the chosen namespace was removed. +func TestDeploy_NamespacePromptAnswer(t *testing.T) { + cfg := deployConfig{Namespace: "default"} + if err := core.WriteAnswer(&cfg, "namespace", "chosen"); err != nil { + t.Fatal(err) + } + f, err := cfg.Configure(fn.Function{Name: "f", Runtime: "go", Root: t.TempDir()}) + if err != nil { + t.Fatal(err) + } + if f.Namespace != "chosen" { + t.Errorf("expected the answered namespace to be deployed to, got %q", f.Namespace) + } + if f.Deploy.Namespace != "" { + t.Errorf("expected no namespace recorded as deployed, got %q", f.Deploy.Namespace) + } +} + // TestDeploy_NamespaceDefaultsToK8sContext ensures that when not specified, a // users's active kubernetes context is used for the namespace if available. func TestDeploy_NamespaceDefaultsToK8sContext(t *testing.T) { @@ -1623,7 +1887,7 @@ func TestDeploy_RemoteBuildURLPermutations(t *testing.T) { var ( remoteValues = []string{"", "true", "false"} buildValues = []string{"", "true", "false", "auto"} - urlValues = []string{"", "https://example.com/user/repo"} + urlValues = []string{"", serveFunction(t, "repo-fn")} // toArgs converts one permutaton of the values into command arguments toArgs = func(remote, build, url string) []string { @@ -1670,6 +1934,14 @@ func TestDeploy_RemoteBuildURLPermutations(t *testing.T) { // Assertions if remote != "" && remote != "false" { // the default of "" is == false + // A function deployed from git takes no --build + if url != "" && build != "" { + if err == nil || !strings.Contains(err.Error(), "--build cannot be used with --source") { + t.Fatalf("expected --build to be rejected, got %v", err) + } + return + } + // REMOTE Assertions if !pipeliner.RunInvoked { // Remote deployer should be triggered t.Error("remote was not invoked") @@ -1836,36 +2108,37 @@ func TestDeploy_UnsetFlag(t *testing.T) { t.Fatal(err) } - // Deploy it, specifying a Git URL - cmd := NewDeployCmd(NewTestClient()) - cmd.SetArgs([]string{"--remote", "--source=https://git.example.com/alice/f"}) + // Deploy it, specifying a domain + client := NewTestClient(fn.WithDeployer(mock.NewDeployer()), fn.WithBuilder(mock.NewBuilder()), fn.WithRegistry(TestRegistry)) + cmd := NewDeployCmd(client) + cmd.SetArgs([]string{"--domain=example.com"}) if err := cmd.Execute(); err != nil { t.Fatal(err) } - // Load the function and confirm the URL was persisted + // Load the function and confirm the domain was persisted f, err = fn.NewFunction(root) if err != nil { t.Fatal(err) } - if f.Build.Source.URL != "https://git.example.com/alice/f" { - t.Fatalf("url not persisted") + if f.Domain != "example.com" { + t.Fatalf("domain not persisted") } // Deploy it again, unsetting the value - cmd = NewDeployCmd(NewTestClient()) - cmd.SetArgs([]string{"--source="}) + cmd = NewDeployCmd(client) + cmd.SetArgs([]string{"--domain="}) if err := cmd.Execute(); err != nil { t.Fatal(err) } - // Load the function and confirm the URL was unset + // Load the function and confirm the domain was unset f, err = fn.NewFunction(root) if err != nil { t.Fatal(err) } - if f.Build.Source.URL != "" { - t.Fatalf("url not cleared") + if f.Domain != "" { + t.Fatalf("domain not cleared") } } @@ -3041,9 +3314,9 @@ func TestDeploy_RemoteExposeRecordsObservation(t *testing.T) { base := pipeliner.RunFn pipeliner.RunFn = func(f fn.Function) (string, fn.Function, error) { // add exposure tracking to the base RunFn - url, f, err := base(f) + endpoint, f, err := base(f) f.Deploy.Expose = tt.observed - return url, f, err + return endpoint, f, err } cmd := NewDeployCmd(NewTestClient( @@ -3053,9 +3326,7 @@ func TestDeploy_RemoteExposeRecordsObservation(t *testing.T) { var out bytes.Buffer cmd.SetOut(&out) cmd.SetErr(&out) - cmd.SetArgs([]string{"--remote", - "--source=https://example.com/user/repo", - "--deployer=raw", "--expose=route"}) + cmd.SetArgs([]string{"--remote", "--deployer=raw", "--expose=route"}) if err := cmd.Execute(); err != nil { t.Fatal(err) } diff --git a/docs/reference/func_deploy.md b/docs/reference/func_deploy.md index 3e3013629b..2fd8b0e5c5 100644 --- a/docs/reference/func_deploy.md +++ b/docs/reference/func_deploy.md @@ -57,6 +57,15 @@ DESCRIPTION eliminating the need for a local container engine. To trigger deployment of a git repository instead of local source, combine with '--source': 'func deploy --remote --source=git.example.com/alice/f.git' + A branch, tag or commit is given with '--revision': + 'func deploy --remote --source=git.example.com/alice/f.git --revision=v1.2.0' + The function is then read from the repository, so no local copy is + needed, and nothing is written locally: to change the function, clone + the repository, edit it and deploy the working tree. Choose the + directory within the repository with '--source-dir'. The function is + deployed as configured by the func.yaml committed there: of the flags + which configure a function, only '--registry' and '--registry-insecure' + apply. Domain When deploying, a function's route is automatically generated using the diff --git a/e2e/e2e_remote_test.go b/e2e/e2e_remote_test.go index e122ee91d2..e36b4c2215 100644 --- a/e2e/e2e_remote_test.go +++ b/e2e/e2e_remote_test.go @@ -5,7 +5,6 @@ package e2e import ( "fmt" "os" - "os/exec" "path/filepath" "testing" "time" @@ -42,25 +41,18 @@ func TestRemote_Deploy(t *testing.T) { } // TestRemote_Source ensures a remote build can be triggered which pulls -// source from a remote repository. +// source from a remote repository, with no local copy of the function. // -// func deploy --remote --source={url} --registry={} --builder=pack +// func deploy --remote --source={url} --registry={} func TestRemote_Source(t *testing.T) { name := "func-e2e-test-remote-source" _ = fromCleanEnv(t, name) - // This command currently requires the function source also be available - // locally in order to use its name. - cmd := exec.Command("git", "clone", "https://github.com/functions-dev/func-e2e-tests", ".") - if err := cmd.Run(); err != nil { - t.Fatal(err) - } - - // Trigger the deploy + // Trigger the deploy from an empty directory: the function is read from + // the repository. if err := newCmd(t, "deploy", "--remote", "--source", "https://github.com/functions-dev/func-e2e-tests", "--registry", Registry, - "--builder", "pack", ).Run(); err != nil { t.Fatal(err) } @@ -77,35 +69,17 @@ func TestRemote_Source(t *testing.T) { // TestRemote_Ref ensures a remote build can be triggered which pulls // source from a specific reference (branch/tag) of a remote repository. +// The function's metadata (name, runtime, etc) is read from that reference, +// so no local checkout is involved. func TestRemote_Ref(t *testing.T) { name := "func-e2e-test-remote-ref" _ = fromCleanEnv(t, name) - // This command currently requires the function source also be available - // locally in order to use its name. - cmd := exec.Command("git", "clone", "https://github.com/functions-dev/func-e2e-tests", ".") - if err := cmd.Run(); err != nil { - t.Fatal(err) - } - - // IMPORTANT: The local func.yaml must match the one in the target branch. - // This is a current limitation where remote builds still require local - // source to determine function metadata (name, runtime, etc). - // TODO: Remove this checkout once the implementation supports fetching - // function metadata from the remote repository. - // https://github.com/knative/func/issues/3203 - cmd = exec.Command("git", "checkout", name) - if err := cmd.Run(); err != nil { - t.Fatal(err) - } - // Trigger the deploy if err := newCmd(t, "deploy", "--remote", "--source", "https://github.com/functions-dev/func-e2e-tests", "--revision", name, "--registry", Registry, - "--builder", "pack", - "--build", ).Run(); err != nil { t.Fatal(err) } @@ -120,39 +94,19 @@ func TestRemote_Ref(t *testing.T) { } // TestRemote_Dir ensures that remote builds can be instructed to build and -// deploy a function located in a subdirectory. +// deploy a function located in a subdirectory of the repository. The +// function's metadata is read from that subdirectory. // -// func deploy --remote --source-dir={subdir} // func deploy --remote --source-dir={subdir} --source={url} func TestRemote_Dir(t *testing.T) { name := "func-e2e-test-remote-dir" _ = fromCleanEnv(t, name) - // This command currently requires the function source also be available - // locally in order to use its name. - cmd := exec.Command("git", "clone", "https://github.com/functions-dev/func-e2e-tests", ".") - if err := cmd.Run(); err != nil { - t.Fatal(err) - } - - // IMPORTANT: When using --source-dir, we need to change to that directory locally - // to ensure the local func.yaml matches the one that will be used in the remote build. - // This is a current limitation where remote builds still require local source to - // determine function metadata (name, runtime, etc). - // TODO: Remove this cd once the implementation supports fetching function metadata - // from the remote repository subdirectory. - // https://github.com/knative/func/issues/3203 - if err := os.Chdir(name); err != nil { - t.Fatalf("failed to change to subdirectory %s: %v", name, err) - } - // Trigger the deploy if err := newCmd(t, "deploy", "--remote", "--source", "https://github.com/functions-dev/func-e2e-tests", "--source-dir", name, "--registry", Registry, - "--builder", "pack", - "--build", ).Run(); err != nil { t.Fatal(err) } diff --git a/pkg/functions/client.go b/pkg/functions/client.go index c926956fae..18fef0f40b 100644 --- a/pkg/functions/client.go +++ b/pkg/functions/client.go @@ -18,7 +18,6 @@ import ( "time" "golang.org/x/sync/errgroup" - "gopkg.in/yaml.v2" "knative.dev/func/pkg/deployers" "knative.dev/func/pkg/utils" ) @@ -1609,11 +1608,8 @@ func hasInitializedFunction(path string) (bool, error) { if err != nil { return false, err } - f := Function{} - if err = yaml.Unmarshal(bb, &f); err != nil { - return false, err - } - if f, err = f.Migrate(); err != nil { + f, err := parseFunction(bb) + if err != nil { return false, err } return f.Initialized(), nil diff --git a/pkg/functions/function.go b/pkg/functions/function.go index 1e6906fbb9..8d4a46e8b4 100644 --- a/pkg/functions/function.go +++ b/pkg/functions/function.go @@ -437,24 +437,10 @@ func NewFunction(root string) (f Function, err error) { if err != nil { return } - var functionMarshallingError error - var functionMigrationError error - if marshallingErr := yaml.Unmarshal(bb, &f); marshallingErr != nil { - functionMarshallingError = formatUnmarshalError(marshallingErr) // human-friendly unmarshalling errors - } - if f, err = f.Migrate(); err != nil { - functionMigrationError = err - } - // Only if migration fail return errors to the user. include marshalling error if present - if functionMigrationError != nil { - //returning both migrations and marshalling errors to the user - errorText := "Error: \n" - if functionMarshallingError != nil { - errorText += "Marshalling: " + functionMarshallingError.Error() - } - errorText += "\n" + "Migration: " + functionMigrationError.Error() - return Function{}, errors.New(errorText) + if f, err = parseFunction(bb); err != nil { + return } + f.Root = root f.Local, err = f.newLocal() if err != nil { @@ -467,13 +453,37 @@ func NewFunction(root string) (f Function, err error) { return } -// Validate function is logically correct, returning a bundled, and quite -// verbose, formatted error detailing any issues. -func (f Function) Validate() error { - if f.Root == "" { - return errors.New("function root path is required") +// parseFunction unmarshals a serialized function (the content of a func.yaml) +// and migrates it to the current spec version. The result has no Root: where +// the bytes came from is the caller's concern. +// +// Unmarshalling errors are reported only when the migration also fails. A +// function whose migration succeeds is accepted even if some of its fields +// did not unmarshal cleanly. +func parseFunction(bb []byte) (f Function, err error) { + f.Build.BuilderImages = make(map[string]string) + f.Deploy.Annotations = make(map[string]string) + var marshallingErr error + if err = yaml.Unmarshal(bb, &f); err != nil { + marshallingErr = formatUnmarshalError(err) // human-friendly unmarshalling errors + } + if f, err = f.migrate(bb); err != nil { + // Return both the migration and any marshalling error to the user + errorText := "Error: \n" + if marshallingErr != nil { + errorText += "Marshalling: " + marshallingErr.Error() + } + errorText += "\n" + "Migration: " + err.Error() + return Function{}, errors.New(errorText) } + return f, nil +} +// Validate function is logically correct, returning a bundled, and quite +// verbose, formatted error detailing any issues. Where the function lives is +// not part of its correctness: a function read from a git repository has no +// Root, which Write requires. +func (f Function) Validate() error { var ctr int errs := [][]string{ validateVolumes(f.Run.Volumes), @@ -596,6 +606,9 @@ func (f Function) MarshalFuncYaml() ([]byte, error) { // Write Function struct (metadata) to Disk at f.Root func (f Function) Write() (err error) { + if f.Root == "" { + return ErrRootRequired + } // Skip writing (and dirtying the work tree) if there were no modifications. f1, _ := NewFunction(f.Root) if reflect.DeepEqual(f, f1) { @@ -743,7 +756,8 @@ func (f Function) HasScaffolding() bool { // https://github.com/knative/func/pull/3436) and can interfere with other // builders (mostly just pack). func WarnIfLegacyS2IScaffolding(f Function, w io.Writer) { - if !f.HasScaffolding() { + // A function without a Root has no working tree to inspect. + if !f.HasScaffolding() || f.Root == "" { return } legacyAssemble := filepath.Join(f.Root, ".s2i", "bin", "assemble") diff --git a/pkg/functions/function_git.go b/pkg/functions/function_git.go new file mode 100644 index 0000000000..ffa4926d8d --- /dev/null +++ b/pkg/functions/function_git.go @@ -0,0 +1,238 @@ +package functions + +import ( + "context" + "errors" + "fmt" + "os" + "path" + "strings" + + "github.com/go-git/go-billy/v5" + "github.com/go-git/go-billy/v5/memfs" + "github.com/go-git/go-billy/v5/util" + "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/config" + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/transport" + "github.com/go-git/go-git/v5/storage/memory" +) + +// NewFunctionFromGit loads the function committed in the repository g +// describes: the func.yaml in g.Dir at g.Revision, which is the remote's +// default branch when empty. A revision may be a branch, a tag or a full +// commit hash. +// +// No working copy is involved. The returned function therefore has no Root +// and none of the state NewFunction reads from one (local settings, last +// built image). Its Build.Source is g, where it was read from, with the +// commit g's revision resolved to: whatever source settings the committed +// func.yaml holds are replaced, so the commit is never paired with another +// repository. +func NewFunctionFromGit(ctx context.Context, g Source) (Function, error) { + if errs := validateSource(g); len(errs) > 0 { + return Function{}, errors.New(strings.Join(errs, "; ")) + } + src, err := resolveGitSource(ctx, g) + if err != nil { + return Function{}, err + } + tree, commit, err := src.checkout(ctx) + if err != nil { + return Function{}, err + } + bb, err := util.ReadFile(tree, path.Join(g.Dir, FunctionFile)) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return Function{}, fmt.Errorf("no %s in %q of %s at %s", FunctionFile, g.Dir, g.URL, src.describe()) + } + return Function{}, fmt.Errorf("cannot read %s from %s: %w", FunctionFile, g.URL, err) + } + f, err := parseFunction(bb) + if err != nil { + return Function{}, err + } + // Where the function was read from, and the commit it was read at, for + // the build to fetch and label. + f.Build.Source = g + f.Build.Source.Commit = commit.String() + return f, nil +} + +// gitSource is a revision of a remote repository, in the form the fetch +// needs it: a ref to clone, a commit to fetch, or neither for the remote's +// default branch. +type gitSource struct { + url string + auth transport.AuthMethod + // ref is the branch or tag to clone. Empty for the default branch, and + // for a bare commit, which hash then names. + ref plumbing.ReferenceName + hash plumbing.Hash +} + +// resolveGitSource decides how g is fetched. An empty revision is the +// remote's default branch, a full ref name (refs/...) and a full commit hash +// are taken as given; none of these needs the remote's refs. A bare name does: +// it is matched against them as git matches one (see gitrevisions), a tag +// before a branch, so a tag wins over a branch of the same name as it does +// for git fetch. Listing the refs costs one round trip and, on large +// repositories, a sizeable advertisement, hence only for bare names. +func resolveGitSource(ctx context.Context, g Source) (gitSource, error) { + src := gitSource{url: g.URL} + if g.URL == "" { + return src, errors.New("git URL required") + } + switch { + case g.Revision == "": + return src, nil + case strings.HasPrefix(g.Revision, "refs/"): + src.ref = plumbing.ReferenceName(g.Revision) + return src, nil + case plumbing.IsHash(g.Revision): + src.hash = plumbing.NewHash(g.Revision) + return src, nil + } + + remote := git.NewRemote(memory.NewStorage(), &config.RemoteConfig{ + Name: git.DefaultRemoteName, + URLs: []string{g.URL}, + }) + var refs []*plumbing.Reference + err := withAuth(g.URL, &src.auth, func(auth transport.AuthMethod) (err error) { + refs, err = remote.ListContext(ctx, &git.ListOptions{Auth: auth}) + return + }) + if err != nil { + return src, fmt.Errorf("cannot list refs of %s: %w", g.URL, err) + } + byName := make(map[plumbing.ReferenceName]bool, len(refs)) + for _, r := range refs { + byName[r.Name()] = true + } + for _, name := range []plumbing.ReferenceName{ + plumbing.NewTagReferenceName(g.Revision), + plumbing.NewBranchReferenceName(g.Revision), + } { + if byName[name] { + src.ref = name + return src, nil + } + } + if isAbbreviatedHash(g.Revision) { + // A remote cannot resolve an abbreviation: it serves objects by their + // full id, and neither can the cluster's fetch. + return src, fmt.Errorf("revision %q not found in %s: a commit must be given as its full hash", g.Revision, g.URL) + } + return src, fmt.Errorf("revision %q not found in %s", g.Revision, g.URL) +} + +// withAuth runs op without credentials and, if the remote asked for +// authentication, once more with those the local git configuration holds +// for url. The credentials that worked are left in auth for later calls. +func withAuth(url string, auth *transport.AuthMethod, op func(transport.AuthMethod) error) error { + err := op(*auth) + if isAuthError(err) && *auth == nil { + if a := credentialsForURL(url); a != nil { + *auth = a + err = op(a) + } + } + return err +} + +// isAbbreviatedHash reports whether s looks like a shortened commit hash. +func isAbbreviatedHash(s string) bool { + if len(s) < 4 || len(s) >= 40 { + return false + } + for _, c := range s { + if !strings.ContainsRune("0123456789abcdef", c) { + return false + } + } + return true +} + +// describe returns the revision for messages: the ref's short name, the +// commit hash, or HEAD for the remote's default branch. +func (s gitSource) describe() string { + switch { + case s.ref != "": + return s.ref.Short() + case !s.hash.IsZero(): + return s.hash.String() + } + return "HEAD" +} + +// checkout fetches the resolved revision, depth one, into memory and returns +// its tree and the commit it is. +func (s *gitSource) checkout(ctx context.Context) (billy.Filesystem, plumbing.Hash, error) { + var repo *git.Repository + err := withAuth(s.url, &s.auth, func(auth transport.AuthMethod) (err error) { + if !s.hash.IsZero() { + // A bare commit cannot be cloned: fetch it by hash and check it out. + repo, err = fetchGitCommit(ctx, s.url, s.hash, auth) + return + } + // An empty ReferenceName clones the remote's default branch. + repo, err = git.CloneContext(ctx, memory.NewStorage(), memfs.New(), &git.CloneOptions{ + URL: s.url, + Auth: auth, + ReferenceName: s.ref, + SingleBranch: true, + Depth: 1, + Tags: git.NoTags, + RecurseSubmodules: git.NoRecurseSubmodules, + }) + return + }) + if err != nil { + return nil, plumbing.ZeroHash, fmt.Errorf("cannot fetch %s at %s: %w", s.url, s.describe(), err) + } + head, err := repo.Head() + if err != nil { + return nil, plumbing.ZeroHash, err + } + wt, err := repo.Worktree() + if err != nil { + return nil, plumbing.ZeroHash, err + } + return wt.Filesystem, head.Hash(), nil +} + +func fetchGitCommit(ctx context.Context, url string, hash plumbing.Hash, auth transport.AuthMethod) (*git.Repository, error) { + repo, err := git.Init(memory.NewStorage(), memfs.New()) + if err != nil { + return nil, err + } + remote, err := repo.CreateRemote(&config.RemoteConfig{ + Name: git.DefaultRemoteName, + URLs: []string{url}, + }) + if err != nil { + return nil, err + } + // Fetching a hash directly requires the server to allow it + // (uploadpack.allowReachableSHA1InWant), as the common hosts do. + err = remote.FetchContext(ctx, &git.FetchOptions{ + Auth: auth, + Depth: 1, + Tags: git.NoTags, + RefSpecs: []config.RefSpec{ + config.RefSpec(hash.String() + ":" + plumbing.NewRemoteReferenceName(git.DefaultRemoteName, "source").String()), + }, + }) + if err != nil { + return nil, err + } + wt, err := repo.Worktree() + if err != nil { + return nil, err + } + if err = wt.Checkout(&git.CheckoutOptions{Hash: hash}); err != nil { + return nil, err + } + return repo, nil +} diff --git a/pkg/functions/function_git_test.go b/pkg/functions/function_git_test.go new file mode 100644 index 0000000000..6555e3144e --- /dev/null +++ b/pkg/functions/function_git_test.go @@ -0,0 +1,194 @@ +package functions_test + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + fn "knative.dev/func/pkg/functions" +) + +// gitFixture is a local repository served over file:// with: +// - main: func.yaml naming "root-fn" and sub/func.yaml naming "sub-fn" +// - tag v1 (annotated) on the first commit of main +// - branch feature: func.yaml naming "feature-fn" +// - branch v1, at feature's head: the same name as the tag, another commit +// - branch legacy: a func.yaml from spec version 0.25.0, needing migration +type gitFixture struct { + url string + main string // full hash of main's head + feature string // full hash of feature's head +} + +func newGitFixture(t *testing.T) gitFixture { + t.Helper() + if runtime.GOOS == "windows" { + // The fixture is served over file://, which go-git only supports + // through the git binary and not from a Windows path. + t.Skip("file:// repositories are not supported on Windows") + } + if _, err := exec.LookPath("git"); err != nil { + t.Skip("No 'git' found in path. Skipping test.") + } + dir := t.TempDir() + run := func(args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=test", "GIT_AUTHOR_EMAIL=test@example.com", + "GIT_COMMITTER_NAME=test", "GIT_COMMITTER_EMAIL=test@example.com") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out) + } + return strings.TrimSpace(string(out)) + } + write := func(rel, name string) { + t.Helper() + p := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + content := "specVersion: " + fn.LastSpecVersion() + "\nname: " + name + "\nruntime: go\ncreated: 2024-01-01T00:00:00Z\n" + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + + run("init", "-q", "-b", "main") + // Let the test fetch a bare commit hash, as the common git hosts allow. + run("config", "uploadpack.allowAnySHA1InWant", "true") + write("func.yaml", "root-fn") + write("sub/func.yaml", "sub-fn") + run("add", ".") + run("commit", "-q", "-m", "initial") + run("tag", "-a", "v1", "-m", "v1") + main := run("rev-parse", "HEAD") + + run("checkout", "-q", "-b", "feature") + write("func.yaml", "feature-fn") + run("commit", "-q", "-am", "feature") + feature := run("rev-parse", "HEAD") + run("branch", "v1") // same name as the tag, a different commit + + run("checkout", "-q", "-b", "legacy", "main") + legacy, err := os.ReadFile(filepath.Join("testdata", "migrations", "v0.34.0", "func.yaml")) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "func.yaml"), legacy, 0o644); err != nil { + t.Fatal(err) + } + run("commit", "-q", "-am", "legacy") + run("checkout", "-q", "main") + + return gitFixture{url: "file://" + dir, main: main, feature: feature} +} + +// TestNewFunctionFromGit ensures a function is loaded from the func.yaml of +// the requested revision and context directory without a local checkout. +func TestNewFunctionFromGit(t *testing.T) { + fx := newGitFixture(t) + tests := []struct { + name string + git fn.Source + wantName string + wantCommit string + }{ + {"default branch", fn.Source{URL: fx.url}, "root-fn", fx.main}, + {"context dir", fn.Source{URL: fx.url, Dir: "sub"}, "sub-fn", fx.main}, + {"branch", fn.Source{URL: fx.url, Revision: "feature"}, "feature-fn", fx.feature}, + {"annotated tag", fn.Source{URL: fx.url, Revision: "v1"}, "root-fn", fx.main}, + {"full ref", fn.Source{URL: fx.url, Revision: "refs/heads/feature"}, "feature-fn", fx.feature}, + {"commit hash", fn.Source{URL: fx.url, Revision: fx.feature}, "feature-fn", fx.feature}, + // A tag wins over a branch of the same name, as for git; the full ref + // name chooses the branch + {"tag over branch", fn.Source{URL: fx.url, Revision: "v1"}, "root-fn", fx.main}, + {"branch by full ref", fn.Source{URL: fx.url, Revision: "refs/heads/v1"}, "feature-fn", fx.feature}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f, err := fn.NewFunctionFromGit(context.Background(), tt.git) + if err != nil { + t.Fatal(err) + } + if f.Name != tt.wantName { + t.Errorf("expected name %q, got %q", tt.wantName, f.Name) + } + if f.Build.Source.Commit != tt.wantCommit { + t.Errorf("expected the function read at %s, got %q", tt.wantCommit, f.Build.Source.Commit) + } + // The function records where it was read from + want := tt.git + want.Commit = tt.wantCommit + if f.Build.Source != want { + t.Errorf("expected source %+v, got %+v", want, f.Build.Source) + } + if f.Runtime != "go" { + t.Errorf("expected runtime go, got %q", f.Runtime) + } + if f.Root != "" { + t.Errorf("expected no root, got %q", f.Root) + } + if !f.Initialized() { + t.Error("expected the function to be initialized") + } + }) + } +} + +// TestNewFunctionFromGit_Migrates ensures a func.yaml of an earlier spec +// version is migrated on load, as NewFunction does for a local checkout: +// migrations read the previous structure from the fetched bytes. +func TestNewFunctionFromGit_Migrates(t *testing.T) { + fx := newGitFixture(t) + f, err := fn.NewFunctionFromGit(context.Background(), fn.Source{URL: fx.url, Revision: "legacy"}) + if err != nil { + t.Fatal(err) + } + if f.SpecVersion != fn.LastSpecVersion() { + t.Errorf("expected spec version %q, got %q", fn.LastSpecVersion(), f.SpecVersion) + } + if f.Name != "testfunc" { + t.Errorf("expected name testfunc, got %q", f.Name) + } + // The legacy func.yaml names another repository (http://test-url, moved + // into the source by migrateToSpecsStructure). The loaded function + // records where it was actually read from instead. + if f.Build.Source.URL != fx.url || f.Build.Source.Revision != "legacy" { + t.Errorf("expected the source it was read from (%s at legacy), got %+v", fx.url, f.Build.Source) + } +} + +// TestNewFunctionFromGit_Errors ensures unknown revisions and directories +// without a func.yaml are reported, not silently returned as empty functions. +func TestNewFunctionFromGit_Errors(t *testing.T) { + fx := newGitFixture(t) + tests := []struct { + name string + git fn.Source + wantErr string + }{ + {"unknown revision", fn.Source{URL: fx.url, Revision: "nope"}, "not found"}, + {"unknown full ref", fn.Source{URL: fx.url, Revision: "refs/heads/nope"}, "couldn't find remote ref"}, + {"abbreviated hash", fn.Source{URL: fx.url, Revision: fx.feature[:7]}, "full hash"}, + {"missing func.yaml", fn.Source{URL: fx.url, Dir: "nope"}, "no func.yaml"}, + {"no url", fn.Source{}, "required"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := fn.NewFunctionFromGit(context.Background(), tt.git) + if err == nil { + t.Fatal("expected an error") + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("expected error containing %q, got %q", tt.wantErr, err) + } + }) + } +} diff --git a/pkg/functions/function_migrations.go b/pkg/functions/function_migrations.go index 1da8a82b25..abdbe2967f 100644 --- a/pkg/functions/function_migrations.go +++ b/pkg/functions/function_migrations.go @@ -1,7 +1,6 @@ package functions import ( - "errors" "fmt" "os" "path/filepath" @@ -18,20 +17,34 @@ var unknownFieldsOnce sync.Once // version of the function. It is the caller's responsibility to // .Write() the function to persist to disk. Additionally it will warn on // up-to-date spec but wrong func.yaml (eg. extraneous fields) +// +// Migrations need the function as it was serialized, which Migrate reads +// from the func.yaml at f.Root. See migrate for functions without a Root. func (f Function) Migrate() (migrated Function, err error) { + var raw []byte + if f.Root != "" { + if raw, err = os.ReadFile(filepath.Join(f.Root, FunctionFile)); err != nil && !os.IsNotExist(err) { + return f, err + } + } + return f.migrate(raw) +} + +// migrate applies any necessary migrations to f, whose serialized form (the +// content of its func.yaml) is raw. Migrations read the previous structure +// from raw, so a function needs no working tree to be migrated. +func (f Function) migrate(raw []byte) (migrated Function, err error) { // Return immediately if the function indicates it has already been // migrated. if f.Migrated() { - // Already at the latest spec — check for unknown fields - if f.Root != "" { - if bb, readErr := os.ReadFile(filepath.Join(f.Root, FunctionFile)); readErr == nil { - unknownFieldsOnce.Do(func() { - var strict Function - if strictErr := yaml.UnmarshalStrict(bb, &strict); strictErr != nil { - fmt.Fprintf(os.Stderr, "Warning (unknown fields will be ignored):\n %v\n.\n", formatUnmarshalError(strictErr)) - } - }) - } + // Already at the latest spec: check for unknown fields + if raw != nil { + unknownFieldsOnce.Do(func() { + var strict Function + if strictErr := yaml.UnmarshalStrict(raw, &strict); strictErr != nil { + fmt.Fprintf(os.Stderr, "Warning (unknown fields will be ignored):\n %v\n.\n", formatUnmarshalError(strictErr)) + } + }) } return f, nil } @@ -45,7 +58,7 @@ func (f Function) Migrate() (migrated Function, err error) { } // Apply this migration when the function's specVersion is less than that which // the migration will impart. - migrated, err = m.migrate(migrated, m) + migrated, err = m.migrate(migrated, raw, m) if err != nil { return // fail fast on any migration errors } @@ -61,7 +74,20 @@ type migration struct { } // migrator is a function which returns a migrated copy of an inbound function. -type migrator func(Function, migration) (Function, error) +// It receives the function's serialized form to read the previous structure. +type migrator func(f Function, raw []byte, m migration) (Function, error) + +// unmarshalPrevious loads the pertinent parts of a previous schema version +// from the serialized function raw, on behalf of the named migration. +func unmarshalPrevious(raw []byte, migration string, previous interface{}) error { + if raw == nil { + return fmt.Errorf("migration '%s' error: the serialized function is required", migration) + } + if err := yaml.Unmarshal(raw, previous); err != nil { + return fmt.Errorf("migration '%s' error: %w", migration, err) + } + return nil +} // Migrated returns whether the function has been migrated to the highest // level the currently executing system is aware of (or beyond). @@ -126,7 +152,7 @@ var migrations = []migration{ // created stamp. Otherwise, this is an in-memory (new) function that is // currently in the process of being created and as such need not be mutated // to consider this migration having been evaluated. -func migrateToCreationStamp(f Function, m migration) (Function, error) { +func migrateToCreationStamp(f Function, _ []byte, m migration) (Function, error) { // For functions with no creation timestamp, but appear to have been pre- // existing, populate their created stamp and version. // Yes, it's a little gnarly, but bootstrapping into the loveliness of a @@ -173,16 +199,11 @@ func migrateToCreationStamp(f Function, m migration) (Function, error) { // a customized builder image, that value is preserved as the builder image // for the 'pack' builder in the new version (s2i did not exist prior). // See associated unit tests. -func migrateToBuilderImages(f1 Function, m migration) (Function, error) { +func migrateToBuilderImages(f1 Function, raw []byte, m migration) (Function, error) { // Load the function using pertinent parts of the previous version's schema: - f0Filename := filepath.Join(f1.Root, FunctionFile) - bb, err := os.ReadFile(f0Filename) - if err != nil { - return f1, errors.New("migration 'migrateToBuilderImages' error: " + err.Error()) - } f0 := migrateToBuilderImages_previousFunction{} - if err = yaml.Unmarshal(bb, &f0); err != nil { - return f1, errors.New("migration 'migrateToBuilderImages' error: " + err.Error()) + if err := unmarshalPrevious(raw, "migrateToBuilderImages", &f0); err != nil { + return f1, err } // At time of this migration, the default pack builder image for all language @@ -206,18 +227,11 @@ func migrateToBuilderImages(f1 Function, m migration) (Function, error) { // migrateToSpecVersion updates a func.yaml file to use SpecVersion // instead of Version to track the migration numbers -func migrateToSpecVersion(f Function, m migration) (Function, error) { +func migrateToSpecVersion(f Function, raw []byte, m migration) (Function, error) { // Load the function func.yaml file - f0Filename := filepath.Join(f.Root, FunctionFile) - bb, err := os.ReadFile(f0Filename) - if err != nil { - return f, errors.New("migration 'migrateToSpecVersion' error: " + err.Error()) - } - - // Only handle the Version field if it exists f0 := migrateToSpecVersion_previousFunction{} - if err = yaml.Unmarshal(bb, &f0); err != nil { - return f, errors.New("migration 'migrateToSpecVersion' error: " + err.Error()) + if err := unmarshalPrevious(raw, "migrateToSpecVersion", &f0); err != nil { + return f, err } f.SpecVersion = m.version @@ -227,16 +241,11 @@ func migrateToSpecVersion(f Function, m migration) (Function, error) { // migrateToSpecsStructure migration makes sure use the sub-specs structs for build, run and deploy phases. // To avoid unmarshalling issues with the old format this migration needs to be executed first. // Further migrations will operate on this new struct with sub-specs -func migrateToSpecsStructure(f1 Function, m migration) (Function, error) { +func migrateToSpecsStructure(f1 Function, raw []byte, m migration) (Function, error) { // Load the Function using pertinent parts of the previous version's schema: - f0Filename := filepath.Join(f1.Root, FunctionFile) - bb, err := os.ReadFile(f0Filename) - if err != nil { - return f1, errors.New("migration 'migrateToSpecsStructure' error: " + err.Error()) - } f0 := migrateToSpecs_previousFunction{} - if err = yaml.Unmarshal(bb, &f0); err != nil { - return f1, errors.New("migration 'migrateToSpecsStructure' error: " + err.Error()) + if err := unmarshalPrevious(raw, "migrateToSpecsStructure", &f0); err != nil { + return f1, err } if f0.Git.URL != "" { @@ -306,16 +315,11 @@ func migrateToSpecsStructure(f1 Function, m migration) (Function, error) { // file. When Invoke now holds default value (http) it will not show up in // func.yaml as the default value is implicitly expected. Otherwise if Invoke // is non-default value, it will be written in func.yaml. -func migrateFromInvokeStructure(f1 Function, m migration) (Function, error) { +func migrateFromInvokeStructure(f1 Function, raw []byte, m migration) (Function, error) { // Load the Function using pertinent parts of the previous version's schema: - f0Filename := filepath.Join(f1.Root, FunctionFile) - bb, err := os.ReadFile(f0Filename) - if err != nil { - return f1, errors.New("migration 'migrateFromInvokeStructure' error: " + err.Error()) - } f0 := migrateFromInvokeStructure_previousFunction{} - if err = yaml.Unmarshal(bb, &f0); err != nil { - return f1, errors.New("migration 'migrateFromInvokeStructure' error: " + err.Error()) + if err := unmarshalPrevious(raw, "migrateFromInvokeStructure", &f0); err != nil { + return f1, err } if f0.Invocation.Format != "" && f0.Invocation.Format != "http" { @@ -327,13 +331,7 @@ func migrateFromInvokeStructure(f1 Function, m migration) (Function, error) { return f1, nil } -func migratePersistentVolumeTypoFixup(fn Function, m migration) (Function, error) { - f, err := os.Open(filepath.Join(fn.Root, FunctionFile)) - if err != nil { - return Function{}, fmt.Errorf("cannot open func.yaml: %w", err) - } - defer f.Close() - +func migratePersistentVolumeTypoFixup(fn Function, raw []byte, m migration) (Function, error) { data := struct { Run struct { Volumes []struct { @@ -341,11 +339,8 @@ func migratePersistentVolumeTypoFixup(fn Function, m migration) (Function, error } `yaml:"volumes,omitempty"` } }{} - - dec := yaml.NewDecoder(f) - err = dec.Decode(&data) - if err != nil { - return Function{}, fmt.Errorf("cannot deserialize old sub-structure: %w", err) + if err := unmarshalPrevious(raw, "migratePersistentVolumeTypoFixup", &data); err != nil { + return fn, err } for idx, volume := range data.Run.Volumes { @@ -363,20 +358,14 @@ func migratePersistentVolumeTypoFixup(fn Function, m migration) (Function, error // are --source, --revision and --source-dir, and nothing about the values // is specific to git. The old keys are carried over; the next write stores // the new ones. -func migrateGitToSource(fn Function, m migration) (Function, error) { - f, err := os.Open(filepath.Join(fn.Root, FunctionFile)) - if err != nil { - return Function{}, fmt.Errorf("cannot open func.yaml: %w", err) - } - defer f.Close() - +func migrateGitToSource(fn Function, raw []byte, m migration) (Function, error) { // Before the specs structure (0.34.0), build was the build type as a // string, so the key is read loosely. data := struct { Build interface{} `yaml:"build"` }{} - if err = yaml.NewDecoder(f).Decode(&data); err != nil { - return Function{}, fmt.Errorf("cannot deserialize old sub-structure: %w", err) + if err := unmarshalPrevious(raw, "migrateGitToSource", &data); err != nil { + return fn, err } if fn.Build.Source.URL == "" { diff --git a/pkg/functions/function_source.go b/pkg/functions/function_source.go index c4f2576eda..6e0a10d9d8 100644 --- a/pkg/functions/function_source.go +++ b/pkg/functions/function_source.go @@ -18,6 +18,13 @@ type Source struct { // Dir is the directory within the repository holding the function. // Empty means the repository root. Dir string `yaml:"dir,omitempty"` + + // Commit is the full hash Revision resolved to when the function was read + // from its repository, in memory only, never stored. The cluster then + // fetches and labels the image with exactly this commit. Empty for a local + // or uploaded source, which the builders label from the git working tree + // on disk: its HEAD, marked dirty when there are uncommitted changes. + Commit string `yaml:"-" json:"-"` } // validateSource validates the source option from Function config @@ -37,5 +44,22 @@ func validateSource(source Source) (errors []string) { errors = append(errors, errMsg) } } + if source.Commit != "" && !isFullHash(source.Commit) { + errors = append(errors, fmt.Sprintf("source commit %q is not a full commit hash", source.Commit)) + } return } + +// isFullHash reports whether s is a full commit hash: 40 lowercase +// hexadecimal digits, as git and go-git print one. +func isFullHash(s string) bool { + if len(s) != 40 { + return false + } + for _, c := range s { + if !strings.ContainsRune("0123456789abcdef", c) { + return false + } + } + return true +} diff --git a/pkg/functions/function_source_unit_test.go b/pkg/functions/function_source_unit_test.go index 93be5217b5..e73c5ad2d6 100644 --- a/pkg/functions/function_source_unit_test.go +++ b/pkg/functions/function_source_unit_test.go @@ -78,6 +78,22 @@ func Test_validateSource(t *testing.T) { Source{}, 0, }, + { + "correct 'Source - URL + full commit", + Source{ + URL: "https://myrepo/foo.git", + Commit: "0123456789abcdef0123456789abcdef01234567", + }, + 0, + }, + { + "incorrect 'Source - abbreviated commit", + Source{ + URL: "https://myrepo/foo.git", + Commit: "0123456", + }, + 1, + }, } for _, tt := range tests { diff --git a/pkg/pipelines/tekton/pipelines_provider.go b/pkg/pipelines/tekton/pipelines_provider.go index 564e4ea7f6..dbf5bd9921 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -123,6 +123,12 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn return "", f, err } + // The source is either a git repository or the local working tree, which + // is uploaded. A function loaded from git has no Root. + if f.Build.Source.URL == "" && f.Root == "" { + return "", f, errors.New("a local function directory is required to upload sources; set a git URL to build from a repository") + } + // Warn if the func-generated legacy .s2i/bin/assemble exists; it will be // uploaded to the PVC and can interfere with the in-cluster build. // Remote deploy doesn't go through Client.Build, so we re-check here. diff --git a/pkg/pipelines/tekton/task-buildpack.yaml.tmpl b/pkg/pipelines/tekton/task-buildpack.yaml.tmpl index c60e2c06df..be025e73db 100644 --- a/pkg/pipelines/tekton/task-buildpack.yaml.tmpl +++ b/pkg/pipelines/tekton/task-buildpack.yaml.tmpl @@ -72,6 +72,23 @@ spec: - name: CNB_PLATFORM_API value: "0.10" steps: + - name: clean-src + # The git-clone StepAction runs as user 65532 and deletes what is in the + # workspace before cloning. A previous run of this pipeline leaves the + # sources there owned by the build user, which 65532 cannot remove, so a + # second build of the same function from git failed. Empty the workspace + # as root first and hand the directory to the clone user. + image: docker.io/library/bash:5.1 + when: + - input: "$(params.GIT_REPOSITORY)" + operator: notin + values: [""] + script: | + #!/usr/bin/env bash + set -e + src="$(workspaces.source.path)" + find "$src" -mindepth 1 -delete + chown 65532:65532 "$src" - name: fetch-src ref: resolver: bundles diff --git a/pkg/pipelines/tekton/task-s2i.yaml.tmpl b/pkg/pipelines/tekton/task-s2i.yaml.tmpl index c739a45a1f..5ea3335222 100644 --- a/pkg/pipelines/tekton/task-s2i.yaml.tmpl +++ b/pkg/pipelines/tekton/task-s2i.yaml.tmpl @@ -57,6 +57,23 @@ spec: An optional workspace that allows providing a .docker/config.json file for Buildah to access the container registry. The file should be placed at the root of the Workspace with name config.json. optional: true steps: + - name: clean-src + # The git-clone StepAction runs as user 65532 and deletes what is in the + # workspace before cloning. A previous run of this pipeline leaves the + # sources there owned by the build user, which 65532 cannot remove, so a + # second build of the same function from git failed. Empty the workspace + # as root first and hand the directory to the clone user. + image: docker.io/library/bash:5.1 + when: + - input: "$(params.GIT_REPOSITORY)" + operator: notin + values: [""] + script: | + #!/usr/bin/env bash + set -e + src="$(workspaces.source.path)" + find "$src" -mindepth 1 -delete + chown 65532:65532 "$src" - name: fetch-src ref: resolver: bundles diff --git a/pkg/pipelines/tekton/tasks_test.go b/pkg/pipelines/tekton/tasks_test.go index f4fca3284e..95d7c74fe9 100644 --- a/pkg/pipelines/tekton/tasks_test.go +++ b/pkg/pipelines/tekton/tasks_test.go @@ -61,6 +61,25 @@ func TestGetTasks(t *testing.T) { if apiErr != nil { t.Fatalf("%+v\n", apiErr) } + + // The workspace is emptied as root right before the clone, and + // only when there is a repository to clone; the upload path must + // keep the sources it received. + steps := task.Spec.Steps + if len(steps) < 2 || steps[0].Name != "clean-src" || steps[1].Name != "fetch-src" { + t.Fatalf("expected clean-src to precede fetch-src, got %v", stepNames(steps)) + } + if len(steps[0].When) != 1 || steps[0].When[0].Input != "$(params.GIT_REPOSITORY)" { + t.Errorf("expected clean-src to be gated on GIT_REPOSITORY, got %+v", steps[0].When) + } }) } } + +func stepNames(steps []tektonv1.Step) []string { + names := make([]string, 0, len(steps)) + for _, s := range steps { + names = append(names, s.Name) + } + return names +} diff --git a/pkg/pipelines/tekton/templates.go b/pkg/pipelines/tekton/templates.go index 362ce1a057..ec9717e716 100644 --- a/pkg/pipelines/tekton/templates.go +++ b/pkg/pipelines/tekton/templates.go @@ -347,6 +347,31 @@ func createAndApplyPipelineTemplate(f fn.Function, namespace string, labels map[ return createAndApplyResource(f.Root, pipelineFileName, template, "pipeline", getPipelineName(f), namespace, data) } +// sourceRevision returns what the cluster fetches and what it labels the +// image with. A function read from its repository has the commit it was +// read at, so the cluster fetches exactly that. Anything else is fetched by +// the revision as configured, the remote's default branch when empty, and +// labelled from the git working tree on disk if it has one, HEAD marked +// dirty when there are uncommitted changes, as every builder labels a local +// source. +func sourceRevision(f fn.Function) (fetch, label string) { + if c := f.Build.Source.Commit; c != "" { + // validateSource requires a full hash; the guard spares a caller that + // skipped validation a panic. + label = c + if len(label) > 7 { + label = label[:7] + } + return c, label + } + // Only a function on disk has a working tree to label from: with no + // Root, GitCommit would search whichever repository func runs in. + if f.Root != "" { + label, _ = fn.GitCommit(f.Root) + } + return f.Build.Source.Revision, label +} + // createAndApplyPipelineRunTemplate creates and applies PipelineRun template for a standard on-cluster build // all resources are created on the fly, if there's a PipelineRun defined in the project directory, it is used instead func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels map[string]string) error { @@ -357,11 +382,6 @@ func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels m contextDir = "." } - pipelinesTargetBranch := f.Build.Source.Revision - if pipelinesTargetBranch == "" { - pipelinesTargetBranch = defaultPipelinesTargetBranch - } - buildEnvs := []string{} if len(f.Build.BuildEnvs) == 0 { buildEnvs = []string{"="} @@ -389,7 +409,7 @@ func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels m tlsVerify = "false" } - commit, _ := fn.GitCommit(f.Root) + fetch, label := sourceRevision(f) data := templateData{ FunctionName: f.Name, @@ -408,10 +428,10 @@ func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels m S2iImageScriptsUrl: s2iImageScriptsUrl, TlsVerify: tlsVerify, - Commit: commit, + Commit: label, RepoUrl: f.Build.Source.URL, - Revision: pipelinesTargetBranch, + Revision: fetch, } var template string @@ -431,12 +451,13 @@ func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels m var manifestivalClient = k8s.GetManifestivalClient // createAndApplyResource tries to create and apply a resource to the k8s cluster from the input template and data, -// if there's the same resource already created in the project directory, it is used instead +// if there's the same resource already created in the project directory, it is used instead. +// An empty projectRoot (a function loaded from git) has no such directory. func createAndApplyResource(projectRoot, fileName, fileTemplate, kind, resourceName, namespace string, data interface{}) error { var source manifestival.Source filePath := path.Join(projectRoot, resourcesDirectory, fileName) - if _, err := os.Stat(filePath); !os.IsNotExist(err) { + if _, err := os.Stat(filePath); projectRoot != "" && !os.IsNotExist(err) { source = manifestival.Path(filePath) } else { tmpl, err := template.New("template").Parse(fileTemplate) diff --git a/pkg/pipelines/tekton/templates_pack.go b/pkg/pipelines/tekton/templates_pack.go index 25a53f804d..b2c7d2824e 100644 --- a/pkg/pipelines/tekton/templates_pack.go +++ b/pkg/pipelines/tekton/templates_pack.go @@ -107,7 +107,7 @@ spec: - name: gitRepository value: "{{.RepoUrl}}" - name: gitRevision - value: {{.Revision}} + value: "{{.Revision}}" - name: contextDir value: "{{.ContextDir}}" - name: imageName diff --git a/pkg/pipelines/tekton/templates_s2i.go b/pkg/pipelines/tekton/templates_s2i.go index 8eb5ccd7a0..7834f2a0da 100644 --- a/pkg/pipelines/tekton/templates_s2i.go +++ b/pkg/pipelines/tekton/templates_s2i.go @@ -114,7 +114,7 @@ spec: - name: gitRepository value: "{{.RepoUrl}}" - name: gitRevision - value: {{.Revision}} + value: "{{.Revision}}" - name: contextDir value: "{{.ContextDir}}" - name: imageName diff --git a/pkg/pipelines/tekton/templates_test.go b/pkg/pipelines/tekton/templates_test.go index 7fc519d184..0831d8c1fd 100644 --- a/pkg/pipelines/tekton/templates_test.go +++ b/pkg/pipelines/tekton/templates_test.go @@ -7,7 +7,10 @@ import ( "strings" "testing" "text/template" + "time" + gogit "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing/object" "github.com/manifestival/manifestival" "github.com/manifestival/manifestival/fake" tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" @@ -315,6 +318,35 @@ var testData = []struct { }, } +// Test_createAndApplyPipelineRunTemplate_NoRoot ensures a function loaded +// from a git repository, which has no Root, yields a Pipeline and a +// PipelineRun: nothing is read from a project directory in that case. +func Test_createAndApplyPipelineRunTemplate_NoRoot(t *testing.T) { + old := manifestivalClient + defer func() { manifestivalClient = old }() + manifestivalClient = func() (manifestival.Client, error) { + return fake.New(), nil + } + + f := fn.Function{ + Name: "remote-fn", + Runtime: "go", + Registry: TestRegistry, + Build: fn.BuildSpec{ + Builder: builders.Pack, + Source: fn.Source{URL: "https://example.com/alice/remote-fn.git", Revision: "main"}, + }, + } + f.Deploy.Image = "docker.io/alice/remote-fn" + + if err := createAndApplyPipelineTemplate(f, "test-ns", nil); err != nil { + t.Fatal(err) + } + if err := createAndApplyPipelineRunTemplate(f, "test-ns", nil); err != nil { + t.Fatal(err) + } +} + func Test_createAndApplyPipelineRunTemplate(t *testing.T) { for _, tt := range testData { t.Run(tt.name, func(t *testing.T) { @@ -346,6 +378,70 @@ func Test_createAndApplyPipelineRunTemplate(t *testing.T) { } } +// Test_sourceRevision ensures a function read from its repository makes the +// cluster fetch, and label the image with, the commit it was read at, while +// any other function keeps the configured revision and the local HEAD. +func Test_sourceRevision(t *testing.T) { + const hash = "0123456789abcdef0123456789abcdef01234567" + + f := fn.Function{Build: fn.BuildSpec{Source: fn.Source{URL: "https://example.com/repo.git", Revision: "main", Commit: hash}}} + if fetch, label := sourceRevision(f); fetch != hash || label != hash[:7] { + t.Errorf("read from git: expected %q, %q; got %q, %q", hash, hash[:7], fetch, label) + } + + f = fn.Function{Build: fn.BuildSpec{Source: fn.Source{URL: "https://example.com/repo.git", Revision: "v1"}}} + if fetch, _ := sourceRevision(f); fetch != "v1" { + t.Errorf("git without a resolved commit: expected the revision as configured, got %q", fetch) + } + + f = fn.Function{Root: t.TempDir()} // no git history, no repository + if fetch, label := sourceRevision(f); fetch != "" || label != "" { + t.Errorf("upload without history: expected \"\", \"\"; got %q, %q", fetch, label) + } + + // A commit shorter than the label (validation rejects it; a caller may + // skip validation) must not panic. + f = fn.Function{Build: fn.BuildSpec{Source: fn.Source{URL: "https://example.com/repo.git", Commit: "abc"}}} + if fetch, label := sourceRevision(f); fetch != "abc" || label != "abc" { + t.Errorf("short commit: expected \"abc\", \"abc\"; got %q, %q", fetch, label) + } + + // With no Root there is no working tree to label from, even when func + // runs inside some unrelated repository. + t.Chdir(gitRepoWithCommit(t)) + f = fn.Function{Build: fn.BuildSpec{Source: fn.Source{URL: "https://example.com/repo.git", Revision: "v1"}}} + if fetch, label := sourceRevision(f); fetch != "v1" || label != "" { + t.Errorf("rootless without a commit: expected \"v1\", \"\"; got %q, %q", fetch, label) + } +} + +// gitRepoWithCommit returns a new directory holding a git repository with +// one commit. +func gitRepoWithCommit(t *testing.T) string { + t.Helper() + dir := t.TempDir() + repo, err := gogit.PlainInit(dir, false) + if err != nil { + t.Fatal(err) + } + wt, err := repo.Worktree() + if err != nil { + t.Fatal(err) + } + if err = os.WriteFile(filepath.Join(dir, "README.md"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if _, err = wt.Add("README.md"); err != nil { + t.Fatal(err) + } + if _, err = wt.Commit("initial", &gogit.CommitOptions{ + Author: &object.Signature{Name: "test", Email: "test@example.com", When: time.Now()}, + }); err != nil { + t.Fatal(err) + } + return dir +} + // strictTektonDecoder returns a strict deserializer that rejects unknown fields // in Tekton v1 resources (Pipeline, PipelineRun, etc.). func strictTektonDecoder(t *testing.T) runtime.Decoder { diff --git a/pkg/testing/testing.go b/pkg/testing/testing.go index e006b0265b..bf6f1ff054 100644 --- a/pkg/testing/testing.go +++ b/pkg/testing/testing.go @@ -26,6 +26,7 @@ import ( "os/exec" "path/filepath" "runtime" + "sort" "strings" "testing" ) @@ -226,6 +227,65 @@ func WithExecutable(t *testing.T, name, goSrc string) { } } +// ServeGitRepository serves over HTTP, as ServeRepo does, a new repository +// with one commit per branch. branches maps a branch name to the files that +// commit writes (slash-separated path to content); main is created first, +// with its files or none, and every other branch starts from main. Returned +// are the repository's URL and the head of each branch as a full hash. +// Requires the git binary. +func ServeGitRepository(t *testing.T, branches map[string]map[string]string) (url string, heads map[string]string) { + t.Helper() + root := t.TempDir() + work := filepath.Join(t.TempDir(), "work") + if err := os.MkdirAll(work, 0o755); err != nil { + t.Fatal(err) + } + git := func(dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=test", "GIT_AUTHOR_EMAIL=test@example.com", + "GIT_COMMITTER_NAME=test", "GIT_COMMITTER_EMAIL=test@example.com") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out) + } + return strings.TrimSpace(string(out)) + } + commit := func(files map[string]string, msg string) string { + t.Helper() + for name, content := range files { + p := filepath.Join(work, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + git(work, "add", "-A") + git(work, "-c", "commit.gpgsign=false", "commit", "-q", "--allow-empty", "-m", msg) + return git(work, "rev-parse", "HEAD") + } + + git(work, "init", "-q", "-b", "main") + heads = map[string]string{"main": commit(branches["main"], "main")} + names := make([]string, 0, len(branches)) + for name := range branches { + if name != "main" { + names = append(names, name) + } + } + sort.Strings(names) + for _, name := range names { + git(work, "checkout", "-q", "-b", name, "main") + heads[name] = commit(branches[name], name) + } + git(root, "clone", "-q", "--bare", work, "repository.git") + return RunGitServer(root, t) + "/repository.git", heads +} + // RunGitServer starts serving git HTTP server and returns its address func RunGitServer(root string, t *testing.T) (url string) { l, err := net.Listen("tcp", "127.0.0.1:0")