Bug 1801875

Summary: substitution of default values for ARG step not occurring with some Dockerfiles
Product: OpenShift Container Platform Reporter: Gabe Montero <gmontero>
Component: ContainersAssignee: Nalin Dahyabhai <nalin>
Status: CLOSED DUPLICATE QA Contact: weiwei jiang <wjiang>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 4.6CC: adam.kaplan, aos-bugs, bparees, dwalsh, jokerman, nalin, tsweeney, wewang, wzheng
Target Milestone: ---   
Target Release: 4.6.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: 1801388 Environment:
Last Closed: 2020-07-14 14:00:49 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1801388    
Bug Blocks: 1802202    

Description Gabe Montero 2020-02-11 19:41:13 UTC
+++ This bug was initially created as a clone of Bug #1801388 +++

See https://github.com/openshift/builder/issues/129

Also produced on ocp install in addition to crc

build.build.openshift.io/test-1 started
Cloning "https://github.com/nginxinc/kubernetes-ingress" ...
	Commit:	65c0fefd20f88aa3832a56e17c4c4e263f54e1c1 (Ensure clean up in fixtures and fix condition)
	Author:	tellet <tellet.tat>
	Date:	Fri Feb 7 14:35:22 2020 +0100
Replaced Dockerfile FROM image base
panic: assignment to entry in nil map

goroutine 1 [running]:
github.com/openshift/builder/vendor/github.com/openshift/imagebuilder.arg(0xc000152580, 0xc000828670, 0x1, 0x1, 0x0, 0x3375c98, 0x0, 0x0, 0xc0005a0750, 0x22, ...)
	/go/src/github.com/openshift/builder/vendor/github.com/openshift/imagebuilder/dispatchers.go:557 +0x234
github.com/openshift/builder/vendor/github.com/openshift/imagebuilder.(*Builder).Run(0xc000152580, 0xc000804d00, 0x1f6f3a0, 0x3375c98, 0x0, 0xc00068ee00, 0x10)
	/go/src/github.com/openshift/builder/vendor/github.com/openshift/imagebuilder/builder.go:324 +0xe5
github.com/openshift/builder/vendor/github.com/openshift/imagebuilder.(*Builder).extractHeadingArgsFromNode(0xc000152580, 0xc000705110, 0xc000781388, 0x14a3ccf)
	/go/src/github.com/openshift/builder/vendor/github.com/openshift/imagebuilder/builder.go:243 +0x341
github.com/openshift/builder/vendor/github.com/openshift/imagebuilder.NewStages(0xc000705110, 0xc000152580, 0xc0006c01c0, 0x1be4fe0, 0x1, 0x0, 0x1)
	/go/src/github.com/openshift/builder/vendor/github.com/openshift/imagebuilder/builder.go:201 +0x4d
github.com/openshift/builder/pkg/build/builder.replaceImagesFromSource(0xc000705110, 0x0, 0x0, 0x0, 0x0, 0x0)
	/go/src/github.com/openshift/builder/pkg/build/builder/common.go:451 +0x1a6
github.com/openshift/builder/pkg/build/builder.addBuildParameters(0xc00051dc60, 0x11, 0xc000430000, 0xc000303040, 0x0, 0x0)
	/go/src/github.com/openshift/builder/pkg/build/builder/common.go:428 +0x35c
github.com/openshift/builder/pkg/build/builder.ManageDockerfile(0xc00051dc60, 0x11, 0xc000430000, 0xc000474e80, 0x0)
	/go/src/github.com/openshift/builder/pkg/build/builder/source.go:123 +0x324
github.com/openshift/builder/pkg/build/builder/cmd.RunManageDockerfile(0x1f21280, 0xc000010020, 0x0, 0x0)
	/go/src/github.com/openshift/builder/pkg/build/builder/cmd/builder.go:411 +0xd6
main.NewCommandManageDockerfile.func1(0xc000708280, 0xc0007fbcb0, 0x0, 0x1)
	/go/src/github.com/openshift/builder/cmd/builder.go:118 +0x5c
