Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion .github/workflows/utils/workflows.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,5 +94,6 @@
"video-intelligence",
"vision/productSearch",
"workflows",
"workflows/invoke-private-endpoint"
"workflows/invoke-private-endpoint",
"security-center/snippets/v2"
Copy link
Contributor

Choose a reason for hiding this comment

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

low

This addition correctly includes the new system tests in the workflow. Consider adding a comment explaining the purpose of this entry for clarity.

Suggested change
"security-center/snippets/v2"
"security-center/snippets/v2" # Add v2 BigQuery export system tests

Copy link
Contributor

Choose a reason for hiding this comment

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

low

Consider adding a descriptive comment above this line indicating the purpose of this addition, e.g., "Add security center v2 snippets for BigQuery export". This will improve the maintainability of the workflows file.

]
167 changes: 167 additions & 0 deletions security-center/snippets/system-test/v2/bigqueryexport.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
/*
* 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.
*/

const {SecurityCenterClient} = require('@google-cloud/security-center').v2;
const {assert} = require('chai');
const {execSync} = require('child_process');
const exec = cmd => execSync(cmd, {encoding: 'utf8'});
const {describe, it, before} = require('mocha');
const {BigQuery} = require('@google-cloud/bigquery');

const organizationId = process.env.GCLOUD_ORGANIZATION;
const projectId = process.env.GOOGLE_SAMPLES_PROJECT;
const location = 'global';
const bigquery = new BigQuery();

async function cleanupDatasets() {
const [datasets] = await bigquery.getDatasets();
for (const dataset of datasets) {
if (dataset.id.startsWith('securitycenter_')) {
console.log(`Deleting dataset: ${dataset.id}`);
await bigquery.dataset(dataset.id).delete({force: true});
}
}
Comment on lines +29 to +36
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 cleanupDatasets function could be more efficient by using a filter when listing datasets. This would avoid iterating through all datasets and improve performance, especially if there are many datasets in the project.

Suggested change
async function cleanupDatasets() {
const [datasets] = await bigquery.getDatasets();
for (const dataset of datasets) {
if (dataset.id.startsWith('securitycenter_')) {
console.log(`Deleting dataset: ${dataset.id}`);
await bigquery.dataset(dataset.id).delete({force: true});
}
}
async function cleanupDatasets() {
const options = {
filter: 'id:securitycenter_*',
};
const [datasets] = await bigquery.getDatasets(options);
for (const dataset of datasets) {
console.log(`Deleting dataset: ${dataset.id}`);
await bigquery.dataset(dataset.id).delete({force: true});
}
}

}

async function cleanupBigQueryExports(client) {
const [exports] = await client.listBigQueryExports({
parent: client.organizationLocationPath(organizationId, location),
});
for (const exportData of exports) {
console.log(`Deleting BigQuery export: ${exportData.name}`);
await client.deleteBigQueryExport({name: exportData.name});
}
}

let dataset;

async function createDataset() {
const randomSuffix = Math.floor(Date.now() / 1000);
const datasetId = `securitycenter_dataset_${randomSuffix}`;
const options = {
location: 'US',
};

try {
const [createdDataset] = await bigquery.createDataset(datasetId, options);
console.log(`Dataset ${createdDataset.id} created.`);
return createdDataset.id;
} catch (error) {
if (error.code === 409) {
// Dataset already exists - Fail the test instead of moving on
console.log(
`Dataset ${datasetId} already exists. Exiting to avoid conflict.`
);
throw new Error(`Dataset ${datasetId} already exists.`);
}
throw error;
}
Comment on lines +63 to +71
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 error handling here could be improved. Instead of failing the test if the dataset already exists, consider implementing a retry mechanism with exponential backoff. This will make the tests more robust and less likely to fail due to transient errors.

Suggested change
if (error.code === 409) {
// Dataset already exists - Fail the test instead of moving on
console.log(
`Dataset ${datasetId} already exists. Exiting to avoid conflict.`
);
throw new Error(`Dataset ${datasetId} already exists.`);
}
throw error;
}
if (error.code === 409) {
// Dataset already exists - retry with a new random suffix after a delay
console.log(`Dataset ${datasetId} already exists. Retrying...`);
await new Promise(resolve => setTimeout(resolve, 2000)); // Wait 2 seconds
return createDataset();
}
throw error;

}
Comment on lines +51 to +72
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 error handling in createDataset could be improved. Instead of throwing an error if the dataset already exists, consider returning the existing dataset ID. This would avoid unnecessary test failures and allow the tests to continue running.

