Skip to content

ci(iam): migrate to new CI #4007

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.

Sign up for GitHub

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

Closed
glasnt wants to merge 2 commits into from
Closed

ci(iam): migrate to new CI #4007

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e7ca74b
ci(iam): migrate to new CI
glasnt Mar 4, 2025
93a7acb
fix
glasnt Mar 4, 2025
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
1 change: 1 addition & 0 deletions .github/config/nodejs-dev.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@
"healthcare/datasets",
"healthcare/dicom",
"healthcare/hl7v2",
"iam/deny",
"kms",
"media/livestream",
"media/transcoder",
Expand Down
1 change: 0 additions & 1 deletion .github/config/nodejs-prod.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@
"functions/slack", // TypeError [ERR_INVALID_ARG_TYPE]: The "key" argument must be of type ... Received undefined
"functions/v2/imagemagick", // (untested) Error: A bucket name is needed to use Cloud Storage.
"healthcare/fhir", // Error: Cannot find module 'whatwg-url'
"iam/deny", // PERMISSION_DENIED: Permission iam.googleapis.com/denypolicies.create denied on resource cloudresourcemanager.googleapis.com/projects/long-door-651
"recaptcha_enterprise/snippets", // Cannot use import statement outside a module
"run/idp-sql", // (untested) Error: Invalid contents in the credentials file
"scheduler", // SyntaxError: Cannot use import statement outside a module
Expand Down
6 changes: 1 addition & 5 deletions iam/deny/test/deny.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,17 @@
const {assert} = require('chai');
const uuid = require('uuid');
const cp = require('child_process');
const {describe, it, before} = require('mocha');

Check failure on line 20 in iam/deny/test/deny.test.js

View workflow job for this annotation

GitHub Actions / lint 

'before' is assigned a value but never used

Check failure on line 20 in iam/deny/test/deny.test.js

View workflow job for this annotation

GitHub Actions / lint 

'before' is assigned a value but never used
const {PoliciesClient} = require('@google-cloud/iam').v2;
const iamClient = new PoliciesClient();

const execSync = cmd => cp.execSync(cmd, {encoding: 'utf-8'});

describe('IAM deny samples', () => {
let projectId;
const {projectId} = process.env;
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 4, 2025

Choose a reason for hiding this comment

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

medium

Consider adding a check to ensure that process.env.projectId is defined before using it. If it's not defined, the tests will fail in a confusing way. You can throw an error or use a default project ID for testing purposes. If you throw an error, make sure to fail fast, and provide a helpful message to the user.

Style Guide References

Suggested change
const {projectId} = process.env;
const {projectId = 'YOUR_DEFAULT_PROJECT_ID'} = process.env; // Provide a default value or throw an error if not set

const policyId = `gcloud-test-policy-${uuid.v4().split('-')[0]}`;
const policyName = `policies/cloudresourcemanager.googleapis.com%2Fprojects%2F949737848314/denypolicies/${policyId}`;
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 4, 2025

Choose a reason for hiding this comment

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

high

The policyName still contains a hardcoded project ID (949737848314). This needs to be updated to use the projectId variable to ensure the tests work correctly for different projects. Consider using template literals to construct the policy name with the project ID.

Style Guide References

Suggested change
const policyName = `policies/cloudresourcemanager.googleapis.com%2Fprojects%2F949737848314/denypolicies/${policyId}`;
const policyName = `policies/cloudresourcemanager.googleapis.com%2Fprojects%2F${projectId}/denypolicies/${policyId}`;


before(async () => {
projectId = await iamClient.getProjectId();
});

it('should create IAM policy', async () => {
const output = execSync(`node createDenyPolicy ${projectId} ${policyId}`);

Expand Down
Loading