x-pack/metricbeat/module/panw: Add a new module #40686

dparkerelastic · 2024-09-03T19:33:50Z

Proposed commit message

The Metricbeat panos module collects metrics from a Palo Alto firewall. It contains metricsets corresponding to the following requirements:

Temperature
HA Interfaces
Logical interfaces
Power consumption alarms
Fan alarms
Disk use
BGP status
IPSec Tunnels
GlobalProtect sessions
GlobalProtect Gateway utilization
System resources
License expiration
Certificate expirations

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Disruptive User Impact

This is a new module and does not affect other modules.

How to test this PR locally

I have not created tests yet for this. Looking at other modules there seems to be a variety of approaches, so I would be looking for some guidance on this.

cla-checker-service · 2024-09-03T19:33:55Z

💚 CLA has been signed

mergify · 2024-09-03T19:34:33Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @dparkerelastic? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

elasticmachine · 2024-09-04T00:23:06Z

Pinging @elastic/sec-deployment-and-devices (Team:Security-Deployment and Devices)

belimawr · 2024-09-04T20:39:31Z

Hi @dparkerelastic, thanks for the contribution.

The first thing would be signing the CLA.

For tests, unit tests are a good starting point.

x-pack/metricbeat/module/panos/certificates/_meta/fields.yml

x-pack/metricbeat/module/panos/certificates/certificates.go

x-pack/metricbeat/module/panos/bgp_peers/bgp_peers.go

x-pack/metricbeat/module/panos/filesystem/filesystem.go

x-pack/metricbeat/module/panos/fans/fans.go

shmsr · 2024-09-06T10:57:58Z

x-pack/metricbeat/module/panos/ha_interfaces/_meta/fields.yml

+      description: Enabled field
+    - name: mode
+      type: keyword
+      description: Mode field


Every field description should be proper as well.

x-pack/metricbeat/module/panos/globalprotect_sessions/_meta/fields.yml

x-pack/metricbeat/module/panos/system/_meta/fields.yml

x-pack/metricbeat/module/panos/tunnels/_meta/fields.yml

shmsr · 2024-09-06T11:16:44Z

Also, there are a lot of metricsets, can we try to reduce them and group some of them so that the groups make much more sense?

For example: power, temperature, and fans (based on the XML query) can be grouped into something called hardware or system?

Similarly, can we group the others as well? Is there any potential issue if we group like that?

shmsr · 2024-09-06T11:23:36Z

Also, in your PR description, you mentioned you need guidance with tests. Here are some resources that might help:

https://www.elastic.co/guide/en/beats/devguide/8.15/metricbeat-developer-guide.html
https://www.elastic.co/guide/en/beats/devguide/8.15/testing.html
vSphere is a newly developed module. You can see how tests are written there: this and this. Infact, you can check other modules too to see how tests are written there.

dparkerelastic · 2024-09-06T12:50:30Z

Thank you for the comments, Subham. I will work on collapsing to fewer metricsets - I basically mapped 1 metricset to 1 requirement initially. I am also adding code around the pango client so that I can use a 'mock' during testing, and will add tests. I will address your other comments as I go.

On the pipeline failures - golangci-lint, etc: is the pipeline something that I have access to run so that I can verify for myself when I have made changes. I did a "go mod tidy" in the beats folder and it looks like pango is in the go.mod now....

Also, do I need to change the name of the module to "panw"?

Thanks again for the review!

DAP

shmsr · 2024-09-06T16:39:59Z

@dparkerelastic Thanks for the contribution. Yes, I can help you out with pipeline errors. Also, yes I forgot to mention that you need to run "go mod tidy" because while reviewing I noticed that updated go.{mod,sum} are not checked in.

On the pipeline failures - golangci-lint, etc: is the pipeline something that I have access to run so that I can verify for myself when I have made changes. I did a "go mod tidy" in the beats folder and it looks like pango is in the go.mod now....

I'll check and I'll let you know. Are you able to see the errors in the CI?

Also, do I need to change the name of the module to "panw"?

Not right now. Let's wait for other reviewers who have more ideas on this.

