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

ZeroFox Solution: Fix project structure for Data Connector deployment #10411

Merged
merged 8 commits into from
Jul 1, 2024

Conversation

DNRRomero
Copy link
Contributor

Change(s):

  • Reorganized the project structure for azure function-based data connectors in the ZeroFox Solution.

Reason for Change(s):

  • The current project structure causes issues during deployment that block use by ZeroFox users

Version Updated:

  • No

Testing Completed:

  • Need Help

Checked that the validations are passing and have addressed any issues that are present:

  • See guidance below

@DNRRomero DNRRomero requested review from a team as code owners April 29, 2024 22:28
@DNRRomero
Copy link
Contributor Author

Hi @v-sudkharat, we created the PR as requested

@v-atulyadav v-atulyadav added the Solution Solution specialty review needed label Apr 30, 2024
@DNRRomero
Copy link
Contributor Author

Hey @v-sudkharat
image
Deployment was successful but I don't see the connectors configured, do you mind if we meet again? or can you see what is wrong from the PR?

@v-sudkharat
Copy link
Contributor

@DNRRomero, I just checked the updated zip file and found that, Inside the zip there are sub-folder is created and due to that the function apps do not get displayed.
So, Could you please correct it and retest it once by taking ref. of below screenshots, and if still having issue. Please let me know will schedule a call for Tomorrow. Thanks!

  1. Inside the zip file remove below highlighted folder, which is not required: -

image

  1. Inside the zip file there is subfolder with the name "AzureFunctionZeroFoxCTI" which contain the function app. So please just place those files outside and remove that created sub-folder.

image

Expected : Below zi there is not subfolder added -

image

@DNRRomero
Copy link
Contributor Author

Hey @v-sudkharat given your instructions, I was able to deploy our data connectors, thank you!
image
The app should be ready for repackaging

@v-sudkharat
Copy link
Contributor

Hey @DNRRomero, The changes look good now. Could you please re-package this solution and commit the into this PR.
Regarding the connector, as it displayed into the function so please add the connector successfully connected screenshots over here for both the connector-
1. CTI connector
2. Alerts connecto
r

After successfully connected the connector, please add screenshot link below :-
image

And for azure functions, need to add screenshots with function app working successfully. Thanks!

@DNRRomero
Copy link
Contributor Author

Hi @v-sudkharat I re submitted the changes you requested and, while trying to re deploy my main template for testing I received the following error
{"code":"InvalidTemplate","message":"Deployment template validation failed: 'The template resource '/Microsoft.SecurityInsights/-dc-t36lq3wueus7i' for type 'Microsoft.OperationalInsights/workspaces/providers/contentTemplates' at line '96' and column '87' has incorrect segment lengths. A nested resource type must have identical number of segments as its resource name. A root resource type must have segment length one greater than its resource name. Please see https://aka.ms/arm-syntax-resources for usage details.'."}

@DNRRomero DNRRomero force-pushed the fix_packaging branch 2 times, most recently from 884b08c to 8cb44de Compare May 14, 2024 02:28
@DNRRomero
Copy link
Contributor Author

Hi @v-sudkharat I have all connectors working, can we sync?
The solution is ready for merge

@DNRRomero
Copy link
Contributor Author

Hi @v-sudkharat
Alerts CCP connector works as expected
image

@v-sudkharat
Copy link
Contributor

Hey @DNRRomero, Thanks for the update, I will review the content, and can we block time on 21-05-2024 for a sync?

@DNRRomero
Copy link
Contributor Author

Hey @v-sudkharat , can it be the 20th? or better yet some day this week? the 21st is a holiday in my country

@v-sudkharat
Copy link
Contributor

@DNRRomero, ok, I will send an invite over mail.

@v-sudkharat
Copy link
Contributor

Hi @DNRRomero, please revert the changes for below highlighted solution folder :-
image

@v-sudkharat
Copy link
Contributor

Remove the duplicate data connector description from Createui file and update the same into 3.0.1 zip as well. -
image

@v-sudkharat
Copy link
Contributor

Hi @DNRRomero, Is function app started received logs for remaining data types?

@v-sudkharat
Copy link
Contributor

Hi @DNRRomero, We are still waiting for your response. Thanks!

@DNRRomero
Copy link
Contributor Author

Hi @v-sudkharat, so far the rest of the connectors have not shown up as connected.
I've been looking at the code to compare those that work with those that did not but have not found anything different, do you have an idea of what might be happening?

@v-sudkharat
Copy link
Contributor

@DNRRomero, Could you please check for the Invocation logs in functions which not show as connected, and if logs are receiving the into the LAW and it still not show as connected then, need to check for Connectivity criteria inside of function app deploy file,
Thanks!

@v-sudkharat
Copy link
Contributor

Hi @DNRRomero, Any update for logs for function app?

@DNRRomero
Copy link
Contributor Author

Hi @v-sudkharat bI saw the logs and connectivity criteria and they are all standard for all connectors, could it be that not all logs are appearing in application insights?

@v-sudkharat
Copy link
Contributor

@DNRRomero, It should. Can you commit latest changes and let me know so i can review it.

@DNRRomero
Copy link
Contributor Author

Hi @v-sudkharat any chance we could meet? I've been trying to identify the issue but have not made much progress

@v-sudkharat
Copy link
Contributor

@DNRRomero, Sure. we can connect. Meantime can I have demo credentials from your end to investigate the function app from my end.
Please share it mail ID, so i can check on it.

@v-sudkharat
Copy link
Contributor

@DNRRomero, waiting for your response.

@DNRRomero
Copy link
Contributor Author

Hi @v-sudkharat I sent you credentials over email a few days ago, also scheduled a meeting
will you be able to attend?

@DNRRomero
Copy link
Contributor Author

Hi @v-sudkharat given what we discussed today, I added some changes that should fix some of the issues we have encountered so far, any other issues that may appear are related to our internal API and should be fixed under another PR upon further investigation!

@v-sudkharat
Copy link
Contributor

Hi @DNRRomero, Thanks for adding changes. Kindly check on below few points : -

  1. Please remove the below highlighted file which is not required: -
    image

  2. After checking the createUI file, find out the duplicate data connector description text, please remove one of them and add same changes into zip: -
    image

  3. After testing the function app, now able to see the data ingesting for 10 functions:
    image

while testing remaining of them, found that the Testing Account provided to me is does not have the required permissions to get the data:
image

Could you please check updated connector by your Account which have the required access and data into it. And share the successfully connected screenshots with us.

Please update this branch from master.

Thanks!

@DNRRomero
Copy link
Contributor Author

hey @v-sudkharat I added the changes requested
also here is the screenshot with the 10 connectors

image

@v-sudkharat
Copy link
Contributor

@DNRRomero, Great. Changes looks good now. Approving the PR and proceeding to Merging it.
Please let us know if you have anything for us. Thanks!

@DNRRomero
Copy link
Contributor Author

DNRRomero commented Jul 1, 2024

@DNRRomero, waiting for your response.

Hi @v-sudkharat no comments on my side, is there anything left for the PR merge?

Copy link
Contributor

@v-sudkharat v-sudkharat left a comment

Choose a reason for hiding this comment

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

Changes looks good

@v-atulyadav v-atulyadav merged commit cbe511a into Azure:master Jul 1, 2024
31 checks passed
@v-sudkharat
Copy link
Contributor

@DNRRomero, We have merged the PR. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Solution Solution specialty review needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants