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

[Azure] Add Azure Application Gateway datastream #3892

Merged
merged 16 commits into from
Oct 4, 2022

Conversation

legoguy1000
Copy link
Contributor

@legoguy1000 legoguy1000 commented Jul 28, 2022

What does this PR do?

Add Azure Application Gateway datastream

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

@elasticmachine
Copy link

elasticmachine commented Jul 28, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-10-04T10:23:58.856+0000

  • Duration: 15 min 51 sec

Test stats 🧪

Test Results
Failed 0
Passed 123
Skipped 0
Total 123

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@legoguy1000 legoguy1000 marked this pull request as ready for review July 28, 2022 22:18
@legoguy1000 legoguy1000 requested a review from a team as a code owner July 28, 2022 22:18
@zmoog
Copy link
Contributor

zmoog commented Aug 1, 2022

Hey @legoguy1000, thank you for your contribution! \o/

I am looking for the most appropriate team/person to review your PR.

@ebeahan ebeahan requested a review from a team August 1, 2022 21:29
@zmoog
Copy link
Contributor

zmoog commented Aug 2, 2022

We may need to add instructions about which Azure resources users must set up to use the integration.

@legoguy1000, which diagnostic setting did you set up to send logs to the event hub?

Looking at the sample_event.json file content, my educated guess is that it is the diagnostic settings from the Application Gateway:

CleanShot 2022-08-02 at 12 43 25@2x

Am I right?

@legoguy1000
Copy link
Contributor Author

@zmoog I don't currently have an azure WAF to test this on, I just went off the documentation here, https://docs.microsoft.com/en-us/azure/web-application-firewall/ag/web-application-firewall-logs.

@elasticmachine
Copy link

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@zmoog
Copy link
Contributor

zmoog commented Aug 3, 2022

I don't currently have an azure WAF to test this on; I just went off the documentation here https://docs.microsoft.com/en-us/azure/web-application-firewall/ag/web-application-firewall-logs.

Got it, thanks! Interesting reading.

I see the log categories used as data sources are the following:

  • Application Gateway Access Log
  • Application Gateway Performance Log
  • Application Gateway Firewall Log

@legoguy1000 I noticed pipeline test documents for "Application Gateway Access Log" and "Application Gateway Firewall Log": are we supporting both, right?

@zmoog
Copy link
Contributor

zmoog commented Aug 3, 2022

Questions for the security-external-integrations team:

  • Is there interest in the "Application Gateway Performance Log"?
  • I see this integration focuses on WAF, but should we also consider an integration about Application Gateway?

@legoguy1000
Copy link
Contributor Author

I don't currently have an azure WAF to test this on; I just went off the documentation here https://docs.microsoft.com/en-us/azure/web-application-firewall/ag/web-application-firewall-logs.

Got it, thanks! Interesting reading.

I see the log categories used as data sources are the following:

  • Application Gateway Access Log
  • Application Gateway Performance Log
  • Application Gateway Firewall Log

@legoguy1000 I noticed pipeline test documents for "Application Gateway Access Log" and "Application Gateway Firewall Log": are we supporting both, right?

Sorry I didn't see this earlier. I don't see any reason you wouldn't want both. One is just access logs, the other is the actual WAF security rules alert logs.

@efd6
Copy link
Contributor

efd6 commented Aug 17, 2022

/test

@elasticmachine
Copy link

elasticmachine commented Aug 17, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (10/10) 💚
Files 86.364% (19/22) 👎 -11.193
Classes 86.364% (19/22) 👎 -11.193
Methods 83.422% (156/187) 👎 -6.334
Lines 85.011% (2779/3269) 👎 -6.743
Conditionals 100.0% (0/0) 💚

@legoguy1000
Copy link
Contributor Author

Questions for the security-external-integrations team:

  • Is there interest in the "Application Gateway Performance Log"?
  • I see this integration focuses on WAF, but should we also consider an integration about Application Gateway?

The performance log is more of a metric but I guess we could add it. Also unless I'm reading something wrong, the waf is the application gateway or at least that's what it's log names are.

@legoguy1000
Copy link
Contributor Author

I think this is good for now.

@zmoog
Copy link
Contributor

zmoog commented Aug 25, 2022

