Conversation
| UUID: strconv.Itoa(i), | ||
| Hostname: strconv.Itoa(i), | ||
| UUID: fmt.Sprintf("uuid%s", strconv.Itoa(i)), | ||
| Hostname: fmt.Sprintf("host%s", strconv.Itoa(i)), |
There was a problem hiding this comment.
This test was always a bit wonky in that it used numbers for the UUID and Hostname as well. Since we added ID to the mix of things you can specify for label membership in GitOps, it led to weird results where if you said "I want hosts 1, 2, and 3" you'd get a label with hosts 1, 2, 3 and 4 because host #4 had a UUID and hostname of "3". Changing these to be strings clears up that confusion in the test.
| // specifying "1" will match both host with ID 1 (whose name is "0") | ||
| // and host with name "1". | ||
| expectedSpecs[4].Hosts = []string{"0", "1", "2", "3", "4"} | ||
| expectedSpecs[4].Hosts = []string{"1", "2", "3", "4"} |
There was a problem hiding this comment.
see comment above
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #34254 +/- ##
==========================================
+ Coverage 64.23% 64.24% +0.01%
==========================================
Files 2058 2059 +1
Lines 207205 207095 -110
Branches 6923 6923
==========================================
- Hits 133103 133055 -48
+ Misses 63648 63606 -42
+ Partials 10454 10434 -20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| func (ds *Datastore) getLabelHostIDs(ctx context.Context, label *fleet.LabelSpec) error { | ||
| sql := ` | ||
| SELECT hostname | ||
| SELECT id |
There was a problem hiding this comment.
The only real functional change. This method is only used by the /spec/labels endpoint and by fleetctl generate-gitops. I posted about the API change in Slack to get @rachaelshaw's feedback but I don't think it should be cause for concern.
sgress454
deleted the
sgress454/34225-export-manual-host-ids-instead-of-names
branch
…ual labels (#34254) <!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #34225 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [X] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) ## Testing - [X] Added/updated automated tests - [X] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [X] QA'd all new/changed functionality manually Did a `generate-gitops` for a manual label, noted the correct IDs were output for hosts. Use `gitops` to re-apply the label, saw the label membership was applied correctly. For unreleased bug fixes in a release candidate, one of: - [X] Confirmed that the fix is not expected to adversely impact load test results - [ ] Alerted the release DRI if additional load testing is needed (cherry picked from commit 45a8749)
2 participants
Related issue: Resolves #34225
Checklist for submitter
If some of the following don't apply, delete the relevant line.
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements)Testing
Added/updated automated tests
Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)
QA'd all new/changed functionality manually
Did a
generate-gitopsfor a manual label, noted the correct IDs were output for hosts. Usegitopsto re-apply the label, saw the label membership was applied correctly.For unreleased bug fixes in a release candidate, one of: