Сборка без использования registry #85
Conversation
WalkthroughMake Docker registry prefix optional by using Bash parameter expansion Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
jdk/Dockerfile (1)
1-5: Default value forDOCKER_REGISTRY_URLconflicts with conditional logic.Line 1 sets
DOCKER_REGISTRY_URL=libraryas default, but line 5 uses:+expansion which only applies when the variable is set. Since there's a default, the prefix will always be applied, resulting inlibrary/debian:bookworm-slim.If the intent is to support registry-less builds, either remove the default or adjust the logic:
-ARG DOCKER_REGISTRY_URL=library +ARG DOCKER_REGISTRY_URL ARG BASE_IMAGE=debian ARG BASE_TAG=bookworm-slimAdditionally, the same
:+syntax parsing issue applies here as flagged in other Dockerfiles.build-base-k8s-jenkins-agent.sh (1)
99-101: Inconsistent push guard logic - potential runtime failure.Lines 44-48 and 61-65 guard pushes based on
DOCKER_REGISTRY_URL, but line 100 only checksPUSH_AGENT. WhenDOCKER_REGISTRY_URLis unset (the registry-less scenario this PR enables), line 100 will attempt to push to an empty registry URL and fail.Apply this diff to align with the guarding pattern used elsewhere:
-if [[ $PUSH_AGENT != "false" ]] ; then - docker push $DOCKER_REGISTRY_URL/base-jenkins-agent:$ONEC_VERSION -fi +if [[ $PUSH_AGENT != "false" ]] && [[ -n "$DOCKER_REGISTRY_URL" ]]; then + docker push $DOCKER_REGISTRY_URL/base-jenkins-agent:$ONEC_VERSION +else + echo "DOCKER_REGISTRY_URL not set or PUSH_AGENT is false, skipping docker push." +fi
🧹 Nitpick comments (5)
build-base-swarm-jenkins-coverage-agent.sh (1)
37-41: LGTM! Push guard logic is correct.The conditional push correctly checks for non-empty
DOCKER_REGISTRY_URLbefore pushing, aligning with the PR's goal of registry-less builds.Minor suggestion: Consider quoting the registry URL in the push command for defensive coding.
Apply this diff for added safety:
- docker push $DOCKER_REGISTRY_URL/base-jenkins-coverage-agent:$ONEC_VERSION + docker push "$DOCKER_REGISTRY_URL/base-jenkins-coverage-agent:$ONEC_VERSION"oscript/Dockerfile (1)
1-1: Consider removing the default empty string for consistency.Line 1 sets
ARG DOCKER_REGISTRY_URL=""with a default empty string, while other Dockerfiles in this PR (client, server, gitsync, etc.) declare the ARG without a default. The conditional expansion on Line 5 works correctly either way, but removing the default would improve consistency across the codebase.Apply this diff for consistency:
-ARG DOCKER_REGISTRY_URL="" +ARG DOCKER_REGISTRY_URLbuild-base-swarm-jenkins-agent.sh (1)
44-48: LGTM! Push guard logic is correct throughout.All conditional pushes correctly check for non-empty
DOCKER_REGISTRY_URLbefore pushing. The logic on lines 99-103 additionally checksPUSH_AGENTas expected.Minor suggestion: Consider quoting registry URLs in push commands for defensive coding.
Example diff for lines 45, 62, and 100:
- docker push $DOCKER_REGISTRY_URL/onec-client:$ONEC_VERSION + docker push "$DOCKER_REGISTRY_URL/onec-client:$ONEC_VERSION"- docker push $DOCKER_REGISTRY_URL/onec-client-vnc:$ONEC_VERSION + docker push "$DOCKER_REGISTRY_URL/onec-client-vnc:$ONEC_VERSION"- docker push "$DOCKER_REGISTRY_URL/base-jenkins-agent:$ONEC_VERSION" + docker push "$DOCKER_REGISTRY_URL/base-jenkins-agent:$ONEC_VERSION"Note: Line 100 already has quotes, which is the preferred approach.
Also applies to: 61-65, 99-103
executor/Dockerfile (1)
4-4: Expected Hadolint error - consider suppressing.The Hadolint DL1000 error is a false positive. The syntax
${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}is valid Bash parameter expansion that Docker's build argument substitution handles correctly. This pattern is essential for the registry-less build feature.To suppress the false positive, add a comment above line 4:
+# hadolint ignore=DL1000 FROM ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}${BASE_IMAGE}:${BASE_TAG}k8s-jenkins-agent/Dockerfile (1)
7-7: Expected Hadolint error - consider suppressing.The Hadolint DL1000 error is a false positive. The parameter expansion syntax is valid and necessary for conditional registry prefixing.
To suppress the false positive:
+# hadolint ignore=DL1000 FROM ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}${BASE_IMAGE}:${BASE_TAG} AS agent
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
.github/copilot-instructions.md(2 hunks)build-base-k8s-jenkins-agent.sh(2 hunks)build-base-k8s-jenkins-coverage-agent.sh(1 hunks)build-base-swarm-jenkins-agent.sh(2 hunks)build-base-swarm-jenkins-coverage-agent.sh(1 hunks)build-crs.sh(1 hunks)build-edt-k8s-agent.sh(1 hunks)build-edt-swarm-agent.sh(1 hunks)build-edt.sh(2 hunks)build-executor.sh(1 hunks)build-oscript-k8s-agent.sh(1 hunks)build-oscript-swarm-agent.sh(1 hunks)build-server.sh(2 hunks)client-vnc/Dockerfile(1 hunks)client/Dockerfile(1 hunks)coverage41C/Dockerfile(1 hunks)crs-apache/Dockerfile(1 hunks)edt/Dockerfile(1 hunks)executor/Dockerfile(1 hunks)gitsync/Dockerfile(1 hunks)jdk/Dockerfile(1 hunks)k8s-jenkins-agent/Dockerfile(1 hunks)oscript-utils/Dockerfile(1 hunks)oscript/Dockerfile(1 hunks)rac-gui/Dockerfile(1 hunks)server/Dockerfile(1 hunks)swarm-jenkins-agent/Dockerfile(1 hunks)test-utils/Dockerfile(1 hunks)thin-client/Dockerfile(1 hunks)vanessa-runner/Dockerfile(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nixel2007
Repo: firstBitMarksistskaya/onec-docker PR: 45
File: .github/workflows/build.yml:29-36
Timestamp: 2024-10-01T21:11:39.311Z
Learning: `DOCKER_LOGIN` и `DOCKER_PASSWORD` намеренно оставляются пустыми, так как используется временный локальный реестр.
Learnt from: nixel2007
Repo: firstBitMarksistskaya/onec-docker PR: 45
File: .github/workflows/build.yml:29-36
Timestamp: 2024-10-09T14:16:49.793Z
Learning: `DOCKER_LOGIN` и `DOCKER_PASSWORD` намеренно оставляются пустыми, так как используется временный локальный реестр.
📚 Learning: 2024-10-09T14:16:49.793Z
Learnt from: nixel2007
Repo: firstBitMarksistskaya/onec-docker PR: 45
File: .github/workflows/build.yml:29-36
Timestamp: 2024-10-09T14:16:49.793Z
Learning: `DOCKER_LOGIN` и `DOCKER_PASSWORD` намеренно оставляются пустыми, так как используется временный локальный реестр.
Applied to files:
thin-client/Dockerfileclient/Dockerfileedt/Dockerfileserver/Dockerfile
🪛 Hadolint (2.14.0)
thin-client/Dockerfile
[error] 5-5: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
k8s-jenkins-agent/Dockerfile
[error] 7-7: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
client/Dockerfile
[error] 5-5: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
executor/Dockerfile
[error] 4-4: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
swarm-jenkins-agent/Dockerfile
[error] 5-5: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
edt/Dockerfile
[error] 10-10: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
test-utils/Dockerfile
[error] 5-5: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
server/Dockerfile
[error] 5-5: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
coverage41C/Dockerfile
[error] 7-7: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
client-vnc/Dockerfile
[error] 3-3: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
vanessa-runner/Dockerfile
[error] 2-2: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
oscript/Dockerfile
[error] 5-5: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
oscript-utils/Dockerfile
[error] 2-2: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
gitsync/Dockerfile
[error] 3-3: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
crs-apache/Dockerfile
[error] 4-4: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
jdk/Dockerfile
[error] 5-5: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
rac-gui/Dockerfile
[error] 3-3: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
🔇 Additional comments (29)
build-edt-swarm-agent.sh (1)
31-39: LGTM!The conditional tagging and guarded push logic is correctly implemented. The bash parameter expansion
${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}works properly in shell scripts, and the push is appropriately guarded.swarm-jenkins-agent/Dockerfile (1)
5-5: Hadolint's parser doesn't support:+parameter expansion, but Docker does.Hadolint reports DL1000 because its parser has a limitation with modern parameter expansion syntax. However, Docker (since syntax 1.6.0) fully supports
${VAR:+value}in FROM instructions. Your syntax is valid and will work at build time.You can silence the Hadolint warning by adding
# hadolint ignore=DL1000above the FROM instruction, or refactor to a simpler approach if preferred:ARG DOCKER_REGISTRY_URL ARG BASE_IMAGE ARG BASE_TAG +ARG BASE_IMAGE_PREFIX=${DOCKER_REGISTRY_URL:+${DOCKER_REGISTRY_URL}/} -FROM ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}${BASE_IMAGE}:${BASE_TAG} +FROM ${BASE_IMAGE_PREFIX}${BASE_IMAGE}:${BASE_TAG}client/Dockerfile (1)
5-5: LGTM! Conditional registry prefix is correctly implemented.The bash parameter expansion
${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}correctly handles optional registry URLs. WhenDOCKER_REGISTRY_URLis unset or empty, the prefix is omitted; when set, it prepends the registry URL with a trailing slash.Note: The Hadolint DL1000 error is a false positive. This syntax is valid in Dockerfiles and is the standard approach for conditional parameter expansion.
build-base-swarm-jenkins-coverage-agent.sh (1)
33-33: LGTM! Conditional registry prefix correctly applied.The image tag correctly uses conditional expansion to handle optional registry URLs.
gitsync/Dockerfile (1)
3-3: LGTM! Conditional registry prefix correctly implemented.The conditional expansion properly handles optional registry URLs, consistent with the broader PR pattern.
Note: The Hadolint DL1000 error is a false positive.
server/Dockerfile (1)
5-5: LGTM! Conditional registry prefix is correctly implemented.The bash parameter expansion correctly handles optional registry URLs, matching the pattern used consistently across the PR.
Note: The Hadolint DL1000 error is a false positive.
oscript/Dockerfile (1)
5-5: LGTM! Conditional registry prefix correctly implemented.The conditional expansion properly handles optional registry URLs.
Note: The Hadolint DL1000 error is a false positive.
build-base-swarm-jenkins-agent.sh (1)
29-29: LGTM! Conditional registry prefixes correctly applied throughout.All image tags properly use conditional expansion to handle optional registry URLs, consistent with the PR's goal.
Also applies to: 40-40, 56-56, 71-71, 79-79, 87-87, 95-95
client-vnc/Dockerfile (1)
3-3: LGTM! Conditional registry prefix correctly implemented.The conditional expansion properly handles optional registry URLs, maintaining consistency with the PR's approach.
Note: The Hadolint DL1000 error is a false positive.
test-utils/Dockerfile (1)
5-5: LGTM! Conditional registry prefix correctly implemented.The conditional expansion properly handles optional registry URLs, consistent with the pattern used throughout the PR.
Note: The Hadolint DL1000 error is a false positive.
build-base-k8s-jenkins-coverage-agent.sh (2)
33-35: LGTM! Conditional registry prefix is correctly implemented.The bash parameter expansion
${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}correctly handles optional registry URLs. WhenDOCKER_REGISTRY_URLis unset or empty, the image will be tagged without a registry prefix for purely local builds.
37-41: LGTM! Push guard correctly implements optional registry behavior.The conditional block properly skips pushing when
DOCKER_REGISTRY_URLis not set, enabling local-only builds as described in the PR objectives.coverage41C/Dockerfile (1)
7-11: LGTM! Hadolint warning is a false positive.The conditional registry prefix syntax
${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}is valid bash parameter expansion and works correctly in Dockerfiles. Hadolint's parser limitation causes it to flag this as DL1000, but the syntax is intentional and aligns with the PR's objective of supporting registry-less builds.rac-gui/Dockerfile (1)
3-3: LGTM! Hadolint warning is a false positive.The conditional registry prefix is correctly implemented. Hadolint's DL1000 error is a parser limitation with bash parameter expansion syntax in FROM statements.
build-crs.sh (2)
28-36: LGTM! CRS image correctly implements optional registry pattern.The conditional tagging and guarded push for the
crsimage properly support registry-less builds.
41-49: LGTM! CRS-Apache image correctly implements optional registry pattern.The conditional tagging and guarded push for the
crs-apacheimage properly support registry-less builds, maintaining consistency with thecrsimage..github/copilot-instructions.md (2)
54-56: LGTM! Documentation properly reflects the new registry pattern.The updated Dockerfile guideline clearly demonstrates the conditional registry prefix syntax, helping maintainers understand and follow the pattern.
141-141: LGTM! Container naming pattern documentation updated.The example correctly demonstrates the conditional registry prefix in container naming, consistent with the broader changes.
thin-client/Dockerfile (1)
5-5: LGTM! Hadolint warning is a false positive.The conditional registry prefix for the downloader stage is correctly implemented. Hadolint's DL1000 error is a parser limitation with this bash parameter expansion syntax.
build-edt.sh (2)
41-43: LGTM! Oscript-downloader tag correctly uses conditional registry prefix.The image tagging properly supports optional registry URLs for local builds.
54-56: LGTM! EDT image tag correctly uses conditional registry prefix.The image tagging properly supports optional registry URLs, consistent with the oscript-downloader image and the PR's objective of enabling registry-less builds.
edt/Dockerfile (1)
10-10: LGTM! Hadolint warning is a false positive.The conditional registry prefix for the downloader stage is correctly implemented using
DOWNLOADER_REGISTRY_URL. Hadolint's DL1000 error is a parser limitation with bash parameter expansion syntax in FROM statements.build-base-k8s-jenkins-agent.sh (2)
29-31: LGTM! Conditional registry prefix correctly applied.The parameter expansion
${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}correctly handles the optional registry URL, preventing malformed image tags when the variable is unset.
44-48: LGTM! Push correctly guarded.The conditional push logic aligns with the PR objective of enabling registry-less builds.
build-oscript-swarm-agent.sh (1)
28-30: LGTM! Registry-less build support correctly implemented.The conditional registry prefix and guarded push are correctly applied, enabling local-only builds when
DOCKER_REGISTRY_URLis unset.Also applies to: 37-39, 41-45
build-server.sh (1)
44-48: LGTM! Push correctly guarded.The conditional push logic correctly implements the registry-less build feature.
build-executor.sh (1)
39-41: LGTM! Registry-less build support correctly implemented.Both the conditional tagging and guarded push are properly applied, aligning with the PR's objective.
Also applies to: 45-49
build-edt-k8s-agent.sh (1)
31-33: LGTM! Registry-less build support correctly implemented.The conditional tagging and guarded push are correctly applied.
Also applies to: 35-39
build-oscript-k8s-agent.sh (1)
28-30: LGTM! Registry-less build support correctly implemented.The conditional registry prefix and guarded push are correctly applied, enabling local-only builds.
Also applies to: 37-39, 41-45
| ARG DOCKER_REGISTRY_URL | ||
| FROM ${DOCKER_REGISTRY_URL}/oscript:1.0.21 | ||
| FROM ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}oscript:1.0.21 |
There was a problem hiding this comment.
Same :+ expansion syntax issue.
Apply the same fix pattern as suggested for other Dockerfiles in this PR.
🧰 Tools
🪛 Hadolint (2.14.0)
[error] 2-2: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
🤖 Prompt for AI Agents
In oscript-utils/Dockerfile lines 1-2, the FROM line uses the incorrect
expansion ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}; change it to use the
standard plus expansion without embedded quotes: replace with FROM
${DOCKER_REGISTRY_URL:+$DOCKER_REGISTRY_URL/}oscript:1.0.21 so the registry
prefix is included only when DOCKER_REGISTRY_URL is set.
|
Что характерно, именно сборка сервера на ci и упала :) |
|
Посмотри замечания кролика, пожалуйста, у том числе те, что out of diff |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
coverage41C/Dockerfile (1)
7-7: Verify Docker compatibility with conditional parameter expansion.Both FROM instructions use
${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}syntax, which Hadolint flags with DL1000. Given the CI build failures mentioned in PR comments, verify this syntax works in your Docker environment.Refer to the verification script provided in the executor/Dockerfile review comment to test compatibility.
Also applies to: 11-11
edt/Dockerfile (1)
10-10: Verify Docker compatibility with conditional parameter expansion.The FROM instruction uses conditional parameter expansion, which Hadolint flags with DL1000. Given the reported CI failures, verify this syntax works with your Docker version.
Refer to the verification script in the executor/Dockerfile review comment.
oscript/Dockerfile (1)
5-5: Verify Docker compatibility with conditional parameter expansion.Hadolint flags DL1000 on this FROM instruction. Given CI build failures mentioned in PR comments, verify the syntax compatibility.
Refer to the verification script in the executor/Dockerfile review comment.
build-server.sh (1)
29-29: Critical: Fix syntax error in parameter expansion.Line 29 uses
:+:instead of:+, which is invalid Bash syntax and will cause the build to fail. This issue was previously flagged by other reviewers.🔎 Apply this diff to fix the syntax error:
- -t ${DOCKER_REGISTRY_URL:+:"$DOCKER_REGISTRY_URL/"}oscript-downloader:latest \ + -t ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}oscript-downloader:latest \
🧹 Nitpick comments (2)
build-base-swarm-jenkins-agent.sh (1)
99-103: Consider consistent quoting in push commands.Line 100 quotes the push command (
"$DOCKER_REGISTRY_URL/base-jenkins-agent:$ONEC_VERSION"), while earlier pushes (lines 45, 62) don't. For consistency and safety with special characters in variables, consider quoting all push commands.🔎 Apply this diff for consistency:
- docker push $DOCKER_REGISTRY_URL/onec-client:$ONEC_VERSION + docker push "$DOCKER_REGISTRY_URL/onec-client:$ONEC_VERSION"- docker push $DOCKER_REGISTRY_URL/onec-client-vnc:$ONEC_VERSION + docker push "$DOCKER_REGISTRY_URL/onec-client-vnc:$ONEC_VERSION"build-base-k8s-jenkins-agent.sh (1)
44-48: Inconsistent quoting in push commands.Line 45 quotes the push target while lines 62 and 100 don't. For safety and consistency, all push commands should use quoted arguments.
🔎 Apply this diff for consistency:
- docker push $DOCKER_REGISTRY_URL/onec-client-vnc:$ONEC_VERSION + docker push "$DOCKER_REGISTRY_URL/onec-client-vnc:$ONEC_VERSION"- docker push $DOCKER_REGISTRY_URL/base-jenkins-agent:$ONEC_VERSION + docker push "$DOCKER_REGISTRY_URL/base-jenkins-agent:$ONEC_VERSION"Also applies to: 61-65, 99-103
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
build-base-k8s-jenkins-agent.sh(2 hunks)build-base-swarm-jenkins-agent.sh(2 hunks)build-server.sh(2 hunks)coverage41C/Dockerfile(1 hunks)edt/Dockerfile(1 hunks)executor/Dockerfile(1 hunks)jdk/Dockerfile(1 hunks)oscript/Dockerfile(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nixel2007
Repo: firstBitMarksistskaya/onec-docker PR: 45
File: .github/workflows/build.yml:29-36
Timestamp: 2024-10-09T14:16:49.793Z
Learning: `DOCKER_LOGIN` и `DOCKER_PASSWORD` намеренно оставляются пустыми, так как используется временный локальный реестр.
Learnt from: nixel2007
Repo: firstBitMarksistskaya/onec-docker PR: 45
File: .github/workflows/build.yml:29-36
Timestamp: 2024-10-01T21:11:39.311Z
Learning: `DOCKER_LOGIN` и `DOCKER_PASSWORD` намеренно оставляются пустыми, так как используется временный локальный реестр.
📚 Learning: 2024-10-09T14:16:49.793Z
Learnt from: nixel2007
Repo: firstBitMarksistskaya/onec-docker PR: 45
File: .github/workflows/build.yml:29-36
Timestamp: 2024-10-09T14:16:49.793Z
Learning: `DOCKER_LOGIN` и `DOCKER_PASSWORD` намеренно оставляются пустыми, так как используется временный локальный реестр.
Applied to files:
executor/Dockerfileedt/Dockerfile
🪛 Hadolint (2.14.0)
executor/Dockerfile
[error] 4-4: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
edt/Dockerfile
[error] 10-10: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
oscript/Dockerfile
[error] 5-5: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
jdk/Dockerfile
[error] 5-5: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
coverage41C/Dockerfile
[error] 7-7: unexpected ':'
expecting '@', '', a new line followed by the next instruction, or the image tag
(DL1000)
🔇 Additional comments (9)
jdk/Dockerfile (2)
1-1: LGTM! Removal of default value aligns with optional registry approach.Removing the default
libraryvalue makesDOCKER_REGISTRY_URLtruly optional, which is consistent with the PR's goal of enabling local builds without a registry.
5-5: Hadolint DL1000 is a parser limitation, not a syntax error—removing quotes won't fix it.The conditional registry prefix using
${DOCKER_REGISTRY_URL:+...}is valid Docker syntax and works correctly. The DL1000 error is a known Hadolint parser limitation that doesn't support shell parameter expansion operators (:+,:-,:?, etc.), not an issue with inner quotes. Both quoted and unquoted forms trigger the same parsing error in Hadolint.Instead of modifying the syntax, add a hadolint ignore pragma:
+# hadolint ignore=DL1000 FROM ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}${BASE_IMAGE}:${BASE_TAG}The syntax is correct and Docker builds work with both
DOCKER_REGISTRY_URLset and unset.Likely an incorrect or invalid review comment.
build-base-swarm-jenkins-agent.sh (2)
29-31: LGTM: Conditional registry prefix implementation.The parameter expansion
${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}correctly applies the registry prefix only whenDOCKER_REGISTRY_URLis set and non-empty, enabling local-only builds.Also applies to: 40-42
44-48: LGTM: Guarded push implementation.The conditional push blocks correctly prevent pushing to a registry when
DOCKER_REGISTRY_URLis unset, with clear skip messages for debugging.Also applies to: 61-65
build-base-k8s-jenkins-agent.sh (1)
29-31: LGTM: Conditional registry prefix implementation.The parameter expansion correctly enables optional registry prefix for local-only builds.
Also applies to: 40-42
build-server.sh (2)
40-42: LGTM: Correct conditional registry prefix.This line correctly uses the
:+parameter expansion syntax.
44-48: LGTM: Guarded push implementation.The conditional push logic correctly prevents pushing when
DOCKER_REGISTRY_URLis unset.edt/Dockerfile (1)
1-10: LGTM: Separate registry variables for multi-stage build.The design using separate
DOWNLOADER_REGISTRY_URLandDOCKER_REGISTRY_URLarguments allows flexibility to pull the downloader stage from a different registry than the base images, which is a good practice for complex multi-stage builds.executor/Dockerfile (1)
4-4: The syntax${VAR:+"value"}is supported in Docker FROM instructions and does not have version/BuildKit limitations.Hadolint flags this with DL1000 ("unexpected ':'"), which is a known Hadolint parser issue, not a Docker limitation. Docker's variable substitution supports POSIX-compatible parameter expansion variants, and BuildKit has been the default builder since Docker 23.0. If CI builds fail, check whether Hadolint validation is the blocker—the syntax itself is valid Docker. The suggested alternatives (build script, empty default) are unnecessary unless you specifically need to avoid using parameter expansion.
Likely an incorrect or invalid review comment.
|
@nixel2007 вроде везде прошелся, починил в последней сборке один из образов не собрался из-за ограничений github, видимо |
nixel2007
merged commit 17247b5
into
firstBitMarksistskaya:feature/first-bit
|
Спасибо! |
2 participants
Сделал так, чтобы образы можно было собирать строго локально, без пуша в registry.
Для многих моих задач registry не нужен. Думаю, другим тоже может быть полезно.
Изменения пока не проверял тщательно, однако, сборка сервера 1С проходит успешно.
Если идея будет одобрена, надо будет доделать скрипты под Windows.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.