Skip to content

Commit

Permalink
fix: check buildkit frontend version to determine context arg compati…
Browse files Browse the repository at this point in the history
…bility (#486)

Not all older frontends support `--build-arg` and sometimes they do no
specify the version as part of the syntax directive. For example:

```
syntax: docker/dockerfile:experimental
```

Chalk cannot infer anything about the meaning of `experimental`
as its not pinned to any version. Therefore we call the dockerfile
frontend container and check its actual version.
This is possibly not as efficient even if the version is specified
however that would also require some heuristics. For example:

```
syntax: docker/dockerfile:1
```

As `1` can change over time it might not be clear which syntax should
actually be honored. Therefore it is probably a lot simpler to simply
ask it what version it is instead of possibly error-prone version
detection heuristics.
  • Loading branch information
miki725 authored Feb 11, 2025
1 parent 511476d commit 746992c
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 7 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
on `hello.py`. Now chalk will compute nearest `git` repository
and run external tools on it instead.
([#485](https://github.com/crashappsec/chalk/pull/485))
- When `Dockerfile` specifies syntax directive, chalk checks buildkit
frontend version compatibility as older frontends do not support
`--build-context` CLI argument. Passing the flag would fail the
wrapped build and chalk would fallback to vanilla docker build.
More about syntax directive
[here](https://docs.docker.com/reference/dockerfile/#syntax).
([#486](https://github.com/crashappsec/chalk/pull/486))

## 0.5.3

Expand Down
3 changes: 2 additions & 1 deletion src/chalk_common.nim
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ type
default*: Option[LineToken]
error*: string
plus*: bool
minus*: bool #
minus*: bool

LineTokenType* = enum ltOther, ltWord, ltQuoted, ltSpace

Expand Down Expand Up @@ -402,6 +402,7 @@ type
# parsed dockerfile
dfSections*: seq[DockerFileSection]
dfSectionAliases*: OrderedTable[string, DockerFileSection]
dfDirectives*: OrderedTableRef[string, string]

of DockerCmd.push:
foundImage*: string
Expand Down
1 change: 1 addition & 0 deletions src/docker/cmdline.nim
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ proc extractDockerCommand*(self: DockerInvocation): DockerCmd =
self.extractSecrets()
# set any other non-automatically initialized attributes to avoid segfaults
self.addedPlatform = newOrderedTable[string, seq[string]]()
self.dfDirectives = newOrderedTable[string, string]()
of DockerCmd.push:
self.extractImage()
self.extractAllTags()
Expand Down
8 changes: 5 additions & 3 deletions src/docker/dockerfile.nim
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ import "."/[ids]
# HEALTHCHECK NONE
# SHELL ["executable", "parameters"]

const validDockerDirectives = ["syntax", "escape"]

proc fromJson*[T](json: JsonNode): T =
if json == nil or json == newJNull():
nil
Expand Down Expand Up @@ -693,12 +691,13 @@ proc parseHashLine(ctx: DockerParse, line: string) =
ctx.currentEscape = arg.runeAt(0)
tok.errors.add("Escape character is multi-byte... not allowed.")

if name notin validDockerDirectives:
if name notin ["syntax", "escape", "check"]:
return

if name in ctx.directives:
tok.errors.add("Docker directive '" & name & "' has already appeared")

trace("docker: directive " & name & "=" & tok.directive.rawArg)
ctx.directives[name] = tok.directive

proc topLevelCmdParse(s: string): (string, string) =
Expand Down Expand Up @@ -940,6 +939,9 @@ proc evalAndExtractDockerfile*(ctx: DockerInvocation, args: Table[string, string
if len(ctx.foundLabels) != 0:
trace("docker: found labels: " & $(ctx.foundLabels))

for k, v in parse.directives.pairs():
ctx.dfDirectives[k] = v.rawArg

if section == nil:
raise newException(
ValueError,
Expand Down
45 changes: 44 additions & 1 deletion src/docker/exe.nim
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@

import std/[os, sets]
import ".."/[config, util, semver]
import "."/[ids]

var
dockerExeLocation = ""
dockerClientVersion = parseVersion("0")
dockerServerVersion = parseVersion("0")
buildXVersion = parseVersion("0")
buildKitVersion = parseVersion("0")
frontendVersion = none(Version)

proc getDockerExeLocation*(): string =
once:
Expand Down Expand Up @@ -214,6 +216,44 @@ proc getBuildKitVersion*(ctx: DockerInvocation): Version =
dumpExOnDebug()
return buildKitVersion

proc getFrontendVersion*(ctx: DockerInvocation): Option[Version] =
## get buildkit frontend version
## * returns none if frontend is not specified
## and default buildkit version will be used
## * returns "0" if the version could not be determined
## * return actual version otherwise
result = frontendVersion
once:
if ctx == nil or ctx.cmd != DockerCmd.build:
return
let syntax = ctx.dfDirectives.getOrDefault(
"syntax",
ctx.foundBuildArgs.getOrDefault("BUILDKIT_SYNTAX", ""),
)
if syntax == "":
return
try:
let
image = parseImage(syntax)
output = runDockerGetEverything(@[
"run",
"--rm",
$image,
"-version",
])
if output.exitCode != 0:
trace("docker: could not get buildkint frontend versioni " & output.getStdErr())
frontendVersion = some(parseVersion("0"))
else:
let version = getVersionFromLine(output.stdOut)
trace("docker: frontend version: " & $version)
frontendVersion = some(version)
return frontendVersion
except:
dumpExOnDebug()
frontendVersion = some(parseVersion("0"))
return frontendVersion

var dockerAuth = newJObject()
proc getDockerAuthConfig*(): JsonNode =
once:
Expand All @@ -236,11 +276,14 @@ proc supportsBuildContextFlag*(ctx: DockerInvocation): bool =
# https://github.com/moby/buildkit/releases/tag/v0.10.0
# which in turn is included in docker server version >= 23
# https://docs.docker.com/engine/release-notes/23.0/#2300
let frontend = getFrontendVersion(ctx)
return (
getDockerClientVersion() >= parseVersion("21") and
getDockerServerVersion() >= parseVersion("23") and
getBuildXVersion() >= parseVersion("0.8") and
ctx.getBuildKitVersion() >= parseVersion("0.10")
ctx.getBuildKitVersion() >= parseVersion("0.10") and
(frontend.isNone() or
frontend.get() >= parseVersion("1.4"))
)

proc supportsCopyChmod*(): bool =
Expand Down
4 changes: 2 additions & 2 deletions src/semver.nim
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ proc parseVersion*(version: string): Version =
patch = 0
suffix = ""
let
name = version.strip(chars={'V', 'v'}, trailing=false).strip(chars={',', '.', '-', '+'})
name = version.strip(chars={'V', 'v'}, trailing=false).strip(chars={',', '.', '-', '+', '/'})
sections = name.split({'-', '+'}, maxsplit=1)
parts = sections[0].split('.')
case len(parts):
Expand Down Expand Up @@ -87,7 +87,7 @@ proc normalize*(self: Version): string =
return s

proc getVersionFromLine*(line: string): Version =
for word in line.splitWhitespace():
for word in line.split(seps = Whitespace + {'/', ','}):
if '.' in word:
try:
return parseVersion(word)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# syntax=docker/dockerfile:experimental
FROM alpine as base
FROM base
RUN adduser -D testuser
Expand Down
14 changes: 14 additions & 0 deletions tests/unit/test_semver.nim
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,20 @@ proc main() =
doAssert parseVersion("0.1") >= parseVersion("0.1")
doAssert parseVersion("0.1.0") >= parseVersion("0.1")

doAssert $(getVersionFromLine(
"/bin/dockerfile-frontend " &
"github.com/moby/buildkit/frontend/dockerfile/cmd/dockerfile-frontend " &
"dockerfile/1.2.1-labs " &
"bf5e780c5e125bb97942ead83ff3c20705e8e8c9"
)) == "1.2.1-labs"
doAssert $(getVersionFromLine(
"github.com/docker/buildx 0.19.2 1fc5647dc281ca3c2ad5b451aeff2dce84f1dc49"
)) == "0.19.2"
doAssert $(getVersionFromLine(
"Docker version 27.3.1, build ce1223035a"
)) == "27.3.1"


static:
main()
main()

0 comments on commit 746992c

Please sign in to comment.