Skip to content

Commit 1981eb1

Browse files
tonistiigitonistiigi
authored
Merge pull request moby#4601 from tonistiigi/0131-fix-validate-nil
Add more validations for nil values
2 parents 3436b4d + 5d7d85f commit 1981eb1

File tree

12 files changed

+446
-10
lines changed

12 files changed

+446
-10
lines changed

‎client/client_test.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){
213213
}
214214

215215
func TestIntegration(t *testing.T) {
216-
testIntegration(t, allTests...)
216+
testIntegration(t, append(allTests, validationTests...)...)
217217
}
218218

219219
func testIntegration(t *testing.T, funcs ...func(t *testing.T, sb integration.Sandbox)) {
@@ -8913,6 +8913,7 @@ cat <<EOF > $BUILDKIT_SCAN_DESTINATION/spdx.json
89138913
EOF
89148914
`
89158915
img.Config.Cmd = []string{"/bin/sh", "-c", cmd}
8916+
img.Platform = p
89168917
config, err := json.Marshal(img)
89178918
if err != nil {
89188919
return nil, errors.Wrapf(err, "failed to marshal image config")
@@ -9191,6 +9192,7 @@ cat <<EOF > $BUILDKIT_SCAN_DESTINATION/spdx.json
91919192
EOF
91929193
`
91939194
img.Config.Cmd = []string{"/bin/sh", "-c", cmd}
9195+
img.Platform = p
91949196
config, err := json.Marshal(img)
91959197
if err != nil {
91969198
return nil, errors.Wrapf(err, "failed to marshal image config")
@@ -9243,6 +9245,7 @@ EOF
92439245

92449246
var img ocispecs.Image
92459247
img.Config.Cmd = []string{"/bin/sh", "-c", "cat /greeting"}
9248+
img.Platform = p
92469249
config, err := json.Marshal(img)
92479250
if err != nil {
92489251
return nil, errors.Wrapf(err, "failed to marshal image config")

‎client/validation_test.go

+319
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,319 @@
1+
package client
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"io"
7+
"testing"
8+
9+
"github.com/moby/buildkit/client/llb"
10+
"github.com/moby/buildkit/exporter/containerimage/exptypes"
11+
"github.com/moby/buildkit/frontend/gateway/client"
12+
sppb "github.com/moby/buildkit/sourcepolicy/pb"
13+
"github.com/moby/buildkit/util/testutil/integration"
14+
"github.com/moby/buildkit/util/testutil/workers"
15+
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
16+
"github.com/stretchr/testify/require"
17+
)
18+
19+
var validationTests = []func(t *testing.T, sb integration.Sandbox){
20+
testValidateNullConfig,
21+
testValidateInvalidConfig,
22+
testValidatePlatformsEmpty,
23+
testValidatePlatformsInvalid,
24+
testValidateSourcePolicy,
25+
}
26+
27+
func testValidateNullConfig(t *testing.T, sb integration.Sandbox) {
28+
requiresLinux(t)
29+
workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter)
30+
31+
ctx := sb.Context()
32+
33+
c, err := New(ctx, sb.Address())
34+
require.NoError(t, err)
35+
defer c.Close()
36+
37+
b := func(ctx context.Context, c client.Client) (*client.Result, error) {
38+
def, err := llb.Scratch().Marshal(ctx)
39+
if err != nil {
40+
return nil, err
41+
}
42+
43+
res, err := c.Solve(ctx, client.SolveRequest{
44+
Evaluate: true,
45+
Definition: def.ToPB(),
46+
})
47+
if err != nil {
48+
return nil, err
49+
}
50+
res.AddMeta("containerimage.config", []byte("null"))
51+
return res, nil
52+
}
53+
54+
_, err = c.Build(ctx, SolveOpt{
55+
Exports: []ExportEntry{
56+
{
57+
Type: ExporterOCI,
58+
Output: fixedWriteCloser(nopWriteCloser{io.Discard}),
59+
},
60+
},
61+
}, "", b, nil)
62+
require.Error(t, err)
63+
require.Contains(t, err.Error(), "invalid null image config for export")
64+
}
65+
66+
func testValidateInvalidConfig(t *testing.T, sb integration.Sandbox) {
67+
requiresLinux(t)
68+
workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter)
69+
70+
ctx := sb.Context()
71+
72+
c, err := New(ctx, sb.Address())
73+
require.NoError(t, err)
74+
defer c.Close()
75+
76+
b := func(ctx context.Context, c client.Client) (*client.Result, error) {
77+
def, err := llb.Scratch().Marshal(ctx)
78+
if err != nil {
79+
return nil, err
80+
}
81+
82+
res, err := c.Solve(ctx, client.SolveRequest{
83+
Evaluate: true,
84+
Definition: def.ToPB(),
85+
})
86+
if err != nil {
87+
return nil, err
88+
}
89+
var img ocispecs.Image
90+
img.Platform = ocispecs.Platform{
91+
Architecture: "amd64",
92+
}
93+
dt, err := json.Marshal(img)
94+
if err != nil {
95+
return nil, err
96+
}
97+
res.AddMeta("containerimage.config", dt)
98+
return res, nil
99+
}
100+
101+
_, err = c.Build(ctx, SolveOpt{
102+
Exports: []ExportEntry{
103+
{
104+
Type: ExporterOCI,
105+
Output: fixedWriteCloser(nopWriteCloser{io.Discard}),
106+
},
107+
},
108+
}, "", b, nil)
109+
require.Error(t, err)
110+
require.Contains(t, err.Error(), "invalid image config: os and architecture must be specified together")
111+
}
112+
113+
func testValidatePlatformsEmpty(t *testing.T, sb integration.Sandbox) {
114+
requiresLinux(t)
115+
workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter)
116+
117+
ctx := sb.Context()
118+
119+
c, err := New(ctx, sb.Address())
120+
require.NoError(t, err)
121+
defer c.Close()
122+
123+
b := func(ctx context.Context, c client.Client) (*client.Result, error) {
124+
def, err := llb.Scratch().Marshal(ctx)
125+
if err != nil {
126+
return nil, err
127+
}
128+
129+
res, err := c.Solve(ctx, client.SolveRequest{
130+
Evaluate: true,
131+
Definition: def.ToPB(),
132+
})
133+
if err != nil {
134+
return nil, err
135+
}
136+
res.AddMeta("refs.platforms", []byte("null"))
137+
return res, nil
138+
}
139+
140+
_, err = c.Build(ctx, SolveOpt{
141+
Exports: []ExportEntry{
142+
{
143+
Type: ExporterOCI,
144+
Output: fixedWriteCloser(nopWriteCloser{io.Discard}),
145+
},
146+
},
147+
}, "", b, nil)
148+
require.Error(t, err)
149+
require.Contains(t, err.Error(), "invalid empty platforms index for exporter")
150+
}
151+
152+
func testValidatePlatformsInvalid(t *testing.T, sb integration.Sandbox) {
153+
requiresLinux(t)
154+
workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter)
155+
156+
ctx := sb.Context()
157+
158+
c, err := New(ctx, sb.Address())
159+
require.NoError(t, err)
160+
defer c.Close()
161+
162+
tcases := []struct {
163+
name string
164+
value []exptypes.Platform
165+
exp string
166+
}{
167+
{
168+
name: "emptyID",
169+
value: []exptypes.Platform{{}},
170+
exp: "invalid empty platform key for exporter",
171+
},
172+
{
173+
name: "missingOS",
174+
value: []exptypes.Platform{
175+
{
176+
ID: "foo",
177+
},
178+
},
179+
exp: "invalid platform value",
180+
},
181+
}
182+
183+
for _, tc := range tcases {
184+
t.Run(tc.name, func(t *testing.T) {
185+
b := func(ctx context.Context, c client.Client) (*client.Result, error) {
186+
def, err := llb.Scratch().Marshal(ctx)
187+
if err != nil {
188+
return nil, err
189+
}
190+
191+
res, err := c.Solve(ctx, client.SolveRequest{
192+
Evaluate: true,
193+
Definition: def.ToPB(),
194+
})
195+
if err != nil {
196+
return nil, err
197+
}
198+
199+
dt, err := json.Marshal(exptypes.Platforms{Platforms: tc.value})
200+
if err != nil {
201+
return nil, err
202+
}
203+
204+
res.AddMeta("refs.platforms", dt)
205+
return res, nil
206+
}
207+
208+
_, err = c.Build(ctx, SolveOpt{
209+
Exports: []ExportEntry{
210+
{
211+
Type: ExporterOCI,
212+
Output: fixedWriteCloser(nopWriteCloser{io.Discard}),
213+
},
214+
},
215+
}, "", b, nil)
216+
require.Error(t, err)
217+
require.Contains(t, err.Error(), tc.exp)
218+
})
219+
}
220+
}
221+
222+
func testValidateSourcePolicy(t *testing.T, sb integration.Sandbox) {
223+
requiresLinux(t)
224+
225+
ctx := sb.Context()
226+
227+
c, err := New(ctx, sb.Address())
228+
require.NoError(t, err)
229+
defer c.Close()
230+
231+
tcases := []struct {
232+
name string
233+
value *sppb.Policy
234+
exp string
235+
}{
236+
// this condition fails on marshaling atm
237+
// {
238+
// name: "nilrule",
239+
// value: &sppb.Policy{
240+
// Rules: []*sppb.Rule{nil},
241+
// },
242+
// exp: "",
243+
// },
244+
{
245+
name: "nilselector",
246+
value: &sppb.Policy{
247+
Rules: []*sppb.Rule{
248+
{
249+
Action: sppb.PolicyAction_CONVERT,
250+
},
251+
},
252+
},
253+
exp: "invalid nil selector in policy",
254+
},
255+
{
256+
name: "emptyaction",
257+
value: &sppb.Policy{
258+
Rules: []*sppb.Rule{
259+
{
260+
Action: sppb.PolicyAction(9000),
261+
Selector: &sppb.Selector{
262+
Identifier: "docker-image://docker.io/library/alpine:latest",
263+
},
264+
},
265+
},
266+
},
267+
exp: "unknown type",
268+
},
269+
{
270+
name: "nilupdates",
271+
value: &sppb.Policy{
272+
Rules: []*sppb.Rule{
273+
{
274+
Action: sppb.PolicyAction_CONVERT,
275+
Selector: &sppb.Selector{
276+
Identifier: "docker-image://docker.io/library/alpine:latest",
277+
},
278+
},
279+
},
280+
},
281+
exp: "missing destination for convert rule",
282+
},
283+
}
284+
285+
for _, tc := range tcases {
286+
t.Run(tc.name, func(t *testing.T) {
287+
var viaFrontend bool
288+
289+
b := func(ctx context.Context, c client.Client) (*client.Result, error) {
290+
def, err := llb.Image("alpine").Marshal(ctx)
291+
if err != nil {
292+
return nil, err
293+
}
294+
295+
req := client.SolveRequest{
296+
Evaluate: true,
297+
Definition: def.ToPB(),
298+
}
299+
if viaFrontend {
300+
req.SourcePolicies = []*sppb.Policy{
301+
tc.value,
302+
}
303+
}
304+
return c.Solve(ctx, req)
305+
}
306+
307+
_, err = c.Build(ctx, SolveOpt{
308+
SourcePolicy: tc.value,
309+
}, "", b, nil)
310+
require.Error(t, err)
311+
require.Contains(t, err.Error(), tc.exp)
312+
313+
viaFrontend = true
314+
_, err = c.Build(ctx, SolveOpt{}, "", b, nil)
315+
require.Error(t, err)
316+
require.Contains(t, err.Error(), tc.exp)
317+
})
318+
}
319+
}

