My schema'd dockerfile fails parsing

[chris13524]

Expected behavior

Arbitrary syntax should be valid when the # syntax directive is enabled.

Actual behavior

Given a Cargo.toml like this:

# syntax = denzp/cargo-wharf-frontend:v0.1.0-alpha.2

[package]
name = "testpkg"
version = "0.1.0"

[package.metadata.wharf.output]
env = { RUST_LOG = "info" }

And this skaffold.yaml:

---
apiVersion: skaffold/v1beta16
kind: Config
metadata:
  name: myconfig
build:
  artifacts:
  - image: myimg
    context: .
    docker:
      dockerfile: Cargo.toml
  local:
    useBuildkit: true

You get this error:

$ skaffold dev
Listing files to watch...
 - myimg
listing files: unable to evaluate build args: removing unused default args: parsing dockerfile: Syntax error - can't find = in "{". Must be of the form: name=value

If you comment out env = { RUST_LOG = "info" } from Cargo.toml then run skaffold run then the build will work.

I believe this test case was missed by #5441.

I'm not familiar with the code at all, but it seems like some actual parsing of the file contents is being done prior to the syntax directive being checked.

Information

• Skaffold version: v1.24.1
• Operating system: Ubuntu 20.04
• Installed via: skaffold.dev

[briandealwis]

5441 was to avoid erroring on Dockerfiles that use some buildkit extensions.

For now I think you're better off using the custom builder to invoke Docker directly. Our docker builders assume a relatively well-formed Dockerfile as they parse the Dockerfile to obtain information such as from ADD and COPY. I'm honestly not sure what we can do in this situation.

[chris13524]

I will use the custom builder, thanks!

5441 says it fixed #5398 which is specifically using with the cargo-wharf-frontend BuildKit frontend as an alternative to the regular Dockerfile syntax. The test cases it added also are testing non-Dockerfile syntax.

Iterating further on this issue, I'm thinking that #5441 should not have been merged. Firstly, it only skips validation for readCopyCmdsFromDockerfile() and not for the function that is causing my error, filterUnusedBuildArgs(). But ultimately I think that if Skaffold is expecting to be able to parse the file as a Dockerfile and extract valuable path information for usage, then it should not accept a "Dockerfile" with custom syntax at all. So instead of skipping validation it should emit an error telling the user to use the custom builder and provide their pathing information manually. This is because in the case of something like cargo-wharf, the "Dockerfile" (actually Cargo.toml) doesn't contain any viable pathing information anyway.

Maybe it would be possible to hook directly into BuildKit to reliably determine the source paths used during the build?

[MarlonGamez]

@chris13524 thanks for the suggestions! I think this needs to be discussed a bit further within the team, so I'll bring up your points and see what kind of conclusion we can come to around this :)

[chris13524]

Maybe it would be possible to hook directly into BuildKit to reliably determine the source paths used during the build?

There is a new proposal for a BuildKit feature here that I think would address this idea/issue: https://github.com/moby/buildkit/issues/2269