Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add linter to CI #24

Merged
merged 2 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/config.yaml
Original file line number Diff line number Diff line change
@@ -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
28 changes: 14 additions & 14 deletions .github/dependabot.yaml
Original file line number Diff line number Diff line change
@@ -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"
146 changes: 82 additions & 64 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
@@ -1,68 +1,86 @@
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.22.6'

GO_VERSION: '1.23.0'
GOLANGCI_LINT_VERSION: '1.60.3'
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
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
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
14 changes: 8 additions & 6 deletions .golangci.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# 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:
Expand All @@ -13,6 +14,7 @@ linters-settings:
- G304
- G101
- G104
- G115 # added to prevent false uint conversion errors
lll:
line-length: 250
tab-width: 4
Expand All @@ -23,12 +25,11 @@ linters-settings:
require-specific: true
stylecheck:
checks: ["all", "-ST1003", "-ST1000", "-ST1001", "-ST1021"]

linters:
enable:
# - errname
# - errorlint
- exportloopref
- copyloopvar
# - forcetypeassert
# - gocritic
- godot
Expand Down Expand Up @@ -56,16 +57,17 @@ 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
10 changes: 10 additions & 0 deletions .yamllint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
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']
5 changes: 1 addition & 4 deletions api/handlers/container/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,7 @@ 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\n" +
"Content-Type: " + contentType + "\r\n" +
"Connection: Upgrade\r\n" +
"Upgrade: tcp\r\n\r\n")
successResponse = fmt.Sprintf("HTTP/1.1 101 UPGRADED\r\nContent-Type: %s\r\nConnection: Upgrade\r\nUpgrade: tcp\r\n\r\n", contentType)
}
return contentType, successResponse
}
Expand Down
6 changes: 3 additions & 3 deletions api/handlers/container/attach_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(expErrMsg))
Return(fmt.Errorf("%s", expErrMsg))
req, _ = http.NewRequest(http.MethodPost, "/containers/123", nil)

h.attach(rr, req)
Expand All @@ -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(expErrMsg)))
Return(errdefs.NewNotFound(fmt.Errorf("%s", expErrMsg)))
req, _ = http.NewRequest(http.MethodPost, "/containers/123", nil)

h.attach(rr, req)
Expand Down Expand Up @@ -243,7 +243,7 @@ func (h *errorResponseRecorder) Code() int {
}

func (h *errorResponseRecorder) Hijack() (net.Conn, *bufio.ReadWriter, error) {
return nil, nil, fmt.Errorf(errRRErrMsg)
return nil, nil, fmt.Errorf("%s", errRRErrMsg)
}

// attachOptsMatcher is adapted from container create to be a wrapper type to
Expand Down
2 changes: 1 addition & 1 deletion api/handlers/container/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (h *handler) create(w http.ResponseWriter, r *http.Request) {
Env: env,
Workdir: req.WorkingDir,
Entrypoint: req.Entrypoint,
EntrypointChanged: req.Entrypoint != nil && len(req.Entrypoint) > 0,
EntrypointChanged: len(req.Entrypoint) > 0,
// #endregion

// #region for metadata flags
Expand Down
2 changes: 1 addition & 1 deletion api/handlers/container/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(errorMsg))
service.EXPECT().List(gomock.Any(), listOpts).Return(nil, fmt.Errorf("%s", errorMsg))

h.list(rr, req)
Expect(rr.Body).Should(MatchJSON(`{"message": "` + errorMsg + `"}`))
Expand Down
4 changes: 2 additions & 2 deletions api/handlers/container/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(expErrMsg))
Return(fmt.Errorf("%s", expErrMsg))

h.logs(rr, req)

Expand All @@ -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(expErrMsg)))
Return(errdefs.NewNotFound(fmt.Errorf("%s", expErrMsg)))

h.logs(rr, req)

Expand Down
2 changes: 1 addition & 1 deletion api/handlers/exec/start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(hijackErrMsg)
return nil, nil, fmt.Errorf("%s", hijackErrMsg)
})

h.start(errRR, req)
Expand Down
2 changes: 1 addition & 1 deletion api/handlers/image/push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(errMsg)
return nil, fmt.Errorf("%s", errMsg)
})

// handler should return error message with 500 status code
Expand Down
4 changes: 2 additions & 2 deletions api/handlers/network/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(expErr))
service.EXPECT().List(gomock.Any()).Return(nil, fmt.Errorf("%s", expErr))
req, _ = http.NewRequest(http.MethodGet, "/networks", nil)
// call api and check if it returns error
router.ServeHTTP(rr, req)
Expand All @@ -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(expErr))
service.EXPECT().List(gomock.Any()).Return(nil, fmt.Errorf("%s", expErr))
req, _ = http.NewRequest(http.MethodGet, "/networks/", nil)
// call api and check if it returns error
router.ServeHTTP(rr, req)
Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/runfinch/finch-daemon

go 1.21.0
go 1.22.7

require (
github.com/containerd/cgroups/v3 v3.0.3
Expand Down Expand Up @@ -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
)
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: newline :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a new line.. line number is still 150

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean you're missing the newline character at the end of file. See the red marker here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh shoot, My bad. Will add this back in the next commit.

Loading
Loading