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.

Sign up for GitHub

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

bake: allow BAKE_CMD_CONTEXT builtin var #671

Merged
merged 1 commit into from
Jul 13, 2021
+113 −30
Merged

bake: allow BAKE_CMD_CONTEXT builtin var #671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 19 additions & 5 deletions bake/bake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ func ReadLocalFiles(names []string) ([]File, error) {
return out, nil
}

func ReadTargets(ctx context.Context, files []File, targets, overrides []string) (map[string]*Target, error) {
c, err := ParseFiles(files)
func ReadTargets(ctx context.Context, files []File, targets, overrides []string, defaults map[string]string) (map[string]*Target, error) {
c, err := ParseFiles(files, defaults)
if err != nil {
return nil, err
}
Expand All @@ -86,7 +86,7 @@ func ReadTargets(ctx context.Context, files []File, targets, overrides []string)
return m, nil
}

func ParseFiles(files []File) (_ *Config, err error) {
func ParseFiles(files []File, defaults map[string]string) (_ *Config, err error) {
defer func() {
err = formatHCLError(err, files)
}()
Expand Down Expand Up @@ -120,6 +120,7 @@ func ParseFiles(files []File) (_ *Config, err error) {
if len(fs) > 0 {
if err := hclparser.Parse(hcl.MergeFiles(fs), hclparser.Opt{
LookupVar: os.LookupEnv,
Vars: defaults,
}, &c); err.HasErrors() {
return nil, err
}
Expand All @@ -143,7 +144,7 @@ func dedupeConfig(c Config) Config {
}

func ParseFile(dt []byte, fn string) (*Config, error) {
return ParseFiles([]File{{Data: dt, Name: fn}})
return ParseFiles([]File{{Data: dt, Name: fn}}, nil)
}

func ParseComposeFile(dt []byte, fn string) (*Config, bool, error) {
Expand Down Expand Up @@ -526,6 +527,12 @@ func updateContext(t *build.Inputs, inp *Input) {
t.ContextPath = inp.URL
return
}
if strings.HasPrefix(t.ContextPath, "cwd://") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document cwd protocol

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it is a good idea. I don't want users to write it in the bake file. If they do then they break the remote URL case. They should use the variable and this should be the internal behavior of the variable. We should be able to change that to workdir:// for example and no old files should be broken.

We should document the builtin var, and also more docs for vars/attrs in general.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they break the remote URL

Ah yes right

We should document the builtin var, and also more docs for vars/attrs in general.

👍

return
}
if IsRemoteURL(t.ContextPath) {
return
}
st := llb.Scratch().File(llb.Copy(*inp.State, t.ContextPath, "/"), llb.WithCustomNamef("set context to %s", t.ContextPath))
t.ContextState = &st
}
Expand All @@ -542,7 +549,9 @@ func toBuildOpt(t *Target, inp *Input) (*build.Options, error) {
if t.Context != nil {
contextPath = *t.Context
}
contextPath = path.Clean(contextPath)
if !strings.HasPrefix(contextPath, "cwd://") && !IsRemoteURL(contextPath) {
contextPath = path.Clean(contextPath)
}
dockerfilePath := "Dockerfile"
if t.Dockerfile != nil {
dockerfilePath = *t.Dockerfile
Expand All @@ -569,6 +578,11 @@ func toBuildOpt(t *Target, inp *Input) (*build.Options, error) {
bi.DockerfileInline = *t.DockerfileInline
}
updateContext(&bi, inp)
if strings.HasPrefix(bi.ContextPath, "cwd://") {
bi.ContextPath = path.Clean(strings.TrimPrefix(bi.ContextPath, "cwd://"))
}

t.Context = &bi.ContextPath

bo := &build.Options{
Inputs: bi,
Expand Down
45 changes: 35 additions & 10 deletions bake/bake_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ target "webapp" {
ctx := context.TODO()

t.Run("NoOverrides", func(t *testing.T) {
m, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, nil)
m, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(m))

Expand All @@ -46,7 +46,7 @@ target "webapp" {
})

t.Run("InvalidTargetOverrides", func(t *testing.T) {
_, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"nosuchtarget.context=foo"})
_, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"nosuchtarget.context=foo"}, nil)
require.NotNil(t, err)
require.Equal(t, err.Error(), "could not find any target matching 'nosuchtarget'")
})
Expand All @@ -63,7 +63,7 @@ target "webapp" {
"webapp.args.VAR_FROMENV" + t.Name(),
"webapp.args.VAR_INHERITED=override",
// not overriding VAR_BOTH on purpose
})
}, nil)
require.NoError(t, err)