github.com/openshift/builder/vendor/github.com/spf13/cobra.(*Command).execute(0xc000708280, 0xc00000e090, 0x1, 0x1, 0xc000708280, 0xc00000e090)
	/go/src/github.com/openshift/builder/vendor/github.com/spf13/cobra/command.go:830 +0x2ae
github.com/openshift/builder/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0xc000708280, 0x1b, 0xc000708280, 0x1b)
	/go/src/github.com/openshift/builder/vendor/github.com/spf13/cobra/command.go:914 +0x2fc
github.com/openshift/builder/vendor/github.com/spf13/cobra.(*Command).Execute(...)
	/go/src/github.com/openshift/builder/vendor/github.com/spf13/cobra/command.go:864
main.main()
	/go/src/github.com/openshift/builder/cmd/main.go:68 +0x668

--- Additional comment from Gabe Montero on 2020-02-10 22:25:03 UTC ---

Reproduced with a unit test based on the Dockerfile and build config from https://github.com/openshift/builder/issues/129

--- Additional comment from Gabe Montero on 2020-02-10 22:50:50 UTC ---

Have fix in hand (one liner) ... just cleaning up / simplifying the unit test


So even after the fix for the panic above, using a openshift BC of

kind: BuildConfig
apiVersion: build.openshift.io/v1
metadata:
  name: test
spec:
  runPolicy: Serial
  source:
    git:
      uri: https://github.com/nginxinc/kubernetes-ingress
      ref: master
    contextDir: build
  strategy:
    dockerStrategy:
      from:
        kind: DockerImage
        name: debian:latest
    type: Docker
  output:
    to:
      kind: ImageStreamTag
      name: test:latest

Where the Dockerfile there as of this writing was

ARG GOLANG_CONTAINER=golang:latest

FROM nginx:1.17.8 AS base

# forward nginx access and error logs to stdout and stderr of the ingress
# controller process
RUN ln -sf /proc/1/fd/1 /var/log/nginx/access.log \
	&& ln -sf /proc/1/fd/1 /var/log/nginx/stream-access.log \
	&& ln -sf /proc/1/fd/2 /var/log/nginx/error.log

