Skip to content

Conversation

@LaurenceJJones
Copy link
Member

fix: #120

  • Fix geo database initialization: remove meaningless IsValid() check in open() that always passed on first call, add proper path validation before loading
  • Improve code redundancy: extract common file checking logic into checkAndUpdateModTime() function, reduce duplication in WatchFiles()
  • Add comprehensive test coverage for Init() with various error cases
  • Fix ISO code header: always set isocode variable when available, regardless of remediation status, so a header can be populated correctly

- Fix geo database initialization: remove meaningless IsValid() check in open()
  that always passed on first call, add proper path validation before loading
- Improve code redundancy: extract common file checking logic into
  checkAndUpdateModTime() function, reduce duplication in WatchFiles()
- Add comprehensive test coverage for Init() with various error cases
- Fix ISO code header: always set isocode variable when available, regardless
  of remediation status, so X-Crowdsec-IsoCode header is populated correctly
- Replace assert.Error with require.Error for error assertions (testifylint)
- Use t.TempDir() instead of empty string for os.CreateTemp (usetesting)
@LaurenceJJones LaurenceJJones added this to the 0.2.1 milestone Nov 24, 2025
Copilot finished reviewing on behalf of LaurenceJJones November 24, 2025 15:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes critical issues with geo database initialization and ISO code handling. It adds proper path validation before loading databases to prevent meaningless checks, ensures ISO codes are always set when available (not just during remediation), and improves code maintainability by extracting duplicated logic into a reusable helper function.

Key Changes:

  • Added comprehensive path validation (existence, type, size) before database initialization
  • Fixed ISO code header to always populate when geo data is available, regardless of remediation status
  • Refactored file watching logic to eliminate code duplication using checkAndUpdateModTime() helper

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
pkg/spoa/root.go Modified to always set ISO code variable when available, separating geo data extraction from country-based remediation logic
internal/geo/root.go Added validatePaths() and validatePath() functions for pre-flight checks, improved error messages, added cleanup logic in open(), and refactored WatchFiles() to use new checkAndUpdateModTime() helper
internal/geo/geo_test.go Added comprehensive test coverage for Init() including empty paths, missing files, one valid path, directory path, empty file, and WatchFiles goroutine scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isocode header

2 participants