From 97b9a18ef69182f029fadd27181ddcff3b5fc574 Mon Sep 17 00:00:00 2001 From: riccardom Date: Tue, 8 Sep 2026 14:03:15 +0200 Subject: [PATCH] [client] Resolve the merge conflicts left in the tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 262ce8c3b landed with the conflict markers still in it, so client/server and the iOS SDK did not compile. Four regions, resolved as follows. client/server/mdm.go — main moved the MDM conflict-check machinery into the mdm package (mdm.ResolveConflicts, mdm.ConflictBool, mdm.ConflictURL, ...). This branch had edited the local copies, which are now dead: dropped, along with the profilemanager import that only the local conflictURL needed. client/server/server.go, Login gate — this branch's value-aware gate stays (the point of the PR: refuse a real divergence, let a restatement through), so main's presence-based `loginRequestHasConfigOverrides` block goes; that helper no longer exists here anyway. Main's other change in the same lines is real and kept: the MDM policy now comes from the daemon-owned s.mdmLoader.Load() instead of the package-level loadMDMPolicy, which main removed. The stale call right below the conflict was the reason the file would not have compiled even with the markers gone. client/server/server.go, getConfig — both sides add something and both are needed. The identity is provisioned and persisted first, then the MDM overlay is applied, so what reaches disk stays the profile's own config: the overlay is runtime-only and re-derived on every load. client/ios/NetBirdSDK/client.go — main reworked SetConfigFromJSON to store the JSON and re-parse it on each load, which is the shape kept; the parse is now only a validity check, and this branch's reason for it (a document with no peer identity is refused, not just an unparseable one) moves into that comment. client/server/update_settings_gate_test.go — follows the sentinel constant to its new home, mdm.PreSharedKeyRedactedSentinel. --- client/ios/NetBirdSDK/client.go | 16 ++- client/server/mdm.go | 119 --------------------- client/server/server.go | 26 ++--- client/server/update_settings_gate_test.go | 3 +- 4 files changed, 15 insertions(+), 149 deletions(-) diff --git a/client/ios/NetBirdSDK/client.go b/client/ios/NetBirdSDK/client.go index 91d56d328..c24366e7f 100644 --- a/client/ios/NetBirdSDK/client.go +++ b/client/ios/NetBirdSDK/client.go @@ -130,17 +130,13 @@ func NewClient(cfgFile, stateFile, cacheDir, logFilePath, deviceName string, osV // SetConfigFromJSON stores the JSON config that later loads resolve instead of the config file (tvOS). func (c *Client) SetConfigFromJSON(jsonStr string) error { -<<<<<<< HEAD - cfg, err := profilemanager.ConfigFromJSON(jsonStr) - if err != nil { - // Not only a parse error any more: a document with no peer identity is - // refused, because Run() would otherwise connect as a peer whose key - // this SDK has no way to hand back to the caller's store. - log.Errorf("SetConfigFromJSON: failed to load config JSON: %v", err) -======= + // Parsed only to reject a bad document early; the JSON itself is what is + // stored, and every load re-parses it. Not only a parse error any more: a + // document with no peer identity is refused too, because Run() would + // otherwise connect as a peer whose key this SDK has no way to hand back + // to the caller's store. if _, err := profilemanager.ConfigFromJSON(jsonStr); err != nil { - log.Errorf("SetConfigFromJSON: failed to parse config JSON: %v", err) ->>>>>>> origin/main + log.Errorf("SetConfigFromJSON: failed to load config JSON: %v", err) return err } c.preloadedConfigJSON.Store(&jsonStr) diff --git a/client/server/mdm.go b/client/server/mdm.go index 8728ebd71..b22c3b0a3 100644 --- a/client/server/mdm.go +++ b/client/server/mdm.go @@ -9,7 +9,6 @@ import ( "google.golang.org/grpc/codes" gstatus "google.golang.org/grpc/status" - "github.com/netbirdio/netbird/client/internal/profilemanager" "github.com/netbirdio/netbird/client/mdm" "github.com/netbirdio/netbird/client/proto" ) @@ -146,124 +145,6 @@ func (s *Server) restartEngineForMDMLocked() error { return nil } -<<<<<<< HEAD -// conflictBool builds a conflictCheck for a boolean MDM key. If p is nil -// the field is treated as matching (no override requested); otherwise the -// check returns true only when the policy contains the key and its -// boolean value equals *p. -func conflictBool(key string, p *bool) conflictCheck { - return conflictCheck{ - key: key, - check: func(pol *mdm.Policy) bool { - if p == nil { - return true // absent → match by definition - } - want, ok := pol.GetBool(key) - return ok && want == *p - }, - } -} - -// conflictURL is conflictString for URL-typed keys: both sides are compared as -// endpoints (profilemanager.SameServiceURL), so an implicit default port, a -// trailing slash or a different host case is not read as a divergence from the -// policy. A value that does not parse as a URL falls back to string equality, -// which is the strictest thing left to do with it. -func conflictURL(key, got string) conflictCheck { - return conflictCheck{ - key: key, - check: func(pol *mdm.Policy) bool { - if got == "" { - return true - } - want, ok := pol.GetString(key) - if !ok { - return false - } - wantURL, wantErr := profilemanager.ParseServiceURL(key, want) - gotURL, gotErr := profilemanager.ParseServiceURL(key, got) - if wantErr != nil || gotErr != nil { - return want == got - } - return profilemanager.SameServiceURL(wantURL, gotURL) - }, - } -} - -// conflictString builds a conflictCheck for a string MDM key. An empty -// `got` is treated as "field not set" (no override requested); otherwise -// the check returns true only when the policy contains the key and its -// value equals got. -func conflictString(key, got string) conflictCheck { - return conflictCheck{ - key: key, - check: func(pol *mdm.Policy) bool { - if got == "" { - return true - } - want, ok := pol.GetString(key) - return ok && want == got - }, - } -} - -// conflictStringPtr is conflictString for optional proto fields, where an -// explicit empty value is still a request to change the setting. If p is -// nil the field is treated as matching (no override requested); otherwise -// the check returns true only when the policy contains the key and its -// value equals *p. -func conflictStringPtr(key string, p *string) conflictCheck { - return conflictCheck{ - key: key, - check: func(pol *mdm.Policy) bool { - if p == nil { - return true - } - want, ok := pol.GetString(key) - return ok && want == *p - }, - } -} - -// conflictInt64 builds a conflictCheck for an integer MDM key. If p is -// nil the field is treated as matching; otherwise the check returns -// true only when the policy contains the key and its int value equals *p. -func conflictInt64(key string, p *int64) conflictCheck { - return conflictCheck{ - key: key, - check: func(pol *mdm.Policy) bool { - if p == nil { - return true - } - want, ok := pol.GetInt(key) - return ok && want == *p - }, - } -} - -// resolveConflicts walks the per-field checks against the active MDM -// policy and returns the names of keys whose requested value diverges -// from the policy-enforced value. Keys not present in the policy are -// skipped silently (the gate fires only for keys the admin has -// actually pushed). Returns nil for an empty policy. -func resolveConflicts(policy *mdm.Policy, checks []conflictCheck) []string { - if policy.IsEmpty() { - return nil - } - var conflicts []string - for _, c := range checks { - if !policy.HasKey(c.key) { - continue - } - if !c.check(policy) { - conflicts = append(conflicts, c.key) - } - } - return conflicts -} - -======= ->>>>>>> origin/main // mdmManagedFieldConflicts returns the names of MDM-managed keys whose // requested value in the SetConfigRequest differs from the MDM-enforced // value. A field set to the same value the policy already enforces is diff --git a/client/server/server.go b/client/server/server.go index 510fcbb18..eb7d4aaa3 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -669,7 +669,6 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro // command `netbird up --management-url=X` (which falls through to // Login when SetConfig is rejected — see cmd/up.go) would silently // bypass `--disable-update-settings` and any MDM policy. -<<<<<<< HEAD // // The update-settings gate is value-aware, as in SetConfig: it looks at // what a login would actually persist (loginOverridesInput) and refuses @@ -679,19 +678,9 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro // NB_MANAGEMENT_URL, working with the kill switch on. if s.checkUpdateSettingsDisabled() && configChangeRequested(stored, loginOverridesInput(msg)) { return nil, gstatus.Errorf(codes.FailedPrecondition, errUpdateSettingsDisabled) -======= - if loginRequestHasConfigOverrides(msg) { - if s.checkUpdateSettingsDisabled() { - return nil, gstatus.Errorf(codes.Unavailable, errUpdateSettingsDisabled) - } - policy := s.mdmLoader.Load() - if err := rejectMDMManagedFieldConflicts(loginRequestMDMConflicts(msg, policy)); err != nil { - return nil, err - } ->>>>>>> origin/main } - policy := loadMDMPolicy() + policy := s.mdmLoader.Load() if err := rejectMDMManagedFieldConflicts(loginRequestMDMConflicts(msg, policy)); err != nil { return nil, err } @@ -1533,7 +1522,6 @@ func (s *Server) getConfig(activeProf *profilemanager.ActiveProfileState) (*prof return nil, false, fmt.Errorf("failed to get config: %w", err) } -<<<<<<< HEAD // This is the daemon's provisioning point: the config resolved here is the // one the peer runs with, so it needs the keys that identify it, and those // have to reach disk — a key that stays in memory would come back different @@ -1549,13 +1537,13 @@ func (s *Server) getConfig(activeProf *profilemanager.ActiveProfileState) (*prof return nil, false, fmt.Errorf("write out profile config: %w", err) } } -======= - // Apply the daemon-owned MDM policy on top of the just-resolved - // Config. profilemanager's apply() initialises the policy to - // empty — the Loader lives outside Config, so this overlay step - // is driven externally here. + + // Apply the daemon-owned MDM policy on top of the just-resolved Config. + // profilemanager's apply() initialises the policy to empty — the Loader + // lives outside Config, so this overlay step is driven externally here. + // After the write above, on purpose: the overlay is runtime-only and + // re-derived on every load, so the file keeps the profile's own values. config.ApplyMDMPolicy(s.mdmLoader.Load()) ->>>>>>> origin/main return config, configExisted, nil } diff --git a/client/server/update_settings_gate_test.go b/client/server/update_settings_gate_test.go index f167b593b..0d2cd8810 100644 --- a/client/server/update_settings_gate_test.go +++ b/client/server/update_settings_gate_test.go @@ -12,6 +12,7 @@ import ( "github.com/netbirdio/netbird/client/internal" "github.com/netbirdio/netbird/client/internal/profilemanager" + "github.com/netbirdio/netbird/client/mdm" "github.com/netbirdio/netbird/client/proto" ) @@ -156,7 +157,7 @@ func TestLoginGateDecision(t *testing.T) { stored, err := profilemanager.GetExistingConfig(seedProfileConfig(t, storedManagementURL, "stored-key")) require.NoError(t, err) - redacted := preSharedKeyRedactedSentinel + redacted := mdm.PreSharedKeyRedactedSentinel empty := "" sameKey := "stored-key" otherKey := "other-key"