Add GitOps support for custom host vital definitions#49277
Merged
nulmete merged 5 commits intoJul 14, 2026
Conversation
Adds a declarative `custom_host_vitals:` GitOps key so fleetctl can create, update, and remove custom host vital definitions, and emits existing definitions via `fleetctl generate-gitops` for round-tripping.
Member
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
make mock wasn't re-run after moving the interface method's position in server/fleet/datastore.go, so the committed mock was stale.
Contributor
WalkthroughAdds global GitOps support for Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
server/service/client.go (1)
3082-3082: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSkip the API request when there are no changes to apply.
If
toDeleteandtoAddare both empty, the GitOps definitions already match the server's state perfectly. You can safely early-return here to avoid making an unnecessary PUT request that overwrites the vitals with identical data.♻️ Proposed refactor
} } + if len(toDelete) == 0 && len(toAdd) == 0 { + return nil + } + if dryRun { if len(toDelete) > 0 {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/service/client.go` at line 3082, Update the flow around the dryRun condition to return before the API request when both toDelete and toAdd are empty, preserving existing behavior when either list contains changes. Ensure the early return occurs before the PUT request and does not alter dry-run handling.pkg/spec/gitops.go (1)
1106-1113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCheck for duplicate names during parsing.
Currently, if the GitOps YAML contains duplicate custom host vital names, the parser accepts them, which will result in an unhelpful database constraint error when sent to the server. You can catch this early and provide a clearer error message by tracking seen names.
♻️ Proposed refactor
// Validate unknown keys in the custom_host_vitals section. multiError = multierror.Append(multiError, validateRawKeys(raw, reflect.TypeFor[[]GitOpsCustomHostVital](), filePath, []string{"custom_host_vitals"})...) + seenNames := make(map[string]bool) for _, v := range vitals { if err := fleet.ValidateCustomHostVitalName(v.Name); err != nil { multiError = multierror.Append(multiError, fmt.Errorf("'custom_host_vitals': %w", err)) continue } + if seenNames[v.Name] { + multiError = multierror.Append(multiError, fmt.Errorf("'custom_host_vitals': duplicate custom host vital %q", v.Name)) + continue + } + seenNames[v.Name] = true result.CustomHostVitals = append(result.CustomHostVitals, fleet.CustomHostVital{Name: v.Name}) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/spec/gitops.go` around lines 1106 - 1113, Update the custom host vitals parsing loop to track names already encountered and append a clear validation error when a duplicate is found. Perform the duplicate check alongside fleet.ValidateCustomHostVitalName before appending to result.CustomHostVitals, and only append unique valid vitals while preserving existing multiError handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/service/custom_host_vitals.go`:
- Around line 248-257: Update the duplicate detection in the custom host vital
validation loop around seen to use utf8mb4_unicode_ci-equivalent comparison or
normalization, matching the database unique index. Ensure collation-equivalent
names, including case-only differences, are rejected before insertion while
preserving the existing invalid-argument error behavior.
---
Nitpick comments:
In `@pkg/spec/gitops.go`:
- Around line 1106-1113: Update the custom host vitals parsing loop to track
names already encountered and append a clear validation error when a duplicate
is found. Perform the duplicate check alongside
fleet.ValidateCustomHostVitalName before appending to result.CustomHostVitals,
and only append unique valid vitals while preserving existing multiError
handling.
In `@server/service/client.go`:
- Line 3082: Update the flow around the dryRun condition to return before the
API request when both toDelete and toAdd are empty, preserving existing behavior
when either list contains changes. Ensure the early return occurs before the PUT
request and does not alter dry-run handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e86b262e-f6f2-448d-b27a-818fedda83f7
📒 Files selected for processing (20)
cmd/fleetctl/fleetctl/generate_gitops.gocmd/fleetctl/fleetctl/generate_gitops_test.gocmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_free/default.ymlcmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/default.ymlcmd/fleetctl/fleetctl/testing_utils/testing_utils.gocmd/fleetctl/integrationtest/gitops/gitops_integration_test.gopkg/spec/gitops.gopkg/spec/gitops_test.goserver/datastore/mysql/custom_host_vitals.goserver/datastore/mysql/custom_host_vitals_test.goserver/fleet/api_custom_host_vitals.goserver/fleet/datastore.goserver/fleet/service.goserver/mock/datastore_mock.goserver/mock/service/service_mock.goserver/service/client.goserver/service/client_custom_host_vitals.goserver/service/custom_host_vitals.goserver/service/custom_host_vitals_test.goserver/service/handler.go
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 44954-custom-host-vitals #49277 +/- ##
===========================================================
Coverage ? 68.02%
===========================================================
Files ? 3684
Lines ? 234523
Branches ? 12385
===========================================================
Hits ? 159532
Misses ? 60617
Partials ? 14374
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
doGitOpsCustomHostVitals unconditionally fetched existing definitions over the network, which ran before org_settings.integrations/CA validation and broke TestGitOpsErrors (a client with no auth token, used to test that local-only validation fails before any network call). Move the call to the end of the global config branch, after all local validation and the EULA network call.
custom_host_vitals.name is unique under utf8mb4_unicode_ci (case-insensitive) collation, but the service-level duplicate check used exact string comparison, so case-only duplicates would pass validation and surface as a raw DB error at insert time instead.
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
lucasmrod
reviewed
Jul 14, 2026
lucasmrod
left a comment
lucasmrod
left a comment
Member
There was a problem hiding this comment.
LGTM! Left two questions/comments.
| // rather than a directive to clear settings. settings keys were handled above. | ||
| if topKey == "name" || topKey == "labels" || topKey == "software" || topKey == "settings" || topKey == "org_settings" { | ||
| // "labels", "software", and "custom_host_vitals" are special cases where omitting may be a | ||
| // no-op (based on exception settings), rather than a directive to clear settings. settings |
Member
There was a problem hiding this comment.
Are custom host vitals in the exception settings for GitOps?
Member
Author
There was a problem hiding this comment.
No, I was wrong and copy-pasted from the labels/software case... custom host vitals have no exceptions.
It's excluded from the generic null-default loop because it needs its own presence tracking, not because of any exception mechanism (omitting the key always means clear-all).
I updated the comment to reflect that.
…st vitals - pkg/spec/gitops.go: custom host vitals have no GitOps exception setting (unlike labels/software) -- correct the comment that implied otherwise; it's excluded from the generic null-default loop only because it needs its own presence tracking. - server/service/client.go: dry-run returned early without ever calling the server, so server-side validation (name format, collation-aware duplicate check) was skipped on --dry-run and would only surface on a real run. Send the request with DryRun set so the server validates without persisting.
nulmete
added a commit
that referenced
this pull request
Jul 14, 2026
…tals section - Add "Upsert custom host vitals" to api-for-contributors.md, alongside the analogous PUT /spec/secret_variables entry, for the new declarative GitOps spec endpoint (added in #49277). - Fix nit from review: name parameter description referenced the wrong prefix (FLEET_SECRET_ instead of FLEET_HOST_VITAL_). - Remove a copy-pasted "value" parameter from Create custom host vital (the endpoint only accepts name; per-host values are set via a separate endpoint). - Fix invalid JSON (trailing commas) in two examples. - Fix "Update host's custom host vital value" missing the /api/v1/fleet path prefix used by every other endpoint in the doc.
lucasmrod
approved these changes
Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue: Resolves #48558
custom_host_vitalsGitOps key, lettingfleetctl gitopscreate, update, and remove custom host vital definitions.PUT /spec/custom_host_vitalsserver endpoint that reconciles the full set of definitions in one callfleetctl generate-gitopsto emit existing vitals so a GitOps repo can capture and round-trip this configuration.Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Will be added in the feature branch.
Testing
Added/updated automated tests
QA'd all new/changed functionality manually
Summary by CodeRabbit
custom_host_vitalssection in global YAML to declare, reconcile, and report custom host vitals.null), while omitting the key leaves current vitals unchanged.