Skip to content

Changes to not rely on Fleet Desktop for Linux setup experience - #33018

Merged
lucasmrod merged 5 commits into
mainfrom
fix-linux-setup-experience-panic
Sep 16, 2025
Merged

Changes to not rely on Fleet Desktop for Linux setup experience#33018
lucasmrod merged 5 commits into
mainfrom
fix-linux-setup-experience-panic

Conversation

@lucasmrod

@lucasmrod lucasmrod commented Sep 15, 2025

Copy link
Copy Markdown
Member

For #32788.

Testing

  • Added/updated automated tests
  • QA'd all new/changed functionality manually

fleetd/orbit/Fleet Desktop

  • Verified compatibility with the latest released version of Fleet (see Must rule)
  • If the change applies to only one platform, confirmed that runtime.GOOS is used as needed to isolate changes
  • Verified that fleetd runs on macOS, Linux and Windows
  • Verified auto-update works from the released version of component to the new version (see tools/tuf/test)

Summary by CodeRabbit

  • New Features

    • Enhanced Linux setup experience: persists status on disk, resumes automatically, and completes when software/scripts finish.
    • Opens the “My Device” page only when desktop is enabled, using a user-aware launcher on Linux.
    • Linux setup status now focuses on software progress for faster, clearer feedback.
  • Bug Fixes

    • Corrected auth/MDM checks: macOS requires Apple MDM; Linux no longer blocked by MDM configuration on shared endpoints.
    • Improved reliability and logging around software installation and temporary directory cleanup.

