Skip to content

Commit

Permalink
Merge pull request #2498 from tonistiigi/warnings-updates
Browse files Browse the repository at this point in the history
Updates to warnings handling
  • Loading branch information
tonistiigi authored Dec 15, 2021
2 parents 4a1cbd7 + 872518e commit 76234fa
Show file tree
Hide file tree
Showing 23 changed files with 939 additions and 298 deletions.
438 changes: 335 additions & 103 deletions api/services/control/control.pb.go

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion api/services/control/control.proto
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,11 @@ message VertexLog {
message VertexWarning {
string vertex = 1 [(gogoproto.customtype) = "github.com/opencontainers/go-digest.Digest", (gogoproto.nullable) = false];
int64 level = 2;
bytes msg = 3;
bytes short = 3;
repeated bytes detail = 4;
string url = 5;
pb.SourceInfo info = 6;
repeated pb.Range ranges = 7;
}

message BytesMessage {
Expand Down
28 changes: 25 additions & 3 deletions client/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,19 @@ func testWarnings(t *testing.T, sb integration.Sandbox) {
return nil, errors.Wrap(err, "failed to solve")
}

require.NoError(t, c.Warn(ctx, dgst, "this is warning"))
require.NoError(t, c.Warn(ctx, dgst, "this is warning", client.WarnOpts{
Level: 3,
SourceInfo: &pb.SourceInfo{
Filename: "mydockerfile",
Data: []byte("filedata"),
},
Range: []*pb.Range{
{Start: pb.Position{Line: 2}, End: pb.Position{Line: 4}},
},
Detail: [][]byte{[]byte("this is detail"), []byte("and more detail")},
URL: "https://example.com",
}))

return r, nil
}

Expand Down Expand Up @@ -229,11 +241,21 @@ func testWarnings(t *testing.T, sb integration.Sandbox) {

w := warnings[0]

require.Equal(t, "this is warning", string(w.Message))
require.Equal(t, 1, w.Level)
require.Equal(t, "this is warning", string(w.Short))
require.Equal(t, 2, len(w.Detail))
require.Equal(t, "this is detail", string(w.Detail[0]))
require.Equal(t, "and more detail", string(w.Detail[1]))
require.Equal(t, "https://example.com", w.URL)
require.Equal(t, 3, w.Level)
_, ok := vertexes[w.Vertex]
require.True(t, ok)

require.Equal(t, "mydockerfile", w.SourceInfo.Filename)
require.Equal(t, "filedata", string(w.SourceInfo.Data))
require.Equal(t, 1, len(w.Range))
require.Equal(t, int32(2), w.Range[0].Start.Line)
require.Equal(t, int32(4), w.Range[0].End.Line)

checkAllReleasable(t, c, sb, true)
}

Expand Down
11 changes: 8 additions & 3 deletions client/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package client
import (
"time"

"github.com/moby/buildkit/solver/pb"
digest "github.com/opencontainers/go-digest"
)

Expand Down Expand Up @@ -35,9 +36,13 @@ type VertexLog struct {
}

type VertexWarning struct {
Vertex digest.Digest
Level int
Message []byte
Vertex digest.Digest
Level int
Short []byte
Detail [][]byte
URL string
SourceInfo *pb.SourceInfo
Range []*pb.Range
}

type SolveStatus struct {
Expand Down
18 changes: 18 additions & 0 deletions client/llb/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,24 @@ func (def *Definition) FromPB(x *pb.Definition) {
}
}

func (def *Definition) Head() (digest.Digest, error) {
if len(def.Def) == 0 {
return "", nil
}

last := def.Def[len(def.Def)-1]

var pop pb.Op
if err := (&pop).Unmarshal(last); err != nil {
return "", err
}
if len(pop.Inputs) == 0 {
return "", nil
}

return pop.Inputs[0].Digest, nil
}

func WriteTo(def *Definition, w io.Writer) error {
b, err := def.ToPB().Marshal()
if err != nil {
Expand Down
10 changes: 7 additions & 3 deletions client/solve.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,13 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
}
for _, v := range resp.Warnings {
s.Warnings = append(s.Warnings, &VertexWarning{
Vertex: v.Vertex,
Level: int(v.Level),
Message: v.Msg,
Vertex: v.Vertex,
Level: int(v.Level),
Short: v.Short,
Detail: v.Detail,
URL: v.Url,
SourceInfo: v.Info,
Range: v.Ranges,
})
}
if statusChan != nil {
Expand Down
6 changes: 5 additions & 1 deletion control/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,11 @@ func (c *Controller) Status(req *controlapi.StatusRequest, stream controlapi.Con
sr.Warnings = append(sr.Warnings, &controlapi.VertexWarning{
Vertex: v.Vertex,
Level: int64(v.Level),
Msg: v.Message,
Short: v.Short,
Detail: v.Detail,
Info: v.SourceInfo,
Ranges: v.Range,
Url: v.URL,
})
}
if err := stream.SendMsg(&sr); err != nil {
Expand Down
3 changes: 2 additions & 1 deletion examples/build-using-dockerfile/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ func action(clicontext *cli.Context) error {
c = cn
}
// not using shared context to not disrupt display but let is finish reporting errors
return progressui.DisplaySolveStatus(context.TODO(), "", c, os.Stdout, ch)
_, err = progressui.DisplaySolveStatus(context.TODO(), "", c, os.Stdout, ch)
return err
})
eg.Go(func() error {
if err := loadDockerTar(pipeR); err != nil {
Expand Down
35 changes: 35 additions & 0 deletions frontend/dockerfile/builder/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,11 @@ func Build(ctx context.Context, c client.Client) (*client.Result, error) {
return nil, errors.Wrapf(err, "failed to marshal local source")
}

defVtx, err := def.Head()
if err != nil {
return nil, err
}

var sourceMap *llb.SourceMap

eg, ctx2 := errgroup.WithContext(ctx)
Expand Down Expand Up @@ -426,6 +431,7 @@ func Build(ctx context.Context, c client.Client) (*client.Result, error) {
err = wrapSource(err, sourceMap, el.Location)
}
}()

st, img, err := dockerfile2llb.Dockerfile2LLB(ctx, dtDockerfile, dockerfile2llb.ConvertOpt{
Target: opts[keyTarget],
MetaResolver: c,
Expand All @@ -449,6 +455,12 @@ func Build(ctx context.Context, c client.Client) (*client.Result, error) {
LLBCaps: &caps,
SourceMap: sourceMap,
Hostname: opts[keyHostname],
Warn: func(msg, url string, detail [][]byte, location *parser.Range) {
if i != 0 {
return
}
c.Warn(ctx, defVtx, msg, warnOpts(sourceMap, location, detail, url))
},
})

if err != nil {
Expand Down Expand Up @@ -767,6 +779,29 @@ func scopeToSubDir(c *llb.State, fileop bool, dir string) *llb.State {
return &bc
}

func warnOpts(sm *llb.SourceMap, r *parser.Range, detail [][]byte, url string) client.WarnOpts {
opts := client.WarnOpts{Level: 1, Detail: detail, URL: url}
if r == nil {
return opts
}
opts.SourceInfo = &pb.SourceInfo{
Data: sm.Data,
Filename: sm.Filename,
Definition: sm.Definition.ToPB(),
}
opts.Range = []*pb.Range{{
Start: pb.Position{
Line: int32(r.Start.Line),
Character: int32(r.Start.Character),
},
End: pb.Position{
Line: int32(r.End.Line),
Character: int32(r.End.Character),
},
}}
return opts
}

func wrapSource(err error, sm *llb.SourceMap, ranges []parser.Range) error {
if sm == nil {
return err
Expand Down
5 changes: 5 additions & 0 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ type ConvertOpt struct {
ContextLocalName string
SourceMap *llb.SourceMap
Hostname string
Warn func(short, url string, detail [][]byte, location *parser.Range)
}

func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State, *Image, error) {
Expand All @@ -91,6 +92,10 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
return nil, nil, err
}

for _, w := range dockerfile.Warnings {
opt.Warn(w.Short, w.URL, w.Detail, w.Location)
}

proxyEnv := proxyEnvFromBuildArgs(opt.BuildArgs)

stages, metaArgs, err := instructions.Parse(dockerfile.AST)
Expand Down
29 changes: 21 additions & 8 deletions frontend/dockerfile/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,15 +269,27 @@ func newNodeFromLine(line string, d *directives, comments []string) (*Node, erro
type Result struct {
AST *Node
EscapeToken rune
Warnings []string
Warnings []Warning
}

type Warning struct {
Short string
Detail [][]byte
URL string
Location *Range
}

// PrintWarnings to the writer
func (r *Result) PrintWarnings(out io.Writer) {
if len(r.Warnings) == 0 {
return
}
fmt.Fprintf(out, strings.Join(r.Warnings, "\n")+"\n")
for _, w := range r.Warnings {
fmt.Fprintf(out, "[WARNING]: %s\n", w.Short)
}
if len(r.Warnings) > 0 {
fmt.Fprintf(out, "[WARNING]: Empty continuation lines will become errors in a future release.\n")
}
}

// Parse reads lines from a Reader, parses the lines into an AST and returns
Expand All @@ -288,7 +300,7 @@ func Parse(rwc io.Reader) (*Result, error) {
root := &Node{StartLine: -1}
scanner := bufio.NewScanner(rwc)
scanner.Split(scanLines)
warnings := []string{}
warnings := []Warning{}
var comments []string

var err error
Expand Down Expand Up @@ -341,7 +353,12 @@ func Parse(rwc io.Reader) (*Result, error) {
}

if hasEmptyContinuationLine {
warnings = append(warnings, "[WARNING]: Empty continuation line found in:\n "+line)
warnings = append(warnings, Warning{
Short: "Empty continuation line found in: " + line,
Detail: [][]byte{[]byte("Empty continuation lines will become errors in a future release")},
URL: "https://github.com/moby/moby/pull/33719",
Location: &Range{Start: Position{Line: currentLine}, End: Position{Line: currentLine}},
})
}

child, err := newNodeFromLine(line, d, comments)
Expand Down Expand Up @@ -384,10 +401,6 @@ func Parse(rwc io.Reader) (*Result, error) {
comments = nil
}

if len(warnings) > 0 {
warnings = append(warnings, "[WARNING]: Empty continuation lines will become errors in a future release.")
}

if root.StartLine < 0 {
return nil, withLocation(errors.New("file with no instructions"), currentLine, 0)
}
Expand Down
10 changes: 5 additions & 5 deletions frontend/dockerfile/parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,11 @@ RUN indented \
result, err := Parse(dockerfile)
require.NoError(t, err)
warnings := result.Warnings
require.Equal(t, 3, len(warnings))
require.Contains(t, warnings[0], "Empty continuation line found in")
require.Contains(t, warnings[0], "RUN something following more")
require.Contains(t, warnings[1], "RUN another thing")
require.Contains(t, warnings[2], "will become errors in a future release")
require.Equal(t, 2, len(warnings))
require.Contains(t, warnings[0].Short, "Empty continuation line found in")
require.Contains(t, warnings[0].Short, "RUN something following more")
require.Contains(t, warnings[1].Short, "RUN another thing")
require.Contains(t, string(warnings[0].Detail[0]), "will become errors in a future release")
}

func TestParseReturnsScannerErrors(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion frontend/frontend.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ type Frontend interface {
type FrontendLLBBridge interface {
Solve(ctx context.Context, req SolveRequest, sid string) (*Result, error)
ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt) (digest.Digest, []byte, error)
Warn(ctx context.Context, dgst digest.Digest, level int, msg string) error
Warn(ctx context.Context, dgst digest.Digest, msg string, opts WarnOpts) error
}

type SolveRequest = gw.SolveRequest

type CacheOptionsEntry = gw.CacheOptionsEntry

type WarnOpts = gw.WarnOpts
10 changes: 9 additions & 1 deletion frontend/gateway/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type Client interface {
BuildOpts() BuildOpts
Inputs(ctx context.Context) (map[string]llb.State, error)
NewContainer(ctx context.Context, req NewContainerRequest) (Container, error)
Warn(ctx context.Context, dgst digest.Digest, msg string) error
Warn(ctx context.Context, dgst digest.Digest, msg string, opts WarnOpts) error
}

// NewContainerRequest encapsulates the requirements for a client to define a
Expand Down Expand Up @@ -134,3 +134,11 @@ type BuildOpts struct {
LLBCaps apicaps.CapSet
Caps apicaps.CapSet
}

type WarnOpts struct {
Level int
SourceInfo *pb.SourceInfo
Range []*pb.Range
Detail [][]byte
URL string
}
4 changes: 2 additions & 2 deletions frontend/gateway/forwarder/forward.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ func (c *bridgeClient) discard(err error) {
}
}

func (c *bridgeClient) Warn(ctx context.Context, dgst digest.Digest, msg string) error {
return c.FrontendLLBBridge.Warn(ctx, dgst, 1, msg)
func (c *bridgeClient) Warn(ctx context.Context, dgst digest.Digest, msg string, opts client.WarnOpts) error {
return c.FrontendLLBBridge.Warn(ctx, dgst, msg, opts)
}

func (c *bridgeClient) NewContainer(ctx context.Context, req client.NewContainerRequest) (client.Container, error) {
Expand Down
8 changes: 7 additions & 1 deletion frontend/gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,13 @@ func (lbf *llbBridgeForwarder) ReleaseContainer(ctx context.Context, in *pb.Rele
}

func (lbf *llbBridgeForwarder) Warn(ctx context.Context, in *pb.WarnRequest) (*pb.WarnResponse, error) {
err := lbf.llbBridge.Warn(ctx, in.Digest, int(in.Level), string(in.Message))
err := lbf.llbBridge.Warn(ctx, in.Digest, string(in.Short), frontend.WarnOpts{
Level: int(in.Level),
SourceInfo: in.Info,
Range: in.Ranges,
Detail: in.Detail,
URL: in.Url,
})
if err != nil {
return nil, err
}
Expand Down
12 changes: 8 additions & 4 deletions frontend/gateway/grpcclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,11 +298,15 @@ func (c *grpcClient) requestForRef(ref client.Reference) (*pb.SolveRequest, erro
return req, nil
}

func (c *grpcClient) Warn(ctx context.Context, dgst digest.Digest, msg string) error {
func (c *grpcClient) Warn(ctx context.Context, dgst digest.Digest, msg string, opts client.WarnOpts) error {
_, err := c.client.Warn(ctx, &pb.WarnRequest{
Digest: dgst,
Level: 1,
Message: []byte(msg),
Digest: dgst,
Level: int64(opts.Level),
Short: []byte(msg),
Info: opts.SourceInfo,
Ranges: opts.Range,
Detail: opts.Detail,
Url: opts.URL,
})
return err
}
Expand Down
Loading

0 comments on commit 76234fa

Please sign in to comment.