mirror of
https://github.com/fleetdm/fleet
synced 2026-04-21 21:47:20 +00:00
1cd37ef966
For #25555 # Checklist for submitter - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files) for more information. - [X] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) This PR updates the `NewLabel` service to use the `UpdateLabelMembershipByHostIDs` method previously added by @jacobshandling rather than using `ApplyLabels`. The latter method has performance issues when adding large numbers of hosts at once to a manual label (see #25555) because it does an expensive lookup of host names before transforming those into Fleet host IDs. The new code skips the middleman and transforms host identifiers directly to Fleet host IDs, and does so using a batching strategy to ensure the queries don't get too large. This PR does update `UpdateLabelMembershipByHostIDs` slightly to return an updated Label object and host IDs array, as this is the expected return value for `NewLabel`. I update the method's tests accordingly. I don't think any new tests for `NewLabel` are needed as it should have the same functionality and return values. ## Manual Testing On the main branch, I launched my local MySQL with the thread stack size set to the minimal allowed, and used the API to try and create a new label with 5,000 hosts attached, and received a 422 response from the server. Server logs showed: ``` level=error ts=2025-01-28T15:08:20.465401Z component=http user=scott@fleetdm.com method=POST uri=/api/latest/fleet/labels took=16.610292ms err="get hostnames by identifiers: Error 1436 (HY000): Thread stack overrun: 111136 bytes used of a 131072 byte stack, and 20000 bytes needed. Use 'mysqld --thread_stack=#' to specify a bigger stack." ``` On this branch, I kept the same MySQL settings and tried my API request again and it was successful: <img width="776" alt="image" src="https://github.com/user-attachments/assets/c4f0f52b-4d09-457b-8096-4dd3a747b1f4" /> ## QA The script I used to create a new manual label with lots of hosts is at: https://gist.github.com/sgress454/84f12064c437da456c456e25c26d9069 To run it, first grab a bearer token from any API request by opening the network tab, clicking a Fleet API request, and in the headers tab scrolling down to Authorization: <img width="892" alt="image" src="https://github.com/user-attachments/assets/5680f3bf-8db8-469a-9f03-000b86622c04" /> (only take the part _after_ "Bearer") Then download the script from that gist and in its folder run: ``` NODE_TLS_REJECT_UNAUTHORIZED=0 node ./add_hosts_to_label.js <the bearer token> "<a label name>" ``` e.g. ``` NODE_TLS_REJECT_UNAUTHORIZED=0 node ./add_hosts_to_label.js U3HpbdtadmJXGKYSB0U/PbwfOpHbBt7FpkWmGKKYolOO1moLNZA6XxP+QO5LVukvAotZ7d+JbNUEEhYHZtxoqg== "some test label" ``` This will invoke the API on https://localhost:8080 and try to add 5000 hosts a new label "some test label". If you need to change the # of hosts or the url of the server, there are additional arguments: ``` NODE_TLS_REJECT_UNAUTHORIZED=0 node ./add_hosts_to_label.js <the bearer token> "<a label name>" <number of hosts> <url> ``` e.g. ``` NODE_TLS_REJECT_UNAUTHORIZED=0 node ./add_hosts_to_label.js U3HpbdtadmJXGKYSB0U/PbwfOpHbBt7FpkWmGKKYolOO1moLNZA6XxP+QO5LVukvAotZ7d+JbNUEEhYHZtxoqg== "some test label" 10000 https://foo.bar ```
1 line
109 B
Text
1 line
109 B
Text
- Updated the way new manual labels are created to better support adding large numbers of hosts at one time.
|