Skip to content

Commit 2e8e38a

Browse files
nikpivkinnikpivkin
authored
fix: improve conversion of image config to Dockerfile (#8308)
Signed-off-by: nikpivkin <[email protected]>
1 parent f258fd5 commit 2e8e38a

File tree

2 files changed

+160
-37
lines changed

2 files changed

+160
-37
lines changed

‎pkg/fanal/analyzer/imgconf/dockerfile/dockerfile.go

+60-37
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"strings"
88

9+
v1 "github.com/google/go-containerregistry/pkg/v1"
910
"golang.org/x/xerrors"
1011

1112
"github.com/aquasecurity/trivy/pkg/fanal/analyzer"
@@ -49,76 +50,98 @@ func (a *historyAnalyzer) Analyze(ctx context.Context, input analyzer.ConfigAnal
4950
if input.Config == nil {
5051
return nil, nil
5152
}
53+
54+
fsys := mapfs.New()
55+
if err := fsys.WriteVirtualFile(
56+
"Dockerfile", imageConfigToDockerfile(input.Config), 0600); err != nil {
57+
return nil, xerrors.Errorf("mapfs write error: %w", err)
58+
}
59+
60+
misconfs, err := a.scanner.Scan(ctx, fsys)
61+
if err != nil {
62+
return nil, xerrors.Errorf("history scan error: %w", err)
63+
}
64+
// The result should be a single element as it passes one Dockerfile.
65+
if len(misconfs) != 1 {
66+
return nil, nil
67+
}
68+
69+
return &analyzer.ConfigAnalysisResult{
70+
Misconfiguration: &misconfs[0],
71+
}, nil
72+
}
73+
74+
func imageConfigToDockerfile(cfg *v1.ConfigFile) []byte {
5275
dockerfile := new(bytes.Buffer)
5376
var userFound bool
54-
baseLayerIndex := image.GuessBaseImageIndex(input.Config.History)
55-
for i := baseLayerIndex + 1; i < len(input.Config.History); i++ {
56-
h := input.Config.History[i]
77+
baseLayerIndex := image.GuessBaseImageIndex(cfg.History)
78+
for i := baseLayerIndex + 1; i < len(cfg.History); i++ {
79+
h := cfg.History[i]
5780
var createdBy string
5881
switch {
5982
case strings.HasPrefix(h.CreatedBy, "/bin/sh -c #(nop)"):
6083
// Instruction other than RUN
6184
createdBy = strings.TrimPrefix(h.CreatedBy, "/bin/sh -c #(nop)")
6285
case strings.HasPrefix(h.CreatedBy, "/bin/sh -c"):
6386
// RUN instruction
64-
createdBy = strings.ReplaceAll(h.CreatedBy, "/bin/sh -c", "RUN")
87+
createdBy = buildRunInstruction(createdBy)
6588
case strings.HasSuffix(h.CreatedBy, "# buildkit"):
6689
// buildkit instructions
6790
// COPY ./foo /foo # buildkit
6891
// ADD ./foo.txt /foo.txt # buildkit
6992
// RUN /bin/sh -c ls -hl /foo # buildkit
7093
createdBy = strings.TrimSuffix(h.CreatedBy, "# buildkit")
71-
if strings.HasPrefix(h.CreatedBy, "RUN /bin/sh -c") {
72-
createdBy = strings.ReplaceAll(createdBy, "RUN /bin/sh -c", "RUN")
73-
}
94+
createdBy = buildRunInstruction(createdBy)
7495
case strings.HasPrefix(h.CreatedBy, "USER"):
7596
// USER instruction
7697
createdBy = h.CreatedBy
7798
userFound = true
7899
case strings.HasPrefix(h.CreatedBy, "HEALTHCHECK"):
79100
// HEALTHCHECK instruction
80-
var interval, timeout, startPeriod, retries, command string
81-
if input.Config.Config.Healthcheck.Interval != 0 {
82-
interval = fmt.Sprintf("--interval=%s ", input.Config.Config.Healthcheck.Interval)
101+
createdBy = buildHealthcheckInstruction(cfg.Config.Healthcheck)
102+
default:
103+
for _, prefix := range []string{"ARG", "ENV", "ENTRYPOINT"} {
104+
strings.HasPrefix(h.CreatedBy, prefix)
105+
createdBy = h.CreatedBy
106+
break
83107
}
84-
if input.Config.Config.Healthcheck.Timeout != 0 {
85-
timeout = fmt.Sprintf("--timeout=%s ", input.Config.Config.Healthcheck.Timeout)
86-
}
87-
if input.Config.Config.Healthcheck.StartPeriod != 0 {
88-
startPeriod = fmt.Sprintf("--startPeriod=%s ", input.Config.Config.Healthcheck.StartPeriod)
89-
}
90-
if input.Config.Config.Healthcheck.Retries != 0 {
91-
retries = fmt.Sprintf("--retries=%d ", input.Config.Config.Healthcheck.Retries)
92-
}
93-
command = strings.Join(input.Config.Config.Healthcheck.Test, " ")
94-
command = strings.ReplaceAll(command, "CMD-SHELL", "CMD")
95-
createdBy = fmt.Sprintf("HEALTHCHECK %s%s%s%s%s", interval, timeout, startPeriod, retries, command)
96108
}
97109
dockerfile.WriteString(strings.TrimSpace(createdBy) + "\n")
98110
}
99111

100-
if !userFound && input.Config.Config.User != "" {
101-
user := fmt.Sprintf("USER %s", input.Config.Config.User)
112+
if !userFound && cfg.Config.User != "" {
113+
user := fmt.Sprintf("USER %s", cfg.Config.User)
102114
dockerfile.WriteString(user)
103115
}
104116

105-
fsys := mapfs.New()
106-
if err := fsys.WriteVirtualFile("Dockerfile", dockerfile.Bytes(), 0600); err != nil {
107-
return nil, xerrors.Errorf("mapfs write error: %w", err)
117+
return dockerfile.Bytes()
118+
}
119+
120+
func buildRunInstruction(s string) string {
121+
pos := strings.Index(s, "/bin/sh -c")
122+
if pos == -1 {
123+
return s
108124
}
125+
return "RUN" + s[pos+len("/bin/sh -c"):]
126+
}
109127

110-
misconfs, err := a.scanner.Scan(ctx, fsys)
111-
if err != nil {
112-
return nil, xerrors.Errorf("history scan error: %w", err)
128+
func buildHealthcheckInstruction(health *v1.HealthConfig) string {
129+
var interval, timeout, startPeriod, retries, command string
130+
if health.Interval != 0 {
131+
interval = fmt.Sprintf("--interval=%s ", health.Interval)
113132
}
114-
// The result should be a single element as it passes one Dockerfile.
115-
if len(misconfs) != 1 {
116-
return nil, nil
133+
if health.Timeout != 0 {
134+
timeout = fmt.Sprintf("--timeout=%s ", health.Timeout)
117135
}
118-
119-
return &analyzer.ConfigAnalysisResult{
120-
Misconfiguration: &misconfs[0],
121-
}, nil
136+
if health.StartPeriod != 0 {
137+
startPeriod = fmt.Sprintf("--startPeriod=%s ", health.StartPeriod)
138+
}
139+
if health.Retries != 0 {
140+
retries = fmt.Sprintf("--retries=%d ", health.Retries)
141+
}
142+
command = strings.Join(health.Test, " ")
143+
command = strings.ReplaceAll(command, "CMD-SHELL", "CMD")
144+
return fmt.Sprintf("HEALTHCHECK %s%s%s%s%s", interval, timeout, startPeriod, retries, command)
122145
}
123146

124147
func (a *historyAnalyzer) Required(_ types.OS) bool {

‎pkg/fanal/analyzer/imgconf/dockerfile/dockerfile_test.go

+100
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package dockerfile
22

33
import (
4+
"bytes"
45
"context"
56
"testing"
67
"time"
78

89
v1 "github.com/google/go-containerregistry/pkg/v1"
10+
"github.com/moby/buildkit/frontend/dockerfile/parser"
911
"github.com/stretchr/testify/assert"
1012
"github.com/stretchr/testify/require"
1113

@@ -343,3 +345,101 @@ func Test_historyAnalyzer_Analyze(t *testing.T) {
343345
})
344346
}
345347
}
348+
349+
func Test_ImageConfigToDockerfile(t *testing.T) {
350+
tests := []struct {
351+
name string
352+
input *v1.ConfigFile
353+
expected string
354+
}{
355+
{
356+
name: "run instruction with build args",
357+
input: &v1.ConfigFile{
358+
History: []v1.History{
359+
{
360+
CreatedBy: "RUN |1 pkg=curl /bin/sh -c apk add $pkg # buildkit",
361+
},
362+
},
363+
},
364+
expected: "RUN apk add $pkg\n",
365+
},
366+
{
367+
name: "healthcheck instruction with system's default shell",
368+
input: &v1.ConfigFile{
369+
History: []v1.History{
370+
{
371+
CreatedBy: "HEALTHCHECK &{[\"CMD-SHELL\" \"curl -f http://localhost/ || exit 1\"] \"5m0s\" \"3s\" \"1s\" \"5s\" '\\x03'}",
372+
},
373+
},
374+
Config: v1.Config{
375+
Healthcheck: &v1.HealthConfig{
376+
Test: []string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"},
377+
Interval: time.Minute * 5,
378+
Timeout: time.Second * 3,
379+
StartPeriod: time.Second * 1,
380+
Retries: 3,
381+
},
382+
},
383+
},
384+
expected: "HEALTHCHECK --interval=5m0s --timeout=3s --startPeriod=1s --retries=3 CMD curl -f http://localhost/ || exit 1\n",
385+
},
386+
{
387+
name: "healthcheck instruction exec arguments directly",
388+
input: &v1.ConfigFile{
389+
History: []v1.History{
390+
{
391+
CreatedBy: "HEALTHCHECK &{[\"CMD\" \"curl\" \"-f\" \"http://localhost/\" \"||\" \"exit 1\"] \"0s\" \"0s\" \"0s\" \"0s\" '\x03'}",
392+
},
393+
},
394+
Config: v1.Config{
395+
Healthcheck: &v1.HealthConfig{
396+
Test: []string{"CMD", "curl", "-f", "http://localhost/", "||", "exit 1"},
397+
Retries: 3,
398+
},
399+
},
400+
},
401+
expected: "HEALTHCHECK --retries=3 CMD curl -f http://localhost/ || exit 1\n",
402+
},
403+
{
404+
name: "nop, no run instruction",
405+
input: &v1.ConfigFile{
406+
History: []v1.History{
407+
{
408+
CreatedBy: "/bin/sh -c #(nop) ARG TAG=latest",
409+
},
410+
},
411+
},
412+
expected: "ARG TAG=latest\n",
413+
},
414+
{
415+
name: "buildkit metadata instructions",
416+
input: &v1.ConfigFile{
417+
History: []v1.History{
418+
{
419+
CreatedBy: "ARG TAG=latest",
420+
},
421+
{
422+
CreatedBy: "ENV TAG=latest",
423+
},
424+
{
425+
CreatedBy: "ENTRYPOINT [\"/bin/sh\" \"-c\" \"echo test\"]",
426+
},
427+
},
428+
},
429+
expected: `ARG TAG=latest
430+
ENV TAG=latest
431+
ENTRYPOINT ["/bin/sh" "-c" "echo test"]
432+
`,
433+
},
434+
}
435+
436+
for _, tt := range tests {
437+
t.Run(tt.name, func(t *testing.T) {
438+
got := imageConfigToDockerfile(tt.input)
439+
_, err := parser.Parse(bytes.NewReader(got))
440+
require.NoError(t, err)
441+
442+
assert.Equal(t, tt.expected, string(got))
443+
})
444+
}
445+
}

0 commit comments

Comments
 (0)