Skip to content

Commit

Permalink
fix: allow getting resources with number as name (#571)
Browse files Browse the repository at this point in the history
If the name of a user-named resource was numeric, it was impossible to
`Get` it from its name, since a number would always be interpreted as an
ID. This PR fixes this behavior on user-namable resources by continuing
to check if a resource with the given `idOrName` exists even though the
`idOrName` is numeric and no resource with the given ID was found.

See hetznercloud/cli#874
  • Loading branch information
phm07 authored Dec 20, 2024
1 parent 51baf8d commit b743a33
Show file tree
Hide file tree
Showing 24 changed files with 584 additions and 13 deletions.
5 changes: 4 additions & 1 deletion hcloud/certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,10 @@ func (c *CertificateClient) GetByName(ctx context.Context, name string) (*Certif
// retrieves a Certificate by its name. If the Certificate does not exist, nil is returned.
func (c *CertificateClient) Get(ctx context.Context, idOrName string) (*Certificate, *Response, error) {
if id, err := strconv.ParseInt(idOrName, 10, 64); err == nil {
return c.GetByID(ctx, id)
cert, res, err := c.GetByID(ctx, id)
if cert != nil || err != nil {
return cert, res, err
}
}
return c.GetByName(ctx, idOrName)
}
Expand Down
42 changes: 42 additions & 0 deletions hcloud/certificate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,48 @@ func TestCertificateClientGetByName(t *testing.T) {
})
}

func TestCertificateClientGetByNumericName(t *testing.T) {
env := newTestEnv()
defer env.Teardown()

env.Mux.HandleFunc("/certificates/123", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusNotFound)
json.NewEncoder(w).Encode(schema.ErrorResponse{
Error: schema.Error{
Code: string(ErrorCodeNotFound),
},
})
})

env.Mux.HandleFunc("/certificates", func(w http.ResponseWriter, r *http.Request) {
if r.URL.RawQuery != "name=123" {
t.Fatal("missing name query")
}
json.NewEncoder(w).Encode(schema.CertificateListResponse{
Certificates: []schema.Certificate{
{
ID: 1,
Name: "123",
},
},
})
})

ctx := context.Background()

certficate, _, err := env.Client.Certificate.Get(ctx, "123")
if err != nil {
t.Fatal(err)
}
if certficate == nil {
t.Fatal("no certficate")
}
if certficate.ID != 1 {
t.Errorf("unexpected certficate ID: %v", certficate.ID)
}
}

func TestCertificateClientGetByNameNotFound(t *testing.T) {
env := newTestEnv()
defer env.Teardown()
Expand Down
5 changes: 4 additions & 1 deletion hcloud/firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,10 @@ func (c *FirewallClient) GetByName(ctx context.Context, name string) (*Firewall,
// retrieves a Firewall by its name. If the Firewall does not exist, nil is returned.
func (c *FirewallClient) Get(ctx context.Context, idOrName string) (*Firewall, *Response, error) {
if id, err := strconv.ParseInt(idOrName, 10, 64); err == nil {
return c.GetByID(ctx, id)
fw, res, err := c.GetByID(ctx, id)
if fw != nil || err != nil {
return fw, res, err
}
}
return c.GetByName(ctx, idOrName)
}
Expand Down
42 changes: 42 additions & 0 deletions hcloud/firewall_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,48 @@ func TestFirewallClientGetByName(t *testing.T) {
})
}

func TestFirewallClientGetByNumericName(t *testing.T) {
env := newTestEnv()
defer env.Teardown()

env.Mux.HandleFunc("/firewalls/123", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusNotFound)
json.NewEncoder(w).Encode(schema.ErrorResponse{
Error: schema.Error{
Code: string(ErrorCodeNotFound),
},
})
})

env.Mux.HandleFunc("/firewalls", func(w http.ResponseWriter, r *http.Request) {
if r.URL.RawQuery != "name=123" {
t.Fatal("missing name query")
}
json.NewEncoder(w).Encode(schema.FirewallListResponse{
Firewalls: []schema.Firewall{
{
ID: 1,
Name: "123",
},
},
})
})

ctx := context.Background()

firewall, _, err := env.Client.Firewall.Get(ctx, "123")
if err != nil {
t.Fatal(err)
}
if firewall == nil {
t.Fatal("no firewall")
}
if firewall.ID != 1 {
t.Errorf("unexpected firewall ID: %v", firewall.ID)
}
}

func TestFirewallClientGetByNameNotFound(t *testing.T) {
env := newTestEnv()
defer env.Teardown()
Expand Down
5 changes: 4 additions & 1 deletion hcloud/floating_ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ func (c *FloatingIPClient) GetByName(ctx context.Context, name string) (*Floatin
// retrieves a Floating IP by its name. If the Floating IP does not exist, nil is returned.
func (c *FloatingIPClient) Get(ctx context.Context, idOrName string) (*FloatingIP, *Response, error) {
if id, err := strconv.ParseInt(idOrName, 10, 64); err == nil {
return c.GetByID(ctx, id)
ip, res, err := c.GetByID(ctx, id)
if ip != nil || err != nil {
return ip, res, err
}
}
return c.GetByName(ctx, idOrName)
}
Expand Down
44 changes: 44 additions & 0 deletions hcloud/floating_ip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,50 @@ func TestFloatingIPClientGetByName(t *testing.T) {
})
}

func TestFloatingIPClientGetByNumericName(t *testing.T) {
env := newTestEnv()
defer env.Teardown()

env.Mux.HandleFunc("/floating_ips/123", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusNotFound)
json.NewEncoder(w).Encode(schema.ErrorResponse{
Error: schema.Error{
Code: string(ErrorCodeNotFound),
},
})
})

env.Mux.HandleFunc("/floating_ips", func(w http.ResponseWriter, r *http.Request) {
if r.URL.RawQuery != "name=123" {
t.Fatal("missing name query")
}
json.NewEncoder(w).Encode(schema.FloatingIPListResponse{
FloatingIPs: []schema.FloatingIP{
{
ID: 1,
Name: "123",
Type: "ipv4",
IP: "131.232.99.1",
},
},
})
})

ctx := context.Background()

floatingIP, _, err := env.Client.FloatingIP.Get(ctx, "123")
if err != nil {
t.Fatal(err)
}
if floatingIP == nil {
t.Fatal("no Floating IP")
}
if floatingIP.ID != 1 {
t.Errorf("unexpected Floating IP ID: %v", floatingIP.ID)
}
}

func TestFloatingIPClientGetByNameNotFound(t *testing.T) {
env := newTestEnv()
defer env.Teardown()
Expand Down
10 changes: 8 additions & 2 deletions hcloud/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,10 @@ func (c *ImageClient) GetByNameAndArchitecture(ctx context.Context, name string,
// Deprecated: Use [ImageClient.GetForArchitecture] instead.
func (c *ImageClient) Get(ctx context.Context, idOrName string) (*Image, *Response, error) {
if id, err := strconv.ParseInt(idOrName, 10, 64); err == nil {
return c.GetByID(ctx, id)
img, res, err := c.GetByID(ctx, id)
if img != nil {
return img, res, err
}
}
return c.GetByName(ctx, idOrName)
}
Expand All @@ -146,7 +149,10 @@ func (c *ImageClient) Get(ctx context.Context, idOrName string) (*Image, *Respon
// check for this in your calling method.
func (c *ImageClient) GetForArchitecture(ctx context.Context, idOrName string, architecture Architecture) (*Image, *Response, error) {
if id, err := strconv.ParseInt(idOrName, 10, 64); err == nil {
return c.GetByID(ctx, id)
img, res, err := c.GetByID(ctx, id)
if img != nil || err != nil {
return img, res, err
}
}
return c.GetByNameAndArchitecture(ctx, idOrName, architecture)
}
Expand Down
41 changes: 41 additions & 0 deletions hcloud/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,47 @@ func TestImageClient(t *testing.T) {
})
})

t.Run("GetByNumericName", func(t *testing.T) {
env := newTestEnv()
defer env.Teardown()

env.Mux.HandleFunc("/images/123", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusNotFound)
json.NewEncoder(w).Encode(schema.ErrorResponse{
Error: schema.Error{
Code: string(ErrorCodeNotFound),
},
})
})

env.Mux.HandleFunc("/images", func(w http.ResponseWriter, r *http.Request) {
if r.URL.RawQuery != "name=123" {
t.Fatal("missing name query")
}
json.NewEncoder(w).Encode(schema.ImageListResponse{
Images: []schema.Image{
{
ID: 1,
},
},
})
})

ctx := context.Background()

image, _, err := env.Client.Image.Get(ctx, "123")
if err != nil {
t.Fatal(err)
}
if image == nil {
t.Fatal("no image")
}
if image.ID != 1 {
t.Errorf("unexpected image ID: %v", image.ID)
}
})

