[client] Move the MDM precedence into ResolveUploadURL

Three of the four bundle paths ordered the destinations themselves, each with
its own copy of "MDM first, then what was asked for". Give the resolver the MDM
value as its first argument so the order lives in one place and a call site
cannot express a different one.

Pure refactor: the daemon passes "" for now, matching what it does today, and
the remote job and both mobile SDKs resolve exactly as before.
This commit is contained in:
riccardom
2026-09-25 13:34:17 +02:00
parent c3993bef73
commit b3ee6c9af2
6 changed files with 52 additions and 24 deletions
+1 -1
View File
@@ -382,7 +382,7 @@ func (c *Client) DebugBundle(platformFiles PlatformFiles, anonymize bool, anonym
// An MDM override wins; otherwise the destination this deployment publishes
// is used, and failing that the service NetBird runs.
uploadURL := debug.ResolveUploadURL(cfg.DebugBundleUploadURL, publishedUploadURL)
uploadURL := debug.ResolveUploadURL(cfg.DebugBundleUploadURL, "", publishedUploadURL)
path, err := bundleGenerator.Generate()
if err != nil {
+26 -12
View File
@@ -4,19 +4,33 @@ import (
"github.com/netbirdio/netbird/upload-server/types"
)
// ResolveUploadURL decides where a debug bundle is uploaded.
// ResolveUploadURL decides where a debug bundle is uploaded, taking the first
// destination that is set:
//
// requested is a destination a caller named explicitly — an MDM override, the
// CLI's --upload-bundle-url, a remote job's upload_url; it always wins, and the
// callers that accept one gate it separately (see requirePrivilegeForUploadURL:
// any host other than the default needs a privileged caller). published is what
// the management server of this deployment advertises, which the engine holds
// (Engine.DebugUploadURL). With neither, the upload service NetBird runs is the
// default, for a self-hosted deployment as much as for a cloud one: an operator
// who needs the bundles to stay inside their own infrastructure points either
// knob at their own upload service, and until they do the everyday
// "collect a bundle and send it to support" flow keeps working.
func ResolveUploadURL(requested, published string) string {
// - mdm is the debugBundleUploadURL policy on this device. It outranks
// everything, including a URL the caller named: pinning the destination on a
// managed device is pointless if the person at the keyboard can send the
// bundle elsewhere.
// - requested is what this particular bundle asked for — the CLI's
// --upload-bundle-url, or a remote job's upload_url. The callers that accept
// one gate it separately (see requirePrivilegeForUploadURL: any host other
// than the default needs a privileged caller).
// - published is what the management server of this deployment advertises,
// which the engine holds (Engine.DebugUploadURL).
//
// With none of them, the upload service NetBird runs is the default, for a
// self-hosted deployment as much as for a cloud one: an operator who needs the
// bundles to stay inside their own infrastructure points one of the knobs at
// their own upload service, and until they do the everyday "collect a bundle and
// send it to support" flow keeps working.
//
// Every caller goes through here rather than ordering the sources itself, so a
// path cannot quietly skip one of them.
func ResolveUploadURL(mdm, requested, published string) string {
if mdm != "" {
return mdm
}
if requested != "" {
return requested
}
+18 -1
View File
@@ -14,12 +14,29 @@ func TestResolveUploadURL(t *testing.T) {
requestedURL = "https://requested.example.com/upload-url"
)
const mdmURL = "https://mdm.example.com/upload-url"
tests := []struct {
name string
mdm string
requested string
published string
want string
}{
{
// Pinning the destination on a managed device is pointless if the
// person at the keyboard can name another one.
name: "MDM outranks a URL the caller named",
mdm: mdmURL,
requested: requestedURL,
published: operatorURL,
want: mdmURL,
},
{
name: "MDM alone wins",
mdm: mdmURL,
want: mdmURL,
},
{
name: "requested wins over published",
requested: requestedURL,
@@ -47,7 +64,7 @@ func TestResolveUploadURL(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.want, ResolveUploadURL(tc.requested, tc.published))
assert.Equal(t, tc.want, ResolveUploadURL(tc.mdm, tc.requested, tc.published))
})
}
}
+5 -8
View File
@@ -1479,16 +1479,13 @@ func (e *Engine) handleBundle(params *mgmProto.BundleParameters) (*mgmProto.JobR
log.Warnf("get latest sync response: %v", err)
}
// Resolve the upload destination: an MDM override, when set, takes
// precedence over the job's URL. Both are validated the same way. With
// neither, the destination this deployment publishes is used, and failing
// that the service NetBird runs.
uploadURL := params.GetUploadUrl()
if override := e.config.ProfileConfig.DebugBundleUploadURL; override != "" {
// Resolve the upload destination: the MDM policy, then the job's URL, then
// what this deployment publishes, then the service NetBird runs.
mdmUploadURL := e.config.ProfileConfig.DebugBundleUploadURL
if mdmUploadURL != "" && params.GetUploadUrl() != "" && mdmUploadURL != params.GetUploadUrl() {
log.Infof("using MDM debug bundle upload URL override instead of the management-supplied value")
uploadURL = override
}
uploadURL = debug.ResolveUploadURL(uploadURL, e.DebugUploadURL())
uploadURL := debug.ResolveUploadURL(mdmUploadURL, params.GetUploadUrl(), e.DebugUploadURL())
// Validated after resolution, so the destination this deployment published
// meets the same rule as one named in the job. Management validates it at
+1 -1
View File
@@ -331,7 +331,7 @@ func (c *Client) DebugBundle(anonymize bool, anonymizeLevel string) (string, err
// An MDM override wins; otherwise the destination this deployment publishes
// is used, and failing that the service NetBird runs.
uploadURL := debug.ResolveUploadURL(cfg.DebugBundleUploadURL, publishedUploadURL)
uploadURL := debug.ResolveUploadURL(cfg.DebugBundleUploadURL, "", publishedUploadURL)
path, err := bundleGenerator.Generate()
if err != nil {
+1 -1
View File
@@ -49,7 +49,7 @@ func (s *Server) DebugBundle(callerCtx context.Context, req *proto.DebugBundleRe
// it is the operator of this deployment naming their own upload service, and
// the peer already trusts that server for its whole configuration. Only a
// URL the local caller named goes through requirePrivilegeForUploadURL above.
uploadURL := debug.ResolveUploadURL(req.GetUploadURL(), publishedUploadURL)
uploadURL := debug.ResolveUploadURL("", req.GetUploadURL(), publishedUploadURL)
// The upload runs without s.mutex held: it does network I/O to a possibly
// slow destination and must not block the other RPCs that take the lock. The