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.

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

Jump to bottom

Update Active Directory Query V2 to use LDAP docker image #29069

Merged
merged 26 commits into from
Aug 24, 2023
Merged

Update Active Directory Query V2 to use LDAP docker image #29069

merged 26 commits into from
Aug 24, 2023

Conversation

anas-yousef
Copy link
Contributor

@anas-yousef anas-yousef commented Aug 20, 2023 •
edited
Loading

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: https://jira-hq.paloaltonetworks.local/browse/XSUP-24646

Description

Changed the docker image of the Active Directory Query V2 integration to the LDAP docker image, which now uses a debian based docker image.

Must have

  • Tests
  • Documentation

anas-yousef added 10 commits May 28, 2023 14:34
@anas-yousef
Created test instance to send to user
1ab05f2
@anas-yousef
Merge branch 'master' of https://github.com/demisto/content into ay-s…
5a88f40 
…ync-issues-servicenow-v2
@anas-yousef
Merge branch 'master' of https://github.com/demisto/content into ay-s…
482c12a 
…ync-issues-servicenow-v2
@anas-yousef
Merge branch 'master' of https://github.com/demisto/content into ay-a…
b1bb441 
…ctive-directory-invalid-address
@anas-yousef
Merge branch 'master' of https://github.com/demisto/content into ay-a…
9f12524 
…ctive-directory-invalid-address
@anas-yousef
Merge branch 'master' of https://github.com/demisto/content into ay-a…
f9d41c4 
…ctive-directory-invalid-address
@anas-yousef
Merge branch 'master' of https://github.com/demisto/content into ay-a…
f803043 
…ctive-directory-invalid-address
@anas-yousef
Integration now uses LDAP docker image, which is debian based
e436f84
@anas-yousef
Removed wrong files
fe2d960
@anas-yousef
Merge branch 'master' into ay-active-directory-invalid-address
1d9f8e9
@anas-yousef anas-yousef mentioned this pull request Aug 20, 2023
Merged
3 tasks
@anas-yousef anas-yousef self-assigned this Aug 20, 2023
@anas-yousef anas-yousef requested a review from ostolero as a code owner August 21, 2023 17:00
@ShacharKidor ShacharKidor self-requested a review August 22, 2023 07:27
ShacharKidor
ShacharKidor approved these changes Aug 22, 2023
View reviewed changes
Tests/conf.json
@@ -2543,29 +2543,34 @@
"playbookID": "Active Directory Test",
"integrations": "Active Directory Query v2",
"instance_names": "active_directory_ninja",
"memory_threshold": 100,
Copy link
Contributor

Choose a reason for hiding this comment

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

@anas-yousef - please explain those changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The build was failing, as can be seen here (line 2562), due to insufficient memory.

@JasBeilin
Copy link
Contributor

Need a force merge due to known NG issues.

@anas-yousef
Copy link
Contributor Author

This PR requires a force merge because:
The XSOAR NG build is failing on a test playbook (Active Directory - manual pagination check), and when I investigated the test playbook on the build machine, I can see that the tasks are configured without any inputs, which makes the test playbook fail. I tried running the test playbook on a disposable tenant, and the test playbook passed and all tasks are show with inputs, and the test playbook passes on all XSOAR 6.x build machines.
image (3)
image (4)
This is from my XSOAR NG disposable tenant, where we can see that the test playbook does indeed succeed.
Active_Directory_-_manual_pagination_check_Tue_Aug_22_2023

@DeanArbel DeanArbel added the ForceMerge Forcing the merge of the PR despite the build status label Aug 24, 2023
@DeanArbel DeanArbel merged commit 9d11279 into master Aug 24, 2023
@DeanArbel DeanArbel deleted the ay-active-directory-invalid-address branch August 24, 2023 06:10
xsoar-bot pushed a commit to xsoar-contrib/content that referenced this pull request Oct 5, 2023
@anas-yousef @JasBeilin @xsoar-bot
Update Active Directory Query V2 to use LDAP docker image (demisto#29069
f65bb90 
)

* Created test instance to send to user

* Integration now uses LDAP docker image, which is debian based

* Removed wrong files

* Added RNs

* Added memory_threshold=100

* Added memory_threshold

* Fixed conflicts

* Added memory threshold again after conflicts

* Update Active_Directory_Query.yml

* Update 1_6_22.md

---------

Co-authored-by: Jasmine Beilin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShacharKidor ShacharKidor ShacharKidor approved these changes

@ostolero ostolero Awaiting requested review from ostolero

Assignees

@anas-yousef anas-yousef

Labels
docs-approved ForceMerge Forcing the merge of the PR despite the build status
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anas-yousef @JasBeilin @ShacharKidor @DeanArbel