From ed8775505caf956111ba44ee978db244db62d5e8 Mon Sep 17 00:00:00 2001 From: Henry Wang Date: Thu, 5 Sep 2024 23:03:53 -0700 Subject: [PATCH] Revert "Add linter to CI (#24)" This reverts commit 4cb929b97df9e04209e6d58c9a8843686014262b. --- .github/ISSUE_TEMPLATE/config.yaml | 2 +- .github/dependabot.yaml | 28 ++--- .github/workflows/ci.yaml | 146 ++++++++++------------ .golangci.yaml | 14 +-- .yamllint.yml | 10 -- api/handlers/container/attach.go | 5 +- api/handlers/container/attach_test.go | 6 +- api/handlers/container/create.go | 2 +- api/handlers/container/list_test.go | 2 +- api/handlers/container/logs_test.go | 4 +- api/handlers/exec/start_test.go | 2 +- api/handlers/image/push_test.go | 2 +- api/handlers/network/network_test.go | 4 +- e2e/tests/tests.go | 4 +- go.mod | 4 +- internal/service/container/attach_test.go | 4 +- internal/service/container/logs_test.go | 4 +- internal/service/image/load_test.go | 2 +- internal/service/network/list_test.go | 2 +- 19 files changed, 110 insertions(+), 137 deletions(-) delete mode 100644 .yamllint.yml diff --git a/.github/ISSUE_TEMPLATE/config.yaml b/.github/ISSUE_TEMPLATE/config.yaml index ba822f0..9fb406d 100644 --- a/.github/ISSUE_TEMPLATE/config.yaml +++ b/.github/ISSUE_TEMPLATE/config.yaml @@ -1,5 +1,5 @@ blank_issues_enabled: false contact_links: - - name: Ask a question + - name: Ask a question url: https://github.com/runfinch/finch-daemon/discussions about: Use GitHub Discussions to ask questions, discuss options, or propose new ideas diff --git a/.github/dependabot.yaml b/.github/dependabot.yaml index 888a05e..e22ac0e 100644 --- a/.github/dependabot.yaml +++ b/.github/dependabot.yaml @@ -1,16 +1,16 @@ version: 2 updates: - - package-ecosystem: "gomod" - directory: "/" - schedule: - interval: "daily" - commit-message: - prefix: "build" - include: "scope" - - package-ecosystem: "github-actions" - directory: "/" - schedule: - interval: "daily" - commit-message: - prefix: "ci" - include: "scope" +- package-ecosystem: "gomod" + directory: "/" + schedule: + interval: "daily" + commit-message: + prefix: "build" + include: "scope" +- package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "daily" + commit-message: + prefix: "ci" + include: "scope" diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c22bd2c..5b33671 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -1,86 +1,68 @@ name: CI on: - push: - branches: - - main - paths-ignore: - - '**.md' - pull_request: - branches: - - main - paths-ignore: - - '**.md' - workflow_dispatch: + push: + branches: + - main + paths-ignore: + - '**.md' + pull_request: + branches: + - main + paths-ignore: + - '**.md' + workflow_dispatch: env: - GO_VERSION: '1.23.0' - GOLANGCI_LINT_VERSION: '1.60.3' + GO_VERSION: '1.22.6' + jobs: - git-secrets: - runs-on: ubuntu-latest - steps: - - name: Pull latest awslabs/git-secrets repo - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 - with: - repository: awslabs/git-secrets - ref: 1.3.0 - fetch-tags: true - path: git-secrets - - name: Install git secrets from source - run: sudo make install - working-directory: git-secrets - - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 - - name: Scan repository for git secrets - run: | - git secrets --register-aws - git secrets --scan-history - lint: - strategy: - matrix: - working_dir: ['.'] - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 - - uses: golangci/golangci-lint-action@v6 # v6.1.0 - with: - version: v${{ env.GOLANGCI_LINT_VERSION }} - working-directory: ${{ matrix.working_dir }} - args: --fix=false --timeout=5m - yamllint: - name: yamllint-lint - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 - - run: yamllint . - unit-test: - runs-on: ubuntu-latest - steps: - - uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2 - with: - go-version: ${{ env.GO_VERSION }} - cache: false - - name: Checkout finch-deamon repo - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 - - name: Build and run unit tests - run: | - make build - make run-unit-tests - e2e-test: - runs-on: ubuntu-latest - steps: - - uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2 - with: - go-version: ${{ env.GO_VERSION }} - - name: Checkout finch-daemon repo - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 - - name: Install Dependencies for e2e Testing - run: ./setup-test-env.sh - - name: Build the daemon - run: make build - - name: Remove default podman network config - run: | - sudo ls /etc/cni/net.d - sudo rm /etc/cni/net.d/87-podman-bridge.conflist - - name: Start finch-daemon - run: sudo bin/finch-daemon --debug --socket-owner $UID & - - name: Run e2e test - run: sudo make run-e2e-tests + git-secrets: + runs-on: ubuntu-latest + steps: + - name: Pull latest awslabs/git-secrets repo + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 + with: + repository: awslabs/git-secrets + ref: 1.3.0 + fetch-tags: true + path: git-secrets + - name: Install git secrets from source + run: sudo make install + working-directory: git-secrets + - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 + - name: Scan repository for git secrets + run: | + git secrets --register-aws + git secrets --scan-history + unit-test: + runs-on: ubuntu-latest + steps: + - uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2 + with: + go-version: ${{ env.GO_VERSION }} + cache: false + - name: Checkout finch-deamon repo + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 + - name: Build and run unit tests + run: | + make build + make run-unit-tests + e2e-test: + runs-on: ubuntu-latest + steps: + - uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2 + with: + go-version: ${{ env.GO_VERSION }} + - name: Checkout finch-daemon repo + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 + - name: Install Dependencies for e2e Testing + run: ./setup-test-env.sh + - name: Build the daemon + run: make build + - name: Remove default podman network config + run: | + sudo ls /etc/cni/net.d + sudo rm /etc/cni/net.d/87-podman-bridge.conflist + - name: Start finch-daemon + run: sudo bin/finch-daemon --debug --socket-owner $UID & + - name: Run e2e test + run: sudo make run-e2e-tests diff --git a/.golangci.yaml b/.golangci.yaml index 6e4f547..937678a 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,5 +1,4 @@ -# The sections in this file are ordered in the order presented in -# https://golangci-lint.run/usage/configuration/. +# The sections in this file are ordered in the order presented in https://golangci-lint.run/usage/configuration/. # The nested fields are ordered alphabetically. linters-settings: @@ -14,7 +13,6 @@ linters-settings: - G304 - G101 - G104 - - G115 # added to prevent false uint conversion errors lll: line-length: 250 tab-width: 4 @@ -25,11 +23,12 @@ linters-settings: require-specific: true stylecheck: checks: ["all", "-ST1003", "-ST1000", "-ST1001", "-ST1021"] + linters: enable: # - errname # - errorlint - - copyloopvar + - exportloopref # - forcetypeassert # - gocritic - godot @@ -57,17 +56,16 @@ linters: - stylecheck disable: - errcheck + issues: exclude-rules: - linters: - lll - # A go:generate statement has to be in the same line: - # https://github.com/golang/go/issues/46050. + # A go:generate statement has to be in the same line: https://github.com/golang/go/issues/46050. source: "^//go:generate " + # Some checks enabled in the stylecheck setting are disabled by default # (e.g., https://golangci-lint.run/usage/false-positives/#exc0013), # so we need to enable them explicitly here. exclude-use-default: false # fix: true -run: - timeout: 5m diff --git a/.yamllint.yml b/.yamllint.yml deleted file mode 100644 index c2dfe13..0000000 --- a/.yamllint.yml +++ /dev/null @@ -1,10 +0,0 @@ -extends: default -rules: - trailing-spaces: - # issue templates intentionally have some whitespace - ignore: .github/ISSUE_TEMPLATE/ - document-start: disable # preference, can enable if desired - comments: disable # doesn't like in-line comments like this - line-length: disable # hard to enforce <80 chars for some cmds - truthy: - allowed-values: ['true', 'false', 'on'] diff --git a/api/handlers/container/attach.go b/api/handlers/container/attach.go index c407cd5..62f082e 100644 --- a/api/handlers/container/attach.go +++ b/api/handlers/container/attach.go @@ -104,7 +104,10 @@ func checkUpgradeStatus(ctx context.Context, upgrade bool) (string, string) { if versions.GreaterThanOrEqualTo(httputils.VersionFromContext(ctx), "1.42") { contentType = "application/vnd.docker.multiplexed-stream" } - successResponse = fmt.Sprintf("HTTP/1.1 101 UPGRADED\r\nContent-Type: %s\r\nConnection: Upgrade\r\nUpgrade: tcp\r\n\r\n", contentType) + successResponse = fmt.Sprintf("HTTP/1.1 101 UPGRADED\r\n" + + "Content-Type: " + contentType + "\r\n" + + "Connection: Upgrade\r\n" + + "Upgrade: tcp\r\n\r\n") } return contentType, successResponse } diff --git a/api/handlers/container/attach_test.go b/api/handlers/container/attach_test.go index 1ccf05d..dddb296 100644 --- a/api/handlers/container/attach_test.go +++ b/api/handlers/container/attach_test.go @@ -72,7 +72,7 @@ var _ = Describe("Container Attach API", func() { expErrCode := http.StatusInternalServerError expErrMsg := "error" service.EXPECT().Attach(gomock.Any(), gomock.Any(), gomock.Any()). - Return(fmt.Errorf("%s", expErrMsg)) + Return(fmt.Errorf(expErrMsg)) req, _ = http.NewRequest(http.MethodPost, "/containers/123", nil) h.attach(rr, req) @@ -86,7 +86,7 @@ var _ = Describe("Container Attach API", func() { expErrCode := http.StatusNotFound expErrMsg := fmt.Sprintf("no container is found given the string: %s", "123") service.EXPECT().Attach(gomock.Any(), gomock.Any(), gomock.Any()). - Return(errdefs.NewNotFound(fmt.Errorf("%s", expErrMsg))) + Return(errdefs.NewNotFound(fmt.Errorf(expErrMsg))) req, _ = http.NewRequest(http.MethodPost, "/containers/123", nil) h.attach(rr, req) @@ -243,7 +243,7 @@ func (h *errorResponseRecorder) Code() int { } func (h *errorResponseRecorder) Hijack() (net.Conn, *bufio.ReadWriter, error) { - return nil, nil, fmt.Errorf("%s", errRRErrMsg) + return nil, nil, fmt.Errorf(errRRErrMsg) } // attachOptsMatcher is adapted from container create to be a wrapper type to diff --git a/api/handlers/container/create.go b/api/handlers/container/create.go index de87e7e..4497f0d 100644 --- a/api/handlers/container/create.go +++ b/api/handlers/container/create.go @@ -157,7 +157,7 @@ func (h *handler) create(w http.ResponseWriter, r *http.Request) { Env: env, Workdir: req.WorkingDir, Entrypoint: req.Entrypoint, - EntrypointChanged: len(req.Entrypoint) > 0, + EntrypointChanged: req.Entrypoint != nil && len(req.Entrypoint) > 0, // #endregion // #region for metadata flags diff --git a/api/handlers/container/list_test.go b/api/handlers/container/list_test.go index 7826e43..de29000 100644 --- a/api/handlers/container/list_test.go +++ b/api/handlers/container/list_test.go @@ -137,7 +137,7 @@ var _ = Describe("Container List API", func() { Filters: nil, } errorMsg := "error from ListContainers" - service.EXPECT().List(gomock.Any(), listOpts).Return(nil, fmt.Errorf("%s", errorMsg)) + service.EXPECT().List(gomock.Any(), listOpts).Return(nil, fmt.Errorf(errorMsg)) h.list(rr, req) Expect(rr.Body).Should(MatchJSON(`{"message": "` + errorMsg + `"}`)) diff --git a/api/handlers/container/logs_test.go b/api/handlers/container/logs_test.go index ec7ea3b..5471f1c 100644 --- a/api/handlers/container/logs_test.go +++ b/api/handlers/container/logs_test.go @@ -70,7 +70,7 @@ var _ = Describe("Container Logs API", func() { expErrCode := http.StatusInternalServerError expErrMsg := "error" service.EXPECT().Logs(gomock.Any(), gomock.Any(), gomock.Any()). - Return(fmt.Errorf("%s", expErrMsg)) + Return(fmt.Errorf(expErrMsg)) h.logs(rr, req) @@ -83,7 +83,7 @@ var _ = Describe("Container Logs API", func() { expErrCode := http.StatusNotFound expErrMsg := fmt.Sprintf("no container is found given the string: %s", "123") service.EXPECT().Logs(gomock.Any(), gomock.Any(), gomock.Any()). - Return(errdefs.NewNotFound(fmt.Errorf("%s", expErrMsg))) + Return(errdefs.NewNotFound(fmt.Errorf(expErrMsg))) h.logs(rr, req) diff --git a/api/handlers/exec/start_test.go b/api/handlers/exec/start_test.go index a2ed482..a49f38b 100644 --- a/api/handlers/exec/start_test.go +++ b/api/handlers/exec/start_test.go @@ -140,7 +140,7 @@ var _ = Describe("Exec Start API", func() { It("should return 500 if Hijacking the connection fails", func() { hijackErrMsg := "error hijacking the connection" errRR := newResponseRecorderWithMockHijack(rr, func() (net.Conn, *bufio.ReadWriter, error) { - return nil, nil, fmt.Errorf("%s", hijackErrMsg) + return nil, nil, fmt.Errorf(hijackErrMsg) }) h.start(errRR, req) diff --git a/api/handlers/image/push_test.go b/api/handlers/image/push_test.go index 06b5bdd..3fa90bd 100644 --- a/api/handlers/image/push_test.go +++ b/api/handlers/image/push_test.go @@ -164,7 +164,7 @@ var _ = Describe("Image Push API", func() { Expect(authCfg.Password).Should(BeEmpty()) // mimic service is trying to push the image and failed with auth error. outStream.Write([]byte(streamMsg)) - return nil, fmt.Errorf("%s", errMsg) + return nil, fmt.Errorf(errMsg) }) // handler should return error message with 500 status code diff --git a/api/handlers/network/network_test.go b/api/handlers/network/network_test.go index 8bdbe4f..ba51c27 100644 --- a/api/handlers/network/network_test.go +++ b/api/handlers/network/network_test.go @@ -80,7 +80,7 @@ var _ = Describe("Network API ", func() { It("should call the network list handler using /networks", func() { // setup mocks expErr := "error from List" - service.EXPECT().List(gomock.Any()).Return(nil, fmt.Errorf("%s", expErr)) + service.EXPECT().List(gomock.Any()).Return(nil, fmt.Errorf(expErr)) req, _ = http.NewRequest(http.MethodGet, "/networks", nil) // call api and check if it returns error router.ServeHTTP(rr, req) @@ -90,7 +90,7 @@ var _ = Describe("Network API ", func() { It("should call the network list handler using /networks/", func() { // setup mocks expErr := "error from List" - service.EXPECT().List(gomock.Any()).Return(nil, fmt.Errorf("%s", expErr)) + service.EXPECT().List(gomock.Any()).Return(nil, fmt.Errorf(expErr)) req, _ = http.NewRequest(http.MethodGet, "/networks/", nil) // call api and check if it returns error router.ServeHTTP(rr, req) diff --git a/e2e/tests/tests.go b/e2e/tests/tests.go index 0a2e28d..74f0b1c 100644 --- a/e2e/tests/tests.go +++ b/e2e/tests/tests.go @@ -174,13 +174,13 @@ func fileShouldExistInContainer(opt *option.Option, containerName, path, content gomega.Expect(command.StdoutStr(opt, "exec", containerName, "cat", path)).To(gomega.Equal(content)) } -//nolint:unused // reserved for future use cases +//nolint: unused // reserved for future use cases func fileShouldNotExistInContainer(opt *option.Option, containerName, path string) { cmdOut := command.RunWithoutSuccessfulExit(opt, "exec", containerName, "cat", path) gomega.Expect(cmdOut.Err.Contents()).To(gomega.ContainSubstring("No such file or directory")) } -//nolint:unused // reserved for future use cases +//nolint: unused // reserved for future use cases func buildImage(opt *option.Option, imageName string) { dockerfile := fmt.Sprintf(`FROM %s CMD ["echo", "finch-test-dummy-output"] diff --git a/go.mod b/go.mod index aa325ce..80e2f9a 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/runfinch/finch-daemon -go 1.22.7 +go 1.21.0 require ( github.com/containerd/cgroups/v3 v3.0.3 @@ -147,4 +147,4 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/cri-api v0.29.3 // indirect lukechampine.com/blake3 v1.2.1 // indirect -) \ No newline at end of file +) diff --git a/internal/service/container/attach_test.go b/internal/service/container/attach_test.go index 454c2ed..19fc254 100644 --- a/internal/service/container/attach_test.go +++ b/internal/service/container/attach_test.go @@ -116,7 +116,7 @@ var _ = Describe("Container Attach API ", func() { cdClient.EXPECT().SearchContainer(gomock.Any(), gomock.Any()).Return([]containerd.Container{con}, nil) logger.EXPECT().Debugf(gomock.Any(), gomock.Any()).Return() con.EXPECT().ID().Return(cid) - ncClient.EXPECT().GetDataStore().Return("", fmt.Errorf("%s", expErr)) + ncClient.EXPECT().GetDataStore().Return("", fmt.Errorf(expErr)) logger.EXPECT().Debugf(gomock.Any(), gomock.Any()).Return() // set up options opts := attachTypes.AttachOptions{ @@ -244,7 +244,7 @@ var _ = Describe("Container Attach API ", func() { k8slabels.ContainerMetadataExtension: testAny, }, nil) cdClient.EXPECT().GetContainerStatus(gomock.Any(), gomock.Any()).Return(containerd.Running) - cdClient.EXPECT().GetContainerTaskWait(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil, fmt.Errorf("%s", expErr)) + cdClient.EXPECT().GetContainerTaskWait(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil, fmt.Errorf(expErr)) logger.EXPECT().Debugf(gomock.Any(), gomock.Any()).Return() // set up options diff --git a/internal/service/container/logs_test.go b/internal/service/container/logs_test.go index 6311488..321901a 100644 --- a/internal/service/container/logs_test.go +++ b/internal/service/container/logs_test.go @@ -97,7 +97,7 @@ var _ = Describe("Container Logs API ", func() { cdClient.EXPECT().SearchContainer(gomock.Any(), gomock.Any()).Return([]containerd.Container{con}, nil) logger.EXPECT().Infof("getting logs for container: %s", cid).Return() con.EXPECT().ID().Return(cid) - ncClient.EXPECT().GetDataStore().Return("", fmt.Errorf("%s", expErr)) + ncClient.EXPECT().GetDataStore().Return("", fmt.Errorf(expErr)) logger.EXPECT().Debugf(gomock.Any(), gomock.Any()).Return() // set up options opts := types.LogsOptions{ @@ -233,7 +233,7 @@ var _ = Describe("Container Logs API ", func() { k8slabels.ContainerMetadataExtension: testAny, }, nil) cdClient.EXPECT().GetContainerStatus(gomock.Any(), gomock.Any()).Return(containerd.Running) - cdClient.EXPECT().GetContainerTaskWait(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil, fmt.Errorf("%s", expErr)) + cdClient.EXPECT().GetContainerTaskWait(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil, fmt.Errorf(expErr)) logger.EXPECT().Debugf(gomock.Any(), gomock.Any()).Return() // set up options diff --git a/internal/service/image/load_test.go b/internal/service/image/load_test.go index f6fc394..c48064e 100644 --- a/internal/service/image/load_test.go +++ b/internal/service/image/load_test.go @@ -18,7 +18,7 @@ import ( "github.com/runfinch/finch-daemon/mocks/mocks_logger" ) -// Unit tests related to image load API. +// Unit tests related to image load API var _ = Describe("Image Load API", func() { var ( ctx context.Context diff --git a/internal/service/network/list_test.go b/internal/service/network/list_test.go index 15b2e95..9d827ca 100644 --- a/internal/service/network/list_test.go +++ b/internal/service/network/list_test.go @@ -46,7 +46,7 @@ var _ = Describe("Network List API ", func() { }) It("should pass through errors from FilterNetworks", func() { expErr := "filter network error" - ncNetClient.EXPECT().FilterNetworks(gomock.Any()).Return(nil, fmt.Errorf("%s", expErr)) + ncNetClient.EXPECT().FilterNetworks(gomock.Any()).Return(nil, fmt.Errorf(expErr)) _, err := service.List(ctx) Expect(err.Error()).Should(Equal(expErr))