Skip to content

Commit

Permalink
Merge pull request #7188 from zhangguanzhang/network-404
Browse files Browse the repository at this point in the history
API returns 500 in case network is not found instead of 404
  • Loading branch information
openshift-merge-robot authored Aug 3, 2020
2 parents de5eb38 + 45b100d commit 96ece0c
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 14 deletions.
3 changes: 3 additions & 0 deletions libpod/define/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
7 changes: 3 additions & 4 deletions pkg/api/handlers/compat/networks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions pkg/api/handlers/libpod/networks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/api/handlers/utils/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
5 changes: 0 additions & 5 deletions pkg/network/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package network

import (
"encoding/json"
"errors"
"net"
)

Expand All @@ -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")
Expand Down
3 changes: 2 additions & 1 deletion pkg/network/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion pkg/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions test/apiv2/35-networks.at
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# -*- sh -*-
#
# network-related tests
#

t GET /networks/non-existing-network 404

# vim: filetype=sh

0 comments on commit 96ece0c

Please sign in to comment.