From 3bfba85aae1758828959d8562ef8ef67b48538e6 Mon Sep 17 00:00:00 2001 From: MSevey <15232757+MSevey@users.noreply.github.com> Date: Mon, 3 Mar 2025 11:51:55 -0500 Subject: [PATCH 1/4] feat(ci): adds baseline project CI --- .githooks/pre-commit | 14 ++ .github/auto_request_review.yml | 15 ++ .github/dependabot.yml | 34 ++++ .github/workflows/approve_merge_bots.yml | 24 +++ .github/workflows/ci.yml | 55 ++++++ .github/workflows/housekeeping.yml | 18 ++ .github/workflows/project-automation.yml | 48 +++++ .github/workflows/release.yml | 6 +- .github/workflows/semantic-pull-request.yml | 20 ++ .github/workflows/stale.yml | 24 +++ .golangci.yml | 45 +++++ .yamllint.yml | 11 ++ LICENSE | 201 ++++++++++++++++++++ Makefile | 88 +++++++++ cmd/agent/main.go | 5 +- config.yaml | 2 +- go.mod | 15 +- go.sum | 22 ++- internal/api/client.go | 9 +- internal/api/client_test.go | 12 +- internal/config/config.go | 1 + internal/config/config_test.go | 6 +- internal/http/server.go | 24 ++- internal/http/server_test.go | 19 +- internal/logging/logger.go | 2 +- internal/logging/logger_test.go | 86 +++++++-- internal/metrics/collector.go | 3 +- internal/metrics/telemetry.go | 9 +- internal/metrics/telemetry_test.go | 91 +++++---- 29 files changed, 826 insertions(+), 83 deletions(-) create mode 100755 .githooks/pre-commit create mode 100644 .github/auto_request_review.yml create mode 100644 .github/dependabot.yml create mode 100644 .github/workflows/approve_merge_bots.yml create mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/housekeeping.yml create mode 100644 .github/workflows/project-automation.yml create mode 100644 .github/workflows/semantic-pull-request.yml create mode 100644 .github/workflows/stale.yml create mode 100644 .golangci.yml create mode 100644 .yamllint.yml create mode 100644 LICENSE create mode 100644 Makefile diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 0000000..eade811 --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,14 @@ +#!/bin/sh + +# Run make lint before committing +echo "Running linters..." +make lint + +# Check the exit status +if [ $? -ne 0 ]; then + echo "❌ Linting failed. Please fix the issues before committing." + exit 1 +fi + +echo "✅ Linting passed." +exit 0 \ No newline at end of file diff --git a/.github/auto_request_review.yml b/.github/auto_request_review.yml new file mode 100644 index 0000000..d4f40f6 --- /dev/null +++ b/.github/auto_request_review.yml @@ -0,0 +1,15 @@ +reviewers: + defaults: + - devops + groups: + devops: + - team:devops +files: + ".github/**": + - MSevey + - devops +options: + ignore_draft: true + ignored_keywords: + - WIP + number_of_reviewers: 3 diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..b870a91 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,34 @@ +--- +version: 2 +updates: + - package-ecosystem: "gomod" + directory: "/" + schedule: + interval: "weekly" + open-pull-requests-limit: 10 + # Group all patch updates into a single PR + groups: + patch-updates: + applies-to: version-updates + update-types: + - "patch" + - "minor" + commit-message: + include: "scope" + prefix: "build" + + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + open-pull-requests-limit: 10 + # Group all patch updates into a single PR + groups: + patch-updates: + applies-to: version-updates + update-types: + - "patch" + - "minor" + commit-message: + include: "scope" + prefix: "build" diff --git a/.github/workflows/approve_merge_bots.yml b/.github/workflows/approve_merge_bots.yml new file mode 100644 index 0000000..a9883fa --- /dev/null +++ b/.github/workflows/approve_merge_bots.yml @@ -0,0 +1,24 @@ +--- +name: Approve and Merge Dependabot PRs +on: + # This is needed to grant permissions for secrets because dependabot PRs are opened by bots + pull_request_target: + +jobs: + dependabot: + name: "Approve and Merge Dependabot PRs" + if: ${{ github.actor == 'dependabot[bot]' }} + runs-on: ubuntu-latest + permissions: + pull-requests: write + steps: + - uses: actions/checkout@v4 + - name: CelestiaBot Approval + run: gh pr review --approve "$PR_URL" + # Leaving out the auto merge step until we have 2 approvals enforced + # run: | + # gh pr review --approve "$PR_URL" + # gh pr merge --auto --squash "$PR_URL" + env: + PR_URL: ${{github.event.pull_request.html_url}} + GH_TOKEN: ${{secrets.PR_APPROVE_PAT_CB}} # should be used automatically by gh cli diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..da15a49 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,55 @@ +--- +name: CI + +on: + push: + branches: + - main + tags: + - v* + pull_request: + merge_group: + +jobs: + yamllint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: celestiaorg/.github/.github/actions/yamllint@v0.5.0 + + golangci-lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: 'go.mod' + - uses: golangci/golangci-lint-action@v6.1.0 + + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version-file: 'go.mod' + + - name: Run tests + run: make test + + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: 'go.mod' + + - name: Build main application + run: make build + + - name: Build CLI + run: make build-cli diff --git a/.github/workflows/housekeeping.yml b/.github/workflows/housekeeping.yml new file mode 100644 index 0000000..d4a7794 --- /dev/null +++ b/.github/workflows/housekeeping.yml @@ -0,0 +1,18 @@ +--- +name: Auto Request Review + +on: + pull_request_target: + types: [opened, ready_for_review] + +jobs: + auto-add-reviewer: + name: Auto add reviewer to PR + if: github.event.pull_request + uses: celestiaorg/.github/.github/workflows/reusable_housekeeping.yml@v0.5.0 + secrets: inherit + permissions: + issues: write + pull-requests: write + with: + run-auto-request-review: true diff --git a/.github/workflows/project-automation.yml b/.github/workflows/project-automation.yml new file mode 100644 index 0000000..28be1a1 --- /dev/null +++ b/.github/workflows/project-automation.yml @@ -0,0 +1,48 @@ +--- +name: DevOps project automation + +on: + issues: + types: + - opened + # https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target + # About security concerns: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ + pull_request_target: + types: + - opened + - ready_for_review + +jobs: + project-automation: + runs-on: ubuntu-latest + permissions: + issues: write + pull-requests: write + steps: + - name: Set issue url and creator login + if: ${{ github.event.issue }} + run: | + echo "ISSUE=${{ github.event.issue.html_url }}" >> $GITHUB_ENV + echo "CREATOR=${{ github.event.issue.user.login }}" >> $GITHUB_ENV + echo "HAS_ASSIGNEE=${{ github.event.issue.assignees[0] != null }}" >> $GITHUB_ENV + - name: Set pull_request url and creator login + if: ${{ github.event.pull_request }} + run: | + echo "ISSUE=${{ github.event.pull_request.html_url }}" >> $GITHUB_ENV + echo "CREATOR=${{ github.event.pull_request.user.login }}" >> $GITHUB_ENV + echo "HAS_ASSIGNEE=${{ github.event.pull_request.assignees[0] != null }}" >> $GITHUB_ENV + - name: Add issue/PR to project + uses: actions/add-to-project@v1.0.2 + with: + project-url: https://github.com/orgs/celestiaorg/projects/38 + github-token: ${{ secrets.ADD_TO_PROJECT_PAT }} + - name: Assign issue to creator (issue) + if: ${{ github.event.issue && env.HAS_ASSIGNEE == 'false' && env.CREATOR != 'dependabot[bot]' }} + run: gh issue edit ${{ env.ISSUE }} --add-assignee ${{ env.CREATOR }} + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Assign issue to creator (PR) + if: ${{ github.event.pull_request && env.HAS_ASSIGNEE == 'false' && env.CREATOR != 'dependabot[bot]' }} + run: gh pr edit ${{ env.ISSUE }} --add-assignee ${{ env.CREATOR }} + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b5d8d33..b63c8ce 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-latest permissions: contents: write # Needed for creating releases - + steps: - name: Checkout code uses: actions/checkout@v4 @@ -39,7 +39,7 @@ jobs: - name: Create Release id: create_release - uses: softprops/action-gh-release@v1 + uses: softprops/action-gh-release@v2 with: name: Release ${{ steps.get_version.outputs.version }} draft: false @@ -55,4 +55,4 @@ jobs: with: name: debian-package path: build/talis-agent_${{ steps.get_version.outputs.version }}_*.deb - retention-days: 5 \ No newline at end of file + retention-days: 5 diff --git a/.github/workflows/semantic-pull-request.yml b/.github/workflows/semantic-pull-request.yml new file mode 100644 index 0000000..e11fe30 --- /dev/null +++ b/.github/workflows/semantic-pull-request.yml @@ -0,0 +1,20 @@ +name: Semantic Pull Request + +on: + pull_request_target: + types: + - opened + - edited + - synchronize + +permissions: + pull-requests: read + +jobs: + main: + name: conventional-commit-pr-title + runs-on: ubuntu-latest + steps: + - uses: amannn/action-semantic-pull-request@v5 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml new file mode 100644 index 0000000..083ba75 --- /dev/null +++ b/.github/workflows/stale.yml @@ -0,0 +1,24 @@ +--- +name: stale +on: + schedule: + - cron: "0 0 * * *" + +jobs: + stale: + permissions: + issues: write + pull-requests: write + runs-on: ubuntu-latest + steps: + - uses: actions/stale@v9 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + stale-issue-message: > + This issue has been automatically marked as stale because it + has not had recent activity. It will be closed if no further + activity occurs. Thank you for your contributions. + stale-pr-message: > + This pull request has been automatically marked as stale because it + has not had recent activity. It will be closed if no further + activity occurs. Thank you for your contributions. diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..58220a4 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,45 @@ +--- +run: + timeout: 5m + modules-download-mode: readonly + +linters: + enable: + - errorlint + - errcheck + - gofmt + - goimports + - gosec + - gosimple + - govet + - ineffassign + - misspell + - revive + - staticcheck + - typecheck + - unconvert + - unused + +issues: + exclude-use-default: false + # mempool and indexer code is borrowed from Tendermint + include: + - EXC0012 # EXC0012 revive: Annoying issue about not having a comment. The rare codebase has such comments + - EXC0014 # EXC0014 revive: Annoying issue about not having a comment. The rare codebase has such comments + +linters-settings: + gosec: + excludes: + - G115 + revive: + rules: + - name: package-comments + disabled: true + - name: duplicated-imports + severity: warning + - name: exported + arguments: + - disableStutteringCheck + + goimports: + local-prefixes: github.com/celestiaorg diff --git a/.yamllint.yml b/.yamllint.yml new file mode 100644 index 0000000..23b0ad6 --- /dev/null +++ b/.yamllint.yml @@ -0,0 +1,11 @@ +--- +# Built from docs https://yamllint.readthedocs.io/en/stable/configuration.html +extends: default + +rules: + # 120 chars should be enough, but don't fail if a line is longer + line-length: + max: 120 + level: warning + # Disable truthy values check (yes, Yes, TRUE, etc) + truthy: disable diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..261eeb9 --- /dev/null +++ b/LICENSE @@ -0,0 +1,201 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + 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. diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..9287b4a --- /dev/null +++ b/Makefile @@ -0,0 +1,88 @@ +# Variables +GO_FILES := $(shell find . -name "*.go" -type f) +NIX_FILES := $(shell find . -name "*.nix" -type f) +PROJECTNAME=$(shell basename "$(PWD)") + +# Go commands +GO := go +GOTEST := $(GO) test +GOVET := $(GO) vet +GOFMT := gofmt +GOMOD := $(GO) mod +GOBUILD := $(GO) build + +# Build flags +LDFLAGS := -ldflags="-s -w" + +## help: Get more info on make commands. +help: Makefile + @echo " Choose a command run in "$(PROJECTNAME)":" + @sed -n 's/^##//p' $< | column -t -s ':' | sed -e 's/^/ /' +.PHONY: help + +## all: Run check-env, lint, test, and build +all: lint test build +.PHONY: all + +## build: Build the application +build: + @echo "Building $(PROJECTNAME)..." + $(GOBUILD) $(LDFLAGS) -o bin/$(PROJECTNAME) ./cmd/main.go +.PHONY: build + +## clean: Clean build artifacts +clean: + @echo "Cleaning..." + rm -rf bin/ + rm -rf dist/ +.PHONY: clean + +## test: Run tests +test: + @echo "Running tests..." + $(GOTEST) -v ./... +.PHONY: test + +## fmt: Format code +fmt: + @echo "Formatting go fmt..." + $(GOFMT) -w $(GO_FILES) + @echo "--> Formatting golangci-lint" + @golangci-lint run --fix +.PHONY: fmt + +## lint: Run all linters +lint: fmt vet + @echo "Running linters..." + @echo "--> Running golangci-lint" + @golangci-lint run + @echo "--> Running actionlint" + @actionlint + @echo "--> Running yamllint" + @yamllint --no-warnings . +.PHONY: lint + +## vet: Run go vet +vet: + @echo "Running go vet..." + $(GOVET) ./... +.PHONY: vet + +## tidy: Tidy and verify dependencies +tidy: + @echo "Tidying dependencies..." + $(GOMOD) tidy + $(GOMOD) verify +.PHONY: tidy + +## run: Run the application +run: + @echo "Running $(PROJECTNAME)..." + @go run ./cmd/main.go +.PHONY: run + +## install-hooks: Install git hooks +install-hooks: + @echo "Installing git hooks..." + @git config core.hooksPath .githooks +.PHONY: install-hooks diff --git a/cmd/agent/main.go b/cmd/agent/main.go index c1b9e29..d59e79a 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "flag" "fmt" "os" @@ -87,7 +88,7 @@ func main() { logging.Info(). Str("interval", cfg.Metrics.CollectionInterval.String()). Msg("Starting telemetry client") - if err := telemetryClient.Start(ctx); err != nil && err != context.Canceled { + if err := telemetryClient.Start(ctx); err != nil && !errors.Is(err, context.Canceled) { logging.Error().Err(err).Msg("Telemetry client error") } }() @@ -101,7 +102,7 @@ func main() { Int("port", cfg.HTTPPort). Msg("Starting HTTP server") if err := server.Start(); err != nil { - if err != http.ErrServerClosed { + if errors.Is(err, http.ErrServerClosed) { logging.Error().Err(err).Msg("HTTP server error") } } diff --git a/config.yaml b/config.yaml index 6982ff2..fd7b9ac 100644 --- a/config.yaml +++ b/config.yaml @@ -9,4 +9,4 @@ metrics: telemetry: "/v1/agent/telemetry" checkin: "/v1/agent/checkin" payload: - path: "/etc/talis-agent/payload" \ No newline at end of file + path: "/etc/talis-agent/payload" diff --git a/go.mod b/go.mod index 5d5760d..34f6a70 100644 --- a/go.mod +++ b/go.mod @@ -2,31 +2,34 @@ module github.com/celestiaorg/talis-agent go 1.23.5 -require gopkg.in/yaml.v3 v3.0.1 +require ( + github.com/prometheus/client_golang v1.21.0 + github.com/rs/zerolog v1.33.0 + github.com/shirou/gopsutil/v3 v3.24.5 + golang.org/x/time v0.10.0 + gopkg.in/natefinch/lumberjack.v2 v2.2.1 + gopkg.in/yaml.v3 v3.0.1 +) require ( github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/go-ole/go-ole v1.2.6 // indirect github.com/klauspost/compress v1.17.11 // indirect + github.com/kr/text v0.2.0 // indirect github.com/kylelemons/godebug v1.1.0 // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.19 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect - github.com/prometheus/client_golang v1.21.0 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.62.0 // indirect github.com/prometheus/procfs v0.15.1 // indirect - github.com/rs/zerolog v1.33.0 // indirect - github.com/shirou/gopsutil/v3 v3.24.5 // indirect github.com/shoenig/go-m1cpu v0.1.6 // indirect github.com/tklauser/go-sysconf v0.3.12 // indirect github.com/tklauser/numcpus v0.6.1 // indirect github.com/yusufpapurcu/wmi v1.2.4 // indirect golang.org/x/sys v0.28.0 // indirect - golang.org/x/time v0.10.0 // indirect google.golang.org/protobuf v1.36.1 // indirect - gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect ) diff --git a/go.sum b/go.sum index aed2c2d..f5bbaec 100644 --- a/go.sum +++ b/go.sum @@ -3,12 +3,21 @@ github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6r github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= +github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/go-ole/go-ole v1.2.6 h1:/Fpf6oFPoeFik9ty7siob0G6Ke8QvQEuVcuChpwXzpY= github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/klauspost/compress v1.17.11 h1:In6xLpyWOi1+C7tXUUWv2ot1QvBjxevKAaI6IXrJmUc= github.com/klauspost/compress v1.17.11/go.mod h1:pMDklpSncoRMuLFrf1W9Ss9KT+0rH90U12bZKk7uwG0= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ81pIr0yLvtUWk2if982qA3F3QD6H4= @@ -21,6 +30,8 @@ github.com/mattn/go-isatty v0.0.19/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c h1:ncq/mPwQF4JjgDlrVEn3C11VoGHZN7m8qihwgMEtzYw= github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE= github.com/prometheus/client_golang v1.21.0 h1:DIsaGmiaBkSangBgMtWdNfxbMNdku5IK6iNhrEqWvdA= @@ -31,6 +42,8 @@ github.com/prometheus/common v0.62.0 h1:xasJaQlnWAeyHdUBeGjXmutelfJHWMRr+Fg4QszZ github.com/prometheus/common v0.62.0/go.mod h1:vyBcEuLSvWos9B1+CyL7JZ2up+uFzXhkqml0W5zIY1I= github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0learggepc= github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk= +github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= +github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog= github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/rs/zerolog v1.33.0 h1:1cU2KZkvPxNyfgEmhHAz/1A9Bz+llsdYzklWFzgp0r8= github.com/rs/zerolog v1.33.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss= @@ -38,6 +51,10 @@ github.com/shirou/gopsutil/v3 v3.24.5 h1:i0t8kL+kQTvpAYToeuiVk3TgDeKOFioZO3Ztz/i github.com/shirou/gopsutil/v3 v3.24.5/go.mod h1:bsoOS1aStSs9ErQ1WWfxllSeS1K5D+U30r2NfcubMVk= github.com/shoenig/go-m1cpu v0.1.6 h1:nxdKQNcEB6vzgA2E2bvzKIYRuNj7XNJ4S/aRSwKzFtM= github.com/shoenig/go-m1cpu v0.1.6/go.mod h1:1JJMcUBvfNwpq05QDQVAnx3gUHr9IYF7GNg9SUEw2VQ= +github.com/shoenig/test v0.6.4 h1:kVTaSd7WLz5WZ2IaoM0RSzRsUD+m8wRR+5qvntpn4LU= +github.com/shoenig/test v0.6.4/go.mod h1:byHiCGXqrVaflBLAMq/srcZIHynQPQgeyvkvXnjqq0k= +github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= +github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/tklauser/go-sysconf v0.3.12 h1:0QaGUFOdQaIVdPgfITYzaTegZvdCjmYO52cSFAEVmqU= github.com/tklauser/go-sysconf v0.3.12/go.mod h1:Ho14jnntGE1fpdOqQEEaiKRpvIavV0hSfmBq8nJbHYI= github.com/tklauser/numcpus v0.6.1 h1:ng9scYS7az0Bk4OZLvrNXNSAO2Pxr1XXRAPyjhIx+Fk= @@ -51,8 +68,6 @@ golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y= -golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA= golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/time v0.10.0 h1:3usCWA8tQn0L8+hFJQNgzpWbd89begxN66o1Ojdn5L4= @@ -60,8 +75,9 @@ golang.org/x/time v0.10.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/protobuf v1.36.1 h1:yBPeRvTftaleIgM3PZ/WBIZ7XM/eEYAaEyCwvyjq/gk= google.golang.org/protobuf v1.36.1/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/natefinch/lumberjack.v2 v2.2.1 h1:bBRl1b0OH9s/DuPhuXpNl+VtCaJXFZ5/uEFST95x9zc= gopkg.in/natefinch/lumberjack.v2 v2.2.1/go.mod h1:YD8tP3GAjkrDg1eZH7EGmyESg/lsYskCTPBJVb9jqSc= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/internal/api/client.go b/internal/api/client.go index 1b7c1a5..d91c022 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -10,8 +10,9 @@ import ( "sync" "time" - "github.com/celestiaorg/talis-agent/internal/logging" "golang.org/x/time/rate" + + "github.com/celestiaorg/talis-agent/internal/logging" ) // CircuitBreakerState represents the state of the circuit breaker @@ -145,7 +146,11 @@ func (c *Client) doRequest(ctx context.Context, method, path string, body interf if err != nil { return nil, fmt.Errorf("request failed: %w", err) } - defer resp.Body.Close() + defer func() { + if cerr := resp.Body.Close(); cerr != nil { + err = fmt.Errorf("error closing response body: %w", cerr) + } + }() respBody, err := io.ReadAll(resp.Body) if err != nil { diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 810f5d8..c01e196 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -60,7 +60,9 @@ func TestRequest(t *testing.T) { // Return success response w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(map[string]string{"status": "success"}) + if err := json.NewEncoder(w).Encode(map[string]string{"status": "success"}); err != nil { + t.Errorf("Failed to encode response: %v", err) + } })) defer server.Close() @@ -104,7 +106,9 @@ func TestCircuitBreaker(t *testing.T) { return } w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(map[string]string{"status": "success"}) + if err := json.NewEncoder(w).Encode(map[string]string{"status": "success"}); err != nil { + t.Errorf("Failed to encode response: %v", err) + } })) defer server.Close() @@ -157,7 +161,9 @@ func TestRateLimiting(t *testing.T) { // Create test server server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(map[string]string{"status": "success"}) + if err := json.NewEncoder(w).Encode(map[string]string{"status": "success"}); err != nil { + t.Errorf("Failed to encode response: %v", err) + } })) defer server.Close() diff --git a/internal/config/config.go b/internal/config/config.go index 617dfb2..9c2f0c7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -35,6 +35,7 @@ type PayloadConfig struct { // Load reads and parses the configuration file func Load(path string) (*Config, error) { + // #nosec G304 -- This is a configuration file that needs to be read from a variable path data, err := os.ReadFile(path) if err != nil { return nil, fmt.Errorf("failed to read config file: %w", err) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 84cb1c4..5cd8853 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -26,7 +26,11 @@ payload: if err != nil { t.Fatalf("Failed to create temp file: %v", err) } - defer os.Remove(tmpfile.Name()) + defer func() { + if err := os.Remove(tmpfile.Name()); err != nil { + t.Errorf("Failed to remove temporary file: %v", err) + } + }() if _, err := tmpfile.Write([]byte(configContent)); err != nil { t.Fatalf("Failed to write config content: %v", err) diff --git a/internal/http/server.go b/internal/http/server.go index f2c9122..95218d5 100644 --- a/internal/http/server.go +++ b/internal/http/server.go @@ -10,11 +10,13 @@ import ( "os" "os/exec" "path/filepath" + "time" + + "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/celestiaorg/talis-agent/internal/config" "github.com/celestiaorg/talis-agent/internal/logging" "github.com/celestiaorg/talis-agent/internal/metrics" - "github.com/prometheus/client_golang/prometheus/promhttp" ) // ErrServerClosed is returned by the Server's Start method after a call to Shutdown @@ -43,8 +45,9 @@ func (s *Server) Start() error { // Create server with configured port s.srv = &http.Server{ - Addr: fmt.Sprintf(":%d", s.config.HTTPPort), - Handler: mux, + Addr: fmt.Sprintf(":%d", s.config.HTTPPort), + Handler: mux, + ReadHeaderTimeout: 10 * time.Second, // Prevent Slowloris attacks } logging.Info().Int("port", s.config.HTTPPort).Msg("Starting HTTP server") @@ -86,7 +89,8 @@ func (s *Server) handlePayload(w http.ResponseWriter, r *http.Request) { // Create directory if it doesn't exist dir := filepath.Dir(s.config.Payload.Path) - if err := os.MkdirAll(dir, 0755); err != nil { + // #nosec G301 -- This directory needs to be readable by other processes + if err := os.MkdirAll(dir, 0750); err != nil { logging.Error(). Err(err). Str("directory", dir). @@ -105,7 +109,16 @@ func (s *Server) handlePayload(w http.ResponseWriter, r *http.Request) { http.Error(w, "Internal server error", http.StatusInternalServerError) return } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + logging.Error(). + Err(err). + Msg("Failed to close file") + if err == nil { // Only set the error if there wasn't one already + http.Error(w, "Failed to close file", http.StatusInternalServerError) + } + } + }() // Copy request body to file and count bytes written, err := io.Copy(file, r.Body) @@ -177,6 +190,7 @@ func (s *Server) handleCommands(w http.ResponseWriter, r *http.Request) { Str("command", req.Command). Msg("Executing command") + // #nosec G204 -- Command execution is a core feature of this endpoint cmd := exec.Command("bash", "-c", req.Command) output, err := cmd.CombinedOutput() diff --git a/internal/http/server_test.go b/internal/http/server_test.go index c5b4d5e..5be035a 100644 --- a/internal/http/server_test.go +++ b/internal/http/server_test.go @@ -20,7 +20,11 @@ func TestHandlePayload(t *testing.T) { if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + t.Errorf("Failed to remove temporary directory: %v", err) + } + }() payloadPath := filepath.Join(tmpDir, "payload") @@ -73,7 +77,11 @@ func TestHandlePayload(t *testing.T) { server.handlePayload(w, req) resp := w.Result() - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + t.Errorf("Failed to close response body: %v", err) + } + }() if resp.StatusCode != tt.expectedCode { t.Errorf("Expected status code %d, got %d", tt.expectedCode, resp.StatusCode) @@ -90,6 +98,7 @@ func TestHandlePayload(t *testing.T) { } } else if tt.expectedCode == http.StatusOK { // Verify payload was written correctly + // #nosec G304 -- This is a test file that needs to be read from a variable path content, err := os.ReadFile(payloadPath) if err != nil { t.Fatalf("Failed to read payload file: %v", err) @@ -165,7 +174,11 @@ func TestHandleCommands(t *testing.T) { server.handleCommands(w, req) resp := w.Result() - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + t.Errorf("Failed to close response body: %v", err) + } + }() if resp.StatusCode != tt.expectedCode { t.Errorf("Expected status code %d, got %d", tt.expectedCode, resp.StatusCode) diff --git a/internal/logging/logger.go b/internal/logging/logger.go index 2e2b002..a15edee 100644 --- a/internal/logging/logger.go +++ b/internal/logging/logger.go @@ -72,7 +72,7 @@ func InitLogger(cfg Config) error { // Configure file output if requested if cfg.File != nil { // Create log directory if it doesn't exist - if err := os.MkdirAll(filepath.Dir(cfg.File.Path), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(cfg.File.Path), 0750); err != nil { return err } diff --git a/internal/logging/logger_test.go b/internal/logging/logger_test.go index 7db6cb2..66e76cc 100644 --- a/internal/logging/logger_test.go +++ b/internal/logging/logger_test.go @@ -20,7 +20,11 @@ func TestLoggerInitialization(t *testing.T) { if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + t.Errorf("Failed to remove temporary directory: %v", err) + } + }() logPath := filepath.Join(tmpDir, "test.log") @@ -98,7 +102,11 @@ func TestLogLevels(t *testing.T) { if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + t.Errorf("Failed to remove temporary directory: %v", err) + } + }() logPath := filepath.Join(tmpDir, "test.log") @@ -127,7 +135,7 @@ func TestLogLevels(t *testing.T) { // Read and verify log file time.Sleep(100 * time.Millisecond) // Wait for logs to be written - logs, err := readLogFile(logPath) + logs, err := readLogFile(t, logPath) if err != nil { t.Fatalf("Failed to read log file: %v", err) } @@ -177,7 +185,11 @@ func TestLogRotation(t *testing.T) { if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + t.Errorf("Failed to remove temporary directory: %v", err) + } + }() logPath := filepath.Join(tmpDir, "test.log") @@ -225,7 +237,11 @@ func TestLogLevelFiltering(t *testing.T) { if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + t.Errorf("Failed to remove temporary directory: %v", err) + } + }() tests := []struct { name string @@ -306,7 +322,7 @@ func TestLogLevelFiltering(t *testing.T) { } time.Sleep(100 * time.Millisecond) - logs, err := readLogFile(logPath) + logs, err := readLogFile(t, logPath) if err != nil { t.Fatalf("Failed to read log file: %v", err) } @@ -337,7 +353,11 @@ func TestStructuredLogging(t *testing.T) { if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + t.Errorf("Failed to remove temporary directory: %v", err) + } + }() logPath := filepath.Join(tmpDir, "structured.log") config := Config{ @@ -365,7 +385,7 @@ func TestStructuredLogging(t *testing.T) { Msg("structured log test") time.Sleep(100 * time.Millisecond) - logs, err := readStructuredLogFile(logPath) + logs, err := readStructuredLogFile(t, logPath) if err != nil { t.Fatalf("Failed to read log file: %v", err) } @@ -399,7 +419,11 @@ func TestConcurrentLogging(t *testing.T) { if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + t.Errorf("Failed to remove temporary directory: %v", err) + } + }() logPath := filepath.Join(tmpDir, "concurrent.log") config := Config{ @@ -439,7 +463,7 @@ func TestConcurrentLogging(t *testing.T) { wg.Wait() time.Sleep(100 * time.Millisecond) - logs, err := readLogFile(logPath) + logs, err := readLogFile(t, logPath) if err != nil { t.Fatalf("Failed to read log file: %v", err) } @@ -463,7 +487,11 @@ func TestLogFilePermissions(t *testing.T) { if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + t.Errorf("Failed to remove temporary directory: %v", err) + } + }() logPath := filepath.Join(tmpDir, "permissions.log") config := Config{ @@ -503,12 +531,17 @@ type logEntry struct { Timestamp time.Time `json:"time"` } -func readLogFile(path string) ([]logEntry, error) { +func readLogFile(t *testing.T, path string) ([]logEntry, error) { + // #nosec G304 -- This is a test file that needs to be read from a variable path file, err := os.Open(path) if err != nil { return nil, err } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + t.Errorf("Failed to close file: %v", err) + } + }() var entries []logEntry scanner := bufio.NewScanner(file) @@ -533,12 +566,17 @@ type structuredLogEntry struct { Boolean bool `json:"boolean"` } -func readStructuredLogFile(path string) ([]structuredLogEntry, error) { +func readStructuredLogFile(t *testing.T, path string) ([]structuredLogEntry, error) { + // #nosec G304 -- This is a test file that needs to be read from a variable path file, err := os.Open(path) if err != nil { return nil, err } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + t.Errorf("Failed to close file: %v", err) + } + }() var entries []structuredLogEntry scanner := bufio.NewScanner(file) @@ -559,7 +597,11 @@ func BenchmarkLogging(b *testing.B) { if err != nil { b.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + b.Errorf("Failed to remove temporary directory: %v", err) + } + }() benchmarks := []struct { name string @@ -668,7 +710,11 @@ func BenchmarkConcurrentLogging(b *testing.B) { if err != nil { b.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + b.Errorf("Failed to remove temporary directory: %v", err) + } + }() config := Config{ Level: "info", @@ -725,7 +771,11 @@ func BenchmarkLogLevels(b *testing.B) { if err != nil { b.Fatalf("Failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + b.Errorf("Failed to remove temporary directory: %v", err) + } + }() levels := []string{"debug", "info", "warn", "error"} logFuncs := map[string]func() *zerolog.Event{ diff --git a/internal/metrics/collector.go b/internal/metrics/collector.go index 1844d8f..7599ef0 100644 --- a/internal/metrics/collector.go +++ b/internal/metrics/collector.go @@ -4,12 +4,13 @@ import ( "fmt" "time" - "github.com/celestiaorg/talis-agent/internal/logging" "github.com/shirou/gopsutil/v3/cpu" "github.com/shirou/gopsutil/v3/disk" "github.com/shirou/gopsutil/v3/host" "github.com/shirou/gopsutil/v3/mem" "github.com/shirou/gopsutil/v3/net" + + "github.com/celestiaorg/talis-agent/internal/logging" ) // SystemMetrics represents the collected system metrics diff --git a/internal/metrics/telemetry.go b/internal/metrics/telemetry.go index 8594551..e077b0a 100644 --- a/internal/metrics/telemetry.go +++ b/internal/metrics/telemetry.go @@ -6,10 +6,11 @@ import ( "net" "time" + "golang.org/x/time/rate" + "github.com/celestiaorg/talis-agent/internal/api" "github.com/celestiaorg/talis-agent/internal/config" "github.com/celestiaorg/talis-agent/internal/logging" - "golang.org/x/time/rate" ) // TelemetryClient handles sending metrics to the API server @@ -147,7 +148,11 @@ func (t *TelemetryClient) getOutboundIP() (net.IP, error) { if err != nil { return nil, err } - defer conn.Close() + defer func() { + if closeErr := conn.Close(); closeErr != nil { + err = fmt.Errorf("failed to close connection: %w", closeErr) + } + }() localAddr := conn.LocalAddr().(*net.UDPAddr) return localAddr.IP, nil diff --git a/internal/metrics/telemetry_test.go b/internal/metrics/telemetry_test.go index 9011f21..57bd090 100644 --- a/internal/metrics/telemetry_test.go +++ b/internal/metrics/telemetry_test.go @@ -3,8 +3,10 @@ package metrics import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" + "sync" "testing" "time" @@ -151,10 +153,14 @@ func TestSendCheckin(t *testing.T) { } func TestTelemetryClientStart(t *testing.T) { - metricsCount := 0 - checkinCount := 0 - expectedMetrics := 1 - expectedCheckins := 1 + var ( + metricsCount = 0 + checkinCount = 0 + expectedMetrics = 1 + expectedCheckins = 1 + mu sync.Mutex + requestWg sync.WaitGroup + ) // Create a test server server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -165,7 +171,12 @@ func TestTelemetryClientStart(t *testing.T) { switch r.URL.Path { case "/v1/agent/telemetry": - metricsCount++ + mu.Lock() + if metricsCount < expectedMetrics { + metricsCount++ + requestWg.Done() + } + mu.Unlock() // Verify metrics payload var metrics SystemMetrics if err := json.NewDecoder(r.Body).Decode(&metrics); err != nil { @@ -175,7 +186,12 @@ func TestTelemetryClientStart(t *testing.T) { t.Errorf("Invalid CPU usage: %v", metrics.CPU.UsagePercent) } case "/v1/agent/checkin": - checkinCount++ + mu.Lock() + if checkinCount < expectedCheckins { + checkinCount++ + requestWg.Done() + } + mu.Unlock() // Verify checkin payload var checkin CheckinPayload if err := json.NewDecoder(r.Body).Decode(&checkin); err != nil { @@ -194,13 +210,13 @@ func TestTelemetryClientStart(t *testing.T) { })) defer server.Close() - // Create a test config with short intervals for testing + // Create a test config with reasonable intervals for testing cfg := &config.Config{ APIServer: server.URL, Token: "test-token", - CheckinInterval: 100 * time.Millisecond, + CheckinInterval: 100 * time.Millisecond, // Keep interval short but not too short Metrics: config.MetricsConfig{ - CollectionInterval: 100 * time.Millisecond, + CollectionInterval: 100 * time.Millisecond, // Keep interval short but not too short Endpoints: struct { Telemetry string `yaml:"telemetry"` Checkin string `yaml:"checkin"` @@ -213,37 +229,48 @@ func TestTelemetryClientStart(t *testing.T) { client := NewTelemetryClient(cfg) + // Set up WaitGroup for expected requests + requestWg.Add(expectedMetrics + expectedCheckins) + // Create a context with timeout - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() // Start the client in a goroutine - errCh := make(chan error) + errCh := make(chan error, 1) go func() { errCh <- client.Start(ctx) }() - // Wait for expected number of metrics and check-ins or timeout - timeout := time.After(1 * time.Second) - ticker := time.NewTicker(50 * time.Millisecond) - defer ticker.Stop() - - for { - select { - case <-ticker.C: - if metricsCount >= expectedMetrics && checkinCount >= expectedCheckins { - cancel() // Stop the client - if err := <-errCh; err != context.Canceled { - t.Errorf("Expected context.Canceled error, got: %v", err) - } - return - } - case <-timeout: - cancel() - t.Fatalf("Test timed out waiting for metrics and check-ins. Got %d/%d metrics and %d/%d check-ins", - metricsCount, expectedMetrics, checkinCount, expectedCheckins) - case err := <-errCh: - t.Fatalf("Client stopped unexpectedly: %v", err) + // Wait for all expected requests or timeout + waitCh := make(chan struct{}) + go func() { + requestWg.Wait() + close(waitCh) + }() + + // Wait for completion or timeout + select { + case <-waitCh: + // Verify we got at least the expected number of each type of request + mu.Lock() + if metricsCount < expectedMetrics { + t.Errorf("Expected at least %d metrics, got %d", expectedMetrics, metricsCount) + } + if checkinCount < expectedCheckins { + t.Errorf("Expected at least %d checkins, got %d", expectedCheckins, checkinCount) + } + mu.Unlock() + cancel() // Stop the client + if err := <-errCh; err != nil && !errors.Is(err, context.Canceled) { + t.Errorf("Expected context.Canceled error, got: %v", err) } + case err := <-errCh: + t.Fatalf("Client stopped unexpectedly: %v", err) + case <-ctx.Done(): + mu.Lock() + t.Fatalf("Test timed out waiting for requests. Got %d/%d metrics and %d/%d checkins", + metricsCount, expectedMetrics, checkinCount, expectedCheckins) + mu.Unlock() } } From f896d9f6db5c44946ae0fa7920e68dd733a51ab1 Mon Sep 17 00:00:00 2001 From: MSevey <15232757+MSevey@users.noreply.github.com> Date: Mon, 3 Mar 2025 12:10:14 -0500 Subject: [PATCH 2/4] chore: minor clean up after reviewing PR --- .github/release_WIP.yml | 58 +++++++++++++++++++++++++++++++++++ .github/workflows/release.yml | 58 ----------------------------------- Makefile | 2 +- cmd/agent/main.go | 2 +- internal/api/client.go | 2 +- internal/http/server.go | 3 -- internal/metrics/telemetry.go | 2 +- 7 files changed, 62 insertions(+), 65 deletions(-) create mode 100644 .github/release_WIP.yml delete mode 100644 .github/workflows/release.yml diff --git a/.github/release_WIP.yml b/.github/release_WIP.yml new file mode 100644 index 0000000..c95e701 --- /dev/null +++ b/.github/release_WIP.yml @@ -0,0 +1,58 @@ +# name: Release Debian Package + +# on: +# push: +# tags: +# - 'v*.*.*' # Trigger on version tags v1.0.0, v2.1.0, etc. + +# jobs: +# build-deb: +# runs-on: ubuntu-latest +# permissions: +# contents: write # Needed for creating releases + +# steps: +# - name: Checkout code +# uses: actions/checkout@v4 + +# - name: Set up Go +# uses: actions/setup-go@v5 +# with: +# go-version: '1.21' +# cache: true + +# - name: Install build dependencies +# run: | +# sudo apt-get update +# sudo apt-get install -y dpkg-dev debhelper fakeroot golang-go + +# - name: Get version from tag +# id: get_version +# run: | +# VERSION=${GITHUB_REF#refs/tags/v} +# echo "version=$VERSION" >> $GITHUB_OUTPUT +# # Update version in build script +# sed -i "s/VERSION=\".*\"/VERSION=\"$VERSION\"/" scripts/build-deb.sh + +# - name: Build Debian package +# run: ./scripts/build-deb.sh + +# - name: Create Release +# id: create_release +# uses: softprops/action-gh-release@v2 +# with: +# name: Release ${{ steps.get_version.outputs.version }} +# draft: false +# prerelease: false +# files: | +# build/talis-agent_${{ steps.get_version.outputs.version }}_*.deb +# generate_release_notes: true +# env: +# GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + +# - name: Upload package to release +# uses: actions/upload-artifact@v4 +# with: +# name: debian-package +# path: build/talis-agent_${{ steps.get_version.outputs.version }}_*.deb +# retention-days: 5 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml deleted file mode 100644 index b63c8ce..0000000 --- a/.github/workflows/release.yml +++ /dev/null @@ -1,58 +0,0 @@ -name: Release Debian Package - -on: - push: - tags: - - 'v*.*.*' # Trigger on version tags v1.0.0, v2.1.0, etc. - -jobs: - build-deb: - runs-on: ubuntu-latest - permissions: - contents: write # Needed for creating releases - - steps: - - name: Checkout code - uses: actions/checkout@v4 - - - name: Set up Go - uses: actions/setup-go@v5 - with: - go-version: '1.21' - cache: true - - - name: Install build dependencies - run: | - sudo apt-get update - sudo apt-get install -y dpkg-dev debhelper fakeroot golang-go - - - name: Get version from tag - id: get_version - run: | - VERSION=${GITHUB_REF#refs/tags/v} - echo "version=$VERSION" >> $GITHUB_OUTPUT - # Update version in build script - sed -i "s/VERSION=\".*\"/VERSION=\"$VERSION\"/" scripts/build-deb.sh - - - name: Build Debian package - run: ./scripts/build-deb.sh - - - name: Create Release - id: create_release - uses: softprops/action-gh-release@v2 - with: - name: Release ${{ steps.get_version.outputs.version }} - draft: false - prerelease: false - files: | - build/talis-agent_${{ steps.get_version.outputs.version }}_*.deb - generate_release_notes: true - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - - name: Upload package to release - uses: actions/upload-artifact@v4 - with: - name: debian-package - path: build/talis-agent_${{ steps.get_version.outputs.version }}_*.deb - retention-days: 5 diff --git a/Makefile b/Makefile index 9287b4a..138c7c5 100644 --- a/Makefile +++ b/Makefile @@ -27,7 +27,7 @@ all: lint test build ## build: Build the application build: @echo "Building $(PROJECTNAME)..." - $(GOBUILD) $(LDFLAGS) -o bin/$(PROJECTNAME) ./cmd/main.go + $(GOBUILD) $(LDFLAGS) -o bin/$(PROJECTNAME) ./cmd/agent/main.go .PHONY: build ## clean: Clean build artifacts diff --git a/cmd/agent/main.go b/cmd/agent/main.go index d59e79a..7b6888d 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -102,7 +102,7 @@ func main() { Int("port", cfg.HTTPPort). Msg("Starting HTTP server") if err := server.Start(); err != nil { - if errors.Is(err, http.ErrServerClosed) { + if !errors.Is(err, http.ErrServerClosed) { logging.Error().Err(err).Msg("HTTP server error") } } diff --git a/internal/api/client.go b/internal/api/client.go index d91c022..2bde864 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -148,7 +148,7 @@ func (c *Client) doRequest(ctx context.Context, method, path string, body interf } defer func() { if cerr := resp.Body.Close(); cerr != nil { - err = fmt.Errorf("error closing response body: %w", cerr) + logging.Error().Err(cerr).Msg("error closing response body") } }() diff --git a/internal/http/server.go b/internal/http/server.go index 95218d5..5db6831 100644 --- a/internal/http/server.go +++ b/internal/http/server.go @@ -114,9 +114,6 @@ func (s *Server) handlePayload(w http.ResponseWriter, r *http.Request) { logging.Error(). Err(err). Msg("Failed to close file") - if err == nil { // Only set the error if there wasn't one already - http.Error(w, "Failed to close file", http.StatusInternalServerError) - } } }() diff --git a/internal/metrics/telemetry.go b/internal/metrics/telemetry.go index e077b0a..c6684ea 100644 --- a/internal/metrics/telemetry.go +++ b/internal/metrics/telemetry.go @@ -150,7 +150,7 @@ func (t *TelemetryClient) getOutboundIP() (net.IP, error) { } defer func() { if closeErr := conn.Close(); closeErr != nil { - err = fmt.Errorf("failed to close connection: %w", closeErr) + logging.Error().Err(closeErr).Msg("failed to close connection") } }() From 09587cd27d98dd3fcd0b1aa299c5a054dd9eb5de Mon Sep 17 00:00:00 2001 From: MSevey <15232757+MSevey@users.noreply.github.com> Date: Mon, 3 Mar 2025 12:22:47 -0500 Subject: [PATCH 3/4] chore(test): minor tweak on test code --- internal/metrics/telemetry_test.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/internal/metrics/telemetry_test.go b/internal/metrics/telemetry_test.go index 57bd090..109ba78 100644 --- a/internal/metrics/telemetry_test.go +++ b/internal/metrics/telemetry_test.go @@ -171,12 +171,6 @@ func TestTelemetryClientStart(t *testing.T) { switch r.URL.Path { case "/v1/agent/telemetry": - mu.Lock() - if metricsCount < expectedMetrics { - metricsCount++ - requestWg.Done() - } - mu.Unlock() // Verify metrics payload var metrics SystemMetrics if err := json.NewDecoder(r.Body).Decode(&metrics); err != nil { @@ -185,13 +179,14 @@ func TestTelemetryClientStart(t *testing.T) { if metrics.CPU.UsagePercent < 0 || metrics.CPU.UsagePercent > 100 { t.Errorf("Invalid CPU usage: %v", metrics.CPU.UsagePercent) } - case "/v1/agent/checkin": + // Increment metrics count and signal completion mu.Lock() - if checkinCount < expectedCheckins { - checkinCount++ + if metricsCount < expectedMetrics { + metricsCount++ requestWg.Done() } mu.Unlock() + case "/v1/agent/checkin": // Verify checkin payload var checkin CheckinPayload if err := json.NewDecoder(r.Body).Decode(&checkin); err != nil { @@ -203,6 +198,13 @@ func TestTelemetryClientStart(t *testing.T) { if checkin.Status != "alive" { t.Errorf("Expected status alive, got %s", checkin.Status) } + // Increment checkin count and signal completion + mu.Lock() + if checkinCount < expectedCheckins { + checkinCount++ + requestWg.Done() + } + mu.Unlock() default: t.Errorf("Unexpected request to %s", r.URL.Path) } From b5fe9ddea154b7fd4e65ec7f9c64b3b8ae569c46 Mon Sep 17 00:00:00 2001 From: MSevey <15232757+MSevey@users.noreply.github.com> Date: Mon, 3 Mar 2025 12:24:40 -0500 Subject: [PATCH 4/4] fix(ci): remove copy paste error) --- .github/workflows/ci.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da15a49..05f1552 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,6 +50,3 @@ jobs: - name: Build main application run: make build - - - name: Build CLI - run: make build-cli