diff --git a/builders/builders.go b/builders/builders.go index 845e8c82f..5f0044bec 100644 --- a/builders/builders.go +++ b/builders/builders.go @@ -1,6 +1,6 @@ /* - Package builders provides constants for builder implementation short names, - shared error types and convienience functions. +Package builders provides constants for builder implementation short names, +shared error types and convienience functions. */ package builders @@ -77,11 +77,11 @@ func (e ErrNoDefaultImage) Error() string { // Image is a convenience function for choosing the correct builder image // given a function, a builder, and defaults grouped by runtime. -// - ErrRuntimeRequired if no runtime was provided on the given function -// - ErrNoDefaultImage if the function has no builder image already defined -// for the given runtieme and there is no default in the provided map. +// - ErrRuntimeRequired if no runtime was provided on the given function +// - ErrNoDefaultImage if the function has no builder image already defined +// for the given runtime and there is no default in the provided map. func Image(f fn.Function, builder string, defaults map[string]string) (string, error) { - v, ok := f.BuilderImages[builder] + v, ok := f.Build.BuilderImages[builder] if ok { return v, nil // found value } diff --git a/builders/builders_test.go b/builders/builders_test.go index dc0a3edbb..000b4a40d 100644 --- a/builders/builders_test.go +++ b/builders/builders_test.go @@ -12,9 +12,11 @@ import ( // it exists on the function for a given builder, no defaults. func TestImage_Named(t *testing.T) { f := fn.Function{ - Builder: builders.Pack, - BuilderImages: map[string]string{ - builders.Pack: "example.com/my/builder-image", + Build: fn.BuildSpec{ + Builder: builders.Pack, + BuilderImages: map[string]string{ + builders.Pack: "example.com/my/builder-image", + }, }, } diff --git a/buildpacks/builder.go b/buildpacks/builder.go index c3d5f44ec..a57ae1b48 100644 --- a/buildpacks/builder.go +++ b/buildpacks/builder.go @@ -108,13 +108,13 @@ func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) { Image: f.Image, LifecycleImage: "quay.io/boson/lifecycle:0.13.2", Builder: image, - Buildpacks: f.Buildpacks, + Buildpacks: f.Build.Buildpacks, ContainerConfig: struct { Network string Volumes []string }{Network: "", Volumes: nil}, } - if opts.Env, err = fn.Interpolate(f.BuildEnvs); err != nil { + if opts.Env, err = fn.Interpolate(f.Build.BuildEnvs); err != nil { return err } if runtime.GOOS == "linux" { @@ -122,7 +122,7 @@ func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) { } var impl = b.impl - // Instantate the pack build client implementation + // Instantiate the pack build client implementation // (and update build opts as necessary) if impl == nil { var ( @@ -155,7 +155,7 @@ func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) { return } -// newImpl returns an instance of the builder implementatoin. Note that this +// newImpl returns an instance of the builder implementation. Note that this // also mutates the provided options' DockerHost and TrustBuilder. func newImpl(ctx context.Context, cli client.CommonAPIClient, dockerHost string, opts *pack.BuildOptions, logger logging.Logger) (impl Impl, err error) { opts.DockerHost = dockerHost diff --git a/buildpacks/builder_test.go b/buildpacks/builder_test.go index f762bc6d9..84b3c2363 100644 --- a/buildpacks/builder_test.go +++ b/buildpacks/builder_test.go @@ -41,8 +41,10 @@ func Test_BuilderImageConfigurable(t *testing.T) { WithName(builders.Pack), WithImpl(i)) f = fn.Function{ // Function with a builder image set Runtime: "node", - BuilderImages: map[string]string{ - builders.Pack: "example.com/user/builder-image", + Build: fn.BuildSpec{ + BuilderImages: map[string]string{ + builders.Pack: "example.com/user/builder-image", + }, }, } ) @@ -68,8 +70,10 @@ func Test_BuildEnvs(t *testing.T) { envName = "NAME" envValue = "{{ env:INTERPOLATE_ME }}" f = fn.Function{ - Runtime: "node", - BuildEnvs: []fn.Env{{Name: &envName, Value: &envValue}}, + Runtime: "node", + Build: fn.BuildSpec{ + BuildEnvs: []fn.Env{{Name: &envName, Value: &envValue}}, + }, } i = &mockImpl{} b = NewBuilder(WithImpl(i)) diff --git a/client_test.go b/client_test.go index 7d17c9008..316c14256 100644 --- a/client_test.go +++ b/client_test.go @@ -1073,7 +1073,9 @@ func TestClient_Pipelines_Deploy_Image(t *testing.T) { Name: "myfunc", Runtime: "node", Root: root, - Git: fn.Git{URL: "http://example-git.com/alice/myfunc.git"}, + Build: fn.BuildSpec{ + Git: fn.Git{URL: "http://example-git.com/alice/myfunc.git"}, + }, } err := client.Create(f) @@ -1167,10 +1169,12 @@ func TestClient_New_BuildersPersisted(t *testing.T) { f0 := fn.Function{ Runtime: TestRuntime, Root: root, - BuilderImages: map[string]string{ - builders.Pack: "example.com/my/custom-pack-builder", - builders.S2I: "example.com/my/custom-s2i-builder", - }} + Build: fn.BuildSpec{ + BuilderImages: map[string]string{ + builders.Pack: "example.com/my/custom-pack-builder", + builders.S2I: "example.com/my/custom-s2i-builder", + }}, + } // Create the function, which should preserve custom builders if err := client.New(context.Background(), f0); err != nil { @@ -1184,8 +1188,8 @@ func TestClient_New_BuildersPersisted(t *testing.T) { } // Assert that our custom builders were retained - if !reflect.DeepEqual(f0.BuilderImages, f1.BuilderImages) { - t.Fatalf("Expected %v but got %v", f0.BuilderImages, f1.BuilderImages) + if !reflect.DeepEqual(f0.Build.BuilderImages, f1.Build.BuilderImages) { + t.Fatalf("Expected %v but got %v", f0.Build.BuilderImages, f1.Build.BuilderImages) } // A Default Builder(image) is not asserted here, because that is @@ -1206,9 +1210,11 @@ func TestClient_New_BuildpacksPersisted(t *testing.T) { } client := fn.New(fn.WithRegistry(TestRegistry)) if err := client.New(context.Background(), fn.Function{ - Runtime: TestRuntime, - Root: root, - Buildpacks: buildpacks, + Runtime: TestRuntime, + Root: root, + Build: fn.BuildSpec{ + Buildpacks: buildpacks, + }, }); err != nil { t.Fatal(err) } @@ -1218,8 +1224,8 @@ func TestClient_New_BuildpacksPersisted(t *testing.T) { } // Assert that our custom buildpacks were set - if !reflect.DeepEqual(f.Buildpacks, buildpacks) { - t.Fatalf("Expected %v but got %v", buildpacks, f.Buildpacks) + if !reflect.DeepEqual(f.Build.Buildpacks, buildpacks) { + t.Fatalf("Expected %v but got %v", buildpacks, f.Build.Buildpacks) } } diff --git a/cmd/build.go b/cmd/build.go index a2de2c54f..eab204473 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -112,39 +112,39 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro // Sets default AND accepts any user-provided overrides f.Registry = config.Registry } - if f.Builder == "" || cmd.Flags().Changed("builder") { + if f.Build.Builder == "" || cmd.Flags().Changed("builder") { // Sets default AND accepts any user-provided overrides - f.Builder = config.Builder + f.Build.Builder = config.Builder } if config.Image != "" { f.Image = config.Image } if config.Builder != "" { - f.Builder = config.Builder + f.Build.Builder = config.Builder } if config.BuilderImage != "" { - f.BuilderImages[config.Builder] = config.BuilderImage + f.Build.BuilderImages[config.Builder] = config.BuilderImage } // Validate that a builder short-name was obtained, whether that be from // the function's prior state, or the value of flags/environment. - if err = ValidateBuilder(f.Builder); err != nil { + if err = ValidateBuilder(f.Build.Builder); err != nil { return } // Choose a builder based on the value of the --builder flag var builder fn.Builder - if f.Builder == builders.Pack { + if f.Build.Builder == builders.Pack { builder = buildpacks.NewBuilder( buildpacks.WithName(builders.Pack), buildpacks.WithVerbose(config.Verbose)) - } else if f.Builder == builders.S2I { + } else if f.Build.Builder == builders.S2I { builder = s2i.NewBuilder( s2i.WithName(builders.S2I), s2i.WithPlatform(config.Platform), s2i.WithVerbose(config.Verbose)) } else { - err = fmt.Errorf("builder '%v' is not recognized", f.Builder) + err = fmt.Errorf("builder '%v' is not recognized", f.Build.Builder) return } diff --git a/cmd/client.go b/cmd/client.go index 88dc8535f..d5581874e 100644 --- a/cmd/client.go +++ b/cmd/client.go @@ -25,7 +25,7 @@ type ClientConfig struct { // currently configured in the client's connection should be used. Namespace string - // Verbose logging. By default logging output is kept to the bare minimum. + // Verbose logging. By default, logging output is kept to the bare minimum. // Use this flag to configure verbose logging throughout. Verbose bool diff --git a/cmd/completion_util.go b/cmd/completion_util.go index 9f24dad96..86dc9dc90 100644 --- a/cmd/completion_util.go +++ b/cmd/completion_util.go @@ -114,8 +114,8 @@ func CompleteBuilderImageList(cmd *cobra.Command, args []string, complete string return } - builderImages = make([]string, 0, len(f.BuilderImages)) - for name := range f.BuilderImages { + builderImages = make([]string, 0, len(f.Build.BuilderImages)) + for name := range f.Build.BuilderImages { if len(complete) == 0 { builderImages = append(builderImages, name) continue diff --git a/cmd/config_envs.go b/cmd/config_envs.go index 47415cdd6..c87dc9c81 100644 --- a/cmd/config_envs.go +++ b/cmd/config_envs.go @@ -115,7 +115,7 @@ set environment variable from a secret } } - function.Envs = append(function.Envs, fn.Env{Name: np, Value: vp}) + function.Run.Envs = append(function.Run.Envs, fn.Env{Name: np, Value: vp}) return loadSaver.Save(function) } @@ -156,13 +156,13 @@ in the current directory or from the directory specified with --path. func listEnvs(f fn.Function, w io.Writer, outputFormat Format) error { switch outputFormat { case Human: - if len(f.Envs) == 0 { + if len(f.Run.Envs) == 0 { _, err := fmt.Fprintln(w, "There aren't any configured Environment variables") return err } fmt.Println("Configured Environment variables:") - for _, e := range f.Envs { + for _, e := range f.Run.Envs { _, err := fmt.Fprintln(w, " - ", e.String()) if err != nil { return err @@ -171,7 +171,7 @@ func listEnvs(f fn.Function, w io.Writer, outputFormat Format) error { return nil case JSON: enc := json.NewEncoder(w) - return enc.Encode(f.Envs) + return enc.Encode(f.Run.Envs) default: return fmt.Errorf("bad format: %v", outputFormat) } @@ -182,9 +182,9 @@ func runAddEnvsPrompt(ctx context.Context, f fn.Function) (err error) { insertToIndex := 0 // SECTION - if there are some envs already set, let choose the position of the new entry - if len(f.Envs) > 0 { + if len(f.Run.Envs) > 0 { options := []string{} - for _, e := range f.Envs { + for _, e := range f.Run.Envs { options = append(options, fmt.Sprintf("Insert before: %s", e.String())) } options = append(options, "Insert here.") @@ -212,11 +212,11 @@ func runAddEnvsPrompt(ctx context.Context, f fn.Function) (err error) { } // SECTION - select the type of Environment variable to be added - secrets, err := k8s.ListSecretsNamesIfConnected(ctx, f.Namespace) + secrets, err := k8s.ListSecretsNamesIfConnected(ctx, f.Deploy.Namespace) if err != nil { return } - configMaps, err := k8s.ListConfigMapsNamesIfConnected(ctx, f.Namespace) + configMaps, err := k8s.ListConfigMapsNamesIfConnected(ctx, f.Deploy.Namespace) if err != nil { return } @@ -445,11 +445,11 @@ func runAddEnvsPrompt(ctx context.Context, f fn.Function) (err error) { } // we have all necessary information -> let's insert the env to the selected position in the list - if insertToIndex == len(f.Envs) { - f.Envs = append(f.Envs, newEnv) + if insertToIndex == len(f.Run.Envs) { + f.Run.Envs = append(f.Run.Envs, newEnv) } else { - f.Envs = append(f.Envs[:insertToIndex+1], f.Envs[insertToIndex:]...) - f.Envs[insertToIndex] = newEnv + f.Run.Envs = append(f.Run.Envs[:insertToIndex+1], f.Run.Envs[insertToIndex:]...) + f.Run.Envs[insertToIndex] = newEnv } err = f.Write() @@ -461,13 +461,13 @@ func runAddEnvsPrompt(ctx context.Context, f fn.Function) (err error) { } func runRemoveEnvsPrompt(f fn.Function) (err error) { - if len(f.Envs) == 0 { + if len(f.Run.Envs) == 0 { fmt.Println("There aren't any configured Environment variables") return } options := []string{} - for _, e := range f.Envs { + for _, e := range f.Run.Envs { options = append(options, e.String()) } @@ -486,16 +486,16 @@ func runRemoveEnvsPrompt(f fn.Function) (err error) { var newEnvs []fn.Env removed := false - for i, e := range f.Envs { + for i, e := range f.Run.Envs { if e.String() == selectedEnv { - newEnvs = append(f.Envs[:i], f.Envs[i+1:]...) + newEnvs = append(f.Run.Envs[:i], f.Run.Envs[i+1:]...) removed = true break } } if removed { - f.Envs = newEnvs + f.Run.Envs = newEnvs err = f.Write() if err == nil { fmt.Println("Environment variable entry was removed from the function configuration") diff --git a/cmd/config_labels.go b/cmd/config_labels.go index 39da67ee5..fd56bb1a0 100644 --- a/cmd/config_labels.go +++ b/cmd/config_labels.go @@ -92,13 +92,13 @@ directory or from the directory specified with --path. } func listLabels(f fn.Function) { - if len(f.Labels) == 0 { + if len(f.Deploy.Labels) == 0 { fmt.Println("There aren't any configured labels") return } fmt.Println("Configured labels:") - for _, e := range f.Labels { + for _, e := range f.Deploy.Labels { fmt.Println(" - ", e.String()) } } @@ -108,9 +108,9 @@ func runAddLabelsPrompt(ctx context.Context, f fn.Function, saver functionSaver) insertToIndex := 0 // SECTION - if there are some labels already set, choose the position of the new entry - if len(f.Labels) > 0 { + if len(f.Deploy.Labels) > 0 { options := []string{} - for _, e := range f.Labels { + for _, e := range f.Deploy.Labels { options = append(options, fmt.Sprintf("Insert before: %s", e.String())) } options = append(options, "Insert here.") @@ -233,11 +233,11 @@ func runAddLabelsPrompt(ctx context.Context, f fn.Function, saver functionSaver) } // we have all necessary information -> let's insert the label to the selected position in the list - if insertToIndex == len(f.Labels) { - f.Labels = append(f.Labels, newPair) + if insertToIndex == len(f.Deploy.Labels) { + f.Deploy.Labels = append(f.Deploy.Labels, newPair) } else { - f.Labels = append(f.Labels[:insertToIndex+1], f.Labels[insertToIndex:]...) - f.Labels[insertToIndex] = newPair + f.Deploy.Labels = append(f.Deploy.Labels[:insertToIndex+1], f.Deploy.Labels[insertToIndex:]...) + f.Deploy.Labels[insertToIndex] = newPair } err = saver.Save(f) @@ -249,13 +249,13 @@ func runAddLabelsPrompt(ctx context.Context, f fn.Function, saver functionSaver) } func runRemoveLabelsPrompt(f fn.Function, saver functionSaver) (err error) { - if len(f.Labels) == 0 { + if len(f.Deploy.Labels) == 0 { fmt.Println("There aren't any configured labels") return } options := []string{} - for _, e := range f.Labels { + for _, e := range f.Deploy.Labels { options = append(options, e.String()) } @@ -274,16 +274,16 @@ func runRemoveLabelsPrompt(f fn.Function, saver functionSaver) (err error) { var newLabels []fn.Label removed := false - for i, e := range f.Labels { + for i, e := range f.Deploy.Labels { if e.String() == selectedLabel { - newLabels = append(f.Labels[:i], f.Labels[i+1:]...) + newLabels = append(f.Deploy.Labels[:i], f.Deploy.Labels[i+1:]...) removed = true break } } if removed { - f.Labels = newLabels + f.Deploy.Labels = newLabels err = saver.Save(f) if err == nil { fmt.Println("Label was removed from the function configuration") diff --git a/cmd/config_labels_test.go b/cmd/config_labels_test.go index c5de7dfec..c0fe5f3b0 100644 --- a/cmd/config_labels_test.go +++ b/cmd/config_labels_test.go @@ -103,7 +103,7 @@ const ( func TestNewConfigLabelsCmd(t *testing.T) { var loaderSaver mockFunctionLoaderSaver - labels := &loaderSaver.f.Labels + labels := &loaderSaver.f.Deploy.Labels cmd := NewConfigLabelsCmd(&loaderSaver) cmd.SetArgs([]string{}) // Do not use test command args @@ -139,7 +139,7 @@ func TestListLabels(t *testing.T) { } var loaderSaver mockFunctionLoaderSaver - labels := &loaderSaver.f.Labels + labels := &loaderSaver.f.Deploy.Labels *labels = append(*labels, p("a", "b"), p("c", "d")) diff --git a/cmd/config_test.go b/cmd/config_test.go index 4deca4e52..998cb7186 100644 --- a/cmd/config_test.go +++ b/cmd/config_test.go @@ -23,7 +23,7 @@ func TestListEnvs(t *testing.T) { if path != "" { t.Fatalf("bad path, got %q but expected ", path) } - return fn.Function{Envs: envs}, nil + return fn.Function{Run: fn.RunSpec{Envs: envs}}, nil } cmd := fnCmd.NewConfigCmd(mock) @@ -58,12 +58,12 @@ func TestListEnvAdd(t *testing.T) { mock := newMockLoaderSaver() mock.load = func(path string) (fn.Function, error) { - return fn.Function{Envs: []fn.Env{{Name: &foo, Value: &bar}}}, nil + return fn.Function{Run: fn.RunSpec{Envs: []fn.Env{{Name: &foo, Value: &bar}}}}, nil } var expectedEnvs []fn.Env mock.save = func(f fn.Function) error { - if !envsEqual(expectedEnvs, f.Envs) { - return fmt.Errorf("unexpected envs: got %v but %v was expected", f.Envs, expectedEnvs) + if !envsEqual(expectedEnvs, f.Run.Envs) { + return fmt.Errorf("unexpected envs: got %v but %v was expected", f.Run.Envs, expectedEnvs) } return nil } diff --git a/cmd/config_volumes.go b/cmd/config_volumes.go index 83d65c1c5..730c0ab03 100644 --- a/cmd/config_volumes.go +++ b/cmd/config_volumes.go @@ -99,24 +99,24 @@ in the current directory or from the directory specified with --path. } func listVolumes(f fn.Function) { - if len(f.Volumes) == 0 { + if len(f.Run.Volumes) == 0 { fmt.Println("There aren't any configured Volume mounts") return } fmt.Println("Configured Volumes mounts:") - for _, v := range f.Volumes { + for _, v := range f.Run.Volumes { fmt.Println(" - ", v.String()) } } func runAddVolumesPrompt(ctx context.Context, f fn.Function) (err error) { - secrets, err := k8s.ListSecretsNamesIfConnected(ctx, f.Namespace) + secrets, err := k8s.ListSecretsNamesIfConnected(ctx, f.Deploy.Namespace) if err != nil { return } - configMaps, err := k8s.ListConfigMapsNamesIfConnected(ctx, f.Namespace) + configMaps, err := k8s.ListConfigMapsNamesIfConnected(ctx, f.Deploy.Namespace) if err != nil { return } @@ -136,7 +136,7 @@ func runAddVolumesPrompt(ctx context.Context, f fn.Function) (err error) { switch len(options) { case 0: - fmt.Printf("There aren't any Secrets or ConfiMaps in the namespace \"%s\"\n", f.Namespace) + fmt.Printf("There aren't any Secrets or ConfiMaps in the namespace \"%s\"\n", f.Deploy.Namespace) return case 1: selectedOption = options[0] @@ -204,7 +204,7 @@ func runAddVolumesPrompt(ctx context.Context, f fn.Function) (err error) { newVolume.Secret = &selectedResource } - f.Volumes = append(f.Volumes, newVolume) + f.Run.Volumes = append(f.Run.Volumes, newVolume) err = f.Write() if err == nil { @@ -215,13 +215,13 @@ func runAddVolumesPrompt(ctx context.Context, f fn.Function) (err error) { } func runRemoveVolumesPrompt(f fn.Function) (err error) { - if len(f.Volumes) == 0 { + if len(f.Run.Volumes) == 0 { fmt.Println("There aren't any configured Volume mounts") return } options := []string{} - for _, v := range f.Volumes { + for _, v := range f.Run.Volumes { options = append(options, v.String()) } @@ -240,16 +240,16 @@ func runRemoveVolumesPrompt(f fn.Function) (err error) { var newVolumes []fn.Volume removed := false - for i, v := range f.Volumes { + for i, v := range f.Run.Volumes { if v.String() == selectedVolume { - newVolumes = append(f.Volumes[:i], f.Volumes[i+1:]...) + newVolumes = append(f.Run.Volumes[:i], f.Run.Volumes[i+1:]...) removed = true break } } if removed { - f.Volumes = newVolumes + f.Run.Volumes = newVolumes err = f.Write() if err == nil { fmt.Println("Volume entry was removed from the function configuration") diff --git a/cmd/create.go b/cmd/create.go index 8264446b9..86309c223 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -141,9 +141,9 @@ func runCreate(cmd *cobra.Command, args []string, newClient ClientFactory) (err // Run Help func runCreateHelp(cmd *cobra.Command, args []string, newClient ClientFactory) { - // Error-tolerant implementataion: + // Error-tolerant implementation: // Help can not fail when creating the client config (such as on invalid - // flag values) because help text is needed in that situation. Therefore + // flag values) because help text is needed in that situation. Therefore, // this implementation must be resilient to cfg zero value. failSoft := func(err error) { if err != nil { diff --git a/cmd/delete.go b/cmd/delete.go index 83e8a1473..e028c6782 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -83,7 +83,7 @@ func runDelete(cmd *cobra.Command, args []string, newClient ClientFactory) (err // If not provided, use the function's extant namespace if config.Namespace == "" { - config.Namespace = function.Namespace + config.Namespace = function.Deploy.Namespace } // Create a client instance from the now-final config diff --git a/cmd/deploy.go b/cmd/deploy.go index ccc753edd..f16c5ba75 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -179,9 +179,9 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err // Sets default AND accepts any user-provided overrides f.Registry = config.Registry } - if f.Builder == "" || cmd.Flags().Changed("builder") { + if f.Build.Builder == "" || cmd.Flags().Changed("builder") { // Sets default AND accepts any user-provided overrides - f.Builder = config.Builder + f.Build.Builder = config.Builder } if config.Image != "" { f.Image = config.Image @@ -191,38 +191,38 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err f.ImageDigest = config.ImageDigest } if config.Builder != "" { - f.Builder = config.Builder + f.Build.Builder = config.Builder } if config.BuilderImage != "" { - f.BuilderImages[config.Builder] = config.BuilderImage + f.Build.BuilderImages[config.Builder] = config.BuilderImage } if config.GitURL != "" { parts := strings.Split(config.GitURL, "#") - f.Git.URL = parts[0] - if len(parts) == 2 { // see Validatee() where len enforced to be <= 2 - f.Git.Revision = parts[1] + f.Build.Git.URL = parts[0] + if len(parts) == 2 { // see Validate() where len enforced to be <= 2 + f.Build.Git.Revision = parts[1] } } if config.GitBranch != "" { - f.Git.Revision = config.GitBranch + f.Build.Git.Revision = config.GitBranch } if config.GitDir != "" { - f.Git.ContextDir = config.GitDir + f.Build.Git.ContextDir = config.GitDir } - f.Namespace = namespace(config, f, cmd.ErrOrStderr()) + f.Deploy.Namespace = namespace(config, f, cmd.ErrOrStderr()) if err != nil { return } - f.Envs, _, err = mergeEnvs(f.Envs, config.EnvToUpdate, config.EnvToRemove) + f.Run.Envs, _, err = mergeEnvs(f.Run.Envs, config.EnvToUpdate, config.EnvToRemove) if err != nil { return } // Validate that a builder short-name was obtained, whether that be from // the function's prior state, or the value of flags/environment. - if err = ValidateBuilder(f.Builder); err != nil { + if err = ValidateBuilder(f.Build.Builder); err != nil { return } @@ -230,21 +230,21 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err // override for the build image for that builder to use from the optional // builder-image flag. var builder fn.Builder - if f.Builder == builders.Pack { + if f.Build.Builder == builders.Pack { builder = buildpacks.NewBuilder( buildpacks.WithName(builders.Pack), buildpacks.WithVerbose(config.Verbose)) - } else if f.Builder == builders.S2I { + } else if f.Build.Builder == builders.S2I { builder = s2i.NewBuilder( s2i.WithName(builders.S2I), s2i.WithPlatform(config.Platform), s2i.WithVerbose(config.Verbose)) } else { - err = fmt.Errorf("builder '%v' is not recognized", f.Builder) + err = fmt.Errorf("builder '%v' is not recognized", f.Build.Builder) return } - client, done := newClient(ClientConfig{Namespace: f.Namespace, Verbose: config.Verbose}, + client, done := newClient(ClientConfig{Namespace: f.Deploy.Namespace, Verbose: config.Verbose}, fn.WithRegistry(config.Registry), fn.WithBuilder(builder)) defer done() @@ -269,7 +269,7 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err // Perform the deployment either remote or local. if config.Remote { - if f.Git.URL == "" { + if f.Build.Git.URL == "" { return ErrURLRequired // Provides CLI-specific help text } // Invoke a remote build/push/deploy pipeline @@ -681,8 +681,8 @@ func namespace(cfg deployConfig, f fn.Function, stderr io.Writer) (namespace str if cfg.Namespace != "" { namespace = cfg.Namespace // --namespace takes precidence - } else if f.Namespace != "" { - namespace = f.Namespace // value from previous deployment (func.yaml) 2nd priority + } else if f.Deploy.Namespace != "" { + namespace = f.Deploy.Namespace // value from previous deployment (func.yaml) 2nd priority } else { // Try to derive a default from the current k8s context, if any. if namespace, err = k8s.GetNamespace(""); err != nil { @@ -692,19 +692,19 @@ func namespace(cfg deployConfig, f fn.Function, stderr io.Writer) (namespace str // If the Function is not yet deployed, then immediately return the chosen // final namespace - if f.Namespace == "" { + if f.Deploy.Namespace == "" { return } // Warn if in a different namespace than active active, err := k8s.GetNamespace("") if err == nil && namespace != active { - fmt.Fprintf(stderr, "Warning: Function is in namespace '%s', but currently active namespace is '%s'. Continuing with redeployment to '%s'.\n", f.Namespace, active, namespace) + fmt.Fprintf(stderr, "Warning: Function is in namespace '%s', but currently active namespace is '%s'. Continuing with redeployment to '%s'.\n", f.Deploy.Namespace, active, namespace) } // Warn if potentially creating an orphan - if f.Namespace != "" && namespace != f.Namespace { - fmt.Fprintf(stderr, "Warning: function is in namespace '%s', but requested namespace is '%s'. Continuing with deployment to '%v'.\n", f.Namespace, namespace, namespace) + if f.Deploy.Namespace != "" && namespace != f.Deploy.Namespace { + fmt.Fprintf(stderr, "Warning: function is in namespace '%s', but requested namespace is '%s'. Continuing with deployment to '%v'.\n", f.Deploy.Namespace, namespace, namespace) } return diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index 9b6b6352b..d8d396c1f 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -270,7 +270,7 @@ func testBuilderPersists(cmdFn commandConstructor, t *testing.T) { if f, err = fn.NewFunction(root); err != nil { t.Fatal(err) } - if f.Builder == "" { + if f.Build.Builder == "" { t.Fatal("value of builder not persisted using a flag default") } @@ -284,7 +284,7 @@ func testBuilderPersists(cmdFn commandConstructor, t *testing.T) { if f, err = fn.NewFunction(root); err != nil { t.Fatal(err) } - if f.Builder != builders.S2I { + if f.Build.Builder != builders.S2I { t.Fatal("value of builder flag not persisted when provided") } @@ -300,7 +300,7 @@ func testBuilderPersists(cmdFn commandConstructor, t *testing.T) { if f, err = fn.NewFunction(root); err != nil { t.Fatal(err) } - if f.Builder != builders.S2I { + if f.Build.Builder != builders.S2I { t.Fatal("value of builder updated when not provided") } @@ -315,7 +315,7 @@ func testBuilderPersists(cmdFn commandConstructor, t *testing.T) { if f, err = fn.NewFunction(root); err != nil { t.Fatal(err) } - if f.Builder != builders.Pack { + if f.Build.Builder != builders.Pack { t.Fatal("value of builder flag not persisted on subsequent build") } @@ -690,9 +690,11 @@ func Test_namespace(t *testing.T) { // Creat a funcction which may be already deployed // (have a namespace) f := fn.Function{ - Runtime: "go", - Root: root, - Namespace: test.funcNamespace, + Runtime: "go", + Root: root, + Deploy: fn.DeploySpec{ + Namespace: test.funcNamespace, + }, } if err := fn.New().Create(f); err != nil { t.Fatal(err) @@ -762,14 +764,14 @@ func TestDeploy_GitArgsPersist(t *testing.T) { if err != nil { t.Fatal(err) } - if f.Git.URL != url { - t.Errorf("expected git URL '%v' got '%v'", url, f.Git.URL) + if f.Build.Git.URL != url { + t.Errorf("expected git URL '%v' got '%v'", url, f.Build.Git.URL) } - if f.Git.Revision != branch { - t.Errorf("expected git branch '%v' got '%v'", branch, f.Git.Revision) + if f.Build.Git.Revision != branch { + t.Errorf("expected git branch '%v' got '%v'", branch, f.Build.Git.Revision) } - if f.Git.ContextDir != dir { - t.Errorf("expected git dir '%v' got '%v'", dir, f.Git.ContextDir) + if f.Build.Git.ContextDir != dir { + t.Errorf("expected git dir '%v' got '%v'", dir, f.Build.Git.ContextDir) } } @@ -792,14 +794,14 @@ func TestDeploy_GitArgsUsed(t *testing.T) { // A Pipelines Provider which will validate the expected values were received pipeliner := mock.NewPipelinesProvider() pipeliner.RunFn = func(f fn.Function) error { - if f.Git.URL != url { - t.Errorf("Pipeline Provider expected git URL '%v' got '%v'", url, f.Git.URL) + if f.Build.Git.URL != url { + t.Errorf("Pipeline Provider expected git URL '%v' got '%v'", url, f.Build.Git.URL) } - if f.Git.Revision != branch { - t.Errorf("Pipeline Provider expected git branch '%v' got '%v'", branch, f.Git.Revision) + if f.Build.Git.Revision != branch { + t.Errorf("Pipeline Provider expected git branch '%v' got '%v'", branch, f.Build.Git.Revision) } - if f.Git.ContextDir != dir { - t.Errorf("Pipeline Provider expected git dir '%v' got '%v'", url, f.Git.ContextDir) + if f.Build.Git.ContextDir != dir { + t.Errorf("Pipeline Provider expected git dir '%v' got '%v'", url, f.Build.Git.ContextDir) } return nil } @@ -848,11 +850,11 @@ func TestDeploy_GitURLBranch(t *testing.T) { if err != nil { t.Fatal(err) } - if f.Git.URL != expectedUrl { - t.Errorf("expected git URL '%v' got '%v'", expectedUrl, f.Git.URL) + if f.Build.Git.URL != expectedUrl { + t.Errorf("expected git URL '%v' got '%v'", expectedUrl, f.Build.Git.URL) } - if f.Git.Revision != expectedBranch { - t.Errorf("expected git branch '%v' got '%v'", expectedBranch, f.Git.Revision) + if f.Build.Git.Revision != expectedBranch { + t.Errorf("expected git branch '%v' got '%v'", expectedBranch, f.Build.Git.Revision) } } @@ -874,7 +876,7 @@ func TestDeploy_NamespaceDefaults(t *testing.T) { // Assert it has no default namespace set f, err := fn.NewFunction(root) if err != nil { - t.Fatalf("newly created functions should not have a namespace set until deployed. Got '%v'", f.Namespace) + t.Fatalf("newly created functions should not have a namespace set until deployed. Got '%v'", f.Deploy.Namespace) } // New deploy command that will not actually deploy or build (mocked) @@ -899,8 +901,8 @@ func TestDeploy_NamespaceDefaults(t *testing.T) { if err != nil { t.Fatal(err) } - if f.Namespace != "test-ns-deploy" { // from testdata/kubeconfig_deploy_namespace - t.Fatalf("expected function to have active namespace 'test-ns-deploy' by default. got '%v'", f.Namespace) + if f.Deploy.Namespace != "test-ns-deploy" { // from testdata/kubeconfig_deploy_namespace + t.Fatalf("expected function to have active namespace 'test-ns-deploy' by default. got '%v'", f.Deploy.Namespace) } // See the knative package's tests for an example of tests which require // the knative or kubernetes API dependency. @@ -916,9 +918,11 @@ func TestDeploy_NamespaceUpdateWarning(t *testing.T) { // Create a Function which appears to have been deployed to 'myns' f := fn.Function{ - Runtime: "go", - Root: root, - Namespace: "myns", + Runtime: "go", + Root: root, + Deploy: fn.DeploySpec{ + Namespace: "myns", + }, } if err := fn.New().Create(f); err != nil { t.Fatal(err) @@ -957,8 +961,8 @@ func TestDeploy_NamespaceUpdateWarning(t *testing.T) { if err != nil { t.Fatal(err) } - if f.Namespace != "newns" { - t.Fatalf("expected function to be deoployed into namespace 'newns'. got '%v'", f.Namespace) + if f.Deploy.Namespace != "newns" { + t.Fatalf("expected function to be deoployed into namespace 'newns'. got '%v'", f.Deploy.Namespace) } } @@ -978,9 +982,9 @@ func TestDeploy_NamespaceRedeployWarning(t *testing.T) { // Create a Function which appears to have been deployed to 'myns' f := fn.Function{ - Runtime: "go", - Root: root, - Namespace: "myns", + Runtime: "go", + Root: root, + Deploy: fn.DeploySpec{Namespace: "myns"}, } if err := fn.New().Create(f); err != nil { t.Fatal(err) @@ -1014,7 +1018,7 @@ func TestDeploy_NamespaceRedeployWarning(t *testing.T) { if err != nil { t.Fatal(err) } - if f.Namespace != "myns" { - t.Fatalf("expected function to be updated with namespace 'myns'. got '%v'", f.Namespace) + if f.Deploy.Namespace != "myns" { + t.Fatalf("expected function to be updated with namespace 'myns'. got '%v'", f.Deploy.Namespace) } } diff --git a/cmd/repository_test.go b/cmd/repository_test.go index 0f557198f..af88cebce 100644 --- a/cmd/repository_test.go +++ b/cmd/repository_test.go @@ -11,7 +11,7 @@ import ( // TestRepository_List ensures that the 'list' subcommand shows the client's // set of repositories by name for builtin repositories, by explicitly -// setting the repositories path to a new path which includes no others. +// setting the repositories' path to a new path which includes no others. func TestRepository_List(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) cmd := NewRepositoryListCmd(NewClient) @@ -32,7 +32,7 @@ func TestRepository_List(t *testing.T) { } // TestRepository_Add ensures that the 'add' subcommand accepts its positional -// arguments, respects the repositories path flag, and the expected name is echoed +// arguments, respects the repositories' path flag, and the expected name is echoed // upon subsequent 'list'. func TestRepository_Add(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) @@ -70,8 +70,8 @@ func TestRepository_Add(t *testing.T) { } // TestRepository_Rename ensures that the 'rename' subcommand accepts its -// positional arguments, respects the repositories path flag, and the name is -// reflected as having been reanamed upon subsequent 'list'. +// positional arguments, respects the repositories' path flag, and the name is +// reflected as having been renamed upon subsequent 'list'. func TestRepository_Rename(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) var ( @@ -115,8 +115,8 @@ func TestRepository_Rename(t *testing.T) { } } -// TestReposotory_Remove ensures that the 'remove' subcommand accepts name as -// its argument, respects the repositorieis flag, and the entry is removed upon +// TestRepository_Remove ensures that the 'remove' subcommand accepts name as +// its argument, respects the repositories' flag, and the entry is removed upon // subsequent 'list'. func TestRepository_Remove(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) diff --git a/cmd/run.go b/cmd/run.go index 94fee327d..24ffdadb1 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -78,7 +78,7 @@ func runRun(cmd *cobra.Command, args []string, newClient ClientFactory) (err err return fmt.Errorf("the given path '%v' does not contain an initialized function", config.Path) } var updated int - function.Envs, updated, err = mergeEnvs(function.Envs, config.EnvToUpdate, config.EnvToRemove) + function.Run.Envs, updated, err = mergeEnvs(function.Run.Envs, config.EnvToUpdate, config.EnvToRemove) if err != nil { return } diff --git a/docker/runner.go b/docker/runner.go index d2c8dc6d6..b0a6ff83d 100644 --- a/docker/runner.go +++ b/docker/runner.go @@ -195,7 +195,7 @@ func newContainerConfig(f fn.Function, _ string, verbose bool) (c container.Conf // Environment Variables // Interpolate references to local environment variables and convert to a // simple string slice for use with container.Config - envs, err := fn.Interpolate(f.Envs) + envs, err := fn.Interpolate(f.Run.Envs) if err != nil { return } diff --git a/function.go b/function.go index bc7fc33fe..1fb72aad2 100644 --- a/function.go +++ b/function.go @@ -18,6 +18,7 @@ import ( // FunctionFile is the file used for the serialized form of a function. const FunctionFile = "func.yaml" +// Function type Function struct { // SpecVersion at which this function is known to be compatible. // More specifically, it is the highest migration which has been applied. @@ -28,12 +29,9 @@ type Function struct { Root string `yaml:"-"` // Name of the function. If not provided, path derivation is attempted when - // requried (such as for initialization). + // required (such as for initialization). Name string `yaml:"name" jsonschema:"pattern=^[a-z0-9]([-a-z0-9]*[a-z0-9])?$"` - // Namespace into which the function is deployed on supported platforms. - Namespace string `yaml:"namespace"` - // Runtime is the language plus context. nodejs|go|quarkus|rust etc. Runtime string `yaml:"runtime"` @@ -58,6 +56,27 @@ type Function struct { // SHA256 hash of the latest image that has been built ImageDigest string `yaml:"imageDigest"` + // Created time is the moment that creation was successfully completed + // according to the client which is in charge of what constitutes being + // fully "Created" (aka initialized) + Created time.Time `yaml:"created"` + + // Invocation defines hints for use when invoking this function. + // See Client.Invoke for usage. + Invocation Invocation `yaml:"invocation,omitempty"` + + //BuildSpec define the build properties for a function + Build BuildSpec `yaml:"build"` + + //RunSpec define the runtime properties for a function + Run RunSpec `yaml:"run"` + + //DeploySpec define the deployment properties for a function + Deploy DeploySpec `yaml:"deploy"` +} + +// BuildSpec +type BuildSpec struct { // Git stores information about an optionally associated git repository. Git Git `yaml:"git,omitempty"` @@ -76,14 +95,23 @@ type Function struct { // build (pack, s2i, etc) Builder string `yaml:"builder" jsonschema:"enum=pack,enum=s2i"` + // Build Env variables to be set + BuildEnvs []Env `yaml:"buildEnvs"` +} + +// RunSpec +type RunSpec struct { // List of volumes to be mounted to the function Volumes []Volume `yaml:"volumes"` - // Build Env variables to be set - BuildEnvs []Env `yaml:"buildEnvs"` - // Env variables to be set Envs []Env `yaml:"envs"` +} + +// DeploySpec +type DeploySpec struct { + // Namespace into which the function is deployed on supported platforms. + Namespace string `yaml:"namespace"` // Map containing user-supplied annotations // Example: { "division": "finance" } @@ -97,15 +125,6 @@ type Function struct { // Health endpoints specified by the language pack HealthEndpoints HealthEndpoints `yaml:"healthEndpoints"` - - // Created time is the moment that creation was successfully completed - // according to the client which is in charge of what constitutes being - // fully "Created" (aka initialized) - Created time.Time `yaml:"created"` - - // Invocation defines hints for use when invoking this function. - // See Client.Invoke for usage. - Invocation Invocation `yaml:"invocation,omitempty"` } // HealthEndpoints specify the liveness and readiness endpoints for a Runtime @@ -150,25 +169,37 @@ func NewFunctionWith(defaults Function) Function { // Functions from earlier versions are brought up to current using migrations. // Migrations are run prior to validators such that validation can omit // concerning itself with backwards compatibility. Migrators must therefore -// selectively consider the minimal validation necesssary to ehable migration. +// selectively consider the minimal validation necessary to enable migration. func NewFunction(path string) (f Function, err error) { + f.Root = path // path is not persisted, as this is the purview of the FS itself f.Root = path // path is not persisted, as this is the purvew of the FS itself - f.BuilderImages = make(map[string]string) - f.Annotations = make(map[string]string) + f.Build.BuilderImages = make(map[string]string) + f.Deploy.Annotations = make(map[string]string) var filename = filepath.Join(path, FunctionFile) if _, err = os.Stat(filename); err != nil { return } - bb, err := ioutil.ReadFile(filename) + bb, err := os.ReadFile(filename) if err != nil { return } - if err = yaml.Unmarshal(bb, &f); err != nil { - err = formatUnmarshalError(err) // human-friendly unmarshalling errors - 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 { - return + 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) } return f, f.Validate() } @@ -188,12 +219,12 @@ func (f Function) Validate() error { var ctr int errs := [][]string{ - validateVolumes(f.Volumes), - ValidateBuildEnvs(f.BuildEnvs), - ValidateEnvs(f.Envs), - validateOptions(f.Options), - ValidateLabels(f.Labels), - validateGit(f.Git), + validateVolumes(f.Run.Volumes), + ValidateBuildEnvs(f.Build.BuildEnvs), + ValidateEnvs(f.Run.Envs), + validateOptions(f.Deploy.Options), + ValidateLabels(f.Deploy.Labels), + validateGit(f.Build.Git), } var b strings.Builder @@ -222,7 +253,7 @@ var envPattern = regexp.MustCompile(`^{{\s*(\w+)\s*:(\w+)\s*}}$`) // Values with no special format are preserved as simple values. // Values which do include the interpolation format (begin with {{) but are not // keyed as "env" are also preserved as is. -// Values properly formated as {{ env:NAME }} are interpolated (substituted) +// Values properly formatted as {{ env:NAME }} are interpolated (substituted) // with the value of the local environment variable "NAME", and an error is // returned if that environment variable does not exist. func Interpolate(ee []Env) (map[string]string, error) { @@ -293,7 +324,7 @@ func (f Function) Write() (err error) { } // TODO: open existing file for writing, such that existing permissions // are preserved. - return ioutil.WriteFile(path, bb, 0644) + return os.WriteFile(path, bb, 0644) } // Initialized returns if the function has been initialized. @@ -365,7 +396,7 @@ func (f Function) LabelsMap() (map[string]string, error) { // --- end of handling usage of deprecated runtime labels } - labels := append(defaultLabels, f.Labels...) + labels := append(defaultLabels, f.Deploy.Labels...) if err := ValidateLabels(labels); len(err) != 0 { return nil, errors.New(strings.Join(err, " ")) } diff --git a/function_migrations.go b/function_migrations.go index 8da55583c..11a30c331 100644 --- a/function_migrations.go +++ b/function_migrations.go @@ -1,7 +1,8 @@ package function import ( - "io/ioutil" + "errors" + "os" "path/filepath" "time" @@ -22,7 +23,7 @@ func (f Function) Migrate() (migrated Function, err error) { migrated = f // initially equivalent for _, m := range migrations { // Skip this migration if the current function's specVersion is not less than - // the migration's applicable specVerion. + // the migration's applicable specVersion. if f.SpecVersion != "" && !semver.New(migrated.SpecVersion).LessThan(*semver.New(m.version)) { continue } @@ -36,7 +37,7 @@ func (f Function) Migrate() (migrated Function, err error) { return } -// migration is a migration which should be applied to functions whose version +// migration is a migration which should be applied to function's whose version // is below that indicated. type migration struct { version string // version before which this migration may be needed. @@ -46,7 +47,7 @@ type migration struct { // migrator is a function which returns a migrated copy of an inbound function. type migrator func(Function, migration) (Function, error) -// Migrated returns whether or not the function has been migrated to the highest +// Migrated returns whether the function has been migrated to the highest // level the currently executing system is aware of (or beyond). // returns true. func (f Function) Migrated() bool { @@ -79,6 +80,7 @@ var migrations = []migration{ {"0.19.0", migrateToCreationStamp}, {"0.23.0", migrateToBuilderImages}, {"0.25.0", migrateToSpecVersion}, + {"0.34.0", migrateToSpecsStructure}, // New Migrations Here. } @@ -93,24 +95,24 @@ var migrations = []migration{ // initialized" in func versions above v0.19.0 // // This migration must be aware of the difference between a function which -// was previously created (but with no create stamp), and a function which +// was previously created (but with no created stamp), and a function which // exists only in memory and should legitimately fail the .Initialized() check. -// The only way to know is to check a side-effect of earlier versions: -// are the .Name and .Runtime fields populated. This was the way the -// .Initialized check was implemented prior to versioning being introduced, so +// The only way to know is to check a side effect of earlier versions: +// are the `.Name` and `.Runtime` fields populated. This was the way the +// `.Initialized` check was implemented prior to versioning being introduced, so // it is equivalent logically to use this here as well. // In summary: if the creation stamp is zero, but name and runtime fields are // populated, then this is an old function and should be migrated to having a -// create stamp. Otherwise, this is an in-memory (new) function that is +// 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) { // For functions with no creation timestamp, but appear to have been pre- - // existing, populate their create stamp and version. - // Yes, it's a little gnarly, but bootstrapping into the lovelieness of a + // existing, populate their created stamp and version. + // Yes, it's a little gnarly, but bootstrapping into the loveliness of a // versioned/migrated system takes cleaning up the trash. - if f.Created.IsZero() { // If there is no create stamp + if f.Created.IsZero() { // If there is no created stamp if f.Name != "" && f.Runtime != "" { // and it appears to be an old function f.Created = time.Now() // Migrate it to having a timestamp. } @@ -155,13 +157,13 @@ func migrateToCreationStamp(f Function, m migration) (Function, error) { func migrateToBuilderImages(f1 Function, m migration) (Function, error) { // Load the function using pertinent parts of the previous version's schema: f0Filename := filepath.Join(f1.Root, FunctionFile) - bb, err := ioutil.ReadFile(f0Filename) + bb, err := os.ReadFile(f0Filename) if err != nil { - return f1, err + return f1, errors.New("migration 'migrateToBuilderImages' error: " + err.Error()) } f0 := migrateToBuilderImages_previousFunction{} if err = yaml.Unmarshal(bb, &f0); err != nil { - return f1, err + return f1, errors.New("migration 'migrateToBuilderImages' error: " + err.Error()) } // At time of this migration, the default pack builder image for all language @@ -171,10 +173,10 @@ func migrateToBuilderImages(f1 Function, m migration) (Function, error) { // If the old function had defined something custom if f0.Builder != "" && f0.Builder != defaultPackBuilderImage { // carry it forward as the new pack builder image - if f1.BuilderImages == nil { - f1.BuilderImages = make(map[string]string) + if f1.Build.BuilderImages == nil { + f1.Build.BuilderImages = make(map[string]string) } - f1.BuilderImages["pack"] = f0.Builder + f1.Build.BuilderImages["pack"] = f0.Builder } // Flag f1 as having had the migration applied @@ -188,21 +190,146 @@ func migrateToBuilderImages(f1 Function, m migration) (Function, error) { func migrateToSpecVersion(f Function, m migration) (Function, error) { // Load the function func.yaml file f0Filename := filepath.Join(f.Root, FunctionFile) - bb, err := ioutil.ReadFile(f0Filename) + bb, err := os.ReadFile(f0Filename) if err != nil { - return f, err + 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, err + return f, errors.New("migration 'migrateToSpecVersion' error: " + err.Error()) } f.SpecVersion = m.version return f, nil } +// 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) { + // 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 f0.Git.URL != "" { + f1.Build.Git.URL = f0.Git.URL + } + if f0.Git.Revision != "" { + f1.Build.Git.Revision = f0.Git.Revision + } + if f0.Git.ContextDir != "" { + f1.Build.Git.ContextDir = f0.Git.ContextDir + } + //Append BuilderImages from old format, without destroying previous migrations + if f0.BuilderImages != nil { + for k, v := range f0.BuilderImages { + f1.Build.BuilderImages[k] = v + } + } + if f0.Buildpacks != nil { + f1.Build.Buildpacks = append(f1.Build.Buildpacks, f0.Buildpacks...) + } + if f0.BuildEnvs != nil { + f1.Build.BuildEnvs = append(f1.Build.BuildEnvs, f0.BuildEnvs...) + } + + if f0.Volumes != nil { + f1.Run.Volumes = append(f1.Run.Volumes, f0.Volumes...) + } + + if f0.Envs != nil { + f1.Run.Envs = append(f1.Run.Envs, f0.Envs...) + } + + if f0.Annotations != nil { + for k, v := range f0.Annotations { + f1.Deploy.Annotations[k] = v + } + } + + if f0.Options.Resources != nil { + f1.Deploy.Options.Resources = f0.Options.Resources + } + + if f0.Options.Scale != nil { + f1.Deploy.Options.Scale = f0.Options.Scale + } + + if f0.Labels != nil { + f1.Deploy.Labels = append(f1.Deploy.Labels, f0.Labels...) + } + + if f0.HealthEndpoints.Readiness != "" { + f1.Deploy.HealthEndpoints.Readiness = f0.HealthEndpoints.Readiness + } + + if f0.HealthEndpoints.Liveness != "" { + f1.Deploy.HealthEndpoints.Liveness = f0.HealthEndpoints.Liveness + } + + f1.Deploy.Namespace = f0.Namespace + f1.Build.Builder = f0.Builder + f1.SpecVersion = m.version + return f1, nil +} + +// The pertinent aspects of the Function's schema prior the 1.0.0 version migrations +type migrateToSpecs_previousFunction struct { + + // Namespace into which the function is deployed on supported platforms. + Namespace string `yaml:"namespace"` + + // Git stores information about remote git repository, + // in case build type "git" is being used + Git Git `yaml:"git"` + + // BuilderImages define optional explicit builder images to use by + // builder implementations in leau of the in-code defaults. They key + // is the builder's short name. For example: + // builderImages: + // pack: example.com/user/my-pack-node-builder + // s2i: example.com/user/my-s2i-node-builder + BuilderImages map[string]string `yaml:"builderImages,omitempty"` + + // Optional list of buildpacks to use when building the function + Buildpacks []string `yaml:"buildpacks"` + + // Builder is the name of the subsystem that will complete the underlying + // build (pack, s2i, etc) + Builder string `yaml:"builder" jsonschema:"enum=pack,enum=s2i"` + + // List of volumes to be mounted to the function + Volumes []Volume `yaml:"volumes"` + + // Build Env variables to be set + BuildEnvs []Env `yaml:"buildEnvs"` + + // Env variables to be set + Envs []Env `yaml:"envs"` + + // Map containing user-supplied annotations + // Example: { "division": "finance" } + Annotations map[string]string `yaml:"annotations"` + + // Options to be set on deployed function (scaling, etc.) + Options Options `yaml:"options"` + + // Map of user-supplied labels + Labels []Label `yaml:"labels"` + + // Health endpoints specified by the language pack + HealthEndpoints HealthEndpoints `yaml:"healthEndpoints"` +} + // Functions prior to 0.25 will have a Version field type migrateToSpecVersion_previousFunction struct { Version string `yaml:"version"` diff --git a/function_migrations_unit_test.go b/function_migrations_unit_test.go index 553db5fc2..da55b2e2b 100644 --- a/function_migrations_unit_test.go +++ b/function_migrations_unit_test.go @@ -114,7 +114,7 @@ func TestMigrateToBuilderImagesCustom(t *testing.T) { if err != nil { t.Fatal(f) } - i, ok := f.BuilderImages["pack"] + i, ok := f.Build.BuilderImages["pack"] if !ok { t.Fatal("migrated function does not include the pack builder images") } @@ -136,3 +136,37 @@ func TestMigrateToSpecVersion(t *testing.T) { t.Fatal("migrated function does not include the Migration field") } } + +// TestMigrateToSpecs ensures that the migration to the sub-specs format from +// the previous Function structure works +func TestMigrateToSpecs(t *testing.T) { + + root := "testdata/migrations/v0.34.0" + expectedGit := Git{URL: "http://test-url", Revision: "test revision", ContextDir: "/test/context/dir"} + expectedNamespace := "test-namespace" + var expectedEnvs []Env + var expectedVolumes []Volume + + f, err := NewFunction(root) + if err != nil { + t.Error(err) + t.Fatal(f) + } + + if f.Build.Git != expectedGit { + t.Fatalf("migrated Function expected Git '%v', got '%v'", expectedGit, f.Build.Git) + } + + if f.Deploy.Namespace != expectedNamespace { + t.Fatalf("migrated Function expected Namespace '%v', got '%v'", expectedNamespace, f.Deploy.Namespace) + } + + if len(f.Run.Envs) != len(expectedEnvs) { + t.Fatalf("migrated Function expected Run Envs '%v', got '%v'", len(expectedEnvs), len(f.Run.Envs)) + } + + if len(f.Run.Volumes) != len(expectedVolumes) { + t.Fatalf("migrated Function expected Run Volumes '%v', got '%v'", len(expectedEnvs), len(f.Run.Envs)) + } + +} diff --git a/function_test.go b/function_test.go index 01ba363c0..789b70963 100644 --- a/function_test.go +++ b/function_test.go @@ -4,6 +4,7 @@ package function_test import ( + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -57,7 +58,7 @@ func TestFunction_NameDefault(t *testing.T) { defer Using(t, root)() f, err := fn.NewFunction(root) if err == nil { - t.Fatal("expected error instantiating a nonexistant function") + t.Fatal("expected error instantiating a nonexistent function") } // Create the function at the path @@ -86,7 +87,7 @@ func TestFunction_NameDefault(t *testing.T) { // environment variables by interpolating properly formatted references to // local environment variables, returning a final simple map structure. // Also ensures that nil value references are interpreted as meaning the -// environment is to be disincluded from the resultant map, rather than included +// environment is not to be included in the resultant map, rather than included // with an empty value. // TODO: Perhaps referring to a nonexistent local env var should be treated // as a "leave as is" (do not set) rather than "required" resulting in error? @@ -135,3 +136,34 @@ func Test_Interpolate(t *testing.T) { t.Fatalf("expected envs with a nil value to not be included in interpolation result") } } + +// TestFunction_MarshallingError check that the correct error gets reported back to the +// user if the function that is being loaded is failing marshalling and cannot be migrated +func TestFunction_MarshallingError(t *testing.T) { + root := "testdata/testFunctionMarshallingError" + + // Load the function to see it fail with a marshalling error + _, err := fn.NewFunction(root) + if err != nil { + if !strings.Contains(err.Error(), "Marshalling: 'func.yaml' is not valid:") { + t.Fatalf("expected unmarshalling error") + } + + } +} + +// TestFunction_MigrationError check that the correct error gets reported back to the +// user if the function that is being loaded is failing marshalling and cannot be migrated +func TestFunction_MigrationError(t *testing.T) { + root := "testdata/testFunctionMigrationError" + + // Load the function to see it fail with a migration error + _, err := fn.NewFunction(root) + if err != nil { + // This function makes the migration fails + if !strings.Contains(err.Error(), "migration 'migrateToBuilderImages' error") { + t.Fatalf("expected migration error") + } + } + +} diff --git a/function_unit_test.go b/function_unit_test.go index e8990ead0..c150feef5 100644 --- a/function_unit_test.go +++ b/function_unit_test.go @@ -173,7 +173,7 @@ func Test_LabelsMap(t *testing.T) { f := Function{ Name: "some-function", Runtime: "golang", - Labels: tt.labels, + Deploy: DeploySpec{Labels: tt.labels}, } got, err := f.LabelsMap() diff --git a/knative/deployer.go b/knative/deployer.go index 78f30c44d..b6ade9bd2 100644 --- a/knative/deployer.go +++ b/knative/deployer.go @@ -222,12 +222,12 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu referencedSecrets := sets.NewString() referencedConfigMaps := sets.NewString() - newEnv, newEnvFrom, err := processEnvs(f.Envs, &referencedSecrets, &referencedConfigMaps) + newEnv, newEnvFrom, err := processEnvs(f.Run.Envs, &referencedSecrets, &referencedConfigMaps) if err != nil { return fn.DeploymentResult{}, err } - newVolumes, newVolumeMounts, err := processVolumes(f.Volumes, &referencedSecrets, &referencedConfigMaps) + newVolumes, newVolumeMounts, err := processVolumes(f.Run.Volumes, &referencedSecrets, &referencedConfigMaps) if err != nil { return fn.DeploymentResult{}, err } @@ -274,11 +274,11 @@ func setHealthEndpoints(f fn.Function, c *corev1.Container) *corev1.Container { c.ReadinessProbe = probeFor(READINESS_ENDPOINT) // If specified in func.yaml, the provided values override the defaults - if f.HealthEndpoints.Liveness != "" { - c.LivenessProbe = probeFor(f.HealthEndpoints.Liveness) + if f.Deploy.HealthEndpoints.Liveness != "" { + c.LivenessProbe = probeFor(f.Deploy.HealthEndpoints.Liveness) } - if f.HealthEndpoints.Readiness != "" { - c.ReadinessProbe = probeFor(f.HealthEndpoints.Readiness) + if f.Deploy.HealthEndpoints.Readiness != "" { + c.ReadinessProbe = probeFor(f.Deploy.HealthEndpoints.Readiness) } return c } @@ -292,14 +292,14 @@ func generateNewService(f fn.Function, decorator DeployDecorator) (*v1.Service, referencedSecrets := sets.NewString() referencedConfigMaps := sets.NewString() - newEnv, newEnvFrom, err := processEnvs(f.Envs, &referencedSecrets, &referencedConfigMaps) + newEnv, newEnvFrom, err := processEnvs(f.Run.Envs, &referencedSecrets, &referencedConfigMaps) if err != nil { return nil, err } container.Env = newEnv container.EnvFrom = newEnvFrom - newVolumes, newVolumeMounts, err := processVolumes(f.Volumes, &referencedSecrets, &referencedConfigMaps) + newVolumes, newVolumeMounts, err := processVolumes(f.Run.Volumes, &referencedSecrets, &referencedConfigMaps) if err != nil { return nil, err } @@ -313,7 +313,7 @@ func generateNewService(f fn.Function, decorator DeployDecorator) (*v1.Service, labels = decorator.UpdateLabels(f, labels) } - annotations := f.Annotations + annotations := f.Deploy.Annotations if decorator != nil { annotations = decorator.UpdateAnnotations(f, annotations) } @@ -344,7 +344,7 @@ func generateNewService(f fn.Function, decorator DeployDecorator) (*v1.Service, }, } - err = setServiceOptions(&service.Spec.Template, f.Options) + err = setServiceOptions(&service.Spec.Template, f.Deploy.Options) if err != nil { return service, err } @@ -364,7 +364,7 @@ func updateService(f fn.Function, newEnv []corev1.EnvVar, newEnvFrom []corev1.En service.ObjectMeta.Annotations = decorator.UpdateAnnotations(f, service.ObjectMeta.Annotations) } - for k, v := range f.Annotations { + for k, v := range f.Deploy.Annotations { service.ObjectMeta.Annotations[k] = v service.Spec.Template.ObjectMeta.Annotations[k] = v } @@ -383,7 +383,7 @@ func updateService(f fn.Function, newEnv []corev1.EnvVar, newEnvFrom []corev1.En cp := &service.Spec.Template.Spec.Containers[0] setHealthEndpoints(f, cp) - err := setServiceOptions(&service.Spec.Template, f.Options) + err := setServiceOptions(&service.Spec.Template, f.Deploy.Options) if err != nil { return service, err } diff --git a/knative/deployer_test.go b/knative/deployer_test.go index b5af681c0..d6b512c98 100644 --- a/knative/deployer_test.go +++ b/knative/deployer_test.go @@ -37,9 +37,11 @@ func cwd() (cwd string) { func Test_setHealthEndpoints(t *testing.T) { f := fn.Function{ Name: "testing", - HealthEndpoints: fn.HealthEndpoints{ - Liveness: "/lively", - Readiness: "/readyAsIllEverBe", + Deploy: fn.DeploySpec{ + HealthEndpoints: fn.HealthEndpoints{ + Liveness: "/lively", + Readiness: "/readyAsIllEverBe", + }, }, } c := corev1.Container{} diff --git a/openshift/metadata.go b/openshift/metadata.go index a0cffa78f..a6d7b5ff0 100644 --- a/openshift/metadata.go +++ b/openshift/metadata.go @@ -26,8 +26,8 @@ func (o OpenshiftMetadataDecorator) UpdateAnnotations(f fn.Function, annotations if annotations == nil { annotations = map[string]string{} } - annotations[annotationOpenShiftVcsUri] = f.Git.URL - annotations[annotationOpenShiftVcsRef] = f.Git.Revision + annotations[annotationOpenShiftVcsUri] = f.Build.Git.URL + annotations[annotationOpenShiftVcsRef] = f.Build.Git.Revision return annotations } diff --git a/pipelines/tekton/pipeplines_provider.go b/pipelines/tekton/pipeplines_provider.go index ed12f6ece..1ef46f661 100644 --- a/pipelines/tekton/pipeplines_provider.go +++ b/pipelines/tekton/pipeplines_provider.go @@ -105,7 +105,7 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) error { labels = pp.decorator.UpdateLabels(f, labels) } - err = k8s.CreatePersistentVolumeClaim(ctx, getPipelinePvcName(f), pp.namespace, labels, f.Annotations, corev1.ReadWriteOnce, *resource.NewQuantity(DefaultPersistentVolumeClaimSize, resource.DecimalSI)) + err = k8s.CreatePersistentVolumeClaim(ctx, getPipelinePvcName(f), pp.namespace, labels, f.Deploy.Annotations, corev1.ReadWriteOnce, *resource.NewQuantity(DefaultPersistentVolumeClaimSize, resource.DecimalSI)) if err != nil { if !errors.IsAlreadyExists(err) { return fmt.Errorf("problem creating persistent volume claim: %v", err) @@ -138,7 +138,7 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) error { registry = authn.DefaultAuthKey } - err = k8s.EnsureDockerRegistrySecretExist(ctx, getPipelineSecretName(f), pp.namespace, labels, f.Annotations, creds.Username, creds.Password, registry) + err = k8s.EnsureDockerRegistrySecretExist(ctx, getPipelineSecretName(f), pp.namespace, labels, f.Deploy.Annotations, creds.Username, creds.Password, registry) if err != nil { return fmt.Errorf("problem in creating secret: %v", err) } diff --git a/pipelines/tekton/resources.go b/pipelines/tekton/resources.go index 2c5823f45..6393c46e6 100644 --- a/pipelines/tekton/resources.go +++ b/pipelines/tekton/resources.go @@ -42,7 +42,7 @@ func generatePipeline(f fn.Function, labels map[string]string) *pplnv1beta1.Pipe { Name: "gitRepository", Description: "Git repository that hosts the function project", - Default: pplnv1beta1.NewArrayOrString(f.Git.URL), + Default: pplnv1beta1.NewArrayOrString(f.Build.Git.URL), }, { Name: "gitRevision", @@ -78,12 +78,12 @@ func generatePipeline(f fn.Function, labels map[string]string) *pplnv1beta1.Pipe // Deploy step that uses an image produced by S2I builds needs explicit reference to the image referenceImageFromPreviousTaskResults := false - if f.Builder == builders.Pack { + if f.Build.Builder == builders.Pack { // ----- Buildpacks related properties workspaces = append(workspaces, pplnv1beta1.PipelineWorkspaceDeclaration{Name: "cache-workspace", Description: "Directory where Buildpacks cache is stored."}) taskBuild = taskBuildpacks(taskNameFetchSources) - } else if f.Builder == builders.S2I { + } else if f.Build.Builder == builders.S2I { // ----- S2I build related properties params = append(params, pplnv1beta1.ParamSpec{Name: "s2iImageScriptsUrl", Description: "URL containing the default assemble and run scripts for the builder image.", @@ -104,7 +104,7 @@ func generatePipeline(f fn.Function, labels map[string]string) *pplnv1beta1.Pipe ObjectMeta: v1.ObjectMeta{ Name: pipelineName, Labels: labels, - Annotations: f.Annotations, + Annotations: f.Deploy.Annotations, }, Spec: pplnv1beta1.PipelineSpec{ Params: params, @@ -116,9 +116,9 @@ func generatePipeline(f fn.Function, labels map[string]string) *pplnv1beta1.Pipe func generatePipelineRun(f fn.Function, labels map[string]string) *pplnv1beta1.PipelineRun { - revision := f.Git.Revision - contextDir := f.Git.ContextDir - if contextDir == "" && f.Builder == builders.S2I { + revision := f.Build.Git.Revision + contextDir := f.Build.Git.ContextDir + if contextDir == "" && f.Build.Builder == builders.S2I { // TODO(lkingland): could instead update S2I to interpret empty string // as cwd, such that builder-specific code can be kept out of here. contextDir = "." @@ -128,9 +128,9 @@ func generatePipelineRun(f fn.Function, labels map[string]string) *pplnv1beta1.P Type: pplnv1beta1.ParamTypeArray, ArrayVal: []string{}, } - if len(f.BuildEnvs) > 0 { + if len(f.Build.BuildEnvs) > 0 { var envs []string - for _, e := range f.BuildEnvs { + for _, e := range f.Build.BuildEnvs { envs = append(envs, e.KeyValuePair()) } buildEnvs.ArrayVal = envs @@ -143,7 +143,7 @@ func generatePipelineRun(f fn.Function, labels map[string]string) *pplnv1beta1.P params := []pplnv1beta1.Param{ { Name: "gitRepository", - Value: *pplnv1beta1.NewArrayOrString(f.Git.URL), + Value: *pplnv1beta1.NewArrayOrString(f.Build.Git.URL), }, { Name: "gitRevision", @@ -183,7 +183,7 @@ func generatePipelineRun(f fn.Function, labels map[string]string) *pplnv1beta1.P }, } - if f.Builder == builders.Pack { + if f.Build.Builder == builders.Pack { // ----- Buildpacks related properties workspaces = append(workspaces, pplnv1beta1.WorkspaceBinding{ @@ -193,7 +193,7 @@ func generatePipelineRun(f fn.Function, labels map[string]string) *pplnv1beta1.P }, SubPath: "cache", }) - } else if f.Builder == builders.S2I { + } else if f.Build.Builder == builders.S2I { if f.Runtime == "quarkus" { params = append(params, pplnv1beta1.Param{Name: "s2iImageScriptsUrl", Value: *pplnv1beta1.NewArrayOrString("image:///usr/local/s2i")}) } @@ -204,7 +204,7 @@ func generatePipelineRun(f fn.Function, labels map[string]string) *pplnv1beta1.P ObjectMeta: v1.ObjectMeta{ GenerateName: fmt.Sprintf("%s-run-", getPipelineName(f)), Labels: labels, - Annotations: f.Annotations, + Annotations: f.Deploy.Annotations, }, Spec: pplnv1beta1.PipelineRunSpec{ PipelineRef: &pplnv1beta1.PipelineRef{ @@ -221,7 +221,7 @@ func generatePipelineRun(f fn.Function, labels map[string]string) *pplnv1beta1.P // language runtime. Errors are checked elsewhere, so at this level they // manifest as an inability to get a builder image = empty string. func getBuilderImage(f fn.Function) (name string) { - if f.Builder == builders.S2I { + if f.Build.Builder == builders.S2I { name, _ = s2i.BuilderImage(f, builders.S2I) } else { name, _ = buildpacks.BuilderImage(f, builders.Pack) @@ -230,7 +230,7 @@ func getBuilderImage(f fn.Function) (name string) { } func getPipelineName(f fn.Function) string { - return fmt.Sprintf("%s-%s-pipeline", f.Name, f.Builder) + return fmt.Sprintf("%s-%s-pipeline", f.Name, f.Build.Builder) } func getPipelineSecretName(f fn.Function) string { diff --git a/pipelines/tekton/resources_test.go b/pipelines/tekton/resources_test.go index e1e16b459..b410b3f01 100644 --- a/pipelines/tekton/resources_test.go +++ b/pipelines/tekton/resources_test.go @@ -23,12 +23,12 @@ func Test_generatePipeline(t *testing.T) { }{ { name: "Pack builder - use buildpacks", - function: fn.Function{Builder: builders.Pack, Git: testGit}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.Pack, Git: testGit}}, taskBuildName: "func-buildpacks", }, { name: "s2i builder - use", - function: fn.Function{Builder: builders.S2I, Git: testGit}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.S2I, Git: testGit}}, taskBuildName: "func-s2i", }, } @@ -41,7 +41,7 @@ func Test_generatePipeline(t *testing.T) { // let's check what is the Task used for build task if task.Name == taskNameBuild { if task.TaskRef.Name != tt.taskBuildName { - t.Errorf("generatePipeline(), for builder = %q: wanted build Task = %q, got = %q", tt.function.Builder, tt.taskBuildName, task.TaskRef.Name) + t.Errorf("generatePipeline(), for builder = %q: wanted build Task = %q, got = %q", tt.function.Build.Builder, tt.taskBuildName, task.TaskRef.Name) } return } diff --git a/pipelines/tekton/validate.go b/pipelines/tekton/validate.go index 330d2667d..998fabf47 100644 --- a/pipelines/tekton/validate.go +++ b/pipelines/tekton/validate.go @@ -25,7 +25,7 @@ func (e ErrRuntimeNotSupported) Error() string { } func validatePipeline(f fn.Function) error { - if f.Builder == builders.Pack { + if f.Build.Builder == builders.Pack { if f.Runtime == "" { return ErrRuntimeRequired } @@ -34,14 +34,14 @@ func validatePipeline(f fn.Function) error { return ErrRuntimeNotSupported{f.Runtime} } - if len(f.Buildpacks) > 0 { + if len(f.Build.Buildpacks) > 0 { return ErrBuilpacksNotSupported } - } else if f.Builder == builders.S2I { + } else if f.Build.Builder == builders.S2I { _, err := s2i.BuilderImage(f, builders.S2I) return err } else { - return builders.ErrUnknownBuilder{Name: f.Builder} + return builders.ErrUnknownBuilder{Name: f.Build.Builder} } return nil diff --git a/pipelines/tekton/validate_test.go b/pipelines/tekton/validate_test.go index e822adb78..efaff9f4e 100644 --- a/pipelines/tekton/validate_test.go +++ b/pipelines/tekton/validate_test.go @@ -26,27 +26,27 @@ func Test_validatePipeline(t *testing.T) { }, { name: "Without runtime - pack builder - without additional Buildpacks", - function: fn.Function{Builder: builders.Pack}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.Pack}}, wantErr: true, }, { name: "Without runtime - s2i builder", - function: fn.Function{Builder: builders.S2I}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.S2I}}, wantErr: true, }, { name: "Without runtime - without builder - with additional Buildpacks", - function: fn.Function{Buildpacks: testBuildpacks}, + function: fn.Function{Build: fn.BuildSpec{Buildpacks: testBuildpacks}}, wantErr: true, }, { name: "Without runtime - pack builder - with additional Buildpacks", - function: fn.Function{Builder: builders.Pack, Buildpacks: testBuildpacks}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.Pack, Buildpacks: testBuildpacks}}, wantErr: true, }, { name: "Without runtime - s2i builder", - function: fn.Function{Builder: builders.S2I, Buildpacks: testBuildpacks}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.S2I, Buildpacks: testBuildpacks}}, wantErr: true, }, { @@ -56,52 +56,52 @@ func Test_validatePipeline(t *testing.T) { }, { name: "Supported runtime - pack builder - without additional Buildpacks", - function: fn.Function{Builder: builders.Pack, Runtime: "node"}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.Pack}, Runtime: "node"}, wantErr: false, }, { name: "Supported runtime - s2i builder", - function: fn.Function{Builder: builders.S2I, Runtime: "node"}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.S2I}, Runtime: "node"}, wantErr: false, }, { name: "Supported runtime - pack builder - with additional Buildpacks", - function: fn.Function{Builder: builders.Pack, Runtime: "node", Buildpacks: testBuildpacks}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.Pack, Buildpacks: testBuildpacks}, Runtime: "node"}, wantErr: true, }, { name: "Unsupported runtime - Go - pack builder - without additional Buildpacks", - function: fn.Function{Builder: builders.Pack, Runtime: "go"}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.Pack}, Runtime: "go"}, wantErr: true, }, { name: "Unsupported runtime - Go - pack builder - with additional Buildpacks", - function: fn.Function{Runtime: "go", Buildpacks: testBuildpacks}, + function: fn.Function{Runtime: "go", Build: fn.BuildSpec{Buildpacks: testBuildpacks}}, wantErr: true, }, { name: "Unsupported runtime - Go - s2i builder", - function: fn.Function{Builder: builders.S2I, Runtime: "go"}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.S2I}, Runtime: "go"}, wantErr: true, }, { name: "Supported runtime - Quarkus - pack builder - without additional Buildpacks", - function: fn.Function{Builder: builders.Pack, Runtime: "quarkus"}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.Pack}, Runtime: "quarkus"}, wantErr: false, }, { name: "Supported runtime - Quarkus - s2i builder", - function: fn.Function{Builder: builders.S2I, Runtime: "quarkus"}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.S2I}, Runtime: "quarkus"}, wantErr: false, }, { name: "Unsupported runtime - Rust - pack builder - without additional Buildpacks", - function: fn.Function{Builder: builders.Pack, Runtime: "rust"}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.Pack}, Runtime: "rust"}, wantErr: true, }, { name: "Unsupported runtime - Rust - s2i builder", - function: fn.Function{Builder: builders.S2I, Runtime: "rust"}, + function: fn.Function{Build: fn.BuildSpec{Builder: builders.S2I}, Runtime: "rust"}, wantErr: true, }, } diff --git a/repository_test.go b/repository_test.go index d5b4fe018..7c7b28881 100644 --- a/repository_test.go +++ b/repository_test.go @@ -95,18 +95,18 @@ func TestRepository_Inheritance(t *testing.T) { } // Assert Template A reflects repo-level settings - if fA.HealthEndpoints.Readiness != "/repoReadiness" { - t.Errorf("Repository-level HealthEndpoint not loaded to template, got %q", fA.HealthEndpoints.Readiness) + if fA.Deploy.HealthEndpoints.Readiness != "/repoReadiness" { + t.Errorf("Repository-level HealthEndpoint not loaded to template, got %q", fA.Deploy.HealthEndpoints.Readiness) } - if diff := cmp.Diff([]string{"repoBuildpack"}, fA.Buildpacks); diff != "" { + if diff := cmp.Diff([]string{"repoBuildpack"}, fA.Build.Buildpacks); diff != "" { t.Errorf("Repository-level Buildpack differs (-want, +got): %s", diff) } // Assert Template B reflects runtime-level settings - if fB.HealthEndpoints.Readiness != "/runtimeReadiness" { - t.Errorf("Runtime-level HealthEndpoint not loaded to template, got %q", fB.HealthEndpoints.Readiness) + if fB.Deploy.HealthEndpoints.Readiness != "/runtimeReadiness" { + t.Errorf("Runtime-level HealthEndpoint not loaded to template, got %q", fB.Deploy.HealthEndpoints.Readiness) } - if diff := cmp.Diff([]string{"runtimeBuildpack"}, fB.Buildpacks); diff != "" { + if diff := cmp.Diff([]string{"runtimeBuildpack"}, fB.Build.Buildpacks); diff != "" { t.Errorf("Runtime-level Buildpack differs (-want, +got): %s", diff) } @@ -119,15 +119,15 @@ func TestRepository_Inheritance(t *testing.T) { }, } - if diff := cmp.Diff(envs, fB.BuildEnvs); diff != "" { + if diff := cmp.Diff(envs, fB.Build.BuildEnvs); diff != "" { t.Fatalf("Unexpected difference between repository's manifest.yaml buildEnvs and function BuildEnvs (-want, +got): %v", diff) } // Assert Template C reflects template-level settings - if fC.HealthEndpoints.Readiness != "/templateReadiness" { - t.Fatalf("Template-level HealthEndpoint not loaded to template, got %q", fC.HealthEndpoints.Readiness) + if fC.Deploy.HealthEndpoints.Readiness != "/templateReadiness" { + t.Fatalf("Template-level HealthEndpoint not loaded to template, got %q", fC.Deploy.HealthEndpoints.Readiness) } - if diff := cmp.Diff([]string{"templateBuildpack"}, fC.Buildpacks); diff != "" { + if diff := cmp.Diff([]string{"templateBuildpack"}, fC.Build.Buildpacks); diff != "" { t.Fatalf("Template-level Buildpack differs (-want, +got): %s", diff) } } diff --git a/s2i/builder.go b/s2i/builder.go index 1e7d6d8e3..aa3fea9ed 100644 --- a/s2i/builder.go +++ b/s2i/builder.go @@ -162,7 +162,7 @@ func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) { // Environment variables // Build Envs have local env var references interpolated then added to the // config as an S2I EnvironmentList struct - buildEnvs, err := fn.Interpolate(f.BuildEnvs) + buildEnvs, err := fn.Interpolate(f.Build.BuildEnvs) if err != nil { return err } diff --git a/s2i/builder_test.go b/s2i/builder_test.go index bcbb20170..95ce28791 100644 --- a/s2i/builder_test.go +++ b/s2i/builder_test.go @@ -125,8 +125,10 @@ func Test_BuilderImageConfigurable(t *testing.T) { s2i.WithName(builders.S2I), s2i.WithImpl(i), s2i.WithDockerClient(c)) f = fn.Function{ // function with a builder image set Runtime: "node", - BuilderImages: map[string]string{ - builders.S2I: "example.com/user/builder-image", + Build: fn.BuildSpec{ + BuilderImages: map[string]string{ + builders.S2I: "example.com/user/builder-image", + }, }, } ) @@ -177,8 +179,10 @@ func Test_BuildEnvs(t *testing.T) { envName = "NAME" envValue = "{{ env:INTERPOLATE_ME }}" f = fn.Function{ - Runtime: "node", - BuildEnvs: []fn.Env{{Name: &envName, Value: &envValue}}, + Runtime: "node", + Build: fn.BuildSpec{ + BuildEnvs: []fn.Env{{Name: &envName, Value: &envValue}}, + }, } i = &mockImpl{} c = mockDocker{} @@ -256,8 +260,10 @@ func TestS2IScriptURL(t *testing.T) { t.Run(tt.name, func(t *testing.T) { f := fn.Function{ Runtime: "node", - BuilderImages: map[string]string{ - builders.S2I: tt.builderImage, + Build: fn.BuildSpec{ + BuilderImages: map[string]string{ + builders.S2I: tt.builderImage, + }, }, } diff --git a/schema/func_yaml-schema.json b/schema/func_yaml-schema.json index 6f92e0c49..c0589720f 100644 --- a/schema/func_yaml-schema.json +++ b/schema/func_yaml-schema.json @@ -2,65 +2,13 @@ "$schema": "http://json-schema.org/draft-04/schema#", "$ref": "#/definitions/Function", "definitions": { - "Env": { + "BuildSpec": { "required": [ - "value" - ], - "properties": { - "name": { - "pattern": "^[-._a-zA-Z][-._a-zA-Z0-9]*$", - "type": "string" - }, - "value": { - "type": "string" - } - }, - "additionalProperties": false, - "type": "object" - }, - "Function": { - "required": [ - "specVersion", - "name", - "namespace", - "runtime", - "registry", - "image", - "imageDigest", "buildpacks", "builder", - "volumes", - "buildEnvs", - "envs", - "annotations", - "options", - "labels", - "healthEndpoints", - "created" + "buildEnvs" ], "properties": { - "specVersion": { - "type": "string" - }, - "name": { - "pattern": "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$", - "type": "string" - }, - "namespace": { - "type": "string" - }, - "runtime": { - "type": "string" - }, - "registry": { - "type": "string" - }, - "image": { - "type": "string" - }, - "imageDigest": { - "type": "string" - }, "git": { "$schema": "http://json-schema.org/draft-04/schema#", "$ref": "#/definitions/Git" @@ -86,25 +34,28 @@ ], "type": "string" }, - "volumes": { - "items": { - "$schema": "http://json-schema.org/draft-04/schema#", - "$ref": "#/definitions/Volume" - }, - "type": "array" - }, "buildEnvs": { "items": { "$schema": "http://json-schema.org/draft-04/schema#", "$ref": "#/definitions/Env" }, "type": "array" - }, - "envs": { - "items": { - "$ref": "#/definitions/Env" - }, - "type": "array" + } + }, + "additionalProperties": false, + "type": "object" + }, + "DeploySpec": { + "required": [ + "namespace", + "annotations", + "options", + "labels", + "healthEndpoints" + ], + "properties": { + "namespace": { + "type": "string" }, "annotations": { "patternProperties": { @@ -128,6 +79,59 @@ "healthEndpoints": { "$schema": "http://json-schema.org/draft-04/schema#", "$ref": "#/definitions/HealthEndpoints" + } + }, + "additionalProperties": false, + "type": "object" + }, + "Env": { + "required": [ + "value" + ], + "properties": { + "name": { + "pattern": "^[-._a-zA-Z][-._a-zA-Z0-9]*$", + "type": "string" + }, + "value": { + "type": "string" + } + }, + "additionalProperties": false, + "type": "object" + }, + "Function": { + "required": [ + "specVersion", + "name", + "runtime", + "registry", + "image", + "imageDigest", + "created", + "build", + "run", + "deploy" + ], + "properties": { + "specVersion": { + "type": "string" + }, + "name": { + "pattern": "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$", + "type": "string" + }, + "runtime": { + "type": "string" + }, + "registry": { + "type": "string" + }, + "image": { + "type": "string" + }, + "imageDigest": { + "type": "string" }, "created": { "type": "string", @@ -136,6 +140,18 @@ "invocation": { "$schema": "http://json-schema.org/draft-04/schema#", "$ref": "#/definitions/Invocation" + }, + "build": { + "$schema": "http://json-schema.org/draft-04/schema#", + "$ref": "#/definitions/BuildSpec" + }, + "run": { + "$schema": "http://json-schema.org/draft-04/schema#", + "$ref": "#/definitions/RunSpec" + }, + "deploy": { + "$schema": "http://json-schema.org/draft-04/schema#", + "$ref": "#/definitions/DeploySpec" } }, "additionalProperties": false, @@ -254,6 +270,29 @@ "additionalProperties": false, "type": "object" }, + "RunSpec": { + "required": [ + "volumes", + "envs" + ], + "properties": { + "volumes": { + "items": { + "$schema": "http://json-schema.org/draft-04/schema#", + "$ref": "#/definitions/Volume" + }, + "type": "array" + }, + "envs": { + "items": { + "$ref": "#/definitions/Env" + }, + "type": "array" + } + }, + "additionalProperties": false, + "type": "object" + }, "ScaleOptions": { "properties": { "min": { diff --git a/template.go b/template.go index d7620a212..1fe100b07 100644 --- a/template.go +++ b/template.go @@ -37,20 +37,20 @@ func (t template) Write(ctx context.Context, f *Function) error { // so it's values are treated as defaults. // TODO: this begs the question: should the Template's manifest.yaml actually // be a partially-populated func.yaml? - if len(f.BuilderImages) == 0 { - f.BuilderImages = t.config.BuilderImages + if len(f.Build.BuilderImages) == 0 { + f.Build.BuilderImages = t.config.BuilderImages } - if len(f.Buildpacks) == 0 { - f.Buildpacks = t.config.Buildpacks + if len(f.Build.Buildpacks) == 0 { + f.Build.Buildpacks = t.config.Buildpacks } - if len(f.BuildEnvs) == 0 { - f.BuildEnvs = t.config.BuildEnvs + if len(f.Build.BuildEnvs) == 0 { + f.Build.BuildEnvs = t.config.BuildEnvs } - if f.HealthEndpoints.Liveness == "" { - f.HealthEndpoints.Liveness = t.config.HealthEndpoints.Liveness + if f.Deploy.HealthEndpoints.Liveness == "" { + f.Deploy.HealthEndpoints.Liveness = t.config.HealthEndpoints.Liveness } - if f.HealthEndpoints.Readiness == "" { - f.HealthEndpoints.Readiness = t.config.HealthEndpoints.Readiness + if f.Deploy.HealthEndpoints.Readiness == "" { + f.Deploy.HealthEndpoints.Readiness = t.config.HealthEndpoints.Readiness } if f.Invocation.Format == "" { f.Invocation.Format = t.config.Invocation.Format diff --git a/templates_test.go b/templates_test.go index 90fe337ca..a687436ed 100644 --- a/templates_test.go +++ b/templates_test.go @@ -407,7 +407,7 @@ func TestTemplates_RuntimeManifestBuildEnvs(t *testing.T) { if err != nil { t.Fatal(err) } - if diff := cmp.Diff(envs, f.BuildEnvs); diff != "" { + if diff := cmp.Diff(envs, f.Build.BuildEnvs); diff != "" { t.Fatalf("Unexpected difference between runtime's manifest.yaml buildEnvs and function BuildEnvs (-want, +got): %v", diff) } } @@ -454,7 +454,7 @@ func TestTemplates_ManifestBuildEnvs(t *testing.T) { if err != nil { t.Fatal(err) } - if diff := cmp.Diff(envs, f.BuildEnvs); diff != "" { + if diff := cmp.Diff(envs, f.Build.BuildEnvs); diff != "" { t.Fatalf("Unexpected difference between template's manifest.yaml buildEnvs and function BuildEnvs (-want, +got): %v", diff) } } @@ -501,7 +501,7 @@ func TestTemplates_RepositoryManifestBuildEnvs(t *testing.T) { if err != nil { t.Fatal(err) } - if diff := cmp.Diff(envs, f.BuildEnvs); diff != "" { + if diff := cmp.Diff(envs, f.Build.BuildEnvs); diff != "" { t.Fatalf("Unexpected difference between repository's manifest.yaml buildEnvs and function BuildEnvs (-want, +got): %v", diff) } } diff --git a/test/_oncluster/git_helper.go b/test/_oncluster/git_helper.go index 13ee769cd..4feae2dad 100644 --- a/test/_oncluster/git_helper.go +++ b/test/_oncluster/git_helper.go @@ -11,7 +11,7 @@ import ( func UpdateFuncGit(t *testing.T, projectDir string, git fn.Git) { f, err := fn.NewFunction(projectDir) AssertNoError(t, err) - f.Git = git + f.Build.Git = git err = f.Write() AssertNoError(t, err) } diff --git a/testdata/migrations/v0.34.0/func.yaml b/testdata/migrations/v0.34.0/func.yaml new file mode 100644 index 000000000..07b8dc436 --- /dev/null +++ b/testdata/migrations/v0.34.0/func.yaml @@ -0,0 +1,27 @@ +specVersion: 0.25.0 +name: testfunc +namespace: "test-namespace" +runtime: go +registry: "" +image: "" +imageDigest: "" +buildpacks: + - paketo-buildpacks/go-dist + - ghcr.io/boson-project/go-function-buildpack:tip +builder: "" +git: + url: "http://test-url" + revision: "test revision" + contextDir: "/test/context/dir" +volumes: [] +buildEnvs: [] +envs: [] +annotations: {} +options: {} +labels: [] +healthEndpoints: + liveness: /health/liveness + readiness: /health/readiness +created: 2022-09-19T09:56:17.933191+01:00 +invocation: + format: http diff --git a/testdata/testFunctionMarshallingError/func.yaml b/testdata/testFunctionMarshallingError/func.yaml new file mode 100644 index 000000000..bfbafb0f7 --- /dev/null +++ b/testdata/testFunctionMarshallingError/func.yaml @@ -0,0 +1 @@ +name: "hel diff --git a/testdata/testFunctionMigrationError/func.yaml b/testdata/testFunctionMigrationError/func.yaml new file mode 100644 index 000000000..d207f333f --- /dev/null +++ b/testdata/testFunctionMigrationError/func.yaml @@ -0,0 +1,5 @@ +name: funcmigration +builder: + - type: grog + +