MCO-1333: Validate OCL Containerfiles#6187
Open
eric200428 wants to merge 6 commits into
Open
Conversation
- Add validInstructions map with all valid Dockerfile instructions - Update validateInstruction to check instruction validity before checking args - Remove overly broad 'Unknown instruction:' from isKnownAdvancedSyntaxError - Add test cases for unknown instruction validation
Update go.mod, go.sum, and vendor/ for github.com/openshift/imagebuilder dependency required for Containerfile validation.
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@eric200428: This pull request references MCO-1333 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughUpdates Go toolchain to 1.25.5 and refreshes dependency graph broadly across container runtimes, OpenAPI tooling, sigstore verification, observability, and standard library modules. Adds Containerfile/Dockerfile syntax validation to build request construction and rendering, with tests covering malformed inputs, missing FROM instructions, argument validation, and rejection of unknown instructions. ChangesDependency Updates
Containerfile Syntax Validation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: eric200428 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/controller/build/buildrequest/buildrequest.go`:
- Around line 45-57: The instructionRequirements map currently lacks entries for
several non-empty instructions accepted by validInstructions, so
validateInstruction/validateContainerfileSyntax won't reject bare forms; add
required-argument entries for "CMD", "ENTRYPOINT", "SHELL", "STOPSIGNAL",
"ONBUILD", and "HEALTHCHECK" to instructionRequirements (e.g., "CMD":"a command
or JSON array", "ENTRYPOINT":"a command or JSON array", "SHELL":"a command
array", "STOPSIGNAL":"a signal name or number", "ONBUILD":"a trigger
instruction", and for "HEALTHCHECK" indicate allowed forms such as "NONE" or "a
command and options") so validateInstruction enforces non-empty arguments for
these instructions and fails fast.
- Around line 392-401: The current bypass in validateContainerfileSyntax uses
isKnownAdvancedSyntaxError to match generic substrings and returns nil for any
match, which lets malformed ENV/LABEL pass; update isKnownAdvancedSyntaxError
(and its callers in validateContainerfileSyntax where parser.Parse error is
handled) to switch from deny-list substring matches to an explicit allow-list of
only the specific advanced constructs you intend to accept (e.g.,
heredoc/continued-line tokens or exact parser error codes/messages that
represent valid advanced syntax), and ensure errors from parseNameVal-related
messages (the ENV/LABEL "can't find = in" / "must be of the form: name=value")
are not allowed and still cause Parse to return an error so malformed ENV/LABEL
fail the preflight check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 47750454-b1e3-415a-88bd-481085815484
⛔ Files ignored due to path filters (297)
go.sumis excluded by!**/*.sumvendor/github.com/asaskevich/govalidator/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/arrays.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/converter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/numerics.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/patterns.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/validator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/asaskevich/govalidator/wercker.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v4/context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v4/exponential.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v4/retry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v4/tries.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v5/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v5/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v5/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v5/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v5/backoff.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v5/error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v5/exponential.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v5/retry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v5/ticker.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cenkalti/backoff/v5/timer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cloudflare/circl/ecc/goldilocks/curve.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cloudflare/circl/internal/sha3/xor_unaligned.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cloudflare/circl/sign/ed25519/point.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/cloudflare/circl/sign/sign.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/mount.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/containerd/containerd/api/types/mount.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/containerd/ttrpc/server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/daemon/sdnotify_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/daemon/sdnotify_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/daemon/watchdog.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/dbus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/methods.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/subscription.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/subscription_set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/journal/journal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/journal/journal_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/unit/deserialize.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/unit/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/unit/escape.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/unit/option.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/unit/serialize.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/docker/docker/api/types/versions/compare.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/asymmetric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/cipher/cbc_hmac.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/cipher/concat_kdf.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/cipher/ecdh_es.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/cipher/key_wrap.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/crypter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/encoding.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/json/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/json/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/json/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/json/indent.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/json/scanner.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/json/stream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/json/tags.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/jwe.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/jwk.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/jws.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/opaque.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/shared.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/signing.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-jose/go-jose/v4/symmetric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/.editorconfigis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/CONTRIBUTORS.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/analyzer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/debug.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/fixer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/flatten.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/flatten_name.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/flatten_options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/go.workis excluded by!**/*.work,!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/go.work.sumis excluded by!**/*.sum,!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/internal/debug/debug.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/internal/flatten/normalize/normalize.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/internal/flatten/operations/operations.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/internal/flatten/replace/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/internal/flatten/replace/replace.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/internal/flatten/schutils/flatten_schema.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/internal/flatten/sortref/keys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/internal/flatten/sortref/sort_ref.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/mixin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/analysis/schema.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/.editorconfigis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/CONTRIBUTORS.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/api.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/auth.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/headers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/middleware.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/parsing.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/errors/schema.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonpointer/.cliff.tomlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonpointer/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonpointer/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonpointer/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonpointer/CONTRIBUTORS.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonpointer/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonpointer/NOTICEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonpointer/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonpointer/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonpointer/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonpointer/pointer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonreference/.editorconfigis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonreference/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonreference/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonreference/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonreference/CONTRIBUTORS.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonreference/NOTICEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonreference/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonreference/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonreference/internal/normalize_url.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/jsonreference/reference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/loads/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/loads/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/loads/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/loads/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/loads/CONTRIBUTORS.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/loads/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/loads/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/loads/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/loads/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/loads/loaders.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/loads/options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/loads/spec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/bytestream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/client_auth_info.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/client_operation.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/client_request.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/client_response.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/constants.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/csv.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/csv_options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/discard.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/go.workis excluded by!**/*.work,!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/go.work.sumis excluded by!**/*.sum,!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/headers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/interfaces.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/request.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/statuses.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/text.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/values.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/runtime/xml.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/CONTRIBUTORS.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/cache.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/contact_info.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/debug.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/embed.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/expander.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/external_docs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/header.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/info.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/items.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/license.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/normalizer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/normalizer_nonwindows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/normalizer_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/operation.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/parameter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/path_item.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/paths.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/properties.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/ref.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/resolver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/response.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/responses.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/schema.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/schema_loader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/security_scheme.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/spec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/swagger.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/tag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/url_go19.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/validations.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/spec/xml_object.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/.codecov.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/CONTRIBUTORS.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/bson.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/date.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/default.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/duration.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/format.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/go.workis excluded by!**/*.work,!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/go.work.sumis excluded by!**/*.sum,!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/ifaces.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/internal/bsonlite/codec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/internal/bsonlite/lite.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/mongo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/strfmt/ulid.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/.codecov.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/.mockery.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/cmdutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/cmdutils/cmd_utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/cmdutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/cmdutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/convert.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/convert_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/format.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/sizeof.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/type_constraints.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/convert.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/convert_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils/file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils/path.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/go.workis excluded by!**/*.work,!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/go.work.sumis excluded by!**/*.sum,!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/initialism_index.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonname/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonname/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonname/name_provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonname_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/ifaces/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/ifaces/ifaces.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/ifaces/registry_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/registry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/adapter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/lexer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/ordered_map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/pool.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/concat.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/ordered_map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/errors.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (3)
go.modpkg/controller/build/buildrequest/buildrequest.gopkg/controller/build/buildrequest/buildrequest_test.go
Comment on lines
+392
to
+401
| result, err := parser.Parse(strings.NewReader(containerfile)) | ||
| if err != nil { | ||
| // For rendered templates, advanced syntax is expected | ||
| // Only fail on obvious syntax errors | ||
| if !isKnownAdvancedSyntaxError(err) { | ||
| return fmt.Errorf("failed to parse Containerfile: %w", err) | ||
| } | ||
| // Log but don't fail for advanced syntax | ||
| klog.V(4).Infof("Containerfile uses advanced syntax, skipping detailed parser validation: %v", err) | ||
| return nil |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In github.com/openshift/imagebuilder v1.2.21 dockerfile/parser, what inputs produce the errors "can't find = in" and "must be of the form: name=value"? Do malformed ARG/ENV instructions emit those same messages?
💡 Result:
In the Dockerfile parser used by openshift/imagebuilder (which relies on the standard Dockerfile syntax parser), the errors "can't find = in [X]" and "must be of the form: name=value" occur when the parser encounters a line (typically ENV or LABEL) that it expects to follow the key=value format but fails to find the required equals sign within the expected token [1][2][3]. This error is almost always caused by malformed syntax in ENV or LABEL instructions, rather than ARG [1][4]. Common causes include: 1. Unexpected spaces: If you write ENV KEY = VALUE (with a space before the =), the parser treats the = as a separate token and fails to associate it correctly, often reporting the failure on the next word [2][5]. 2. Trailing characters or comments: If you add a comment at the end of an ENV or LABEL line using #, the parser may attempt to treat that comment as part of the instruction. If it encounters a word in the comment that lacks an =, it will fail [1][6]. 3. Unquoted values with special characters: If an environment variable value contains spaces or special characters that are not properly quoted, the parser may break the line into multiple tokens and attempt to interpret them as key=value pairs, eventually failing when it finds a token without an = [7][3][8]. 4. Line continuation issues: If a backslash () is used incorrectly, or if there is trailing whitespace after the backslash, it can disrupt the parser's view of the next line, leading to parsing errors [1][5]. Regarding malformed ARG/ENV instructions: The error message is characteristic of ENV and LABEL instructions, which strictly enforce the key=value syntax (or the older space-separated syntax, which is often source of confusion when mixed) [9][10][1]. ARG instructions behave differently; while they also support key=value, they are generally more lenient or throw different errors (e.g., "ARG requires exactly one argument") when formatted incorrectly [4]. Therefore, malformed ARG instructions do not typically emit these specific "can't find =" messages [4].
Citations:
- 1: https://stackoverflow.com/questions/68351805/error-response-from-daemon-failed-to-parse-dockerfile-syntax-error-cant-fin
- 2: https://stackoverflow.com/questions/68957046/error-response-from-daemon-failed-to-parse-dockerfile-syntax-error-cant-fin
- 3: Dockerfile scan error on Env entries containing tab characters aquasecurity/trivy#9494
- 4: False positive flagged if quotes used when replacing variables in an ARG, ENV, or LABEL rcjsuen/dockerfile-utils#36
- 5: https://stackoverflow.com/questions/57486507/docker-cant-find-in-env-must-be-of-the-form-name-value
- 6: Comments nested in ENV or LABEL instructions delimited by an equals sign are incorrectly flagged as a syntax error rcjsuen/dockerfile-language-server#167
- 7: Can't build images with labels that contains space inside of quotes moby/moby#31697
- 8: Syntax error - can't find = in "-a." with features enabled devcontainers/cli#454
- 9: manage-dockerfile: use the original form of ENV/LABEL builder#134
- 10: [release-4.3] Bug 1821861: manage-dockerfile: use the original form of ENV/LABEL builder#149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="pkg/controller/build/buildrequest/buildrequest.go"
# Show the exact helper + its callsites around the reported lines
sed -n '350,430p' "$FILE" | cat -n
echo "---- search for isKnownAdvancedSyntaxError definition ----"
rg -n "func isKnownAdvancedSyntaxError|isKnownAdvancedSyntaxError\(" "$FILE"
echo "---- show surrounding definition if found ----"
# Print a bit more around the function definition
# (rg line number output used by awk to slice)
line_def=$(rg -n "func isKnownAdvancedSyntaxError" "$FILE" | head -n1 | cut -d: -f1 || true)
if [[ -n "${line_def:-}" ]]; then
start=$((line_def-40))
end=$((line_def+80))
sed -n "${start},${end}p" "$FILE" | cat -n
fiRepository: openshift/machine-config-operator
Length of output: 7899
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="pkg/controller/build/buildrequest/buildrequest.go"
echo "---- locate instructionRequirements ----"
rg -n "instructionRequirements" "$FILE"
echo "---- show around instructionRequirements ----"
line=$(rg -n "instructionRequirements" "$FILE" | head -n1 | cut -d: -f1)
start=$((line-30)); end=$((line+120))
sed -n "${start},${end}p" "$FILE" | cat -n
echo "---- search for validateInstruction usage / other syntax validation ----"
rg -n "validateContainerfileSyntax|validateBasicSyntax|validateInstruction|validInstructions" "$FILE"Repository: openshift/machine-config-operator
Length of output: 7771
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="pkg/controller/build/buildrequest/buildrequest.go"
echo "---- inspect validateInstruction fully (incl. instructionRequirements usage) ----"
sed -n '430,560p' "$FILE" | cat -n
echo "---- locate containerfileToConfigMap + renderedContainerfile flow ----"
rg -n "containerfileToConfigMap|renderContainerfile|validateContainerfileSyntax" "$FILE"
echo "---- show around containerfileToConfigMap ----"
line=$(rg -n "func \\(br buildRequestImpl\\) containerfileToConfigMap" "$FILE" | head -n1 | cut -d: -f1)
start=$((line-60)); end=$((line+220))
sed -n "${start},${end}p" "$FILE" | cat -n
echo "---- search for other isKnownAdvancedSyntaxError references ----"
rg -n "isKnownAdvancedSyntaxError" "$FILE"
echo "---- search repo for these exact substrings (any other suppression) ----"
rg -n "can't find = in|must be of the form: name=value" . || true
echo "---- inspect embedded Containerfile templates for heredoc/ENV/LABEL patterns ----"
ls -la pkg/controller/build/buildrequest/assets || true
for f in pkg/controller/build/buildrequest/assets/*; do
echo "### $f"; sed -n '1,120p' "$f" | cat -n; echo; doneRepository: openshift/machine-config-operator
Length of output: 31915
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact parser code that emits the two error strings.
FILE1="vendor/github.com/openshift/imagebuilder/dockerfile/parser/line_parsers.go"
FILE2="vendor/github.com/moby/buildkit/frontend/dockerfile/parser/line_parsers.go"
echo "---- openshift/imagebuilder parser: lines around 'can't find = in' ----"
rg -n "can't find = in|must be of the form: name=value" "$FILE1"
line1=$(rg -n "can't find = in" "$FILE1" | head -n1 | cut -d: -f1)
start=$((line1-40)); end=$((line1+40))
sed -n "${start},${end}p" "$FILE1" | cat -n
echo
echo "---- openshift/imagebuilder parser: search for instruction types that use this error ----"
rg -n "Must be of the form: name=value|can't find = in" "$FILE1"
echo
echo "---- buildkit frontend parser: lines around 'can't find = in' ----"
rg -n "can't find = in|must be of the form: name=value" "$FILE2"
line2=$(rg -n "can't find = in" "$FILE2" | head -n1 | cut -d: -f1)
start2=$((line2-40)); end2=$((line2+40))
sed -n "${start2},${end2}p" "$FILE2" | cat -nRepository: openshift/machine-config-operator
Length of output: 5935
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="vendor/github.com/openshift/imagebuilder/dockerfile/parser/line_parsers.go"
echo "---- locate parseArg / ARG parsing ----"
rg -n "parseArg\\b|ARG" "$FILE"
# show around parseArg if present
line=$(rg -n "func parseArg" "$FILE" | head -n1 | cut -d: -f1 || true)
if [[ -n "${line:-}" ]]; then
start=$((line-60)); end=$((line+120))
sed -n "${start},${end}p" "$FILE" | cat -n
fi
echo "---- also search entire vendor parser for the two error substrings ----"
rg -n "Syntax error - can't find = in|must be of the form: name=value" "vendor/github.com/openshift/imagebuilder/dockerfile/parser"Repository: openshift/machine-config-operator
Length of output: 117
🏁 Script executed:
#!/bin/bash
set -euo pipefail
PARSER_DIR="vendor/github.com/openshift/imagebuilder/dockerfile/parser"
echo "---- search for ARG parsing entrypoints / directive constants ----"
rg -n "ARG\b|commandArg|parseArg|parse.*Arg|parse.*ARG" "$PARSER_DIR"
echo "---- search for common ARG error messages ----"
rg -n "ARG.*(exactly|one|argument|must have|requires)|must be of the form|can't find = in" "$PARSER_DIR"
echo "---- search for ENV/LABEL parser usage (parseEnv/parseLabel) ----"
rg -n "parseEnv\\b|parseLabel\\b" "$PARSER_DIR"
echo "---- inspect dispatcher that maps instruction token to parseX ----"
# Look for the function that dispatches based on instruction name
rg -n "switch .*\\{|case .*ARG|case .*ENV|case .*LABEL" "$PARSER_DIR" | head -n 50Repository: openshift/machine-config-operator
Length of output: 144
🏁 Script executed:
#!/bin/bash
set -euo pipefail
PARSER_DIR="vendor/github.com/openshift/imagebuilder/dockerfile/parser"
echo "---- list vendored parser files ----"
find "$PARSER_DIR" -maxdepth 2 -type f | sort | sed -n '1,200p'
echo "---- search for ARG tokens anywhere in vendored parser (case-sensitive) ----"
rg -n "\\bARG\\b" "$PARSER_DIR" || true
echo "---- search for ENV/LABEL dispatch targets (case-sensitive) ----"
rg -n "\\bENV\\b|\\bLABEL\\b" "$PARSER_DIR" || true
echo "---- search for 'parse' functions that mention NameVal or key/value parsing ----"
rg -n "parseNameVal|parseEnv|parseLabel|name=value|can't find = in|must be of the form: name=value" "$PARSER_DIR" || true
echo "---- broad search for substring 'arg' (case-insensitive) to locate ARG parsing ----"
rg -n -i "arg" "$PARSER_DIR" | head -n 50 || true
echo "---- inspect any dispatcher/switch over instruction names ----"
rg -n "switch|case" "$PARSER_DIR" | head -n 80Repository: openshift/machine-config-operator
Length of output: 5129
🏁 Script executed:
#!/bin/bash
set -euo pipefail
PARSER_DIR="vendor/github.com/openshift/imagebuilder/dockerfile/parser"
FILE="$PARSER_DIR/parser.go"
LP="$PARSER_DIR/line_parsers.go"
echo "---- show dispatcher mapping for command.Arg/Env/Label ----"
rg -n "command\\.Arg|parseNameOrNameVal|command\\.Env|command\\.Label" "$FILE"
sed -n '210,270p' "$FILE" | cat -n
echo "---- find parseNameOrNameVal in line_parsers.go ----"
rg -n "func parseNameOrNameVal" "$LP"
line=$(rg -n "func parseNameOrNameVal" "$LP" | head -n1 | cut -d: -f1)
start=$((line-40)); end=$((line+140))
sed -n "${start},${end}p" "$LP" | cat -n
echo "---- search for specific error strings in line_parsers.go ----"
rg -n "can't find = in|must be of the form: name=value" "$LP"
echo "---- show nearby comment about allowed assignments (parseNameVal section) ----"
# show around line 230-270 where parseNameVal comment starts
sed -n '220,280p' "$LP" | cat -nRepository: openshift/machine-config-operator
Length of output: 11638
Narrow the parser-error bypass in validateContainerfileSyntax
isKnownAdvancedSyntaxError()currently matches generic substrings ("can't find = in"/"must be of the form: name=value") and then returnsnil, skipping all AST/instruction validation (pkg/controller/build/buildrequest/buildrequest.go:392-401).- In
github.com/openshift/imagebuilder/dockerfile/parser, those exact errors are produced byparseNameVal()and are used forENVandLABELparsing (notARG), so malformedENV/LABELcan slip through the preflight check. - Replace this broad deny-list substring matching with an allow-list approach that tolerates only the specific advanced/heredoc constructs you intend to accept, while still failing invalid
ENV/LABELsyntax.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/controller/build/buildrequest/buildrequest.go` around lines 392 - 401,
The current bypass in validateContainerfileSyntax uses
isKnownAdvancedSyntaxError to match generic substrings and returns nil for any
match, which lets malformed ENV/LABEL pass; update isKnownAdvancedSyntaxError
(and its callers in validateContainerfileSyntax where parser.Parse error is
handled) to switch from deny-list substring matches to an explicit allow-list of
only the specific advanced constructs you intend to accept (e.g.,
heredoc/continued-line tokens or exact parser error codes/messages that
represent valid advanced syntax), and ensure errors from parseNameVal-related
messages (the ENV/LABEL "can't find = in" / "must be of the form: name=value")
are not allowed and still cause Parse to return an error so malformed ENV/LABEL
fail the preflight check.
Source: Coding guidelines
Contributor
Author
|
/retest |
Contributor
Author
|
/retest |
Contributor
|
@eric200428: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
|
||
| // validateBasicSyntax performs basic sanity checks that don't require parsing | ||
| func (br buildRequestImpl) validateBasicSyntax(containerfile string) error { | ||
| // Check for at least one FROM instruction |
Member
There was a problem hiding this comment.
thought (non-blocking): We could achieve the same effect using a regex:
// This matches all lines that begin with "FROM", case-insensitive. Buildah and Podman accept both `FROM image` and `from image`.
var basicSyntaxRegex = regexp.MustCompile(`(?m)(?i)^FROM`)
func (br buildRequestImpl) validateBasicSyntax(containerfile string) error {
if matches := basicSyntaxRegex.FindAllStringSubmatchIndex(containerfile, -1); matches == nil {
return fmt.Errorf("containerfile must contain at least one FROM instruction")
}
return nil
}| } | ||
|
|
||
| for _, pattern := range knownPatterns { | ||
| if strings.Contains(errMsg, pattern) { |
Member
There was a problem hiding this comment.
thought (non-blocking): Typically, one shouldn't match errors in this way. However, given that the parser returns an error whose concrete type is general-purpose, this approach is fine here.
| } | ||
|
|
||
| // Valid Dockerfile instructions (comprehensive list including multi-stage build and advanced features) | ||
| var validInstructions = map[string]bool{ |
Member
There was a problem hiding this comment.
suggestion: A version of this map exists here: https://github.com/openshift/imagebuilder/blob/master/dockerfile/command/command.go. Why not import that package and use that map instead? You would need to change EDIT: These values are already lower-cased by the parsing library, so you don't have to call strings.ToUpper() to strings.ToLower(), however.strings.ToLower() on them.
| // instructionRequirements maps Containerfile instructions that MUST have arguments to their requirement descriptions | ||
| // Instructions not in this map either don't require arguments or have optional arguments | ||
| var instructionRequirements = map[string]string{ | ||
| "FROM": "a base image", |
Member
There was a problem hiding this comment.
suggestion: You could also use the individual command definitions found in https://github.com/openshift/imagebuilder/blob/master/dockerfile/command/command.go as the map keys, e.g.:
var instructionRequirements = map[string]string{
command.Add: "source and destination paths",
command.Arg: "a variable name",
// ...
}It would also be a good readability improvement to sort this map literal alphabetically, e.g.:
var instructionRequirements = map[string]string{
"ADD": "source and destination paths",
"ARG": "a variable name",
// ...
}| } | ||
|
|
||
| // NONE is valid without additional arguments | ||
| if instruction == "HEALTHCHECK" && node.Next != nil && strings.EqualFold(node.Next.Value, "NONE") { |
Member
There was a problem hiding this comment.
suggestion: If you import https://github.com/openshift/imagebuilder/blob/master/dockerfile/command/command.go, you could change this to:
if instruction == command.Healthcheck // ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
- What I did
- How to verify it
For in-cluster testing look at the Jira card comment
https://redhat.atlassian.net/browse/MCO-1333?focusedCommentId=17267022
or run the unit test
go test ./pkg/controller/build/buildrequest/...Summary by CodeRabbit
FROMand supported instructions; invalid user inputs may be warned on, while invalid generated output fails validation.