From 45b100d21ce1ca1843e26366466d09eac2d4add0 Mon Sep 17 00:00:00 2001 From: zhangguanzhang Date: Sun, 2 Aug 2020 21:08:13 +0800 Subject: API returns 500 in case network is not found instead of 404 Signed-off-by: zhangguanzhang --- libpod/define/errors.go | 3 +++ pkg/api/handlers/compat/networks.go | 7 +++---- pkg/api/handlers/libpod/networks.go | 6 +++--- pkg/api/handlers/utils/errors.go | 9 +++++++++ pkg/network/config.go | 5 ----- pkg/network/files.go | 3 ++- pkg/network/network.go | 3 ++- test/apiv2/35-networks.at | 8 ++++++++ 8 files changed, 30 insertions(+), 14 deletions(-) create mode 100644 test/apiv2/35-networks.at diff --git a/libpod/define/errors.go b/libpod/define/errors.go index 4a0df3983..6e372eb5e 100644 --- a/libpod/define/errors.go +++ b/libpod/define/errors.go @@ -20,6 +20,9 @@ var ( // ErrNoSuchVolume indicates the requested volume does not exist ErrNoSuchVolume = errors.New("no such volume") + // ErrNoSuchNetwork indicates the requested network does not exist + ErrNoSuchNetwork = errors.New("network not found") + // ErrNoSuchExecSession indicates that the requested exec session does // not exist. ErrNoSuchExecSession = errors.New("no such exec session") diff --git a/pkg/api/handlers/compat/networks.go b/pkg/api/handlers/compat/networks.go index 1e80cc91d..80b7505df 100644 --- a/pkg/api/handlers/compat/networks.go +++ b/pkg/api/handlers/compat/networks.go @@ -10,6 +10,7 @@ import ( "github.com/containernetworking/cni/libcni" "github.com/containers/podman/v2/libpod" + "github.com/containers/podman/v2/libpod/define" "github.com/containers/podman/v2/pkg/api/handlers/utils" "github.com/containers/podman/v2/pkg/domain/entities" "github.com/containers/podman/v2/pkg/domain/infra/abi" @@ -44,9 +45,7 @@ func InspectNetwork(w http.ResponseWriter, r *http.Request) { name := utils.GetName(r) _, err = network.InspectNetwork(config, name) if err != nil { - // TODO our network package does not distinguish between not finding a - // specific network vs not being able to read it - utils.InternalServerError(w, err) + utils.NetworkNotFound(w, name, err) return } report, err := getNetworkResourceByName(name, runtime) @@ -285,7 +284,7 @@ func RemoveNetwork(w http.ResponseWriter, r *http.Request) { return } if !exists { - utils.Error(w, "network not found", http.StatusNotFound, network.ErrNetworkNotFound) + utils.Error(w, "network not found", http.StatusNotFound, define.ErrNoSuchNetwork) return } if err := network.RemoveNetwork(config, name); err != nil { diff --git a/pkg/api/handlers/libpod/networks.go b/pkg/api/handlers/libpod/networks.go index 9237a41ce..475522664 100644 --- a/pkg/api/handlers/libpod/networks.go +++ b/pkg/api/handlers/libpod/networks.go @@ -5,10 +5,10 @@ import ( "net/http" "github.com/containers/podman/v2/libpod" + "github.com/containers/podman/v2/libpod/define" "github.com/containers/podman/v2/pkg/api/handlers/utils" "github.com/containers/podman/v2/pkg/domain/entities" "github.com/containers/podman/v2/pkg/domain/infra/abi" - "github.com/containers/podman/v2/pkg/network" "github.com/gorilla/schema" "github.com/pkg/errors" ) @@ -78,7 +78,7 @@ func RemoveNetwork(w http.ResponseWriter, r *http.Request) { } if reports[0].Err != nil { // If the network cannot be found, we return a 404. - if errors.Cause(err) == network.ErrNetworkNotFound { + if errors.Cause(err) == define.ErrNoSuchNetwork { utils.Error(w, "Something went wrong", http.StatusNotFound, err) return } @@ -104,7 +104,7 @@ func InspectNetwork(w http.ResponseWriter, r *http.Request) { reports, err := ic.NetworkInspect(r.Context(), []string{name}, options) if err != nil { // If the network cannot be found, we return a 404. - if errors.Cause(err) == network.ErrNetworkNotFound { + if errors.Cause(err) == define.ErrNoSuchNetwork { utils.Error(w, "Something went wrong", http.StatusNotFound, err) return } diff --git a/pkg/api/handlers/utils/errors.go b/pkg/api/handlers/utils/errors.go index 5a99529c6..bf9b18960 100644 --- a/pkg/api/handlers/utils/errors.go +++ b/pkg/api/handlers/utils/errors.go @@ -39,6 +39,7 @@ func VolumeNotFound(w http.ResponseWriter, name string, err error) { msg := fmt.Sprintf("No such volume: %s", name) Error(w, msg, http.StatusNotFound, err) } + func ContainerNotFound(w http.ResponseWriter, name string, err error) { if errors.Cause(err) != define.ErrNoSuchCtr { InternalServerError(w, err) @@ -55,6 +56,14 @@ func ImageNotFound(w http.ResponseWriter, name string, err error) { Error(w, msg, http.StatusNotFound, err) } +func NetworkNotFound(w http.ResponseWriter, name string, err error) { + if errors.Cause(err) != define.ErrNoSuchNetwork { + InternalServerError(w, err) + } + msg := fmt.Sprintf("No such network: %s", name) + Error(w, msg, http.StatusNotFound, err) +} + func PodNotFound(w http.ResponseWriter, name string, err error) { if errors.Cause(err) != define.ErrNoSuchPod { InternalServerError(w, err) diff --git a/pkg/network/config.go b/pkg/network/config.go index a504e0ad0..0115433e1 100644 --- a/pkg/network/config.go +++ b/pkg/network/config.go @@ -2,7 +2,6 @@ package network import ( "encoding/json" - "errors" "net" ) @@ -20,10 +19,6 @@ const ( DefaultPodmanDomainName = "dns.podman" ) -var ( - ErrNetworkNotFound = errors.New("network not found") -) - // GetDefaultPodmanNetwork outputs the default network for podman func GetDefaultPodmanNetwork() (*net.IPNet, error) { _, n, err := net.ParseCIDR("10.88.1.0/24") diff --git a/pkg/network/files.go b/pkg/network/files.go index beb3289f3..38ce38b97 100644 --- a/pkg/network/files.go +++ b/pkg/network/files.go @@ -10,6 +10,7 @@ import ( "github.com/containernetworking/cni/libcni" "github.com/containernetworking/plugins/plugins/ipam/host-local/backend/allocator" "github.com/containers/common/pkg/config" + "github.com/containers/podman/v2/libpod/define" "github.com/pkg/errors" ) @@ -55,7 +56,7 @@ func GetCNIConfigPathByName(config *config.Config, name string) (string, error) return confFile, nil } } - return "", errors.Wrap(ErrNetworkNotFound, fmt.Sprintf("unable to find network configuration for %s", name)) + return "", errors.Wrap(define.ErrNoSuchNetwork, fmt.Sprintf("unable to find network configuration for %s", name)) } // ReadRawCNIConfByName reads the raw CNI configuration for a CNI diff --git a/pkg/network/network.go b/pkg/network/network.go index 6c84c8a8a..b24c72f5f 100644 --- a/pkg/network/network.go +++ b/pkg/network/network.go @@ -8,6 +8,7 @@ import ( "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/plugins/plugins/ipam/host-local/backend/allocator" "github.com/containers/common/pkg/config" + "github.com/containers/podman/v2/libpod/define" "github.com/containers/podman/v2/pkg/util" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -200,7 +201,7 @@ func InspectNetwork(config *config.Config, name string) (map[string]interface{}, func Exists(config *config.Config, name string) (bool, error) { _, err := ReadRawCNIConfByName(config, name) if err != nil { - if errors.Cause(err) == ErrNetworkNotFound { + if errors.Cause(err) == define.ErrNoSuchNetwork { return false, nil } return false, err diff --git a/test/apiv2/35-networks.at b/test/apiv2/35-networks.at new file mode 100644 index 000000000..fff3f3b1f --- /dev/null +++ b/test/apiv2/35-networks.at @@ -0,0 +1,8 @@ +# -*- sh -*- +# +# network-related tests +# + +t GET /networks/non-existing-network 404 + +# vim: filetype=sh -- cgit v1.2.3-54-g00ecf