Skip to content

Commit

Permalink
pkg/bindings: add new APIVersionError error type
Browse files Browse the repository at this point in the history
When a new API call is added to the bindings we should guard it based on
the version and throw a useful error. Right now an old server that does
not implement a given endpoint would throw a "NOT FOUND" error which is
not good for callers.

Instead implement a custom error type to give a usefule error instead.
This allows bindings users to call errors.As() to know if they call and
to old version.

Signed-off-by: Paul Holzinger <[email protected]>
  • Loading branch information
Luap99 authored and openshift-cherrypick-robot committed Dec 1, 2023
1 parent 86bb910 commit a8b8dc5
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/bindings/containers/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func ExecRemove(ctx context.Context, sessionID string, options *ExecRemoveOption
// The exec remove endpoint was added in 4.8.
if v.Major < 4 || (v.Major == 4 && v.Minor < 8) {
// Do no call this endpoint as it will not be supported on the server and throw an "NOT FOUND" error.
return nil
return bindings.NewAPIVersionError("/exec/{id}/remove", v, "4.8.0")
}
if options == nil {
options = new(ExecRemoveOptions)
Expand Down
24 changes: 24 additions & 0 deletions pkg/bindings/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io"

"github.com/blang/semver/v4"
"github.com/containers/podman/v4/pkg/errorhandling"
)

Expand Down Expand Up @@ -61,3 +62,26 @@ func CheckResponseCode(inError error) (int, error) {
return -1, errors.New("is not type ErrorModel")
}
}

type APIVersionError struct {
endpoint string
serverVersion *semver.Version
requiredVersion string
}

// NewAPIVersionError create bindings error when the endpoint on the server is not supported
// because the version is to old.
// - endpoint is the name fo the endpoint (e.g. /containers/json)
// - version is the server API version
// - requiredVersion is the server version need to use said endpoint
func NewAPIVersionError(endpoint string, version *semver.Version, requiredVersion string) *APIVersionError {
return &APIVersionError{
endpoint: endpoint,
serverVersion: version,
requiredVersion: requiredVersion,
}
}

func (e *APIVersionError) Error() string {
return fmt.Sprintf("API server version is %s, need at least %s to call %s", e.serverVersion.String(), e.requiredVersion, e.endpoint)
}
6 changes: 6 additions & 0 deletions pkg/domain/infra/tunnel/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/containers/podman/v4/libpod/define"
"github.com/containers/podman/v4/libpod/events"
"github.com/containers/podman/v4/pkg/api/handlers"
"github.com/containers/podman/v4/pkg/bindings"
"github.com/containers/podman/v4/pkg/bindings/containers"
"github.com/containers/podman/v4/pkg/bindings/images"
"github.com/containers/podman/v4/pkg/domain/entities"
Expand Down Expand Up @@ -588,6 +589,11 @@ func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrID string, o
}
defer func() {
if err := containers.ExecRemove(ic.ClientCtx, sessionID, nil); err != nil {
apiErr := new(bindings.APIVersionError)
if errors.As(err, &apiErr) {
// if the API is to old do not throw an error
return
}
if retErr == nil {
exitCode = -1
retErr = err
Expand Down

0 comments on commit a8b8dc5

Please sign in to comment.