[ASM] EXPANDR-4374 #30601

johnnywilkes · 2023-11-01T12:55:42Z

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

https://jira-dc.paloaltonetworks.com/browse/EXPANDR-4374

Description

Update the dev check automation to reference the GCP Folder hierarchy

Must have

Tests
Documentation

content-bot · 2023-11-01T12:57:37Z

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-11-01T12:57:38Z

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

kball-pa

overall lgtm! some minor comments about organization and docs, and one erroneous test case i think

kball-pa · 2023-11-02T19:06:06Z

.../CortexAttackSurfaceManagement/Scripts/InferWhetherServiceIsDev/InferWhetherServiceIsDev.yml

  isArray: true
  name: active_classifications
+- description: infrastructure hierarchy information to include CSPs.


i'm not sure i understand what this means, can you elaborate slightly?

PR feedback

kball-pa · 2023-11-02T19:16:19Z

...s/CortexAttackSurfaceManagement/Scripts/InferWhetherServiceIsDev/InferWhetherServiceIsDev.py

+                    if (("env" in key) or (key in ("stage", "function", "lifecycle", "usage", "tier"))) and \
+                       is_indicator_match(value):
+                        indicators.append(list_entry)
+        elif comparison_type == "string":


i might pull this logic out into a separate function to handle list[str] inputs, while preserving this one for list[dict] inputs, but that just be a matter of preference since functionally they're equivalent

kball-pa · 2023-11-02T19:21:32Z

...s/CortexAttackSurfaceManagement/Scripts/InferWhetherServiceIsDev/InferWhetherServiceIsDev.py

@@ -217,13 +236,19 @@ def main():
        args = demisto.args()


can you add the new hierarchy and provider args to the docstring for the main function? The reference to parameter observed_key_value_pairs might also need updating

also this might be the more appropriate place to elaborate on what hierarchy_info is (as opposed to where i commented on the yml)

PR feedback

kball-pa · 2023-11-02T19:27:01Z

...texAttackSurfaceManagement/Scripts/InferWhetherServiceIsDev/InferWhetherServiceIsDev_test.py

+    # Dev Hierachy only
+    assert final_decision(sample_no_match, sample_no_match, sample_no_match, sample_dev_hierarchy, sample_no_match, "")["result"]
+    # Both Dev Tags and Hierarchy
+    assert final_decision(sample_no_match, sample_no_match, sample_no_match, sample_dev_hierarchy, sample_no_match, "")["result"]


i think this was intended to have sample_dev_tag as the second arg

fixed in PR feedback

…ANDR-4374

johnnywilkes · 2023-11-07T18:51:51Z

@ShirleyDenkberg , can you please take a look?

johnnywilkes · 2023-11-07T18:59:25Z

@capanw / @BigEasyJ , please review

Packs/CortexAttackSurfaceManagement/ReleaseNotes/1_7_11.md

Packs/CortexAttackSurfaceManagement/Scripts/InferWhetherServiceIsDev/README.md

ShirleyDenkberg · 2023-11-08T07:41:26Z

@sapirshuker @kball-pa Doc review completed.

Co-authored-by: ShirleyDenkberg <[email protected]>

…ANDR-4374

kball-pa

LGTM!

BigEasyJ · 2023-11-08T20:16:34Z

...s/CortexAttackSurfaceManagement/Scripts/InferWhetherServiceIsDev/InferWhetherServiceIsDev.py

-                demisto.info(f"Ignoring item because it lacks the keys 'key' and/or 'value': {sorted(kv_pair.keys())}")
+    for list_entry in observed_list:
+        if comparison_type == "dictionary":
+            if not isinstance(list_entry, Mapping):


Suggested change

if not isinstance(list_entry, Mapping):

if not isinstance(list_entry, dict):

Does this work instead?

from what I have seen, it works exactly the same. I didn't write that line, but can change if you want.

BigEasyJ · 2023-11-08T20:50:20Z

...texAttackSurfaceManagement/Scripts/InferWhetherServiceIsDev/InferWhetherServiceIsDev_test.py

@@ -72,7 +73,7 @@ def test_get_indicators_from_external_classification(classifications, matches):
 def test_determine_reason(external, internal, reason):
    from InferWhetherServiceIsDev import determine_reason

-    assert determine_reason(external, internal) == reason
+    assert determine_reason(external, internal, [], "") == reason


Do we want another test for if provider: in determine_reason or is provider not functionally relevant?

I am not doing full testing on determine_reason(). i can, but next person will have to maintain 93%+ unit test coverage

johnnywilkes · 2023-11-08T23:32:44Z

@sapirshuker , please merge when possible

content-bot · 2023-11-09T07:36:20Z

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

sapirshuker · 2023-11-09T11:43:10Z

