mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-12 17:59:06 +02:00
[client] Keep the admin panel path part of its identity
The endpoint comparison introduced for the management URL was applied to the admin URL too, and that one is opened in a browser rather than dialed over gRPC: a panel served under /netbird is not the panel served at the root. So a config whose admin URL differed only by path reported no change, and the new path was never persisted — a custom panel URL could not be updated at all. SameServiceURLIncludingPath adds what a URL carries past its endpoint (path, query, fragment, userinfo) while still treating equivalent spellings as equal: a missing path and "/" are the same root, and so is a trailing slash. The management URL keeps the endpoint-only comparison, since only the endpoint is ever dialed. Ports are also normalized numerically now, so ":0443" and ":443" are one port. Reported by cubic-dev-ai on PR #7398 (two findings).
This commit is contained in:
@@ -13,6 +13,7 @@ import (
|
||||
"reflect"
|
||||
"runtime"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -409,13 +410,15 @@ func (config *Config) apply(input ConfigInput) (updated bool, err error) {
|
||||
return false, err
|
||||
}
|
||||
}
|
||||
// Same endpoint comparison as the Management URL above.
|
||||
// The admin panel is opened, not dialed, so unlike the Management URL its
|
||||
// path is part of what identifies it: a panel served under /netbird is not
|
||||
// the one served at the root.
|
||||
if input.AdminURL != "" {
|
||||
newURL, err := parseURL("Admin Panel URL", input.AdminURL)
|
||||
if err != nil {
|
||||
return updated, err
|
||||
}
|
||||
if !SameServiceURL(newURL, config.AdminURL) {
|
||||
if !SameServiceURLIncludingPath(newURL, config.AdminURL) {
|
||||
log.Infof("new Admin Panel URL provided, updated to %#v (old value %#v)",
|
||||
newURL.String(), config.AdminURL.String())
|
||||
config.AdminURL = newURL
|
||||
@@ -947,20 +950,49 @@ func SameServiceURL(a, b *url.URL) bool {
|
||||
}
|
||||
|
||||
// ServiceURLPort returns the port a service URL addresses, resolving an absent
|
||||
// one to the default of its scheme.
|
||||
// one to the default of its scheme. The port is normalized numerically, so a
|
||||
// zero-padded ":0443" is the same port as ":443".
|
||||
func ServiceURLPort(u *url.URL) string {
|
||||
if port := u.Port(); port != "" {
|
||||
return port
|
||||
port := u.Port()
|
||||
if port == "" {
|
||||
switch u.Scheme {
|
||||
case "https":
|
||||
return "443"
|
||||
case "http":
|
||||
return "80"
|
||||
default:
|
||||
return ""
|
||||
}
|
||||
}
|
||||
|
||||
switch u.Scheme {
|
||||
case "https":
|
||||
return "443"
|
||||
case "http":
|
||||
return "80"
|
||||
default:
|
||||
return ""
|
||||
if n, err := strconv.Atoi(port); err == nil {
|
||||
return strconv.Itoa(n)
|
||||
}
|
||||
return port
|
||||
}
|
||||
|
||||
// SameServiceURLIncludingPath is SameServiceURL plus everything a URL carries
|
||||
// past its endpoint: path, query, fragment and userinfo.
|
||||
//
|
||||
// Use it for a URL that gets opened rather than dialed. The admin panel can
|
||||
// live under a path, so two URLs with the same endpoint and different paths are
|
||||
// two different panels — where for a URL the client dials over gRPC only the
|
||||
// endpoint is ever used. Equivalent spellings still compare equal: a missing
|
||||
// path and "/" are the same root, and so is a trailing slash on any path.
|
||||
func SameServiceURLIncludingPath(a, b *url.URL) bool {
|
||||
if a == nil || b == nil {
|
||||
return a == b
|
||||
}
|
||||
|
||||
return SameServiceURL(a, b) &&
|
||||
normalizedURLPath(a) == normalizedURLPath(b) &&
|
||||
a.RawQuery == b.RawQuery &&
|
||||
a.Fragment == b.Fragment &&
|
||||
a.User.String() == b.User.String()
|
||||
}
|
||||
|
||||
func normalizedURLPath(u *url.URL) string {
|
||||
return strings.TrimSuffix(u.Path, "/")
|
||||
}
|
||||
|
||||
func parseURL(serviceName, serviceURL string) (*url.URL, error) {
|
||||
|
||||
@@ -329,3 +329,50 @@ func TestCreateInMemoryConfigCarriesAnIdentity(t *testing.T) {
|
||||
require.NotEmpty(t, cfg.PrivateKey)
|
||||
require.NotEmpty(t, cfg.SSHKey)
|
||||
}
|
||||
|
||||
// The admin panel is opened, not dialed, so its path identifies it. Comparing
|
||||
// it as a bare endpoint left a custom panel URL unable to change.
|
||||
func TestAdminURLPathIsPartOfTheIdentity(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "panel.json")
|
||||
_, err := UpdateOrCreateConfig(ConfigInput{
|
||||
ConfigPath: path,
|
||||
AdminURL: "https://app.example.com/netbird",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
cfg, err := GetExistingConfig(path)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "https://app.example.com:443/netbird", cfg.AdminURL.String())
|
||||
|
||||
// Equivalent spellings of the same panel are still not a change.
|
||||
for _, same := range []string{
|
||||
"https://app.example.com/netbird",
|
||||
"https://app.example.com:443/netbird",
|
||||
"https://app.example.com/netbird/",
|
||||
"https://APP.example.com/netbird",
|
||||
} {
|
||||
changed, err := cfg.WouldChange(ConfigInput{AdminURL: same})
|
||||
require.NoError(t, err)
|
||||
require.False(t, changed, "%q is the stored panel written differently", same)
|
||||
}
|
||||
|
||||
// A different path is a different panel, and it must be persisted.
|
||||
changed, err := cfg.WouldChange(ConfigInput{AdminURL: "https://app.example.com/other"})
|
||||
require.NoError(t, err)
|
||||
require.True(t, changed, "a different panel path is a change")
|
||||
|
||||
updated, err := UpdateConfig(ConfigInput{ConfigPath: path, AdminURL: "https://app.example.com/other"})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "https://app.example.com:443/other", updated.AdminURL.String(), "the new panel path was not persisted")
|
||||
}
|
||||
|
||||
// A zero-padded port addresses the same port.
|
||||
func TestServiceURLPortIsNormalizedNumerically(t *testing.T) {
|
||||
padded, err := ParseServiceURL("padded", "https://mgmt.example.com:0443")
|
||||
require.NoError(t, err)
|
||||
plain, err := ParseServiceURL("plain", "https://mgmt.example.com:443")
|
||||
require.NoError(t, err)
|
||||
|
||||
require.Equal(t, "443", ServiceURLPort(padded))
|
||||
require.True(t, SameServiceURL(padded, plain))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user