Conversation
CrowdSec v1.7.x replaced the "valid" JSON field with "revoked: false" in the bouncers list API response. The manager still reads "valid", which no longer exists, causing all bouncers to show as "disconnected". This fix reads the "revoked" field and derives Valid = !Revoked. Falls back to treating the bouncer as valid if neither field is present. Fixes #47
Address review feedback from Gemini and Copilot: - Consolidate the validity check into one if/else-if/else block - Check "revoked" first (new CrowdSec >= v1.7.x), fall back to "valid" (old versions) - Remove redundant double-parsing of both fields - Clearer control flow, no duplicate JSON lookups Fixes #47
…ked field Rework based on CrowdSec team feedback (Issue #47, @LaurenceJJones): - "revoked" is always false and will never be used by CrowdSec - CrowdSec deletes bouncers rather than revoking them - A bouncer in the list is valid by definition Changes: - Remove Revoked field from Bouncer struct (unused by CrowdSec) - Add CreatedAt and UpdatedAt fields for activity detection - Set Valid = true for all listed bouncers (existence = valid) - Determine status from last_pull with updated_at as fallback when last_pull is null (updated_at - created_at > 5s = has pulled) - New "registered" status for bouncers that enrolled but never pulled - Update BouncerStatusMonitor to show status instead of valid/invalid - Add "registered" badge to Bouncers page Status matrix: | last_pull | Status | |--------------------|-------------| | < 5 min ago | connected | | 5-60 min ago | connected | | > 60 min ago | stale | | null + updated_at | connected | | null + just created| registered | Fixes #47
fix: derive bouncer Valid status from revoked field (CrowdSec >= v1.7.x)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the accuracy and granularity of bouncer status monitoring. By incorporating additional timestamp data and revising the logic for determining bouncer validity and activity, the system now provides a more reliable and informative overview of bouncer health, improving operational visibility. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces CreatedAt and UpdatedAt fields to the Bouncer model and refactors bouncer status calculation in the backend to use a more robust lastActivity determination logic, considering both LastPull and UpdatedAt fields. This enables more granular bouncer statuses (e.g., "registered") which are then reflected in the frontend UI. A review comment highlights that the lastActivity logic is duplicated across two functions and suggests centralizing it into a models.Bouncer method for improved maintainability.
| lastActivity := b.LastPull | ||
| if lastActivity.IsZero() && | ||
| b.UpdatedAt.After(b.CreatedAt.Add(5*time.Second)) { | ||
| lastActivity = b.UpdatedAt | ||
| } |
There was a problem hiding this comment.
This logic to determine the lastActivity of a bouncer is duplicated from the parseBouncersJSON function (lines 66-70). To improve maintainability and avoid future inconsistencies, this logic should be centralized. A good approach would be to add a method to the models.Bouncer struct, like LastActivity() time.Time, to encapsulate this behavior. This would ensure that any future changes to this logic are made in only one place.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 904092ac5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if lastActivity.IsZero() && | ||
| bouncer.UpdatedAt.After(bouncer.CreatedAt.Add(5*time.Second)) { |
There was a problem hiding this comment.
The fallback condition treats updated_at as activity even when created_at was not parsed, because zero-time CreatedAt makes UpdatedAt.After(CreatedAt.Add(5*time.Second)) true for any real timestamp. In environments where created_at is omitted or unparsable but updated_at is present, bouncers with no last_pull will be misclassified as active (connected/stale) instead of registered, which overstates connectivity. Require parsed created_at (and non-zero updated_at) before applying this fallback.
Useful? React with 👍 / 👎.
2 participants
Bouncer status fix