Skip to content

Commit

Permalink
osbuild-worker-executor: fix lint warnings/errors
Browse files Browse the repository at this point in the history
The osbuild-composer linting found a bunch of issues that this
commit fixes.
  • Loading branch information
croissanne authored and mvo5 committed Jun 5, 2024
1 parent 5a2f2c3 commit 2fa36fc
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 31 deletions.
5 changes: 2 additions & 3 deletions cmd/osbuild-worker-executor/build_result.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package main

import (
"io/ioutil"
"os"
"path/filepath"
)
Expand All @@ -20,9 +19,9 @@ func newBuildResult(config *Config) *buildResult {

func (br *buildResult) Mark(err error) error {
if err == nil {
return ioutil.WriteFile(br.resultGood, nil, 0600)
return os.WriteFile(br.resultGood, nil, 0600)
} else {
return ioutil.WriteFile(br.resultBad, nil, 0600)
return os.WriteFile(br.resultBad, nil, 0600)
}
}

Expand Down
5 changes: 3 additions & 2 deletions cmd/osbuild-worker-executor/export_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package main

import (
"io/ioutil"
"os"
"path/filepath"
"testing"

Expand Down Expand Up @@ -35,7 +35,8 @@ func MockOsbuildBinary(t *testing.T, new string) (restore func()) {

tmpdir := t.TempDir()
osbuildBinary = filepath.Join(tmpdir, "fake-osbuild")
if err := ioutil.WriteFile(osbuildBinary, []byte(new), 0755); err != nil {
/* #nosec G306 */
if err := os.WriteFile(osbuildBinary, []byte(new), 0755); err != nil {
t.Fatal(err)
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/osbuild-worker-executor/handler_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func runOsbuild(buildDir string, control *controlJSON, output io.Writer) (string
if err := cmd.Wait(); err != nil {
// we cannot use "http.Error()" here because the http
// header was already set to "201" when we started streaming
mw.Write([]byte(fmt.Sprintf("cannot run osbuild: %v", err)))
_, _ = mw.Write([]byte(fmt.Sprintf("cannot run osbuild: %v", err)))
return "", err
}

Expand All @@ -90,7 +90,7 @@ func runOsbuild(buildDir string, control *controlJSON, output io.Writer) (string
if err != nil {
err = fmt.Errorf("cannot tar output directory: %w, output:\n%s", err, out)
logrus.Errorf(err.Error())
mw.Write([]byte(err.Error()))
_, _ = mw.Write([]byte(err.Error()))
return "", err
}
logrus.Infof("tar output:\n%s", out)
Expand Down
27 changes: 14 additions & 13 deletions cmd/osbuild-worker-executor/handler_build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"bufio"
"bytes"
"fmt"
"io/ioutil"
"io"
"net/http"
"os"
"os/exec"
Expand Down Expand Up @@ -50,7 +50,7 @@ func TestBuildChecksContentType(t *testing.T) {
assert.NoError(t, err)
defer rsp.Body.Close()
assert.Equal(t, rsp.StatusCode, http.StatusUnsupportedMediaType)
body, err := ioutil.ReadAll(rsp.Body)
body, err := io.ReadAll(rsp.Body)
assert.NoError(t, err)
assert.Equal(t, string(body), "Content-Type must be [application/x-tar], got random/encoding\n")
}
Expand Down Expand Up @@ -102,20 +102,21 @@ echo "fake-build-result" > %[1]s/build/output/image/disk.img
buf := makeTestPost(t, `{"exports": ["tree"], "environments": ["MY=env"]}`, `{"fake": "manifest"}`)
rsp, err := http.Post(endpoint, "application/x-tar", buf)

Check failure on line 103 in cmd/osbuild-worker-executor/handler_build_test.go

View workflow job for this annotation

GitHub Actions / ⌨ Golang Lint

G107: Potential HTTP request made with variable url (gosec)
assert.NoError(t, err)
defer ioutil.ReadAll(rsp.Body)
defer io.ReadAll(rsp.Body)

Check failure on line 105 in cmd/osbuild-worker-executor/handler_build_test.go

View workflow job for this annotation

GitHub Actions / ⌨ Golang Lint

Error return value of `io.ReadAll` is not checked (errcheck)
defer rsp.Body.Close()

assert.Equal(t, rsp.StatusCode, http.StatusCreated)
reader := bufio.NewReader(rsp.Body)

// check that we get the output of osbuild streamed to us
expectedContent := fmt.Sprintf(`fake-osbuild --export tree --output-dir %[1]s/build/output --store %[1]s/build/store --json
---
{"fake": "manifest"}`, baseBuildDir)
content, err := ioutil.ReadAll(reader)
content, err := io.ReadAll(reader)
assert.NoError(t, err)
assert.Equal(t, string(content), expectedContent)
// check log too
logFileContent, err := ioutil.ReadFile(filepath.Join(baseBuildDir, "build/build.log"))
logFileContent, err := os.ReadFile(filepath.Join(baseBuildDir, "build/build.log"))
assert.NoError(t, err)
assert.Equal(t, expectedContent, string(logFileContent))
// check that the "store" dir got created
Expand All @@ -129,7 +130,7 @@ echo "fake-build-result" > %[1]s/build/output/image/disk.img
assert.NoError(t, err)
defer rsp.Body.Close()
assert.Equal(t, http.StatusOK, rsp.StatusCode)
body, err := ioutil.ReadAll(rsp.Body)
body, err := io.ReadAll(rsp.Body)
assert.NoError(t, err)
assert.Equal(t, "fake-build-result\n", string(body))

Expand All @@ -139,7 +140,7 @@ echo "fake-build-result" > %[1]s/build/output/image/disk.img
assert.NoError(t, err)
defer rsp.Body.Close()
assert.Equal(t, http.StatusOK, rsp.StatusCode)
body, err = ioutil.ReadAll(rsp.Body)
body, err = io.ReadAll(rsp.Body)
assert.NoError(t, err)
tarPath := filepath.Join(baseBuildDir, "output.tar")
assert.NoError(t, os.WriteFile(tarPath, body, 0644))
Expand All @@ -164,7 +165,7 @@ echo "fake-build-result" > %[1]s/build/output/image/disk.img
rsp, err := http.Post(endpoint, "application/x-tar", buf)
assert.NoError(t, err)
assert.Equal(t, rsp.StatusCode, http.StatusCreated)
defer ioutil.ReadAll(rsp.Body)
defer io.ReadAll(rsp.Body)

Check failure on line 168 in cmd/osbuild-worker-executor/handler_build_test.go

View workflow job for this annotation

GitHub Actions / ⌨ Golang Lint

Error return value of `io.ReadAll` is not checked (errcheck)
defer rsp.Body.Close()

buf = makeTestPost(t, `{"exports": ["tree"]}`, `{"fake": "manifest"}`)
Expand Down Expand Up @@ -232,12 +233,12 @@ exit 23
buf := makeTestPost(t, `{"exports": ["tree"], "environments": ["MY=env"]}`, `{"fake": "manifest"}`)
rsp, err := http.Post(endpoint, "application/x-tar", buf)
assert.NoError(t, err)
defer ioutil.ReadAll(rsp.Body)
defer io.ReadAll(rsp.Body)

Check failure on line 236 in cmd/osbuild-worker-executor/handler_build_test.go

View workflow job for this annotation

GitHub Actions / ⌨ Golang Lint

Error return value of `io.ReadAll` is not checked (errcheck)
defer rsp.Body.Close()

assert.Equal(t, rsp.StatusCode, http.StatusCreated)
reader := bufio.NewReader(rsp.Body)
content, err := ioutil.ReadAll(reader)
content, err := io.ReadAll(reader)
assert.NoError(t, err)
expectedContent := `err on stdout
err on stderr
Expand All @@ -251,7 +252,7 @@ cannot run osbuild: exit status 23`
defer rsp.Body.Close()
assert.Equal(t, http.StatusBadRequest, rsp.StatusCode)
reader = bufio.NewReader(rsp.Body)
content, err = ioutil.ReadAll(reader)
content, err = io.ReadAll(reader)
assert.NoError(t, err)
assert.Equal(t, "build failed\n"+expectedContent, string(content))
}
Expand All @@ -276,7 +277,7 @@ echo "fake-build-result" > %[1]s/build/output/image/disk.img
buf := makeTestPost(t, `{"exports": ["tree"], "environments": ["MY=env"]}`, `{"fake": "manifest"}`)
rsp, err := http.Post(endpoint, "application/x-tar", buf)
assert.NoError(t, err)
defer ioutil.ReadAll(rsp.Body)
defer io.ReadAll(rsp.Body)

Check failure on line 280 in cmd/osbuild-worker-executor/handler_build_test.go

View workflow job for this annotation

GitHub Actions / ⌨ Golang Lint

Error return value of `io.ReadAll` is not checked (errcheck)
defer rsp.Body.Close()

assert.Equal(t, rsp.StatusCode, http.StatusCreated)
Expand Down Expand Up @@ -314,7 +315,7 @@ func TestBuildErrorHandlingTar(t *testing.T) {
defer rsp.Body.Close()
assert.Equal(t, rsp.StatusCode, http.StatusCreated)

body, err := ioutil.ReadAll(rsp.Body)
body, err := io.ReadAll(rsp.Body)
assert.NoError(t, err)
assert.Contains(t, string(body), "cannot tar output directory:")
assert.Contains(t, loggerHook.LastEntry().Message, "cannot tar output directory:")
Expand Down
4 changes: 3 additions & 1 deletion cmd/osbuild-worker-executor/handler_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ func handleResult(logger *logrus.Logger, config *Config) http.Handler {
return
}
defer f.Close()
io.Copy(w, f)
if _, err := io.Copy(w, f); err != nil {
logger.Errorf("Unable to write log to response")
}
return
case buildResult.Good():
// good result
Expand Down
14 changes: 7 additions & 7 deletions cmd/osbuild-worker-executor/handler_result_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package main_test

import (
"io/ioutil"
"io"
"net/http"
"os"
"path/filepath"
Expand All @@ -27,15 +27,15 @@ func TestResultBad(t *testing.T) {
// todo: make a nice helper method
err := os.MkdirAll(filepath.Join(buildBaseDir, "build"), 0755)
assert.NoError(t, err)
err = ioutil.WriteFile(filepath.Join(buildBaseDir, "result.bad"), nil, 0644)
err = os.WriteFile(filepath.Join(buildBaseDir, "result.bad"), nil, 0644)
assert.NoError(t, err)
err = ioutil.WriteFile(filepath.Join(buildBaseDir, "build/build.log"), []byte("failure log"), 0644)
err = os.WriteFile(filepath.Join(buildBaseDir, "build/build.log"), []byte("failure log"), 0644)
assert.NoError(t, err)

rsp, err := http.Get(endpoint)
assert.NoError(t, err)
assert.Equal(t, http.StatusBadRequest, rsp.StatusCode)
body, err := ioutil.ReadAll(rsp.Body)
body, err := io.ReadAll(rsp.Body)
assert.NoError(t, err)
assert.Equal(t, "build failed\nfailure log", string(body))
}
Expand All @@ -48,15 +48,15 @@ func TestResultGood(t *testing.T) {
// todo: make a nice helper method
err := os.MkdirAll(filepath.Join(buildBaseDir, "build/output"), 0755)
assert.NoError(t, err)
err = ioutil.WriteFile(filepath.Join(buildBaseDir, "result.good"), nil, 0644)
err = os.WriteFile(filepath.Join(buildBaseDir, "result.good"), nil, 0644)
assert.NoError(t, err)
err = ioutil.WriteFile(filepath.Join(buildBaseDir, "build/output/disk.img"), []byte("fake-build-result"), 0644)
err = os.WriteFile(filepath.Join(buildBaseDir, "build/output/disk.img"), []byte("fake-build-result"), 0644)
assert.NoError(t, err)

rsp, err := http.Get(endpoint)
assert.NoError(t, err)
assert.Equal(t, http.StatusOK, rsp.StatusCode)
body, err := ioutil.ReadAll(rsp.Body)
body, err := io.ReadAll(rsp.Body)
assert.NoError(t, err)
assert.Equal(t, "fake-build-result", string(body))
}
3 changes: 1 addition & 2 deletions cmd/osbuild-worker-executor/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ func run(ctx context.Context, args []string, getenv func(string) string) error {
go func() {
defer wg.Done()
<-ctx.Done()
shutdownCtx := context.Background()
shutdownCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
shutdownCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
if err := httpServer.Shutdown(shutdownCtx); err != nil {
fmt.Fprintf(os.Stderr, "error shutting down http server: %s\n", err)
Expand Down
4 changes: 3 additions & 1 deletion cmd/osbuild-worker-executor/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ func runTestServer(t *testing.T) (baseURL, buildBaseDir string, loggerHook *logr
"-port", port,
"-build-path", buildBaseDir,
}
go main.Run(ctx, args, os.Getenv)
go func() {
_ = main.Run(ctx, args, os.Getenv)
}()

err := waitReady(ctx, defaultTimeout, baseURL)
assert.NoError(t, err)
Expand Down

0 comments on commit 2fa36fc

Please sign in to comment.