pipeline in Gitlab failed due to a known issue CIAC-6419.

* init * lint/format updates * update unit tests * PR feedback * test cov * done for now * docker try1 * Apply suggestions from code review * bump ver --------- Co-authored-by: johnnywilkes <[email protected]> Co-authored-by: ShirleyDenkberg <[email protected]>

johnnywilkes added 2 commits October 31, 2023 18:55

init

a09a0f8

lint/format updates

c6ad0c0

johnnywilkes marked this pull request as draft November 1, 2023 12:55

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

Merge branch 'master' into ASM-EXPANDR-4374

4842bb3

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 Nov 1, 2023

content-bot changed the base branch from master to contrib/PaloAltoNetworks_ASM-EXPANDR-4374 November 1, 2023 12:57

content-bot assigned sapirshuker Nov 1, 2023

content-bot requested a review from sapirshuker November 1, 2023 12:57

update unit tests

01347ce

kball-pa reviewed Nov 2, 2023

View reviewed changes

johnnywilkes and others added 4 commits November 5, 2023 19:26

Merge branch 'contrib/PaloAltoNetworks_ASM-EXPANDR-4374' into ASM-EXP…

47d0a66

…ANDR-4374

PR feedback

cc59bdd

test cov

f18c67e

done for now

b26f5d0

johnnywilkes requested a review from kball-pa November 7, 2023 18:22

johnnywilkes and others added 3 commits November 7, 2023 13:22

Merge branch 'contrib/PaloAltoNetworks_ASM-EXPANDR-4374' into ASM-EXP…

21d611d

…ANDR-4374

Merge branch 'contrib/PaloAltoNetworks_ASM-EXPANDR-4374' into ASM-EXP…

39552f9

…ANDR-4374

docker try1

8433618

ShirleyDenkberg reviewed Nov 8, 2023

View reviewed changes

ShirleyDenkberg added the docs-approved label Nov 8, 2023

ShirleyDenkberg self-assigned this Nov 8, 2023

johnnywilkes and others added 3 commits November 8, 2023 08:45

Apply suggestions from code review

0a27c67

Co-authored-by: ShirleyDenkberg <[email protected]>

bump ver

14a9763

Merge branch 'contrib/PaloAltoNetworks_ASM-EXPANDR-4374' into ASM-EXP…

ba57d5b

…ANDR-4374

kball-pa approved these changes Nov 8, 2023

View reviewed changes

BigEasyJ approved these changes Nov 8, 2023

View reviewed changes

johnnywilkes marked this pull request as ready for review November 8, 2023 23:32

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 9, 2023

sapirshuker approved these changes Nov 9, 2023

View reviewed changes

sapirshuker merged commit 1a12874 into demisto:contrib/PaloAltoNetworks_ASM-EXPANDR-4374 Nov 9, 2023
14 of 15 checks passed

content-bot mentioned this pull request Nov 9, 2023

[ASM] EXPANDR-4374 #30772

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ASM] EXPANDR-4374 #30601

[ASM] EXPANDR-4374 #30601

johnnywilkes commented Nov 1, 2023

content-bot commented Nov 1, 2023

content-bot commented Nov 1, 2023

kball-pa left a comment

kball-pa Nov 2, 2023

johnnywilkes Nov 6, 2023

kball-pa Nov 2, 2023

kball-pa Nov 2, 2023

kball-pa Nov 2, 2023

johnnywilkes Nov 6, 2023

kball-pa Nov 2, 2023

johnnywilkes Nov 6, 2023

johnnywilkes commented Nov 7, 2023

johnnywilkes commented Nov 7, 2023

ShirleyDenkberg commented Nov 8, 2023

kball-pa left a comment

BigEasyJ Nov 8, 2023

johnnywilkes Nov 8, 2023

BigEasyJ Nov 8, 2023

johnnywilkes Nov 8, 2023

johnnywilkes commented Nov 8, 2023

content-bot commented Nov 9, 2023

sapirshuker commented Nov 9, 2023

	if not isinstance(list_entry, Mapping):
	if not isinstance(list_entry, dict):

[ASM] EXPANDR-4374 #30601

[ASM] EXPANDR-4374 #30601

Conversation

johnnywilkes commented Nov 1, 2023

Contributing to Cortex XSOAR Content

Status

Related Issues

Description

Must have

content-bot commented Nov 1, 2023

content-bot commented Nov 1, 2023

kball-pa 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

johnnywilkes commented Nov 7, 2023

johnnywilkes commented Nov 7, 2023

ShirleyDenkberg commented Nov 8, 2023

kball-pa 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

johnnywilkes commented Nov 8, 2023

content-bot commented Nov 9, 2023

sapirshuker commented Nov 9, 2023