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

ci: regenerate workflows with updated template #3143

Merged
merged 21 commits into from
Apr 19, 2023
Merged

Conversation

kweinmeister
Copy link
Collaborator

@kweinmeister kweinmeister commented Apr 15, 2023

Updated existing workflows to use common template from #3079 and #3112.

In the process, added multiple workflows for samples that haven't been tested. See .github/workflows/utils/workflows.json changes.

There are a couple blocking tests that were skipped. There are open issues to resolve these: #3121 and #3117; and new issues created: #3144 and #3153.

@product-auto-label product-auto-label bot added samples Issues that are directly related to samples. api: workflows Issues related to the Workflows API. labels Apr 15, 2023
@kweinmeister kweinmeister marked this pull request as ready for review April 16, 2023 15:57
@kweinmeister kweinmeister requested review from a team as code owners April 16, 2023 15:57
Copy link
Member

@rsamborski rsamborski left a comment

Choose a reason for hiding this comment

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

LGTM with a comment re: IAM.

One more clarification - do we need to regenerate workflows with every new folder added to this repository? If yes, then we should add this information in the contributors documentation and PR template to make sure people remember about it.

@@ -23,7 +23,7 @@ const iamClient = new PoliciesClient();

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

describe('IAM deny samples', () => {
describe.skip('IAM deny samples', () => {
Copy link
Member

Choose a reason for hiding this comment

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

I have updated service account permissions and this test should pass now. Please double check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! Retrying now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried again and still received a failure message:

PERMISSION_DENIED: Permission iam.googleapis.com/denypolicies.create denied on resource cloudresourcemanager.googleapis.com/projects/long-door-651

Will revert and we can handle this separately. It should work, thanks for trying here.

@kweinmeister
Copy link
Collaborator Author

LGTM with a comment re: IAM.

One more clarification - do we need to regenerate workflows with every new folder added to this repository? If yes, then we should add this information in the contributors documentation and PR template to make sure people remember about it.

No, it is not necessary to regenerate all workflows. Contributors just need to add an entry to .github/workflows/utils/workflows.json with the new sample directory, and then run the generate.js script with only the new directory they added (using the -s option).

@kweinmeister kweinmeister merged commit 4e15865 into main Apr 19, 2023
@kweinmeister kweinmeister deleted the update-workflows branch April 19, 2023 22:57
rsamborski added a commit that referenced this pull request Apr 20, 2023
[Per discussion](#3143 (comment)) we should make sure that users don't miss workflow generation when creating a new folder. Moreover a missing CODEOWNERS step was added here as well.
kweinmeister added a commit that referenced this pull request Apr 20, 2023
[Per discussion](#3143 (comment)) we should make sure that users don't miss workflow generation when creating a new folder. Moreover a missing CODEOWNERS step was added here as well.

Co-authored-by: Karl Weinmeister <11586922+kweinmeister@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: workflows Issues related to the Workflows API. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants