Fix the Inactivity Expiration problem.

[ismail0234]

Describe your changes

Even if inactivity is disabled before the change, the scheduled method can still be triggered. The method is never triggered when inactivity is disabled after the change. This is necessary to solve the error problem of SQL Tests.

Checklist

• [x] Is it a bug fix
• [ ] Is a typo/documentation fix
• [ ] Is a feature enhancement
• [ ] It is a refactor
• [ ] Created tests that fail without the change (if possible)
• [ ] Extended the README / documentation, if necessary

[sonarcloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[ismail0234]

@pascal-fischer This fix solves multiple problems.

The value I add here, if there is a new value, we overwrite the old value. Because when the scheduled task runs, it always considers the old value as valid and triggers the scheduled task at the old time.

https://github.com/netbirdio/netbird/blob/156fa32179288f256ec9ce496203e83763c1ced5/management/server/account.go#L1193

The method triggering order is as follows.

1. UpdateAccountSettings (account.go)
2. handleInactivityExpirationSettings (account.go)
3. checkAndSchedulePeerInactivityExpiration (account.go)
4. account.GetNextInactivePeerExpiration() (account.go)
5. a.Settings.PeerInactivityExpiration

https://github.com/netbirdio/netbird/blob/669904cd060186d0ea760057699aa3f4076ac13b/management/server/account.go#L1273-L1279

The value “a.Settings.PeerInactivityExpiration” contains the old value.

https://github.com/netbirdio/netbird/blob/669904cd060186d0ea760057699aa3f4076ac13b/management/server/account.go#L650

[ismail0234]

I also recommend you to look carefully at the old code. Even if you set “PeerInactivityExpirationEnabled” to false, the timer continues to be triggered if there is a different “PeerInactivityExpiration” value. There seems to be a bug here.

&Settings{
  PeerInactivityExpirationEnabled: false,
  PeerInactivityExpiration: 100,
}

[ismail0234]

This is the test code that gives an error.

The default values given when calling “manager.UpdateAccountSettings” are (PeerInactivityExpirationEnabled: false) and (PeerInactivityExpiration: 0). But in the current peer, these values are set to "true" and "10 minutes". Therefore, even if you set “PeerInactivityExpirationEnabled” to false, the “PeerInactivityExpiration” value is triggered.

https://github.com/netbirdio/netbird/blob/669904cd060186d0ea760057699aa3f4076ac13b/management/server/account_test.go#L1898-L1944

[ismail0234]

This ensures that “PeerInactivityExpirationEnabled” is only triggered when true. It also makes the new value “PeerInactivityExpiration” work.

[pascal-fischer]

Okay, you have me convinced. The PeerInactivityExpiration with 0 and the default is triggering this. It makes sense to disregard the timeout when the expiration is not enabled in the first place. I checked the code and tested the feature, and everything seems to be working fine.