t.Run("GetByName (not found)", func(t *testing.T) {
env := newTestEnv()
defer env.Teardown()
Expand Down
5 changes: 4 additions & 1 deletion hcloud/iso.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ func (c *ISOClient) GetByName(ctx context.Context, name string) (*ISO, *Response
// Get retrieves an ISO by its ID if the input can be parsed as an integer, otherwise it retrieves an ISO by its name.
func (c *ISOClient) Get(ctx context.Context, idOrName string) (*ISO, *Response, error) {
if id, err := strconv.ParseInt(idOrName, 10, 64); err == nil {
return c.GetByID(ctx, id)
iso, res, err := c.GetByID(ctx, id)
if iso != nil || err != nil {
return iso, res, err
}
}
return c.GetByName(ctx, idOrName)
}
Expand Down
41 changes: 41 additions & 0 deletions hcloud/iso_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,47 @@ func TestISOClient(t *testing.T) {
})
})

t.Run("GetByNumericName", func(t *testing.T) {
env := newTestEnv()
defer env.Teardown()

env.Mux.HandleFunc("/isos/123", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusNotFound)
json.NewEncoder(w).Encode(schema.ErrorResponse{
Error: schema.Error{
Code: string(ErrorCodeNotFound),
},
})
})

env.Mux.HandleFunc("/isos", func(w http.ResponseWriter, r *http.Request) {
if r.URL.RawQuery != "name=123" {
t.Fatal("missing name query")
}
json.NewEncoder(w).Encode(schema.ISOListResponse{
ISOs: []schema.ISO{
{
ID: 1,
},
},
})
})

ctx := context.Background()

iso, _, err := env.Client.ISO.Get(ctx, "123")
if err != nil {
t.Fatal(err)
}
if iso == nil {
t.Fatal("no iso")
}
if iso.ID != 1 {
t.Errorf("unexpected iso ID: %v", iso.ID)
}
})

t.Run("GetByName (not found)", func(t *testing.T) {
env := newTestEnv()
defer env.Teardown()
Expand Down
5 changes: 4 additions & 1 deletion hcloud/load_balancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,10 @@ func (c *LoadBalancerClient) GetByName(ctx context.Context, name string) (*LoadB
// retrieves a Load Balancer by its name. If the Load Balancer does not exist, nil is returned.
func (c *LoadBalancerClient) Get(ctx context.Context, idOrName string) (*LoadBalancer, *Response, error) {
if id, err := strconv.ParseInt(idOrName, 10, 64); err == nil {
return c.GetByID(ctx, id)
lb, res, err := c.GetByID(ctx, id)
if lb != nil || err != nil {
return lb, res, err
}
}
return c.GetByName(ctx, idOrName)
}
Expand Down
42 changes: 42 additions & 0 deletions hcloud/load_balancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,48 @@ func TestLoadBalancerClientGetByName(t *testing.T) {
})
}

func TestLoadBalancerClientGetByNumericName(t *testing.T) {
env := newTestEnv()
defer env.Teardown()

env.Mux.HandleFunc("/load_balancers/123", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusNotFound)
json.NewEncoder(w).Encode(schema.ErrorResponse{
Error: schema.Error{
Code: string(ErrorCodeNotFound),
},
})
})

env.Mux.HandleFunc("/load_balancers", func(w http.ResponseWriter, r *http.Request) {
if r.URL.RawQuery != "name=123" {
t.Fatal("missing name query")
}
json.NewEncoder(w).Encode(schema.LoadBalancerListResponse{
LoadBalancers: []schema.LoadBalancer{
{
ID: 1,
Name: "123",
},
},
})
})

ctx := context.Background()

loadBalancer, _, err := env.Client.LoadBalancer.Get(ctx, "123")
if err != nil {
t.Fatal(err)
}
if loadBalancer == nil {
t.Fatal("no load balancer")
}
if loadBalancer.ID != 1 {
t.Errorf("unexpected load balancer ID: %v", loadBalancer.ID)
}
}

func TestLoadBalancerClientGetByNameNotFound(t *testing.T) {
env := newTestEnv()
defer env.Teardown()
Expand Down
5 changes: 4 additions & 1 deletion hcloud/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,10 @@ func (c *NetworkClient) GetByName(ctx context.Context, name string) (*Network, *
// retrieves a network by its name. If the network does not exist, nil is returned.
func (c *NetworkClient) Get(ctx context.Context, idOrName string) (*Network, *Response, error) {
if id, err := strconv.ParseInt(idOrName, 10, 64); err == nil {
return c.GetByID(ctx, id)
n, res, err := c.GetByID(ctx, id)
if n != nil || err != nil {
return n, res, err
}
}
return c.GetByName(ctx, idOrName)
}
Expand Down
Loading

0 comments on commit b743a33

Please sign in to comment.