RUN mkdir -p /var/lib/nginx \
	&& mkdir -p /etc/nginx/secrets \
	&& apt-get update \
	&& apt-get install -y libcap2-bin \
	&& setcap 'cap_net_bind_service=+ep' /usr/sbin/nginx \
	&& setcap 'cap_net_bind_service=+ep' /usr/sbin/nginx-debug \
	&& chown -R nginx:0 /etc/nginx \
	&& chown -R nginx:0 /var/cache/nginx \
	&& chown -R nginx:0 /var/lib/nginx \
	&& apt-get remove --purge -y libcap2-bin \
	&& rm /etc/nginx/conf.d/* \
	&& rm -rf /var/lib/apt/lists/*

COPY internal/configs/version1/nginx.ingress.tmpl \
	internal/configs/version1/nginx.tmpl \
	internal/configs/version2/nginx.virtualserver.tmpl /

# Uncomment the line below if you would like to add the default.pem to the image
# and use it as a certificate and key for the default server
# ADD default.pem /etc/nginx/secrets/default

USER nginx

ENTRYPOINT ["/nginx-ingress"]


FROM base AS local
COPY nginx-ingress /


FROM $GOLANG_CONTAINER AS builder
ARG VERSION
ARG GIT_COMMIT
WORKDIR /go/src/github.com/nginxinc/kubernetes-ingress/nginx-ingress/cmd/nginx-ingress
COPY . /go/src/github.com/nginxinc/kubernetes-ingress/nginx-ingress/
RUN CGO_ENABLED=0 GOFLAGS='-mod=vendor' \
	go build -installsuffix cgo -ldflags "-w -X main.version=${VERSION} -X main.gitCommit=${GIT_COMMIT}" -o /nginx-ingress


FROM base AS container
COPY --from=builder /nginx-ingress /

When we try to do the buildah build, buildah complains with 

error parsing image name "$GOLANG_CONTAINER": invalid reference format: repository name must be lowercase 

i.e. it does not pick up the default set for the ARG GOLANG_CONTAINER in the Dockerfile above.

Comment 1 Gabe Montero 2020-02-11 19:42:57 UTC
Adding Nalin, openshift builds point of contact on the conatiners/buildah team, to the CC list.

Comment 2 Gabe Montero 2020-02-12 00:31:25 UTC
Ben Parees, Nalin, and myself had slack discussion on this and Ben came up with a simplified reproducer 
for an ARG default value not showing up in a FROM reference, which bypasses the golang panic getting fixed
in the blocking BZ to this one.

The reproducing openshift build config is 

apiVersion: build.openshift.io/v1
kind: BuildConfig
metadata:
  name: sample-build-docker-args-preset
spec:
  source:
    dockerfile: |-
      FROM centos
      ARG foo=centos
      FROM $foo
    type: Dockerfile
  strategy:
    dockerStrategy: {}
    type: Docker


which results in 

I0211 22:06:43.140914       1 builder.go:318] Starting Docker build from build config sample-build-docker-args-preset-17 ...
F0211 22:06:43.209605       1 helpers.go:114] error: build error: error parsing name "$foo": error parsing image name "$foo": invalid reference format

Nalin said he is investigating.

Comment 3 Tom Sweeney 2020-06-04 00:11:46 UTC
I believe a number of fixes culminating with a couple of fixes in ImageBuilder, https://github.com/openshift/imagebuilder/pull/163 and https://github.com/openshift/imagebuilder/pull/164 has resolved this issue.  The plan is to have this fixes in RHEL 8.3.

Adam or Nalin, do you have the simplified Dockerfile that I could run against Buildah to validate?

Using the Dockerfile in the original report, I'm seeing:
https://github.com/openshift/imagebuilder/pull/163

STEP 8: FROM golang:latest AS builder
Getting image source signatures
Copying blob 496548a8c952 done  
Copying blob 039b991354af done  
Copying blob 5a63a0a859d8 done  
Copying blob 2adae3950d4d done  
Copying blob 6b823afb12d9 [======================================] 118.0MiB / 118.0MiB
Copying blob 376057ac6fa1 done  
Copying blob 30b9d62bd869 done  
Copying config 05feda5424 done  
Writing manifest to image destination
Storing signatures
STEP 9: ARG VERSION


Which corresponds to these lines in the Dockerfile:
FROM $GOLANG_CONTAINER AS builder
ARG VERSION

Comment 4 Gabe Montero 2020-06-05 15:30:32 UTC
per recent discussion with Nalin, moving these openshfit build induced buildah items back to OCP/containers

Comment 5 Nalin Dahyabhai 2020-07-09 20:16:10 UTC
I think that most of this should have been sorted out in https://github.com/openshift/builder/pull/132, which has been merged.  Opened https://github.com/openshift/builder/pull/162 to pull in the changes Tom mentioned.

Comment 6 Nalin Dahyabhai 2020-07-09 20:29:09 UTC
I should note that this part of comment #2 is rejected by `docker build`, regardless of whether or not `--build-arg foo=...` is passed to it, so it's not an ideal test case:
      FROM centos
      ARG foo=centos
      FROM $foo

The ARG above is scoped to the stage in which it's defined, as described in https://github.com/openshift/imagebuilder/pull/151.  The example from the bug description can be boiled down to a subtly different example, putting the ARG instruction before the FROM instruction that is treated as the start of the first stage:
      ARG foo=centos
      FROM centos
      FROM $foo

The latter example should be expected to work, and that's what we're focusing on here.

Comment 7 Adam Kaplan 2020-07-14 14:00:49 UTC

*** This bug has been marked as a duplicate of bug 1839683 ***