[Jamf_Pro] - Fix empty string issue for date query param in filter for Jamf Pro inventory data stream #13329

ShourieG · 2025-03-27T07:38:53Z

Type of change

Bug

Proposed commit message

WHAT: Fixed empty string issue for date query param in filter for Jamf Pro inventory data stream.

WHY: Recently there was a commit changing how the filter works detailed here. The issue is that this change does not take into account the fact that due to the previously existing cursor logic, there could be scenarios where the cursor.last_report_date is an empty string.

Sending the date in the filter as an empty string causes the API to respond with a status_code: 400 leading to failures. The fix in this PR tries to address this specific issue along with some minor cleanup.

Commit Message: The current cursor handling assumes that state.cursor.last_report_date is either absent or a valid cursor. However, previously stored cursor values may include an empty string for this value to indicate an absence. The outcome of this is that an invalid filter parameter was being sent in those cases. This changes the logic to correctly handle cursors that have used an empty string as an absence marker.

NOTE

Some minor cleanup and additions are included since they are all + changes. Some are just elastic-package format behaviour.

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.
I have verified that any added dashboard complies with Kibana's Dashboard good practices

Author's Checklist

[ ]

How to test this PR locally

Related issues

Relates [jamf_pro] Inventory pagination fix #12231

Screenshots

…ventory data stream.

elasticmachine · 2025-03-27T07:39:14Z

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

ShourieG · 2025-03-27T07:42:28Z

packages/jamf_pro/data_stream/inventory/_dev/test/pipeline/test-inventory.json

-                        "capableUsers": ["user.mcuserface@company.com"]
+                        "capableUsers": [
+                            "user.mcuserface@company.com"
+                        ]


cause by elastic-package, but its a + change so I let it stay.

ShourieG · 2025-03-27T07:42:58Z

packages/jamf_pro/data_stream/inventory/agent/stream/cel.yml.hbs


 keep_null: true
+redact:
+    fields: ~


added recommended redact field since it's a + change.

ShourieG · 2025-03-27T07:43:33Z

packages/jamf_pro/data_stream/inventory/manifest.yml

-      *NOTE* Enabling many sections may impact query processing.
-      It is recommended to only enable sections of interest.
-      The General section is always included.
+    description: "Settings for inventory requests.  \n*NOTE* Enabling many sections may impact query processing.\nIt is recommended to only enable sections of interest.\nThe General section is always included.\n"


caused by elastic-package format, but is good to have

I'm not sure this improves things.

Yes agree but since it aligns with the format expected by elastic-package, I think it's ok to keep it in.

I think it reflects a bug in the yaml implementation and it makes the code harder to read, but I won't push.

elastic-vault-github-plugin-prod · 2025-03-27T07:59:06Z

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

efd6

Suggest the following for the commit message:

The current cursor handling assumes that state.cursor.last_report_date is
either absent or a valid cursor. However, previously stored cursor values may
include an empty string for this value to indicate an absence. The outcome of
this is that an invalid filter parameter was being sent in those cases. This
changes the logic to correctly handle cursors that have used an empty string
as an absence marker.

Note that commit messages do not render markdown.

efd6 · 2025-03-27T08:02:35Z

packages/jamf_pro/data_stream/inventory/agent/stream/cel.yml.hbs

+  		?"filter": (has(state.?cursor.last_report_date) && state.cursor.last_report_date.orValue("") != "") ?
+   			optional.of(["general.reportDate>=\"" + state.cursor.last_report_date + "\""])
+   		:
+   			optional.none(),


Suggested change

?"filter": (has(state.?cursor.last_report_date) && state.cursor.last_report_date.orValue("") != "") ?

optional.of(["general.reportDate>=\"" + state.cursor.last_report_date + "\""])

:

optional.none(),

?"filter": state.?cursor.last_report_date.orValue("") != "" ?

optional.of(["general.reportDate>=\"" + state.cursor.last_report_date + "\""])

:

optional.none(),

state.cursor.last_report_date.orValue("") is not valid since state.cursor.last_report_date is not an optional type.

efd6 · 2025-03-27T08:02:54Z

packages/jamf_pro/data_stream/inventory/manifest.yml

-      *NOTE* Enabling many sections may impact query processing.
-      It is recommended to only enable sections of interest.
-      The General section is always included.
+    description: "Settings for inventory requests.  \n*NOTE* Enabling many sections may impact query processing.\nIt is recommended to only enable sections of interest.\nThe General section is always included.\n"


I'm not sure this improves things.

ShourieG · 2025-03-27T08:51:33Z

@efd6, addressed all suggested changes.

elastic-sonarqube · 2025-03-27T09:08:15Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

elasticmachine · 2025-03-27T09:08:39Z

💚 Build Succeeded

Buildkite Build
Commit: 0ca15d1

History

💚 Build #23940 succeeded 346981b

cc @ShourieG

efd6

Thanks.

Please make sure the line wraps are sane in the final commit message.

efd6 · 2025-03-27T09:37:54Z

packages/jamf_pro/data_stream/inventory/manifest.yml

-      *NOTE* Enabling many sections may impact query processing.
-      It is recommended to only enable sections of interest.
-      The General section is always included.
+    description: "Settings for inventory requests.  \n*NOTE* Enabling many sections may impact query processing.\nIt is recommended to only enable sections of interest.\nThe General section is always included.\n"


I think it reflects a bug in the yaml implementation and it makes the code harder to read, but I won't push.

elastic-vault-github-plugin-prod · 2025-03-27T10:10:24Z

Package jamf_pro - 0.5.1 containing this change is available at https://epr.elastic.co/package/jamf_pro/0.5.1/

…r Jamf Pro inventory data stream (#13329) The current cursor handling assumes that state.cursor.last_report_date is either absent or a valid cursor. However, previously stored cursor values may include an empty string for this value to indicate an absence. The outcome of this is that an invalid filter parameter was being sent in those cases. This changes the logic to correctly handle cursors that have used an empty string as an absence marker.

Fix empty string issue for date query param in filter for Jamf Pro in…

fc74b21

…ventory data stream.

ShourieG requested a review from a team as a code owner March 27, 2025 07:38

ShourieG self-assigned this Mar 27, 2025

ShourieG added the Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] label Mar 27, 2025

ShourieG added Integration:jamf_pro Jamf Pro bugfix Pull request that fixes a bug issue labels Mar 27, 2025

updated changelog

346981b

ShourieG commented Mar 27, 2025

View reviewed changes

ShourieG changed the title ~~[Jamf_Pro] - Fix empty string issue for date query param in filter for Jamf Pro in inventory data stream~~ [Jamf_Pro] - Fix empty string issue for date query param in filter for Jamf Pro inventory data stream Mar 27, 2025

efd6 reviewed Mar 27, 2025

View reviewed changes

addressed Dan's comments

0ca15d1

ShourieG requested a review from efd6 March 27, 2025 08:52

efd6 approved these changes Mar 27, 2025

View reviewed changes

ShourieG merged commit fa56b59 into elastic:main Mar 27, 2025
7 checks passed

ShourieG deleted the jamf_pro/filter_bugfix_5779 branch March 27, 2025 09:53

[Jamf_Pro] - Fix empty string issue for date query param in filter for Jamf Pro inventory data stream #13329

[Jamf_Pro] - Fix empty string issue for date query param in filter for Jamf Pro inventory data stream #13329

Uh oh!

Conversation

ShourieG commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change

Proposed commit message

NOTE

Checklist

Author's Checklist

How to test this PR locally

Related issues

Screenshots

Uh oh!

elasticmachine commented Mar 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShourieG Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elastic-vault-github-plugin-prod bot commented Mar 27, 2025

🚀 Benchmarks report

Uh oh!

efd6 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShourieG commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elastic-sonarqube bot commented Mar 27, 2025

Quality Gate passed

Uh oh!

elasticmachine commented Mar 27, 2025

💚 Build Succeeded

History

Uh oh!

efd6 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elastic-vault-github-plugin-prod bot commented Mar 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ShourieG commented Mar 27, 2025 •

edited

Loading

ShourieG Mar 27, 2025 •

edited

Loading

ShourieG commented Mar 27, 2025 •

edited

Loading