Add Roles to GCP IAM policy lookup - EXPANDR 5945 #30477

BigEasyJ · 2023-10-26T17:25:15Z

Contributing to Cortex XSOAR Content

Make sure to register your contribution by filling the contribution registration form

The Pull Request will be reviewed only after the contribution registration form is filled.

Status

In Progress
Ready
In Hold - (Reason for hold)

Related Issues

EXPANDR-5945

Description

Updated the gcp_iam_project_iam_policy_get_command command to support filtering by roles.

Must have

Tests
Documentation

- Add roles to gcp_iam_project_iam_policy_get_command

content-bot · 2023-10-26T17:28:16Z

Thank you for your contribution. Your generosity and caring are unrivaled! Make sure to register your contribution by filling the Contribution Registration form, so our content wizard @sapirshuker will know the proposed changes are ready to be reviewed.
For your convenience, here is a link to the contributions SLAs document.

content-bot · 2023-10-26T17:28:17Z

Hi @BigEasyJ, thanks for contributing to a Cortex XSOAR supported pack. To receive credit for your generous contribution please follow this link.

ShirleyDenkberg · 2023-10-30T04:42:11Z

Packs/GCP-IAM/Integrations/GCPIAM/GCPIAM.yml

@@ -87,6 +87,8 @@ script:
    - defaultValue: '1'
      description: The page number of the results to retrieve. Minimum value is 1.
      name: page
+    - description: 'A list of roles. Must be given as a comma separated list (Ex: "roles/bigquery.admin, roles/editor, roles/owner").'


Suggested change

- description: 'A list of roles. Must be given as a comma separated list (Ex: "roles/bigquery.admin, roles/editor, roles/owner").'

- description: 'A comma-separated list of roles. (Ex: "roles/bigquery.admin, roles/editor, roles/owner").'

ShirleyDenkberg · 2023-10-30T04:42:36Z

Packs/GCP-IAM/Integrations/GCPIAM/README.md

@@ -385,7 +385,8 @@ Retrieves the IAM access control policy for the specified folder.
 | --- | --- | --- |
 | folder_name | The folder name for which the policy is being requested. For example, folders/12342. | Required | 
 | limit | The maximum number of results to retrieve. Minimum value is 1. Default is 50. | Optional | 
-| page | The page number of the results to retrieve. Minimum value is 1. Default is 1. | Optional | 
+| page | The page number of the results to retrieve. Minimum value is 1. Default is 1. | Optional |
+| roles | A comma separated list of roles. Must be given as a comma separated list (Ex: "roles/bigquery.admin, roles/editor, roles/owner"). | Optional | 


ShirleyDenkberg · 2023-10-30T04:45:24Z

@sapirshuker Doc review completed.

sapirshuker

Hey, @BigEasyJ
Thank you for your contribution!
Good work :)
Please see my comments.
In addition, Please update the docker image, you can see the error here

sapirshuker · 2023-10-29T16:20:46Z

Packs/GCP-IAM/Integrations/GCPIAM/GCPIAM.yml

+    - description: 'A list of roles. Must be given as a comma separated list (Ex: "roles/bigquery.admin, roles/editor, roles/owner").'
+      name: roles


Suggested change

- description: 'A list of roles. Must be given as a comma separated list (Ex: "roles/bigquery.admin, roles/editor, roles/owner").'

name: roles

- description: 'A list of roles. Must be given as a comma separated list (Ex: "roles/bigquery.admin, roles/editor, roles/owner").'

name: roles

isArray: true

required: false

sapirshuker · 2023-10-29T16:28:33Z

Packs/GCP-IAM/Integrations/GCPIAM/GCPIAM.py

+    if roles and bindings:
+        bindings_roles_only = []
+        for index, entry in enumerate(bindings):
+            if entry["role"] in roles:


Please use entry.get("role")

sapirshuker · 2023-10-30T09:14:38Z

Packs/GCP-IAM/Integrations/GCPIAM/GCPIAM.py

        outputs["bindings"] = bindings[start:end]
+        if len(bindings) < 50:


What is this change? The limit default value is 50 but could be larger, why do we need to change the header?
Additionally, you check the len of the original list without paging (bindings[start:end]), is that your intention?

The limit default value is 50 but could be larger, why do we need to change the header?
I have updated it to use limit. The default header would come from get_pagination_readable_message which would use limit and that would be an incorrect output when filtering by a list of roles. If this was not changed whe using !gcp-iam-project-iam-policy-get project_name="projects/target_project" roles="roles/editor,roles/owner" the example would look like:

Current page size: 50 Showing page 1 out of others that may exist.

Since there are only 2 roles ( roles/editor, roles/owner), it should be:

