Skip to content

Commit

Permalink
adds lint backend subrequest
Browse files Browse the repository at this point in the history
  • Loading branch information
daghack committed Mar 13, 2024
1 parent fa21e77 commit 3d98105
Show file tree
Hide file tree
Showing 5 changed files with 233 additions and 0 deletions.
5 changes: 5 additions & 0 deletions frontend/dockerfile/builder/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/moby/buildkit/frontend/dockerui"
"github.com/moby/buildkit/frontend/gateway/client"
gwpb "github.com/moby/buildkit/frontend/gateway/pb"
"github.com/moby/buildkit/frontend/subrequests/lint"
"github.com/moby/buildkit/frontend/subrequests/outline"
"github.com/moby/buildkit/frontend/subrequests/targets"
"github.com/moby/buildkit/solver/errdefs"
Expand Down Expand Up @@ -85,6 +86,10 @@ func Build(ctx context.Context, c client.Client) (_ *client.Result, err error) {
ListTargets: func(ctx context.Context) (*targets.List, error) {
return dockerfile2llb.ListTargets(ctx, src.Data)
},
Lint: func(ctx context.Context) (*lint.WarningList, error) {
warnings, err := dockerfile2llb.DockerfileLint(ctx, src.Data, convertOpt)
return &lint.WarningList{Warnings: warnings}, err
},
}); err != nil {
return nil, err
} else if ok {
Expand Down
128 changes: 128 additions & 0 deletions frontend/dockerfile/dockerfile_linter_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
package dockerfile

import (
"context"
"encoding/json"
"os"
"sort"
"testing"

"github.com/containerd/continuity/fs/fstest"
"github.com/moby/buildkit/client"
"github.com/moby/buildkit/frontend/dockerfile/linter"
"github.com/moby/buildkit/frontend/dockerui"
gateway "github.com/moby/buildkit/frontend/gateway/client"
"github.com/moby/buildkit/frontend/subrequests/lint"
"github.com/moby/buildkit/util/testutil/integration"
"github.com/moby/buildkit/util/testutil/workers"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
"github.com/tonistiigi/fsutil"
)

var lintTests = integration.TestFuncs(
testLintRules,
)

func testLintRules(t *testing.T, sb integration.Sandbox) {
integration.SkipOnPlatform(t, "windows")
workers.CheckFeatureCompat(t, sb, workers.FeatureFrontendOutline)
f := getFrontend(t, sb)
if _, ok := f.(*clientFrontend); !ok {
t.Skip("only test with client frontend")
}

dockerfile := []byte(`
# 'First' should produce a lint warning (lowercase stage name)
FROM scratch as First
# 'Run' should produce a lint warning (inconsistent command case)
Run echo "Hello"
# No lint warning
run echo "World"
# No lint warning
RUN touch /file
# No lint warning
FROM scratch as second
# 'First' should produce a lint warning (lowercase flags)
COPY --from=First /file /file
# No lint warning
FROM scratch as target
# No lint warning
COPY --from=second /file /file
`)

dir := integration.Tmpdir(
t,
fstest.CreateFile("Dockerfile", []byte(dockerfile), 0600),
)

c, err := client.New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()

destDir, err := os.MkdirTemp("", "buildkit")
require.NoError(t, err)
defer os.RemoveAll(destDir)

called := false
frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
res, err := c.Solve(ctx, gateway.SolveRequest{
FrontendOpt: map[string]string{
"frontend.caps": "moby.buildkit.frontend.subrequests",
"requestid": "frontend.lint",
"build-arg:BAR": "678",
"target": "target",
},
Frontend: "dockerfile.v0",
})
require.NoError(t, err)

warnings, err := unmarshalLintWarnings(res)
require.NoError(t, err)
require.Len(t, warnings.Warnings, 3)

warningList := warnings.Warnings
sort.Slice(warningList, func(i, j int) bool {
return warningList[i].Location.Start.Line < warningList[j].Location.Start.Line
})

require.Equal(t, 3, warningList[0].Location.Start.Line)
require.Equal(t, linter.LinterRules[linter.RuleStageNameCasing].Short, warningList[0].Short)

require.Equal(t, 6, warningList[1].Location.Start.Line)
require.Equal(t, linter.LinterRules[linter.RuleCommandCasing].Short, warningList[1].Short)

require.Equal(t, 18, warningList[2].Location.Start.Line)
require.Equal(t, linter.LinterRules[linter.RuleFlagCasing].Short, warningList[2].Short)

called = true
return nil, nil
}
_, err = c.Build(sb.Context(), client.SolveOpt{
LocalMounts: map[string]fsutil.FS{
dockerui.DefaultLocalNameDockerfile: dir,
},
}, "", frontend, nil)
require.NoError(t, err)

require.True(t, called)
}

