From 2c0250d29a46bbedc36672e4acb84f4d3a935064 Mon Sep 17 00:00:00 2001 From: 0x524a Date: Tue, 2 Dec 2025 22:57:34 -0500 Subject: [PATCH] chore: update golangci-lint configuration and improve CI workflow documentation - Increased thresholds for funlen and lll linters to accommodate complex functions. - Added exclusions for dupl linter in specific files and directories to reduce false positives. - Updated CI workflow documentation to clarify triggers and requirements for SonarCloud analysis. - Removed unnecessary linter directives in several files for improved readability. --- .github/workflows/README.md | 23 +++++++++++--- .github/workflows/ci.yml | 19 +++++++++--- .golangci.yml | 17 +++++++++-- cmd/generate-tests/main.go | 1 - cmd/onvif-cli/ascii.go | 2 +- cmd/onvif-server/main.go | 5 ++- device_security.go | 8 ++--- media.go | 2 -- server/imaging_test.go | 2 +- server/ptz_test.go | 21 ++++++------- server/types.go | 61 +++++++++++++++++++------------------ 11 files changed, 94 insertions(+), 67 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index d8e8841..a340468 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -21,7 +21,7 @@ fmt → lint → test → sonarcloud | **fmt** | Format check using `gofmt -s` | - | | **lint** | Static analysis with `go vet` and `golangci-lint` | fmt | | **test** | Unit tests with race detector + coverage | lint | -| **sonarcloud** | Code quality & security analysis | test | +| **sonarcloud** | Code quality & security analysis (push to master only) | test | | **build** | Build verification for all packages | test | | **ci-success** | Final status check | all | @@ -33,8 +33,21 @@ fmt → lint → test → sonarcloud - ✅ Concurrency control (cancels in-progress runs) **Triggers:** -- Push to `master`, `main`, `develop` -- Pull requests to `master`, `main`, `develop` +- Push to `master`, `main` +- All pull requests targeting `master`, `main` + +**Required for PR Merge:** +All stages must pass before a PR can be merged. Configure branch protection rules in GitHub: +1. Go to **Settings → Branches → Branch protection rules** +2. Add rule for `master` +3. Enable **Require status checks to pass before merging** +4. Select these required checks: + - `Format Check` + - `Lint` + - `Test & Coverage` + - `SonarCloud Analysis` + - `Build Verification` + - `CI Success` --- @@ -113,7 +126,8 @@ Dependency vulnerability review. │ ▼ ▼ │ │ ┌────────────┐ ┌───────────┐ │ │ │ SONARCLOUD │ │ BUILD │ │ -│ └────────────┘ └───────────┘ │ +│ │ (push only)│ └───────────┘ │ +│ └────────────┘ │ │ │ │ │ │ │ └─────────┬─────────┘ │ │ ▼ │ @@ -124,6 +138,7 @@ Dependency vulnerability review. └─────────────────────────────────────────────────────────────────┘ ❌ If any stage fails, the pipeline stops immediately (fail-fast) +ℹ️ SonarCloud only runs on push to master/main (skipped for PRs) ``` --- diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ebe75a5..9b50e3b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,9 +2,10 @@ name: CI on: push: - branches: [master, main, develop] + branches: [master, main] pull_request: - branches: [master, main, develop] + branches: [master, main] + types: [opened, synchronize, reopened] permissions: contents: read @@ -12,7 +13,7 @@ permissions: pull-requests: write concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: true env: @@ -138,14 +139,18 @@ jobs: files: ./coverage.out flags: unittests name: codecov-onvif-go - fail_ci_if_error: true + # Don't fail on PRs from forks where token may not be available + fail_ci_if_error: ${{ github.event_name == 'push' }} verbose: true # Stage 4: SonarCloud Analysis (depends on test) + # Only runs on push to master/main when SONAR_TOKEN is available + # Skipped for PRs from forks where secrets are not accessible sonarcloud: name: SonarCloud Analysis runs-on: ubuntu-latest needs: test + if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/main') && github.repository == '0x524a/onvif-go' steps: - name: Checkout code uses: actions/checkout@v4 @@ -235,10 +240,14 @@ jobs: echo "❌ Tests failed" exit 1 fi - if [[ "${{ needs.sonarcloud.result }}" != "success" ]]; then + # SonarCloud is optional - only fails if it ran and failed (not if skipped) + if [[ "${{ needs.sonarcloud.result }}" == "failure" ]]; then echo "❌ SonarCloud analysis failed" exit 1 fi + if [[ "${{ needs.sonarcloud.result }}" == "skipped" ]]; then + echo "ℹ️ SonarCloud analysis skipped (only runs on push to master/main)" + fi if [[ "${{ needs.build.result }}" != "success" ]]; then echo "❌ Build verification failed" exit 1 diff --git a/.golangci.yml b/.golangci.yml index ff0450d..7e149cb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -53,11 +53,11 @@ linters-settings: min-complexity: 15 funlen: - lines: 100 - statements: 50 + lines: 120 + statements: 60 lll: - line-length: 120 + line-length: 150 gocritic: enabled-tags: @@ -99,6 +99,7 @@ issues: - funlen - gocyclo - gocognit + - dupl # Exclude known false positives - text: "Error return value of .((os\\.)?std(out|err)\\..*|.*Close|.*Flush|.*Write|.*Read|.*Printf?|.*Fprintf?) is not checked" @@ -109,6 +110,16 @@ issues: - path: _test\.go linters: - lll + + # Exclude dupl from ONVIF API files - similar patterns are expected + - path: (media|device|ptz|imaging|device_security|device_additional)\.go + linters: + - dupl + + # Exclude dupl from cmd directories + - path: cmd/ + linters: + - dupl max-issues-per-linter: 50 max-same-issues: 10 diff --git a/cmd/generate-tests/main.go b/cmd/generate-tests/main.go index c0da36c..f96257c 100644 --- a/cmd/generate-tests/main.go +++ b/cmd/generate-tests/main.go @@ -135,7 +135,6 @@ type AdditionalTest struct { Code string } -//nolint:funlen // Main function has many statements due to test generation logic func main() { flag.Parse() diff --git a/cmd/onvif-cli/ascii.go b/cmd/onvif-cli/ascii.go index 4ce3eba..8a29540 100644 --- a/cmd/onvif-cli/ascii.go +++ b/cmd/onvif-cli/ascii.go @@ -70,7 +70,7 @@ func ImageToASCII(imageData []byte, config ASCIIConfig) (string, error) { // imageToASCIIFromImage is the core conversion function. // -//nolint:gocyclo,lll // Image to ASCII conversion has high complexity due to multiple pixel processing paths +//nolint:gocyclo // Image to ASCII conversion has high complexity due to multiple pixel processing paths func imageToASCIIFromImage(img image.Image, config ASCIIConfig, format string) (string, error) { //nolint:unparam // format reserved for future use // Validate configuration if config.Width <= 0 { diff --git a/cmd/onvif-server/main.go b/cmd/onvif-server/main.go index aef8fd3..2521a41 100644 --- a/cmd/onvif-server/main.go +++ b/cmd/onvif-server/main.go @@ -27,7 +27,6 @@ const ( ptzSpeed = 0.5 ) -//nolint:funlen // Main function has many statements due to server setup and configuration func main() { // Define command-line flags host := flag.String("host", "0.0.0.0", "Server host address") @@ -39,7 +38,7 @@ func main() { firmware := flag.String("firmware", "1.0.0", "Firmware version") serial := flag.String("serial", "SN-12345678", "Serial number") profiles := flag.Int( - "profiles", maxWorkers, "Number of camera profiles (1-10)", //nolint:mnd // Default profile count is reasonable + "profiles", maxWorkers, "Number of camera profiles (1-10)", ) ptz := flag.Bool("ptz", true, "Enable PTZ support") imaging := flag.Bool("imaging", true, "Enable Imaging support") @@ -217,7 +216,7 @@ func buildConfig(host string, port int, username, password, manufacturer, model, Token: fmt.Sprintf("preset_%d_1", i), Name: "Entrance", Position: server.PTZPosition{ - Pan: -45, Tilt: -10, Zoom: template.ptzZoomMax * ptzSpeed, //nolint:mnd // Preset position values + Pan: -45, Tilt: -10, Zoom: template.ptzZoomMax * ptzSpeed, }, }, }, diff --git a/device_security.go b/device_security.go index 362f376..ea421ff 100644 --- a/device_security.go +++ b/device_security.go @@ -142,9 +142,7 @@ func (c *Client) GetIPAddressFilter(ctx context.Context) (*IPAddressFilter, erro return filter, nil } -// SetIPAddressFilter sets the IP address filter settings on a device -// -//nolint:dupl // Similar structure to AddIPAddressFilter but different operation +// SetIPAddressFilter sets the IP address filter settings on a device. func (c *Client) SetIPAddressFilter(ctx context.Context, filter *IPAddressFilter) error { type SetIPAddressFilter struct { XMLName xml.Name `xml:"tds:SetIPAddressFilter"` @@ -197,9 +195,7 @@ func (c *Client) SetIPAddressFilter(ctx context.Context, filter *IPAddressFilter return nil } -// AddIPAddressFilter adds an IP filter address to a device -// -//nolint:dupl // Similar structure to SetIPAddressFilter but different operation +// AddIPAddressFilter adds an IP filter address to a device. func (c *Client) AddIPAddressFilter(ctx context.Context, filter *IPAddressFilter) error { type AddIPAddressFilter struct { XMLName xml.Name `xml:"tds:AddIPAddressFilter"` diff --git a/media.go b/media.go index 8d56b22..8f72318 100644 --- a/media.go +++ b/media.go @@ -2491,8 +2491,6 @@ func (c *Client) GetAudioSourceConfigurations(ctx context.Context) ([]*AudioSour } // GetVideoEncoderConfigurations retrieves all video encoder configurations. -// -//nolint:funlen // GetVideoEncoderConfigurations has many statements due to parsing complex encoder configurations func (c *Client) GetVideoEncoderConfigurations(ctx context.Context) ([]*VideoEncoderConfiguration, error) { endpoint := c.mediaEndpoint if endpoint == "" { diff --git a/server/imaging_test.go b/server/imaging_test.go index c34a552..c7fa2d5 100644 --- a/server/imaging_test.go +++ b/server/imaging_test.go @@ -128,7 +128,7 @@ func TestHandleGetOptions(t *testing.T) { // TestHandleMove - DISABLED due to SOAP namespace requirements. // -//nolint:unused,thelper // Disabled test function kept for reference +//nolint:unused // Disabled test function kept for reference func _DisabledTestHandleMove(t *testing.T) { t.Helper() config := createTestConfig() diff --git a/server/ptz_test.go b/server/ptz_test.go index 02a8748..e66c2d5 100644 --- a/server/ptz_test.go +++ b/server/ptz_test.go @@ -8,7 +8,7 @@ import ( // These handlers are better tested through the SOAP handler in integration tests. // -//nolint:unused,thelper // Disabled test function kept for reference +//nolint:unused // Disabled test function kept for reference func _DisabledTestHandleGetPresets(t *testing.T) { t.Helper() config := createTestConfig() @@ -79,7 +79,7 @@ func TestHandleGotoPreset(t *testing.T) { // TestHandleGetStatus - DISABLED due to SOAP namespace requirements. // -//nolint:unused,thelper // Disabled test function kept for reference +//nolint:unused // Disabled test function kept for reference func _DisabledTestHandleGetStatus(t *testing.T) { t.Helper() config := createTestConfig() @@ -115,10 +115,9 @@ func _DisabledTestHandleGetStatus(t *testing.T) { } } -// TestHandleAbsoluteMove - DISABLED due to SOAP namespace requirements +// TestHandleAbsoluteMove - DISABLED due to SOAP namespace requirements. // -//nolint:dupl // Disabled test functions have similar structure -//nolint:unused,thelper // Disabled test function kept for reference +//nolint:unused // Disabled test function kept for reference func _DisabledTestHandleAbsoluteMove(t *testing.T) { t.Helper() config := createTestConfig() @@ -159,10 +158,9 @@ func _DisabledTestHandleAbsoluteMove(t *testing.T) { } } -// TestHandleRelativeMove - DISABLED due to SOAP namespace requirements +// TestHandleRelativeMove - DISABLED due to SOAP namespace requirements. // -//nolint:dupl // Disabled test functions have similar structure -//nolint:unused,thelper // Disabled test function kept for reference +//nolint:unused // Disabled test function kept for reference func _DisabledTestHandleRelativeMove(t *testing.T) { t.Helper() config := createTestConfig() @@ -203,10 +201,9 @@ func _DisabledTestHandleRelativeMove(t *testing.T) { } } -// TestHandleContinuousMove - DISABLED due to SOAP namespace requirements +// TestHandleContinuousMove - DISABLED due to SOAP namespace requirements. // -//nolint:dupl // Disabled test functions have similar structure -//nolint:unused,thelper // Disabled test function kept for reference +//nolint:unused // Disabled test function kept for reference func _DisabledTestHandleContinuousMove(t *testing.T) { t.Helper() config := createTestConfig() @@ -249,7 +246,7 @@ func _DisabledTestHandleContinuousMove(t *testing.T) { // TestHandleStop - DISABLED due to SOAP namespace requirements. // -//nolint:unused,thelper // Disabled test function kept for reference +//nolint:unused // Disabled test function kept for reference func _DisabledTestHandleStop(t *testing.T) { t.Helper() config := createTestConfig() diff --git a/server/types.go b/server/types.go index ba42d6b..8a66047 100644 --- a/server/types.go +++ b/server/types.go @@ -8,6 +8,7 @@ import ( ) const ( + defaultPort = 8080 defaultTimeoutSec = 30 defaultWidth = 1920 defaultHeight = 1080 @@ -250,10 +251,12 @@ type WDRSettings struct { } // DefaultConfig returns a default server configuration with a multi-lens camera setup. -func DefaultConfig() *Config { //nolint:funlen // DefaultConfig has many statements due to comprehensive default configuration +// +//nolint:funlen // DefaultConfig has many statements due to comprehensive default configuration +func DefaultConfig() *Config { return &Config{ Host: "0.0.0.0", - Port: 8080, //nolint:mnd // Default HTTP port + Port: defaultPort, BasePath: "/onvif", Timeout: defaultTimeoutSec * time.Second, DeviceInfo: DeviceInfo{ @@ -302,14 +305,14 @@ func DefaultConfig() *Config { //nolint:funlen // DefaultConfig has many stateme {Token: "preset_0", Name: "Home", Position: PTZPosition{Pan: 0, Tilt: 0, Zoom: 0}}, { Token: "preset_1", Name: "Entrance", - Position: PTZPosition{Pan: -45, Tilt: -10, Zoom: defaultPTZSpeed}, //nolint:mnd // Preset position + Position: PTZPosition{Pan: -45, Tilt: -10, Zoom: defaultPTZSpeed}, }, }, }, Snapshot: SnapshotConfig{ Enabled: true, - Resolution: Resolution{Width: defaultWidth, Height: defaultHeight}, //nolint:mnd // Default resolution - Quality: highQuality, //nolint:mnd // High quality + Resolution: Resolution{Width: defaultWidth, Height: defaultHeight}, + Quality: highQuality, }, }, { @@ -318,22 +321,22 @@ func DefaultConfig() *Config { //nolint:funlen // DefaultConfig has many stateme VideoSource: VideoSourceConfig{ Token: "video_source_1", Name: "Wide Angle Camera", - Resolution: Resolution{Width: mediumWidth, Height: mediumHeight}, //nolint:mnd // Medium resolution - Framerate: defaultFramerate, //nolint:mnd // Default framerate - Bounds: Bounds{X: 0, Y: 0, Width: mediumWidth, Height: mediumHeight}, //nolint:mnd // Medium bounds + Resolution: Resolution{Width: mediumWidth, Height: mediumHeight}, + Framerate: defaultFramerate, + Bounds: Bounds{X: 0, Y: 0, Width: mediumWidth, Height: mediumHeight}, }, VideoEncoder: VideoEncoderConfig{ Encoding: "H264", - Resolution: Resolution{Width: mediumWidth, Height: mediumHeight}, //nolint:mnd // Medium resolution - Quality: mediumQuality, //nolint:mnd // Medium quality - Framerate: defaultFramerate, //nolint:mnd // Default framerate - Bitrate: mediumBitrate, //nolint:mnd // Medium bitrate - GovLength: defaultFramerate, //nolint:mnd // Default gov length + Resolution: Resolution{Width: mediumWidth, Height: mediumHeight}, + Quality: mediumQuality, + Framerate: defaultFramerate, + Bitrate: mediumBitrate, + GovLength: defaultFramerate, }, Snapshot: SnapshotConfig{ Enabled: true, - Resolution: Resolution{Width: mediumWidth, Height: mediumHeight}, //nolint:mnd // Medium resolution - Quality: defaultQuality, //nolint:mnd // Default quality + Resolution: Resolution{Width: mediumWidth, Height: mediumHeight}, + Quality: defaultQuality, }, }, { @@ -342,25 +345,25 @@ func DefaultConfig() *Config { //nolint:funlen // DefaultConfig has many stateme VideoSource: VideoSourceConfig{ Token: "video_source_2", Name: "Telephoto Camera", - Resolution: Resolution{Width: defaultWidth, Height: defaultHeight}, //nolint:mnd // Default resolution - Framerate: lowFramerate, //nolint:mnd // Low framerate - Bounds: Bounds{X: 0, Y: 0, Width: defaultWidth, Height: defaultHeight}, //nolint:mnd // Default bounds + Resolution: Resolution{Width: defaultWidth, Height: defaultHeight}, + Framerate: lowFramerate, + Bounds: Bounds{X: 0, Y: 0, Width: defaultWidth, Height: defaultHeight}, }, VideoEncoder: VideoEncoderConfig{ Encoding: "H264", - Resolution: Resolution{Width: defaultWidth, Height: defaultHeight}, //nolint:mnd // Default resolution - Quality: highQuality, //nolint:mnd // High quality - Framerate: lowFramerate, //nolint:mnd // Low framerate - Bitrate: highBitrate, //nolint:mnd // High bitrate - GovLength: lowFramerate, //nolint:mnd // Low framerate + Resolution: Resolution{Width: defaultWidth, Height: defaultHeight}, + Quality: highQuality, + Framerate: lowFramerate, + Bitrate: highBitrate, + GovLength: lowFramerate, }, PTZ: &PTZConfig{ NodeToken: "ptz_node_2", PanRange: Range{Min: -maxPan, Max: maxPan}, TiltRange: Range{Min: -maxTilt, Max: maxTilt}, - ZoomRange: Range{Min: 0, Max: maxZoom}, //nolint:mnd // Max zoom + ZoomRange: Range{Min: 0, Max: maxZoom}, DefaultSpeed: PTZSpeed{ - Pan: lowPTZSpeed, Tilt: lowPTZSpeed, Zoom: lowPTZSpeed, //nolint:mnd // Low PTZ speed + Pan: lowPTZSpeed, Tilt: lowPTZSpeed, Zoom: lowPTZSpeed, }, SupportsContinuous: true, SupportsAbsolute: true, @@ -369,14 +372,14 @@ func DefaultConfig() *Config { //nolint:funlen // DefaultConfig has many stateme {Token: "preset_2_0", Name: "Home", Position: PTZPosition{Pan: 0, Tilt: 0, Zoom: 0}}, { Token: "preset_2_1", Name: "Zoom In", - Position: PTZPosition{Pan: 0, Tilt: 0, Zoom: presetZoom}, //nolint:mnd // Preset zoom + Position: PTZPosition{Pan: 0, Tilt: 0, Zoom: presetZoom}, }, }, }, Snapshot: SnapshotConfig{ Enabled: true, - Resolution: Resolution{Width: defaultWidth, Height: defaultHeight}, //nolint:mnd // Default resolution - Quality: highQuality, //nolint:mnd // High quality + Resolution: Resolution{Width: defaultWidth, Height: defaultHeight}, + Quality: highQuality, }, }, }, @@ -393,7 +396,7 @@ func (c *Config) ServiceEndpoints(host string) map[string]string { } var baseURL string - const httpPort = 80 //nolint:mnd // Standard HTTP port + const httpPort = 80 if c.Port == httpPort { baseURL = "http://" + host + c.BasePath } else {