‎control/control.go

+3
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,9 @@ func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (*
420420

421421
var cacheImports []frontend.CacheOptionsEntry
422422
for _, im := range req.Cache.Imports {
423+
if im == nil {
424+
continue
425+
}
423426
cacheImports = append(cacheImports, frontend.CacheOptionsEntry{
424427
Type: im.Type,
425428
Attrs: im.Attrs,

‎exporter/containerimage/exptypes/parse.go

+14
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,18 @@ func ParsePlatforms(meta map[string][]byte) (Platforms, error) {
1717
return Platforms{}, errors.Wrapf(err, "failed to parse platforms passed to provenance processor")
1818
}
1919
}
20+
if len(ps.Platforms) == 0 {
21+
return Platforms{}, errors.Errorf("invalid empty platforms index for exporter")
22+
}
23+
for i, p := range ps.Platforms {
24+
if p.ID == "" {
25+
return Platforms{}, errors.Errorf("invalid empty platform key for exporter")
26+
}
27+
if p.Platform.OS == "" || p.Platform.Architecture == "" {
28+
return Platforms{}, errors.Errorf("invalid platform value %v for exporter", p.Platform)
29+
}
30+
ps.Platforms[i].Platform = platforms.Normalize(p.Platform)
31+
}
2032
return ps, nil
2133
}
2234

@@ -36,6 +48,8 @@ func ParsePlatforms(meta map[string][]byte) (Platforms, error) {
3648
OSFeatures: img.OSFeatures,
3749
Variant: img.Variant,
3850
}
51+
} else if img.OS != "" || img.Architecture != "" {
52+
return Platforms{}, errors.Errorf("invalid image config: os and architecture must be specified together")
3953
}
4054
}
4155
p = platforms.Normalize(p)

0 commit comments

Comments
 (0)