func unmarshalLintWarnings(res *gateway.Result) (*lint.WarningList, error) {
dt, ok := res.Metadata["result.json"]
if !ok {
return nil, errors.Errorf("missing frontend.lint")
}
var warnings lint.WarningList
if err := json.Unmarshal(dt, &warnings); err != nil {
return nil, err
}
return &warnings, nil
}
1 change: 1 addition & 0 deletions frontend/dockerfile/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ func TestIntegration(t *testing.T) {
}))...)
integration.Run(t, heredocTests, opts...)
integration.Run(t, outlineTests, opts...)
integration.Run(t, lintTests, opts...)
integration.Run(t, targetsTests, opts...)

integration.Run(t, reproTests, append(opts,
Expand Down
14 changes: 14 additions & 0 deletions frontend/dockerui/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/moby/buildkit/frontend/gateway/client"
"github.com/moby/buildkit/frontend/subrequests"
"github.com/moby/buildkit/frontend/subrequests/lint"
"github.com/moby/buildkit/frontend/subrequests/outline"
"github.com/moby/buildkit/frontend/subrequests/targets"
"github.com/moby/buildkit/solver/errdefs"
Expand All @@ -19,6 +20,7 @@ const (
type RequestHandler struct {
Outline func(context.Context) (*outline.Outline, error)
ListTargets func(context.Context) (*targets.List, error)
Lint func(context.Context) (*lint.WarningList, error)
AllowOther bool
}

Expand Down Expand Up @@ -55,6 +57,18 @@ func (bc *Client) HandleSubrequest(ctx context.Context, h RequestHandler) (*clie
res, err := targets.ToResult()
return res, true, err
}
case lint.SubrequestLintDefinition.Name:
if f := h.Lint; f != nil {
warnings, err := f(ctx)
if err != nil {
return nil, false, err
}
if warnings == nil {
return nil, true, nil
}
res, err := warnings.ToResult()
return res, true, err
}
}
if h.AllowOther {
return nil, false, nil
Expand Down
85 changes: 85 additions & 0 deletions frontend/subrequests/lint/lint.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package lint

import (
"bytes"
"encoding/json"
"fmt"
"io"
"sort"
"text/tabwriter"

"github.com/moby/buildkit/frontend/dockerfile/linter"
"github.com/moby/buildkit/frontend/gateway/client"
"github.com/moby/buildkit/frontend/subrequests"
)

const RequestLint = "frontend.lint"

var SubrequestLintDefinition = subrequests.Request{
Name: RequestLint,
Version: "1.0.0",
Type: subrequests.TypeRPC,
Description: "Lint a Dockerfile",
Opts: []subrequests.Named{},
Metadata: []subrequests.Named{
{Name: "result.json"},
{Name: "result.txt"},
},
}

type WarningList struct {
Warnings []linter.Warning `json:"warnings"`
}

func (warns WarningList) ToResult() (*client.Result, error) {
res := client.NewResult()
dt, err := json.MarshalIndent(warns, "", " ")
if err != nil {
return nil, err
}
res.AddMeta("result.json", dt)

b := bytes.NewBuffer(nil)
if err := PrintLintViolations(dt, b); err != nil {
return nil, err
}
res.AddMeta("result.txt", b.Bytes())

res.AddMeta("version", []byte(SubrequestLintDefinition.Version))
return res, nil
}

func PrintLintViolations(dt []byte, w io.Writer) error {
var warnings WarningList

if err := json.Unmarshal(dt, &warnings); err != nil {
return err
}

// Group warnings by short
lintWarnings := make(map[string][]linter.Warning)
lintWarningShorts := []string{}
for _, warning := range warnings.Warnings {
if _, ok := lintWarnings[warning.Short]; !ok {
lintWarningShorts = append(lintWarningShorts, warning.Short)
lintWarnings[warning.Short] = []linter.Warning{}
}
lintWarnings[warning.Short] = append(lintWarnings[warning.Short], warning)
}
sort.Strings(lintWarningShorts)

tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0)
fmt.Fprintf(tw, "Lint Warnings\n")
for _, short := range lintWarningShorts {
fmt.Fprintf(tw, "\t%s\n", short)
for _, warning := range lintWarnings[short] {
fmt.Fprintf(tw, "\t\t%s:%d\n", warning.Filename, warning.Location.Start.Line)
for _, line := range warning.Source {
fmt.Fprintf(tw, "\t\t%s\n", line)
}
fmt.Fprintf(tw, "\n")
}
}

return tw.Flush()
}

0 comments on commit 3d98105

Please sign in to comment.