Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of old oci hooks #6631

Merged
merged 1 commit into from
Jun 17, 2020
Merged

Fix handling of old oci hooks #6631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions pkg/hooks/0.1.0/hook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"errors"
"strings"

"github.com/containers/libpod/pkg/hooks"
current "github.com/containers/libpod/pkg/hooks/1.0.0"
rspec "github.com/opencontainers/runtime-spec/specs-go"
)
Expand All @@ -32,8 +31,9 @@ type Hook struct {
HasBindMounts *bool `json:"hasbindmounts,omitempty"`
}

func read(content []byte) (hook *current.Hook, err error) {
func Read(content []byte) (hook *current.Hook, err error) {
var raw Hook

if err = json.Unmarshal(content, &raw); err != nil {
return nil, err
}
Expand Down Expand Up @@ -86,8 +86,3 @@ func read(content []byte) (hook *current.Hook, err error) {

return hook, nil
}

func init() {
hooks.Readers[""] = read
hooks.Readers[Version] = read
}
26 changes: 13 additions & 13 deletions pkg/hooks/0.1.0/hook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

func TestGood(t *testing.T) {
hook, err := read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"cmds\": [\"sh\"]}"))
hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"cmds\": [\"sh\"]}"))
if err != nil {
t.Fatal(err)
}
Expand All @@ -27,15 +27,15 @@ func TestGood(t *testing.T) {
}

func TestInvalidJSON(t *testing.T) {
_, err := read([]byte("{"))
_, err := Read([]byte("{"))
if err == nil {
t.Fatal("unexpected success")
}
assert.Regexp(t, "^unexpected end of JSON input$", err.Error())
}

func TestArguments(t *testing.T) {
hook, err := read([]byte("{\"hook\": \"/a/b/c\", \"arguments\": [\"d\", \"e\"], \"stages\": [\"prestart\"], \"cmds\": [\"sh\"]}"))
hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"arguments\": [\"d\", \"e\"], \"stages\": [\"prestart\"], \"cmds\": [\"sh\"]}"))
if err != nil {
t.Fatal(err)
}
Expand All @@ -54,23 +54,23 @@ func TestArguments(t *testing.T) {
}

func TestEmptyObject(t *testing.T) {
_, err := read([]byte("{}"))
_, err := Read([]byte("{}"))
if err == nil {
t.Fatal("unexpected success")
}
assert.Regexp(t, "^missing required property: hook$", err.Error())
}

func TestNoStages(t *testing.T) {
_, err := read([]byte("{\"hook\": \"/a/b/c\"}"))
_, err := Read([]byte("{\"hook\": \"/a/b/c\"}"))
if err == nil {
t.Fatal("unexpected success")
}
assert.Regexp(t, "^missing required property: stages$", err.Error())
}

func TestStage(t *testing.T) {
hook, err := read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"]}"))
hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"]}"))
if err != nil {
t.Fatal(err)
}
Expand All @@ -85,15 +85,15 @@ func TestStage(t *testing.T) {
}

func TestStagesAndStage(t *testing.T) {
_, err := read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"stage\": [\"prestart\"]}"))
_, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"stage\": [\"prestart\"]}"))
if err == nil {
t.Fatal("unexpected success")
}
assert.Regexp(t, "^cannot set both 'stage' and 'stages'$", err.Error())
}

func TestCmd(t *testing.T) {
hook, err := read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"cmd\": [\"sh\"]}"))
hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"cmd\": [\"sh\"]}"))
if err != nil {
t.Fatal(err)
}
Expand All @@ -111,15 +111,15 @@ func TestCmd(t *testing.T) {
}

func TestCmdsAndCmd(t *testing.T) {
_, err := read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"cmds\": [\"sh\"], \"cmd\": [\"true\"]}"))
_, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"cmds\": [\"sh\"], \"cmd\": [\"true\"]}"))
if err == nil {
t.Fatal("unexpected success")
}
assert.Regexp(t, "^cannot set both 'cmd' and 'cmds'$", err.Error())
}

func TestAnnotations(t *testing.T) {
hook, err := read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"annotations\": [\"a\", \"b\"]}"))
hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"annotations\": [\"a\", \"b\"]}"))
if err != nil {
t.Fatal(err)
}
Expand All @@ -137,7 +137,7 @@ func TestAnnotations(t *testing.T) {
}

func TestAnnotation(t *testing.T) {
hook, err := read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"annotation\": [\"a\", \"b\"]}"))
hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"annotation\": [\"a\", \"b\"]}"))
if err != nil {
t.Fatal(err)
}
Expand All @@ -155,15 +155,15 @@ func TestAnnotation(t *testing.T) {
}

func TestAnnotationsAndAnnotation(t *testing.T) {
_, err := read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"annotations\": [\"a\"], \"annotation\": [\"b\"]}"))
_, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"annotations\": [\"a\"], \"annotation\": [\"b\"]}"))
if err == nil {
t.Fatal("unexpected success")
}
assert.Regexp(t, "^cannot set both 'annotation' and 'annotations'$", err.Error())
}

func TestHasBindMounts(t *testing.T) {
hook, err := read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"hasbindmounts\": true}"))
hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"hasbindmounts\": true}"))
if err != nil {
t.Fatal(err)
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/hooks/read.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ package hooks

import (
"encoding/json"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"strings"

old "github.com/containers/libpod/pkg/hooks/0.1.0"
current "github.com/containers/libpod/pkg/hooks/1.0.0"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -49,7 +49,7 @@ func read(content []byte) (hook *current.Hook, err error) {
}
reader, ok := Readers[ver.Version]
if !ok {
return nil, fmt.Errorf("unrecognized hook version: %q", ver.Version)
return nil, errors.Errorf("unrecognized hook version: %q", ver.Version)
}

hook, err = reader(content)
Expand Down Expand Up @@ -95,4 +95,6 @@ func ReadDir(path string, extensionStages []string, hooks map[string]*current.Ho

func init() {
Readers[current.Version] = current.Read
Readers[old.Version] = old.Read
Readers[""] = old.Read
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it works, I guess, but this usage is new to me so of course I had to edit /usr/share/containers/oci/hooks.d/oci-umount.json and add "version": "xx", for several values of xx. Results:

Version Error
"0.1.0" works fine
"0.1.1" error setting up OCI Hooks ... unrecognized hook version: "0.1.1"
"1.0.0" error setting up OCI Hooks ... 1.0.0: json: cannot unmarshal string into Go struct field Hook.hook of type specs.Hook

I'll buy that my 1.0.0 is my own problem, probably because the format has changed somehow and I can't just add "version" without changing other things.

But, is there ever a possibility of a version other than 0.1.0, 1.0.0, or (absent)? Is there a way to specify a fallback Reader?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it's code. I will default to current.Read

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this helps...but I just tried editing /usr/share/containers/oci/hooks.d/oci-umount.json and add "version": "0.1.0", with result:

Error: error setting up OCI Hooks: parsing hook "/usr/share/containers/oci/hooks.d/oci-umount.json": unrecognized hook version: "0.1.0"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heatmiser Try that with my patch and it should work.

}