Suggested change
async function createDataset() {
const randomSuffix = Math.floor(Date.now() / 1000);
const datasetId = `securitycenter_dataset_${randomSuffix}`;
const options = {
location: 'US',
};
try {
const [createdDataset] = await bigquery.createDataset(datasetId, options);
console.log(`Dataset ${createdDataset.id} created.`);
return createdDataset.id;
} catch (error) {
if (error.code === 409) {
// Dataset already exists - Fail the test instead of moving on
console.log(
`Dataset ${datasetId} already exists. Exiting to avoid conflict.`
);
throw new Error(`Dataset ${datasetId} already exists.`);
}
throw error;
}
}
async function createDataset() {
const randomSuffix = Math.floor(Date.now() / 1000);
const datasetId = `securitycenter_dataset_${randomSuffix}`;
const options = {
location: 'US',
};
try {
const [createdDataset] = await bigquery.createDataset(datasetId, options);
console.log(`Dataset ${createdDataset.id} created.`);
return createdDataset.id;
} catch (error) {
if (error.code === 409) {
// Dataset already exists - return the existing dataset ID
console.log(`Dataset ${datasetId} already exists. Using existing dataset.`);
const [datasets] = await bigquery.getDatasets({ filter: `id:${datasetId}` });
return datasets[0].id;
}
throw error;
}
}


describe('Client with bigquery export V2', async () => {
let data;
before(async () => {
// Creates a new client.
const client = new SecurityCenterClient();

// Clean up any existing datasets or BigQuery exports
await cleanupDatasets();
Copy link
Contributor

Choose a reason for hiding this comment

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

low

Add a comment to explain what cleanupDatasets does.

Suggested change
await cleanupDatasets();
// Clean up any existing datasets or BigQuery exports
await cleanupDatasets(); // Delete datasets starting with 'securitycenter_'

await cleanupBigQueryExports(client);

// Create a new dataset
const createdDataset = await createDataset();
dataset = `projects/${projectId}/datasets/${createdDataset}`;

// Build the create bigquery export request.
const bigQueryExportId =
'bigqueryexportid-' + Math.floor(Math.random() * 10000);
Comment on lines +89 to +90
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 bigQueryExportId could be generated using a more robust method, such as a UUID library, to ensure uniqueness and avoid potential collisions.

Suggested change
const bigQueryExportId =
'bigqueryexportid-' + Math.floor(Math.random() * 10000);
const { v4: uuidv4 } = require('uuid');
const bigQueryExportId = uuidv4();

const filter = 'severity="LOW" OR severity="MEDIUM"';
const bigQueryExport = {
name: 'bigQueryExport node',
description:
'Export low and medium findings if the compute resource has an IAM anomalous grant',
filter: filter,
dataset: dataset,
};
const createBigQueryExportRequest = {
parent: client.organizationLocationPath(organizationId, location),
bigQueryExport,
bigQueryExportId,
};

try {
const response = await client.createBigQueryExport(
createBigQueryExportRequest
);
const bigQueryExportResponse = response[0];
data = {
orgId: organizationId,
bigQueryExportId: bigQueryExportId,
bigQueryExportName: bigQueryExportResponse.name,
untouchedbigQueryExportName: '',
};
console.log('Created BigQuery export %j', data);
} catch (error) {
console.error('Error creating BigQuery export:', error);
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

It's good practice to clean up created resources after the tests are completed. Consider adding an after hook to delete the BigQuery export and dataset.

Suggested change
});
});
after(async () => {
const client = new SecurityCenterClient();
try {
await client.deleteBigQueryExport({name: data.bigQueryExportName});
await bigquery.dataset(dataset.split('/').pop()).delete({force: true});
} catch (error) {
console.error("Error cleaning up resources:", error);
}
});


it('client can create bigquery export V2', done => {
const output = exec(
`node v2/createBigQueryExport.js ${data.orgId} ${dataset}`
);
assert(output.includes(data.orgId));
assert.match(output, /BigQuery export request created successfully/);
assert.notMatch(output, /undefined/);
done();
});

it('client can list all bigquery export V2', done => {
const output = exec(`node v2/listAllBigQueryExports.js ${data.orgId}`);
assert(output.includes(data.bigQueryExportName));
assert.match(output, /Sources/);
assert.notMatch(output, /undefined/);
done();
});

it('client can get a bigquery export V2', done => {
const output = exec(
`node v2/getBigQueryExport.js ${data.orgId} ${data.bigQueryExportId}`
);
assert(output.includes(data.bigQueryExportName));
assert.match(output, /Retrieved the BigQuery export/);
assert.notMatch(output, /undefined/);
done();
});

it('client can update a bigquery export V2', done => {
const output = exec(
`node v2/updateBigQueryExport.js ${data.orgId} ${data.bigQueryExportId} ${dataset}`
);
assert.match(output, /BigQueryExport updated successfully/);
assert.notMatch(output, /undefined/);
done();
});

it('client can delete a bigquery export V2', done => {
const output = exec(
`node v2/deleteBigQueryExport.js ${data.orgId} ${data.bigQueryExportId}`
);
assert.match(output, /BigQuery export request deleted successfully/);
assert.notMatch(output, /undefined/);
done();
});
});
23 changes: 8 additions & 15 deletions security-center/snippets/v2/createBigQueryExport.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,12 @@ function main(organizationId, dataset, location = 'global') {
// Create a Security Center client
const client = new SecurityCenterClient();

/**
* Required. The name of the parent resource of the new BigQuery export. Its
* format is "organizations/[organization_id]/locations/[location_id]",
* "folders/[folder_id]/locations/[location_id]", or
* "projects/[project_id]/locations/[location_id]".
*/
// format is "organizations/[organization_id]/locations/[location_id]",
// "folders/[folder_id]/locations/[location_id]", or
// "projects/[project_id]/locations/[location_id]".
Comment on lines +29 to +31
Copy link
Contributor

Choose a reason for hiding this comment

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

unknown

While the comments are helpful, they could be more concise. Consider removing the redundant information about the format of the parent resource, as it's already implied by the function call.

Suggested change
// format is "organizations/[organization_id]/locations/[location_id]",
// "folders/[folder_id]/locations/[location_id]", or
// "projects/[project_id]/locations/[location_id]".
// The parent resource of the new BigQuery export.

Comment on lines +29 to +31
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 comment here could be more concise and descriptive. For example:

// The parent resource for the new BigQuery export.
// Its format can be:
// - organizations/{organization}/locations/{location}
// - folders/{folder}/locations/{location}
// - projects/{project}/locations/{location}
Suggested change
// format is "organizations/[organization_id]/locations/[location_id]",
// "folders/[folder_id]/locations/[location_id]", or
// "projects/[project_id]/locations/[location_id]".
// The parent resource for the new BigQuery export.
// Its format can be:
// - organizations/{organization}/locations/{location}
// - folders/{folder}/locations/{location}
// - projects/{project}/locations/{location}

const parent = client.organizationLocationPath(organizationId, location);

/**
* Required. The BigQuery export being created.
*/
// Required. The BigQuery export being created.
// filter: Expression that defines the filter to apply across create/update events of findings.
const filter = 'severity="LOW" OR severity="MEDIUM"';

Expand All @@ -48,12 +43,10 @@ function main(organizationId, dataset, location = 'global') {
dataset,
};

/**
* Required. Unique identifier provided by the client within the parent scope.
* It must consist of only lowercase letters, numbers, and hyphens, must start
* with a letter, must end with either a letter or a number, and must be 63
* characters or less.
*/
// Required. Unique identifier provided by the client within the parent scope.
// It must consist of only lowercase letters, numbers, and hyphens, must start
// with a letter, must end with either a letter or a number, and must be 63
// characters or less.
Comment on lines +46 to +49
Copy link
Contributor

Choose a reason for hiding this comment

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

unknown

The comment explaining the requirements for the BigQuery export ID is quite verbose. Consider shortening it to be more concise.

Suggested change
// Required. Unique identifier provided by the client within the parent scope.
// It must consist of only lowercase letters, numbers, and hyphens, must start
// with a letter, must end with either a letter or a number, and must be 63
// characters or less.
// Unique identifier for the BigQuery export.

Comment on lines +46 to +49
Copy link
Contributor

Choose a reason for hiding this comment

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

low

Instead of generating a random ID, consider using a UUID library to ensure uniqueness and avoid potential collisions.

Suggested change
// Required. Unique identifier provided by the client within the parent scope.
// It must consist of only lowercase letters, numbers, and hyphens, must start
// with a letter, must end with either a letter or a number, and must be 63
// characters or less.
const { v4: uuidv4 } = require('uuid');
const bigQueryExportId = uuidv4();

const bigQueryExportId =
'bigqueryexportid-' + Math.floor(Math.random() * 10000);

Expand Down
4 changes: 1 addition & 3 deletions security-center/snippets/v2/deleteBigQueryExport.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ function main(organizationId, exportId, location = 'global') {
const client = new SecurityCenterClient();

// Build the full resource path for the BigQuery export to delete.
/*
* TODO(developer): Update the following references for your own environment before running the sample.
*/
// TODO(developer): Update the following references for your own environment before running the sample.
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 comment here seems to be leftover from a template. Since the organizationId and exportId are now being passed as arguments to the function, this TODO is no longer relevant and can be removed.

Suggested change
// TODO(developer): Update the following references for your own environment before running the sample.
// Build the full resource path for the BigQuery export to delete.

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 comment should be removed after the references have been updated. It's best to avoid leaving TODOs in production code.

Suggested change
// TODO(developer): Update the following references for your own environment before running the sample.
// Build the full resource path for the BigQuery export to delete.

// const organizationId = 'YOUR_ORGANIZATION_ID';
// const exportId = 'EXPORT_ID';
const name = `organizations/${organizationId}/locations/${location}/bigQueryExports/${exportId}`;
Expand Down
4 changes: 1 addition & 3 deletions security-center/snippets/v2/getBigQueryExport.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ function main(organizationId, exportId, location = 'global') {
const client = new SecurityCenterClient();

// Build the full resource path for the BigQuery export to retrieve.
/*
* TODO(developer): Update the following references for your own environment before running the sample.
*/
// TODO(developer): Update the following references for your own environment before running the sample.
Copy link
Contributor

Choose a reason for hiding this comment

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

low

Similar to the deleteBigQueryExport.js file, this TODO comment is no longer necessary and can be removed.

Suggested change
// TODO(developer): Update the following references for your own environment before running the sample.
// Build the full resource path for the BigQuery export to retrieve.

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 comment should be removed after updating the references. It's good practice to avoid leaving TODOs in production code.

Suggested change
// TODO(developer): Update the following references for your own environment before running the sample.
// Build the full resource path for the BigQuery export to retrieve.

// const organizationId = 'YOUR_ORGANIZATION_ID';
// const exportId = 'EXPORT_ID';
// const location = 'LOCATION_ID';
Expand Down
27 changes: 17 additions & 10 deletions security-center/snippets/v2/listAllBigQueryExports.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,27 @@ function main(organizationId, location = 'global') {

// Creates a new client.
const client = new SecurityCenterClient();
/**
* Required. The parent, which owns the collection of BigQuery exports. Its
* format is "organizations/[organization_id]/locations/[location_id]",
* "folders/[folder_id]/locations/[location_id]", or
* "projects/[project_id]/locations/[location_id]".
*/

/**
* TODO(developer): Update the following references for your own environment before running the sample.
*/

// TODO(developer): Update the following references for your own environment before running the sample.
// const organizationId = 'YOUR_ORGANIZATION_ID';
Comment on lines +29 to 30
Copy link
Contributor

Choose a reason for hiding this comment

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

low

This TODO comment should be removed after the references have been updated. It's best to avoid leaving TODOs in production code.

Suggested change
// TODO(developer): Update the following references for your own environment before running the sample.
// const organizationId = 'YOUR_ORGANIZATION_ID';
// const organizationId = 'YOUR_ORGANIZATION_ID';
// const location = 'LOCATION_ID';

// const location = 'LOCATION_ID';
Comment on lines +29 to 31
Copy link
Contributor

Choose a reason for hiding this comment

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

low

This TODO comment is also redundant and can be removed.

Suggested change
// TODO(developer): Update the following references for your own environment before running the sample.
// const organizationId = 'YOUR_ORGANIZATION_ID';
// const location = 'LOCATION_ID';
// The parent resource for listing BigQuery exports.

const parent = client.organizationLocationPath(organizationId, location);

//
// The maximum number of configs to return. The service may return fewer than
// this value.
// If unspecified, at most 10 configs will be returned.
// The maximum value is 1000; values above 1000 will be coerced to 1000.
//
// const pageSize = 1234
//
// A page token, received from a previous `ListBigQueryExports` call.
// Provide this to retrieve the subsequent page.
// When paginating, all other parameters provided to `ListBigQueryExports`
// must match the call that provided the page token.
//
// const pageToken = 'abc123'

// Build the request.
const listBigQueryExportsRequest = {
parent,
Expand Down
23 changes: 5 additions & 18 deletions security-center/snippets/v2/updateBigQueryExport.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,15 @@ function main(organizationId, exportId, dataset, location = 'global') {

// Creates a new client.
const client = new SecurityCenterClient();
/**
* Required. Name of the BigQuery export to retrieve. The following list shows
* some examples of the format:
* +
* `organizations/{organization}/locations/{location}/bigQueryExports/{export_id}`
* + `folders/{folder}/locations/{location}/bigQueryExports/{export_id}`
* + `projects/{project}locations/{location}/bigQueryExports/{export_id}`
*/

/**
* TODO(developer): Update the following references for your own environment before running the sample.
*/
// Required. Name of the BigQuery export to retrieve.
// TODO(developer): Update the following references for your own environment before running the sample.
Copy link
Contributor

Choose a reason for hiding this comment

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

low

This TODO comment is redundant and can be removed.

Suggested change
// TODO(developer): Update the following references for your own environment before running the sample.
// The name of the BigQuery export to update.

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 comment should be removed after updating the references. It's best to avoid leaving TODOs in production code.

Suggested change
// TODO(developer): Update the following references for your own environment before running the sample.
// Required. Name of the BigQuery export to retrieve.

// const organizationId = 'YOUR_ORGANIZATION_ID';
// const location = 'LOCATION_ID';
// const exportId = 'EXPORT_ID';
const name = `organizations/${organizationId}/locations/${location}/bigQueryExports/${exportId}`;

/**
* Required. The BigQuery export being updated.
*/
// Required. The BigQuery export being updated.
const filter =
'severity="LOW" OR severity="MEDIUM" AND category="Persistence: IAM Anomalous Grant" AND -resource.type:"compute"';

Expand All @@ -55,10 +44,8 @@ function main(organizationId, exportId, dataset, location = 'global') {
filter: filter,
};

/**
* The list of fields to be updated.
* If empty all mutable fields will be updated.
*/
// The list of fields to be updated.
// If empty all mutable fields will be updated.
const fieldMask = {
paths: ['description', 'filter'],
};
Expand Down