Return error if basename is expanded to blank

[yui-knk]

Fix: #37325

Signed-off-by: Yuichiro Kaneko spiketeika@gmail.com

[boaz0]

LGTM 👍

[thaJeztah]

Looking good; testing with empty values; both with a :tag and without;

docker build --no-cache -<<'EOF'
ARG SOMETHING
ARG ALPINE

FROM ${SOMETHING} as builder
# No actual build steps, just copying

FROM ${ALPINE}

EXPOSE 8080
EOF

Sending build context to Docker daemon  2.048kB
Step 1/5 : ARG SOMETHING
Step 2/5 : ARG ALPINE
Step 3/5 : FROM ${SOMETHING} as builder
base name (${SOMETHING}) should not be blank

With :tag:

docker build --no-cache -<<'EOF'
ARG SOMETHING
ARG ALPINE

FROM ${SOMETHING}:latest as builder
# No actual build steps, just copying

FROM ${ALPINE}:latest

EXPOSE 8080
EOF

Sending build context to Docker daemon  2.048kB
Step 1/5 : ARG SOMETHING
Step 2/5 : ARG ALPINE
Step 3/5 : FROM ${SOMETHING}:latest as builder
invalid reference format

[thaJeztah]

Wondering if we already have a test covering the "with :tag" option somewhere

[thaJeztah]

Also; janky failure is not related; tracked through #32673

10:46:48 FAIL: docker_api_swarm_test.go:296: DockerSwarmSuite.TestAPISwarmLeaderElection
10:46:48 
10:46:48 [dd124c5850d58] waiting for daemon to start
10:46:48 [dd124c5850d58] daemon started
10:46:48 
10:46:48 [df0c66ae7bfc0] waiting for daemon to start
10:46:48 [df0c66ae7bfc0] daemon started
10:46:48 
10:46:48 [d107b37dcc526] waiting for daemon to start
10:46:48 [d107b37dcc526] daemon started
10:46:48 
10:46:48 [dd124c5850d58] exiting daemon
10:46:48 assertion failed: error is not nil: Error response from daemon: rpc error: code = DeadlineExceeded desc = context deadline exceeded
10:46:48 [df0c66ae7bfc0] exiting daemon
10:46:48 [d107b37dcc526] exiting daemon

restarting, but all other tests were green

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@17dc101). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #37396   +/-   ##
=========================================
  Coverage          ?   34.99%           
=========================================
  Files             ?      610           
  Lines             ?    44883           
  Branches          ?        0           
=========================================
  Hits              ?    15708           
  Misses            ?    27060           
  Partials          ?     2115

[thaJeztah]

LGTM

[yongtang]

LGTM

[thaJeztah]

I think @tonistiigi had some questions about this; @tonistiigi PTAL

[tonistiigi]

The real reason why the error didn't appear seems to be that empty string is reused to mean FROM scratch in

moby/daemon/images/image_builder.go

Line 171 in 328c089

if refOrID == "" { // ie FROM scratch

. So unless we want to refactor this to be safer the current solution LGTM but should have a comment about why this extra validation is needed.

[yui-knk]

I added comment. Can you review again when you have a chance?