fix(build): Validate Dockerfile using buildkit parser

[Syedowais312]

This PR improves Dockerfile detection in builder_dockerfile.

Previously, detection relied only on filename matching which
could incorrectly accept non-Dockerfiles or reject valid ones.

The implementation now:

• Parses the file using the BuildKit Dockerfile parser
• Ensures a valid AST exists
• Verifies at least one FROM instruction is present
• Keeps warnings when filename suggests Dockerfile but syntax invalid

This makes Dockerfile handling reliable and removes dependence
on naming conventions.

[Syedowais312]

ping for review.
CC: @nderjung @craciunoiuc

[Copilot]

Pull request overview

This PR enhances Dockerfile detection in the builder_dockerfile by replacing naive filename-based validation with proper syntax parsing using the BuildKit Dockerfile parser. The implementation now validates Dockerfiles by parsing their syntax and verifying they contain at least one FROM instruction, making the detection more reliable and independent of naming conventions.

Changes:

• Replaced simple filename check with BuildKit parser-based validation
• Added validation to ensure Dockerfile has valid syntax and at least one FROM instruction
• Added warning when a file looks like a Dockerfile but has invalid syntax

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Errors from isValidDockerfile are silently discarded in the isDockerfile function. When an error occurs (e.g., file not found, permission denied), the function returns false without distinguishing between "not a Dockerfile" and "error reading the file". This makes debugging difficult and can hide real IO errors. Consider returning the error to the caller or at least logging it.

[craciunoiuc]

Hey copilot found some stuff, at least one comment is valid

Can you go through them and fix the ones that make sense?

[Syedowais312]

Hey copilot found some stuff, at least one comment is valid

Can you go through them and fix the ones that make sense?

Yup, I have made the changes

[Syedowais312]

I was wondering if it would be great to add test cases for this validation logic?

Should I add them in this PR or create a follow-up?

CC: @craciunoiuc

[craciunoiuc]

Sure, add the tests in the same PR as a separate commit

[jedevc]

🤔

This makes the kraftfile significantly more difficult to parse + handle. To know how we'd do this, we need to bundle the entire buildkit dockerfile parser?

I hit this when working on unikraft-cloud/x#143. Potentially, instead of having isDockerfile, we should instead keep all the existing heuristics for backwards compat, and introduce a new type field, which allows the user to explicitly define what to do.

[Syedowais312]

@ijaidev thanks for the detailed feedback.

My intention was mainly to avoid false negatives for valid Dockerfiles with non-standard names, but I agree builder selection probably shouldn’t depend on deep parsing semantics.

Introducing an explicit type while keeping the current heuristics for backwards compatibility sounds cleaner and preserves existing behavior.
I’ve also been thinking about a few alternative approaches and would appreciate your thoughts on them as well. I’m exploring those now and will come back shortly.

[Syedowais312]

After exploring further, I think the cleanest approach is to introduce an explicit source-type field in the kraftfile rootfs spec, along the lines of what you suggested:

 rootfs: 
    source: ./my-custom-named-file 
    source-type: dockerfile

Does this direction match what you had in mind, or would you prefer the type to live at a different level (e.g. part of a generic rootfs “kind” instead of dockerfile-specific)?