Skip to content

Azure Firewall integration#3578

Merged
ebeahan merged 37 commits intoelastic:mainfrom
ebeahan:package-azure-firewall
Jul 8, 2022
Merged

Azure Firewall integration#3578
ebeahan merged 37 commits intoelastic:mainfrom
ebeahan:package-azure-firewall

Conversation

@ebeahan
Copy link
Copy Markdown
Member

@ebeahan ebeahan commented Jun 23, 2022 •
edited
Loading

What does this PR do?

This PR adds a new integration for Azure Firewall.

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.

How to test this PR locally

cd packages/azure
elastic-package test pipeline

Related issues

Relates to #1205

Screenshots

Screen Shot 2022-06-23 at 3 13 20 PM

Screen Shot 2022-06-23 at 3 15 21 PM

filebeat-azure-firewall-overview

filebeat-azure-firewall-dns-proxy

@ebeahan ebeahan added enhancement New feature or request Team:Security-External Integrations Integration:azure Azure Logs New Integration Issue or pull request for creating a new integration package. labels Jun 23, 2022
@ebeahan ebeahan requested a review from a team as a code owner June 23, 2022 20:17
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Jun 23, 2022

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

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Jun 23, 2022 •
edited
Loading

💚 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-07-08T14:17:03.279+0000

  • Duration: 15 min 58 sec

Test stats 🧪

Test Results
Failed 0
Passed 105
Skipped 0
Total 105

🤖 GitHub comments

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

  • /test : Re-trigger the build.

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Jun 23, 2022 •
edited
Loading

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (7/7) 💚
Files 81.25% (13/16) 👎 -15.676
Classes 81.25% (13/16) 👎 -15.676
Methods 80.451% (107/133) 👎 -8.894
Lines 81.81% (1853/2265) 👎 -8.403
Conditionals 100.0% (0/0) 💚

@andrewkroh andrewkroh requested a review from a team June 23, 2022 21:37
efd6
efd6 reviewed Jun 24, 2022
View reviewed changes
packages/azure/data_stream/firewall_logs/fields/fields.yml Outdated
Comment on lines 41 to 44
Copy link
Copy Markdown
Contributor

@efd6 efd6 Jun 24, 2022

Choose a reason for hiding this comment

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

network_traffic has this as a long for the icmp datastream, but kubernetes has it as a keyword for an HTTP request code. I can't see it defined, so is there a preference?

Copy link
Copy Markdown
Member Author

@ebeahan ebeahan Jun 24, 2022

Choose a reason for hiding this comment

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

Unless a numeric field needs to support range queries, ES recommends keyword for the better term query performance.

For ICMP messages, I think users are more likely query for a specific request code than across a range.

@ebeahan ebeahan requested a review from a team July 6, 2022 20:16
@ebeahan
Copy link
Copy Markdown
Member Author

ebeahan commented Jul 6, 2022

Last round of comments have been addressed - ready for another review round.

efd6
efd6 reviewed Jul 7, 2022
View reviewed changes
@ebeahan
Copy link
Copy Markdown
Member Author

ebeahan commented Jul 7, 2022

Thanks for the feedback, @efd6. I've incorporated your suggestions.

efd6
efd6 approved these changes Jul 7, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

LGTM

ebeahan added 22 commits July 8, 2022 09:14
@ebeahan ebeahan force-pushed the package-azure-firewall branch from a4721e7 to 7cdd3df Compare July 8, 2022 14:16
@ebeahan
Copy link
Copy Markdown
Member Author

ebeahan commented Jul 8, 2022

@elastic/obs-cloud-monitoring would you also please review as package owners?

kaiyan-sheng
kaiyan-sheng approved these changes Jul 8, 2022
View reviewed changes
Copy link
Copy Markdown

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

LGTM!

@ebeahan ebeahan merged commit baa50ca into elastic:main Jul 8, 2022
@ebeahan ebeahan deleted the package-azure-firewall branch July 8, 2022 16:32
@ebeahan ebeahan mentioned this pull request Jul 8, 2022
Closed
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@efd6 efd6 efd6 approved these changes

+1 more reviewer

@kaiyan-sheng kaiyan-sheng kaiyan-sheng approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement New feature or request Integration:azure Azure Logs New Integration Issue or pull request for creating a new integration package.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ebeahan @elasticmachine @kaiyan-sheng @efd6