Skip to content

Mobile node connections configuration is optional now - #565

Merged
tadovas merged 1 commit into
masterfrom
feature/reorganize-mobile-node-api
Nov 22, 2018
Merged

Mobile node connections configuration is optional now#565
tadovas merged 1 commit into
masterfrom
feature/reorganize-mobile-node-api

Conversation

@tadovas

@tadovas tadovas commented Nov 22, 2018

Copy link
Copy Markdown
Contributor
  • Keep mobile node constructor with essential and mandatory params only
  • Move openvpn connection registration/configuration to separate method called optionally
  • Do the same with wireguard

Introducing new mobile specific service configuration should not affect existing clients (to some extent)

Bonus: @Waldz can now roam freely between wifi and 4G :)

…e types, use separate specific methods instead
@tadovas
tadovas requested review from vkuznecovas and zolia November 22, 2018 12:24
@tadovas

tadovas commented Nov 22, 2018

Copy link
Copy Markdown
Contributor Author

Ping @donce as these gonna API breaking changes. Nothing super complicated, but still...

type WireguardTunnelSetup tunnSetupPlaceholder

// OverrideWireguardConnection overrides default wireguard connection implementation to more mobile adapted one
func (mobNode *MobileNode) OverrideWireguardConnection(wgTunnelSetup WireguardTunnelSetup) {

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.

Why to call it Override? Its simple connection through our bridge.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because it exactly what it does - overrides default "openvpn" connection configured in node

type Openvpn3TunnelSetup openvpn3.TunnelSetup

// OverrideOpenvpnConnection replaces default openvpn connection factory with mobile related one
func (mobNode *MobileNode) OverrideOpenvpnConnection(tunnelSetup Openvpn3TunnelSetup) {

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.

Maybe we can call it MobileOpenvpnConnection. From user standpoint he is not overriding anything..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See my comment above

@tadovas
tadovas merged commit 917d594 into master Nov 22, 2018
@tadovas
tadovas deleted the feature/reorganize-mobile-node-api branch November 22, 2018 12:56
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.

3 participants