Feat: Improve cloud plugin #240
Conversation
…eload-configuration
…b-improve-cloud-plugin # Conflicts: # plugins/beelzebub-cloud_test.go
…ud-plugin # Conflicts: # parser/configurations_parser_test.go
WalkthroughA new configuration change verification feature is introduced with periodic polling to detect modifications in Beelzebub cloud configurations. API signatures are updated across multiple components to support hash-based change detection and background monitoring, with optional enablement via boolean flags. Changes
Sequence DiagramsequenceDiagram
participant Init as Initialization
participant Cloud as beelzebubCloud
participant Verifier as Verification Loop
participant API as Beelzebub API
participant Process as Process Exit
Init->>Cloud: InitBeelzebubCloud(uri, token, enableVerify=true)
Cloud->>Cloud: Set PollingInterval (15s)
alt enableVerify is true
Cloud->>Verifier: Start background goroutine
end
loop Every PollingInterval
Verifier->>Cloud: verifyConfigurationsChanged()
Cloud->>API: GetHoneypotsConfigurations()
API-->>Cloud: configs + hash
Cloud->>Cloud: Compute current hash
alt Hash changed
Cloud->>Process: exitFunction(1)
Process-->>Process: Exit process
else Hash unchanged
Note over Cloud: Continue polling
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #240 +/- ##
==========================================
- Coverage 83.33% 81.55% -1.78%
==========================================
Files 7 7
Lines 546 618 +72
==========================================
+ Hits 455 504 +49
- Misses 77 93 +16
- Partials 14 21 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
parser/configurations_parser.go (1)
4-7: HashCode implementation looks correct; consider documenting its scope for future fieldsThe SHA‑256 over the JSON‑marshalled
BeelzebubServiceConfigurationis straightforward and deterministic with the current struct (no maps, and runtime fields likeCommand.Regexeffectively serialize as{}only). This is fine for now.However, any future JSON‑serializable, non‑config/runtime fields added to
BeelzebubServiceConfigurationwill change the hash (and thus trigger “config changed”) even if the YAML config is logically the same. A brief doc comment onHashCodeor on the struct to treat non-config fields asjson:"-"(similar toyaml:"-") would make this more future‑proof.Also applies to: 86-93
parser/configurations_parser_test.go (1)
185-198: HashCode test is strong but quite brittle to future changesThe test correctly exercises the full path and validates that
HashCodereturns a deterministic value for the mocked config. However, asserting a specific digest string means:
- Any benign schema change (adding a field, renaming, etc.) will require updating this magic value.
- The test couples tightly to the exact JSON representation.
You might instead assert properties such as “no error, non‑empty hash, and the hash changes when a config field is changed”, which still protects behavior while being less fragile.
builder/director.go (1)
61-73: Disabling config-change polling in the per-event strategy is appropriatePassing
falseintoInitBeelzebubCloudhere ensures the background configuration polling goroutine is not spawned for every traced event, which would be wasteful. This keeps the send‑only strategy lightweight; if you ever see this path become hot, caching a singlebeelzebubCloudinstance instead of constructing one per event would be a natural follow‑up.plugins/beelzebub-cloud_test.go (1)
298-350: MapToEventDTO test is good; consider adding a headers-focused caseThe test thoroughly checks that core fields are copied 1:1 from
tracer.EventtoEventDTO, which is useful coverage.Given the new header handling in
mapToEventDTO, it would be worth adding a small test that exercises an event with non-emptyHeaders(and possiblyHeadersMap) so regressions in header serialization are caught by the suite.plugins/beelzebub-cloud.go (1)
107-155: Config hashing in GetHoneypotsConfigurations is reasonable; minor cleanup and design noteThe new
(configs, hash, error)signature and implementation are clear:
- YAML is unmarshalled into
parser.BeelzebubServiceConfiguration.- Regexes are compiled with a helpful error message on failure.
HashCode()is computed per config and concatenated into a single hash string for change detection.Two minor suggestions:
- Inside the loop,
var honeypotsConfig parser.BeelzebubServiceConfigurationshadows the outer[]HoneypotConfigResponseDTOslice of the same name, which hurts readability. Renaming the inner variable (e.g. toserviceCfg) would make the code clearer.- Concatenating per-config hashes makes the overall hash sensitive to the order returned by the API. If the API’s array order is not guaranteed but you still care only about set equality, you might want to sort the per-config hashes before concatenation. If “order change == config change” is intentional, the current logic is fine.
Otherwise, the error propagation and return shape look good.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
builder/builder.go(2 hunks)builder/director.go(1 hunks)docker-compose.yml(0 hunks)parser/configurations_parser.go(2 hunks)parser/configurations_parser_test.go(1 hunks)plugins/beelzebub-cloud.go(5 hunks)plugins/beelzebub-cloud_test.go(9 hunks)
💤 Files with no reviewable changes (1)
- docker-compose.yml
🧰 Additional context used
🧬 Code graph analysis (5)
parser/configurations_parser_test.go (1)
parser/configurations_parser.go (1)
Init(132-139)
builder/director.go (1)
plugins/beelzebub-cloud.go (1)
InitBeelzebubCloud(59-74)
plugins/beelzebub-cloud_test.go (2)
plugins/beelzebub-cloud.go (3)
InitBeelzebubCloud(59-74)HoneypotConfigResponseDTO(52-57)EventDTO(18-43)tracer/tracer.go (3)
Event(15-41)Protocol(44-44)Status(45-45)
builder/builder.go (1)
plugins/beelzebub-cloud.go (1)
InitBeelzebubCloud(59-74)
plugins/beelzebub-cloud.go (2)
tracer/tracer.go (3)
Protocol(44-44)Status(45-45)Event(15-41)parser/configurations_parser.go (2)
Command(96-104)BeelzebubServiceConfiguration(68-84)
🔇 Additional comments (1)
plugins/beelzebub-cloud_test.go (1)
17-52: Tests correctly reflect the new InitBeelzebubCloud/GetHoneypotsConfigurations APIsAll the updated tests now:
- Pass the new boolean flag into
InitBeelzebubCloud(usingfalseto avoid spawning the polling goroutine in unit tests).- Correctly handle the extra
configurationsHashreturn value fromGetHoneypotsConfigurations, including asserting a deterministic hash where desired.This keeps the suite aligned with the new plugin API and exercises both success and error paths appropriately.
Also applies to: 78-134, 136-235
| "github.com/mariocandela/beelzebub/v3/protocols/strategies/MCP" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
New MCP strategy wiring and cloud init look good; double-check process-exit semantics from polling
The MCP strategy import and "mcp" protocol wiring are consistent with the existing pattern, and handling the extra return from GetHoneypotsConfigurations by ignoring the hash here is fine.
Enabling verification via InitBeelzebubCloud(conf.URI, conf.AuthToken, true) means Run() now starts a background goroutine that may terminate the whole process on:
- Any error in
verifyConfigurationsChanged(vialog.Fatalf), or - A configuration change (via
exitFunction, defaulting toos.Exit).
If that “exit on transient API failure / config change” behavior is intentional, this is fine; otherwise, consider softening it (e.g., log and retry with backoff, or invoke a reload callback) rather than exiting the process from within a background goroutine.
Also applies to: 114-135
| func TestVerifyConfigurationsChanged(t *testing.T) { | ||
| client := resty.New() | ||
| httpmock.ActivateNonDefault(client.GetClient()) | ||
| defer httpmock.DeactivateAndReset() | ||
|
|
||
| uri := "localhost:8081" | ||
|
|
||
| callCount := 0 | ||
|
|
||
| httpmock.RegisterResponder("GET", fmt.Sprintf("%s/honeypots", uri), | ||
| func(req *http.Request) (*http.Response, error) { | ||
| callCount++ | ||
|
|
||
| config := "apiVersion: \"v1\"\nprotocol: \"ssh\"\naddress: \":2222\"\ndescription: \"SSH interactive ChatGPT\"\ncommands:\n - regex: \"^(.+)$\"\n plugin: \"LLMHoneypot\"\nserverVersion: \"OpenSSH\"\nserverName: \"ubuntu\"\npasswordRegex: \"^(root|qwerty|Smoker666|123456|jenkins|minecraft|sinus|alex|postgres|Ly123456)$\"\ndeadlineTimeoutSeconds: 60\nplugin:\n llmModel: \"gpt-4o\"\n openAISecretKey: \"1234\"\n" | ||
|
|
||
| if callCount > 1 { | ||
| config = "apiVersion: \"v1\"\nprotocol: \"ssh\"\naddress: \":2222\"\ndescription: \"SSH interactive ChatGPT MODIFIED\"\ncommands:\n - regex: \"^(.+)$\"\n plugin: \"LLMHoneypot\"\nserverVersion: \"OpenSSH\"\nserverName: \"ubuntu\"\npasswordRegex: \"^(root|qwerty|Smoker666|123456|jenkins|minecraft|sinus|alex|postgres|Ly123456)$\"\ndeadlineTimeoutSeconds: 60\nplugin:\n llmModel: \"gpt-3.5-turbo\"\n openAISecretKey: \"1234\"\n" | ||
| } | ||
|
|
||
| resp, err := httpmock.NewJsonResponse(200, &[]HoneypotConfigResponseDTO{ | ||
| { | ||
| ID: "123456", | ||
| Config: config, | ||
| TokenID: "1234567", | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| return httpmock.NewStringResponse(500, ""), nil | ||
| } | ||
| return resp, nil | ||
| }, | ||
| ) | ||
|
|
||
| var exitInvoked bool = false | ||
| exitCalled := make(chan bool) | ||
|
|
||
| exitFunction = func(c int) { | ||
| exitInvoked = true | ||
| exitCalled <- true | ||
| } | ||
|
|
||
| beelzebubCloud := InitBeelzebubCloud(uri, "sdjdnklfjndslkjanfk", false) | ||
| beelzebubCloud.client = client | ||
| beelzebubCloud.PollingInterval = 100 * time.Millisecond | ||
|
|
||
| go func() { | ||
| err := beelzebubCloud.verifyConfigurationsChanged() | ||
| if err != nil { | ||
| t.Errorf("verifyConfigurationsChanged returned with error: %v", err) | ||
| } | ||
| }() | ||
|
|
||
| select { | ||
| case <-exitCalled: | ||
| assert.True(t, exitInvoked, "exitFunction should have been invoked") | ||
| assert.Greater(t, callCount, 1, "Should have made at least 2 API calls") | ||
| case <-time.After(2 * time.Second): | ||
| t.Fatal("Test timed out waiting for exitFunction to be called") | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix data races in TestVerifyConfigurationsChanged (exitInvoked and callCount)
exitInvoked and callCount are written inside the HTTP responder / verification goroutine and read on the main test goroutine, which is a data race under go test -race.
You can avoid sharing mutable state by sending the call count through the existing channel and dropping exitInvoked entirely. For example:
- callCount := 0
+ callCount := 0
@@
- var exitInvoked bool = false
- exitCalled := make(chan bool)
-
- exitFunction = func(c int) {
- exitInvoked = true
- exitCalled <- true
- }
+ exitCalled := make(chan int, 1)
+
+ exitFunction = func(c int) {
+ // Capture the current callCount when a change is detected.
+ exitCalled <- callCount
+ }
@@
- case <-exitCalled:
- assert.True(t, exitInvoked, "exitFunction should have been invoked")
- assert.Greater(t, callCount, 1, "Should have made at least 2 API calls")
+ case count := <-exitCalled:
+ assert.Greater(t, count, 1, "Should have made at least 2 API calls")This preserves the assertions (including “at least 2 API calls”) while eliminating racy shared variables.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func TestVerifyConfigurationsChanged(t *testing.T) { | |
| client := resty.New() | |
| httpmock.ActivateNonDefault(client.GetClient()) | |
| defer httpmock.DeactivateAndReset() | |
| uri := "localhost:8081" | |
| callCount := 0 | |
| httpmock.RegisterResponder("GET", fmt.Sprintf("%s/honeypots", uri), | |
| func(req *http.Request) (*http.Response, error) { | |
| callCount++ | |
| config := "apiVersion: \"v1\"\nprotocol: \"ssh\"\naddress: \":2222\"\ndescription: \"SSH interactive ChatGPT\"\ncommands:\n - regex: \"^(.+)$\"\n plugin: \"LLMHoneypot\"\nserverVersion: \"OpenSSH\"\nserverName: \"ubuntu\"\npasswordRegex: \"^(root|qwerty|Smoker666|123456|jenkins|minecraft|sinus|alex|postgres|Ly123456)$\"\ndeadlineTimeoutSeconds: 60\nplugin:\n llmModel: \"gpt-4o\"\n openAISecretKey: \"1234\"\n" | |
| if callCount > 1 { | |
| config = "apiVersion: \"v1\"\nprotocol: \"ssh\"\naddress: \":2222\"\ndescription: \"SSH interactive ChatGPT MODIFIED\"\ncommands:\n - regex: \"^(.+)$\"\n plugin: \"LLMHoneypot\"\nserverVersion: \"OpenSSH\"\nserverName: \"ubuntu\"\npasswordRegex: \"^(root|qwerty|Smoker666|123456|jenkins|minecraft|sinus|alex|postgres|Ly123456)$\"\ndeadlineTimeoutSeconds: 60\nplugin:\n llmModel: \"gpt-3.5-turbo\"\n openAISecretKey: \"1234\"\n" | |
| } | |
| resp, err := httpmock.NewJsonResponse(200, &[]HoneypotConfigResponseDTO{ | |
| { | |
| ID: "123456", | |
| Config: config, | |
| TokenID: "1234567", | |
| }, | |
| }) | |
| if err != nil { | |
| return httpmock.NewStringResponse(500, ""), nil | |
| } | |
| return resp, nil | |
| }, | |
| ) | |
| var exitInvoked bool = false | |
| exitCalled := make(chan bool) | |
| exitFunction = func(c int) { | |
| exitInvoked = true | |
| exitCalled <- true | |
| } | |
| beelzebubCloud := InitBeelzebubCloud(uri, "sdjdnklfjndslkjanfk", false) | |
| beelzebubCloud.client = client | |
| beelzebubCloud.PollingInterval = 100 * time.Millisecond | |
| go func() { | |
| err := beelzebubCloud.verifyConfigurationsChanged() | |
| if err != nil { | |
| t.Errorf("verifyConfigurationsChanged returned with error: %v", err) | |
| } | |
| }() | |
| select { | |
| case <-exitCalled: | |
| assert.True(t, exitInvoked, "exitFunction should have been invoked") | |
| assert.Greater(t, callCount, 1, "Should have made at least 2 API calls") | |
| case <-time.After(2 * time.Second): | |
| t.Fatal("Test timed out waiting for exitFunction to be called") | |
| } | |
| } | |
| func TestVerifyConfigurationsChanged(t *testing.T) { | |
| client := resty.New() | |
| httpmock.ActivateNonDefault(client.GetClient()) | |
| defer httpmock.DeactivateAndReset() | |
| uri := "localhost:8081" | |
| callCount := 0 | |
| httpmock.RegisterResponder("GET", fmt.Sprintf("%s/honeypots", uri), | |
| func(req *http.Request) (*http.Response, error) { | |
| callCount++ | |
| config := "apiVersion: \"v1\"\nprotocol: \"ssh\"\naddress: \":2222\"\ndescription: \"SSH interactive ChatGPT\"\ncommands:\n - regex: \"^(.+)$\"\n plugin: \"LLMHoneypot\"\nserverVersion: \"OpenSSH\"\nserverName: \"ubuntu\"\npasswordRegex: \"^(root|qwerty|Smoker666|123456|jenkins|minecraft|sinus|alex|postgres|Ly123456)$\"\ndeadlineTimeoutSeconds: 60\nplugin:\n llmModel: \"gpt-4o\"\n openAISecretKey: \"1234\"\n" | |
| if callCount > 1 { | |
| config = "apiVersion: \"v1\"\nprotocol: \"ssh\"\naddress: \":2222\"\ndescription: \"SSH interactive ChatGPT MODIFIED\"\ncommands:\n - regex: \"^(.+)$\"\n plugin: \"LLMHoneypot\"\nserverVersion: \"OpenSSH\"\nserverName: \"ubuntu\"\npasswordRegex: \"^(root|qwerty|Smoker666|123456|jenkins|minecraft|sinus|alex|postgres|Ly123456)$\"\ndeadlineTimeoutSeconds: 60\nplugin:\n llmModel: \"gpt-3.5-turbo\"\n openAISecretKey: \"1234\"\n" | |
| } | |
| resp, err := httpmock.NewJsonResponse(200, &[]HoneypotConfigResponseDTO{ | |
| { | |
| ID: "123456", | |
| Config: config, | |
| TokenID: "1234567", | |
| }, | |
| }) | |
| if err != nil { | |
| return httpmock.NewStringResponse(500, ""), nil | |
| } | |
| return resp, nil | |
| }, | |
| ) | |
| exitCalled := make(chan int, 1) | |
| exitFunction = func(c int) { | |
| // Capture the current callCount when a change is detected. | |
| exitCalled <- callCount | |
| } | |
| beelzebubCloud := InitBeelzebubCloud(uri, "sdjdnklfjndslkjanfk", false) | |
| beelzebubCloud.client = client | |
| beelzebubCloud.PollingInterval = 100 * time.Millisecond | |
| go func() { | |
| err := beelzebubCloud.verifyConfigurationsChanged() | |
| if err != nil { | |
| t.Errorf("verifyConfigurationsChanged returned with error: %v", err) | |
| } | |
| }() | |
| select { | |
| case count := <-exitCalled: | |
| assert.Greater(t, count, 1, "Should have made at least 2 API calls") | |
| case <-time.After(2 * time.Second): | |
| t.Fatal("Test timed out waiting for exitFunction to be called") | |
| } | |
| } |
🤖 Prompt for AI Agents
In plugins/beelzebub-cloud_test.go around lines 237 to 296,
TestVerifyConfigurationsChanged has data races because exitInvoked and callCount
are mutated in the HTTP responder goroutine and read on the main test goroutine;
remove the shared mutable state by dropping exitInvoked and instead send the
call count over a channel when you want to signal test completion (reuse or add
a channel like exitCalled to carry the int), increment a local callCount inside
the responder and send that value on the channel when the responder triggers the
exit path, update exitFunction to only signal via the channel (or have the
responder send after calling it), and then assert on the received call count in
the main goroutine — this eliminates the race while preserving the “at least 2
API calls” and exit invocation assertions.
| type EventDTO struct { | ||
| DateTime string | ||
| RemoteAddr string | ||
| Protocol string | ||
| Command string | ||
| CommandOutput string | ||
| Status string | ||
| Msg string | ||
| ID string | ||
| Environ string | ||
| User string | ||
| Password string | ||
| Client string | ||
| Headers string | ||
| HeadersMap map[string][]string | ||
| Cookies string | ||
| UserAgent string | ||
| HostHTTPRequest string | ||
| Body string | ||
| HTTPMethod string | ||
| RequestURI string | ||
| Description string | ||
| SourceIp string | ||
| SourcePort string | ||
| TLSServerName string | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
EventDTO/mapping are mostly straightforward; header handling may be unintentionally changed
Introducing EventDTO and mapping via mapToEventDTO cleanly decouples the wire format from tracer.Event, and most fields are copied 1:1, which preserves existing behavior.
Two points to double-check:
Headersis now set asjson.Marshal(event.Headers)whenevent.Headersis non-empty. SinceHeadersis already astring, this effectively wraps it in JSON quotes (e.g.Header: value→"\"Header: value\""). Unless the consumer explicitly expects a JSON-encoded string literal, you probably want either:eventDTO.Headers = event.Headers, or- JSON-marshalling
event.HeadersMapinstead and leavingHeadersas-is.
HeadersMapexists onEventDTObut is never populated inmapToEventDTO. If the remote side relies on that field, this will silently drop information; if not, consider removing it fromEventDTOto avoid confusion.
Also, the error path in mapToEventDTO (failing to marshal a string) is effectively unreachable; you can keep it for completeness, but it’s unlikely to ever trigger.
Also applies to: 76-105, 178-213
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect the EventDTO struct and related code sections
cat plugins/beelzebub-cloud.go | head -250 | tail -250
Length of output: 5767
🏁 Script executed:
#!/bin/bash
# Find and display the mapToEventDTO function
rg -n "mapToEventDTO" plugins/beelzebub-cloud.go -A 30Length of output: 2052
🏁 Script executed:
#!/bin/bash
# Find GetHoneypotsConfigurations implementation
rg -n "GetHoneypotsConfigurations" plugins/beelzebub-cloud.go -A 20Length of output: 1516
🏁 Script executed:
#!/bin/bash
# Find verifyConfigurationsChanged function
rg -n "verifyConfigurationsChanged" plugins/beelzebub-cloud.go -A 25Length of output: 1662
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for HeadersMap usage across the codebase
rg -n "HeadersMap" --type go
Length of output: 196
🏁 Script executed:
#!/bin/bash
# Check tracer.Event definition
rg -n "type Event struct" -A 30 --type goLength of output: 1290
🏁 Script executed:
#!/bin/bash
# Check if Headers field in tracer.Event and how it's set
rg -n "Headers" plugins/beelzebub-cloud.go --type go -B 2 -A 2Length of output: 438
Fix header handling: stop double-encoding Headers and populate HeadersMap
The review findings are confirmed. The mapToEventDTO function introduces two issues:
-
Headers is double-encoded (line 205):
json.Marshal(event.Headers)treats the already-stringevent.Headersas a value to encode, wrapping it in JSON quotes. This transforms"Header: value"into"\"Header: value\"", changing the wire format unexpectedly. -
HeadersMap is silently dropped (line 178-213): The
event.HeadersMapfield is populated by the HTTP protocol handler (seeprotocols/strategies/HTTP/http.go:134) but is never copied toeventDTO.HeadersMap. If the remote consumer expects structured headers in this field, information is lost.
Fixes required (lines 204-209):
- Replace
json.Marshal(event.Headers)with direct assignment:eventDTO.Headers = event.Headers - Add:
eventDTO.HeadersMap = event.HeadersMap(or conditionally if nil-checking is preferred)
If the remote API contract specifies JSON-encoded headers, adjust the approach accordingly, but do not silently lose the HeadersMap data.
🤖 Prompt for AI Agents
In plugins/beelzebub-cloud.go around lines 18 to 43, the EventDTO struct
handling in mapToEventDTO double-encodes the Headers field and omits copying
HeadersMap: change the mapping so you assign event.Headers directly to
eventDTO.Headers (no json.Marshal) and also set eventDTO.HeadersMap =
event.HeadersMap (or conditionally copy when non-nil) so structured headers are
preserved; ensure any nil checks as needed but do not JSON-encode the
already-string Headers.
| func InitBeelzebubCloud(uri, authToken string, enableVerifyConfigurationsChanged bool) *beelzebubCloud { | ||
| beelzebubCloud := &beelzebubCloud{ | ||
| URI: uri, | ||
| AuthToken: authToken, | ||
| client: resty.New(), | ||
| PollingInterval: 15 * time.Second, | ||
| } | ||
| if enableVerifyConfigurationsChanged { | ||
| go func() { | ||
| if err := beelzebubCloud.verifyConfigurationsChanged(); err != nil { | ||
| log.Fatalf("Error verify configurations changed: %s", err.Error()) | ||
| } | ||
| }() | ||
| } | ||
| return beelzebubCloud | ||
| } |
There was a problem hiding this comment.
Background config polling is useful but currently exits the process on any error/change
InitBeelzebubCloud now optionally starts a goroutine that loops in verifyConfigurationsChanged, which:
- Calls
GetHoneypotsConfigurations()on every interval. - Returns on any error, causing the goroutine to invoke
log.Fatalf(...)(process exit with code 1). - Calls
exitFunction(0)(defaultos.Exit(0)) when a hash change is detected, also terminating the process.
This is a strong behavior change: transient network/API issues or configuration updates now kill the entire process from a background goroutine.
Consider softening this behavior:
- Log errors and continue with backoff instead of
log.Fatalf, so brief outages don’t crash the honeypot. - After calling
exitFunction, eitherreturnorbreakto make the control flow explicit (especially important whenexitFunctionis overridden in tests or future code). - Optionally accept a context or a stop channel so the polling goroutine can be shut down cleanly if the plugin is ever reused in a shorter-lived context.
These tweaks would keep the verification feature while improving robustness and testability.
Also applies to: 157-176
🤖 Prompt for AI Agents
In plugins/beelzebub-cloud.go around lines 59-74 (and similarly 157-176), the
background polling goroutine currently calls log.Fatalf on any error and invokes
exitFunction(0) on hash changes which unconditionally terminates the process;
change this to log the error and retry with a backoff (do not call log.Fatalf),
make the control flow explicit after calling exitFunction by returning or
breaking so tests/overrides behave deterministically, and add an optional
context.Context or stop channel parameter to InitBeelzebubCloud so the goroutine
can be cancelled cleanly; implement backoff/retry on transient
GetHoneypotsConfigurations errors and ensure errors are logged with details
rather than exiting the process.
All Submissions:
New Feature Submissions:
Changes to Core Features:
Summary by CodeRabbit
New Features
Tests
Chores