Add gitops endpoints to api_endpoints catalog#44291
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughExtends the Fleet API endpoint registry (server/api_endpoints/api_endpoints.yml) with ~25 new route declarations (PATCH/POST/GET/PUT/DELETE across fleet config, scripts batch, current-user, spec resources, ABM tokens, certificate authorities/templates, secret variables, reports/policies/fleets specs, policy listing/deletion, MDM profiles/bootstrap, software batch/app association/fleet-maintained apps, and software title icons). Fixes newline/EOF formatting for an existing configuration profile status entry. Adds two enterprise integration tests validating api-only access and endpoint-restriction behavior for GitOps-related routes. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #44291 +/- ##
==========================================
+ Coverage 66.78% 66.79% +0.01%
==========================================
Files 2630 2630
Lines 211232 211354 +122
Branches 9510 9547 +37
==========================================
+ Hits 141063 141181 +118
- Misses 57348 57349 +1
- Partials 12821 12824 +3
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:
|
lucasmrod
left a comment
There was a problem hiding this comment.
Left two comments to tackle.
| - method: "POST" | ||
| path: "/api/v1/fleet/spec/packs" | ||
| display_name: "Apply packs spec" |
There was a problem hiding this comment.
Double checking: Do we support setting the (deprecated) "user packs" in GitOps?
There was a problem hiding this comment.
I'll remove it
| - method: "POST" | ||
| path: "/api/v1/fleet/users/roles/spec" | ||
| display_name: "Apply user roles spec" |
There was a problem hiding this comment.
Let's remove this (for security purposes, to not allow these users to create users with more power).
GitOps doesn't use it (fleetctl apply uses it, but we don't need to support fleetctl apply for API-only users without API-endpoints.)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/service/integration_enterprise_test.go (1)
29153-29157: Add a timeout to the HTTP request helper.The
assertNot403helper useshttp.DefaultClient.Do(req)(line 29156) without a timeout. If the handler stalls, this test can hang indefinitely in CI. Usehttp.NewRequestWithContext(t.Context(), ...)instead ofhttp.NewRequest(...)to leverage the per-test timeout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/service/integration_enterprise_test.go` around lines 29153 - 29157, The test helper assertNot403 creates requests with http.NewRequest(...); change it to create the request with the test's context using http.NewRequestWithContext(t.Context(), verb, s.server.URL+path, bytes.NewReader(raw)) so the per-test timeout/cancellation is honored, then proceed to set the Authorization header and call http.DefaultClient.Do(req) as before; ensure you still check require.NoError(t, err) after creating the request and after doing the request.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/service/integration_enterprise_test.go`:
- Around line 29153-29157: The test helper assertNot403 creates requests with
http.NewRequest(...); change it to create the request with the test's context
using http.NewRequestWithContext(t.Context(), verb, s.server.URL+path,
bytes.NewReader(raw)) so the per-test timeout/cancellation is honored, then
proceed to set the Authorization header and call http.DefaultClient.Do(req) as
before; ensure you still check require.NoError(t, err) after creating the
request and after doing the request.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 94f2b901-8ae5-4d66-82b1-d52077d0c019
📒 Files selected for processing (2)
server/api_endpoints/api_endpoints.ymlserver/service/integration_enterprise_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- server/api_endpoints/api_endpoints.yml
Related issue: Resolves ##44279
Add gitops endpoints to api_endpoints catalog
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Testing
Summary by CodeRabbit
New Features
Tests