Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build driver experimental flag checks #1825

Merged
merged 1 commit into from
Nov 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/build/dockerfile-generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"

"get.porter.sh/porter/pkg/config"
"get.porter.sh/porter/pkg/experimental"
"get.porter.sh/porter/pkg/manifest"
"get.porter.sh/porter/pkg/mixin"
"get.porter.sh/porter/pkg/templates"
Expand All @@ -26,6 +27,7 @@ func TestPorter_buildDockerfile(t *testing.T) {

c := config.NewTestConfig(t)
c.Data.BuildDriver = driver
c.SetExperimentalFlags(experimental.FlagBuildDrivers)
tmpl := templates.NewTemplates(c.Config)
configTpl, err := tmpl.GetManifest()
require.Nil(t, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func TestLoadHierarchicalConfig(t *testing.T) {

t.Run("build-driver from config", func(t *testing.T) {
os.Unsetenv("PORTER_BUILD_DRIVER")
defer os.Unsetenv("PORTER_EXPERIMENTAL")
defer os.Unsetenv("PORTER_BUILD_DRIVER")

c := config.NewTestConfig(t)
c.SetHomeDir("/root/.porter")
Expand Down
10 changes: 10 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,3 +242,13 @@ func (c *Config) IsFeatureEnabled(flag experimental.FeatureFlags) bool {
func (c *Config) SetExperimentalFlags(flags experimental.FeatureFlags) {
c.experimental = &flags
}

// GetBuildDriver determines the correct build driver to use, taking
// into account experimental flags.
// Use this instead of Config.Data.BuildDriver directly.
func (c *Config) GetBuildDriver() string {
if c.IsFeatureEnabled(experimental.FlagBuildDrivers) {
return c.Data.BuildDriver
}
return BuildDriverDocker
}
9 changes: 9 additions & 0 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,12 @@ func TestConfigExperimentalFlags(t *testing.T) {
assert.True(t, c.IsFeatureEnabled(experimental.FlagBuildDrivers))
})
}

func TestConfig_GetBuildDriver(t *testing.T) {
c := NewTestConfig(t)
c.Data.BuildDriver = BuildDriverBuildkit
require.Equal(t, BuildDriverDocker, c.GetBuildDriver(), "Default to docker when experimental is false, even when a build driver is set")

c.SetExperimentalFlags(experimental.FlagBuildDrivers)
require.Equal(t, BuildDriverBuildkit, c.GetBuildDriver(), "Use the specified driver when the build driver feature is enabled")
}
1 change: 1 addition & 0 deletions pkg/config/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ type Data struct {

// BuildDriver is the driver to use when building bundles.
// Available values are: docker, buildkit.
// Do not use directly, use Config.GetBuildDriver.
BuildDriver string `mapstructure:"build-driver"`

// RuntimeDriver is the driver to use when executing bundles.
Expand Down
4 changes: 2 additions & 2 deletions pkg/porter/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (o *BuildOptions) Validate(p *Porter) error {
}

if o.Driver == "" {
o.Driver = p.Data.BuildDriver
o.Driver = p.GetBuildDriver()
}
if !stringSliceContains(BuildDriverAllowedValues, o.Driver) {
return errors.Errorf("invalid --driver value %s", o.Driver)
Expand All @@ -68,7 +68,7 @@ func (p *Porter) Build(opts BuildOptions) error {
opts.Apply(p.Context)

if p.Debug {
fmt.Fprintf(p.Err, "Using %s build driver\n", p.Data.BuildDriver)
fmt.Fprintf(p.Err, "Using %s build driver\n", p.GetBuildDriver())
}

// Start with a fresh .cnab directory before building
Expand Down
14 changes: 5 additions & 9 deletions pkg/porter/porter.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
cnabprovider "get.porter.sh/porter/pkg/cnab/provider"
"get.porter.sh/porter/pkg/config"
"get.porter.sh/porter/pkg/credentials"
"get.porter.sh/porter/pkg/experimental"
"get.porter.sh/porter/pkg/manifest"
"get.porter.sh/porter/pkg/mixin"
"get.porter.sh/porter/pkg/parameters"
Expand Down Expand Up @@ -137,14 +136,11 @@ func (p *Porter) LoadManifestFrom(file string) error {
// NewBuilder creates a Builder based on the current configuration.
func (p *Porter) GetBuilder() build.Builder {
if p.builder == nil {
if p.IsFeatureEnabled(experimental.FlagBuildDrivers) {
switch p.Config.Data.BuildDriver {
case config.BuildDriverBuildkit:
p.builder = buildkit.NewBuilder(p.Context)
case config.BuildDriverDocker:
p.builder = docker.NewBuilder(p.Context)
}
} else {
switch p.GetBuildDriver() {
case config.BuildDriverBuildkit:
p.builder = buildkit.NewBuilder(p.Context)
case config.BuildDriverDocker:
default:
p.builder = docker.NewBuilder(p.Context)
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/templates/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (t *Templates) GetDockerignore() ([]byte, error) {

// GetDockerfileTemplate returns a Dockerfile.tmpl file for use in new bundles.
func (t *Templates) GetDockerfileTemplate() ([]byte, error) {
tmpl := fmt.Sprintf("templates/create/template.%s.Dockerfile", t.Data.BuildDriver)
tmpl := fmt.Sprintf("templates/create/template.%s.Dockerfile", t.GetBuildDriver())
return t.fs.ReadFile(tmpl)
}

Expand All @@ -75,6 +75,6 @@ func (t *Templates) GetSchema() ([]byte, error) {

// GetDockerfile returns the default Dockerfile for invocation images.
func (t *Templates) GetDockerfile() ([]byte, error) {
tmpl := fmt.Sprintf("templates/build/%s.Dockerfile", t.Data.BuildDriver)
tmpl := fmt.Sprintf("templates/build/%s.Dockerfile", t.GetBuildDriver())
return t.fs.ReadFile(tmpl)
}
18 changes: 16 additions & 2 deletions pkg/templates/templates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"testing"

"get.porter.sh/porter/pkg/config"
"get.porter.sh/porter/pkg/experimental"
"get.porter.sh/porter/pkg/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -37,12 +39,24 @@ func TestTemplates_GetDockerfile(t *testing.T) {
for _, driver := range testcases {
c := config.NewTestConfig(t)
c.Data.BuildDriver = driver
c.SetExperimentalFlags(experimental.FlagBuildDrivers)
tmpl := NewTemplates(c.Config)

gotTmpl, err := tmpl.GetDockerfile()
require.NoError(t, err)

wantTmpl, _ := ioutil.ReadFile(fmt.Sprintf("./templates/build/%s.Dockerfile", driver))
assert.Equal(t, wantTmpl, gotTmpl)
test.CompareGoldenFile(t, fmt.Sprintf("./templates/build/%s.Dockerfile", driver), string(gotTmpl))
}

t.Run("experimental flag required to use buildkit", func(t *testing.T) {
// Should use the docker template because the experimental feature isn't set
c := config.NewTestConfig(t)
c.Data.BuildDriver = "buildkit"
tmpl := NewTemplates(c.Config)

gotTmpl, err := tmpl.GetDockerfile()
require.NoError(t, err)

test.CompareGoldenFile(t, "./templates/build/docker.Dockerfile", string(gotTmpl))
})
}