From 923ec55b36f0d3ec0065dc525e6a579b7b048663 Mon Sep 17 00:00:00 2001
From: zvier <liuzekun@huawei.com>
Date: Wed, 18 Nov 2020 14:56:08 +0800
Subject: [PATCH] isula-build: support build Dockerfile only have FROM command

Signed-off-by: liuzekun <liuzekun@huawei.com>
---
 Makefile                                 |  2 +-
 builder/dockerfile/builder_test.go       | 16 ++++-----
 builder/dockerfile/cmd_builder.go        |  1 +
 builder/dockerfile/cmd_builder_test.go   | 41 ++++++++++++------------
 builder/dockerfile/parser/parser.go      |  8 -----
 builder/dockerfile/parser/parser_test.go |  2 --
 builder/dockerfile/stage_builder.go      |  2 --
 7 files changed, 31 insertions(+), 41 deletions(-)

diff --git a/Makefile b/Makefile
index 11166cc2..7e268678 100644
--- a/Makefile
+++ b/Makefile
@@ -97,7 +97,7 @@ proto:
 install:
 	install -D -m0551 bin/isula-build $(BINDIR)
 	install -D -m0550 bin/isula-builder $(BINDIR)
-	@[ ! -d ${CONFIG_DIR}/${CONFIG_FILE} ] && install -dm0640 ${CONFIG_DIR}
+	@[ ! -d ${CONFIG_DIR}/${CONFIG_FILE} ] && install -dm0650 ${CONFIG_DIR}
 	@( [ -f ${CONFIG_DIR}/${CONFIG_FILE} ] && printf "%-20s %s\n" "${CONFIG_FILE}" "already exist in ${CONFIG_DIR}, please replace it manually." ) || install -D -m0600 ${LOCAL_CONF_PREFIX}/${CONFIG_FILE} ${CONFIG_DIR}/${CONFIG_FILE}
 	@( [ -f ${CONFIG_DIR}/${POLICY_FILE} ] && printf "%-20s %s\n" "${POLICY_FILE}" "already exist in ${CONFIG_DIR}, please replace it manually." ) || install -D -m0600 ${LOCAL_CONF_PREFIX}/${POLICY_FILE} ${CONFIG_DIR}/${POLICY_FILE}
 	@( [ -f ${CONFIG_DIR}/${REGIST_FILE} ] && printf "%-20s %s\n" "${REGIST_FILE}" "already exist in ${CONFIG_DIR}, please replace it manually." ) || install -D -m0600 ${LOCAL_CONF_PREFIX}/${REGIST_FILE} ${CONFIG_DIR}/${REGIST_FILE}
diff --git a/builder/dockerfile/builder_test.go b/builder/dockerfile/builder_test.go
index 94842c71..b02768ea 100644
--- a/builder/dockerfile/builder_test.go
+++ b/builder/dockerfile/builder_test.go
@@ -248,13 +248,13 @@ RUN ls
 	}
 
 	// check the arg and env taken by the command: RUN ls
-	assert.DeepEqual(t, b.stageBuilders[0].commands[0].args,
+	assert.DeepEqual(t, b.stageBuilders[0].commands[1].args,
 		map[string]string{"no_proxy": "10.0.0.0"})
-	assert.DeepEqual(t, b.stageBuilders[1].commands[1].args,
+	assert.DeepEqual(t, b.stageBuilders[1].commands[2].args,
 		map[string]string{"testArg": "0.1", "no_proxy": "10.0.0.0"})
-	assert.DeepEqual(t, b.stageBuilders[2].commands[2].args,
+	assert.DeepEqual(t, b.stageBuilders[2].commands[3].args,
 		map[string]string{"no_proxy": "10.0.0.0"})
-	assert.DeepEqual(t, b.stageBuilders[2].commands[2].envs,
+	assert.DeepEqual(t, b.stageBuilders[2].commands[3].envs,
 		map[string]string{"testArg": "1.0"})
 }
 
@@ -291,13 +291,13 @@ RUN ls
 	}
 
 	// check the arg and env taken by the command: RUN ls
-	assert.DeepEqual(t, b.stageBuilders[0].commands[0].args,
+	assert.DeepEqual(t, b.stageBuilders[0].commands[1].args,
 		map[string]string{"HTTPS_PROXY": "127.0.0.1"})
-	assert.DeepEqual(t, b.stageBuilders[1].commands[1].args,
+	assert.DeepEqual(t, b.stageBuilders[1].commands[2].args,
 		map[string]string{"testArg": "0.1", "HTTPS_PROXY": "127.0.0.1"})
-	assert.DeepEqual(t, b.stageBuilders[2].commands[2].args,
+	assert.DeepEqual(t, b.stageBuilders[2].commands[3].args,
 		map[string]string{"HTTPS_PROXY": "127.0.0.1"})
-	assert.DeepEqual(t, b.stageBuilders[2].commands[2].envs,
+	assert.DeepEqual(t, b.stageBuilders[2].commands[3].envs,
 		map[string]string{"testArg": "1.0"})
 }
 
diff --git a/builder/dockerfile/cmd_builder.go b/builder/dockerfile/cmd_builder.go
index 65ae364d..8b0d5ab9 100644
--- a/builder/dockerfile/cmd_builder.go
+++ b/builder/dockerfile/cmd_builder.go
@@ -39,6 +39,7 @@ var (
 
 func init() {
 	cmdExecutors = map[string]func(cb *cmdBuilder) error{
+		dockerfile.From:        executeNoop,
 		dockerfile.Add:         executeAdd,
 		dockerfile.Arg:         executeNoop,
 		dockerfile.Copy:        executeCopy,
diff --git a/builder/dockerfile/cmd_builder_test.go b/builder/dockerfile/cmd_builder_test.go
index 471314ba..df1c08f9 100644
--- a/builder/dockerfile/cmd_builder_test.go
+++ b/builder/dockerfile/cmd_builder_test.go
@@ -113,7 +113,7 @@ func TestExecuteHealthCheck(t *testing.T) {
 			}
 			err := s.analyzeStage(context.Background())
 			assert.NilError(t, err)
-			if err := s.commands[0].cmdExecutor(); (err != nil) != tt.wantErr {
+			if err := s.commands[1].cmdExecutor(); (err != nil) != tt.wantErr {
 				t.Errorf("CmdExecutor() error: %v, wantErr: %v", err, tt.wantErr)
 			}
 			tt.funcCheck(t, s)
@@ -195,7 +195,7 @@ CMD [""]`,
 			}
 			err := s.analyzeStage(context.Background())
 			assert.NilError(t, err)
-			if err := s.commands[0].cmdExecutor(); (err != nil) != tt.wantErr {
+			if err := s.commands[1].cmdExecutor(); (err != nil) != tt.wantErr {
 				t.Errorf("cmdExecutor() error: %v, wantErr: %v", err, tt.wantErr)
 			}
 			tt.funcCheck(t, s)
@@ -288,7 +288,7 @@ SHELL ["/bin/bash", "-c"]`,
 			}
 			err := s.analyzeStage(context.Background())
 			assert.NilError(t, err)
-			if err := s.commands[0].cmdExecutor(); (err != nil) != tt.wantErr {
+			if err := s.commands[1].cmdExecutor(); (err != nil) != tt.wantErr {
 				t.Errorf("SHELL cmdExecutor() error: %v, wantErr: %v", err, tt.wantErr)
 			}
 			tt.funcCheck(t, s)
@@ -316,15 +316,15 @@ CMD ls`
 	err := s.analyzeStage(context.Background())
 	assert.NilError(t, err)
 
-	if err := s.commands[0].cmdExecutor(); err != nil {
+	if err := s.commands[1].cmdExecutor(); err != nil {
 		t.Errorf("CMD cmdExecutor() error: %v", err)
 	}
 	assert.DeepEqual(t, s.docker.Config.Cmd, strslice.StrSlice{"/bin/sh", "-c", "ls"})
 
-	if err := s.commands[1].cmdExecutor(); err != nil {
+	if err := s.commands[2].cmdExecutor(); err != nil {
 		t.Errorf("SHELL cmdExecutor() error: %v", err)
 	}
-	if err := s.commands[2].cmdExecutor(); err != nil {
+	if err := s.commands[3].cmdExecutor(); err != nil {
 		t.Errorf("CMD cmdExecutor() error: %v", err)
 	}
 	assert.DeepEqual(t, s.shellForm, strslice.StrSlice{"/bin/bash", "-c"})
@@ -360,9 +360,10 @@ func TestExecuteNoop(t *testing.T) {
 
 	// the "STEP 1: FROM alpine" in production is done at stageBuilder.prepare()
 	// no cmdExecutor for FROM, so no print for FROM here
-	expectedString := `STEP  1: ARG testArg
-STEP  2: ENV env1=env2
-STEP  3: ONBUILD CMD ls
+	expectedString := `STEP  1: FROM alpine
+STEP  2: ARG testArg
+STEP  3: ENV env1=env2
+STEP  4: ONBUILD CMD ls
 `
 	assert.Equal(t, stepPrints, expectedString)
 }
@@ -441,7 +442,7 @@ ENTRYPOINT [""]`,
 			}
 			err := s.analyzeStage(context.Background())
 			assert.NilError(t, err)
-			if err := s.commands[0].cmdExecutor(); (err != nil) != tt.wantErr {
+			if err := s.commands[1].cmdExecutor(); (err != nil) != tt.wantErr {
 				t.Errorf("cmdExecutor() error: %v, wantErr: %v", err, tt.wantErr)
 			}
 			tt.funcCheck(t, s)
@@ -903,15 +904,15 @@ WORKDIR /c`
 	}
 	err := s.analyzeStage(context.Background())
 	assert.NilError(t, err)
-	if err := s.commands[0].cmdExecutor(); err != nil {
+	if err := s.commands[1].cmdExecutor(); err != nil {
 		t.Errorf("WORKDIR cmdExecutor() error: %v", err)
 	}
 	assert.DeepEqual(t, s.docker.Config.WorkingDir, "/a")
-	if err := s.commands[1].cmdExecutor(); err != nil {
+	if err := s.commands[2].cmdExecutor(); err != nil {
 		t.Errorf("WORKDIR cmdExecutor() error: %v", err)
 	}
 	assert.DeepEqual(t, s.docker.Config.WorkingDir, "/b")
-	if err := s.commands[2].cmdExecutor(); err != nil {
+	if err := s.commands[3].cmdExecutor(); err != nil {
 		t.Errorf("WORKDIR cmdExecutor() error: %v", err)
 	}
 	assert.DeepEqual(t, s.docker.Config.WorkingDir, "/c")
@@ -938,15 +939,15 @@ WORKDIR c`
 	}
 	err := s.analyzeStage(context.Background())
 	assert.NilError(t, err)
-	if err := s.commands[0].cmdExecutor(); err != nil {
+	if err := s.commands[1].cmdExecutor(); err != nil {
 		t.Errorf("WORKDIR cmdExecutor() error: %v", err)
 	}
 	assert.DeepEqual(t, s.docker.Config.WorkingDir, "/a")
-	if err := s.commands[1].cmdExecutor(); err != nil {
+	if err := s.commands[2].cmdExecutor(); err != nil {
 		t.Errorf("WORKDIR cmdExecutor() error: %v", err)
 	}
 	assert.DeepEqual(t, s.docker.Config.WorkingDir, "/a/b")
-	if err := s.commands[2].cmdExecutor(); err != nil {
+	if err := s.commands[3].cmdExecutor(); err != nil {
 		t.Errorf("WORKDIR cmdExecutor() error: %v", err)
 	}
 	assert.DeepEqual(t, s.docker.Config.WorkingDir, "/a/b/c")
@@ -973,15 +974,15 @@ WORKDIR $DIRPATH/$DIRNAME`
 	}
 	err := s.analyzeStage(context.Background())
 	assert.NilError(t, err)
-	if err = s.commands[0].cmdExecutor(); err != nil {
+	if err = s.commands[1].cmdExecutor(); err != nil {
 		t.Errorf("WORKDIR cmdExecutor() error: %v", err)
 	}
 	assert.NilError(t, err)
-	if err = s.commands[1].cmdExecutor(); err != nil {
+	if err = s.commands[2].cmdExecutor(); err != nil {
 		t.Errorf("WORKDIR cmdExecutor() error: %v", err)
 	}
 	assert.NilError(t, err)
-	if err = s.commands[2].cmdExecutor(); err != nil {
+	if err = s.commands[3].cmdExecutor(); err != nil {
 		t.Errorf("WORKDIR cmdExecutor() error: %v", err)
 	}
 	assert.NilError(t, err)
@@ -1025,7 +1026,7 @@ Maintainer iSula iSula@huawei.com`,
 			}
 			err := s.analyzeStage(context.Background())
 			assert.NilError(t, err)
-			if err := s.commands[0].cmdExecutor(); (err != nil) != tt.wantErr {
+			if err := s.commands[1].cmdExecutor(); (err != nil) != tt.wantErr {
 				t.Errorf("cmdExecutor() error: %v, wantErr: %v", err, tt.wantErr)
 			}
 			tt.funcCheck(t, s)
diff --git a/builder/dockerfile/parser/parser.go b/builder/dockerfile/parser/parser.go
index e8711fe7..82835e67 100644
--- a/builder/dockerfile/parser/parser.go
+++ b/builder/dockerfile/parser/parser.go
@@ -196,8 +196,6 @@ func constructPages(lines []*parser.Line, onbuild bool) ([]*parser.Page, error)
 		currentPage *parser.Page
 		pageNum     int
 	)
-	// a stage should have at least one FROM and one command
-	const minLinesPerPage = 2
 
 	for _, line := range lines {
 		if line == nil {
@@ -215,9 +213,6 @@ func constructPages(lines []*parser.Line, onbuild bool) ([]*parser.Page, error)
 				return nil, errors.New("onbuild does not support the from command")
 			}
 			if currentPage != nil {
-				if !onbuild && len(currentPage.Lines) < minLinesPerPage {
-					return nil, errors.Errorf("stage %s should have at least one command", currentPage.Name)
-				}
 				pages = append(pages, currentPage)
 			}
 
@@ -254,9 +249,6 @@ func constructPages(lines []*parser.Line, onbuild bool) ([]*parser.Page, error)
 		currentPage.End = line.End
 		currentPage.AddLine(line)
 	}
-	if !onbuild && len(currentPage.Lines) < minLinesPerPage {
-		return nil, errors.Errorf("stage %s should have at least one command", currentPage.Name)
-	}
 	// the last stage always need to commit
 	currentPage.NeedCommit = true
 	pages = append(pages, currentPage)
diff --git a/builder/dockerfile/parser/parser_test.go b/builder/dockerfile/parser/parser_test.go
index 34b1a61c..fe27dd95 100644
--- a/builder/dockerfile/parser/parser_test.go
+++ b/builder/dockerfile/parser/parser_test.go
@@ -180,8 +180,6 @@ func TestParse(t *testing.T) {
 		},
 		{
 			name:   "busybox_no_command",
-			isErr:  true,
-			errStr: "stage 1 should have at least one command",
 		},
 		{
 			name:   "env_before_from",
diff --git a/builder/dockerfile/stage_builder.go b/builder/dockerfile/stage_builder.go
index 7c928e34..47f55bf1 100644
--- a/builder/dockerfile/stage_builder.go
+++ b/builder/dockerfile/stage_builder.go
@@ -172,9 +172,7 @@ func (s *stageBuilder) analyzeStage(ctx context.Context) error {
 		cb := newCmdBuilder(ctx, line, s, stageArgs, stageEnvs)
 
 		switch line.Command {
-		// From cmd is already pre-processed, we just pass it
 		case dockerfile.From:
-			continue
 		case dockerfile.Arg:
 			if cb.args, err = analyzeArg(s.builder, line, stageArgs, stageEnvs); err != nil {
 				return err
-- 
2.19.1

