Skip to content

Commit

Permalink
Cosmetic changes
Browse files Browse the repository at this point in the history 
Signed-off-by: Daniel Thornton <dwillist@vmware.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Yael Harel <yharel@vmware.com>
  • Loading branch information
Yael Harel authored and Daniel Thornton committed Feb 5, 2021
1 parent d410f33 commit 287714a
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 46 deletions.
34 changes: 12 additions & 22 deletions builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,11 @@ 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
return nil, err
}
}
slices = append(slices, br.Slices...)
Expand Down Expand Up @@ -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.
Expand All @@ -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
}
Expand Down
95 changes: 95 additions & 0 deletions builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, "")
})
})
})
})
})
}
17 changes: 7 additions & 10 deletions exporter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,6 @@ version = "4.5.6"
expectedJSON := `
{
"bom": null,
"buildpack-default-process-type": "",
"buildpacks": [
{
"id": "buildpack.id",
Expand Down Expand Up @@ -458,7 +457,6 @@ version = "4.5.6"
}
}
],
"buildpack-default-process-type": "",
"buildpacks": [
{
"id": "buildpack.id",
Expand Down Expand Up @@ -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"
}
]
}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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))
})
})

Expand All @@ -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() {
Expand Down Expand Up @@ -1039,15 +1036,15 @@ 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))
})
})

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, filepath.Join(rootDir, "cnb", "process", "some-process-type"+execExt))
assertHasEntrypoint(t, fakeAppImage, filepath.Join(rootDir, "cnb", "process", "some-process-type"+execExt))
})
})
})
Expand Down Expand Up @@ -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)
Expand Down
19 changes: 6 additions & 13 deletions launch/launch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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"`
Expand Down
2 changes: 1 addition & 1 deletion metadata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 287714a

Please sign in to comment.