I see this integration is using the Application Gateway resource logs to collect WAF data; in particular, we are leveraging the following log categories:

  • ApplicationGatewayAccessLog
  • ApplicationGatewayFirewallLog

I just noticed that Front Door also produces WAF data using its resource logs named FrontdoorAccessLog and FrontdoorWebApplicationFirewallLog.

@efd6, do you think we should also support WAF data from Front Door's logs in this integration?

@efd6, another more general question: what if someone asks us to support Application Gateway logs or Front Door logs without focusing on WAF? For example, they want the access logs with no particular focus on WAF.

@zmoog
Copy link
Contributor

zmoog commented Aug 25, 2022

Hey @legoguy1000, in a previous comment, you mentioned that you don't azure WAF to test this on. @efd6, did you get a chance to test this integration?

Otherwise, I can give it a try.

@zmoog
Copy link
Contributor

zmoog commented Aug 25, 2022

@jamiehynds, if I am not wrong, this integration does not have a dashboard. Can we go with it and add it later, or should we add it in this PR? I believe the questions I asked Dan in a previous comment were also for you 😇

@zmoog
Copy link
Contributor

zmoog commented Aug 25, 2022

One last thing.

@legoguy1000, thank you for your contribution! I'll keep an eye on this PR, and I won't let you wait again for more than necessary.

@legoguy1000
Copy link
Contributor Author

@zmoog you're all good, we're all busy with a million things. I could always rename the data stream to application gateway and we could create a separate one for front door?

@efd6
Copy link
Contributor

efd6 commented Aug 25, 2022

@zmoog No I have not tested this. Please take it for a spin.

@jamiehynds
Copy link

@jamiehynds, if I am not wrong, this integration does not have a dashboard. Can we go with it and add it later, or should we add it in this PR? I believe the questions I asked Dan in a previous comment were also for you 😇

Hey @zmoog - with regards to Frontdoor logs, we seem to have a community contribution for Frontdoor support (including a dashboard) - #2497. I'm not overly familiar on the differences between WAF and Frontdoor. Should these be separate integrations or should Frontdoor be a datastream under Azure WAF?

With regards to a dashboard, it's always our preference to include dashboards with new integrations as when we don't ship dashboards, it's easy to put it on the long finger and we end up with dashboards without integrations. Do you think we could add a dashboad as part of this PR?

@P1llus P1llus self-requested a review September 28, 2022 07:52
@P1llus P1llus self-assigned this Sep 28, 2022
@elasticmachine
Copy link

elasticmachine commented Sep 28, 2022

🚀 Benchmarks report

Package azure 👍(6) 💚(3) 💔(1)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
auditlogs 1515.15 1088.14 -427.01 (-28.18%) 💔

To see the full report comment with /test benchmark fullreport

@legoguy1000
Copy link
Contributor Author

@P1llus updated.

@legoguy1000 legoguy1000 changed the title [Azure] Add Azure WAF datastream [Azure] Add Azure Application Gateway datastream Sep 28, 2022
Copy link
Member

@P1llus P1llus left a comment

Choose a reason for hiding this comment

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

Added a few small nitpicks, could you also remove any changes done to the other datastreams? As they should be updated in another PR if necessary.

Could you also rename it to application_gateway rather than application_gateway_logs ?

After the changes and a review from obs-cloud-monitoring we can merge.

@zmoog zmoog self-requested a review September 29, 2022 09:30
This completes the transition from Azure WAF to Azure Application
Gateway.
Like the AWS integration, we keep fields and event reference material
in the individual integration docs.

The README.md file was getting hard to read due to the extra long
reference section.
We are working to define a more defined process to graduate from beta
to a GA for each integration or data stream that we release.
@zmoog
Copy link
Contributor

zmoog commented Oct 4, 2022

Hey @legoguy1000, thank you again for the initial work and the will to update it!

I made all the pending changes from my last comments and LGTM. I'll take care of the CI.

When all is green, we can merge.

Renamed field with the expected name.
@P1llus P1llus merged commit 6adb710 into elastic:main Oct 4, 2022
@legoguy1000 legoguy1000 deleted the 1306-azure-waf branch October 4, 2022 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure WAF
6 participants