Current size: 2 ``` > Additionally, you check the len of the original list without paging (bindings[start:end]), is that your intention? Yes, because I would like to return all possible roles, but I could page the roles if we think a user would input over 50 roles into a list.

sapirshuker · 2023-10-30T09:25:48Z

Packs/GCP-IAM/ReleaseNotes/1_0_21.md

+
+##### GCP-IAM
+
+Updated the ***gcp_iam_project_iam_policy_get_command*** command, to support filtering by roles.


Suggested change

Updated the ***gcp_iam_project_iam_policy_get_command*** command, to support filtering by roles.

Updated the ***gcp-iam-project-iam-policy-get*** command, to support filtering by roles.

sapirshuker · 2023-10-30T09:42:40Z

Packs/GCP-IAM/Integrations/GCPIAM/GCPIAM.py

+        for index, entry in enumerate(bindings):
+            if entry["role"] in roles:
+                bindings_roles_only.append(bindings.pop(index))


You're changing the bindings list while going through it, I don't think it's a good idea to do it like that. It could cause bugs.

Updated to retrieve index value only.

sapirshuker · 2023-10-30T09:58:24Z

Packs/GCP-IAM/Integrations/GCPIAM/GCPIAM.py

@@ -1221,7 +1221,7 @@ def update_time_format(data: Union[dict, list], keys: list) -> list:

 def generate_iam_policy_command_output(response: dict, resource_name: str = None,


Please add the roles arg to the doc string

sapirshuker · 2023-10-30T12:06:15Z

Packs/GCP-IAM/Integrations/GCPIAM/GCPIAM.py

+            if entry["role"] in roles:
+                bindings_roles_only.append(bindings.pop(index))
+
+        bindings = bindings_roles_only


If you don't have any results after filtering, what happens?

- Update role input description - Pop to index - remove explicit conditional

…ANDR-5945-GCP-IAM

sapirshuker

Hey, thanks for making the changes!
Awesome! Let's add some unit tests for this functionality.
Also, I notice that the generate_iam_policy_command_output function is being called from 7 different places. Will changing the readable header have an impact on every call where len(bindings) < limit is true? Just wondering if this is what you intended, because I think it might affect other commands too.

…ANDR-5945-GCP-IAM

BigEasyJ · 2023-11-01T23:30:45Z

I notice that the generate_iam_policy_command_output function is being called from 7 different places. Will changing the readable header have an impact on every call where len(bindings) < limit is true? Just wondering if this is what you intended, because I think it might affect other commands too.

There are only 3 places that use limit and page.

gcp_iam_project_iam_policy_get_command
gcp_iam_folder_iam_policy_get_command
gcp_iam_organization_iam_policy_get_command

I have updated the readable header to work for all of them if they do not have more than the limit (which would mean it has less than 1 page). I would have liked to not affect those commands, but this change does work for any paging output with minimal changes to trying to avoid affecting the other commands.

Also, @sapirshuker do I need to make changes to the Flake8 errors that were previously existing?

sapirshuker · 2023-11-06T10:18:07Z

Hi @BigEasyJ , we haven’t heard from you in a while.
I am waiting to the addition of unit-tests.
Please feel free to reach out to me here or on Slack.
Thanks again for contributing to our repo, hope to hear from you soon

BigEasyJ · 2023-11-06T17:48:58Z

Sorry, I was expecting a meeting fro the demo, also @sapirshuker I'm unsure what additional unit tests are needed as I have updated the unit test primarily in question.

…ANDR-5945-GCP-IAM

sapirshuker · 2023-11-09T15:27:32Z

Hey, @BigEasyJ
I sent you a Slack message about the options to fix the demo issue.

sapirshuker · 2023-11-13T07:58:45Z

Hey @BigEasyJ
we haven’t heard from you in a while.
Do you need any help with to implement the changes from the demo and fix the validation errors?
Please feel free to reach out to me here or on Slack.
Thank you

BigEasyJ · 2023-11-13T08:01:03Z

Hi @sapirshuker I was out on Friday and not in on Sundays, I will get back to you as soon as I can.

…ANDR-5945-GCP-IAM

content-bot · 2023-11-14T16:17:13Z

For the Reviewer: Successfully created a pipeline in Gitlab with url: https://code.pan.run/xsoar/content/-/pipelines/6908138

…ANDR-5945-GCP-IAM

sapirshuker · 2023-11-16T15:51:56Z

Build failed due to a known issue CIAC-6419

sapirshuker

Hey please see my comments

sapirshuker · 2023-11-16T16:19:05Z

Packs/GCP-IAM/Integrations/GCPIAM/GCPIAM.py

@@ -2959,8 +2970,8 @@ def gcp_iam_service_account_generate_access_token_command(client: Client, args:
        CommandResults: outputs, readable outputs and raw response for XSOAR.

    """
-    service_account_email = args['service_account_email']
-    lifetime = args['lifetime']
+    service_account_email = args.get('service_account_email', '')


did you do it on purpose?

sapirshuker

Great!

* Add Roles to GCP IAM policy lookup - EXPANDR 5945 (demisto#30477) * Update gcp_iam_project_iam_policy_get_command - Add roles to gcp_iam_project_iam_policy_get_command * Remove Prints * Update header * Update MD header * Add release notes and metadata * Some of the suggested changes * Update role - Update role input description - Pop to index - remove explicit conditional * Update unit test, docker, and header verbiage * Update header * Flake8 fixes * Update release note * revert changes --------- Co-authored-by: John <40349459+BigEasyJ@users.noreply.github.com> Co-authored-by: sapirshuker <sshuker@paloaltonetworks.com> Co-authored-by: Sapir Shuker <49246861+sapirshuker@users.noreply.github.com>

* Add Roles to GCP IAM policy lookup - EXPANDR 5945 (#30477) * Update gcp_iam_project_iam_policy_get_command - Add roles to gcp_iam_project_iam_policy_get_command * Remove Prints * Update header * Update MD header * Add release notes and metadata * Some of the suggested changes * Update role - Update role input description - Pop to index - remove explicit conditional * Update unit test, docker, and header verbiage * Update header * Flake8 fixes * Update release note * revert changes --------- Co-authored-by: John <40349459+BigEasyJ@users.noreply.github.com> Co-authored-by: sapirshuker <sshuker@paloaltonetworks.com> Co-authored-by: Sapir Shuker <49246861+sapirshuker@users.noreply.github.com>

BigEasyJ added 5 commits October 26, 2023 12:02

Update gcp_iam_project_iam_policy_get_command

ea3aa4a

- Add roles to gcp_iam_project_iam_policy_get_command

Remove Prints

a713af2

Update header

501f190

Update MD header

66c771f

Add release notes and metadata

c4039d0

content-bot added Contribution Thank you! Contributions are always welcome! External PR Xsoar Support Level Indicates that the contribution is for XSOAR supported pack labels Oct 26, 2023

content-bot changed the base branch from master to contrib/PaloAltoNetworks_EXPANDR-5945-GCP-IAM October 26, 2023 17:27

content-bot assigned sapirshuker Oct 26, 2023

content-bot requested a review from sapirshuker October 26, 2023 17:28

sapirshuker assigned ShirleyDenkberg and unassigned ShirleyDenkberg Oct 29, 2023

ShirleyDenkberg reviewed Oct 30, 2023

View reviewed changes

ShirleyDenkberg self-assigned this Oct 30, 2023

ShirleyDenkberg added the docs-approved label Oct 30, 2023

sapirshuker requested changes Oct 30, 2023

View reviewed changes

BigEasyJ and others added 3 commits October 30, 2023 23:57

Some of the suggested changes

859f212

Update role

b183d5d

- Update role input description - Pop to index - remove explicit conditional

Merge branch 'contrib/PaloAltoNetworks_EXPANDR-5945-GCP-IAM' into EXP…

814e630

…ANDR-5945-GCP-IAM

sapirshuker self-requested a review November 1, 2023 11:54

sapirshuker requested changes Nov 1, 2023

View reviewed changes

BigEasyJ and others added 2 commits November 1, 2023 18:25

Update unit test, docker, and header verbiage

9fb0382

Merge branch 'contrib/PaloAltoNetworks_EXPANDR-5945-GCP-IAM' into EXP…

51ba96f

…ANDR-5945-GCP-IAM

content-bot added Community Contribution Form Filled Whether contribution form filled or not. labels Nov 1, 2023

BigEasyJ added 2 commits November 6, 2023 11:49

Merge branch 'contrib/PaloAltoNetworks_EXPANDR-5945-GCP-IAM' into EXP…

10be81e

…ANDR-5945-GCP-IAM

Merge branch 'contrib/PaloAltoNetworks_EXPANDR-5945-GCP-IAM' into EXP…

9d7bbd5

…ANDR-5945-GCP-IAM

sapirshuker added the pending-demo Demo pending label Nov 7, 2023

omerKarkKatz added the post-demo label Nov 12, 2023

BigEasyJ and others added 3 commits November 13, 2023 21:54

Update header

9b3de27

Merge branch 'contrib/PaloAltoNetworks_EXPANDR-5945-GCP-IAM' into EXP…

aa06218

…ANDR-5945-GCP-IAM

Flake8 fixes

28ec806

sapirshuker added the ready-for-instance-test In contribution PRs, this label will cause a trigger of a build with a modified pack from the PR. label Nov 14, 2023

BigEasyJ and others added 2 commits November 16, 2023 00:49

Merge branch 'contrib/PaloAltoNetworks_EXPANDR-5945-GCP-IAM' into EXP…

9b59edf

…ANDR-5945-GCP-IAM

Update release note

25cf1d9

sapirshuker added ready-for-instance-test In contribution PRs, this label will cause a trigger of a build with a modified pack from the PR. and removed ready-for-instance-test In contribution PRs, this label will cause a trigger of a build with a modified pack from the PR. labels Nov 16, 2023

Merge branch 'contrib/PaloAltoNetworks_EXPANDR-5945-GCP-IAM' into EXP…

e7fb7d1

…ANDR-5945-GCP-IAM

sapirshuker requested changes Nov 16, 2023

View reviewed changes

sapirshuker reviewed Nov 16, 2023

View reviewed changes

sapirshuker self-requested a review November 16, 2023 16:29

sapirshuker approved these changes Nov 16, 2023

View reviewed changes

sapirshuker merged commit 735db7a into demisto:contrib/PaloAltoNetworks_EXPANDR-5945-GCP-IAM Nov 16, 2023
20 of 21 checks passed

content-bot mentioned this pull request Nov 16, 2023

Add Roles to GCP IAM policy lookup - EXPANDR 5945 #30952

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Roles to GCP IAM policy lookup - EXPANDR 5945 #30477

Add Roles to GCP IAM policy lookup - EXPANDR 5945 #30477

BigEasyJ commented Oct 26, 2023

content-bot commented Oct 26, 2023

content-bot commented Oct 26, 2023

ShirleyDenkberg Oct 30, 2023

ShirleyDenkberg Oct 30, 2023

ShirleyDenkberg commented Oct 30, 2023

sapirshuker left a comment

sapirshuker Oct 29, 2023

sapirshuker Oct 29, 2023

sapirshuker Oct 30, 2023

BigEasyJ Oct 31, 2023

sapirshuker Oct 30, 2023

sapirshuker Oct 30, 2023

BigEasyJ Oct 31, 2023

sapirshuker Oct 30, 2023

BigEasyJ Oct 31, 2023

sapirshuker Oct 30, 2023

BigEasyJ Oct 31, 2023

sapirshuker left a comment

BigEasyJ commented Nov 1, 2023 •

edited

sapirshuker commented Nov 6, 2023

BigEasyJ commented Nov 6, 2023

sapirshuker commented Nov 9, 2023

sapirshuker commented Nov 13, 2023

BigEasyJ commented Nov 13, 2023

content-bot commented Nov 14, 2023 •

edited

sapirshuker commented Nov 16, 2023

sapirshuker left a comment

sapirshuker Nov 16, 2023

sapirshuker left a comment

	- description: 'A list of roles. Must be given as a comma separated list (Ex: "roles/bigquery.admin, roles/editor, roles/owner").'
	- description: 'A comma-separated list of roles. (Ex: "roles/bigquery.admin, roles/editor, roles/owner").'

	\| roles \| A comma separated list of roles. Must be given as a comma separated list (Ex: "roles/bigquery.admin, roles/editor, roles/owner"). \| Optional \|
	\| roles \| A comma-separated list of roles. (Ex: "roles/bigquery.admin, roles/editor, roles/owner"). \| Optional \|

		- description: 'A list of roles. Must be given as a comma separated list (Ex: "roles/bigquery.admin, roles/editor, roles/owner").'
		name: roles

		outputs["bindings"] = bindings[start:end]
		if len(bindings) < 50:


		##### GCP-IAM

		Updated the *gcp_iam_project_iam_policy_get_command* command, to support filtering by roles.

	Updated the *gcp_iam_project_iam_policy_get_command* command, to support filtering by roles.
	Updated the *gcp-iam-project-iam-policy-get* command, to support filtering by roles.

		@@ -1221,7 +1221,7 @@ def update_time_format(data: Union[dict, list], keys: list) -> list:

		def generate_iam_policy_command_output(response: dict, resource_name: str = None,

Add Roles to GCP IAM policy lookup - EXPANDR 5945 #30477

Add Roles to GCP IAM policy lookup - EXPANDR 5945 #30477

Conversation

BigEasyJ commented Oct 26, 2023

Contributing to Cortex XSOAR Content

Status

Related Issues

Description

Must have

content-bot commented Oct 26, 2023

content-bot commented Oct 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShirleyDenkberg commented Oct 30, 2023

sapirshuker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sapirshuker left a comment

Choose a reason for hiding this comment

BigEasyJ commented Nov 1, 2023 • edited

sapirshuker commented Nov 6, 2023

BigEasyJ commented Nov 6, 2023

sapirshuker commented Nov 9, 2023

sapirshuker commented Nov 13, 2023

BigEasyJ commented Nov 13, 2023

content-bot commented Nov 14, 2023 • edited

sapirshuker commented Nov 16, 2023

sapirshuker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sapirshuker left a comment

Choose a reason for hiding this comment

BigEasyJ commented Nov 1, 2023 •

edited

content-bot commented Nov 14, 2023 •

edited