fix: interface_name strategy, instance hosts in sftp/scp, alias rewrites
Found by the new Docker test environment against a real NetBox 4.7: - The interface_name strategy filtered IP addresses with interface_name and vminterface_name, which NetBox ignores, so it returned the first address of the host instead of the named interface's. Use the real filters interface and vminterface; a missing interface (HTTP 400) falls through to the next strategy. - sftp and scp took instance-qualified hosts (prod/web01:/path) for local paths, because a "/" comes before the ":". Such arguments are remote when the prefix is a configured NetBox instance. - Shell aliases of instance-qualified hosts ran the shortcut rules on "instance/host", so anchored rewrite rules never matched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
3c9e54b254
commit
55e7a5106b
10 files changed
+144
-11
No files matched your search
@@ -235,7 +235,7 @@ func TestGetIPsWithFilter(t *testing.T) {
|
||||
defer srv.Close()
|
||||
|
||||
c := NewClient(srv.URL, "token", 0)
|
||||
ips, err := c.GetIPsWithFilter(context.Background(), "device_id=1&interface_name=mgmt0")
|
||||
ips, err := c.GetIPsWithFilter(context.Background(), "device_id=1&interface=mgmt0")
|
||||
if err != nil {
|
||||
t.Fatalf("GetIPsWithFilter: %v", err)
|
||||
}
|
||||
|
||||
@@ -78,7 +78,7 @@ func TestMultiSourceSearchAndRouting(t *testing.T) {
|
||||
if err != nil || len(ips) != 1 || ips[0] != tc.ip {
|
||||
t.Fatalf("wrong IP source: %v, %v", ips, err)
|
||||
}
|
||||
ips, err = c.GetIPsWithFilterForHost(ctx, host, "device_id=1&interface_name=mgmt0")
|
||||
ips, err = c.GetIPsWithFilterForHost(ctx, host, "device_id=1&interface=mgmt0")
|
||||
if err != nil || len(ips) != 1 || ips[0] != tc.ip {
|
||||
t.Fatalf("wrong interface source: %v, %v", ips, err)
|
||||
}
|
||||
|
||||
@@ -16,13 +16,16 @@ type InterfaceNameStrategy struct {
|
||||
func (s *InterfaceNameStrategy) Name() string { return "interface_name" }
|
||||
|
||||
func (s *InterfaceNameStrategy) Resolve(ctx context.Context, entry *netbox.HostEntry, client *netbox.Client) (string, error) {
|
||||
// Build filter parameters for IP addresses attached to the named interface.
|
||||
// NetBox filters IP addresses by interface name with interface= (devices)
|
||||
// and vminterface= (VMs); unknown parameters would be ignored silently and
|
||||
// return every address of the host. A missing interface is an HTTP 400,
|
||||
// which lets the chain try the next strategy.
|
||||
var filterParam string
|
||||
switch entry.Kind {
|
||||
case "device":
|
||||
filterParam = fmt.Sprintf("device_id=%d&interface_name=%s", entry.ID, url.QueryEscape(s.name))
|
||||
filterParam = fmt.Sprintf("device_id=%d&interface=%s", entry.ID, url.QueryEscape(s.name))
|
||||
case "vm":
|
||||
filterParam = fmt.Sprintf("virtual_machine_id=%d&vminterface_name=%s", entry.ID, url.QueryEscape(s.name))
|
||||
filterParam = fmt.Sprintf("virtual_machine_id=%d&vminterface=%s", entry.ID, url.QueryEscape(s.name))
|
||||
default:
|
||||
return "", fmt.Errorf("unknown kind %q", entry.Kind)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
package resolver
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
"git.zb-server.de/Sebi/ssh-netbox-wrapper/internal/netbox"
|
||||
)
|
||||
|
||||
// NetBox ignores unknown filters, so the strategy must use the real filter
|
||||
// names; otherwise it silently returns the host's first address.
|
||||
func TestInterfaceNameStrategyFilters(t *testing.T) {
|
||||
for kind, want := range map[string]string{
|
||||
"device": "device_id=7&interface=mgmt0",
|
||||
"vm": "virtual_machine_id=7&vminterface=mgmt0",
|
||||
} {
|
||||
var got string
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
q := r.URL.Query()
|
||||
q.Del("limit")
|
||||
q.Del("offset")
|
||||
got = q.Encode()
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.Write([]byte(`{"count":1,"results":[{"address":"10.0.0.7/24"}]}`))
|
||||
}))
|
||||
ip, err := (&InterfaceNameStrategy{name: "mgmt0"}).Resolve(context.Background(), &netbox.HostEntry{ID: 7, Kind: kind}, netbox.NewClient(srv.URL, "t", 1))
|
||||
srv.Close()
|
||||
if err != nil || ip != "10.0.0.7" {
|
||||
t.Fatalf("%s: %q %v", kind, ip, err)
|
||||
}
|
||||
if parsed, _ := urlQuery(want); got != parsed {
|
||||
t.Errorf("%s: query %q, want %q", kind, got, parsed)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestInterfaceNameStrategyMissingInterfaceFallsThrough(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
http.Error(w, `{"vminterface":["Select a valid choice."]}`, http.StatusBadRequest)
|
||||
}))
|
||||
defer srv.Close()
|
||||
client := netbox.NewClient(srv.URL, "t", 1)
|
||||
chain := &Chain{strategies: []Strategy{&InterfaceNameStrategy{name: "mgmt0"}, &PrimaryIPStrategy{}}}
|
||||
ip, err := chain.Resolve(context.Background(), &netbox.HostEntry{ID: 7, Kind: "vm", PrimaryIP4: "10.0.0.9"}, client)
|
||||
if err != nil || ip != "10.0.0.9" {
|
||||
t.Fatalf("got %q %v", ip, err)
|
||||
}
|
||||
}
|
||||
|
||||
func urlQuery(s string) (string, error) {
|
||||
q, err := url.ParseQuery(s)
|
||||
return q.Encode(), err
|
||||
}
|
||||
@@ -59,6 +59,11 @@ func MakeNormalizer(cfg config.ShortcutsConfig) func(string) string {
|
||||
// It normalizes the name (strips configured domains, applies rewrites, optionally strips hyphens) then
|
||||
// replaces any remaining dots with underscores so the result is a valid identifier.
|
||||
func AliasName(name string, cfg config.ShortcutsConfig) string {
|
||||
s := Normalize(name, cfg)
|
||||
// Shorten only the hostname: anchored rewrite rules must not see "instance/".
|
||||
prefix := ""
|
||||
if source, host, ok := strings.Cut(name, "/"); ok {
|
||||
prefix, name = strings.ToLower(source)+"_", host
|
||||
}
|
||||
s := prefix + Normalize(name, cfg)
|
||||
return strings.NewReplacer(".", "_", "/", "_").Replace(s)
|
||||
}
|
||||
@@ -115,3 +115,13 @@ func TestNormalizeRewritesOrder(t *testing.T) {
|
||||
t.Fatalf("got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAliasNameRewritesInstanceQualifiedHosts(t *testing.T) {
|
||||
cfg := config.ShortcutsConfig{
|
||||
Domains: []string{".hoster.test"},
|
||||
Rewrites: []config.ShortcutRewrite{{Pattern: `^([a-z]+)[0-9]+-`, Replace: "$1"}},
|
||||
}
|
||||
if got := shortcuts.AliasName("fsn/fsn1-pve1.hoster.test", cfg); got != "fsn_fsnpve1" {
|
||||
t.Fatalf("got %q", got)
|
||||
}
|
||||
}
|
||||
@@ -59,7 +59,7 @@ func (c Client) Parse(args []string) *ParsedArgs {
|
||||
func (c Client) parsed(args []string, i int) *ParsedArgs {
|
||||
dest, path := args[i], ""
|
||||
if c.paths {
|
||||
if d, p, remote := splitRemote(dest); remote {
|
||||
if d, p, remote := c.splitRemote(dest); remote {
|
||||
dest, path = d, p
|
||||
}
|
||||
}
|
||||
@@ -88,7 +88,7 @@ func (c Client) replaceHost(args []string, dests []int, newHost string) []string
|
||||
prefix, original = original[:at+1], original[at+1:]
|
||||
}
|
||||
if c.paths {
|
||||
if _, path, remote := splitRemote(original); remote {
|
||||
if _, path, remote := c.splitRemote(original); remote {
|
||||
suffix = ":" + path
|
||||
}
|
||||
}
|
||||
|
||||
+28
-3
@@ -13,6 +13,18 @@ type Client struct {
|
||||
port byte // flag for the port: -p (ssh) or -P (sftp, scp)
|
||||
user byte // flag for the login user; 0 when only user@ or -o User work
|
||||
paths bool // destinations are host[:path]
|
||||
// instances are the configured NetBox instance names; "inst/host:path"
|
||||
// is remote only for these, so local paths like "dir/file:x" stay local.
|
||||
instances map[string]bool
|
||||
}
|
||||
|
||||
// WithInstances returns c aware of the NetBox instance prefixes in host:path.
|
||||
func (c Client) WithInstances(names []string) Client {
|
||||
c.instances = make(map[string]bool, len(names))
|
||||
for _, name := range names {
|
||||
c.instances[strings.ToLower(name)] = true
|
||||
}
|
||||
return c
|
||||
}
|
||||
|
||||
var (
|
||||
@@ -61,7 +73,7 @@ func (c Client) Operands(args []string) ([]Operand, error) {
|
||||
if strings.HasPrefix(arg, "scp://") || strings.HasPrefix(arg, "sftp://") {
|
||||
return nil, fmt.Errorf("%q: use host:path instead of a URI", arg)
|
||||
}
|
||||
dest, path, remote := splitRemote(arg)
|
||||
dest, path, remote := c.splitRemote(arg)
|
||||
if !remote {
|
||||
continue
|
||||
}
|
||||
@@ -105,9 +117,22 @@ func (c Client) insertionPoint(args []string) int {
|
||||
return len(args)
|
||||
}
|
||||
|
||||
// splitRemote mirrors scp's colon(): an argument is remote when a ':' comes
|
||||
// splitRemote is splitHostPath with netssh's [user@]instance/host:path form.
|
||||
func (c Client) splitRemote(arg string) (dest, path string, remote bool) {
|
||||
if head, rest, ok := strings.Cut(arg, "/"); ok {
|
||||
instance := head[strings.LastIndex(head, "@")+1:]
|
||||
if c.instances[strings.ToLower(instance)] {
|
||||
if d, p, r := splitHostPath(rest); r {
|
||||
return head + "/" + d, p, true
|
||||
}
|
||||
}
|
||||
}
|
||||
return splitHostPath(arg)
|
||||
}
|
||||
|
||||
// splitHostPath mirrors scp's colon(): an argument is remote when a ':' comes
|
||||
// before any '/'. A leading "[" starts a bracketed IPv6 address.
|
||||
func splitRemote(arg string) (dest, path string, remote bool) {
|
||||
func splitHostPath(arg string) (dest, path string, remote bool) {
|
||||
if strings.HasPrefix(arg, "/") || strings.HasPrefix(arg, ".") {
|
||||
return "", "", false
|
||||
}
|
||||
|
||||
@@ -154,3 +154,27 @@ func TestApplyInsertsOptionsBeforeDoubleDash(t *testing.T) {
|
||||
t.Fatalf("got %v, want %v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
// Instance-qualified hosts contain "/" before the ":" of host:path; only
|
||||
// configured instances make such an argument remote.
|
||||
func TestInstanceQualifiedHostPath(t *testing.T) {
|
||||
scp := SCP.WithInstances([]string{"prod"})
|
||||
ops, err := scp.Operands([]string{"a.txt", "ops@prod/web01:/tmp/", "dir/file:x"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if want := []Operand{{Idx: 1, User: "ops", Host: "prod/web01", Path: "/tmp/"}}; !reflect.DeepEqual(ops, want) {
|
||||
t.Fatalf("got %+v", ops)
|
||||
}
|
||||
got := scp.Apply([]string{"a.txt", "ops@prod/web01:/tmp/"}, []int{1}, Host{Name: "web01", IP: "192.0.2.1"}, config.SSHOptions{}, WrapperFlags{}).Args
|
||||
if want := []string{"a.txt", "ops@192.0.2.1:/tmp/"}; !reflect.DeepEqual(got, want) {
|
||||
t.Fatalf("apply: %v", got)
|
||||
}
|
||||
sftp := SFTP.WithInstances([]string{"prod"})
|
||||
if p := sftp.Parse([]string{"prod/web01:/var/log"}); p == nil || p.Host != "prod/web01" || p.Path != "/var/log" {
|
||||
t.Fatalf("sftp: %+v", p)
|
||||
}
|
||||
if ops, _ := SCP.Operands([]string{"prod/web01:/tmp/", "."}); len(ops) != 0 {
|
||||
t.Fatalf("unknown instance must stay local: %+v", ops)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user