require.Equal(t, "Dockerfile.webapp", *m["webapp"].Dockerfile)
Expand All @@ -88,31 +88,31 @@ target "webapp" {
m, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{
"webDEP.args.VAR_INHERITED=override",
"webDEP.args.VAR_BOTH=override",
})
}, nil)
require.NoError(t, err)
require.Equal(t, m["webapp"].Args["VAR_INHERITED"], "override")
require.Equal(t, m["webapp"].Args["VAR_BOTH"], "webapp")
})
})

t.Run("ContextOverride", func(t *testing.T) {
_, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.context"})
_, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.context"}, nil)
require.NotNil(t, err)

m, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.context=foo"})
m, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.context=foo"}, nil)
require.NoError(t, err)

require.Equal(t, "foo", *m["webapp"].Context)
})

t.Run("NoCacheOverride", func(t *testing.T) {
m, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.no-cache=false"})
m, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.no-cache=false"}, nil)
require.NoError(t, err)
require.Equal(t, false, *m["webapp"].NoCache)
})

t.Run("PullOverride", func(t *testing.T) {
m, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.pull=false"})
m, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.pull=false"}, nil)
require.NoError(t, err)
require.Equal(t, false, *m["webapp"].Pull)
})
Expand Down Expand Up @@ -172,7 +172,7 @@ target "webapp" {
}
for _, test := range cases {
t.Run(test.name, func(t *testing.T) {
m, err := ReadTargets(ctx, []File{fp}, test.targets, test.overrides)
m, err := ReadTargets(ctx, []File{fp}, test.targets, test.overrides, nil)
test.check(t, m, err)
})
}
Expand Down Expand Up @@ -215,7 +215,7 @@ services:

ctx := context.TODO()

m, err := ReadTargets(ctx, []File{fp, fp2}, []string{"default"}, nil)
m, err := ReadTargets(ctx, []File{fp, fp2}, []string{"default"}, nil, nil)
require.NoError(t, err)

require.Equal(t, 3, len(m))
Expand All @@ -226,3 +226,28 @@ services:
require.Equal(t, "1", m["webapp"].Args["buildno"])
require.Equal(t, "12", m["webapp"].Args["buildno2"])
}

func TestHCLCwdPrefix(t *testing.T) {

fp := File{
Name: "docker-bake.hc",
Data: []byte(
`target "app" {
context = "cwd://foo"
dockerfile = "test"
}`),
}
ctx := context.TODO()
m, err := ReadTargets(ctx, []File{fp}, []string{"app"}, nil, nil)
require.NoError(t, err)

require.Equal(t, 1, len(m))
_, ok := m["app"]
require.True(t, ok)

_, err = TargetsToBuildOpt(m, &Input{})
require.NoError(t, err)

require.Equal(t, "test", *m["app"].Dockerfile)
require.Equal(t, "foo", *m["app"].Context)
}
35 changes: 28 additions & 7 deletions bake/hcl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ func TestHCLMultiFileSharedVariables(t *testing.T) {
c, err := ParseFiles([]File{
{Data: dt, Name: "c1.hcl"},
{Data: dt2, Name: "c2.hcl"},
})
}, nil)
require.NoError(t, err)
require.Equal(t, 1, len(c.Targets))
require.Equal(t, c.Targets[0].Name, "app")
Expand All @@ -288,7 +288,7 @@ func TestHCLMultiFileSharedVariables(t *testing.T) {
c, err = ParseFiles([]File{
{Data: dt, Name: "c1.hcl"},
{Data: dt2, Name: "c2.hcl"},
})
}, nil)
require.NoError(t, err)

require.Equal(t, 1, len(c.Targets))
Expand Down Expand Up @@ -326,7 +326,7 @@ func TestHCLVarsWithVars(t *testing.T) {
c, err := ParseFiles([]File{
{Data: dt, Name: "c1.hcl"},
{Data: dt2, Name: "c2.hcl"},
})
}, nil)
require.NoError(t, err)
require.Equal(t, 1, len(c.Targets))
require.Equal(t, c.Targets[0].Name, "app")
Expand All @@ -338,7 +338,7 @@ func TestHCLVarsWithVars(t *testing.T) {
c, err = ParseFiles([]File{
{Data: dt, Name: "c1.hcl"},
{Data: dt2, Name: "c2.hcl"},
})
}, nil)
require.NoError(t, err)

require.Equal(t, 1, len(c.Targets))
Expand Down Expand Up @@ -483,7 +483,7 @@ func TestHCLMultiFileAttrs(t *testing.T) {
c, err := ParseFiles([]File{
{Data: dt, Name: "c1.hcl"},
{Data: dt2, Name: "c2.hcl"},
})
}, nil)
require.NoError(t, err)
require.Equal(t, 1, len(c.Targets))
require.Equal(t, c.Targets[0].Name, "app")
Expand All @@ -494,7 +494,7 @@ func TestHCLMultiFileAttrs(t *testing.T) {
c, err = ParseFiles([]File{
{Data: dt, Name: "c1.hcl"},
{Data: dt2, Name: "c2.hcl"},
})
}, nil)
require.NoError(t, err)

