New Components - splunk #15966
New Components - splunk #15966
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThis pull request introduces a series of new Splunk modules. It adds several action modules (create-event, run-search, get-search-job-status) and multiple source modules (new-alert-fired, new-search-event, new-search-result, and a common base) that integrate with Splunk. Additionally, the Splunk application module has been expanded with new property definitions and methods for interacting with the Splunk API, and the package configuration has been updated to bump the version and add new dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateEvent as Create Event Action
participant SplunkAPI as Splunk API
User->>CreateEvent: Trigger "Create Event" action
CreateEvent->>SplunkAPI: Call sendEvent(index, eventData, source, sourcetype)
SplunkAPI-->>CreateEvent: Return success response
CreateEvent-->>User: Emit summary message
sequenceDiagram
participant Scheduler
participant NewAlert as New Alert Source
participant SplunkSys as Splunk App
Scheduler->>NewAlert: Poll for fired alerts
NewAlert->>SplunkSys: Call listFiredAlerts()
SplunkSys-->>NewAlert: Return alert data
NewAlert->>NewAlert: Generate metadata for each alert
NewAlert-->>Scheduler: Emit alert events
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
components/splunk/actions/create-event/create-event.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/splunk/sources/common/base.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/splunk/sources/new-alert-fired/new-alert-fired.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ 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:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (13)
components/splunk/actions/create-event/create-event.mjs (2)
44-57: Add error handling to the run methodConsider adding try/catch block around the API call to handle potential errors gracefully and provide better error messages to users.
async run({ $ }) { + try { const response = await this.splunk.sendEvent({ $, selfSigned: this.selfSigned, params: { index: this.indexName, source: this.source, sourcetype: this.sourcetype, }, data: this.eventData, }); $.export("$summary", `Event sent to index ${this.indexName} successfully`); return response; + } catch (error) { + $.export("$summary", `Failed to send event: ${error.message}`); + throw error; + } },
55-55: Include more details in the success summaryConsider adding more context to the success message to help users understand what was sent.
- $.export("$summary", `Event sent to index ${this.indexName} successfully`); + $.export("$summary", `Event sent to index "${this.indexName}" successfully${this.source ? ` with source "${this.source}"` : ""}`);components/splunk/actions/get-search-job-status/get-search-job-status.mjs (1)
27-35: Add error handling to the run methodConsider adding try/catch block around the API call to handle potential errors gracefully and provide better error messages to users.
async run({ $ }) { + try { const response = await this.splunk.getSearchJobStatus({ $, selfSigned: this.selfSigned, jobId: this.jobId, }); $.export("$summary", `Successfully retrieved status for job ID ${this.jobId}`); return response; + } catch (error) { + $.export("$summary", `Failed to retrieve job status: ${error.message}`); + throw error; + } },components/splunk/actions/run-search/run-search.mjs (3)
32-32: Fix formatting in latestTime descriptionThere's extra leading whitespace in the latestTime description that should be removed for consistency with the earliestTime description.
- description: " Specify a time string. Sets the latest (exclusive), respectively, time bounds for the search. The time string can be either a UTC time (with fractional seconds), a relative time specifier (to now) or a formatted time string. Refer to [Time modifiers](https://docs.splunk.com/Documentation/Splunk/9.4.1/SearchReference/SearchTimeModifiers) for search for information and examples of specifying a time string.", + description: "Specify a time string. Sets the latest (exclusive), respectively, time bounds for the search. The time string can be either a UTC time (with fractional seconds), a relative time specifier (to now) or a formatted time string. Refer to [Time modifiers](https://docs.splunk.com/Documentation/Splunk/9.4.1/SearchReference/SearchTimeModifiers) for search for information and examples of specifying a time string.",
36-48: Add error handling to the run methodConsider adding try/catch block around the API call to handle potential errors gracefully and provide better error messages to users.
async run({ $ }) { + try { const response = await this.splunk.executeSearchQuery({ $, selfSigned: this.selfSigned, data: { search: this.query, earliest_time: this.earliestTime, latest_time: this.latestTime, }, }); $.export("$summary", `Executed Splunk search query: ${this.query}`); return response; + } catch (error) { + $.export("$summary", `Failed to execute search query: ${error.message}`); + throw error; + } },
46-46: Include more context in success summaryConsider adding more information to the success message, such as the time range if specified.
- $.export("$summary", `Executed Splunk search query: ${this.query}`); + $.export("$summary", `Executed Splunk search query: ${this.query}${this.earliestTime ? ` from ${this.earliestTime}` : ""}${this.latestTime ? ` to ${this.latestTime}` : ""}`);components/splunk/sources/new-search-result/new-search-result.mjs (1)
13-19: Consider enhancing the metadata generation with result timestamps.The
generateMetamethod currently usesDate.now()for the timestamp, which represents when the event was processed rather than when it occurred. If the Splunk result contains its own timestamp, consider using that for more accurate event sequencing.generateMeta(result) { return { id: result.sid, summary: `New Search Results with SID: ${result.sid}`, - ts: Date.now(), + ts: result._time ? new Date(result._time).getTime() : Date.now(), }; },components/splunk/sources/new-search-event/new-search-event.mjs (1)
14-20: Enhance event metadata for better information and deduplication.The current metadata generation has several areas for improvement:
- The summary is generic and could be more informative
- Using MD5 of the entire event might not be the most efficient approach if a unique field is available
- The timestamp should ideally come from the event itself when available
generateMeta(event) { + // Extract a more meaningful summary from the event if possible + const eventName = event.name || event._raw?.slice(0, 30) || "Unknown"; + + // Use a specific field if available, or fall back to MD5 hash + const eventId = event.id || event._cd || md5(JSON.stringify(event)); + + // Use event timestamp if available, fall back to current time + const eventTime = event._time ? new Date(event._time).getTime() : Date.now(); + return { - id: md5(JSON.stringify(event)), - summary: "New Search Event", - ts: Date.now(), + id: eventId, + summary: `New Search Event: ${eventName}`, + ts: eventTime, }; },components/splunk/sources/new-alert-fired/new-alert-fired.mjs (1)
13-19: Use alert timestamp for more accurate event sequencing.The current implementation uses the current time for the event timestamp rather than the actual time when the alert was triggered. If the alert object contains its own timestamp, it would be more accurate to use that.
generateMeta(alert) { + // Use the alert's trigger time if available + const alertTime = alert.trigger_time || alert.triggered_at || Date.now(); return { id: alert.id, summary: `New Alert Fired: ${alert.name}`, - ts: Date.now(), + ts: alertTime, }; },components/splunk/splunk.app.mjs (4)
8-74: Fix minor spelling issue in the documentation.In the description for
query(line 73), "sytax" should be "syntax". This helps keep the documentation polished and clear.- description: "The Splunk search query. Example: `search *`. [See the documentation](https://docs.splunk.com/Documentation/Splunk/9.4.1/SearchReference/Search) for more information about search command sytax." + description: "The Splunk search query. Example: `search *`. [See the documentation](https://docs.splunk.com/Documentation/Splunk/9.4.1/SearchReference/Search) for more information about search command syntax."
65-69: Document security implications.While allowing self-signed certificates (via
selfSigned) is sometimes necessary, it poses security risks in production environments. Consider providing a cautionary note in user-facing documentation to emphasize these risks and encourage secure certificates whenever possible.
105-128: Use consistent naming in resource methods.While the
listJobs,listIndexes,listSavedSearches, andlistFiredAlertsmethods have uniform naming, consider a shared pattern for request parameters (e.g., passingoptsconsistently). This keeps the API predictable.
160-166: Validate input before sending events.
sendEventposts data to Splunk. Consider adding minimal validation (e.g., verifying a non-empty payload) to prevent unintended or malformed events.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
components/splunk/actions/create-event/create-event.mjs(1 hunks)components/splunk/actions/get-search-job-status/get-search-job-status.mjs(1 hunks)components/splunk/actions/run-search/run-search.mjs(1 hunks)components/splunk/package.json(2 hunks)components/splunk/sources/common/base.mjs(1 hunks)components/splunk/sources/new-alert-fired/new-alert-fired.mjs(1 hunks)components/splunk/sources/new-search-event/new-search-event.mjs(1 hunks)components/splunk/sources/new-search-result/new-search-result.mjs(1 hunks)components/splunk/splunk.app.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
🔇 Additional comments (9)
components/splunk/package.json (2)
3-3: Version update looks goodThe version bump from 0.0.1 to 0.1.0 follows semantic versioning practices, correctly indicating this is a minor feature update rather than just a patch.
15-18:✅ Verification successful
Verify necessity of the 'https' dependency
The 'https' module is a native Node.js module that should be available without explicit installation. Consider removing this dependency unless you're using a specific version or feature not available in the native module.
Additionally, note that MD5 is considered cryptographically weak for security-sensitive applications. If you're using it for non-security purposes like generating unique identifiers, that's acceptable.
🏁 Script executed:
#!/bin/bash # Check if the 'https' module is used in a non-standard way in the codebase echo "Checking for custom usage of 'https' module..." rg -A 3 -B 3 "import.*https" --type jsLength of output: 2741
Action: Remove explicit installation of the native 'https' module
After verifying the codebase, there’s no evidence of any non-standard or custom usage of the 'https' module that would justify keeping its explicit installation in the dependencies. Since Node.js provides 'https' natively, it's safe to remove it from
components/splunk/package.jsonunless a specific version or extended functionality is actually required. Additionally, please ensure that the use of MD5 (if any) is limited to non-security-related purposes, due to its known cryptographic weaknesses.
- File:
components/splunk/package.json(Lines 15–18)components/splunk/actions/create-event/create-event.mjs (1)
1-58: Well-structured new action moduleThe Create Event action is well-organized, following Pipedream's component patterns with appropriate documentation links, clear property definitions, and a clean implementation.
components/splunk/actions/get-search-job-status/get-search-job-status.mjs (1)
1-36: Well-structured new action moduleThe Get Search Job Status action is well-organized, following Pipedream's component patterns with appropriate documentation links, clear property definitions, and a clean implementation.
components/splunk/actions/run-search/run-search.mjs (1)
1-49: Well-structured new action moduleThe Run Search action is well-organized, following Pipedream's component patterns with appropriate documentation links, clear property definitions, and a clean implementation.
components/splunk/sources/new-alert-fired/new-alert-fired.mjs (1)
21-38:✅ Verification successful
Limit and filter alert processing for better performance.
The current implementation collects all alerts without any filtering or limits, which could lead to processing a large number of alerts and impact performance.
async run() { + // Use pagination parameters to limit and filter alerts + const paginationParams = { + count: 20, // Process a reasonable number of alerts + offset: 0, + // Only retrieve relatively recent alerts + timeRange: "-24h", + }; + const results = this.splunk.paginate({ resourceFn: this.splunk.listFiredAlerts, args: { selfSigned: this.selfSigned, + ...paginationParams, }, }); const alerts = []; + let count = 0; for await (const item of results) { alerts.push(item); + count++; + // Additional safeguard to prevent processing too many alerts + if (count >= 50) break; } alerts.forEach((alert) => { const meta = this.generateMeta(alert); this.$emit(alert, meta); }); },
🏁 Script executed:
#!/bin/bash # Check if the Splunk listFiredAlerts method supports pagination and time range parameters rg -A 10 "listFiredAlerts\s*\([^)]*\)\s*{" components/splunk/Length of output: 189
Below is a revised shell script using a corrected regex pattern that should properly match the definition of the
listFiredAlertsmethod. Please run the script to manually verify whether this method supports additional pagination parameters (likecount,offset, and atimeRange) as suggested in the review comment.Next Steps:
- Run the revised shell script to check the method’s signature.
- Based on the output, verify that the Splunk API indeed supports the additional parameters before proceeding with the refactor.
Once you confirm the method supports these parameters, you can safely adopt the refactoring changes suggested in the review.
🏁 Script executed:
#!/bin/bash # Corrected regex pattern to search for the definition of listFiredAlerts rg -A 10 'listFiredAlerts\s*\([^)]*\)\s*\{' components/splunk/Length of output: 827
Action Required: Confirm Pagination Parameters in Alert Processing
After verifying the implementation of
listFiredAlertsincomponents/splunk/splunk.app.mjs, we see that the method accepts anoptsobject and spreads its properties into the request. This confirms that passing additional options (including pagination parameters likecount,offset, andtimeRange) is supported.
Verified Location:
components/splunk/splunk.app.mjs– ThelistFiredAlerts(opts = {})method spreadsoptsinto the request.Key Considerations:
- The suggested approach to limit and filter alerts by spreading
paginationParamsinto the arguments oflistFiredAlertsis valid.- The additional safeguard in the loop (using a counter and breaking after 50 alerts) is an extra measure to protect against excessive processing.
Please ensure that the backend API correctly interprets these pagination parameters. Once confirmed, you can incorporate the diff snippet below to improve performance by limiting alert processing:
async run() { + // Use pagination parameters to limit and filter alerts + const paginationParams = { + count: 20, // Process a reasonable number of alerts at once + offset: 0, + // Only retrieve alerts from the past 24 hours + timeRange: "-24h", + }; const results = this.splunk.paginate({ resourceFn: this.splunk.listFiredAlerts, args: { selfSigned: this.selfSigned, + ...paginationParams, }, }); const alerts = []; + let count = 0; for await (const item of results) { alerts.push(item); + count++; + // Additional safeguard to prevent processing too many alerts + if (count >= 50) break; } alerts.forEach((alert) => { const meta = this.generateMeta(alert); this.$emit(alert, meta); }); },components/splunk/splunk.app.mjs (3)
80-104: Review certificate-rejection override for potential security impact.The
_makeRequest()method setsrejectUnauthorized: falseifselfSignedis true. This can allow MITM attacks. If this is only for local dev or test scenarios, confirm it’s properly documented or restricted from production usage.
129-159: Confirm request methods.
executeSearchQueryusesPOST, butgetSearchJobStatus,getSearchResults, andgetSearchEventsuseGETby default. Ensure both the method and endpoint path align with Splunk’s expectations (e.g., older Splunk versions might differ in endpoints).
77-79:❓ Verification inconclusive
Ensure base URL correctness.
The
_baseUrl()method constructs a URL that appends:${this.$auth.api_port}. Confirm that user-supplied values don’t inadvertently include a port, which could cause malformed URLs.
🏁 Script executed:
#!/bin/bash # Searching for references to `api_url` in the repo to confirm usage consistency. rg -A 3 '\$auth\.api_url'Length of output: 2818
Attention: Validate API URL Inputs in Splunk App
Please double-check that the value provided in
this.$auth.api_urldoes not already include a port. Unlike other components that simply append additional path segments, the Splunk app explicitly concatenates:${this.$auth.api_port}to the URL when constructing the base endpoint. If the API URL already embeds a port, it could yield a malformed URL (e.g., duplicating port information). Ensure that any user-supplied values are sanitized or validated accordingly.
- Location:
components/splunk/splunk.app.mjs(Lines 77-79)
|
Hi @michelle0927, could you remove the Self-signed prop on each action/source and use the variable |
Resolves #15893
Note to QA (@vunguyenhung ): Set the
selfSignedprop totruefor testing.Summary by CodeRabbit
New Features
Chores