diff --git a/management/server/account.go b/management/server/account.go index 7cab037a2..ddc8d4bbc 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -298,6 +298,9 @@ func BuildManager( am.integratedPeerValidator.SetPeerInvalidationListener(func(accountID string, peerIDs []string) { am.onPeersInvalidated(ctx, accountID, peerIDs) }) + am.integratedPeerValidator.SetPeerValidationChangeListener(func(accountID string, peerIDs []string) { + am.onPeerValidationChanged(ctx, accountID, peerIDs) + }) // Started last: a manager that fails to build is never returned, so nothing would // stop its refresher. @@ -2102,6 +2105,22 @@ func (am *DefaultAccountManager) onPeersInvalidated(ctx context.Context, account log.WithContext(ctx).Debugf("invalidated peers have been expired for account %s", accountID) } +// onPeerValidationChanged pushes fresh network maps to peers whose validation state +// changed in the background, and to the peers that can reach them. +func (am *DefaultAccountManager) onPeerValidationChanged(ctx context.Context, accountID string, peerIDs []string) { + if len(peerIDs) == 0 { + return + } + affected := am.resolveAffectedPeersForPeerChanges(ctx, am.Store, accountID, peerIDs) + if len(affected) == 0 { + // Resolution failed or found nothing; the changed peers' own maps still differ. + affected = peerIDs + } + if err := am.networkMapController.OnPeersUpdated(ctx, accountID, peerIDs, affected); err != nil { + log.WithContext(ctx).Errorf("failed to update peers after validation change for account %s: %v", accountID, err) + } +} + func (am *DefaultAccountManager) FindExistingPostureCheck(accountID string, checks *posture.ChecksDefinition) (*posture.Checks, error) { return am.Store.GetPostureCheckByChecksDefinition(accountID, checks) } diff --git a/management/server/integrated_validator.go b/management/server/integrated_validator.go index 5928a8ed2..63d3ef280 100644 --- a/management/server/integrated_validator.go +++ b/management/server/integrated_validator.go @@ -167,6 +167,10 @@ func (MockIntegratedValidator) SetPeerInvalidationListener(func(accountID string // just a dummy } +func (MockIntegratedValidator) SetPeerValidationChangeListener(func(accountID string, peerIDs []string)) { + // just a dummy +} + func (MockIntegratedValidator) Stop(_ context.Context) { // just a dummy } diff --git a/management/server/integrations/integrated_validator/integrated_validator_mock.go b/management/server/integrations/integrated_validator/integrated_validator_mock.go index 73178a869..27f61a79c 100644 --- a/management/server/integrations/integrated_validator/integrated_validator_mock.go +++ b/management/server/integrations/integrated_validator/integrated_validator_mock.go @@ -130,6 +130,18 @@ func (mr *MockIntegratedValidatorMockRecorder) SetPeerInvalidationListener(fn an return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetPeerInvalidationListener", reflect.TypeOf((*MockIntegratedValidator)(nil).SetPeerInvalidationListener), fn) } +// SetPeerValidationChangeListener mocks base method. +func (m *MockIntegratedValidator) SetPeerValidationChangeListener(fn func(string, []string)) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetPeerValidationChangeListener", fn) +} + +// SetPeerValidationChangeListener indicates an expected call of SetPeerValidationChangeListener. +func (mr *MockIntegratedValidatorMockRecorder) SetPeerValidationChangeListener(fn any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetPeerValidationChangeListener", reflect.TypeOf((*MockIntegratedValidator)(nil).SetPeerValidationChangeListener), fn) +} + // Stop mocks base method. func (m *MockIntegratedValidator) Stop(ctx context.Context) { m.ctrl.T.Helper() diff --git a/management/server/integrations/integrated_validator/interface.go b/management/server/integrations/integrated_validator/interface.go index dc3332177..eb5ce8815 100644 --- a/management/server/integrations/integrated_validator/interface.go +++ b/management/server/integrations/integrated_validator/interface.go @@ -21,6 +21,9 @@ type IntegratedValidator interface { GetInvalidPeers(ctx context.Context, accountID string, extraSettings *types.ExtraSettings) (map[string]string, error) PeerDeleted(ctx context.Context, accountID, peerID string, extraSettings *types.ExtraSettings) error SetPeerInvalidationListener(fn func(accountID string, peerIDs []string)) + // SetPeerValidationChangeListener registers fn to be called with the peers whose + // validation state changed outside a login or sync, so their network maps can be pushed. + SetPeerValidationChangeListener(fn func(accountID string, peerIDs []string)) Stop(ctx context.Context) ValidateFlowResponse(ctx context.Context, peerKey string, flowResponse *proto.PKCEAuthorizationFlow) *proto.PKCEAuthorizationFlow } diff --git a/management/server/integrations/integrated_validator/validator/validator.go b/management/server/integrations/integrated_validator/validator/validator.go index 33199c065..ec4c4827d 100644 --- a/management/server/integrations/integrated_validator/validator/validator.go +++ b/management/server/integrations/integrated_validator/validator/validator.go @@ -55,6 +55,9 @@ func (v *IntegratedValidatorImpl) PeerDeleted(_ context.Context, _, _ string, _ func (v *IntegratedValidatorImpl) SetPeerInvalidationListener(_ func(accountID string, peerIDs []string)) { } +func (v *IntegratedValidatorImpl) SetPeerValidationChangeListener(_ func(accountID string, peerIDs []string)) { +} + func (v *IntegratedValidatorImpl) Stop(_ context.Context) { } diff --git a/management/server/peer_validation_change_test.go b/management/server/peer_validation_change_test.go new file mode 100644 index 000000000..14c179fee --- /dev/null +++ b/management/server/peer_validation_change_test.go @@ -0,0 +1,59 @@ +package server + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/netbirdio/netbird/management/internals/controllers/network_map" + "github.com/netbirdio/netbird/management/server/types" +) + +// A peer that the background validator approves must get its own map and appear in the +// maps of the peers a policy connects it to, without waiting for a login or sync. +func TestOnPeerValidationChanged_UpdatesChangedPeerAndItsPeers(t *testing.T) { + _, s, accountID, peerIDs, groupIDs := setupAffectedPeersTest(t) + ctx := context.Background() + + err := s.CreatePolicy(ctx, &types.Policy{ + ID: "policy-validation-change", + AccountID: accountID, + Enabled: true, + Rules: []*types.PolicyRule{{ + ID: "rule-validation-change", + PolicyID: "policy-validation-change", + Enabled: true, + Sources: []string{groupIDs[0]}, + Destinations: []string{groupIDs[1]}, + Bidirectional: true, + Action: types.PolicyTrafficActionAccept, + }}, + }) + require.NoError(t, err) + + var changed, affected []string + controller := network_map.NewMockController(gomock.NewController(t)) + controller.EXPECT().OnPeersUpdated(gomock.Any(), accountID, gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ string, peerIDs, affectedPeerIDs []string) error { + changed, affected = peerIDs, affectedPeerIDs + return nil + }) + // A separate manager over the same store: the setup's manager still has peer update + // goroutines in flight that read its controller. + manager := &DefaultAccountManager{Store: s, networkMapController: controller} + + manager.onPeerValidationChanged(ctx, accountID, []string{peerIDs[0]}) + + assert.Equal(t, []string{peerIDs[0]}, changed, "changed peers should be passed through") + assert.ElementsMatch(t, []string{peerIDs[0], peerIDs[1]}, affected, "the changed peer and its policy peer should be updated") +} + +func TestOnPeerValidationChanged_NoPeersNoUpdate(t *testing.T) { + // Any call on the mock fails the test. + manager := &DefaultAccountManager{networkMapController: network_map.NewMockController(gomock.NewController(t))} + + manager.onPeerValidationChanged(context.Background(), "account", nil) +}