From e6f31399318f08aca356a4cf74a9a2b3faf00fb2 Mon Sep 17 00:00:00 2001 From: Yael Harel Date: Mon, 25 Jan 2021 16:08:21 -0500 Subject: [PATCH 01/10] Add support for Builpack API 0.6 Signed-off-by: Yael Harel --- api/apis.go | 2 +- lifecycle.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/apis.go b/api/apis.go index 465854eda..02dee991a 100644 --- a/api/apis.go +++ b/api/apis.go @@ -6,7 +6,7 @@ import ( var ( Platform = newApisMustParse([]string{"0.3", "0.4", "0.5", "0.6"}, nil) - Buildpack = newApisMustParse([]string{"0.2", "0.3", "0.4", "0.5"}, nil) + Buildpack = newApisMustParse([]string{"0.2", "0.3", "0.4", "0.5", "0.6"}, nil) ) type APIs struct { diff --git a/lifecycle.toml b/lifecycle.toml index 7e80b91ab..ca7575982 100644 --- a/lifecycle.toml +++ b/lifecycle.toml @@ -1,7 +1,7 @@ [apis] [apis.buildpack] deprecated = [] - supported = ["0.2", "0.3", "0.4", "0.5"] + supported = ["0.2", "0.3", "0.4", "0.5", "0.6"] [apis.platform] deprecated = [] supported = ["0.3", "0.4", "0.5", "0.6"] From 196bc36e53e1b30aecab696c82160a2098473a03 Mon Sep 17 00:00:00 2001 From: Yael Harel Date: Fri, 29 Jan 2021 16:47:39 -0500 Subject: [PATCH 02/10] Add support of default process type to buildpacks Signed-off-by: Daniel Thornton Signed-off-by: Yael Harel --- builder.go | 80 +++++-- builder_test.go | 168 ++++++++++++- buildpacktoml.go | 37 ++- buildpacktoml_test.go | 224 +++++++++++++++-- exporter.go | 20 +- exporter_test.go | 225 ++++++++++-------- launch/launch.go | 13 + .../layers/config/metadata.toml | 21 ++ .../layers/config/metadata.toml | 6 + 9 files changed, 637 insertions(+), 157 deletions(-) create mode 100644 testdata/exporter/default-process/multiple-processes-with-default/layers/config/metadata.toml create mode 100644 testdata/exporter/default-process/one-non-default-process/layers/config/metadata.toml diff --git a/builder.go b/builder.go index 3b48574cd..9592b027d 100644 --- a/builder.go +++ b/builder.go @@ -1,10 +1,11 @@ package lifecycle import ( + "container/list" "fmt" "io" "path/filepath" - "sort" + "strings" "github.com/buildpacks/lifecycle/api" "github.com/buildpacks/lifecycle/env" @@ -31,6 +32,7 @@ type BuildConfig struct { Env BuildEnv AppDir string PlatformDir string + PlatformAPI string LayersDir string Out io.Writer Err io.Writer @@ -76,7 +78,7 @@ func (b *Builder) Build() (*BuildMetadata, error) { return nil, err } - procMap := processMap{} + procStruct := newProcessStruct() plan := b.Plan var bom []BOMEntry var slices []layers.Slice @@ -97,7 +99,17 @@ func (b *Builder) Build() (*BuildMetadata, error) { bom = append(bom, br.BOM...) labels = append(labels, br.Labels...) plan = plan.filter(br.MetRequires) - procMap.add(br.Processes) + replacedDefaults, err := procStruct.add(br.Processes) + if err != nil { + return nil, err + } + + if len(replacedDefaults) > 0 { + warning := fmt.Sprintf("Warning: redefining the following default process types with processes not marked as default: [%s]", strings.Join(replacedDefaults, ", ")) + if _, err := b.Out.Write([]byte(warning)); err != nil { + return nil, err + } + } slices = append(slices, br.Slices...) } @@ -106,12 +118,16 @@ func (b *Builder) Build() (*BuildMetadata, error) { bom[i].convertMetadataToVersion() } } + procList, err := procStruct.list() + if err != nil { + return nil, err + } return &BuildMetadata{ BOM: bom, Buildpacks: b.Group.Group, Labels: labels, - Processes: procMap.list(), + Processes: procList, Slices: slices, }, nil } @@ -134,6 +150,7 @@ func (b *Builder) BuildConfig() (BuildConfig, error) { Env: b.Env, AppDir: appDir, PlatformDir: platformDir, + PlatformAPI: b.PlatformAPI.String(), LayersDir: layersDir, Out: b.Out, Err: b.Err, @@ -175,25 +192,54 @@ func containsEntry(metRequires []string, entry BuildPlanEntry) bool { return false } -type processMap map[string]launch.Process +// orderedProcesses is a mapping from process types to Processes, it will preserve ordering. +// processList is the ordered list +// wwe keep typeToPtr map in order to delete elements when others are overriding them +type orderedProcesses struct { + typeToProcess map[string]*list.Element + processList *list.List +} -func (m processMap) add(l []launch.Process) { - for _, proc := range l { - m[proc.Type] = proc +func newProcessStruct() orderedProcesses { + return orderedProcesses{ + typeToProcess: make(map[string]*list.Element), + processList: list.New(), } } -func (m processMap) list() []launch.Process { - var keys []string - for key := range m { - keys = append(keys, key) +func (m orderedProcesses) add(listToAdd []launch.Process) ([]string, error) { + result := []string{} + for _, proc := range listToAdd { + // when we replace a default process type with a non-default process type, add to result + if p, ok := m.typeToProcess[proc.Type]; ok { + cast, success := (p.Value).(launch.Process) + if !success { + return []string{}, fmt.Errorf("can't cast an element from the list to a process") + } + if cast.Default && !proc.Default { + result = append(result, proc.Type) + } + m.processList.Remove(p) + } + m.processList.PushBack(proc) + m.typeToProcess[proc.Type] = m.processList.Back() } - sort.Strings(keys) - procs := []launch.Process{} - for _, key := range keys { - procs = append(procs, m[key]) + + return result, nil +} + +// list returns an ordered array of process types. The ordering is based on the +// order that the processes were added to this struct. +func (m orderedProcesses) list() ([]launch.Process, error) { + result := []launch.Process{} + for e := m.processList.Front(); e != nil; e = e.Next() { + cast, success := (e.Value).(launch.Process) + if !success { + return []launch.Process{}, fmt.Errorf("can't cast an element from the list to a process") + } + result = append(result, cast) } - return procs + return result, nil } func (bom *BOMEntry) convertMetadataToVersion() { diff --git a/builder_test.go b/builder_test.go index f2cf5d409..b6a394238 100644 --- a/builder_test.go +++ b/builder_test.go @@ -310,6 +310,20 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { t.Fatalf("Unexpected error:\n%s\n", err) } if s := cmp.Diff(metadata.Processes, []launch.Process{ + { + Type: "some-type", + Command: "some-command", + Args: []string{"some-arg"}, + Direct: true, + BuildpackID: "A", + }, + { + Type: "some-other-type", + Command: "some-other-command", + Args: []string{"some-other-arg"}, + Direct: true, + BuildpackID: "B", + }, { Type: "override-type", Command: "bpB-command", @@ -317,19 +331,166 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { Direct: false, BuildpackID: "B", }, + }); s != "" { + t.Fatalf("Unexpected:\n%s\n", s) + } + }) + + it("should warn when overriding default process type, with non-default process type", func() { + bpA := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) + bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "override-type", + Command: "bpA-command", + Args: []string{"bpA-arg"}, + Direct: true, + BuildpackID: "A", + Default: true, + }, + { + Type: "second-override-type", + Command: "some-command", + Args: []string{"some-arg"}, + Direct: true, + BuildpackID: "A", + Default: true, + }, + }, + }, nil) + bpB := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) + bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "override-type", + Command: "bpB-command", + Args: []string{"bpB-arg"}, + Direct: false, + BuildpackID: "B", + }, + { + Type: "second-override-type", + Command: "some-other-command", + Args: []string{"some-other-arg"}, + Direct: true, + BuildpackID: "B", + Default: true, + }, + }, + }, nil) + + metadata, err := builder.Build() + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + if s := cmp.Diff(metadata.Processes, []launch.Process{ { - Type: "some-other-type", + Type: "override-type", + Command: "bpB-command", + Args: []string{"bpB-arg"}, + Direct: false, + BuildpackID: "B", + Default: false, + }, + { + Type: "second-override-type", Command: "some-other-command", Args: []string{"some-other-arg"}, Direct: true, BuildpackID: "B", + Default: true, + }, + }); s != "" { + t.Fatalf("Unexpected:\n%s\n", s) + } + + expected := "Warning: redefining the following default process types with processes not marked as default: [override-type]" + h.AssertStringContains(t, stdout.String(), expected) + }) + + it("should preserve ordering of processes", func() { + bpA := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) + bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "first", + Command: "first-command", + Args: []string{"bpA-arg"}, + Direct: true, + BuildpackID: "A", + Default: true, + }, + { + Type: "second", + Command: "second-command", + Args: []string{"some-arg"}, + Direct: true, + BuildpackID: "A", + Default: true, + }, + }, + }, nil) + bpB := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) + bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "third", + Command: "third-command", + Args: []string{"bpB-arg"}, + Direct: false, + BuildpackID: "B", + }, + { + Type: "fourth", + Command: "fourth-command", + Args: []string{"some-other-arg"}, + Direct: true, + BuildpackID: "B", + Default: true, + }, }, + }, nil) + + metadata, err := builder.Build() + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + + if s := cmp.Diff(metadata.Processes, []launch.Process{ { - Type: "some-type", - Command: "some-command", + Type: "first", + Command: "first-command", + Args: []string{"bpA-arg"}, + Direct: true, + BuildpackID: "A", + Default: true, + }, + { + Type: "second", + Command: "second-command", Args: []string{"some-arg"}, Direct: true, BuildpackID: "A", + Default: true, + }, + { + Type: "third", + Command: "third-command", + Args: []string{"bpB-arg"}, + Direct: false, + BuildpackID: "B", + }, + { + Type: "fourth", + Command: "fourth-command", + Args: []string{"some-other-arg"}, + Direct: true, + BuildpackID: "B", + Default: true, }, }); s != "" { t.Fatalf("Unexpected:\n%s\n", s) @@ -411,6 +572,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("platform api < 0.4", func() { it.Before(func() { builder.PlatformAPI = api.MustParse("0.3") + config.PlatformAPI = "0.3" }) when("build metadata", func() { diff --git a/buildpacktoml.go b/buildpacktoml.go index 7a25654df..b665a4e4a 100644 --- a/buildpacktoml.go +++ b/buildpacktoml.go @@ -84,7 +84,7 @@ func (b *BuildpackTOML) Build(bpPlan BuildpackPlan, config BuildConfig) (BuildRe return BuildResult{}, err } - return b.readOutputFiles(bpLayersDir, bpPlanPath, bpPlan) + return b.readOutputFiles(bpLayersDir, bpPlanPath, bpPlan, config.PlatformAPI) } func preparePaths(bpID string, bpPlan BuildpackPlan, layersDir, planDir string) (string, string, error) { @@ -181,7 +181,7 @@ func isBuild(path string) bool { return err == nil && layerTOML.Build } -func (b *BuildpackTOML) readOutputFiles(bpLayersDir, bpPlanPath string, bpPlanIn BuildpackPlan) (BuildResult, error) { +func (b *BuildpackTOML) readOutputFiles(bpLayersDir, bpPlanPath string, bpPlanIn BuildpackPlan, platformAPI string) (BuildResult, error) { br := BuildResult{} bpFromBpInfo := GroupBuildpack{ID: b.Buildpack.ID, Version: b.Buildpack.Version} @@ -243,6 +243,12 @@ func (b *BuildpackTOML) readOutputFiles(bpLayersDir, bpPlanPath string, bpPlanIn br.BOM = withBuildpack(bpFromBpInfo, launchTOML.BOM) } + if err := validateProcesses(launchTOML.Processes); err != nil { + return BuildResult{}, err + } + + updateDefaultProcesses(launchTOML.Processes, b.API, platformAPI) + // set data from launch.toml br.Labels = append([]Label{}, launchTOML.Labels...) for i := range launchTOML.Processes { @@ -254,6 +260,33 @@ func (b *BuildpackTOML) readOutputFiles(bpLayersDir, bpPlanPath string, bpPlanIn return br, nil } +// we set default = true for web processes when platformAPI >= 0.6 and buildpackAPI < 0.6 +func updateDefaultProcesses(processes []launch.Process, bpAPI string, platformAPI string) { + if api.MustParse(platformAPI).Compare(api.MustParse("0.6")) < 0 || api.MustParse(bpAPI).Compare(api.MustParse("0.6")) >= 0 { + return + } + + for i := range processes { + if processes[i].Type == "web" { + processes[i].Default = true + } + } +} + +// check that there are not multiple default processes in a buildpack with different process types +func validateProcesses(processes []launch.Process) error { + defaultType := "" + for _, process := range processes { + if process.Default && defaultType != "" && defaultType != process.Type { + return fmt.Errorf("multiple default process types aren't allowed") + } + if process.Default { + defaultType = process.Type + } + } + return nil +} + func validateBOM(bom []BOMEntry, bpAPI string) error { if api.MustParse(bpAPI).Compare(api.MustParse("0.5")) < 0 { for _, entry := range bom { diff --git a/buildpacktoml_test.go b/buildpacktoml_test.go index 39adcb7ae..4e360008c 100644 --- a/buildpacktoml_test.go +++ b/buildpacktoml_test.go @@ -27,6 +27,7 @@ import ( ) var latestBuildpackAPI = api.Buildpack.Latest() +var latestPlatformAPI = api.Platform.Latest() func TestBuildpackTOML(t *testing.T) { spec.Run(t, "BuildpackTOML", testBuildpackTOML, spec.Report(report.Terminal{})) @@ -72,6 +73,7 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { Env: mockEnv, AppDir: appDir, PlatformDir: platformDir, + PlatformAPI: latestPlatformAPI.String(), LayersDir: layersDir, Out: stdout, Err: stderr, @@ -286,32 +288,107 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { } }) - it("should include processes", func() { - h.Mkfile(t, - `[[processes]]`+"\n"+ - `type = "some-type"`+"\n"+ - `command = "some-cmd"`+"\n"+ + when("processes", func() { + when("platform API < 0.6", func() { + it.Before(func() { + config.PlatformAPI = "0.5" + }) + it("should include processes and use the default value that is set", func() { + h.Mkfile(t, + `[[processes]]`+"\n"+ + `type = "some-type"`+"\n"+ + `command = "some-cmd"`+"\n"+ + `default = true`+"\n"+ + `[[processes]]`+"\n"+ + `type = "web"`+"\n"+ + `command = "other-cmd"`+"\n", + // default is false and therefore doesn't appear + filepath.Join(appDir, "launch-A-v1.toml"), + ) + br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + if s := cmp.Diff(br, lifecycle.BuildResult{ + BOM: nil, + Labels: []lifecycle.Label{}, + MetRequires: nil, + Processes: []launch.Process{ + {Type: "some-type", Command: "some-cmd", BuildpackID: "A", Default: true}, + {Type: "web", Command: "other-cmd", BuildpackID: "A", Default: false}, + }, + Slices: []layers.Slice{}, + }); s != "" { + t.Fatalf("Unexpected metadata:\n%s\n", s) + } + }) + }) + + it("should include processes and use the default value that is set", func() { + h.Mkfile(t, `[[processes]]`+"\n"+ - `type = "other-type"`+"\n"+ - `command = "other-cmd"`+"\n", - filepath.Join(appDir, "launch-A-v1.toml"), - ) - br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) - if err != nil { - t.Fatalf("Unexpected error:\n%s\n", err) - } - if s := cmp.Diff(br, lifecycle.BuildResult{ - BOM: nil, - Labels: []lifecycle.Label{}, - MetRequires: nil, - Processes: []launch.Process{ - {Type: "some-type", Command: "some-cmd", BuildpackID: "A"}, - {Type: "other-type", Command: "other-cmd", BuildpackID: "A"}, - }, - Slices: []layers.Slice{}, - }); s != "" { - t.Fatalf("Unexpected metadata:\n%s\n", s) - } + `type = "some-type"`+"\n"+ + `command = "some-cmd"`+"\n"+ + `default = true`+"\n"+ + `[[processes]]`+"\n"+ + `type = "web"`+"\n"+ + `command = "other-cmd"`+"\n", + // default is false and therefore doesn't appear + filepath.Join(appDir, "launch-A-v1.toml"), + ) + br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + if s := cmp.Diff(br, lifecycle.BuildResult{ + BOM: nil, + Labels: []lifecycle.Label{}, + MetRequires: nil, + Processes: []launch.Process{ + {Type: "some-type", Command: "some-cmd", BuildpackID: "A", Default: true}, + {Type: "web", Command: "other-cmd", BuildpackID: "A", Default: false}, + }, + Slices: []layers.Slice{}, + }); s != "" { + t.Fatalf("Unexpected metadata:\n%s\n", s) + } + }) + when("when there are multiple default=true processes but they are all of the same type", func() { + it("should succeed", func() { + h.Mkfile(t, + `[[processes]]`+"\n"+ + `type = "some-type"`+"\n"+ + `command = "some-cmd"`+"\n"+ + `default = true`+"\n"+ + `[[processes]]`+"\n"+ + `type = "other-type"`+"\n"+ + `command = "other-cmd"`+"\n"+ + `[[processes]]`+"\n"+ + `type = "some-type"`+"\n"+ + `command = "some-other-cmd"`+"\n"+ + `default = true`+"\n", + // default is false and therefore doesn't appear + filepath.Join(appDir, "launch-A-v1.toml"), + ) + br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + if s := cmp.Diff(br, lifecycle.BuildResult{ + BOM: nil, + Labels: []lifecycle.Label{}, + MetRequires: nil, + Processes: []launch.Process{ + {Type: "some-type", Command: "some-cmd", BuildpackID: "A", Default: true}, + {Type: "other-type", Command: "other-cmd", BuildpackID: "A", Default: false}, + {Type: "some-type", Command: "some-other-cmd", BuildpackID: "A", Default: true}, + }, + Slices: []layers.Slice{}, + }); s != "" { + t.Fatalf("Unexpected metadata:\n%s\n", s) + } + }) + }) }) it("should include slices", func() { @@ -523,6 +600,31 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { }) }) }) + + when("when there is more than one default=true process", func() { + it.Before(func() { + mockEnv.EXPECT().WithPlatform(platformDir).Return(append(os.Environ(), "TEST_ENV=Av1"), nil) + }) + + it("should error", func() { + h.Mkfile(t, + `[[processes]]`+"\n"+ + `type = "some-type"`+"\n"+ + `command = "some-cmd"`+"\n"+ + `default = true`+"\n"+ + `[[processes]]`+"\n"+ + `type = "other-type"`+"\n"+ + `command = "other-cmd"`+"\n"+ + `default = true`+"\n", + // default is false and therefore doesn't appear + filepath.Join(appDir, "launch-A-v1.toml"), + ) + _, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) + h.AssertNotNil(t, err) + expected := "multiple default process types aren't allowed" + h.AssertStringContains(t, err.Error(), expected) + }) + }) }) when("buildpack api = 0.2", func() { @@ -767,6 +869,78 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { }) }) }) + + when("buildpack api < 0.6", func() { + it.Before(func() { + bpTOML.API = "0.5" + mockEnv.EXPECT().WithPlatform(platformDir).Return(append(os.Environ(), "TEST_ENV=Av1"), nil) + }) + + when("platform API < 0.6", func() { + it.Before(func() { + config.PlatformAPI = "0.5" + }) + it("should include processes and set their default value to false", func() { + h.Mkfile(t, + `[[processes]]`+"\n"+ + `type = "some-type"`+"\n"+ + `command = "some-cmd"`+"\n"+ + `[[processes]]`+"\n"+ + `type = "web"`+"\n"+ + `command = "other-cmd"`+"\n", + // default is false and therefore doesn't appear + filepath.Join(appDir, "launch-A-v1.toml"), + ) + br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + if s := cmp.Diff(br, lifecycle.BuildResult{ + BOM: nil, + Labels: []lifecycle.Label{}, + MetRequires: nil, + Processes: []launch.Process{ + {Type: "some-type", Command: "some-cmd", BuildpackID: "A", Default: false}, + {Type: "web", Command: "other-cmd", BuildpackID: "A", Default: false}, + }, + Slices: []layers.Slice{}, + }); s != "" { + t.Fatalf("Unexpected metadata:\n%s\n", s) + } + }) + }) + + when("platform API >= 0.6", func() { + it("should include processes, all default values should be false, except for the web processes types", func() { + h.Mkfile(t, + `[[processes]]`+"\n"+ + `type = "some-type"`+"\n"+ + `command = "some-cmd"`+"\n"+ + `[[processes]]`+"\n"+ + `type = "web"`+"\n"+ + `command = "other-cmd"`+"\n", + // default is false and therefore doesn't appear + filepath.Join(appDir, "launch-A-v1.toml"), + ) + br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + if s := cmp.Diff(br, lifecycle.BuildResult{ + BOM: nil, + Labels: []lifecycle.Label{}, + MetRequires: nil, + Processes: []launch.Process{ + {Type: "some-type", Command: "some-cmd", BuildpackID: "A", Default: false}, + {Type: "web", Command: "other-cmd", BuildpackID: "A", Default: true}, + }, + Slices: []layers.Slice{}, + }); s != "" { + t.Fatalf("Unexpected metadata:\n%s\n", s) + } + }) + }) + }) }) } diff --git a/exporter.go b/exporter.go index 285df0b94..689771671 100644 --- a/exporter.go +++ b/exporter.go @@ -383,16 +383,22 @@ func (e *Exporter) entrypoint(launchMD launch.Metadata, defaultProcessType strin if !e.supportsMulticallLauncher() { return launch.LauncherPath, nil } - if defaultProcessType == "" { - if len(launchMD.Processes) == 1 { - e.Logger.Infof("Setting default process type '%s'", launchMD.Processes[0].Type) - return launch.ProcessPath(launchMD.Processes[0].Type), nil + + if defaultProcessType != "" { + defaultProcess, ok := launchMD.FindProcessType(defaultProcessType) + if !ok { + if e.PlatformAPI.Compare(api.MustParse("0.6")) < 0 { + e.Logger.Warn(processTypeWarning(launchMD, defaultProcessType)) + return launch.LauncherPath, nil + } + return "", fmt.Errorf("tried to set %s to default but it doesn't exist", defaultProcessType) } - return launch.LauncherPath, nil + e.Logger.Infof("Setting default process type '%s'", defaultProcess.Type) + return launch.ProcessPath(defaultProcess.Type), nil } - defaultProcess, ok := launchMD.FindProcessType(defaultProcessType) + defaultProcess, ok := launchMD.FindLastDefaultProcessType() if !ok { - e.Logger.Warn(processTypeWarning(launchMD, defaultProcessType)) + e.Logger.Info("no default process type") return launch.LauncherPath, nil } e.Logger.Infof("Setting default process type '%s'", defaultProcess.Type) diff --git a/exporter_test.go b/exporter_test.go index 205146126..40148c24f 100644 --- a/exporter_test.go +++ b/exporter_test.go @@ -484,7 +484,8 @@ version = "4.5.6" "direct": true, "command": "/some/command", "args": ["some", "command", "args"], - "buildpackID": "buildpack.id" + "buildpackID": "buildpack.id", + "default": false } ] } @@ -918,156 +919,163 @@ version = "4.5.6" h.AssertEq(t, val, opts.AppDir) }) - when("default process type is set", func() { - it.Before(func() { - opts.DefaultProcessType = "some-process-type" - }) + it("sets empty CMD", func() { + _, err := exporter.Export(opts) + h.AssertNil(t, err) + + val, err := fakeAppImage.Cmd() + h.AssertNil(t, err) + h.AssertEq(t, val, []string(nil)) + }) + + it("saves the image for all provided AdditionalNames", func() { + _, err := exporter.Export(opts) + h.AssertNil(t, err) + h.AssertContains(t, fakeAppImage.SavedNames(), append(opts.AdditionalNames, fakeAppImage.Name())...) + }) + }) - when("platform API is < 0.4", func() { + when("default process", func() { + when("-process-type is set", func() { + when("it is set to an existing type", func() { it.Before(func() { - exporter.PlatformAPI = api.MustParse("0.3") + opts.DefaultProcessType = "some-process-type" + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "one-non-default-process", "layers"), opts.LayersDir) }) - it("sets CNB_PROCESS_TYPE", func() { + it("sets the ENTRYPOINT to this process type", func() { _, err := exporter.Export(opts) h.AssertNil(t, err) - val, err := fakeAppImage.Env("CNB_PROCESS_TYPE") - h.AssertNil(t, err) - h.AssertEq(t, val, "some-process-type") + checkEntrypoint(t, fakeAppImage, "process", "some-process-type") }) - it("sets ENTRYPOINT to launcher", func() { + it("doesn't set CNB_PROCESS_TYPE", func() { _, err := exporter.Export(opts) h.AssertNil(t, err) - ep, err := fakeAppImage.Entrypoint() + val, err := fakeAppImage.Env("CNB_PROCESS_TYPE") h.AssertNil(t, err) - h.AssertEq(t, len(ep), 1) - if runtime.GOOS == "windows" { - h.AssertEq(t, ep[0], `c:\cnb\lifecycle\launcher.exe`) - } else { - h.AssertEq(t, ep[0], `/cnb/lifecycle/launcher`) - } + h.AssertEq(t, val, "") }) + }) - when("default process type is not in metadata.toml", func() { - it("returns an error", func() { - opts.DefaultProcessType = "some-missing-process" - _, err := exporter.Export(opts) - h.AssertError(t, err, "default process type 'some-missing-process' not present in list [some-process-type]") - }) + when("it is set to a process type that doesn't exist", func() { + it.Before(func() { + opts.DefaultProcessType = "some-non-existing-process-type" + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "one-non-default-process", "layers"), opts.LayersDir) + }) + it("fails", func() { + _, err := exporter.Export(opts) + h.AssertError(t, err, "tried to set some-non-existing-process-type to default but it doesn't exist") }) }) + }) - when("platform API is >= 0.4", func() { - it("sets the ENTRYPOINT to the default process", func() { - opts.DefaultProcessType = "some-process-type" + when("-process-type is not set", func() { + when("there are no default=true processes in metadata.toml", func() { + it.Before(func() { + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "one-non-default-process", "layers"), opts.LayersDir) + }) + it("send an info message that there is no default process, and sets the ENTRYPOINT to the launcher", func() { _, err := exporter.Export(opts) h.AssertNil(t, err) - - ep, err := fakeAppImage.Entrypoint() - h.AssertNil(t, err) - h.AssertEq(t, len(ep), 1) - if runtime.GOOS == "windows" { - h.AssertEq(t, ep[0], `c:\cnb\process\some-process-type.exe`) - } else { - h.AssertEq(t, ep[0], `/cnb/process/some-process-type`) - } + assertLogEntry(t, logHandler, "no default process type") + checkEntrypoint(t, fakeAppImage, "lifecycle", "launcher") }) + }) - it("does not set CNB_PROCESS_TYPE", func() { + when("there is a default=true process in metadata.toml", func() { + it.Before(func() { + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "multiple-processes-with-default", "layers"), opts.LayersDir) + layerFactory.EXPECT(). + ProcessTypesLayer(launch.Metadata{ + Processes: []launch.Process{ + { + Type: "some-default-process-type", + Command: "/some/command", + Args: []string{"some", "command", "args"}, + Direct: true, + BuildpackID: "buildpack1.id", + Default: true, + }, + { + Type: "some-second-default-process-type", + Command: "/some/command", + Args: []string{"some", "command", "args"}, + Direct: true, + BuildpackID: "buildpack2.id", + Default: true, + }, + { + Type: "some-process-type", + Command: "/some/command", + Args: []string{"some", "command", "args"}, + Direct: true, + BuildpackID: "buildpack3.id", + Default: false, + }}, + }). + DoAndReturn(func(_ launch.Metadata) (layers.Layer, error) { + return createTestLayer("process-types", tmpDir) + }). + AnyTimes() + }) + it("sets the last default=true process to be the default", func() { _, err := exporter.Export(opts) h.AssertNil(t, err) - - val, err := fakeAppImage.Env("CNB_PROCESS_TYPE") - h.AssertNil(t, err) - h.AssertEq(t, val, "") - }) - - when("default process type is not in metadata.toml", func() { - it("warns and sets the ENTRYPOINT to launcher", func() { - opts.DefaultProcessType = "some-missing-process" - _, err := exporter.Export(opts) - h.AssertNil(t, err) - - assertLogEntry(t, logHandler, "default process type 'some-missing-process' not present in list [some-process-type]") - ep, err := fakeAppImage.Entrypoint() - h.AssertNil(t, err) - h.AssertEq(t, len(ep), 1) - if runtime.GOOS == "windows" { - h.AssertEq(t, ep[0], `c:\cnb\lifecycle\launcher.exe`) - } else { - h.AssertEq(t, ep[0], `/cnb/lifecycle/launcher`) - } - }) + checkEntrypoint(t, fakeAppImage, "process", "some-second-default-process-type") }) }) }) - when("default process type is empty", func() { - when("platform API is >= 0.4", func() { - when("there is exactly one process", func() { - it("sets the ENTRYPOINT to the only process", func() { - _, err := exporter.Export(opts) - h.AssertNil(t, err) - - ep, err := fakeAppImage.Entrypoint() - h.AssertNil(t, err) - h.AssertEq(t, len(ep), 1) - if runtime.GOOS == "windows" { - h.AssertEq(t, ep[0], `c:\cnb\process\some-process-type.exe`) - } else { - h.AssertEq(t, ep[0], `/cnb/process/some-process-type`) - } - }) + when("platform API < 0.6", func() { + when("-process-type is set to a process type that doesn't exist", func() { + it.Before(func() { + exporter.PlatformAPI = api.MustParse("0.5") + opts.DefaultProcessType = "some-non-existing-process-type" + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "one-non-default-process", "layers"), opts.LayersDir) + }) + it("warns the process type doesn't exist, and sets the ENTRYPOINT to the launcher", func() { + _, err := exporter.Export(opts) + h.AssertNil(t, err) + assertLogEntry(t, logHandler, "default process type 'some-non-existing-process-type' not present in list [some-process-type]") + checkEntrypoint(t, fakeAppImage, "lifecycle", "launcher") }) }) + }) - when("platform API is < 0.4", func() { + when("platform API < 0.4", func() { + it.Before(func() { + exporter.PlatformAPI = api.MustParse("0.3") + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "one-non-default-process", "layers"), opts.LayersDir) + }) + when("-process-type is set to an existing process type", func() { it.Before(func() { - exporter.PlatformAPI = api.MustParse("0.3") + opts.DefaultProcessType = "some-process-type" }) - it("does not set CNB_PROCESS_TYPE", func() { + it("sets CNB_PROCESS_TYPE", func() { _, err := exporter.Export(opts) h.AssertNil(t, err) val, err := fakeAppImage.Env("CNB_PROCESS_TYPE") h.AssertNil(t, err) - h.AssertEq(t, val, "") + h.AssertEq(t, val, "some-process-type") }) + }) - it("sets ENTRYPOINT to launcher", func() { + when("-process-type is not set", func() { + it("doesn't set CNB_PROCESS_TYPE", func() { _, err := exporter.Export(opts) h.AssertNil(t, err) - ep, err := fakeAppImage.Entrypoint() + val, err := fakeAppImage.Env("CNB_PROCESS_TYPE") h.AssertNil(t, err) - h.AssertEq(t, len(ep), 1) - if runtime.GOOS == "windows" { - h.AssertEq(t, ep[0], `c:\cnb\lifecycle\launcher.exe`) - } else { - h.AssertEq(t, ep[0], `/cnb/lifecycle/launcher`) - } + h.AssertEq(t, val, "") }) }) }) - - it("sets empty CMD", func() { - _, err := exporter.Export(opts) - h.AssertNil(t, err) - - val, err := fakeAppImage.Cmd() - h.AssertNil(t, err) - h.AssertEq(t, val, []string(nil)) - }) - - it("saves the image for all provided AdditionalNames", func() { - _, err := exporter.Export(opts) - h.AssertNil(t, err) - h.AssertContains(t, fakeAppImage.SavedNames(), append(opts.AdditionalNames, fakeAppImage.Name())...) - }) }) when("report.toml", func() { @@ -1307,6 +1315,17 @@ version = "4.5.6" }) } +func checkEntrypoint(t *testing.T, image *fakes.Image, expectedDir string, expectedFile string) { + ep, err := image.Entrypoint() + h.AssertNil(t, err) + h.AssertEq(t, len(ep), 1) + if runtime.GOOS == "windows" { + h.AssertEq(t, ep[0], fmt.Sprintf(`c:\cnb\%s\%s.exe`, expectedDir, expectedFile)) + } else { + h.AssertEq(t, ep[0], fmt.Sprintf(`/cnb/%s/%s`, expectedDir, expectedFile)) + } +} + func createTestLayer(id string, tmpDir string) (layers.Layer, error) { tarPath := filepath.Join(tmpDir, "artifacts", strings.Replace(id, "/", "_", -1)) f, err := os.Create(tarPath) diff --git a/launch/launch.go b/launch/launch.go index dacff7c6e..868bac619 100644 --- a/launch/launch.go +++ b/launch/launch.go @@ -11,6 +11,7 @@ type Process struct { Command string `toml:"command" json:"command"` Args []string `toml:"args" json:"args"` Direct bool `toml:"direct" json:"direct"` + Default bool `toml:"default, omitzero" json:"default"` BuildpackID string `toml:"buildpack-id" json:"buildpackID"` } @@ -33,6 +34,18 @@ func (m Metadata) FindProcessType(pType string) (Process, bool) { return Process{}, false } +func (m Metadata) FindLastDefaultProcessType() (Process, bool) { + defaultFound := false + var lastDefaultProcess Process + for _, p := range m.Processes { + if p.Default { + lastDefaultProcess = p + defaultFound = true + } + } + return lastDefaultProcess, defaultFound +} + type Buildpack struct { API string `toml:"api"` ID string `toml:"id"` diff --git a/testdata/exporter/default-process/multiple-processes-with-default/layers/config/metadata.toml b/testdata/exporter/default-process/multiple-processes-with-default/layers/config/metadata.toml new file mode 100644 index 000000000..9928a8d12 --- /dev/null +++ b/testdata/exporter/default-process/multiple-processes-with-default/layers/config/metadata.toml @@ -0,0 +1,21 @@ +[[processes]] + type = "some-default-process-type" + direct = true + command = "/some/command" + args = ["some", "command", "args"] + buildpack-id = "buildpack1.id" + default = true +[[processes]] + type = "some-second-default-process-type" + direct = true + command = "/some/command" + args = ["some", "command", "args"] + buildpack-id = "buildpack2.id" + default = true +[[processes]] + type = "some-process-type" + direct = true + command = "/some/command" + args = ["some", "command", "args"] + buildpack-id = "buildpack3.id" + diff --git a/testdata/exporter/default-process/one-non-default-process/layers/config/metadata.toml b/testdata/exporter/default-process/one-non-default-process/layers/config/metadata.toml new file mode 100644 index 000000000..61ec8b8fd --- /dev/null +++ b/testdata/exporter/default-process/one-non-default-process/layers/config/metadata.toml @@ -0,0 +1,6 @@ +[[processes]] + type = "some-process-type" + direct = true + command = "/some/command" + args = ["some", "command", "args"] + buildpack-id = "buildpack.id" From f8904081277d99c18ebde5a78eef54ba512c5396 Mon Sep 17 00:00:00 2001 From: Yael Harel Date: Mon, 1 Feb 2021 23:21:28 -0500 Subject: [PATCH 03/10] Move some logic that relates to platform API from buildpacktoml to builder Signed-off-by: Yael Harel --- builder.go | 18 +++++- builder_test.go | 140 ++++++++++++++++++++++++++++++++++-------- buildpacktoml.go | 23 ++----- buildpacktoml_test.go | 125 ++++++++----------------------------- testmock/buildpack.go | 8 ++- 5 files changed, 169 insertions(+), 145 deletions(-) diff --git a/builder.go b/builder.go index 9592b027d..8e4abca92 100644 --- a/builder.go +++ b/builder.go @@ -26,13 +26,13 @@ type BuildpackStore interface { type Buildpack interface { Build(bpPlan BuildpackPlan, config BuildConfig) (BuildResult, error) + BuilpackAPI() string } type BuildConfig struct { Env BuildEnv AppDir string PlatformDir string - PlatformAPI string LayersDir string Out io.Writer Err io.Writer @@ -96,6 +96,8 @@ func (b *Builder) Build() (*BuildMetadata, error) { return nil, err } + updateDefaultProcesses(br.Processes, api.MustParse(bpTOML.BuilpackAPI()), b.PlatformAPI) + bom = append(bom, br.BOM...) labels = append(labels, br.Labels...) plan = plan.filter(br.MetRequires) @@ -132,6 +134,19 @@ func (b *Builder) Build() (*BuildMetadata, error) { }, nil } +// we set default = true for web processes when platformAPI >= 0.6 and buildpackAPI < 0.6 +func updateDefaultProcesses(processes []launch.Process, buildpackAPI *api.Version, platformAPI *api.Version) { + if platformAPI.Compare(api.MustParse("0.6")) < 0 || buildpackAPI.Compare(api.MustParse("0.6")) >= 0 { + return + } + + for i := range processes { + if processes[i].Type == "web" { + processes[i].Default = true + } + } +} + func (b *Builder) BuildConfig() (BuildConfig, error) { appDir, err := filepath.Abs(b.AppDir) if err != nil { @@ -150,7 +165,6 @@ func (b *Builder) BuildConfig() (BuildConfig, error) { Env: b.Env, AppDir: appDir, PlatformDir: platformDir, - PlatformAPI: b.PlatformAPI.String(), LayersDir: layersDir, Out: b.Out, Err: b.Err, diff --git a/builder_test.go b/builder_test.go index b6a394238..088b87fcd 100644 --- a/builder_test.go +++ b/builder_test.go @@ -68,8 +68,8 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { Env: mockEnv, Group: lifecycle.BuildpackGroup{ Group: []lifecycle.GroupBuildpack{ - {ID: "A", Version: "v1", API: "0.5", Homepage: "Buildpack A Homepage"}, - {ID: "B", Version: "v2", API: "0.2"}, + {ID: "A", Version: "v1", API: latestBuildpackAPI.String(), Homepage: "Buildpack A Homepage"}, + {ID: "B", Version: "v2", API: "0.5"}, }, }, Out: stdout, @@ -121,8 +121,8 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, } - bpA := testmock.NewMockBuildpack(mockCtrl) - bpB := testmock.NewMockBuildpack(mockCtrl) + bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) expectedPlanA := lifecycle.BuildpackPlan{Entries: []lifecycle.Require{ {Name: "some-dep", Version: "v1"}, @@ -152,7 +152,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { {ID: "B", Version: "v2", API: "0.2"}, } - bpA := testmock.NewMockBuildpack(mockCtrl) + bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ BOM: []lifecycle.BOMEntry{ @@ -165,7 +165,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl) + bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ BOM: []lifecycle.BOMEntry{ @@ -208,10 +208,10 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("buildpacks", func() { it("should include builder buildpacks", func() { - bpA := testmock.NewMockBuildpack(mockCtrl) + bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config) - bpB := testmock.NewMockBuildpack(mockCtrl) + bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config) @@ -220,8 +220,8 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { t.Fatalf("Unexpected error:\n%s\n", err) } if s := cmp.Diff(metadata.Buildpacks, []lifecycle.GroupBuildpack{ - {ID: "A", Version: "v1", API: "0.5", Homepage: "Buildpack A Homepage"}, - {ID: "B", Version: "v2", API: "0.2"}, + {ID: "A", Version: "v1", API: latestBuildpackAPI.String(), Homepage: "Buildpack A Homepage"}, + {ID: "B", Version: "v2", API: "0.5"}, }); s != "" { t.Fatalf("Unexpected:\n%s\n", s) } @@ -230,7 +230,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("labels", func() { it("should aggregate labels from each buildpack", func() { - bpA := testmock.NewMockBuildpack(mockCtrl) + bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Labels: []lifecycle.Label{ @@ -238,7 +238,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { {Key: "some-other-bpA-key", Value: "some-other-bpA-value"}, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl) + bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Labels: []lifecycle.Label{ @@ -264,7 +264,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("processes", func() { it("should override identical processes from earlier buildpacks", func() { - bpA := testmock.NewMockBuildpack(mockCtrl) + bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ @@ -284,7 +284,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl) + bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ @@ -337,7 +337,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("should warn when overriding default process type, with non-default process type", func() { - bpA := testmock.NewMockBuildpack(mockCtrl) + bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ @@ -359,7 +359,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl) + bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ @@ -411,7 +411,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("should preserve ordering of processes", func() { - bpA := testmock.NewMockBuildpack(mockCtrl) + bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ @@ -433,7 +433,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl) + bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ @@ -496,11 +496,100 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { t.Fatalf("Unexpected:\n%s\n", s) } }) + + when("builpack API < 0.6", func() { + it("set web processes as default", func() { + bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) + bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "some-type", + Command: "some-cmd", + Args: []string{"some-arg"}, + Direct: true, + BuildpackID: "A", + Default: false, + }, + { + Type: "another-type", + Command: "another-cmd", + Args: []string{"another-arg"}, + Direct: true, + BuildpackID: "A", + Default: false, + }, + }, + }, nil) + bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) + buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) + bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "web", + Command: "web-command", + Args: []string{"web-arg"}, + Direct: false, + BuildpackID: "B", + }, + { + Type: "other", + Command: "other-cmd", + Args: []string{"other-arg"}, + Direct: true, + BuildpackID: "B", + }, + }, + }, nil) + + metadata, err := builder.Build() + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + + if s := cmp.Diff(metadata.Processes, []launch.Process{ + { + Type: "some-type", + Command: "some-cmd", + Args: []string{"some-arg"}, + Direct: true, + BuildpackID: "A", + Default: false, + }, + { + Type: "another-type", + Command: "another-cmd", + Args: []string{"another-arg"}, + Direct: true, + BuildpackID: "A", + Default: false, + }, + { + Type: "web", + Command: "web-command", + Args: []string{"web-arg"}, + Direct: false, + BuildpackID: "B", + Default: true, + }, + { + Type: "other", + Command: "other-cmd", + Args: []string{"other-arg"}, + Direct: true, + BuildpackID: "B", + Default: false, + }, + }); s != "" { + t.Fatalf("Unexpected:\n%s\n", s) + } + }) + }) }) when("slices", func() { it("should aggregate slices from each buildpack", func() { - bpA := testmock.NewMockBuildpack(mockCtrl) + bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Slices: []layers.Slice{ @@ -509,7 +598,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { {Paths: []string{"extra-path"}}, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl) + bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Slices: []layers.Slice{ @@ -539,7 +628,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("building fails", func() { when("first buildpack build fails", func() { it("should error", func() { - bpA := testmock.NewMockBuildpack(mockCtrl) + bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{}, errors.New("some error")) @@ -553,10 +642,10 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("later buildpack build fails", func() { it("should error", func() { - bpA := testmock.NewMockBuildpack(mockCtrl) + bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{}, nil) - bpB := testmock.NewMockBuildpack(mockCtrl) + bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{}, errors.New("some error")) @@ -572,13 +661,12 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("platform api < 0.4", func() { it.Before(func() { builder.PlatformAPI = api.MustParse("0.3") - config.PlatformAPI = "0.3" }) when("build metadata", func() { when("bom", func() { it("should convert metadata.version to top level version", func() { - bpA := testmock.NewMockBuildpack(mockCtrl) + bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ BOM: []lifecycle.BOMEntry{ @@ -591,7 +679,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl) + bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config) diff --git a/buildpacktoml.go b/buildpacktoml.go index b665a4e4a..2e1b79a08 100644 --- a/buildpacktoml.go +++ b/buildpacktoml.go @@ -58,6 +58,10 @@ func (b *BuildpackTOML) String() string { return b.Buildpack.Name + " " + b.Buildpack.Version } +func (b *BuildpackTOML) BuilpackAPI() string { + return b.API +} + func (b *BuildpackTOML) Build(bpPlan BuildpackPlan, config BuildConfig) (BuildResult, error) { if api.MustParse(b.API).Equal(api.MustParse("0.2")) { for i := range bpPlan.Entries { @@ -84,7 +88,7 @@ func (b *BuildpackTOML) Build(bpPlan BuildpackPlan, config BuildConfig) (BuildRe return BuildResult{}, err } - return b.readOutputFiles(bpLayersDir, bpPlanPath, bpPlan, config.PlatformAPI) + return b.readOutputFiles(bpLayersDir, bpPlanPath, bpPlan) } func preparePaths(bpID string, bpPlan BuildpackPlan, layersDir, planDir string) (string, string, error) { @@ -181,7 +185,7 @@ func isBuild(path string) bool { return err == nil && layerTOML.Build } -func (b *BuildpackTOML) readOutputFiles(bpLayersDir, bpPlanPath string, bpPlanIn BuildpackPlan, platformAPI string) (BuildResult, error) { +func (b *BuildpackTOML) readOutputFiles(bpLayersDir, bpPlanPath string, bpPlanIn BuildpackPlan) (BuildResult, error) { br := BuildResult{} bpFromBpInfo := GroupBuildpack{ID: b.Buildpack.ID, Version: b.Buildpack.Version} @@ -247,8 +251,6 @@ func (b *BuildpackTOML) readOutputFiles(bpLayersDir, bpPlanPath string, bpPlanIn return BuildResult{}, err } - updateDefaultProcesses(launchTOML.Processes, b.API, platformAPI) - // set data from launch.toml br.Labels = append([]Label{}, launchTOML.Labels...) for i := range launchTOML.Processes { @@ -260,19 +262,6 @@ func (b *BuildpackTOML) readOutputFiles(bpLayersDir, bpPlanPath string, bpPlanIn return br, nil } -// we set default = true for web processes when platformAPI >= 0.6 and buildpackAPI < 0.6 -func updateDefaultProcesses(processes []launch.Process, bpAPI string, platformAPI string) { - if api.MustParse(platformAPI).Compare(api.MustParse("0.6")) < 0 || api.MustParse(bpAPI).Compare(api.MustParse("0.6")) >= 0 { - return - } - - for i := range processes { - if processes[i].Type == "web" { - processes[i].Default = true - } - } -} - // check that there are not multiple default processes in a buildpack with different process types func validateProcesses(processes []launch.Process) error { defaultType := "" diff --git a/buildpacktoml_test.go b/buildpacktoml_test.go index 4e360008c..d00749676 100644 --- a/buildpacktoml_test.go +++ b/buildpacktoml_test.go @@ -27,7 +27,6 @@ import ( ) var latestBuildpackAPI = api.Buildpack.Latest() -var latestPlatformAPI = api.Platform.Latest() func TestBuildpackTOML(t *testing.T) { spec.Run(t, "BuildpackTOML", testBuildpackTOML, spec.Report(report.Terminal{})) @@ -73,7 +72,6 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { Env: mockEnv, AppDir: appDir, PlatformDir: platformDir, - PlatformAPI: latestPlatformAPI.String(), LayersDir: layersDir, Out: stdout, Err: stderr, @@ -289,41 +287,6 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { }) when("processes", func() { - when("platform API < 0.6", func() { - it.Before(func() { - config.PlatformAPI = "0.5" - }) - it("should include processes and use the default value that is set", func() { - h.Mkfile(t, - `[[processes]]`+"\n"+ - `type = "some-type"`+"\n"+ - `command = "some-cmd"`+"\n"+ - `default = true`+"\n"+ - `[[processes]]`+"\n"+ - `type = "web"`+"\n"+ - `command = "other-cmd"`+"\n", - // default is false and therefore doesn't appear - filepath.Join(appDir, "launch-A-v1.toml"), - ) - br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) - if err != nil { - t.Fatalf("Unexpected error:\n%s\n", err) - } - if s := cmp.Diff(br, lifecycle.BuildResult{ - BOM: nil, - Labels: []lifecycle.Label{}, - MetRequires: nil, - Processes: []launch.Process{ - {Type: "some-type", Command: "some-cmd", BuildpackID: "A", Default: true}, - {Type: "web", Command: "other-cmd", BuildpackID: "A", Default: false}, - }, - Slices: []layers.Slice{}, - }); s != "" { - t.Fatalf("Unexpected metadata:\n%s\n", s) - } - }) - }) - it("should include processes and use the default value that is set", func() { h.Mkfile(t, `[[processes]]`+"\n"+ @@ -876,69 +839,33 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { mockEnv.EXPECT().WithPlatform(platformDir).Return(append(os.Environ(), "TEST_ENV=Av1"), nil) }) - when("platform API < 0.6", func() { - it.Before(func() { - config.PlatformAPI = "0.5" - }) - it("should include processes and set their default value to false", func() { - h.Mkfile(t, - `[[processes]]`+"\n"+ - `type = "some-type"`+"\n"+ - `command = "some-cmd"`+"\n"+ - `[[processes]]`+"\n"+ - `type = "web"`+"\n"+ - `command = "other-cmd"`+"\n", - // default is false and therefore doesn't appear - filepath.Join(appDir, "launch-A-v1.toml"), - ) - br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) - if err != nil { - t.Fatalf("Unexpected error:\n%s\n", err) - } - if s := cmp.Diff(br, lifecycle.BuildResult{ - BOM: nil, - Labels: []lifecycle.Label{}, - MetRequires: nil, - Processes: []launch.Process{ - {Type: "some-type", Command: "some-cmd", BuildpackID: "A", Default: false}, - {Type: "web", Command: "other-cmd", BuildpackID: "A", Default: false}, - }, - Slices: []layers.Slice{}, - }); s != "" { - t.Fatalf("Unexpected metadata:\n%s\n", s) - } - }) - }) - - when("platform API >= 0.6", func() { - it("should include processes, all default values should be false, except for the web processes types", func() { - h.Mkfile(t, + it("should include processes and set their default value to false", func() { + h.Mkfile(t, + `[[processes]]`+"\n"+ + `type = "some-type"`+"\n"+ + `command = "some-cmd"`+"\n"+ `[[processes]]`+"\n"+ - `type = "some-type"`+"\n"+ - `command = "some-cmd"`+"\n"+ - `[[processes]]`+"\n"+ - `type = "web"`+"\n"+ - `command = "other-cmd"`+"\n", - // default is false and therefore doesn't appear - filepath.Join(appDir, "launch-A-v1.toml"), - ) - br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) - if err != nil { - t.Fatalf("Unexpected error:\n%s\n", err) - } - if s := cmp.Diff(br, lifecycle.BuildResult{ - BOM: nil, - Labels: []lifecycle.Label{}, - MetRequires: nil, - Processes: []launch.Process{ - {Type: "some-type", Command: "some-cmd", BuildpackID: "A", Default: false}, - {Type: "web", Command: "other-cmd", BuildpackID: "A", Default: true}, - }, - Slices: []layers.Slice{}, - }); s != "" { - t.Fatalf("Unexpected metadata:\n%s\n", s) - } - }) + `type = "web"`+"\n"+ + `command = "other-cmd"`+"\n", + // default is false and therefore doesn't appear + filepath.Join(appDir, "launch-A-v1.toml"), + ) + br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + if s := cmp.Diff(br, lifecycle.BuildResult{ + BOM: nil, + Labels: []lifecycle.Label{}, + MetRequires: nil, + Processes: []launch.Process{ + {Type: "some-type", Command: "some-cmd", BuildpackID: "A", Default: false}, + {Type: "web", Command: "other-cmd", BuildpackID: "A", Default: false}, + }, + Slices: []layers.Slice{}, + }); s != "" { + t.Fatalf("Unexpected metadata:\n%s\n", s) + } }) }) }) diff --git a/testmock/buildpack.go b/testmock/buildpack.go index e6dbb44f6..28b26d93b 100644 --- a/testmock/buildpack.go +++ b/testmock/buildpack.go @@ -16,6 +16,7 @@ import ( type MockBuildpack struct { ctrl *gomock.Controller recorder *MockBuildpackMockRecorder + api string } // MockBuildpackMockRecorder is the mock recorder for MockBuildpack @@ -24,9 +25,10 @@ type MockBuildpackMockRecorder struct { } // NewMockBuildpack creates a new mock instance -func NewMockBuildpack(ctrl *gomock.Controller) *MockBuildpack { +func NewMockBuildpack(ctrl *gomock.Controller, api string) *MockBuildpack { mock := &MockBuildpack{ctrl: ctrl} mock.recorder = &MockBuildpackMockRecorder{mock} + mock.api = api return mock } @@ -44,6 +46,10 @@ func (m *MockBuildpack) Build(arg0 lifecycle.BuildpackPlan, arg1 lifecycle.Build return ret0, ret1 } +func (m *MockBuildpack) BuilpackAPI() string { + return m.api +} + // Build indicates an expected call of Build func (mr *MockBuildpackMockRecorder) Build(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() From 39b54f99de390ffee44535cdb45f3973cc59b698 Mon Sep 17 00:00:00 2001 From: Yael Harel Date: Tue, 2 Feb 2021 00:04:44 -0500 Subject: [PATCH 04/10] Review feedback Signed-off-by: Yael Harel --- builder.go | 4 ++-- buildpacktoml_test.go | 2 -- exporter.go | 6 ++++++ exporter_test.go | 24 ++++++++++++++++++++++-- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/builder.go b/builder.go index 8e4abca92..34ade3ef4 100644 --- a/builder.go +++ b/builder.go @@ -207,8 +207,8 @@ func containsEntry(metRequires []string, entry BuildPlanEntry) bool { } // orderedProcesses is a mapping from process types to Processes, it will preserve ordering. -// processList is the ordered list -// wwe keep typeToPtr map in order to delete elements when others are overriding them +// processList is the ordered list. +// we keep typeToProcess map in order to delete elements when others are overriding them. type orderedProcesses struct { typeToProcess map[string]*list.Element processList *list.List diff --git a/buildpacktoml_test.go b/buildpacktoml_test.go index d00749676..14cac17d8 100644 --- a/buildpacktoml_test.go +++ b/buildpacktoml_test.go @@ -330,7 +330,6 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { `type = "some-type"`+"\n"+ `command = "some-other-cmd"`+"\n"+ `default = true`+"\n", - // default is false and therefore doesn't appear filepath.Join(appDir, "launch-A-v1.toml"), ) br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) @@ -579,7 +578,6 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { `type = "other-type"`+"\n"+ `command = "other-cmd"`+"\n"+ `default = true`+"\n", - // default is false and therefore doesn't appear filepath.Join(appDir, "launch-A-v1.toml"), ) _, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) diff --git a/exporter.go b/exporter.go index 689771671..8d506e46e 100644 --- a/exporter.go +++ b/exporter.go @@ -384,6 +384,12 @@ func (e *Exporter) entrypoint(launchMD launch.Metadata, defaultProcessType strin return launch.LauncherPath, nil } + if defaultProcessType == "" && e.PlatformAPI.Compare(api.MustParse("0.6")) < 0 && len(launchMD.Processes) == 1 { + // if there is only one process, we set it to the default for platform API < 0.6 + e.Logger.Infof("Setting default process type '%s'", launchMD.Processes[0].Type) + return launch.ProcessPath(launchMD.Processes[0].Type), nil + } + if defaultProcessType != "" { defaultProcess, ok := launchMD.FindProcessType(defaultProcessType) if !ok { diff --git a/exporter_test.go b/exporter_test.go index 40148c24f..919e839f0 100644 --- a/exporter_test.go +++ b/exporter_test.go @@ -1026,15 +1026,27 @@ version = "4.5.6" h.AssertNil(t, err) checkEntrypoint(t, fakeAppImage, "process", "some-second-default-process-type") }) + + it("doesn't set CNB_PROCESS_TYPE", func() { + _, err := exporter.Export(opts) + h.AssertNil(t, err) + + val, err := fakeAppImage.Env("CNB_PROCESS_TYPE") + h.AssertNil(t, err) + h.AssertEq(t, val, "") + }) }) }) when("platform API < 0.6", func() { + it.Before(func() { + exporter.PlatformAPI = api.MustParse("0.5") + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "one-non-default-process", "layers"), opts.LayersDir) + }) + when("-process-type is set to a process type that doesn't exist", func() { it.Before(func() { - exporter.PlatformAPI = api.MustParse("0.5") opts.DefaultProcessType = "some-non-existing-process-type" - h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "one-non-default-process", "layers"), opts.LayersDir) }) it("warns the process type doesn't exist, and sets the ENTRYPOINT to the launcher", func() { _, err := exporter.Export(opts) @@ -1043,6 +1055,14 @@ version = "4.5.6" checkEntrypoint(t, fakeAppImage, "lifecycle", "launcher") }) }) + + when("-process-type is not set and there is exactly one process", func() { + it("sets the ENTRYPOINT to the only process", func() { + _, err := exporter.Export(opts) + h.AssertNil(t, err) + checkEntrypoint(t, fakeAppImage, "process", "some-process-type") + }) + }) }) when("platform API < 0.4", func() { From d7ab168860f191f6a739be5f5b90ef6284f66963 Mon Sep 17 00:00:00 2001 From: Yael Harel Date: Tue, 2 Feb 2021 16:10:06 -0500 Subject: [PATCH 05/10] Changes per feedback Signed-off-by: Yael Harel --- builder.go | 19 +++-- builder_test.go | 161 ++++++++++++++++++++++++++++++++---------- buildpacktoml.go | 4 -- buildpacktoml_test.go | 7 +- testmock/buildpack.go | 8 +-- 5 files changed, 139 insertions(+), 60 deletions(-) diff --git a/builder.go b/builder.go index 34ade3ef4..3bc6a5ddd 100644 --- a/builder.go +++ b/builder.go @@ -26,7 +26,6 @@ type BuildpackStore interface { type Buildpack interface { Build(bpPlan BuildpackPlan, config BuildConfig) (BuildResult, error) - BuilpackAPI() string } type BuildConfig struct { @@ -78,7 +77,7 @@ func (b *Builder) Build() (*BuildMetadata, error) { return nil, err } - procStruct := newProcessStruct() + orderedProcessList := newOrderedProcessList() plan := b.Plan var bom []BOMEntry var slices []layers.Slice @@ -96,12 +95,12 @@ func (b *Builder) Build() (*BuildMetadata, error) { return nil, err } - updateDefaultProcesses(br.Processes, api.MustParse(bpTOML.BuilpackAPI()), b.PlatformAPI) + updateDefaultProcesses(br.Processes, api.MustParse(bp.API), b.PlatformAPI) bom = append(bom, br.BOM...) labels = append(labels, br.Labels...) plan = plan.filter(br.MetRequires) - replacedDefaults, err := procStruct.add(br.Processes) + replacedDefaults, err := orderedProcessList.add(br.Processes) if err != nil { return nil, err } @@ -120,7 +119,7 @@ func (b *Builder) Build() (*BuildMetadata, error) { bom[i].convertMetadataToVersion() } } - procList, err := procStruct.list() + procList, err := orderedProcessList.list() if err != nil { return nil, err } @@ -214,7 +213,7 @@ type orderedProcesses struct { processList *list.List } -func newProcessStruct() orderedProcesses { +func newOrderedProcessList() orderedProcesses { return orderedProcesses{ typeToProcess: make(map[string]*list.Element), processList: list.New(), @@ -226,11 +225,11 @@ func (m orderedProcesses) add(listToAdd []launch.Process) ([]string, error) { for _, proc := range listToAdd { // when we replace a default process type with a non-default process type, add to result if p, ok := m.typeToProcess[proc.Type]; ok { - cast, success := (p.Value).(launch.Process) + existingProcess, success := (p.Value).(launch.Process) if !success { return []string{}, fmt.Errorf("can't cast an element from the list to a process") } - if cast.Default && !proc.Default { + if existingProcess.Default && !proc.Default { result = append(result, proc.Type) } m.processList.Remove(p) @@ -247,11 +246,11 @@ func (m orderedProcesses) add(listToAdd []launch.Process) ([]string, error) { func (m orderedProcesses) list() ([]launch.Process, error) { result := []launch.Process{} for e := m.processList.Front(); e != nil; e = e.Next() { - cast, success := (e.Value).(launch.Process) + currentProcess, success := (e.Value).(launch.Process) if !success { return []launch.Process{}, fmt.Errorf("can't cast an element from the list to a process") } - result = append(result, cast) + result = append(result, currentProcess) } return result, nil } diff --git a/builder_test.go b/builder_test.go index 088b87fcd..d8dc600ce 100644 --- a/builder_test.go +++ b/builder_test.go @@ -121,8 +121,8 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, } - bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) - bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) + bpA := testmock.NewMockBuildpack(mockCtrl) + bpB := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) expectedPlanA := lifecycle.BuildpackPlan{Entries: []lifecycle.Require{ {Name: "some-dep", Version: "v1"}, @@ -152,7 +152,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { {ID: "B", Version: "v2", API: "0.2"}, } - bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + bpA := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ BOM: []lifecycle.BOMEntry{ @@ -165,7 +165,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) + bpB := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ BOM: []lifecycle.BOMEntry{ @@ -208,10 +208,10 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("buildpacks", func() { it("should include builder buildpacks", func() { - bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + bpA := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config) - bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) + bpB := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config) @@ -230,7 +230,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("labels", func() { it("should aggregate labels from each buildpack", func() { - bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + bpA := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Labels: []lifecycle.Label{ @@ -238,7 +238,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { {Key: "some-other-bpA-key", Value: "some-other-bpA-value"}, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) + bpB := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Labels: []lifecycle.Label{ @@ -264,7 +264,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("processes", func() { it("should override identical processes from earlier buildpacks", func() { - bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + bpA := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ @@ -284,7 +284,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) + bpB := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ @@ -337,12 +337,12 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) it("should warn when overriding default process type, with non-default process type", func() { - bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + bpA := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ { - Type: "override-type", + Type: "override-with-not-default", Command: "bpA-command", Args: []string{"bpA-arg"}, Direct: true, @@ -350,7 +350,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { Default: true, }, { - Type: "second-override-type", + Type: "override-with-default", Command: "some-command", Args: []string{"some-arg"}, Direct: true, @@ -359,19 +359,19 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) + bpB := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ { - Type: "override-type", + Type: "override-with-not-default", Command: "bpB-command", Args: []string{"bpB-arg"}, Direct: false, BuildpackID: "B", }, { - Type: "second-override-type", + Type: "override-with-default", Command: "some-other-command", Args: []string{"some-other-arg"}, Direct: true, @@ -387,7 +387,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { } if s := cmp.Diff(metadata.Processes, []launch.Process{ { - Type: "override-type", + Type: "override-with-not-default", Command: "bpB-command", Args: []string{"bpB-arg"}, Direct: false, @@ -395,7 +395,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { Default: false, }, { - Type: "second-override-type", + Type: "override-with-default", Command: "some-other-command", Args: []string{"some-other-arg"}, Direct: true, @@ -406,12 +406,12 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { t.Fatalf("Unexpected:\n%s\n", s) } - expected := "Warning: redefining the following default process types with processes not marked as default: [override-type]" + expected := "Warning: redefining the following default process types with processes not marked as default: [override-with-not-default]" h.AssertStringContains(t, stdout.String(), expected) }) it("should preserve ordering of processes", func() { - bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + bpA := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ @@ -433,7 +433,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) + bpB := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ @@ -497,9 +497,96 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { } }) + it("shouldn't set web processes as default", func() { + bpA := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) + bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "not-web", + Command: "not-web-cmd", + Args: []string{"not-web-arg"}, + Direct: true, + BuildpackID: "A", + Default: false, + }, + { + Type: "web", + Command: "web-cmd", + Args: []string{"web-arg"}, + Direct: true, + BuildpackID: "A", + Default: false, + }, + }, + }, nil) + bpB := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) + bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "another-type", + Command: "another-cmd", + Args: []string{"another-arg"}, + Direct: false, + BuildpackID: "B", + }, + { + Type: "other", + Command: "other-cmd", + Args: []string{"other-arg"}, + Direct: true, + BuildpackID: "B", + }, + }, + }, nil) + + metadata, err := builder.Build() + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + + if s := cmp.Diff(metadata.Processes, []launch.Process{ + { + Type: "not-web", + Command: "not-web-cmd", + Args: []string{"not-web-arg"}, + Direct: true, + BuildpackID: "A", + Default: false, + }, + { + Type: "web", + Command: "web-cmd", + Args: []string{"web-arg"}, + Direct: true, + BuildpackID: "A", + Default: false, + }, + { + Type: "another-type", + Command: "another-cmd", + Args: []string{"another-arg"}, + Direct: false, + BuildpackID: "B", + Default: false, + }, + { + Type: "other", + Command: "other-cmd", + Args: []string{"other-arg"}, + Direct: true, + BuildpackID: "B", + Default: false, + }, + }); s != "" { + t.Fatalf("Unexpected:\n%s\n", s) + } + }) + when("builpack API < 0.6", func() { - it("set web processes as default", func() { - bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + it("sets web processes as default", func() { + bpA := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ @@ -521,7 +608,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) + bpB := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ @@ -533,9 +620,9 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { BuildpackID: "B", }, { - Type: "other", - Command: "other-cmd", - Args: []string{"other-arg"}, + Type: "not-web", + Command: "not-web-cmd", + Args: []string{"not-web-arg"}, Direct: true, BuildpackID: "B", }, @@ -573,9 +660,9 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { Default: true, }, { - Type: "other", - Command: "other-cmd", - Args: []string{"other-arg"}, + Type: "not-web", + Command: "not-web-cmd", + Args: []string{"not-web-arg"}, Direct: true, BuildpackID: "B", Default: false, @@ -589,7 +676,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("slices", func() { it("should aggregate slices from each buildpack", func() { - bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + bpA := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Slices: []layers.Slice{ @@ -598,7 +685,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { {Paths: []string{"extra-path"}}, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) + bpB := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Slices: []layers.Slice{ @@ -628,7 +715,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("building fails", func() { when("first buildpack build fails", func() { it("should error", func() { - bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + bpA := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{}, errors.New("some error")) @@ -642,10 +729,10 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("later buildpack build fails", func() { it("should error", func() { - bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + bpA := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{}, nil) - bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) + bpB := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{}, errors.New("some error")) @@ -666,7 +753,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("build metadata", func() { when("bom", func() { it("should convert metadata.version to top level version", func() { - bpA := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[0].API) + bpA := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ BOM: []lifecycle.BOMEntry{ @@ -679,7 +766,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }, }, }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl, builder.Group.Group[1].API) + bpB := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) bpB.EXPECT().Build(gomock.Any(), config) diff --git a/buildpacktoml.go b/buildpacktoml.go index 2e1b79a08..1aeb24bdc 100644 --- a/buildpacktoml.go +++ b/buildpacktoml.go @@ -58,10 +58,6 @@ func (b *BuildpackTOML) String() string { return b.Buildpack.Name + " " + b.Buildpack.Version } -func (b *BuildpackTOML) BuilpackAPI() string { - return b.API -} - func (b *BuildpackTOML) Build(bpPlan BuildpackPlan, config BuildConfig) (BuildResult, error) { if api.MustParse(b.API).Equal(api.MustParse("0.2")) { for i := range bpPlan.Entries { diff --git a/buildpacktoml_test.go b/buildpacktoml_test.go index 14cac17d8..4ea97e6e4 100644 --- a/buildpacktoml_test.go +++ b/buildpacktoml_test.go @@ -316,7 +316,10 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { t.Fatalf("Unexpected metadata:\n%s\n", s) } }) - when("when there are multiple default=true processes but they are all of the same type", func() { + + // The following test isn't something that buildpacks should do. + // We might change the spec so we'll fail in such case. + when("there are multiple default=true processes but they are all of the same type", func() { it("should succeed", func() { h.Mkfile(t, `[[processes]]`+"\n"+ @@ -563,7 +566,7 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { }) }) - when("when there is more than one default=true process", func() { + when("there is more than one default=true process", func() { it.Before(func() { mockEnv.EXPECT().WithPlatform(platformDir).Return(append(os.Environ(), "TEST_ENV=Av1"), nil) }) diff --git a/testmock/buildpack.go b/testmock/buildpack.go index 28b26d93b..e6dbb44f6 100644 --- a/testmock/buildpack.go +++ b/testmock/buildpack.go @@ -16,7 +16,6 @@ import ( type MockBuildpack struct { ctrl *gomock.Controller recorder *MockBuildpackMockRecorder - api string } // MockBuildpackMockRecorder is the mock recorder for MockBuildpack @@ -25,10 +24,9 @@ type MockBuildpackMockRecorder struct { } // NewMockBuildpack creates a new mock instance -func NewMockBuildpack(ctrl *gomock.Controller, api string) *MockBuildpack { +func NewMockBuildpack(ctrl *gomock.Controller) *MockBuildpack { mock := &MockBuildpack{ctrl: ctrl} mock.recorder = &MockBuildpackMockRecorder{mock} - mock.api = api return mock } @@ -46,10 +44,6 @@ func (m *MockBuildpack) Build(arg0 lifecycle.BuildpackPlan, arg1 lifecycle.Build return ret0, ret1 } -func (m *MockBuildpack) BuilpackAPI() string { - return m.api -} - // Build indicates an expected call of Build func (mr *MockBuildpackMockRecorder) Build(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() From 6e2fc9b5283e4f03c3291471897da9a03b21474d Mon Sep 17 00:00:00 2001 From: Yael Harel Date: Tue, 2 Feb 2021 22:32:59 -0500 Subject: [PATCH 06/10] Change the checkEntrypoint signature Signed-off-by: Yael Harel --- exporter_test.go | 18 +++++++----------- lifecycle_unix_test.go | 8 ++++++++ lifecycle_windows_test.go | 6 ++++++ 3 files changed, 21 insertions(+), 11 deletions(-) create mode 100644 lifecycle_unix_test.go create mode 100644 lifecycle_windows_test.go diff --git a/exporter_test.go b/exporter_test.go index 919e839f0..aa8aa1b0c 100644 --- a/exporter_test.go +++ b/exporter_test.go @@ -947,7 +947,7 @@ version = "4.5.6" _, err := exporter.Export(opts) h.AssertNil(t, err) - checkEntrypoint(t, fakeAppImage, "process", "some-process-type") + checkEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "process", "some-process-type"+execExt)) }) it("doesn't set CNB_PROCESS_TYPE", func() { @@ -981,7 +981,7 @@ version = "4.5.6" _, err := exporter.Export(opts) h.AssertNil(t, err) assertLogEntry(t, logHandler, "no default process type") - checkEntrypoint(t, fakeAppImage, "lifecycle", "launcher") + checkEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "lifecycle", "launcher"+execExt)) }) }) @@ -1024,7 +1024,7 @@ version = "4.5.6" it("sets the last default=true process to be the default", func() { _, err := exporter.Export(opts) h.AssertNil(t, err) - checkEntrypoint(t, fakeAppImage, "process", "some-second-default-process-type") + checkEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "process", "some-second-default-process-type"+execExt)) }) it("doesn't set CNB_PROCESS_TYPE", func() { @@ -1052,7 +1052,7 @@ version = "4.5.6" _, err := exporter.Export(opts) h.AssertNil(t, err) assertLogEntry(t, logHandler, "default process type 'some-non-existing-process-type' not present in list [some-process-type]") - checkEntrypoint(t, fakeAppImage, "lifecycle", "launcher") + checkEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "lifecycle", "launcher"+execExt)) }) }) @@ -1060,7 +1060,7 @@ version = "4.5.6" it("sets the ENTRYPOINT to the only process", func() { _, err := exporter.Export(opts) h.AssertNil(t, err) - checkEntrypoint(t, fakeAppImage, "process", "some-process-type") + checkEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "process", "some-process-type"+execExt)) }) }) }) @@ -1335,15 +1335,11 @@ version = "4.5.6" }) } -func checkEntrypoint(t *testing.T, image *fakes.Image, expectedDir string, expectedFile string) { +func checkEntrypoint(t *testing.T, image *fakes.Image, entrypointPath string) { ep, err := image.Entrypoint() h.AssertNil(t, err) h.AssertEq(t, len(ep), 1) - if runtime.GOOS == "windows" { - h.AssertEq(t, ep[0], fmt.Sprintf(`c:\cnb\%s\%s.exe`, expectedDir, expectedFile)) - } else { - h.AssertEq(t, ep[0], fmt.Sprintf(`/cnb/%s/%s`, expectedDir, expectedFile)) - } + h.AssertEq(t, ep[0], entrypointPath) } func createTestLayer(id string, tmpDir string) (layers.Layer, error) { diff --git a/lifecycle_unix_test.go b/lifecycle_unix_test.go new file mode 100644 index 000000000..2adf4f6f2 --- /dev/null +++ b/lifecycle_unix_test.go @@ -0,0 +1,8 @@ +// +build linux darwin + +package lifecycle_test + +const ( + rootDir = "/" + execExt = "" +) diff --git a/lifecycle_windows_test.go b/lifecycle_windows_test.go new file mode 100644 index 000000000..cb7665f0b --- /dev/null +++ b/lifecycle_windows_test.go @@ -0,0 +1,6 @@ +package lifecycle_test + +const ( + rootDir = `c:\` + execExt = ".exe" +) From 5bfdbae4143f612e3230909a3729ca31a69355c9 Mon Sep 17 00:00:00 2001 From: Yael Harel Date: Wed, 3 Feb 2021 16:42:04 -0500 Subject: [PATCH 07/10] Handle buildpack api < 0.6 with default process set: Buildpacks that implements api < 0.6 should have the default value set to false Signed-off-by: Yael Harel --- buildpacktoml.go | 34 ++++++++++++++++++++++++++++++---- buildpacktoml_test.go | 16 +++++++++------- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/buildpacktoml.go b/buildpacktoml.go index 1aeb24bdc..76d3e66d8 100644 --- a/buildpacktoml.go +++ b/buildpacktoml.go @@ -3,10 +3,12 @@ package lifecycle import ( "errors" "fmt" + "io" "io/ioutil" "os" "os/exec" "path/filepath" + "strings" "github.com/BurntSushi/toml" @@ -84,7 +86,7 @@ func (b *BuildpackTOML) Build(bpPlan BuildpackPlan, config BuildConfig) (BuildRe return BuildResult{}, err } - return b.readOutputFiles(bpLayersDir, bpPlanPath, bpPlan) + return b.readOutputFiles(bpLayersDir, bpPlanPath, bpPlan, config.Out) } func preparePaths(bpID string, bpPlan BuildpackPlan, layersDir, planDir string) (string, string, error) { @@ -181,7 +183,7 @@ func isBuild(path string) bool { return err == nil && layerTOML.Build } -func (b *BuildpackTOML) readOutputFiles(bpLayersDir, bpPlanPath string, bpPlanIn BuildpackPlan) (BuildResult, error) { +func (b *BuildpackTOML) readOutputFiles(bpLayersDir, bpPlanPath string, bpPlanIn BuildpackPlan, out io.Writer) (BuildResult, error) { br := BuildResult{} bpFromBpInfo := GroupBuildpack{ID: b.Buildpack.ID, Version: b.Buildpack.Version} @@ -243,7 +245,11 @@ func (b *BuildpackTOML) readOutputFiles(bpLayersDir, bpPlanPath string, bpPlanIn br.BOM = withBuildpack(bpFromBpInfo, launchTOML.BOM) } - if err := validateProcesses(launchTOML.Processes); err != nil { + if err := overrideDefaultForOldBuildpacks(launchTOML.Processes, b.API, out); err != nil { + return BuildResult{}, err + } + + if err := validateNoMultipleDefaults(launchTOML.Processes); err != nil { return BuildResult{}, err } @@ -258,8 +264,28 @@ func (b *BuildpackTOML) readOutputFiles(bpLayersDir, bpPlanPath string, bpPlanIn return br, nil } +func overrideDefaultForOldBuildpacks(processes []launch.Process, bpAPI string, out io.Writer) error { + if api.MustParse(bpAPI).Compare(api.MustParse("0.6")) >= 0 { + return nil + } + replacedDefaults := []string{} + for i := range processes { + if processes[i].Default { + replacedDefaults = append(replacedDefaults, processes[i].Type) + } + processes[i].Default = false + } + if len(replacedDefaults) > 0 { + warning := fmt.Sprintf("Warning: default processes aren't supported in this buildpack api version. Overriding the default value to false for the following processes: [%s]", strings.Join(replacedDefaults, ", ")) + if _, err := out.Write([]byte(warning)); err != nil { + return err + } + } + return nil +} + // check that there are not multiple default processes in a buildpack with different process types -func validateProcesses(processes []launch.Process) error { +func validateNoMultipleDefaults(processes []launch.Process) error { defaultType := "" for _, process := range processes { if process.Default && defaultType != "" && defaultType != process.Type { diff --git a/buildpacktoml_test.go b/buildpacktoml_test.go index 4ea97e6e4..3f6f752fb 100644 --- a/buildpacktoml_test.go +++ b/buildpacktoml_test.go @@ -840,15 +840,15 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { mockEnv.EXPECT().WithPlatform(platformDir).Return(append(os.Environ(), "TEST_ENV=Av1"), nil) }) - it("should include processes and set their default value to false", func() { + it("should include processes and set/override their default value to false", func() { h.Mkfile(t, `[[processes]]`+"\n"+ - `type = "some-type"`+"\n"+ + `type = "type-with-no-default"`+"\n"+ `command = "some-cmd"`+"\n"+ `[[processes]]`+"\n"+ - `type = "web"`+"\n"+ - `command = "other-cmd"`+"\n", - // default is false and therefore doesn't appear + `type = "type-with-default"`+"\n"+ + `command = "other-cmd"`+"\n"+ + `default = true`+"\n", filepath.Join(appDir, "launch-A-v1.toml"), ) br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) @@ -860,13 +860,15 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { Labels: []lifecycle.Label{}, MetRequires: nil, Processes: []launch.Process{ - {Type: "some-type", Command: "some-cmd", BuildpackID: "A", Default: false}, - {Type: "web", Command: "other-cmd", BuildpackID: "A", Default: false}, + {Type: "type-with-no-default", Command: "some-cmd", BuildpackID: "A", Default: false}, + {Type: "type-with-default", Command: "other-cmd", BuildpackID: "A", Default: false}, }, Slices: []layers.Slice{}, }); s != "" { t.Fatalf("Unexpected metadata:\n%s\n", s) } + expected := "Warning: default processes aren't supported in this buildpack api version. Overriding the default value to false for the following processes: [type-with-default]" + h.AssertStringContains(t, stdout.String(), expected) }) }) }) From 4401dec2ac23fdc17898301be0bff1352bf37078 Mon Sep 17 00:00:00 2001 From: Yael Harel Date: Wed, 3 Feb 2021 19:49:34 -0500 Subject: [PATCH 08/10] Error if there are multiple default processes in the same buildpack no matter if they are having the same type or different types Signed-off-by: Yael Harel --- buildpacktoml.go | 3 +- buildpacktoml_test.go | 90 ++++++++++++++++++------------------------- 2 files changed, 38 insertions(+), 55 deletions(-) diff --git a/buildpacktoml.go b/buildpacktoml.go index 76d3e66d8..c9cbd2d39 100644 --- a/buildpacktoml.go +++ b/buildpacktoml.go @@ -284,11 +284,10 @@ func overrideDefaultForOldBuildpacks(processes []launch.Process, bpAPI string, o return nil } -// check that there are not multiple default processes in a buildpack with different process types func validateNoMultipleDefaults(processes []launch.Process) error { defaultType := "" for _, process := range processes { - if process.Default && defaultType != "" && defaultType != process.Type { + if process.Default && defaultType != "" { return fmt.Errorf("multiple default process types aren't allowed") } if process.Default { diff --git a/buildpacktoml_test.go b/buildpacktoml_test.go index 3f6f752fb..258b98427 100644 --- a/buildpacktoml_test.go +++ b/buildpacktoml_test.go @@ -316,44 +316,6 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { t.Fatalf("Unexpected metadata:\n%s\n", s) } }) - - // The following test isn't something that buildpacks should do. - // We might change the spec so we'll fail in such case. - when("there are multiple default=true processes but they are all of the same type", func() { - it("should succeed", func() { - h.Mkfile(t, - `[[processes]]`+"\n"+ - `type = "some-type"`+"\n"+ - `command = "some-cmd"`+"\n"+ - `default = true`+"\n"+ - `[[processes]]`+"\n"+ - `type = "other-type"`+"\n"+ - `command = "other-cmd"`+"\n"+ - `[[processes]]`+"\n"+ - `type = "some-type"`+"\n"+ - `command = "some-other-cmd"`+"\n"+ - `default = true`+"\n", - filepath.Join(appDir, "launch-A-v1.toml"), - ) - br, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) - if err != nil { - t.Fatalf("Unexpected error:\n%s\n", err) - } - if s := cmp.Diff(br, lifecycle.BuildResult{ - BOM: nil, - Labels: []lifecycle.Label{}, - MetRequires: nil, - Processes: []launch.Process{ - {Type: "some-type", Command: "some-cmd", BuildpackID: "A", Default: true}, - {Type: "other-type", Command: "other-cmd", BuildpackID: "A", Default: false}, - {Type: "some-type", Command: "some-other-cmd", BuildpackID: "A", Default: true}, - }, - Slices: []layers.Slice{}, - }); s != "" { - t.Fatalf("Unexpected metadata:\n%s\n", s) - } - }) - }) }) it("should include slices", func() { @@ -571,22 +533,44 @@ func testBuildpackTOML(t *testing.T, when spec.G, it spec.S) { mockEnv.EXPECT().WithPlatform(platformDir).Return(append(os.Environ(), "TEST_ENV=Av1"), nil) }) - it("should error", func() { - h.Mkfile(t, - `[[processes]]`+"\n"+ - `type = "some-type"`+"\n"+ - `command = "some-cmd"`+"\n"+ - `default = true`+"\n"+ + when("the processes are with the same type", func() { + it("should error", func() { + h.Mkfile(t, `[[processes]]`+"\n"+ - `type = "other-type"`+"\n"+ - `command = "other-cmd"`+"\n"+ - `default = true`+"\n", - filepath.Join(appDir, "launch-A-v1.toml"), - ) - _, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) - h.AssertNotNil(t, err) - expected := "multiple default process types aren't allowed" - h.AssertStringContains(t, err.Error(), expected) + `type = "some-type"`+"\n"+ + `command = "some-cmd"`+"\n"+ + `default = true`+"\n"+ + `[[processes]]`+"\n"+ + `type = "some-type"`+"\n"+ + `command = "some-other-cmd"`+"\n"+ + `default = true`+"\n", + filepath.Join(appDir, "launch-A-v1.toml"), + ) + _, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) + h.AssertNotNil(t, err) + expected := "multiple default process types aren't allowed" + h.AssertStringContains(t, err.Error(), expected) + }) + }) + + when("the processes are with different types", func() { + it("should error", func() { + h.Mkfile(t, + `[[processes]]`+"\n"+ + `type = "some-type"`+"\n"+ + `command = "some-cmd"`+"\n"+ + `default = true`+"\n"+ + `[[processes]]`+"\n"+ + `type = "other-type"`+"\n"+ + `command = "other-cmd"`+"\n"+ + `default = true`+"\n", + filepath.Join(appDir, "launch-A-v1.toml"), + ) + _, err := bpTOML.Build(lifecycle.BuildpackPlan{}, config) + h.AssertNotNil(t, err) + expected := "multiple default process types aren't allowed" + h.AssertStringContains(t, err.Error(), expected) + }) }) }) }) From d410f33cc73455e09918bb963a0db17cc0cfc1ee Mon Sep 17 00:00:00 2001 From: Yael Harel Date: Thu, 4 Feb 2021 20:54:58 -0500 Subject: [PATCH 09/10] Changes: - Introduce a new field in metadata.toml - Buildpack-default-process-type - Remove the default field from each process in metadata.toml - Sort the processes in metadata.toml alphabetically based on their types - Don't fall back to old default process if another default buildpack overrided it and then the new default process was overrided by a non-default process (X default -> Y default -> Y not-default ==> no default process) Signed-off-by: Daniel Thornton Signed-off-by: Yael Harel Signed-off-by: Daniel Thornton --- builder.go | 101 ++-- builder_test.go | 524 ++++++++---------- exporter.go | 21 +- exporter_test.go | 44 +- metadata.go | 13 +- .../layers/config/metadata.toml | 8 + .../layers/config/metadata.toml | 0 .../layers/config/metadata.toml | 21 - 8 files changed, 319 insertions(+), 413 deletions(-) create mode 100644 testdata/exporter/default-process/metadata-with-default/layers/config/metadata.toml rename testdata/exporter/default-process/{one-non-default-process => metadata-with-no-default}/layers/config/metadata.toml (100%) delete mode 100644 testdata/exporter/default-process/multiple-processes-with-default/layers/config/metadata.toml diff --git a/builder.go b/builder.go index 3bc6a5ddd..748b97ac2 100644 --- a/builder.go +++ b/builder.go @@ -1,11 +1,10 @@ package lifecycle import ( - "container/list" "fmt" "io" "path/filepath" - "strings" + "sort" "github.com/buildpacks/lifecycle/api" "github.com/buildpacks/lifecycle/env" @@ -77,7 +76,7 @@ func (b *Builder) Build() (*BuildMetadata, error) { return nil, err } - orderedProcessList := newOrderedProcessList() + processMap := newProcessMap() plan := b.Plan var bom []BOMEntry var slices []layers.Slice @@ -100,13 +99,14 @@ func (b *Builder) Build() (*BuildMetadata, error) { bom = append(bom, br.BOM...) labels = append(labels, br.Labels...) plan = plan.filter(br.MetRequires) - replacedDefaults, err := orderedProcessList.add(br.Processes) + + replacedDefault := processMap.add(br.Processes) if err != nil { return nil, err } - if len(replacedDefaults) > 0 { - warning := fmt.Sprintf("Warning: redefining the following default process types with processes not marked as default: [%s]", strings.Join(replacedDefaults, ", ")) + if replacedDefault != "" { + warning := fmt.Sprintf("Warning: redefining the following default process type with a process not marked as default: %s", replacedDefault) if _, err := b.Out.Write([]byte(warning)); err != nil { return nil, err } @@ -119,17 +119,15 @@ func (b *Builder) Build() (*BuildMetadata, error) { bom[i].convertMetadataToVersion() } } - procList, err := orderedProcessList.list() - if err != nil { - return nil, err - } + procList := processMap.list() return &BuildMetadata{ - BOM: bom, - Buildpacks: b.Group.Group, - Labels: labels, - Processes: procList, - Slices: slices, + BOM: bom, + Buildpacks: b.Group.Group, + Labels: labels, + Processes: procList, + Slices: slices, + BuildpackDefaultProcessType: processMap.defaultType, }, nil } @@ -205,54 +203,57 @@ func containsEntry(metRequires []string, entry BuildPlanEntry) bool { return false } -// orderedProcesses is a mapping from process types to Processes, it will preserve ordering. -// processList is the ordered list. -// we keep typeToProcess map in order to delete elements when others are overriding them. -type orderedProcesses struct { - typeToProcess map[string]*list.Element - processList *list.List +type processMap struct { + typeToProcess map[string]launch.Process + defaultType string } -func newOrderedProcessList() orderedProcesses { - return orderedProcesses{ - typeToProcess: make(map[string]*list.Element), - processList: list.New(), +func newProcessMap() processMap { + return processMap{ + typeToProcess: make(map[string]launch.Process), + defaultType: "", } } -func (m orderedProcesses) add(listToAdd []launch.Process) ([]string, error) { - result := []string{} - for _, proc := range listToAdd { - // when we replace a default process type with a non-default process type, add to result - if p, ok := m.typeToProcess[proc.Type]; ok { - existingProcess, success := (p.Value).(launch.Process) - if !success { - return []string{}, fmt.Errorf("can't cast an element from the list to a process") +// This function adds the processes from listToAdd to processMap +// it sets m.defaultType to the last default process +// if a non-default process overrides a default process, it returns its type and unset m.defaultType +func (m *processMap) add(listToAdd []launch.Process) string { + result := "" + for _, procToAdd := range listToAdd { + if procToAdd.Default { + m.defaultType = procToAdd.Type + result = "" + } else { + existingProc, ok := m.typeToProcess[procToAdd.Type] + if ok && existingProc.Type == m.defaultType { // existingProc.Default = true + // non-default process overrides a default process + m.defaultType = "" + result = procToAdd.Type } - if existingProcess.Default && !proc.Default { - result = append(result, proc.Type) - } - m.processList.Remove(p) } - m.processList.PushBack(proc) - m.typeToProcess[proc.Type] = m.processList.Back() + m.typeToProcess[procToAdd.Type] = procToAdd } - return result, nil + return result } -// list returns an ordered array of process types. The ordering is based on the -// order that the processes were added to this struct. -func (m orderedProcesses) list() ([]launch.Process, error) { +// list returns a sorted array of processes. +// The array is sorted based on the process types. +// The list is sorted for reproducibility. +func (m processMap) list() []launch.Process { + var keys []string + for proc := range m.typeToProcess { + keys = append(keys, proc) + } + sort.Strings(keys) result := []launch.Process{} - for e := m.processList.Front(); e != nil; e = e.Next() { - currentProcess, success := (e.Value).(launch.Process) - if !success { - return []launch.Process{}, fmt.Errorf("can't cast an element from the list to a process") - } - result = append(result, currentProcess) + for _, key := range keys { + processWithNoDefault := m.typeToProcess[key] + processWithNoDefault.Default = false // we set the default to false so it won't be part of metadata.toml + result = append(result, processWithNoDefault) } - return result, nil + return result } func (bom *BOMEntry) convertMetadataToVersion() { diff --git a/builder_test.go b/builder_test.go index d8dc600ce..2d5f1f1c3 100644 --- a/builder_test.go +++ b/builder_test.go @@ -69,7 +69,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { Group: lifecycle.BuildpackGroup{ Group: []lifecycle.GroupBuildpack{ {ID: "A", Version: "v1", API: latestBuildpackAPI.String(), Homepage: "Buildpack A Homepage"}, - {ID: "B", Version: "v2", API: "0.5"}, + {ID: "B", Version: "v2", API: latestBuildpackAPI.String()}, }, }, Out: stdout, @@ -221,7 +221,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { } if s := cmp.Diff(metadata.Buildpacks, []lifecycle.GroupBuildpack{ {ID: "A", Version: "v1", API: latestBuildpackAPI.String(), Homepage: "Buildpack A Homepage"}, - {ID: "B", Version: "v2", API: "0.5"}, + {ID: "B", Version: "v2", API: latestBuildpackAPI.String()}, }); s != "" { t.Fatalf("Unexpected:\n%s\n", s) } @@ -310,20 +310,6 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { t.Fatalf("Unexpected error:\n%s\n", err) } if s := cmp.Diff(metadata.Processes, []launch.Process{ - { - Type: "some-type", - Command: "some-command", - Args: []string{"some-arg"}, - Direct: true, - BuildpackID: "A", - }, - { - Type: "some-other-type", - Command: "some-other-command", - Args: []string{"some-other-arg"}, - Direct: true, - BuildpackID: "B", - }, { Type: "override-type", Command: "bpB-command", @@ -331,280 +317,47 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { Direct: false, BuildpackID: "B", }, - }); s != "" { - t.Fatalf("Unexpected:\n%s\n", s) - } - }) - - it("should warn when overriding default process type, with non-default process type", func() { - bpA := testmock.NewMockBuildpack(mockCtrl) - buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) - bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ - Processes: []launch.Process{ - { - Type: "override-with-not-default", - Command: "bpA-command", - Args: []string{"bpA-arg"}, - Direct: true, - BuildpackID: "A", - Default: true, - }, - { - Type: "override-with-default", - Command: "some-command", - Args: []string{"some-arg"}, - Direct: true, - BuildpackID: "A", - Default: true, - }, - }, - }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl) - buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) - bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ - Processes: []launch.Process{ - { - Type: "override-with-not-default", - Command: "bpB-command", - Args: []string{"bpB-arg"}, - Direct: false, - BuildpackID: "B", - }, - { - Type: "override-with-default", - Command: "some-other-command", - Args: []string{"some-other-arg"}, - Direct: true, - BuildpackID: "B", - Default: true, - }, - }, - }, nil) - - metadata, err := builder.Build() - if err != nil { - t.Fatalf("Unexpected error:\n%s\n", err) - } - if s := cmp.Diff(metadata.Processes, []launch.Process{ - { - Type: "override-with-not-default", - Command: "bpB-command", - Args: []string{"bpB-arg"}, - Direct: false, - BuildpackID: "B", - Default: false, - }, { - Type: "override-with-default", + Type: "some-other-type", Command: "some-other-command", Args: []string{"some-other-arg"}, Direct: true, BuildpackID: "B", - Default: true, - }, - }); s != "" { - t.Fatalf("Unexpected:\n%s\n", s) - } - - expected := "Warning: redefining the following default process types with processes not marked as default: [override-with-not-default]" - h.AssertStringContains(t, stdout.String(), expected) - }) - - it("should preserve ordering of processes", func() { - bpA := testmock.NewMockBuildpack(mockCtrl) - buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) - bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ - Processes: []launch.Process{ - { - Type: "first", - Command: "first-command", - Args: []string{"bpA-arg"}, - Direct: true, - BuildpackID: "A", - Default: true, - }, - { - Type: "second", - Command: "second-command", - Args: []string{"some-arg"}, - Direct: true, - BuildpackID: "A", - Default: true, - }, - }, - }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl) - buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) - bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ - Processes: []launch.Process{ - { - Type: "third", - Command: "third-command", - Args: []string{"bpB-arg"}, - Direct: false, - BuildpackID: "B", - }, - { - Type: "fourth", - Command: "fourth-command", - Args: []string{"some-other-arg"}, - Direct: true, - BuildpackID: "B", - Default: true, - }, - }, - }, nil) - - metadata, err := builder.Build() - if err != nil { - t.Fatalf("Unexpected error:\n%s\n", err) - } - - if s := cmp.Diff(metadata.Processes, []launch.Process{ - { - Type: "first", - Command: "first-command", - Args: []string{"bpA-arg"}, - Direct: true, - BuildpackID: "A", - Default: true, }, { - Type: "second", - Command: "second-command", + Type: "some-type", + Command: "some-command", Args: []string{"some-arg"}, Direct: true, BuildpackID: "A", - Default: true, - }, - { - Type: "third", - Command: "third-command", - Args: []string{"bpB-arg"}, - Direct: false, - BuildpackID: "B", - }, - { - Type: "fourth", - Command: "fourth-command", - Args: []string{"some-other-arg"}, - Direct: true, - BuildpackID: "B", - Default: true, }, }); s != "" { t.Fatalf("Unexpected:\n%s\n", s) } + h.AssertEq(t, metadata.BuildpackDefaultProcessType, "") }) - it("shouldn't set web processes as default", func() { - bpA := testmock.NewMockBuildpack(mockCtrl) - buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) - bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ - Processes: []launch.Process{ - { - Type: "not-web", - Command: "not-web-cmd", - Args: []string{"not-web-arg"}, - Direct: true, - BuildpackID: "A", - Default: false, - }, - { - Type: "web", - Command: "web-cmd", - Args: []string{"web-arg"}, - Direct: true, - BuildpackID: "A", - Default: false, - }, - }, - }, nil) - bpB := testmock.NewMockBuildpack(mockCtrl) - buildpackStore.EXPECT().Lookup("B", "v2").Return(bpB, nil) - bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ - Processes: []launch.Process{ - { - Type: "another-type", - Command: "another-cmd", - Args: []string{"another-arg"}, - Direct: false, - BuildpackID: "B", - }, - { - Type: "other", - Command: "other-cmd", - Args: []string{"other-arg"}, - Direct: true, - BuildpackID: "B", - }, - }, - }, nil) - - metadata, err := builder.Build() - if err != nil { - t.Fatalf("Unexpected error:\n%s\n", err) - } - - if s := cmp.Diff(metadata.Processes, []launch.Process{ - { - Type: "not-web", - Command: "not-web-cmd", - Args: []string{"not-web-arg"}, - Direct: true, - BuildpackID: "A", - Default: false, - }, - { - Type: "web", - Command: "web-cmd", - Args: []string{"web-arg"}, - Direct: true, - BuildpackID: "A", - Default: false, - }, - { - Type: "another-type", - Command: "another-cmd", - Args: []string{"another-arg"}, - Direct: false, - BuildpackID: "B", - Default: false, - }, - { - Type: "other", - Command: "other-cmd", - Args: []string{"other-arg"}, - Direct: true, - BuildpackID: "B", - Default: false, - }, - }); s != "" { - t.Fatalf("Unexpected:\n%s\n", s) - } - }) + when("multiple default process types", func() { + it.Before(func() { + builder.Group.Group = []lifecycle.GroupBuildpack{ + {ID: "A", Version: "v1", API: latestBuildpackAPI.String()}, + {ID: "B", Version: "v2", API: latestBuildpackAPI.String()}, + {ID: "C", Version: "v3", API: latestBuildpackAPI.String()}, + } + }) - when("builpack API < 0.6", func() { - it("sets web processes as default", func() { + it("last default process type wins", func() { bpA := testmock.NewMockBuildpack(mockCtrl) buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ { - Type: "some-type", - Command: "some-cmd", - Args: []string{"some-arg"}, - Direct: true, - BuildpackID: "A", - Default: false, - }, - { - Type: "another-type", - Command: "another-cmd", - Args: []string{"another-arg"}, + Type: "override-type", + Command: "bpA-command", + Args: []string{"bpA-arg"}, Direct: true, BuildpackID: "A", - Default: false, + Default: true, }, }, }, nil) @@ -613,18 +366,26 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ Processes: []launch.Process{ { - Type: "web", - Command: "web-command", - Args: []string{"web-arg"}, + Type: "some-type", + Command: "bpB-command", + Args: []string{"bpB-arg"}, Direct: false, BuildpackID: "B", + Default: true, }, + }, + }, nil) + + bpC := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("C", "v3").Return(bpC, nil) + bpC.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ { - Type: "not-web", - Command: "not-web-cmd", - Args: []string{"not-web-arg"}, - Direct: true, - BuildpackID: "B", + Type: "override-type", + Command: "bpC-command", + Args: []string{"bpC-arg"}, + Direct: false, + BuildpackID: "C", }, }, }, nil) @@ -636,40 +397,209 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { if s := cmp.Diff(metadata.Processes, []launch.Process{ { - Type: "some-type", - Command: "some-cmd", - Args: []string{"some-arg"}, - Direct: true, - BuildpackID: "A", - Default: false, + Type: "override-type", + Command: "bpC-command", + Args: []string{"bpC-arg"}, + Direct: false, + BuildpackID: "C", }, { - Type: "another-type", - Command: "another-cmd", - Args: []string{"another-arg"}, - Direct: true, - BuildpackID: "A", - Default: false, + Type: "some-type", + Command: "bpB-command", + Args: []string{"bpB-arg"}, + Direct: false, + BuildpackID: "B", }, + }); s != "" { + t.Fatalf("Unexpected:\n%s\n", s) + } + h.AssertEq(t, metadata.BuildpackDefaultProcessType, "some-type") + }) + }) + + when("overriding default process type, with a non-default process type", func() { + it.Before(func() { + builder.Group.Group = []lifecycle.GroupBuildpack{ + {ID: "A", Version: "v1", API: latestBuildpackAPI.String()}, + {ID: "B", Version: "v2", API: latestBuildpackAPI.String()}, + {ID: "C", Version: "v3", API: latestBuildpackAPI.String()}, + } + }) + + it("should warn and not set any default process", func() { + bpB := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("A", "v1").Return(bpB, nil) + bpB.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "some-type", + Command: "bpA-command", + Args: []string{"bpA-arg"}, + Direct: false, + BuildpackID: "A", + Default: true, + }, + }, + }, nil) + + bpA := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("B", "v2").Return(bpA, nil) + bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "override-type", + Command: "bpB-command", + Args: []string{"bpB-arg"}, + Direct: true, + BuildpackID: "B", + Default: true, + }, + }, + }, nil) + + bpC := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("C", "v3").Return(bpC, nil) + bpC.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "override-type", + Command: "bpC-command", + Args: []string{"bpC-arg"}, + Direct: false, + BuildpackID: "C", + }, + }, + }, nil) + + metadata, err := builder.Build() + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + if s := cmp.Diff(metadata.Processes, []launch.Process{ { - Type: "web", - Command: "web-command", - Args: []string{"web-arg"}, + Type: "override-type", + Command: "bpC-command", + Args: []string{"bpC-arg"}, Direct: false, - BuildpackID: "B", - Default: true, + BuildpackID: "C", }, { - Type: "not-web", - Command: "not-web-cmd", - Args: []string{"not-web-arg"}, - Direct: true, - BuildpackID: "B", - Default: false, + Type: "some-type", + Command: "bpA-command", + Args: []string{"bpA-arg"}, + Direct: false, + BuildpackID: "A", }, }); s != "" { t.Fatalf("Unexpected:\n%s\n", s) } + + expected := "Warning: redefining the following default process type with a process not marked as default: override-type" + h.AssertStringContains(t, stdout.String(), expected) + + h.AssertEq(t, metadata.BuildpackDefaultProcessType, "") + }) + }) + + when("there is a web process", func() { + when("buildpack API >= 0.6", func() { + it.Before(func() { + builder.Group.Group = []lifecycle.GroupBuildpack{ + {ID: "A", Version: "v1", API: latestBuildpackAPI.String()}, + } + }) + it("shouldn't set it as a default process", func() { + bpA := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) + bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "web", + Command: "web-cmd", + Args: []string{"web-arg"}, + Direct: false, + BuildpackID: "A", + Default: false, + }, + }, + }, nil) + + metadata, err := builder.Build() + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + + if s := cmp.Diff(metadata.Processes, []launch.Process{ + { + Type: "web", + Command: "web-cmd", + Args: []string{"web-arg"}, + Direct: false, + BuildpackID: "A", + Default: false, + }, + }); s != "" { + t.Fatalf("Unexpected:\n%s\n", s) + } + h.AssertEq(t, metadata.BuildpackDefaultProcessType, "") + }) + }) + + when("buildpack api < 0.6", func() { + it.Before(func() { + builder.Group.Group = []lifecycle.GroupBuildpack{ + {ID: "A", Version: "v1", API: "0.5"}, + } + }) + it("should set it as a default process", func() { + bpA := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) + bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "web", + Command: "web-cmd", + Args: []string{"web-arg"}, + Direct: false, + BuildpackID: "A", + Default: false, + }, + { + Type: "not-web", + Command: "not-web-cmd", + Args: []string{"not-web-arg"}, + Direct: true, + BuildpackID: "A", + Default: false, + }, + }, + }, nil) + + metadata, err := builder.Build() + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + + if s := cmp.Diff(metadata.Processes, []launch.Process{ + { + Type: "not-web", + Command: "not-web-cmd", + Args: []string{"not-web-arg"}, + Direct: true, + BuildpackID: "A", + }, + { + Type: "web", + Command: "web-cmd", + Args: []string{"web-arg"}, + Direct: false, + BuildpackID: "A", + }, + }); s != "" { + t.Fatalf("Unexpected:\n%s\n", s) + } + h.AssertEq(t, metadata.BuildpackDefaultProcessType, "web") + }) }) }) }) diff --git a/exporter.go b/exporter.go index 8d506e46e..d6185f893 100644 --- a/exporter.go +++ b/exporter.go @@ -136,7 +136,7 @@ func (e *Exporter) Export(opts ExportOptions) (ExportReport, error) { } } - entrypoint, err := e.entrypoint(buildMD.toLaunchMD(), opts.DefaultProcessType) + entrypoint, err := e.entrypoint(buildMD.toLaunchMD(), opts.DefaultProcessType, buildMD.BuildpackDefaultProcessType) if err != nil { return ExportReport{}, errors.Wrap(err, "determining entrypoint") } @@ -379,36 +379,35 @@ func (e *Exporter) setWorkingDir(opts ExportOptions) error { return opts.WorkingImage.SetWorkingDir(opts.AppDir) } -func (e *Exporter) entrypoint(launchMD launch.Metadata, defaultProcessType string) (string, error) { +func (e *Exporter) entrypoint(launchMD launch.Metadata, userDefaultProcessType, buildpackDefaultProcessType string) (string, error) { if !e.supportsMulticallLauncher() { return launch.LauncherPath, nil } - if defaultProcessType == "" && e.PlatformAPI.Compare(api.MustParse("0.6")) < 0 && len(launchMD.Processes) == 1 { + if userDefaultProcessType == "" && e.PlatformAPI.Compare(api.MustParse("0.6")) < 0 && len(launchMD.Processes) == 1 { // if there is only one process, we set it to the default for platform API < 0.6 e.Logger.Infof("Setting default process type '%s'", launchMD.Processes[0].Type) return launch.ProcessPath(launchMD.Processes[0].Type), nil } - if defaultProcessType != "" { - defaultProcess, ok := launchMD.FindProcessType(defaultProcessType) + if userDefaultProcessType != "" { + defaultProcess, ok := launchMD.FindProcessType(userDefaultProcessType) if !ok { if e.PlatformAPI.Compare(api.MustParse("0.6")) < 0 { - e.Logger.Warn(processTypeWarning(launchMD, defaultProcessType)) + e.Logger.Warn(processTypeWarning(launchMD, userDefaultProcessType)) return launch.LauncherPath, nil } - return "", fmt.Errorf("tried to set %s to default but it doesn't exist", defaultProcessType) + return "", fmt.Errorf("tried to set %s to default but it doesn't exist", userDefaultProcessType) } e.Logger.Infof("Setting default process type '%s'", defaultProcess.Type) return launch.ProcessPath(defaultProcess.Type), nil } - defaultProcess, ok := launchMD.FindLastDefaultProcessType() - if !ok { + if buildpackDefaultProcessType == "" { e.Logger.Info("no default process type") return launch.LauncherPath, nil } - e.Logger.Infof("Setting default process type '%s'", defaultProcess.Type) - return launch.ProcessPath(defaultProcess.Type), nil + e.Logger.Infof("Setting default process type '%s'", buildpackDefaultProcessType) + return launch.ProcessPath(buildpackDefaultProcessType), nil } // processTypes adds diff --git a/exporter_test.go b/exporter_test.go index aa8aa1b0c..60cb7c896 100644 --- a/exporter_test.go +++ b/exporter_test.go @@ -400,6 +400,7 @@ version = "4.5.6" expectedJSON := ` { "bom": null, + "buildpack-default-process-type": "", "buildpacks": [ { "id": "buildpack.id", @@ -457,6 +458,7 @@ version = "4.5.6" } } ], + "buildpack-default-process-type": "", "buildpacks": [ { "id": "buildpack.id", @@ -940,7 +942,7 @@ version = "4.5.6" when("it is set to an existing type", func() { it.Before(func() { opts.DefaultProcessType = "some-process-type" - h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "one-non-default-process", "layers"), opts.LayersDir) + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "metadata-with-no-default", "layers"), opts.LayersDir) }) it("sets the ENTRYPOINT to this process type", func() { @@ -963,7 +965,7 @@ version = "4.5.6" when("it is set to a process type that doesn't exist", func() { it.Before(func() { opts.DefaultProcessType = "some-non-existing-process-type" - h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "one-non-default-process", "layers"), opts.LayersDir) + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "metadata-with-no-default", "layers"), opts.LayersDir) }) it("fails", func() { _, err := exporter.Export(opts) @@ -973,10 +975,11 @@ version = "4.5.6" }) when("-process-type is not set", func() { - when("there are no default=true processes in metadata.toml", func() { + when("buildpack-default-process-type is not set in metadata.toml", func() { it.Before(func() { - h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "one-non-default-process", "layers"), opts.LayersDir) + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "metadata-with-no-default", "layers"), opts.LayersDir) }) + it("send an info message that there is no default process, and sets the ENTRYPOINT to the launcher", func() { _, err := exporter.Export(opts) h.AssertNil(t, err) @@ -985,35 +988,18 @@ version = "4.5.6" }) }) - when("there is a default=true process in metadata.toml", func() { + when("buildpack-default-process-type is set in metadata.toml", func() { it.Before(func() { - h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "multiple-processes-with-default", "layers"), opts.LayersDir) + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "metadata-with-default", "layers"), opts.LayersDir) layerFactory.EXPECT(). ProcessTypesLayer(launch.Metadata{ Processes: []launch.Process{ - { - Type: "some-default-process-type", - Command: "/some/command", - Args: []string{"some", "command", "args"}, - Direct: true, - BuildpackID: "buildpack1.id", - Default: true, - }, - { - Type: "some-second-default-process-type", - Command: "/some/command", - Args: []string{"some", "command", "args"}, - Direct: true, - BuildpackID: "buildpack2.id", - Default: true, - }, { Type: "some-process-type", Command: "/some/command", Args: []string{"some", "command", "args"}, Direct: true, - BuildpackID: "buildpack3.id", - Default: false, + BuildpackID: "buildpack.id", }}, }). DoAndReturn(func(_ launch.Metadata) (layers.Layer, error) { @@ -1021,10 +1007,11 @@ version = "4.5.6" }). AnyTimes() }) - it("sets the last default=true process to be the default", func() { + + it("sets the ENTRYPOINT to this process type", func() { _, err := exporter.Export(opts) h.AssertNil(t, err) - checkEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "process", "some-second-default-process-type"+execExt)) + checkEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "process", "some-process-type"+execExt)) }) it("doesn't set CNB_PROCESS_TYPE", func() { @@ -1041,7 +1028,7 @@ version = "4.5.6" when("platform API < 0.6", func() { it.Before(func() { exporter.PlatformAPI = api.MustParse("0.5") - h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "one-non-default-process", "layers"), opts.LayersDir) + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "metadata-with-no-default", "layers"), opts.LayersDir) }) when("-process-type is set to a process type that doesn't exist", func() { @@ -1068,8 +1055,9 @@ version = "4.5.6" when("platform API < 0.4", func() { it.Before(func() { exporter.PlatformAPI = api.MustParse("0.3") - h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "one-non-default-process", "layers"), opts.LayersDir) + h.RecursiveCopy(t, filepath.Join("testdata", "exporter", "default-process", "metadata-with-no-default", "layers"), opts.LayersDir) }) + when("-process-type is set to an existing process type", func() { it.Before(func() { opts.DefaultProcessType = "some-process-type" diff --git a/metadata.go b/metadata.go index 0ad0f7ef2..b37067a7a 100644 --- a/metadata.go +++ b/metadata.go @@ -16,12 +16,13 @@ const ( ) type BuildMetadata struct { - BOM []BOMEntry `toml:"bom" json:"bom"` - Buildpacks []GroupBuildpack `toml:"buildpacks" json:"buildpacks"` - Labels []Label `toml:"labels" json:"-"` - Launcher LauncherMetadata `toml:"-" json:"launcher"` - Processes []launch.Process `toml:"processes" json:"processes"` - Slices []layers.Slice `toml:"slices" json:"-"` + BOM []BOMEntry `toml:"bom" json:"bom"` + Buildpacks []GroupBuildpack `toml:"buildpacks" json:"buildpacks"` + Labels []Label `toml:"labels" json:"-"` + Launcher LauncherMetadata `toml:"-" json:"launcher"` + Processes []launch.Process `toml:"processes" json:"processes"` + Slices []layers.Slice `toml:"slices" json:"-"` + BuildpackDefaultProcessType string `toml:"buildpack-default-process-type, omitempty" json:"buildpack-default-process-type"` } type LauncherMetadata struct { diff --git a/testdata/exporter/default-process/metadata-with-default/layers/config/metadata.toml b/testdata/exporter/default-process/metadata-with-default/layers/config/metadata.toml new file mode 100644 index 000000000..b66084d31 --- /dev/null +++ b/testdata/exporter/default-process/metadata-with-default/layers/config/metadata.toml @@ -0,0 +1,8 @@ +buildpack-default-process-type = "some-process-type" + +[[processes]] + type = "some-process-type" + direct = true + command = "/some/command" + args = ["some", "command", "args"] + buildpack-id = "buildpack.id" diff --git a/testdata/exporter/default-process/one-non-default-process/layers/config/metadata.toml b/testdata/exporter/default-process/metadata-with-no-default/layers/config/metadata.toml similarity index 100% rename from testdata/exporter/default-process/one-non-default-process/layers/config/metadata.toml rename to testdata/exporter/default-process/metadata-with-no-default/layers/config/metadata.toml diff --git a/testdata/exporter/default-process/multiple-processes-with-default/layers/config/metadata.toml b/testdata/exporter/default-process/multiple-processes-with-default/layers/config/metadata.toml deleted file mode 100644 index 9928a8d12..000000000 --- a/testdata/exporter/default-process/multiple-processes-with-default/layers/config/metadata.toml +++ /dev/null @@ -1,21 +0,0 @@ -[[processes]] - type = "some-default-process-type" - direct = true - command = "/some/command" - args = ["some", "command", "args"] - buildpack-id = "buildpack1.id" - default = true -[[processes]] - type = "some-second-default-process-type" - direct = true - command = "/some/command" - args = ["some", "command", "args"] - buildpack-id = "buildpack2.id" - default = true -[[processes]] - type = "some-process-type" - direct = true - command = "/some/command" - args = ["some", "command", "args"] - buildpack-id = "buildpack3.id" - From 7b24df17bb72f57cfba08d780795cbaba5aaddca Mon Sep 17 00:00:00 2001 From: Yael Harel Date: Fri, 5 Feb 2021 12:26:48 -0500 Subject: [PATCH 10/10] Cosmetic changes Signed-off-by: Daniel Thornton Signed-off-by: Natalie Arellano Signed-off-by: Yael Harel --- builder.go | 32 ++++++---------- builder_test.go | 95 ++++++++++++++++++++++++++++++++++++++++++++++++ exporter_test.go | 17 ++++----- launch/launch.go | 19 +++------- metadata.go | 2 +- 5 files changed, 120 insertions(+), 45 deletions(-) diff --git a/builder.go b/builder.go index 748b97ac2..4928d5879 100644 --- a/builder.go +++ b/builder.go @@ -100,13 +100,9 @@ func (b *Builder) Build() (*BuildMetadata, error) { labels = append(labels, br.Labels...) plan = plan.filter(br.MetRequires) - replacedDefault := processMap.add(br.Processes) - if err != nil { - return nil, err - } + warning := processMap.add(br.Processes) - if replacedDefault != "" { - warning := fmt.Sprintf("Warning: redefining the following default process type with a process not marked as default: %s", replacedDefault) + if warning != "" { if _, err := b.Out.Write([]byte(warning)); err != nil { return nil, err } @@ -217,25 +213,21 @@ func newProcessMap() processMap { // This function adds the processes from listToAdd to processMap // it sets m.defaultType to the last default process -// if a non-default process overrides a default process, it returns its type and unset m.defaultType +// if a non-default process overrides a default process, it returns a warning and unset m.defaultType func (m *processMap) add(listToAdd []launch.Process) string { - result := "" + warning := "" for _, procToAdd := range listToAdd { if procToAdd.Default { m.defaultType = procToAdd.Type - result = "" - } else { - existingProc, ok := m.typeToProcess[procToAdd.Type] - if ok && existingProc.Type == m.defaultType { // existingProc.Default = true - // non-default process overrides a default process - m.defaultType = "" - result = procToAdd.Type - } + warning = "" + } else if procToAdd.Type == m.defaultType { + // non-default process overrides a default process + m.defaultType = "" + warning = fmt.Sprintf("Warning: redefining the following default process type with a process not marked as default: %s\n", procToAdd.Type) } m.typeToProcess[procToAdd.Type] = procToAdd } - - return result + return warning } // list returns a sorted array of processes. @@ -249,9 +241,7 @@ func (m processMap) list() []launch.Process { sort.Strings(keys) result := []launch.Process{} for _, key := range keys { - processWithNoDefault := m.typeToProcess[key] - processWithNoDefault.Default = false // we set the default to false so it won't be part of metadata.toml - result = append(result, processWithNoDefault) + result = append(result, m.typeToProcess[key].NoDefault()) // we set the default to false so it won't be part of metadata.toml } return result } diff --git a/builder_test.go b/builder_test.go index 2d5f1f1c3..d7d4988f7 100644 --- a/builder_test.go +++ b/builder_test.go @@ -721,5 +721,100 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) }) }) + + when("platform api < 0.6", func() { + it.Before(func() { + builder.PlatformAPI = api.MustParse("0.5") + }) + + when("there is a web process", func() { + when("buildpack API >= 0.6", func() { + it.Before(func() { + builder.Group.Group = []lifecycle.GroupBuildpack{ + {ID: "A", Version: "v1", API: latestBuildpackAPI.String()}, + } + }) + it("shouldn't set it as a default process", func() { + bpA := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) + bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "web", + Command: "web-cmd", + Args: []string{"web-arg"}, + Direct: false, + BuildpackID: "A", + Default: false, + }, + }, + }, nil) + + metadata, err := builder.Build() + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + + if s := cmp.Diff(metadata.Processes, []launch.Process{ + { + Type: "web", + Command: "web-cmd", + Args: []string{"web-arg"}, + Direct: false, + BuildpackID: "A", + Default: false, + }, + }); s != "" { + t.Fatalf("Unexpected:\n%s\n", s) + } + h.AssertEq(t, metadata.BuildpackDefaultProcessType, "") + }) + }) + + when("buildpack api < 0.6", func() { + it.Before(func() { + builder.Group.Group = []lifecycle.GroupBuildpack{ + {ID: "A", Version: "v1", API: "0.5"}, + } + }) + + it("shouldn't set it as a default process", func() { + bpA := testmock.NewMockBuildpack(mockCtrl) + buildpackStore.EXPECT().Lookup("A", "v1").Return(bpA, nil) + bpA.EXPECT().Build(gomock.Any(), config).Return(lifecycle.BuildResult{ + Processes: []launch.Process{ + { + Type: "web", + Command: "web-cmd", + Args: []string{"web-arg"}, + Direct: false, + BuildpackID: "A", + Default: false, + }, + }, + }, nil) + + metadata, err := builder.Build() + if err != nil { + t.Fatalf("Unexpected error:\n%s\n", err) + } + + if s := cmp.Diff(metadata.Processes, []launch.Process{ + { + Type: "web", + Command: "web-cmd", + Args: []string{"web-arg"}, + Direct: false, + BuildpackID: "A", + Default: false, + }, + }); s != "" { + t.Fatalf("Unexpected:\n%s\n", s) + } + h.AssertEq(t, metadata.BuildpackDefaultProcessType, "") + }) + }) + }) + }) }) } diff --git a/exporter_test.go b/exporter_test.go index 60cb7c896..bf81a14e0 100644 --- a/exporter_test.go +++ b/exporter_test.go @@ -400,7 +400,6 @@ version = "4.5.6" expectedJSON := ` { "bom": null, - "buildpack-default-process-type": "", "buildpacks": [ { "id": "buildpack.id", @@ -458,7 +457,6 @@ version = "4.5.6" } } ], - "buildpack-default-process-type": "", "buildpacks": [ { "id": "buildpack.id", @@ -486,8 +484,7 @@ version = "4.5.6" "direct": true, "command": "/some/command", "args": ["some", "command", "args"], - "buildpackID": "buildpack.id", - "default": false + "buildpackID": "buildpack.id" } ] } @@ -949,7 +946,7 @@ version = "4.5.6" _, err := exporter.Export(opts) h.AssertNil(t, err) - checkEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "process", "some-process-type"+execExt)) + assertHasEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "process", "some-process-type"+execExt)) }) it("doesn't set CNB_PROCESS_TYPE", func() { @@ -984,7 +981,7 @@ version = "4.5.6" _, err := exporter.Export(opts) h.AssertNil(t, err) assertLogEntry(t, logHandler, "no default process type") - checkEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "lifecycle", "launcher"+execExt)) + assertHasEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "lifecycle", "launcher"+execExt)) }) }) @@ -1011,7 +1008,7 @@ version = "4.5.6" it("sets the ENTRYPOINT to this process type", func() { _, err := exporter.Export(opts) h.AssertNil(t, err) - checkEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "process", "some-process-type"+execExt)) + assertHasEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "process", "some-process-type"+execExt)) }) it("doesn't set CNB_PROCESS_TYPE", func() { @@ -1039,7 +1036,7 @@ version = "4.5.6" _, err := exporter.Export(opts) h.AssertNil(t, err) assertLogEntry(t, logHandler, "default process type 'some-non-existing-process-type' not present in list [some-process-type]") - checkEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "lifecycle", "launcher"+execExt)) + assertHasEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "lifecycle", "launcher"+execExt)) }) }) @@ -1047,7 +1044,7 @@ version = "4.5.6" it("sets the ENTRYPOINT to the only process", func() { _, err := exporter.Export(opts) h.AssertNil(t, err) - checkEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "process", "some-process-type"+execExt)) + assertHasEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "process", "some-process-type"+execExt)) }) }) }) @@ -1323,7 +1320,7 @@ version = "4.5.6" }) } -func checkEntrypoint(t *testing.T, image *fakes.Image, entrypointPath string) { +func assertHasEntrypoint(t *testing.T, image *fakes.Image, entrypointPath string) { ep, err := image.Entrypoint() h.AssertNil(t, err) h.AssertEq(t, len(ep), 1) diff --git a/launch/launch.go b/launch/launch.go index 868bac619..3d0c3e408 100644 --- a/launch/launch.go +++ b/launch/launch.go @@ -11,10 +11,15 @@ type Process struct { Command string `toml:"command" json:"command"` Args []string `toml:"args" json:"args"` Direct bool `toml:"direct" json:"direct"` - Default bool `toml:"default, omitzero" json:"default"` + Default bool `toml:"default,omitempty" json:"default,omitempty"` BuildpackID string `toml:"buildpack-id" json:"buildpackID"` } +func (p Process) NoDefault() Process { + p.Default = false + return p +} + // ProcessPath returns the absolute path to the symlink for a given process type func ProcessPath(pType string) string { return filepath.Join(ProcessDir, pType+exe) @@ -34,18 +39,6 @@ func (m Metadata) FindProcessType(pType string) (Process, bool) { return Process{}, false } -func (m Metadata) FindLastDefaultProcessType() (Process, bool) { - defaultFound := false - var lastDefaultProcess Process - for _, p := range m.Processes { - if p.Default { - lastDefaultProcess = p - defaultFound = true - } - } - return lastDefaultProcess, defaultFound -} - type Buildpack struct { API string `toml:"api"` ID string `toml:"id"` diff --git a/metadata.go b/metadata.go index b37067a7a..848d3fd97 100644 --- a/metadata.go +++ b/metadata.go @@ -22,7 +22,7 @@ type BuildMetadata struct { Launcher LauncherMetadata `toml:"-" json:"launcher"` Processes []launch.Process `toml:"processes" json:"processes"` Slices []layers.Slice `toml:"slices" json:"-"` - BuildpackDefaultProcessType string `toml:"buildpack-default-process-type, omitempty" json:"buildpack-default-process-type"` + BuildpackDefaultProcessType string `toml:"buildpack-default-process-type,omitempty" json:"buildpack-default-process-type,omitempty"` } type LauncherMetadata struct {