-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New Components - sailpoint #14935
base: master
Are you sure you want to change the base?
New Components - sailpoint #14935
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThe pull request introduces several new modules and actions for integration with the SailPoint IdentityNow API. Key additions include actions for listing certification campaigns, submitting access requests, and uploading account source files. Two new event sources are also created for emitting events related to new identity creation and changes in identity attributes. Additionally, constants and utility functions are added to support these functionalities, while some files are deleted or modified to streamline the integration process. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
Sources - New Identity (Instant) - New Identity Attribute Change (Instant) Actions - Upload Account Source File - List Certification Campaigns - Submit Access Request
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (9)
components/sailpoint/sources/new-identity-instant/new-identity-instant.mjs (2)
4-11
: Consider starting with version 1.0.0The module configuration looks good, but since this is a production component for SailPoint integration, consider starting with version 1.0.0 instead of 0.0.1 to indicate a stable, production-ready release.
- version: "0.0.1", + version: "1.0.0",
4-22
: Consider documenting webhook security requirementsSince this is a webhook-based source for identity events, consider adding documentation about:
- Required webhook authentication methods
- IP allowlisting recommendations
- Rate limiting considerations
- Payload validation best practices
Would you like me to help create a documentation template for these security considerations?
components/sailpoint/sailpoint.app.mjs (1)
123-143
: Ensure proper handling of thehasMore
flag in pagination logic.The
hasMore
flag is assigned the value ofdata.length
, which will be a numeric value. While JavaScript treats non-zero numbers as truthy values, for clarity and to prevent potential logical errors, consider explicitly checking ifdata.length
is greater than zero.Apply this diff to enhance the clarity of the condition:
- hasMore = data.length; + hasMore = data.length > 0;components/sailpoint/common/utils.mjs (1)
2-6
: Handle absolute paths that do not start with/tmp
.The
checkTmp
function prepends/tmp/
to filenames that do not start with/tmp
. If the filename is an absolute path starting with/
, but not with/tmp
, this could result in an unintended path like/tmp//absolute/path
.Consider normalizing the path to handle such cases:
import path from "path"; export default { checkTmp(filename) { - if (!filename.startsWith("/tmp")) { - return `/tmp/${filename}`; + const tmpPath = "/tmp"; + const normalizedPath = path.isAbsolute(filename) + ? path.join(tmpPath, path.basename(filename)) + : path.join(tmpPath, filename); + return normalizedPath; } return filename; }, };components/sailpoint/sources/identity-attribute-change-instant/identity-attribute-change-instant.mjs (1)
17-19
: Handle potential undefinedevent.source.id
.In the
getSummary
method,event.source.id
is used directly. Ifevent.source
orevent.source.id
is undefined, this could lead to runtime errors. Consider adding a check to ensure thatevent.source.id
is defined before using it.Apply this diff to add a default value:
getSummary(event) { - return `Attributes changed for identity ${event.source.id}`; + const identityId = event.source?.id || "unknown"; + return `Attributes changed for identity ${identityId}`; }components/sailpoint/actions/list-certification-campaigns/list-certification-campaigns.mjs (2)
6-6
: Documentation link needs version updateThe API documentation link includes "v2024" which might need to be updated when new API versions are released.
Consider using a version variable or constant for the API version in the documentation URL.
27-31
: Consider memory usage for large datasetsThe current implementation collects all items into an array before returning, which could consume significant memory for large datasets.
Consider implementing streaming or chunking for large datasets:
- const responseArray = []; - for await (const item of response) { - responseArray.push(item); - } + const CHUNK_SIZE = 1000; + const responseArray = []; + let count = 0; + for await (const item of response) { + responseArray.push(item); + count++; + if (count % CHUNK_SIZE === 0) { + $.export("$summary", `Processing ${count} campaigns...`); + } + }components/sailpoint/actions/upload-account-source-file/upload-account-source-file.mjs (1)
9-9
: Add missing documentation linkThe documentation link in the description is empty.
Add the appropriate SailPoint API documentation link.
components/sailpoint/sources/common/base.mjs (1)
29-29
: Avoid using hardcoded IDs in documentationThe filter example contains a hardcoded ID which might be confusing for users. Consider using a more generic example.
-description: "JSONPath filter to conditionally invoke trigger when expression evaluates to true. **Example: $[?($.identityId == \"201327fda1c44704ac01181e963d463c\")]**. [See the documentation](https://developer.sailpoint.com/docs/extensibility/event-triggers/filtering-events/) for further information.", +description: "JSONPath filter to conditionally invoke trigger when expression evaluates to true. **Example: $[?($.identityId == \"<identity-id>\")]**. [See the documentation](https://developer.sailpoint.com/docs/extensibility/event-triggers/filtering-events/) for further information.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
components/sailpoint/.gitignore
(0 hunks)components/sailpoint/actions/list-certification-campaigns/list-certification-campaigns.mjs
(1 hunks)components/sailpoint/actions/submit-access-request/submit-access-request.mjs
(1 hunks)components/sailpoint/actions/upload-account-source-file/upload-account-source-file.mjs
(1 hunks)components/sailpoint/app/sailpoint.app.ts
(0 hunks)components/sailpoint/common/constants.mjs
(1 hunks)components/sailpoint/common/utils.mjs
(1 hunks)components/sailpoint/package.json
(1 hunks)components/sailpoint/sailpoint.app.mjs
(1 hunks)components/sailpoint/sources/common/base.mjs
(1 hunks)components/sailpoint/sources/identity-attribute-change-instant/identity-attribute-change-instant.mjs
(1 hunks)components/sailpoint/sources/identity-attribute-change-instant/test-event.mjs
(1 hunks)components/sailpoint/sources/new-identity-instant/new-identity-instant.mjs
(1 hunks)components/sailpoint/sources/new-identity-instant/test-event.mjs
(1 hunks)
💤 Files with no reviewable changes (2)
- components/sailpoint/.gitignore
- components/sailpoint/app/sailpoint.app.ts
✅ Files skipped from review due to trivial changes (2)
- components/sailpoint/sources/new-identity-instant/test-event.mjs
- components/sailpoint/sources/identity-attribute-change-instant/test-event.mjs
🔇 Additional comments (8)
components/sailpoint/package.json (2)
3-3
: LGTM! Version bump aligns with new features
The minor version increment (0.0.3 → 0.1.0) appropriately reflects the addition of new features like certification campaigns and access request actions.
5-5
: Verify the entry point structure
The entry point has been changed from dist/app/sailpoint.app.mjs
to sailpoint.app.mjs
. Please ensure this aligns with Pipedream's current component structure.
✅ Verification successful
Based on the output, I can now generate the final response since I have enough information to verify the entry point structure. The results show that most Pipedream components use a direct entry point pattern (e.g., componentname.app.mjs
) rather than a dist
directory, with only a small number of components still using the dist/app/
pattern.
Entry point change aligns with common component structure
The change from dist/app/sailpoint.app.mjs
to sailpoint.app.mjs
aligns with the predominant pattern used across Pipedream components. The majority of components in the codebase use a direct entry point without the dist
directory.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if other Pipedream components follow similar structure
# and don't use dist directory in their main entry point
# Check main entry point patterns in other components
fd package.json components/ --exec jq -r '. | select(.name | startswith("@pipedream/")) | {name: .name, main: .main}' {} | grep -v "sailpoint"
Length of output: 65711
components/sailpoint/sources/new-identity-instant/new-identity-instant.mjs (2)
1-2
: Verify imported module paths and exports
Let's ensure the imported modules exist and export the expected functionality.
✅ Verification successful
Import paths and exports are correctly configured
The verification shows that:
- Both imported files exist and are accessible
base.mjs
exports a default object with the expected functionalitytest-event.mjs
exports a default object with sample event data
The import paths are correct relative to the file location, and both modules have the required default exports.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and exports of imported modules
# Check if the imported files exist
echo "Checking for imported files..."
fd --type f "base.mjs$" --exec echo "Found base module: {}"
fd --type f "test-event.mjs$" --exec echo "Found test event module: {}"
# Check the content of these files
echo -e "\nChecking base module exports..."
ast-grep --pattern 'export default $_' components/sailpoint/sources/common/base.mjs || echo "No default export found in base.mjs"
echo -e "\nChecking test event exports..."
ast-grep --pattern 'export default $_' components/sailpoint/sources/new-identity-instant/test-event.mjs || echo "No default export found in test-event.mjs"
Length of output: 66707
Script:
#!/bin/bash
# Let's verify the specific files we need exist and check their content
echo "Checking for the specific imported files..."
ls -l components/sailpoint/sources/common/base.mjs components/sailpoint/sources/new-identity-instant/test-event.mjs || echo "Files not found"
echo -e "\nChecking base module content..."
cat components/sailpoint/sources/common/base.mjs || echo "Cannot read base.mjs"
echo -e "\nChecking test event content..."
cat components/sailpoint/sources/new-identity-instant/test-event.mjs || echo "Cannot read test-event.mjs"
Length of output: 3008
21-21
: Verify sample event structure
Let's ensure the sample event data in test-event.mjs matches the expected SailPoint identity creation event structure.
✅ Verification successful
Let me try a different approach to verify the sample event structure.
Let me try one more search to find the sample emit implementation.
Let me try one final search to locate the sample emit implementation specifically for SailPoint identity events.
Sample event structure verified and matches SailPoint identity creation format
The test event provides a valid representation of a SailPoint identity creation event with the expected structure containing:
- Identity object with type, id and name
- Attributes object with identity properties
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the structure of sample event data
# Look for the sample event structure
echo "Checking sample event structure..."
ast-grep --pattern 'export default {
$$$
identity: {
name: $_,
$$$
},
$$$
}'
Length of output: 173
Script:
#!/bin/bash
# Search for test event files and their content
echo "Looking for test event files..."
fd -e mjs -e js test-event | xargs cat
# Also search for sample emit implementation
echo -e "\nChecking sample emit implementation..."
rg "sampleEmit" -A 5
Length of output: 74825
Script:
#!/bin/bash
# Search for sample emit implementation and test events
echo "Looking for sample emit and test events..."
rg -l "sampleEmit"
echo -e "\nChecking test event content..."
fd -e mjs -e js "test-event|sample" | xargs cat
Length of output: 66869
Script:
#!/bin/bash
# Search for SailPoint identity sample emit implementation
echo "Looking for SailPoint identity sample emit..."
rg -A 10 "sampleEmit" components/sailpoint/sources/new-identity-instant/
echo -e "\nChecking for identity test event..."
cat components/sailpoint/sources/new-identity-instant/test-event.mjs
Length of output: 1819
components/sailpoint/sailpoint.app.mjs (2)
51-51
: Confirm the API base URL version v2024
.
The base URL is set to https://sailpoint.api.identitynow.com/v2024
. Ensure that v2024
is the correct API version for your integration, as using an incorrect version might lead to incompatibility issues.
Check the API documentation to confirm the correct base URL version.
110-113
: Verify the HTTP method used in deleteWebhook
function.
The deleteWebhook
method is using the POST
HTTP method to remove a webhook. Typically, deleting resources utilizes the DELETE
HTTP method. Please confirm whether the SailPoint IdentityNow API requires a POST
request for deletion or if it should be a DELETE
request.
Run the following script to verify the correct HTTP method for deleting a webhook:
✅ Verification successful
Based on the search results, I can see that most applications in the codebase use the DELETE
HTTP method for deleting resources, which is the standard RESTful approach. However, there are some applications that use POST
for deletion operations, including:
- SailPoint (the app in question)
- Tawk.to
- Klenty
- Sonarcloud
- Smartsuite
Let's verify if there's any SailPoint API documentation in the codebase:
Based on the search results, I can see that the SailPoint API is using an experimental feature for webhook management, as indicated by the X-SailPoint-Experimental
header. The API documentation links in the codebase point to developer.sailpoint.com
, so let's check if there are any comments or documentation about the webhook deletion endpoint:
The POST
method for webhook deletion appears to be correct due to experimental API status
The code inspection reveals that the webhook management in SailPoint's API is marked as experimental with the X-SailPoint-Experimental
header. While DELETE
is typically the standard HTTP method for resource deletion, it's not uncommon for APIs in experimental/beta stages to use different HTTP methods. The consistent use of the X-SailPoint-Experimental
header for both webhook creation and deletion endpoints suggests this is an intentional API design choice by SailPoint during their experimental phase.
Key observations:
- Both webhook creation and deletion endpoints use
POST
method - Both endpoints include the
X-SailPoint-Experimental: true
header - The API base URL and documentation links point to SailPoint's official developer portal
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the HTTP method required to delete a webhook in the SailPoint IdentityNow API.
# Test: Check the API documentation or use curl to test.
# Expected Result: Determine if 'DELETE' is the correct method.
# Using curl to test the DELETE method
curl -X DELETE "https://sailpoint.api.identitynow.com/v2024/trigger-subscriptions/{webhookId}" \
-H "Authorization: Bearer $OAUTH_ACCESS_TOKEN" \
-H "Accept: application/json"
# Note: Replace {webhookId} and $OAUTH_ACCESS_TOKEN with actual values.
Length of output: 455
Script:
#!/bin/bash
# Let's search for any API documentation or comments in the codebase that might indicate the correct HTTP method
rg -i "trigger-subscriptions|webhook.*delete" -g "!node_modules" -g "!dist" -A 3 -B 3
# Also search for any test files related to webhook deletion
fd -e test.js -e spec.js -e test.mjs -e spec.mjs -X rg -l "deleteWebhook"
# Check if there are any other similar DELETE operations in the codebase
rg "method: \"DELETE\"" -g "!node_modules" -g "!dist" -A 2 -B 2
Length of output: 65803
Script:
#!/bin/bash
# Search for SailPoint API documentation or comments
rg -i "sailpoint.*api|trigger-subscriptions" -g "!node_modules" -g "!dist" -A 3 -B 3
# Also check for any test files related to SailPoint webhook deletion
fd -e test.js -e spec.js -e test.mjs -e spec.mjs -X rg -l "sailpoint.*deleteWebhook"
Length of output: 6480
Script:
#!/bin/bash
# Search for any comments or documentation about webhook deletion in SailPoint
rg -i "delete.*webhook|webhook.*delete|trigger.?subscriptions.*delete" -g "!node_modules" -g "!dist" -A 3 -B 3 components/sailpoint/
# Also check for any test files or examples
rg -i "trigger.?subscriptions" -g "*.test.*" -g "*.spec.*" -g "example*" -g "*.md"
Length of output: 1149
components/sailpoint/common/constants.mjs (1)
1-20
: LGTM!
The constants are defined correctly and provide clear options for request types and webhook types.
components/sailpoint/sources/common/base.mjs (1)
34-39
: Verify db service usage pattern
The implementation of webhook ID storage looks correct. Let's verify this pattern is consistent across other components.
✅ Verification successful
Based on the search results, I can see that the $.service.db
pattern is consistently used across many components in the codebase for storing and managing state, including webhook IDs. The implementation in the review file follows this established pattern.
Pattern verified: db service usage is consistent
The implementation of webhook ID storage using this.db.get()
and this.db.set()
methods follows the standard pattern used throughout the codebase for state management. This is evidenced by hundreds of components using the same $.service.db
service in their props and similar get/set methods for state management.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar db service usage patterns in other components
rg -A 2 '\$\.service\.db' components/
Length of output: 65697
components/sailpoint/sources/new-identity-instant/new-identity-instant.mjs
Show resolved
Hide resolved
components/sailpoint/actions/list-certification-campaigns/list-certification-campaigns.mjs
Show resolved
Hide resolved
components/sailpoint/actions/upload-account-source-file/upload-account-source-file.mjs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi lgtm. Ready gor QA!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/sailpoint/actions/submit-access-request/submit-access-request.mjs (1)
47-48
: Move success message inside try blockThe success message should only be exported if the API call succeeds.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/sailpoint/actions/submit-access-request/submit-access-request.mjs
(1 hunks)
🔇 Additional comments (4)
components/sailpoint/actions/submit-access-request/submit-access-request.mjs (4)
1-9
: LGTM! Module definition and imports are well structured.
The imports use correct relative paths and the module is well-documented with appropriate metadata.
25-29
: Add JSON parsing validation for requestedItems
The requestedItems property accepts JSON strings but lacks validation, which could lead to runtime errors if invalid JSON is provided.
41-44
:
Fix critical typos in property names
There are mismatches between the defined props and the properties used in the request:
resquestItems
(typo) vsrequestedItems
(correct)this.resquestItems
(undefined) vsthis.requestedItems
(correct)
Apply this fix:
- resquestItems: this.resquestItems,
+ requestedItems: this.requestedItems,
Likely invalid or redundant comment.
37-46
: 🛠️ Refactor suggestion
Add error handling and input validation
The run method should include:
- Input validation before making the API call
- Error handling for the API response
Consider this implementation:
async run({ $ }) {
+ // Validate required inputs
+ if (!this.requestedItems?.length) {
+ throw new Error("At least one requested item is required");
+ }
+
+ try {
const response = await this.sailpoint.submitAccessRequest({
$,
data: {
requestedFor: this.requestedFor,
requestType: this.requestType,
requestedItems: this.requestedItems,
clientMetadata: this.clientMetadata,
},
});
+ $.export("$summary", "Access request submitted successfully.");
+ return response;
+ } catch (error) {
+ throw new Error(`Failed to submit access request: ${error.message}`);
+ }
- $.export("$summary", "Access request submitted successfully.");
- return response;
}
Likely invalid or redundant comment.
const response = await this.sailpoint.submitAccessRequest({ | ||
$, | ||
data: { | ||
requestedFor: this.requestedFor, | ||
requestType: this.requestType, | ||
resquestItems: this.resquestItems, | ||
clientMetadata: this.clientMetadata, | ||
}, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Fix typo in request payload property name
There's a typo in the request payload: resquestItems
should be requestedItems
. This doesn't match:
- The prop name defined in the component (
requestedItems
) - The documentation link provided in the component's description
Additionally, the property name in the payload doesn't match the actual prop name defined in the component's configuration, which could cause the request to fail.
🔗 Analysis chain
Verify API implementation against latest documentation
Please verify that the implementation matches the latest SailPoint API documentation, especially the request payload structure.
Let me search the codebase for more information about the API implementation.
Let me check the specific implementation of the submit-access-request component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the API documentation URL is valid and accessible
curl -I "https://developer.sailpoint.com/docs/api/v2024/create-access-request"
Length of output: 1032
Script:
#!/bin/bash
# Search for SailPoint API related files and documentation
fd sailpoint --type f --exec cat {}
# Also search for any test files that might contain example payloads
fd "sailpoint.*test|test.*sailpoint" --type f --exec cat {}
Length of output: 3605
Script:
#!/bin/bash
# Check the submit-access-request component implementation
cat components/sailpoint/actions/submit-access-request/submit-access-request.mjs
# Search for any related test files
fd "submit-access-request.*test" --type f --exec cat {}
Length of output: 2217
Resolves #13241.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
submitAccessRequest
action.Chores
.gitignore
file and deprecated application file.Constants
Utilities