require.Equal(t, 1, len(c.Targets))
Expand Down Expand Up @@ -589,7 +589,7 @@ services:
c, err := ParseFiles([]File{
{Data: dt, Name: "c1.hcl"},
{Data: dt2, Name: "c2.yml"},
})
}, nil)
require.NoError(t, err)

require.Equal(t, 1, len(c.Targets))
Expand All @@ -599,3 +599,24 @@ services:
require.Equal(t, "dir", *c.Targets[0].Context)
require.Equal(t, "Dockerfile-alternate", *c.Targets[0].Dockerfile)
}

func TestHCLBuiltinVars(t *testing.T) {
dt := []byte(`
target "app" {
context = BAKE_CMD_CONTEXT
dockerfile = "test"
}
`)

c, err := ParseFiles([]File{
{Data: dt, Name: "c1.hcl"},
}, map[string]string{
"BAKE_CMD_CONTEXT": "foo",
})
require.NoError(t, err)

require.Equal(t, 1, len(c.Targets))
require.Equal(t, c.Targets[0].Name, "app")
require.Equal(t, "foo", *c.Targets[0].Context)
require.Equal(t, "test", *c.Targets[0].Dockerfile)
}
15 changes: 13 additions & 2 deletions bake/hclparser/hclparser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

type Opt struct {
LookupVar func(string) (string, bool)
Vars map[string]string
}

type variable struct {
Expand Down Expand Up @@ -178,7 +179,7 @@ func (p *parser) resolveValue(name string) (err error) {
}()

def, ok := p.attrs[name]
if !ok {
if _, builtin := p.opt.Vars[name]; !ok && !builtin {
vr, ok := p.vars[name]
if !ok {
return errors.Errorf("undefined variable %q", name)
Expand All @@ -187,7 +188,10 @@ func (p *parser) resolveValue(name string) (err error) {
}

if def == nil {
val, _ := p.opt.LookupVar(name)
val, ok := p.opt.Vars[name]
if !ok {
val, _ = p.opt.LookupVar(name)
}
vv := cty.StringVal(val)
v = &vv
return
Expand Down Expand Up @@ -243,6 +247,9 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics {
for _, bs := range schema.Blocks {
reserved[bs.Type] = struct{}{}
}
for k := range opt.Vars {
reserved[k] = struct{}{}
}

var defs inputs
if err := gohcl.DecodeBody(b, nil, &defs); err != nil {
Expand Down Expand Up @@ -303,6 +310,10 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics {
}
delete(p.attrs, "function")

for k := range p.opt.Vars {
_ = p.resolveValue(k)
}

for k := range p.attrs {
if err := p.resolveValue(k); err != nil {
if diags, ok := err.(hcl.Diagnostics); ok {
Expand Down
24 changes: 18 additions & 6 deletions commands/bake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,19 @@ func runBake(dockerCli command.Cli, targets []string, in bakeOptions) (err error
}()

var url string
cmdContext := "cwd://"

if len(targets) > 0 {
if bake.IsRemoteURL(targets[0]) {
url = targets[0]
targets = targets[1:]
if len(targets) > 0 {
if bake.IsRemoteURL(targets[0]) {
cmdContext = targets[0]
targets = targets[1:]

}
}
}
}

Expand Down Expand Up @@ -85,6 +93,7 @@ func runBake(dockerCli command.Cli, targets []string, in bakeOptions) (err error

var files []bake.File
var inp *bake.Input

if url != "" {
files, inp, err = bake.ReadRemoteFiles(ctx, dis, url, in.files, printer)
} else {
Expand All @@ -94,7 +103,15 @@ func runBake(dockerCli command.Cli, targets []string, in bakeOptions) (err error
return err
}

m, err := bake.ReadTargets(ctx, files, targets, overrides)
m, err := bake.ReadTargets(ctx, files, targets, overrides, map[string]string{
"BAKE_CMD_CONTEXT": cmdContext,
})
if err != nil {
return err
}

// this function can update target context string from the input so call before printOnly check
bo, err := bake.TargetsToBuildOpt(m, inp)
if err != nil {
return err
}
Expand All @@ -113,11 +130,6 @@ func runBake(dockerCli command.Cli, targets []string, in bakeOptions) (err error
return nil
}

bo, err := bake.TargetsToBuildOpt(m, inp)
if err != nil {
return err
}

resp, err := build.Build(ctx, dis, bo, dockerAPI(dockerCli), dockerCli.ConfigFile(), printer)
if err != nil {
return err
Expand Down