Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 29 additions & 15 deletions security-center/snippets/system-test/v1/notifications.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,37 +36,51 @@ describe('Client with Notifications', async () => {
before(async () => {
const client = new SecurityCenterClient();
async function createNotificationConfig(configId) {
/*eslint no-unused-vars: ["error", { "varsIgnorePattern": "^_" }]*/
const [_response] = await client.createNotificationConfig({
parent: orgName,
configId: configId,
notificationConfig: {
description: 'Sample config for node.js',
pubsubTopic: pubsubTopic,
streamingConfig: {filter: 'state = "ACTIVE"'},
},
});
try {
/*eslint no-unused-vars: ["error", { "varsIgnorePattern": "^_" }]*/
const [_response] = await client.createNotificationConfig({
parent: orgName,
configId: configId,
notificationConfig: {
description: 'Sample config for node.js',
pubsubTopic: pubsubTopic,
streamingConfig: {filter: 'state = "ACTIVE"'},
},
});
} catch (err) {
console.error(`Error creating config ${configId}:`, err.message);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Adding a try-catch block is a good practice for error handling. Consider adding more specific error handling within the try block to catch potential issues like incorrect input or network errors. This would make debugging easier and improve the robustness of the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

}

await createNotificationConfig(deleteConfig);
await new Promise(resolve => setTimeout(resolve, 1000)); // 1-second delay
await createNotificationConfig(getConfig);
await new Promise(resolve => setTimeout(resolve, 1000)); // 1-second delay
await createNotificationConfig(listConfig);
await new Promise(resolve => setTimeout(resolve, 1000)); // 1-second delay
await createNotificationConfig(updateConfig);
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Adding delays using setTimeout can make tests flaky and increase execution time. Consider using a more robust approach like polling or a dedicated testing library that provides utilities for waiting for asynchronous operations to complete.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

});

after(async () => {
const client = new SecurityCenterClient();
async function deleteNotificationConfig(configId) {
const name = client.organizationNotificationConfigPath(
organizationId,
configId
);
await client.deleteNotificationConfig({name: name});
try {
const name = client.organizationNotificationConfigPath(
organizationId,
configId
);
await client.deleteNotificationConfig({name: name});
} catch (err) {
console.error(`Error deleting config ${configId}:`, err.message);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Similar to the creation process, adding a try-catch block is good practice for error handling during deletion. Consider adding more specific error handling within the try block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

}

await deleteNotificationConfig(createConfig);
await new Promise(resolve => setTimeout(resolve, 1000)); // 1-second delay
await deleteNotificationConfig(getConfig);
await new Promise(resolve => setTimeout(resolve, 1000)); // 1-second delay
await deleteNotificationConfig(listConfig);
await new Promise(resolve => setTimeout(resolve, 1000)); // 1-second delay
await deleteNotificationConfig(updateConfig);
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Using setTimeout for delays can lead to flaky tests. Consider using a more robust approach like polling or a dedicated testing library for waiting for asynchronous operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// Copyright 2024 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

'use strict';

const {SecurityCenterClient} = require('@google-cloud/security-center');
const {assert} = require('chai');
const {describe, it, before} = require('mocha');
const {execSync} = require('child_process');

// TODO(developers): update for your own environment
const organizationId = '1081635000895';
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The organization ID is hardcoded. Consider storing this in an environment variable or a configuration file to make the test more portable and avoid exposing sensitive information directly in the code.

Suggested change
const organizationId = '1081635000895';
// Retrieve organization ID from environment variable
const organizationId = process.env.ORGANIZATION_ID || 'YOUR_ORGANIZATION_ID';
if (!organizationId) {
throw new Error('Environment variable ORGANIZATION_ID is not set.');
}


describe('client with security marks for assets', async () => {
let data;
before(async () => {
// Creates a new client.
const client = new SecurityCenterClient();

const [assetResults] = await client.listAssets({
parent: client.organizationPath(organizationId),
});
const randomAsset = assetResults[0].asset;
console.log('random %j', randomAsset);
data = {
orgId: organizationId,
assetName: randomAsset.name,
};
console.log('data %j', data);
});
it('client can add security marks to asset.', () => {
const output = execSync(
`node v2/addSecurityMarks.js ${data.assetName}`
).toString();
assert.include(output, data.assetName);
assert.match(output, /key_a/);
assert.match(output, /value_a/);
assert.match(output, /key_b/);
assert.match(output, /value_b/);
assert.notMatch(output, /undefined/);
});

it('client can add and delete security marks', () => {
// Ensure marks are set.
execSync(`node v2/addSecurityMarks.js ${data.assetName}`).toString();

const output = execSync(
`node v2/addDeleteSecurityMarks.js ${data.assetName}`
).toString();
assert.match(output, /key_a/);
assert.match(output, /new_value_a/);
assert.notMatch(output, /key_b/);
assert.notMatch(output, /undefined/);
});

it('client can delete security marks', () => {
// Ensure marks are set.
execSync(`node v2/addSecurityMarks.js ${data.assetName}`).toString();

const output = execSync(
`node v2/deleteAssetsSecurityMarks.js ${data.assetName}`
).toString();
assert.notMatch(output, /key_a/);
assert.notMatch(output, /value_a/);
assert.notMatch(output, /key_b/);
assert.notMatch(output, /value_b/);
assert.include(output, data.assetName);
assert.include(output, data.assetName);
assert.notMatch(output, /undefined/);
});
});
49 changes: 49 additions & 0 deletions security-center/snippets/v2/addDeleteSecurityMarks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2024 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
'use strict';

/**
* Demonstrates adding/updating at the same time as deleting security
* marks from an asset.
*/
function main(assetName = 'full asset path to add marks to') {
// [START securitycenter_add_delete_security_marks_v2]
// Imports the Google Cloud client library.
const {SecurityCenterClient} = require('@google-cloud/security-center').v2;

// Creates a new client.
const client = new SecurityCenterClient();

async function addDeleteSecurityMarks() {
// assetName is the full resource path for the asset to update.
// Specify the value of 'assetName' in one of the following formats:
// `organizations/${org-id}/assets/${asset-id}`;
// `projects/${project-id}/assets/${asset-id}`;
// `folders/${folder-id}/assets/${asset-id}`;
const [newMarks] = await client.updateSecurityMarks({
securityMarks: {
name: `${assetName}/securityMarks`,
marks: {key_a: 'new_value_a'},
},
// Only update the enableAssetDiscovery field.
updateMask: {paths: ['marks.key_a', 'marks.key_b']},
});

console.log('New marks: %j', newMarks);
}
addDeleteSecurityMarks();
// [END securitycenter_add_delete_security_marks_v2]
}

main(...process.argv.slice(2));
53 changes: 53 additions & 0 deletions security-center/snippets/v2/addSecurityMarks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright 2024 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

'use strict';

/**
* Demostrates adding security marks to an asset.
*/
function main(assetName = 'full asset path to add marks to') {
// [START securitycenter_add_security_marks_v2]
// Imports the Google Cloud client library.
const {SecurityCenterClient} = require('@google-cloud/security-center').v2;

// Creates a new client.
const client = new SecurityCenterClient();

async function addSecurityMarks() {
// assetName is the full resource path for the asset to update.
/*
* TODO(developer): Uncomment the following lines
*/
// Specify the value of 'assetName' in one of the following formats:
// `organizations/${org-id}/assets/${asset-id}`;
// `projects/${project-id}/assets/${asset-id}`;
// `folders/${folder-id}/assets/${asset-id}`;
// const assetName = "organizations/123123342/assets/12312321";
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 TODO comments suggest uncommenting lines to specify the asset name. It's better to provide clear instructions or examples on how to obtain and use the asset name rather than relying on commented-out code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

const [newMarks] = await client.updateSecurityMarks({
securityMarks: {
name: `${assetName}/securityMarks`,
marks: {key_a: 'value_a', key_b: 'value_b'},
},
// Only update the marks with these keys.
updateMask: {paths: ['marks.key_a', 'marks.key_b']},
});

console.log('New marks: %', newMarks);
}
addSecurityMarks();
// [END securitycenter_add_security_marks_v2]
}

main(...process.argv.slice(2));
53 changes: 53 additions & 0 deletions security-center/snippets/v2/deleteAssetsSecurityMarks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright 2024 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

'use strict';

/**
* Demostrates deleting security marks on an asset.
*/
function main(assetName = 'full asset path to add marks to') {
// [START securitycenter_delete_security_marks_v2]
// Imports the Google Cloud client library.
const {SecurityCenterClient} = require('@google-cloud/security-center').v2;

// Creates a new client.
const client = new SecurityCenterClient();

async function deleteSecurityMarks() {
// assetName is the full resource path for the asset to update.
/*
* TODO(developer): Uncomment the following lines
*/
// Specify the value of 'assetName' in one of the following formats:
// `organizations/${org-id}/assets/${asset-id}`;
// `projects/${project-id}/assets/${asset-id}`;
// `folders/${folder-id}/assets/${asset-id}`;
// const assetName = "organizations/123123342/assets/12312321";
const [newMarks] = await client.updateSecurityMarks({
securityMarks: {
name: `${assetName}/securityMarks`,
// Intentionally, not setting marks to delete them.
},
// Only delete marks for the following keys.
updateMask: {paths: ['marks.key_a', 'marks.key_b']},
});

console.log('Updated marks: %j', newMarks);
}
deleteSecurityMarks();
// [END securitycenter_delete_security_marks_v2]
}

main(...process.argv.slice(2));