From fe8f53375ea90012ea4578725f256b60df083046 Mon Sep 17 00:00:00 2001 From: Shubham Bansal <62992590+shubham-bansal96@users.noreply.github.com> Date: Mon, 13 Jun 2022 19:26:05 +0530 Subject: [PATCH] builder: Allow command with arguments via custom build options (#2453) * allow buildcmd command to process arguments * Add unit tests for builder with different scenarios Signed-off-by: Sanket Sudake Co-authored-by: Sanket Sudake --- pkg/builder/builder.go | 45 ++++++---- pkg/builder/builder_test.go | 171 ++++++++++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+), 19 deletions(-) create mode 100644 pkg/builder/builder_test.go diff --git a/pkg/builder/builder.go b/pkg/builder/builder.go index ec2bc92f..5536f1f8 100644 --- a/pkg/builder/builder.go +++ b/pkg/builder/builder.go @@ -38,8 +38,8 @@ import ( const ( // supported environment variables - envSrcPkg = "SRC_PKG" - envDeployPkg = "DEPLOY_PKG" + envSrcPkg string = "SRC_PKG" + envDeployPkg string = "DEPLOY_PKG" ) type ( @@ -114,16 +114,27 @@ func (builder *Builder) Handler(w http.ResponseWriter, r *http.Request) { } builder.logger.Info("builder received request", zap.Any("request", req)) - builder.logger.Info("starting build") + builder.logger.Debug("starting build") srcPkgPath := filepath.Join(builder.sharedVolumePath, req.SrcPkgFilename) - deployPkgFilename := fmt.Sprintf("%v-%v", req.SrcPkgFilename, strings.ToLower(uniuri.NewLen(6))) + deployPkgFilename := fmt.Sprintf("%s-%s", req.SrcPkgFilename, strings.ToLower(uniuri.NewLen(6))) deployPkgPath := filepath.Join(builder.sharedVolumePath, deployPkgFilename) + + var buildArgs []string buildCmd := req.BuildCommand if len(buildCmd) == 0 { // use default build command buildCmd = "/build" + } else { + // split executable command and arguments + args := strings.Split(buildCmd, " ") + buildCmd = args[0] // get the executable command, executable command will always be on Zero index + + // get all the arguments + for i := 1; i < len(args); i++ { + buildArgs = append(buildArgs, args[i]) + } } - buildLogs, err := builder.build(buildCmd, srcPkgPath, deployPkgPath) + buildLogs, err := builder.build(buildCmd, buildArgs, srcPkgPath, deployPkgPath) if err != nil { e := "error building source package" builder.logger.Error(e, zap.Error(err)) @@ -146,7 +157,7 @@ func (builder *Builder) reply(w http.ResponseWriter, pkgFilename string, buildLo rBody, err := json.Marshal(resp) if err != nil { e := errors.Wrap(err, "error encoding response body") - rBody = []byte(fmt.Sprintf(`{"buildLogs": "%v"}`, e.Error())) + rBody = []byte(fmt.Sprintf(`{"buildLogs": "%s"}`, e.Error())) statusCode = http.StatusInternalServerError } @@ -163,8 +174,8 @@ func (builder *Builder) reply(w http.ResponseWriter, pkgFilename string, buildLo } } -func (builder *Builder) build(command string, srcPkgPath string, deployPkgPath string) (string, error) { - cmd := exec.Command(command) +func (builder *Builder) build(command string, args []string, srcPkgPath string, deployPkgPath string) (string, error) { + cmd := exec.Command(command, args...) fi, err := os.Stat(srcPkgPath) if err != nil { @@ -178,8 +189,8 @@ func (builder *Builder) build(command string, srcPkgPath string, deployPkgPath s // set env variables for build command cmd.Env = append(os.Environ(), - fmt.Sprintf("%v=%v", envSrcPkg, srcPkgPath), - fmt.Sprintf("%v=%v", envDeployPkg, deployPkgPath), + fmt.Sprintf("%s=%s", envSrcPkg, srcPkgPath), + fmt.Sprintf("%s=%s", envDeployPkg, deployPkgPath), ) stdout, err := cmd.StdoutPipe() @@ -192,12 +203,8 @@ func (builder *Builder) build(command string, srcPkgPath string, deployPkgPath s return "", errors.Wrap(err, "error creating stderr pipe for cmd") } - var buildLogs string - - fmt.Printf("\n=== Build Logs ===") // Init logs - fmt.Printf("command=%v\n", command) - fmt.Printf("env=%v\n", cmd.Env) + builder.logger.Info("building source package", zap.String("command", command), zap.Strings("args", args), zap.Strings("env", cmd.Env)) out := io.MultiReader(stdout, stderr) scanner := bufio.NewScanner(out) @@ -206,12 +213,14 @@ func (builder *Builder) build(command string, srcPkgPath string, deployPkgPath s if err != nil { return "", errors.Wrap(err, "error starting cmd") } - + fmt.Printf("========= START =========\n") + defer fmt.Printf("========= END ===========\n") + var buildLogs string // Runtime logs for scanner.Scan() { output := scanner.Text() fmt.Println(output) - buildLogs += fmt.Sprintf("%v\n", output) + buildLogs += fmt.Sprintf("%s\n", output) } if err := scanner.Err(); err != nil { @@ -226,7 +235,5 @@ func (builder *Builder) build(command string, srcPkgPath string, deployPkgPath s fmt.Println(cmdErr) return buildLogs, cmdErr } - fmt.Printf("==================\n") - return buildLogs, nil } diff --git a/pkg/builder/builder_test.go b/pkg/builder/builder_test.go new file mode 100644 index 00000000..1ae46048 --- /dev/null +++ b/pkg/builder/builder_test.go @@ -0,0 +1,171 @@ +/* +Copyright 2022 The Fission Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package builder + +import ( + "bytes" + "encoding/json" + "io/ioutil" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + "github.com/fission/fission/pkg/utils/loggerfactory" +) + +func TestBuilder(t *testing.T) { + logger := loggerfactory.GetLogger() + + dir, err := os.MkdirTemp("/tmp", "fission-builder-test") + if err != nil { + t.Fatal(err) + } + builder := MakeBuilder(logger, dir) + + // Test VersionHandler + t.Run("VersionHandler", func(t *testing.T) { + t.Run("should return version", func(t *testing.T) { + w := &httptest.ResponseRecorder{} + r := &http.Request{} + builder.VersionHandler(w, r) + if w.Result().StatusCode != http.StatusOK { + t.Errorf("expected status code %d, got %d", http.StatusOK, w.Result().StatusCode) + } + if w.Result().Body == nil { + t.Error("expected body, got nil") + } + }) + }) + + // Test BuildHandler + t.Run("BuildHandler", func(t *testing.T) { + + for _, test := range []struct { + name string + buildRequest *PackageBuildRequest + expected *PackageBuildResponse + status int + }{ + { + name: "should work with build command without argument", + buildRequest: &PackageBuildRequest{ + SrcPkgFilename: "test", + BuildCommand: "ls", + }, + expected: &PackageBuildResponse{ + ArtifactFilename: "test", + BuildLogs: "", + }, + status: http.StatusOK, + }, + { + name: "should work with build command with argument", + buildRequest: &PackageBuildRequest{ + SrcPkgFilename: "test1", + BuildCommand: "ls -la", + }, + expected: &PackageBuildResponse{ + ArtifactFilename: "test1", + BuildLogs: "", + }, + status: http.StatusOK, + }, + { + name: "should fail with argument and pipe", + buildRequest: &PackageBuildRequest{ + SrcPkgFilename: "test2", + BuildCommand: "ps -ef | grep fission", + }, + expected: &PackageBuildResponse{ + ArtifactFilename: "test2", + BuildLogs: "", + }, + status: http.StatusInternalServerError, + }, + { + name: "should fail with invalid build command", + buildRequest: &PackageBuildRequest{ + SrcPkgFilename: "test3", + BuildCommand: "lsalas -la", + }, + expected: &PackageBuildResponse{ + ArtifactFilename: "test3", + BuildLogs: "", + }, + status: http.StatusInternalServerError, + }, + { + name: "should fail with valid command and invalid argument", + buildRequest: &PackageBuildRequest{ + SrcPkgFilename: "test3", + BuildCommand: "ls --la", + }, + expected: &PackageBuildResponse{ + ArtifactFilename: "test3", + BuildLogs: "", + }, + status: http.StatusInternalServerError, + }, + } { + t.Run(test.name, func(t *testing.T) { + srcFile, err := os.Create(dir + "/" + test.buildRequest.SrcPkgFilename) + if err != nil { + t.Fatal(err) + } + defer srcFile.Close() + body, err := json.Marshal(test.buildRequest) + if err != nil { + t.Fatal(err) + } + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(body)) + builder.Handler(w, r) + resp := w.Result() + if resp.StatusCode != test.status { + t.Errorf("expected status code %d, got %d", test.status, resp.StatusCode) + } + body, err = ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + var buildResp PackageBuildResponse + err = json.Unmarshal(body, &buildResp) + if err != nil { + t.Fatal(err) + } + if test.status == http.StatusOK { + if strings.Contains(buildResp.BuildLogs, "error") { + t.Errorf("expected build logs to not contain error, got %s", buildResp.BuildLogs) + } + artifacts := strings.Split(buildResp.ArtifactFilename, "-") + if len(artifacts) != 2 { + t.Errorf("expected artifact filename to be of the form -, got %s", buildResp.ArtifactFilename) + } + if artifacts[0] != test.expected.ArtifactFilename { + t.Errorf("expected artifact filename to be %s, got %s", test.expected.ArtifactFilename, artifacts[0]) + } + } else { + if !strings.Contains(buildResp.BuildLogs, "error") { + t.Errorf("expected build logs to contain error, got %s", buildResp.BuildLogs) + } + } + + }) + } + }) +}