func (svc *Service) GetOrbitSetupExperienceStatus(ctx context.Context, orbitNodeKey string, forceRelease bool) (*fleet.SetupExperienceStatusPayload, error) {
// this is not a user-authenticated endpoint
svc.authz.SkipAuthorization(ctx)
host, err := svc.ds.LoadHostByOrbitNodeKey(ctx, orbitNodeKey)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The auth middleware already sets the host in the context, so this is saving an unnecessary DB read.

Comment thread orbit/cmd/orbit/orbit.go
Msg("checking setup experience preflight values")

openMyDevicePage := func() error {
if !c.Bool("fleet-desktop") {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this because orbit was panicking when building fleetd without --fleet-desktop (trw.Read was panicking below because trw is nil when there's no Fleet Desktop)

Comment thread orbit/cmd/orbit/orbit.go
go sigusrListener(c.String("root-dir"))

isLinux := runtime.GOOS == "linux"
serverHasWebSetup := orbitClient.GetServerCapabilities().Has(fleet.CapabilityWebSetupExperience)

@lucasmrod lucasmrod Sep 15, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this check to processSetupExperience so that if a host is setting up and the Fleet server is old, then we will just write the file to not try on next restart.

Comment thread server/service/handler.go
oeAppleMDM := oe.WithCustomMiddleware(mdmConfiguredMiddleware.VerifyAppleMDM())
// POST /api/fleet/orbit/setup_experience/status is used by macOS and Linux hosts.
// For macOS hosts we verify Apple MDM is enabled and configured.
oeAppleMDM := oe.WithCustomMiddlewareAfterAuth(mdmConfiguredMiddleware.VerifyAppleMDMOnMacOSHosts())

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the new WithCustomMiddlewareAfterAuth because the MDM check depends on the host's platform (so authentication needs to happen before the MDM check)

@codecov

codecov Bot commented Sep 15, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 32.54438% with 114 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.79%. Comparing base (587eef0) to head (09e8268).
⚠️ Report is 23 commits behind head on main.

Files with missing lines Patch % Lines
orbit/cmd/orbit/orbit.go 0.00% 62 Missing ⚠️
orbit/pkg/setup_experience/setup_experience.go 29.82% 37 Missing and 3 partials ⚠️
ee/server/service/orbit.go 50.00% 5 Missing and 2 partials ⚠️
.../service/middleware/mdmconfigured/mdmconfigured.go 75.00% 2 Missing and 1 partial ⚠️
orbit/pkg/installer/installer.go 33.33% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #33018    +/-   ##
========================================
  Coverage   63.79%   63.79%            
========================================
  Files        2044     2044            
  Lines      201367   201494   +127     
  Branches     6686     6686            
========================================
+ Hits       128465   128553    +88     
- Misses      62741    62780    +39     
  Partials    10161    10161            
Flag Coverage Δ
backend 64.97% <32.54%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lucasmrod

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 15, 2025

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Sep 15, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Refactors setup-experience status retrieval to use host-from-context and a shared helper; adds Linux-specific setup-experience handling in Orbit and a new public setupexperience package with persistent state; introduces after-auth middleware support and Apple MDM verification limited to macOS; updates tests to exercise Orbit-auth-first behavior and Linux flows.

Changes

Cohort / File(s) Summary
Service: host setup helper
ee/server/service/devices.go
Extracts getHostSetupExperienceStatus helper; GetDeviceSetupExperienceStatus delegates; logic for gathering software results and advancing steps moved into helper with wrapped errors.
Service: Orbit setup status/init flow
ee/server/service/orbit.go
Uses host from context; returns auth-required error if missing; for Linux, returns Software-only via helper; non-Linux retains full aggregation; SetupExperienceInit also uses host-from-context.
Orbit CLI: setup-experience integration
orbit/cmd/orbit/orbit.go
Removes server capability gating for preflight; switches to setupexperience package for persistence; processSetupExperience signature now takes rootDir; adds LinuxSetupExperiencer registration; My Device opener gated by fleet-desktop and runs as GUI user on Linux.
Orbit constants/installer minor
orbit/pkg/constant/constant.go, orbit/pkg/installer/installer.go
Formatting newline in consts; logging tweaks; file mode literal to 0o700; no logic changes.
New package: setup_experience (public API)
orbit/pkg/setup_experience/setup_experience.go, orbit/pkg/setup_experience/setup_experience_test.go
Adds LinuxSetupExperiencer with Run method; introduces SetupExperienceInfo {TimeInitiated, Enabled, TimeFinished}; adds Read/Write JSON status file helpers; tests verify persistence and transitions.
Fleet platform helper
server/fleet/hosts.go
Adds IsApplePlatform(hostPlatform string) for darwin/ios/ipados checks.
HTTP middleware framework updates
server/service/middleware/endpoint_utils/endpoint_utils.go, .../endpoint_utils_test.go
Adds after-auth middleware support and Versions field in CommonEndpointer; new WithCustomMiddlewareAfterAuth; tests verify execution order around auth.
MDM-configured middleware (Apple on macOS only)
server/service/middleware/mdmconfigured/mdmconfigured.go
Adds VerifyAppleMDMOnMacOSHosts: fetch host from context; if Apple platform, enforce VerifyMDMAppleConfigured; otherwise bypass.
Service handler wiring
server/service/handler.go
Applies VerifyAppleMDMOnMacOSHosts after auth for Orbit setup status endpoint; switches to WithCustomMiddlewareAfterAuth; comments updated.
Integration tests: core/enterprise
server/service/integration_core_test.go, server/service/integration_enterprise_test.go
Ensure Orbit enrollment and pass OrbitNodeKey body for status POST; add test to assert auth-before-MDM behavior for macOS vs Linux.
Integration test: Linux setup-experience E2E
server/service/integration_mdm_setup_experience_test.go
Adds Linux-without-desktop E2E covering software installers through success and DeviceConfigured command issuance.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Orbit
  participant Router as Router
  participant EP as Endpointer
  participant Auth as Auth Middleware
  participant After as After-Auth MDM Check
  participant SVC as GetOrbitSetupExperienceStatus

  Orbit->>Router: POST /api/fleet/orbit/setup_experience/status
  Router->>EP: route match
  EP->>Auth: wrap endpoint with auth
  Auth->>After: on success, call after-auth middleware
  After->>SVC: pass context (host in context)
  alt Linux host
    SVC->>SVC: svc.getHostSetupExperienceStatus(host)
    SVC-->>Orbit: { Software: [...] }
  else non-Linux host (macOS/Windows)
    SVC->>SVC: aggregate Bootstrap/Profiles/Account/Software
    SVC-->>Orbit: full SetupExperienceStatusPayload
  end
Loading
sequenceDiagram
  autonumber
  participant Orbitd as Orbit daemon
  participant SE as LinuxSetupExperiencer
  participant File as Status File (JSON)
  participant API as OrbitClient
  participant Cfg as Config Update

  Orbitd->>SE: Register on config updates
  Cfg-->>SE: Run(*OrbitConfig)
  SE->>File: ReadSetupExperienceStatusFile(rootDir)
  alt status missing
    SE-->>Orbitd: no-op (wait for init)
  else status present and Enabled
    SE->>API: GetOrbitSetupExperienceStatus
    API-->>SE: { Software/Script statuses }
    SE->>SE: check setupExperienceDone()
    alt done
      SE->>File: WriteSetupExperienceStatusFile(TimeFinished=now)
      SE-->>Orbitd: completed
    else not done
      SE-->>Orbitd: continue waiting
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • lukeheath
  • dantecatalfamo
  • rachaelshaw

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ❓ Inconclusive The PR description includes a brief issue reference and populated checklist entries under "Testing" and "fleetd/orbit/Fleet Desktop," but it omits other sections from the repository template such as the top-level "Checklist for submitter" items (changes file, input validation, endpoint/path changes), Database migrations, and New Fleet configuration settings, so it is unclear whether those items were intentionally omitted or not applicable. Because the template expects either those lines to be present and filled or explicitly removed/annotated, the current description is inconclusive against the required template structure. Please update the PR description to follow the repository template by either adding the missing checklist sections or explicitly deleting them if not applicable; specifically state whether a user-visible changes file was added (path), confirm input/SQL validation and any endpoint/path changes, and indicate whether any database migrations or new Fleet configuration settings are required or intentionally absent.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely summarizes the primary intent of the changes — removing reliance on Fleet Desktop for the Linux setup experience — and aligns with the main edits in orbit and setupexperience that remove fleet-desktop gating and add Linux-specific flows; it is specific, not noisy, and easy for teammates scanning history to understand.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-linux-setup-experience-panic

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (1)
orbit/cmd/orbit/orbit.go (1)

1562-1565: Good guard to avoid nil token access when Desktop is disabled.

Prevents the panic noted in prior discussion.

🧹 Nitpick comments (20)
orbit/pkg/installer/installer.go (1)

322-351: Potential working-directory bug when installerPath is a directory (after .tar.gz extraction).

After extraction, installerPath is set to the extraction directory. In runInstallerScript, using filepath.Dir(installerPath) will place the script one level above the extracted contents, contradicting the “run script in installer directory” intent and potentially breaking scripts that expect CWD inside the extracted dir.

Apply this refactor in runInstallerScript to ensure the script is created/executed in the installer directory (file or dir):

-	installerDir := filepath.Dir(installerPath)
-	scriptPath := filepath.Join(installerDir, fileName)
+	installerDir := installerPath
+	if fi, err := os.Stat(installerPath); err == nil && !fi.IsDir() {
+		installerDir = filepath.Dir(installerPath)
+	}
+	scriptPath := filepath.Join(installerDir, fileName)

If scripts.ExecCmd sets the working dir implicitly from scriptPath, this also ensures execution occurs within the extracted tree.

ee/server/service/devices.go (1)

270-304: Helper logic looks sound; continues next-step and scopes to Software results.

Error wrapping is consistent. Consider adding a test asserting multiple canceled installs are marked failed and next-step is advanced.

Would you like me to draft a unit test that seeds mixed results (success/failed/canceled) and validates the returned payload plus NextStep advancement?

server/service/middleware/endpoint_utils/endpoint_utils_test.go (3)

3-9: Remove noisy printf; use test logs or drop entirely.

The handler isn’t asserted and printing to stdout is noisy in CI.

Apply this diff:

-import (
-	"context"
-	"fmt"
+import (
+	"context"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@
-	ce.handleEndpoint("/", func(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) {
-		fmt.Printf("handler\n")
-		return nopResponse{}, nil
-	}, nil, "GET")
+	ce.handleEndpoint("/", func(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) {
+		return nopResponse{}, nil
+	}, nil, "GET")

Also applies to: 74-76


73-77: The passed handler is never invoked by nopEP; clarify intent or invoke it.

nopEP.CallHandlerFunc ignores f, so the handler body isn’t exercised (your printf would never run). If that’s intentional, fine—then keep the handler trivial. If not, prefer calling f to increase coverage.

For example (only if a stub fleet.Service is available):

 func (n nopEP) CallHandlerFunc(f HandlerFunc, ctx context.Context, request interface{}, svc interface{}) (fleet.Errorer, error) {
-	return nopResponse{}, nil
+	if f == nil {
+		return nopResponse{}, nil
+	}
+	// If svc implements fleet.Service, pass it through; otherwise fall back.
+	if fs, ok := svc.(fleet.Service); ok {
+		return f(ctx, request, fs)
+	}
+	return f(ctx, request, nil)
 }

If wiring a stub service is out of scope, keeping the no-op is acceptable—just don’t rely on handler side effects in this test.

Also applies to: 104-112


44-50: Optional: Assert auth context presence in after-auth middleware.

This strengthens the guarantee that “after-auth” runs with an auth context attached.

Apply this minimal check:

 	afterAuthMiddleware := func(next endpoint.Endpoint) endpoint.Endpoint {
 		return func(ctx context.Context, req interface{}) (interface{}, error) {
 			i++
 			afterIndex = i
+			if _, ok := authz_ctx.FromContext(ctx); !ok {
+				return nil, fmt.Errorf("auth context missing in after-auth middleware")
+			}
 			return next(ctx, req)
 		}
 	}

If you’d rather keep the test pure-require style, replace the return with require.True(t, ok, "auth context missing") and import hygiene accordingly.

Also applies to: 68-70

orbit/pkg/setup_experience/setup_experience_test.go (1)

37-44: Optional: Assert temporal consistency (finished >= initiated).

Protects against clock skew/serialization bugs.

Apply this diff:

 	require.NotNil(t, s.TimeFinished)
 	require.NotZero(t, *s.TimeFinished)
+	require.False(t, s.TimeFinished.Before(s.TimeInitiated), "finished time should not be before initiated time")
server/service/integration_core_test.go (2)

10036-10039: Assert OrbitNodeKey is set to avoid a panic later.

h.OrbitNodeKey is dereferenced below when creating the request body. Add an assertion to fail fast if enrollment ever stops setting it.

 h := createHostAndDeviceToken(t, s.ds, tkn)
 orbitKey := setOrbitEnrollment(t, h, s.ds)
 h.OrbitNodeKey = &orbitKey
+require.NotNil(t, h.OrbitNodeKey, "OrbitNodeKey must be set after enrollment")

10056-10063: Isolate per-route bodies to a helper to reduce brittle string checks.

Hardcoding the path in-line is easy to miss when new routes need bodies. A tiny helper keeps this centralized and adds the nil‑safety check.

- var params interface{}
- if route.method == "POST" && route.path == "/api/fleet/orbit/setup_experience/status" {
-   params = getOrbitSetupExperienceStatusRequest{
-     OrbitNodeKey: *h.OrbitNodeKey,
-   }
- }
- res := s.Do(route.method, path, params, expectedErr.StatusCode())
+ params := mdmRouteBody(t, route.method, route.path, h)
+ res := s.Do(route.method, path, params, expectedErr.StatusCode())

Add this helper in the same file:

func mdmRouteBody(t *testing.T, method, path string, h *fleet.Host) interface{} {
	switch {
	case method == "POST" && path == "/api/fleet/orbit/setup_experience/status":
		require.NotNil(t, h.OrbitNodeKey)
		return getOrbitSetupExperienceStatusRequest{OrbitNodeKey: *h.OrbitNodeKey}
	default:
		return nil
	}
}
server/service/middleware/mdmconfigured/mdmconfigured.go (2)

46-64: Name says “MacOS” but code gates on all Apple platforms.

Either constrain to macOS or rename for accuracy.

Option A (keep behavior, fix name/comment):

-// VerifyAppleMDMOnMacOSHosts verifies that MDM is enabled and configured when it's an Apple host making the request.
+// VerifyAppleMDMOnAppleHosts verifies that MDM is enabled and configured when it's an Apple host making the request.
-func (m *Middleware) VerifyAppleMDMOnMacOSHosts() endpoint.Middleware {
+func (m *Middleware) VerifyAppleMDMOnAppleHosts() endpoint.Middleware {

And adjust the call site in server/service/handler.go:

-oeAppleMDM := oe.WithCustomMiddlewareAfterAuth(mdmConfiguredMiddleware.VerifyAppleMDMOnMacOSHosts())
+oeAppleMDM := oe.WithCustomMiddlewareAfterAuth(mdmConfiguredMiddleware.VerifyAppleMDMOnAppleHosts())

Option B (keep name, constrain behavior to macOS only):

- if fleet.IsApplePlatform(host.Platform) {
+ if host.Platform == "darwin" {

51-55: Inconsistent error semantics for missing host in context.

Here you return an AuthRequired error, while SetupExperienceInit uses ctxerr.New (500). Pick one approach and apply consistently across orbit endpoints.

Would you prefer 401 (AuthRequired) or 500 (internal) when host context is unexpectedly missing post‑auth?

ee/server/service/orbit.go (2)

25-33: Linux early return: confirm step advancement and payload parity.

  • Verify svc.getHostSetupExperienceStatus advances steps (parity with macOS path’s SetupExperienceNextStep).
  • Consider including OrgLogoURL for UI parity if the Orbit UI expects it.

If you want OrgLogoURL for Linux too, this minimal change works:

 if fleet.IsLinux(host.Platform) {
-    status, err := svc.getHostSetupExperienceStatus(ctx, host)
+    status, err := svc.getHostSetupExperienceStatus(ctx, host)
     if err != nil {
         return nil, ctxerr.Wrap(ctx, err, "get host setup experience status")
     }
-    return &fleet.SetupExperienceStatusPayload{
-        Software: status.Software,
-    }, nil
+    appCfg, err := svc.ds.AppConfig(ctx)
+    if err != nil {
+        return nil, ctxerr.Wrap(ctx, err, "getting app config")
+    }
+    return &fleet.SetupExperienceStatusPayload{
+        Software:   status.Software,
+        OrgLogoURL: appCfg.OrgInfo.OrgLogoURLLightBackground,
+    }, nil
 }

307-311: Align missing-host error with the status endpoint.

Status uses AuthRequired on missing host; init uses a 500. Recommend choosing one style for both.

- if !ok {
-     return nil, ctxerr.New(ctx, "internal error: missing host from request context")
- }
+ if !ok {
+     return nil, ctxerr.Wrap(ctx, fleet.NewAuthRequiredError("internal error: missing host from request context"))
+ }
server/service/integration_mdm_setup_experience_test.go (3)

2418-2440: Unneeded device auth token in an Orbit-only test.

This test only hits Orbit endpoints; the device token setup can be dropped to reduce noise.

-        err = s.ds.SetOrUpdateDeviceAuthToken(ctx, host.ID, "fleet-desktop-token-"+hostPlatform)
-        require.NoError(t, err)

2511-2521: Duplicate assertion.

require.NotNil(t, orbitRes.Results) appears twice.

-    require.NotNil(t, orbitRes.Results)
-    require.NotNil(t, orbitRes.Results)
+    require.NotNil(t, orbitRes.Results)

2535-2546: Duplicate assertion.

Second duplicate of require.NotNil(t, orbitRes.Results).

-    require.NotNil(t, orbitRes.Results)
-    require.NotNil(t, orbitRes.Results)
+    require.NotNil(t, orbitRes.Results)
server/service/handler.go (1)

944-947: If you rename the middleware, update call site.

Follow-up to naming fix suggested in mdmconfigured.go.

-oeAppleMDM := oe.WithCustomMiddlewareAfterAuth(mdmConfiguredMiddleware.VerifyAppleMDMOnMacOSHosts())
+oeAppleMDM := oe.WithCustomMiddlewareAfterAuth(mdmConfiguredMiddleware.VerifyAppleMDMOnAppleHosts())
server/service/integration_enterprise_test.go (1)

5052-5058: Make route match resilient to path formatting

The equality check uses route.path, but the request is sent with path (which may be formatted). Compare against path to avoid brittleness.

-        if route.method == "POST" && route.path == "/api/fleet/orbit/setup_experience/status" {
+        if route.method == "POST" && path == "/api/fleet/orbit/setup_experience/status" {
orbit/pkg/setup_experience/setup_experience.go (2)

360-364: Nit: typo in comment.

s/ununsed/unused/.

Apply this diff:

-// Currently the fleet.OrbitConfig is ununsed but might be used in the future.
+// Currently the fleet.OrbitConfig is unused but might be used in the future.

378-382: Use UTC when persisting timestamps.

Aligns with server logs and cross‑TZ analysis.

Apply this diff:

-        info.TimeFinished = ptr.Time(time.Now())
+        info.TimeFinished = ptr.Time(time.Now().UTC())
orbit/cmd/orbit/orbit.go (1)

1645-1649: Use UTC for persisted initiation timestamps.

Consistency with server-side and other logs.

Apply this diff:

-      initTime := time.Now()
+      initTime := time.Now().UTC()
-    initTime := time.Now()
+    initTime := time.Now().UTC()

Also applies to: 1668-1676

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7412e52 and dc102f8.

📒 Files selected for processing (15)
  • ee/server/service/devices.go (1 hunks)
  • ee/server/service/orbit.go (1 hunks)
  • orbit/cmd/orbit/orbit.go (3 hunks)
  • orbit/pkg/constant/constant.go (1 hunks)
  • orbit/pkg/installer/installer.go (3 hunks)
  • orbit/pkg/setup_experience/setup_experience.go (2 hunks)
  • orbit/pkg/setup_experience/setup_experience_test.go (1 hunks)
  • server/fleet/hosts.go (1 hunks)
  • server/service/handler.go (1 hunks)
  • server/service/integration_core_test.go (2 hunks)
  • server/service/integration_enterprise_test.go (3 hunks)
  • server/service/integration_mdm_setup_experience_test.go (1 hunks)
  • server/service/middleware/endpoint_utils/endpoint_utils.go (3 hunks)
  • server/service/middleware/endpoint_utils/endpoint_utils_test.go (1 hunks)
  • server/service/middleware/mdmconfigured/mdmconfigured.go (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go

⚙️ CodeRabbit configuration file

When reviewing SQL queries that are added or modified, ensure that appropriate filtering criteria are applied—especially when a query is intended to return data for a specific entity (e.g., a single host). Check for missing WHERE clauses or incorrect filtering that could lead to incorrect or non-deterministic results (e.g., returning the first row instead of the correct one). Flag any queries that may return unintended results due to lack of precise scoping.

Files:

  • server/fleet/hosts.go
  • orbit/pkg/constant/constant.go
  • orbit/pkg/setup_experience/setup_experience_test.go
  • server/service/middleware/mdmconfigured/mdmconfigured.go
  • server/service/integration_core_test.go
  • server/service/middleware/endpoint_utils/endpoint_utils_test.go
  • server/service/integration_mdm_setup_experience_test.go
  • ee/server/service/devices.go
  • orbit/pkg/installer/installer.go
  • orbit/pkg/setup_experience/setup_experience.go
  • server/service/integration_enterprise_test.go
  • server/service/middleware/endpoint_utils/endpoint_utils.go
  • ee/server/service/orbit.go
  • server/service/handler.go
  • orbit/cmd/orbit/orbit.go
🔇 Additional comments (19)
orbit/pkg/constant/constant.go (1)

80-80: No functional change; safe to merge.

Whitespace-only change in const block. No impact.

server/fleet/hosts.go (1)

1027-1029: Helper reads clearly and matches existing platform constants.

Covers "darwin", "ios", "ipados" as expected. Call sites can now avoid repeating literals.

orbit/pkg/installer/installer.go (3)

132-133: Logging change is fine.

log.Error().Err(err)... is equivalent to the prior log.Err(err)... at error level.


267-270: Cleanup logging improvement LGTM.

More explicit message; no behavior change.


325-325: Octal literal update is correct.

0o700 is the idiomatic Go octal form; same permissions as before.

ee/server/service/devices.go (1)

267-269: Good split: thin wrapper delegates to host-scoped helper.

Reduces duplication and clarifies responsibility.

server/service/middleware/endpoint_utils/endpoint_utils.go (2)

515-527: API surface additions LGTM.

Versions and CustomMiddlewareAfterAuth fields plus comments read clearly.


631-635: Fluent builder reads well.

Method name and placement are consistent with existing WithCustomMiddleware.

orbit/pkg/setup_experience/setup_experience_test.go (1)

11-44: LGTM: solid round‑trip persistence test.

Covers first-run (no file), initial write, and subsequent update with finished time. Assertions look good.

server/service/integration_core_test.go (1)

10056-10061: Good fix: provide Orbit context for setup experience status.

Supplying OrbitNodeKey aligns the test with the endpoint’s host-from-context requirement and prevents false negatives.

ee/server/service/orbit.go (1)

19-24: LGTM: host-from-context avoids extra DB work.

Leverages auth middleware’s host injection; nice simplification.

server/service/handler.go (2)

944-947: LGTM: post-auth Apple MDM check for shared macOS/Linux endpoint.

Appropriate use of WithCustomMiddlewareAfterAuth so platform can be determined first.


549-555: Ensure mdmConfigurationRequiredEndpoints stays accurate.

POST /api/fleet/orbit/setup_experience/status is already listed in mdmConfigurationRequiredEndpoints (server/service/testing_utils.go:843). If you also moved MDM gating to the device token endpoint (/api/version/fleet/device/{token}/setup_experience/status or /api/v1/fleet/device/{token}/setup_experience/status), add that canonical path to the list.

server/service/integration_enterprise_test.go (2)

5036-5038: No change required — setOrbitEnrollment persists the Orbit node key

setOrbitEnrollment calls ds.EnrollOrbit (persisting the mapping) and handlers authenticate/load hosts via DS lookups (e.g. LoadHostByOrbitNodeKey / AuthenticateOrbitHost); h.OrbitNodeKey = &orbitKey is a test convenience only.


19439-19456: Ignore OS identifier concern — 'ubuntu' is acceptable; add negative-auth subtest

createOrbitEnrolledHost assigns Platform directly and the repo already uses distro strings (e.g., "ubuntu", "debian", "rhel", "linuxmint") alongside "linux", so using "ubuntu" is valid.

  • Add a subtest in TestOrbitSetupExperienceStatusChecksAuthBeforeMDM (server/service/integration_enterprise_test.go) that POSTs to /api/fleet/orbit/setup_experience/status with an invalid/unknown OrbitNodeKey and asserts a 401 or 403 to verify auth is enforced before MDM checks.

Likely an incorrect or invalid review comment.

orbit/pkg/setup_experience/setup_experience.go (2)

5-5: Imports look good.

New deps are used and scoped appropriately.

Also applies to: 9-9, 12-12, 16-16


346-358: Solid Linux separation.

Clear constructor and state; no concurrency exposure.

orbit/cmd/orbit/orbit.go (2)

1555-1555: Gate setup experience by OS and flag.

Simple and correct.


1609-1611: Updated callsite looks correct.

Matches new signature with rootDir and callback.

Comment thread orbit/pkg/setup_experience/setup_experience.go
Comment thread orbit/pkg/setup_experience/setup_experience.go
Comment thread orbit/pkg/setup_experience/setup_experience.go
Comment thread server/service/middleware/endpoint_utils/endpoint_utils_test.go
Comment thread server/service/middleware/endpoint_utils/endpoint_utils.go
Comment thread orbit/cmd/orbit/orbit.go
switch {
case err == nil:
// OK, continue
case errors.Is(err, service.ErrMissingLicense):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to store the file with enabled false on Fleet Free, to try only once.

@dantecatalfamo dantecatalfamo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'm just going to have to rebase the windows code into this after it merges

@lucasmrod
lucasmrod merged commit 330a708 into main Sep 16, 2025
72 of 74 checks passed
@lucasmrod
lucasmrod deleted the fix-linux-setup-experience-panic branch September 16, 2025 16:26
lucasmrod added a commit that referenced this pull request Sep 16, 2025
For #32788.

## Testing

- [X] Added/updated automated tests
- [X] QA'd all new/changed functionality manually

## fleetd/orbit/Fleet Desktop

- [X] Verified compatibility with the latest released version of Fleet
(see [Must
rule](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/workflows/fleetd-development-and-release-strategy.md))
- [X] If the change applies to only one platform, confirmed that
`runtime.GOOS` is used as needed to isolate changes
- [x] Verified that fleetd runs on macOS, Linux and Windows
- [X] Verified auto-update works from the released version of component
to the new version (see [tools/tuf/test](../tools/tuf/test/README.md))


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- New Features
- Enhanced Linux setup experience: persists status on disk, resumes
automatically, and completes when software/scripts finish.
- Opens the “My Device” page only when desktop is enabled, using a
user-aware launcher on Linux.
- Linux setup status now focuses on software progress for faster,
clearer feedback.

- Bug Fixes
- Corrected auth/MDM checks: macOS requires Apple MDM; Linux no longer
blocked by MDM configuration on shared endpoints.
- Improved reliability and logging around software installation and
temporary directory cleanup.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
lucasmrod added a commit that referenced this pull request Sep 16, 2025
lucasmrod added a commit that referenced this pull request Sep 24, 2025
For #32788.

## Testing

- [X] Added/updated automated tests
- [X] QA'd all new/changed functionality manually

## fleetd/orbit/Fleet Desktop

- [X] Verified compatibility with the latest released version of Fleet
(see [Must
rule](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/workflows/fleetd-development-and-release-strategy.md))
- [X] If the change applies to only one platform, confirmed that
`runtime.GOOS` is used as needed to isolate changes
- [x] Verified that fleetd runs on macOS, Linux and Windows
- [X] Verified auto-update works from the released version of component
to the new version (see [tools/tuf/test](../tools/tuf/test/README.md))


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- New Features
- Enhanced Linux setup experience: persists status on disk, resumes
automatically, and completes when software/scripts finish.
- Opens the “My Device” page only when desktop is enabled, using a
user-aware launcher on Linux.
- Linux setup status now focuses on software progress for faster,
clearer feedback.

- Bug Fixes
- Corrected auth/MDM checks: macOS requires Apple MDM; Linux no longer
blocked by MDM configuration on shared endpoints.
- Improved reliability and logging around software installation and
temporary directory cleanup.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
lucasmrod added a commit that referenced this pull request Sep 24, 2025
…Linux setup experience (#33018) (#33409)

Cherry-pick for #32788 (unreleased bug in 1.48.0).

Server side code changes don't matter because this patch branch will be
used to release fleetd.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants