From 64dbd5fbfca49f641c3752adb8d6d15d81feb3ba Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Tue, 7 Feb 2023 11:40:05 +0100 Subject: [PATCH] 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 | 2 +- client/cmd/root.go | 4 +- client/cmd/service_controller.go | 2 +- client/cmd/testutil.go | 3 +- client/cmd/up.go | 1 + client/internal/config.go | 64 +++++++++++++++++++------------- client/internal/config_test.go | 28 ++++++++++---- client/internal/connect.go | 7 +++- client/server/server.go | 6 +-- 9 files changed, 74 insertions(+), 43 deletions(-) diff --git a/client/cmd/login.go b/client/cmd/login.go index 752b5a11c..c1fe39d2f 100644 --- a/client/cmd/login.go +++ b/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) diff --git a/client/cmd/root.go b/client/cmd/root.go index f69979210..0b2739bbc 100644 --- a/client/cmd/root.go +++ b/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") diff --git a/client/cmd/service_controller.go b/client/cmd/service_controller.go index 80a4556c2..b92e1cc05 100644 --- a/client/cmd/service_controller.go +++ b/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) } diff --git a/client/cmd/testutil.go b/client/cmd/testutil.go index 4c923c704..bbd4c0cb9 100644 --- a/client/cmd/testutil.go +++ b/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) } diff --git a/client/cmd/up.go b/client/cmd/up.go index 4aa3f421c..1f874c744 100644 --- a/client/cmd/up.go +++ b/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, diff --git a/client/internal/config.go b/client/internal/config.go index d5dbb7ad5..07f3f38e2 100644 --- a/client/internal/config.go +++ b/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 diff --git a/client/internal/config_test.go b/client/internal/config_test.go index d4bf8433e..3ca8a5213 100644 --- a/client/internal/config_test.go +++ b/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, diff --git a/client/internal/connect.go b/client/internal/connect.go index fb9eab9f3..bbc430d3f 100644 --- a/client/internal/connect.go +++ b/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 } diff --git a/client/server/server.go b/client/server/server.go index 24db0ebe0..4b2efeb78 100644 --- a/client/server/server.go +++ b/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, }