Skip to content
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

feat(securitycenter): Add Resource SCC Management API Org SHA Custom … #3952

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

agggaurav2024
Copy link

…Modules

Description

Fixes # b/347347427, b/347347480, b/347347258, b/347347835
This PR adds SCC Managament API Org Security Health Analytics Custom Module Code Samples for Create, Update, Get and GetEffective.

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

  • I have followed guidelines from CONTRIBUTING.MD and Samples Style Guide
  • Tests pass: npm test (see Testing)
  • Lint pass: npm run lint (see Style)
  • These samples need a new API enabled in testing projects to pass (let us know which ones)
  • These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
  • This pull request is from a branch created directly off of GoogleCloudPlatform/nodejs-docs-samples. Not a fork.
  • This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
  • This sample adds a new sample directory, and I created GitHub Actions workflow for this sample
  • This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
  • Please merge this PR for me once it is approved

@agggaurav2024 agggaurav2024 requested review from a team as code owners January 16, 2025 08:34
Copy link

snippet-bot bot commented Jan 16, 2025

Here is the summary of changes.

You are about to add 4 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@product-auto-label product-auto-label bot added the api: securitycenter Issues related to the Security Command Center API. label Jan 16, 2025
Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi team,

I'm currently reviewing this pull request and will provide my feedback shortly. In the meantime, I'm providing this summary of the pull request's content for you and other reviewers to quickly get up to speed on the changes and intent.

This pull request, authored by agggaurav2024, aims to add code samples for the Security Health Analytics Custom Module in the Google Cloud Security Command Center (SCC) Management API. The changes include:

  • New files added:
    • security-center/snippets/management_api/createSecurityHealthAnalyticsCustomModule.js: Adds a function to create a new Security Health Analytics Custom Module. (Lines 1-99)
    • security-center/snippets/management_api/getEffectiveSecurityHealthAnalyticsCustomModule.js: Adds a function to retrieve an existing effective Security Health Analytics Custom Module. (Lines 1-52)
    • security-center/snippets/management_api/getSecurityHealthAnalyticsCustomModule.js: Adds a function to retrieve an existing Security Health Analytics Custom Module. (Lines 1-51)
    • security-center/snippets/management_api/updateSecurityHealthAnalyticsCustomModule.js: Adds a function to update an existing Security Health Analytics Custom Module. (Lines 1-69)
    • security-center/snippets/system-test/management_api/securityHealthAnalyticsCustomModule.test.js: Adds comprehensive system tests for the new functionality. (Lines 1-175)
  • security-center/snippets/package.json updated: Adds @google-cloud/securitycentermanagement as a dependency. (Line 18)

The pull request addresses issues b/347347427, b/347347480, b/347347258, and b/347347835. The author has indicated that the tests pass and that the code follows the style guidelines.

I'll be checking for completeness, correctness, and adherence to best practices in my full review. I'll also look at the test coverage and ensure the new samples are well-documented and easy to use.


A little poem to brighten your day:

Code flows like a river,
Testing's the current's strong pull,
Bugs swim to the sea.

Thanks!
Gemini

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Jan 16, 2025
Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request adds Security Health Analytics Custom Module code samples. The code generally looks good, but there are a few improvements that can be made.

// define the CEL expression here and this will scans for keys that have not been rotated in
// the last 30 days, change it according to the your requirements
const expr = {
expression: `has(resource.rotationPeriod) && (resource.rotationPeriod > duration('2592000s'))`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Use duration('30d') instead of '2592000s' for better readability.

Suggested change
expression: `has(resource.rotationPeriod) && (resource.rotationPeriod > duration('2592000s'))`,
expression: `has(resource.rotationPeriod) && (resource.rotationPeriod > duration('30d'))`,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agggaurav2024 what do you think? this seems like a good suggestion to me to make it match what's in the comment.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +71 to +72
description: 'add your description here',
recommendation: 'add your recommendation here',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Instead of placeholder comments, provide a clear explanation of what should be added to the description and recommendation fields. Give examples of good descriptions and recommendations.

Suggested change
description: 'add your description here',
recommendation: 'add your recommendation here',
description: 'CryptoKey has not been rotated in the last 30 days.',
recommendation: 'Rotate the CryptoKey to ensure key hygiene.'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agggaurav2024 this looks like another good suggestion although I'd recommend putting whatever the description and recommendation is for the sample itself.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try {
if (module.displayName === customModuleDisplayName) {
const customModuleId = module.name.split('/').pop();
// Proceed with deletion if module exist
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

The condition if (customModuleId) is always true since customModuleId is derived from splitting a string and popping the last element. It's redundant to check if it exists. Remove the conditional check.

          await client.deleteSecurityHealthAnalyticsCustomModule({
            name: `organizations/${organizationId}/locations/${locationId}/securityHealthAnalyticsCustomModules/${customModuleId}`,
          });

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed.
Refactored the cleanup code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: securitycenter Issues related to the Security Command Center API. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants