Add CompStatus Writing Locations #161
Conversation
mykeelium
added
bug
WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant CollectionTask
participant StealthProducer
participant ComputerFileProducer
participant LdapProducer
participant CompStatusChannel
CollectionTask->>StealthProducer: new StealthProducer(..., CompStatusChannel)
CollectionTask->>ComputerFileProducer: new ComputerFileProducer(..., CompStatusChannel)
CollectionTask->>LdapProducer: new LdapProducer(..., CompStatusChannel)
StealthProducer->>CompStatusChannel: Write CSVComputerStatus on SID resolution
ComputerFileProducer->>CompStatusChannel: Write CSVComputerStatus on SID resolution
sequenceDiagram
participant ObjectProcessors
participant CompStatusChannel
ObjectProcessors->>ObjectProcessors: ProcessEnterpriseCA(..., CompStatusChannel)
ObjectProcessors->>CompStatusChannel: Write CSVComputerStatus on CA SID resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
mykeelium
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Runtime/ObjectProcessors.cs (2)
705-711: Good implementation with minor consistency suggestion.The status reporting logic is well-implemented with proper error handling and cancellation token usage.
Consider using the constant
StatusSuccess(defined at line 26) instead ofComputerStatus.Successfor consistency with other status reporting in the codebase.- Status = ComputerStatus.Success, + Status = StatusSuccess,
734-740: Consistent implementation with the same minor suggestion.The status reporting logic is correctly implemented and mirrors the previous block appropriately.
Same suggestion as above: consider using the
StatusSuccessconstant for consistency.- Status = ComputerStatus.Success, + Status = StatusSuccess,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/Producers/BaseProducer.cs(1 hunks)src/Producers/ComputerFileProducer.cs(2 hunks)src/Producers/LdapProducer.cs(1 hunks)src/Producers/StealthProducer.cs(2 hunks)src/Runtime/CollectionTask.cs(1 hunks)src/Runtime/ObjectProcessors.cs(4 hunks)
🔇 Additional comments (8)
src/Producers/BaseProducer.cs (2)
27-27: LGTM: Well-structured addition of CompStatusChannel field.The new protected readonly field follows the established pattern of other channel fields in the class.
29-34: LGTM: Constructor properly updated to handle new channel parameter.The constructor signature and field assignment are correctly implemented, maintaining consistency with existing channel handling patterns.
src/Runtime/CollectionTask.cs (1)
57-61: LGTM: Consistent channel propagation to all producer types.All producer constructors are correctly updated to receive the _compStatusChannel parameter, ensuring unified status reporting across Stealth, ComputerFile, and LDAP producers.
src/Producers/LdapProducer.cs (1)
15-18: LGTM: Constructor properly updated for base class compatibility.The constructor correctly accepts and passes the compStatusChannel parameter to the base class, maintaining architectural consistency even though LdapProducer doesn't currently write status messages.
src/Producers/StealthProducer.cs (1)
28-31: LGTM: Constructor properly updated to accept compStatusChannel.The constructor correctly accepts and passes the compStatusChannel parameter to the base class constructor.
src/Producers/ComputerFileProducer.cs (2)
20-23: LGTM: Constructor properly updated with compStatusChannel parameter.The constructor correctly accepts and passes the compStatusChannel parameter to the base class constructor, maintaining consistency with other producer classes.
71-78: LGTM: Well-implemented status reporting on successful host resolution.The status writing logic is correctly placed after successful host-to-SID resolution and properly uses the cancellation token. The task string accurately identifies the producer and method.
src/Runtime/ObjectProcessors.cs (1)
95-95: LGTM: Method signature update is consistent.The method signature change properly adds the
compStatusChannelparameter and the call site is updated accordingly. This follows the established pattern from other processing methods.Also applies to: 661-662
mykeelium
removed
the
blocked by SHC PR
Description
This change adds locations where machines could be logged into the compstatus.csv.
Motivation and Context
Helps to add possible locations for logging computers that were connected to in collection. This should hopefully log the situation that is occurring in ticket BED-5935
How Has This Been Tested?
This has been tested by creating executables and ensure these new locations logged in a lab environment.
Screenshots (if appropriate):
Types of changes
Checklist:
Summary by CodeRabbit