Refactor Management and Admin URL config (#674)

avoid sending admin or management URLs on service start
as it doesn't have an input

Parse management and admin URL when needed

Pass empty admin url on commands to prevent default overwrite

client/cmd/login.go

@@ -162,7 +162,7 @@ func foregroundGetTokenInfo(ctx context.Context, cmd *cobra.Command, config *int
 		} else if ok && s.Code() == codes.Unimplemented {
 			mgmtURL := managementURL
 			if mgmtURL == "" {
-				mgmtURL = internal.ManagementURLDefault().String()
+				mgmtURL = internal.DefaultManagementURL
 			}
 			return nil, fmt.Errorf("the management server, %s, does not support SSO providers, "+
 				"please update your servver or use Setup Keys to login", mgmtURL)

client/cmd/root.go

@@ -87,8 +87,8 @@ func init() {
 		defaultDaemonAddr = "tcp://127.0.0.1:41731"
 	}
 	rootCmd.PersistentFlags().StringVar(&daemonAddr, "daemon-addr", defaultDaemonAddr, "Daemon service address to serve CLI requests [unix|tcp]://[path|host:port]")
-	rootCmd.PersistentFlags().StringVarP(&managementURL, "management-url", "m", "", fmt.Sprintf("Management Service URL [http|https]://[host]:[port] (default \"%s\")", internal.ManagementURLDefault().String()))
-	rootCmd.PersistentFlags().StringVar(&adminURL, "admin-url", "https://app.netbird.io", "Admin Panel URL [http|https]://[host]:[port]")
+	rootCmd.PersistentFlags().StringVarP(&managementURL, "management-url", "m", "", fmt.Sprintf("Management Service URL [http|https]://[host]:[port] (default \"%s\")", internal.DefaultManagementURL))
+	rootCmd.PersistentFlags().StringVar(&adminURL, "admin-url", "", fmt.Sprintf("Admin Panel URL [http|https]://[host]:[port] (default \"%s\")", internal.DefaultAdminURL))
 	rootCmd.PersistentFlags().StringVarP(&configPath, "config", "c", defaultConfigPath, "Netbird config file location")
 	rootCmd.PersistentFlags().StringVarP(&logLevel, "log-level", "l", "info", "sets Netbird log level")
 	rootCmd.PersistentFlags().StringVar(&logFile, "log-file", defaultLogFile, "sets Netbird log path. If console is specified the the log will be output to stdout")

client/cmd/service_controller.go

@@ -54,7 +54,7 @@ func (p *program) Start(svc service.Service) error {
 			}
 		}
 
-		serverInstance := server.New(p.ctx, managementURL, adminURL, configPath, logFile)
+		serverInstance := server.New(p.ctx, configPath, logFile)
 		if err := serverInstance.Start(); err != nil {
 			log.Fatalf("failed to start daemon: %v", err)
 		}

client/cmd/testutil.go

@@ -102,7 +102,8 @@ func startClientDaemon(
 	}
 	s := grpc.NewServer()
 
-	server := client.New(ctx, managementURL, adminURL, configPath, "")
+	server := client.New(ctx,
+		configPath, "")
 	if err := server.Start(); err != nil {
 		t.Fatal(err)
 	}

client/cmd/up.go

@@ -132,6 +132,7 @@ func runInDaemonMode(ctx context.Context, cmd *cobra.Command) error {
 		SetupKey:            setupKey,
 		PreSharedKey:        preSharedKey,
 		ManagementUrl:       managementURL,
+		AdminURL:            adminURL,
 		NatExternalIPs:      natExternalIPs,
 		CleanNATExternalIPs: natExternalIPs != nil && len(natExternalIPs) == 0,
 		CustomDNSAddress:    customDNSAddressConverted,

client/internal/config.go

@@ -16,28 +16,20 @@ import (
 	"google.golang.org/grpc/status"
 )
 
-// ManagementLegacyPort is the port that was used before by the Management gRPC server.
-// It is used for backward compatibility now.
-// NB: hardcoded from github.com/netbirdio/netbird/management/cmd to avoid import
-const ManagementLegacyPort = 33073
+const (
+	// ManagementLegacyPort is the port that was used before by the Management gRPC server.
+	// It is used for backward compatibility now.
+	// NB: hardcoded from github.com/netbirdio/netbird/management/cmd to avoid import
+	ManagementLegacyPort = 33073
+	// DefaultManagementURL points to the NetBird's cloud management endpoint
+	DefaultManagementURL = "https://api.wiretrustee.com:443"
+	// DefaultAdminURL points to NetBird's cloud management console
+	DefaultAdminURL = "https://app.netbird.io:443"
+)
 
 var defaultInterfaceBlacklist = []string{iface.WgInterfaceDefault, "wt", "utun", "tun0", "zt", "ZeroTier", "wg", "ts",
 	"Tailscale", "tailscale", "docker", "veth", "br-"}
 
-var managementURLDefault *url.URL
-
-func ManagementURLDefault() *url.URL {
-	return managementURLDefault
-}
-
-func init() {
-	managementURL, err := ParseURL("Management URL", "https://api.wiretrustee.com:443")
-	if err != nil {
-		panic(err)
-	}
-	managementURLDefault = managementURL
-}
-
 // ConfigInput carries configuration changes to the client
 type ConfigInput struct {
 	ManagementURL    string
@@ -99,20 +91,31 @@ func createNewConfig(input ConfigInput) (*Config, error) {
 		NATExternalIPs:       input.NATExternalIPs,
 		CustomDNSAddress:     string(input.CustomDNSAddress),
 	}
+
+	defaultManagementURL, err := ParseURL("Management URL", DefaultManagementURL)
+	if err != nil {
+		return nil, err
+	}
+
+	config.ManagementURL = defaultManagementURL
 	if input.ManagementURL != "" {
 		URL, err := ParseURL("Management URL", input.ManagementURL)
 		if err != nil {
 			return nil, err
 		}
 		config.ManagementURL = URL
-	} else {
-		config.ManagementURL = managementURLDefault
 	}
 
 	if input.PreSharedKey != nil {
 		config.PreSharedKey = *input.PreSharedKey
 	}
 
+	defaultAdminURL, err := ParseURL("Admin URL", DefaultAdminURL)
+	if err != nil {
+		return nil, err
+	}
+
+	config.AdminURL = defaultAdminURL
 	if input.AdminURL != "" {
 		newURL, err := ParseURL("Admin Panel URL", input.AdminURL)
 		if err != nil {
@@ -131,18 +134,29 @@ func createNewConfig(input ConfigInput) (*Config, error) {
 	return config, nil
 }
 
-// ParseURL parses and validates management URL
-func ParseURL(serviceName, managementURL string) (*url.URL, error) {
-	parsedMgmtURL, err := url.ParseRequestURI(managementURL)
+// ParseURL parses and validates a service URL
+func ParseURL(serviceName, serviceURL string) (*url.URL, error) {
+	parsedMgmtURL, err := url.ParseRequestURI(serviceURL)
 	if err != nil {
-		log.Errorf("failed parsing management URL %s: [%s]", managementURL, err.Error())
+		log.Errorf("failed parsing %s URL %s: [%s]", serviceName, serviceURL, err.Error())
 		return nil, err
 	}
 
 	if parsedMgmtURL.Scheme != "https" && parsedMgmtURL.Scheme != "http" {
 		return nil, fmt.Errorf(
 			"invalid %s URL provided %s. Supported format [http|https]://[host]:[port]",
-			serviceName, managementURL)
+			serviceName, serviceURL)
+	}
+
+	if parsedMgmtURL.Port() == "" {
+		switch parsedMgmtURL.Scheme {
+		case "https":
+			parsedMgmtURL.Host = parsedMgmtURL.Host + ":443"
+		case "http":
+			parsedMgmtURL.Host = parsedMgmtURL.Host + ":80"
+		default:
+			log.Infof("unable to determine a default port for schema %s in URL %s", parsedMgmtURL.Scheme, serviceURL)
+		}
 	}
 
 	return parsedMgmtURL, err

client/internal/config_test.go

@@ -10,17 +10,29 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestReadConfig(t *testing.T) {
-}
-
 func TestGetConfig(t *testing.T) {
+	// case 1: new default config has to be generated
+	config, err := GetConfig(ConfigInput{
+		ConfigPath: filepath.Join(t.TempDir(), "config.json"),
+	})
+
+	if err != nil {
+		return
+	}
+
+	assert.Equal(t, config.ManagementURL.String(), DefaultManagementURL)
+	assert.Equal(t, config.AdminURL.String(), DefaultAdminURL)
+
+	if err != nil {
+		return
+	}
 	managementURL := "https://test.management.url:33071"
-	adminURL := "https://app.admin.url"
+	adminURL := "https://app.admin.url:443"
 	path := filepath.Join(t.TempDir(), "config.json")
 	preSharedKey := "preSharedKey"
 
-	// case 1: new config has to be generated
-	config, err := GetConfig(ConfigInput{
+	// case 2: new config has to be generated
+	config, err = GetConfig(ConfigInput{
 		ManagementURL: managementURL,
 		AdminURL:      adminURL,
 		ConfigPath:    path,
@@ -37,7 +49,7 @@ func TestGetConfig(t *testing.T) {
 		t.Errorf("config file was expected to be created under path %s", path)
 	}
 
-	// case 2: existing config -> fetch it
+	// case 3: existing config -> fetch it
 	config, err = GetConfig(ConfigInput{
 		ManagementURL: managementURL,
 		AdminURL:      adminURL,
@@ -51,7 +63,7 @@ func TestGetConfig(t *testing.T) {
 	assert.Equal(t, config.ManagementURL.String(), managementURL)
 	assert.Equal(t, config.PreSharedKey, preSharedKey)
 
-	// case 3: existing config, but new managementURL has been provided -> update config
+	// case 4: existing config, but new managementURL has been provided -> update config
 	newManagementURL := "https://test.newManagement.url:33071"
 	config, err = GetConfig(ConfigInput{
 		ManagementURL: newManagementURL,

client/internal/connect.go

@@ -251,7 +251,12 @@ func loginToManagement(ctx context.Context, client mgm.Client, pubSSHKey []byte)
 // The check is performed only for the NetBird's managed version.
 func UpdateOldManagementPort(ctx context.Context, config *Config, configPath string) (*Config, error) {
 
-	if config.ManagementURL.Hostname() != ManagementURLDefault().Hostname() {
+	defaultManagementURL, err := ParseURL("Management URL", DefaultManagementURL)
+	if err != nil {
+		return nil, err
+	}
+
+	if config.ManagementURL.Hostname() != defaultManagementURL.Hostname() {
 		// only do the check for the NetBird's managed version
 		return config, nil
 	}

client/server/server.go

@@ -45,13 +45,11 @@ type oauthAuthFlow struct {
 }
 
 // New server instance constructor.
-func New(ctx context.Context, managementURL, adminURL, configPath, logFile string) *Server {
+func New(ctx context.Context, configPath, logFile string) *Server {
 	return &Server{
 		rootCtx: ctx,
 		latestConfigInput: internal.ConfigInput{
-			ManagementURL: managementURL,
-			AdminURL:      adminURL,
-			ConfigPath:    configPath,
+			ConfigPath: configPath,
 		},
 		logFile: logFile,
 	}