fix(config-profiles): restart nodes on snippet create/update/delete#183
fix(config-profiles): restart nodes on snippet create/update/delete#183TopPro104 wants to merge 1 commit into
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Greptile SummaryThis PR fixes a bug where creating, updating, or deleting a snippet did not trigger a node restart, leaving nodes serving stale configs until manually restarted. It injects
Confidence Score: 4/5The core fix is correct and follows the existing queue-dispatch pattern in the codebase, but the startAllNodes call sits after an already-committed DB write with no isolation, so a transient Redis failure leaves the snippet persisted while the caller receives an error. The queue dispatch for create/update/delete is logically sound and uses proper deduplication. The risk is that if BullMQ is briefly unavailable after a snippet is written to the database, the caller gets an error and — for createSnippet — cannot retry without hitting the unique-name constraint. This is the same unguarded pattern used in ConfigProfileService, but it is most disruptive on the create path because the write is not idempotent. src/modules/config-profiles/snippets.service.ts — specifically the error-handling boundaries around the startAllNodes calls in createSnippet and deleteSnippetByName. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant SnippetsService
participant SnippetsRepository
participant NodesQueuesService
participant BullMQ
Client->>SnippetsService: createSnippet / updateSnippet / deleteSnippetByName
SnippetsService->>SnippetsRepository: create / update / deleteByName
SnippetsRepository-->>SnippetsService: OK (committed to DB)
SnippetsService->>NodesQueuesService: "startAllNodes({ emitter })"
NodesQueuesService->>BullMQ: enqueue START_ALL_NODES (deduplicated)
BullMQ-->>NodesQueuesService: job added
NodesQueuesService-->>SnippetsService: OK
SnippetsService->>SnippetsService: getSnippets()
SnippetsService-->>Client: updated snippet list
Reviews (1): Last reviewed commit: "fix(config-profiles): restart nodes on s..." | Re-trigger Greptile |
| # Security Policy | ||
|
|
||
| ## Reporting a Vulnerability | ||
|
|
||
| Use this section to tell people how to report a vulnerability. | ||
|
|
||
| Tell them where to go, how often they can expect to get an update on a | ||
| reported vulnerability, what to expect if the vulnerability is accepted or | ||
| declined, etc. |
There was a problem hiding this comment.
Placeholder SECURITY.md with no actual policy
This file contains only the GitHub-generated template text. Before merging, it should be filled in with the actual vulnerability reporting contact, expected response times, and disclosure process. Shipping an empty template signals to security researchers that there is no real reporting channel.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| await this.snippetsRepository.create(snippetEntity); | ||
|
|
||
| await this.nodesQueuesService.startAllNodes({ emitter: 'createSnippet' }); |
There was a problem hiding this comment.
Queue error after committed write can strand a created snippet
If startAllNodes throws (e.g. Redis unavailable), the snippetsRepository.create has already committed to the database, but the caller receives an error response. On retry the user gets SNIPPET_NAME_ALREADY_EXISTS, making the snippet unreachable through the normal create flow. The delete case is similar: the row is gone, but the caller sees an error and the follow-up retry returns SNIPPET_NOT_FOUND. This pattern is the same one used in ConfigProfileService.updateConfigProfile, so it is a codebase-level concern, but it is most disruptive here for createSnippet because of the non-idempotent unique constraint.
bdc973f to
34bed0e
Compare
|
Вообще, такая логика была намеренной, так как ручной перезапуск в некоторых случаях намного лучше, чем постоянные перезапуске при каждом создании/изменении/тд сниппета. Подумаю на счет вашей реализации. |
…/delete Snippets are expanded into the node config at generation time via XRayConfig.replaceSnippets(), but creating, updating or deleting a snippet did not trigger any node restart, unlike config profile updates. As a result nodes kept serving the previous snippet values until a manual restart. Restart only the nodes of config profiles that actually reference the changed snippet (mirroring how updating a config profile restarts its own nodes via startAllNodesByProfile), instead of restarting every node. Failures while enqueuing restarts are logged and swallowed, since the snippet mutation itself has already been committed. Adds XRayConfig.getReferencedSnippetNames() to resolve which profiles use a given snippet. Closes remnawave/panel#398
34bed0e to
fa7a0b9
Compare
|
Согласен, рестартить все ноды это неправильно. Переделал, теперь перезапускаются только ноды профилей, которые реально ссылаются на изменённый сниппет |
Кстати, если в каких-то сценариях ручной перезапуск удобнее то могу добавить настройку, чтобы авто-рестарт при изменении сниппета можно было включать/выключать. Дефолт на ваше усмотрение. |
Description
Snippets are expanded into the node config at generation time via
XRayConfig.replaceSnippets()(seeget-prepared-config-with-users.handler.ts). However, creating, updating or deleting a snippet did not trigger any node restart — unlike updating a config profile, which callsstartAllNodesByProfile.As a result, after editing a snippet the panel saved the new value but nodes kept serving the old config until a manual node restart.
Fix
Trigger
nodesQueuesService.startAllNodes()after snippetcreate,updateanddeleteinSnippetsService, so the regenerated config (with the new snippet values) is propagated to nodes automatically.Since snippets are global and may be referenced by any config profile,
startAllNodesis used rather than restarting a single profile's nodes.Closes remnawave/panel#398