cmacknz

Approving go.mod changes.

shmsr · 2024-09-17T13:43:46Z

/test

pierrehilbert · 2024-09-17T14:00:07Z

run docs-build

ycombinator · 2024-09-17T14:00:32Z

Thank you to @qcorporation for reaching out to me off-PR about the longer-term ownership of this new module. I was assuming it would be @elastic/sec-deployment-and-devices, given that the team owns the logs equivalent of this panw module: https://github.com/elastic/beats/blob/f4f1e3f9a4e81f41cba55939663cddc2cae67f02/.github/CODEOWNERS#L163C30-L163C65

Assuming this is case, then @dparkerelastic please wait until engineers from @elastic/sec-deployment-and-devices have had a chance to review this PR. Also, please add an entry to the https://github.com/elastic/beats/blob/main/.github/CODEOWNERS file reflecting the ownership. Thanks!

tommyers-elastic · 2024-09-17T14:10:14Z

hey @ycombinator the current plan is that we (obs integrations) will own this. we're really short on time and need to merge this today for 8.15.2 FF (long story). if we have concerns on ownership long term, let's discuss is outside of this current PR review.

andrewkroh · 2024-09-17T14:25:59Z

From an Fleet integration point of view, if this is fetching XML and generating metric events (I haven't looked into the code) this could be implemented purely in as integration config. The CEL input can hit APIs and has good support for XML responses. This has the benefit of not creating a strong coupling to the Metricbeat codebase such that any bug-fixes can be released independently of Agent.

Also, please add an entry to the https://github.com/elastic/beats/blob/main/.github/CODEOWNERS file reflecting the ownership. Thanks!

++ to this to keep ownership clearly defined.

…happy

shmsr · 2024-09-17T15:02:20Z

/test

tommyers-elastic · 2024-09-17T15:07:51Z

@andrewkroh this is something we strongly considered, but on balance decided going with metricbeat was the right approach here, at least for right now.

shmsr · 2024-09-17T15:26:02Z

/test

pazone · 2024-09-17T15:32:40Z

/test

shmsr · 2024-09-17T18:21:32Z

/test

shmsr · 2024-09-17T20:12:19Z

run docs-build

* initial module creation * panos.system metricset running * remove testing data * panos.disk metricset working * rename metricset * change metricset name * bgp_peers metricset working * temperature metricset * more metricsets * use MetricSetFields * license notices * update fields.yml * added doc * refactor down to 4 metricsets * more cleanup * cleanup field names * remove yml * panos.yml.disabled * PR comment fixes * more PR comments addressed. Still to do: tests * Changes to: - move tunnels from vpn to interfaces metricset - address PR comments for field names in field.yml - split local/peer addresses into host and port for bgp - handle license expires of "never" * Fixes for PR comments * add license header * add pango package * mage check && mage update * remove mappings & make update * make linter happy * add the untracked docs * update the fields.yml * update the fields.yml with example fields to make python integ tests happy * make docs check happy and update codeowners * add result of 'mage update' in x-pack/metricbeat --------- Co-authored-by: subham sarkar <[email protected]> Co-authored-by: tommyers-elastic <[email protected]> (cherry picked from commit cc2c925) # Conflicts: # go.mod # x-pack/metricbeat/metricbeat.reference.yml

* initial module creation * panos.system metricset running * remove testing data * panos.disk metricset working * rename metricset * change metricset name * bgp_peers metricset working * temperature metricset * more metricsets * use MetricSetFields * license notices * update fields.yml * added doc * refactor down to 4 metricsets * more cleanup * cleanup field names * remove yml * panos.yml.disabled * PR comment fixes * more PR comments addressed. Still to do: tests * Changes to: - move tunnels from vpn to interfaces metricset - address PR comments for field names in field.yml - split local/peer addresses into host and port for bgp - handle license expires of "never" * Fixes for PR comments * add license header * add pango package * mage check && mage update * remove mappings & make update * make linter happy * add the untracked docs * update the fields.yml * update the fields.yml with example fields to make python integ tests happy * make docs check happy and update codeowners * add result of 'mage update' in x-pack/metricbeat --------- Co-authored-by: subham sarkar <[email protected]> Co-authored-by: tommyers-elastic <[email protected]> (cherry picked from commit cc2c925)

…le (#40866) * x-pack/metricbeat/module/panw: Add a new module (#40686) * initial module creation * panos.system metricset running * remove testing data * panos.disk metricset working * rename metricset * change metricset name * bgp_peers metricset working * temperature metricset * more metricsets * use MetricSetFields * license notices * update fields.yml * added doc * refactor down to 4 metricsets * more cleanup * cleanup field names * remove yml * panos.yml.disabled * PR comment fixes * more PR comments addressed. Still to do: tests * Changes to: - move tunnels from vpn to interfaces metricset - address PR comments for field names in field.yml - split local/peer addresses into host and port for bgp - handle license expires of "never" * Fixes for PR comments * add license header * add pango package * mage check && mage update * remove mappings & make update * make linter happy * add the untracked docs * update the fields.yml * update the fields.yml with example fields to make python integ tests happy * make docs check happy and update codeowners * add result of 'mage update' in x-pack/metricbeat --------- Co-authored-by: subham sarkar <[email protected]> Co-authored-by: tommyers-elastic <[email protected]> (cherry picked from commit cc2c925) # Conflicts: # go.mod # x-pack/metricbeat/metricbeat.reference.yml * fix merge issues --------- Co-authored-by: dparkerelastic <[email protected]> Co-authored-by: tommyers-elastic <[email protected]>

* initial module creation * panos.system metricset running * remove testing data * panos.disk metricset working * rename metricset * change metricset name * bgp_peers metricset working * temperature metricset * more metricsets * use MetricSetFields * license notices * update fields.yml * added doc * refactor down to 4 metricsets * more cleanup * cleanup field names * remove yml * panos.yml.disabled * PR comment fixes * more PR comments addressed. Still to do: tests * Changes to: - move tunnels from vpn to interfaces metricset - address PR comments for field names in field.yml - split local/peer addresses into host and port for bgp - handle license expires of "never" * Fixes for PR comments * add license header * add pango package * mage check && mage update * remove mappings & make update * make linter happy * add the untracked docs * update the fields.yml * update the fields.yml with example fields to make python integ tests happy * make docs check happy and update codeowners * add result of 'mage update' in x-pack/metricbeat --------- Co-authored-by: subham sarkar <[email protected]> Co-authored-by: tommyers-elastic <[email protected]> (cherry picked from commit cc2c925) Co-authored-by: dparkerelastic <[email protected]>

lalit-satapathy · 2024-09-18T10:30:16Z

hey @ycombinator the current plan is that we (obs integrations) will own this. we're really short on time and need to merge this today for 8.15.2 FF (long story). if we have concerns on ownership long term, let's discuss is outside of this current PR review.

Agree these are obs integrations packages, as they focus on metrics. We will raise PR updating the same.

dparkerelastic added 13 commits August 29, 2024 15:23

initial module creation

c286f92

panos.system metricset running

741fc30

remove testing data

b4140e2

panos.disk metricset working

f78742f

rename metricset

daf2caf

change metricset name

6004852

bgp_peers metricset working

6ff51e6

temperature metricset

4c986a1

more metricsets

eff0e0b

use MetricSetFields

7a359ea

license notices

cdafd8b

update fields.yml

246d065

added doc

e26bd4f

dparkerelastic requested a review from a team as a code owner September 3, 2024 19:33

dparkerelastic requested review from belimawr and rdner September 3, 2024 19:33

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Sep 3, 2024

mergify bot assigned dparkerelastic Sep 3, 2024

ycombinator added the Team:Security-Deployment and Devices Deployment and Devices Team in Security Solution label Sep 4, 2024

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Sep 4, 2024

ycombinator requested a review from a team September 4, 2024 00:23

shmsr requested changes Sep 6, 2024

View reviewed changes

tommyers-elastic approved these changes Sep 17, 2024

View reviewed changes

update the fields.yml

1d55a22

cmacknz approved these changes Sep 17, 2024

View reviewed changes

update the fields.yml with example fields to make python integ tests …

77ba48e

…happy

make docs check happy and update codeowners

d51944e

shmsr requested a review from a team as a code owner September 17, 2024 15:15

tommyers-elastic added the backport-8.15 Automated backport to the 8.15 branch with mergify label Sep 17, 2024

pazone approved these changes Sep 17, 2024

View reviewed changes

shmsr and others added 2 commits September 17, 2024 22:41

Merge branch 'main' into dap_panos

162bec0

add result of 'mage update' in x-pack/metricbeat

87e57f5

shmsr enabled auto-merge (squash) September 17, 2024 19:36

shmsr merged commit cc2c925 into elastic:main Sep 17, 2024
122 of 125 checks passed

mergify bot mentioned this pull request Sep 17, 2024

[8.15](backport #40686) x-pack/metricbeat/module/panw: Add a new module #40866

Merged

6 tasks

mergify bot mentioned this pull request Sep 17, 2024

[8.x](backport #40686) x-pack/metricbeat/module/panw: Add a new module #40867

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x-pack/metricbeat/module/panw: Add a new module #40686

x-pack/metricbeat/module/panw: Add a new module #40686

dparkerelastic commented Sep 3, 2024 •

edited by shmsr

Loading

cla-checker-service bot commented Sep 3, 2024 •

edited

Loading

mergify bot commented Sep 3, 2024

elasticmachine commented Sep 4, 2024

belimawr commented Sep 4, 2024

shmsr Sep 6, 2024

shmsr commented Sep 6, 2024

shmsr commented Sep 6, 2024 •

edited

Loading

dparkerelastic commented Sep 6, 2024 •

edited

Loading

shmsr commented Sep 6, 2024

cmacknz left a comment

shmsr commented Sep 17, 2024

pierrehilbert commented Sep 17, 2024

ycombinator commented Sep 17, 2024

tommyers-elastic commented Sep 17, 2024

andrewkroh commented Sep 17, 2024 •

edited

Loading

shmsr commented Sep 17, 2024

tommyers-elastic commented Sep 17, 2024

shmsr commented Sep 17, 2024

pazone commented Sep 17, 2024

shmsr commented Sep 17, 2024

shmsr commented Sep 17, 2024

lalit-satapathy commented Sep 18, 2024

x-pack/metricbeat/module/panw: Add a new module #40686

x-pack/metricbeat/module/panw: Add a new module #40686

Conversation

dparkerelastic commented Sep 3, 2024 • edited by shmsr Loading

Proposed commit message

Checklist

Disruptive User Impact

How to test this PR locally

cla-checker-service bot commented Sep 3, 2024 • edited Loading

mergify bot commented Sep 3, 2024

elasticmachine commented Sep 4, 2024

belimawr commented Sep 4, 2024

shmsr Sep 6, 2024

Choose a reason for hiding this comment

shmsr commented Sep 6, 2024

shmsr commented Sep 6, 2024 • edited Loading

dparkerelastic commented Sep 6, 2024 • edited Loading

shmsr commented Sep 6, 2024

cmacknz left a comment

Choose a reason for hiding this comment

shmsr commented Sep 17, 2024

pierrehilbert commented Sep 17, 2024

ycombinator commented Sep 17, 2024

tommyers-elastic commented Sep 17, 2024

andrewkroh commented Sep 17, 2024 • edited Loading

shmsr commented Sep 17, 2024

tommyers-elastic commented Sep 17, 2024

shmsr commented Sep 17, 2024

pazone commented Sep 17, 2024

shmsr commented Sep 17, 2024

shmsr commented Sep 17, 2024

lalit-satapathy commented Sep 18, 2024

dparkerelastic commented Sep 3, 2024 •

edited by shmsr

Loading

cla-checker-service bot commented Sep 3, 2024 •

edited

Loading

shmsr commented Sep 6, 2024 •

edited

Loading

dparkerelastic commented Sep 6, 2024 •

edited

Loading

andrewkroh commented Sep 17, 2024 •

edited

Loading