summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorW. Trevor King <wking@tremily.us>2018-05-10 11:06:53 -0700
committerAtomic Bot <atomic-devel@projectatomic.io>2018-05-11 16:26:35 +0000
commit89430ffe65636b45cab834c94f176c1dc1d8a167 (patch)
treeca0620a2f47e1a805f874826f67e3a912935e570
parent4b22913e11208eb2a46085c1fede48a8e9168936 (diff)
downloadpodman-89430ffe65636b45cab834c94f176c1dc1d8a167.tar.gz
podman-89430ffe65636b45cab834c94f176c1dc1d8a167.tar.bz2
podman-89430ffe65636b45cab834c94f176c1dc1d8a167.zip
hooks: Order injection by collated JSON filename
We also considered ordering with sort.Strings, but Matthew rejected that because it uses a byte-by-byte UTF-8 comparison [1] which would fail many language-specific conventions [2]. There's some more discussion of the localeToLanguage mapping in [3]. Currently language.Parse does not handle either 'C' or 'POSIX', returning: und, language: tag is not well-formed for both. [1]: https://github.com/projectatomic/libpod/pull/686#issuecomment-387914358 [2]: https://en.wikipedia.org/wiki/Alphabetical_order#Language-specific_conventions [3]: https://github.com/golang/go/issues/25340 Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #686 Approved by: mheon
-rw-r--r--libpod/container_internal.go39
-rw-r--r--pkg/hooks/README.md8
-rw-r--r--pkg/hooks/hooks.go65
-rw-r--r--pkg/hooks/hooks_test.go49
-rw-r--r--pkg/hooks/monitor_test.go16
5 files changed, 157 insertions, 20 deletions
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 992c1d07b..1c0bcb07f 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -32,6 +32,7 @@ import (
"github.com/sirupsen/logrus"
"github.com/ulule/deepcopier"
"golang.org/x/sys/unix"
+ "golang.org/x/text/language"
)
const (
@@ -39,6 +40,15 @@ const (
artifactsDir = "artifacts"
)
+var (
+ // localeToLanguage maps from locale values to language tags.
+ localeToLanguage = map[string]string{
+ "": "und-u-va-posix",
+ "c": "und-u-va-posix",
+ "posix": "und-u-va-posix",
+ }
+)
+
// rootFsSize gets the size of the container's root filesystem
// A container FS is split into two parts. The first is the top layer, a
// mutable layer, and the rest is the RootFS: the set of immutable layers
@@ -1287,7 +1297,34 @@ func (c *Container) setupOCIHooks(ctx context.Context, g *generate.Generator) er
return nil
}
- manager, err := hooks.New(ctx, []string{c.runtime.config.HooksDir})
+ var locale string
+ var ok bool
+ for _, envVar := range []string{
+ "LC_ALL",
+ "LC_COLLATE",
+ "LANG",
+ } {
+ locale, ok = os.LookupEnv(envVar)
+ if ok {
+ break
+ }
+ }
+
+ langString, ok := localeToLanguage[strings.ToLower(locale)]
+ if !ok {
+ langString = locale
+ }
+
+ lang, err := language.Parse(langString)
+ if err != nil {
+ logrus.Warnf("failed to parse language %q: %s", langString, err)
+ lang, err = language.Parse("und-u-va-posix")
+ if err != nil {
+ return err
+ }
+ }
+
+ manager, err := hooks.New(ctx, []string{c.runtime.config.HooksDir}, lang)
if err != nil {
if c.runtime.config.HooksDirNotExistFatal || !os.IsNotExist(err) {
return err
diff --git a/pkg/hooks/README.md b/pkg/hooks/README.md
index e47000a09..0d7037222 100644
--- a/pkg/hooks/README.md
+++ b/pkg/hooks/README.md
@@ -23,6 +23,10 @@ For both `crio` and `podman`, hooks are read from `/usr/share/containers/oci/hoo
For `crio`, hook JSON is also read from `/etc/containers/oci/hooks.d/*.json`.
If files of with the same name exist in both directories, the one in `/etc/containers/oci/hooks.d` takes precedence.
+Hooks MUST be injected in the JSON filename case- and width-insensitive collation order.
+Collation order depends on your locale, as set by [`LC_ALL`][LC_ALL], [`LC_COLLATE`][LC_COLLATE], or [`LANG`][LANG] (in order of decreasing precedence).
+For example, in the [POSIX locale][LC_COLLATE-POSIX], a matching hook defined in `01-my-hook.json` would be injected before matching hooks defined in `02-another-hook.json` and `01-UPPERCASE.json`.
+
Each JSON file should contain an object with the following properties:
### 1.0.0 Hook Schema
@@ -160,6 +164,10 @@ $ cat /etc/containers/oci/hooks.d/osystemd-hook.json
```
[JSON]: https://tools.ietf.org/html/rfc8259
+[LANG]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_02
+[LC_ALL]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_02
+[LC_COLLATE]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap07.html#tag_07_03_02
+[LC_COLLATE-POSIX]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap07.html#tag_07_03_02_06
[nvidia-container-runtime-hook]: https://github.com/NVIDIA/nvidia-container-runtime/tree/master/hook/nvidia-container-runtime-hook
[oci-systemd-hook]: https://github.com/projectatomic/oci-systemd-hook
[oci-umount]: https://github.com/projectatomic/oci-umount
diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go
index f079dd0f7..337d6e3e2 100644
--- a/pkg/hooks/hooks.go
+++ b/pkg/hooks/hooks.go
@@ -10,6 +10,8 @@ import (
rspec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
current "github.com/projectatomic/libpod/pkg/hooks/1.0.0"
+ "golang.org/x/text/collate"
+ "golang.org/x/text/language"
)
// Version is the current hook configuration version.
@@ -26,18 +28,27 @@ const (
// Manager provides an opaque interface for managing CRI-O hooks.
type Manager struct {
hooks map[string]*current.Hook
+ language language.Tag
directories []string
lock sync.Mutex
}
+type namedHook struct {
+ name string
+ hook *current.Hook
+}
+
+type namedHooks []*namedHook
+
// New creates a new hook manager. Directories are ordered by
// increasing preference (hook configurations in later directories
// override configurations with the same filename from earlier
// directories).
-func New(ctx context.Context, directories []string) (manager *Manager, err error) {
+func New(ctx context.Context, directories []string, lang language.Tag) (manager *Manager, err error) {
manager = &Manager{
hooks: map[string]*current.Hook{},
directories: directories,
+ language: lang,
}
for _, dir := range directories {
@@ -50,29 +61,48 @@ func New(ctx context.Context, directories []string) (manager *Manager, err error
return manager, nil
}
-// Hooks injects OCI runtime hooks for a given container configuration.
-func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBindMounts bool) (err error) {
+// filenames returns sorted hook entries.
+func (m *Manager) namedHooks() (hooks []*namedHook) {
m.lock.Lock()
defer m.lock.Unlock()
+
+ hooks = make([]*namedHook, len(m.hooks))
+ i := 0
for name, hook := range m.hooks {
- match, err := hook.When.Match(config, annotations, hasBindMounts)
+ hooks[i] = &namedHook{
+ name: name,
+ hook: hook,
+ }
+ i++
+ }
+
+ return hooks
+}
+
+// Hooks injects OCI runtime hooks for a given container configuration.
+func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBindMounts bool) (err error) {
+ hooks := m.namedHooks()
+ collator := collate.New(m.language, collate.IgnoreCase, collate.IgnoreWidth)
+ collator.Sort(namedHooks(hooks))
+ for _, namedHook := range hooks {
+ match, err := namedHook.hook.When.Match(config, annotations, hasBindMounts)
if err != nil {
- return errors.Wrapf(err, "matching hook %q", name)
+ return errors.Wrapf(err, "matching hook %q", namedHook.name)
}
if match {
if config.Hooks == nil {
config.Hooks = &rspec.Hooks{}
}
- for _, stage := range hook.Stages {
+ for _, stage := range namedHook.hook.Stages {
switch stage {
case "prestart":
- config.Hooks.Prestart = append(config.Hooks.Prestart, hook.Hook)
+ config.Hooks.Prestart = append(config.Hooks.Prestart, namedHook.hook.Hook)
case "poststart":
- config.Hooks.Poststart = append(config.Hooks.Poststart, hook.Hook)
+ config.Hooks.Poststart = append(config.Hooks.Poststart, namedHook.hook.Hook)
case "poststop":
- config.Hooks.Poststop = append(config.Hooks.Poststop, hook.Hook)
+ config.Hooks.Poststop = append(config.Hooks.Poststop, namedHook.hook.Hook)
default:
- return fmt.Errorf("hook %q: unknown stage %q", name, stage)
+ return fmt.Errorf("hook %q: unknown stage %q", namedHook.name, stage)
}
}
}
@@ -102,3 +132,18 @@ func (m *Manager) add(path string) (err error) {
m.hooks[filepath.Base(path)] = hook
return nil
}
+
+// Len is part of the collate.Lister interface.
+func (hooks namedHooks) Len() int {
+ return len(hooks)
+}
+
+// Swap is part of the collate.Lister interface.
+func (hooks namedHooks) Swap(i, j int) {
+ hooks[i], hooks[j] = hooks[j], hooks[i]
+}
+
+// Bytes is part of the collate.Lister interface.
+func (hooks namedHooks) Bytes(i int) []byte {
+ return []byte(hooks[i].name)
+}
diff --git a/pkg/hooks/hooks_test.go b/pkg/hooks/hooks_test.go
index 109f6b046..9da2ec8b0 100644
--- a/pkg/hooks/hooks_test.go
+++ b/pkg/hooks/hooks_test.go
@@ -12,6 +12,7 @@ import (
rspec "github.com/opencontainers/runtime-spec/specs-go"
current "github.com/projectatomic/libpod/pkg/hooks/1.0.0"
"github.com/stretchr/testify/assert"
+ "golang.org/x/text/language"
)
// path is the path to an example hook executable.
@@ -26,13 +27,28 @@ func TestGoodNew(t *testing.T) {
}
defer os.RemoveAll(dir)
- jsonPath := filepath.Join(dir, "a.json")
- err = ioutil.WriteFile(jsonPath, []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\", \"poststart\", \"poststop\"]}", path)), 0644)
+ for i, name := range []string{
+ "01-my-hook.json",
+ "01-UPPERCASE.json",
+ "02-another-hook.json",
+ } {
+ jsonPath := filepath.Join(dir, name)
+ var extraStages string
+ if i == 0 {
+ extraStages = ", \"poststart\", \"poststop\""
+ }
+ err = ioutil.WriteFile(jsonPath, []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\", \"timeout\": %d}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"%s]}", path, i+1, extraStages)), 0644)
+ if err != nil {
+ t.Fatal(err)
+ }
+ }
+
+ lang, err := language.Parse("und-u-va-posix")
if err != nil {
t.Fatal(err)
}
- manager, err := New(ctx, []string{dir})
+ manager, err := New(ctx, []string{dir}, lang)
if err != nil {
t.Fatal(err)
}
@@ -43,20 +59,34 @@ func TestGoodNew(t *testing.T) {
t.Fatal(err)
}
+ one := 1
+ two := 2
+ three := 3
assert.Equal(t, &rspec.Hooks{
Prestart: []rspec.Hook{
{
- Path: path,
+ Path: path,
+ Timeout: &one,
+ },
+ {
+ Path: path,
+ Timeout: &two,
+ },
+ {
+ Path: path,
+ Timeout: &three,
},
},
Poststart: []rspec.Hook{
{
- Path: path,
+ Path: path,
+ Timeout: &one,
},
},
Poststop: []rspec.Hook{
{
- Path: path,
+ Path: path,
+ Timeout: &one,
},
},
}, config.Hooks)
@@ -77,7 +107,12 @@ func TestBadNew(t *testing.T) {
t.Fatal(err)
}
- _, err = New(ctx, []string{dir})
+ lang, err := language.Parse("und-u-va-posix")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ _, err = New(ctx, []string{dir}, lang)
if err == nil {
t.Fatal("unexpected success")
}
diff --git a/pkg/hooks/monitor_test.go b/pkg/hooks/monitor_test.go
index 9cbde1986..3887b2072 100644
--- a/pkg/hooks/monitor_test.go
+++ b/pkg/hooks/monitor_test.go
@@ -11,6 +11,7 @@ import (
rspec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/stretchr/testify/assert"
+ "golang.org/x/text/language"
)
func TestMonitorGood(t *testing.T) {
@@ -21,7 +22,12 @@ func TestMonitorGood(t *testing.T) {
}
defer os.RemoveAll(dir)
- manager, err := New(ctx, []string{dir})
+ lang, err := language.Parse("und-u-va-posix")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ manager, err := New(ctx, []string{dir}, lang)
if err != nil {
t.Fatal(err)
}
@@ -114,7 +120,13 @@ func TestMonitorGood(t *testing.T) {
func TestMonitorBadWatcher(t *testing.T) {
ctx := context.Background()
- manager, err := New(ctx, []string{})
+
+ lang, err := language.Parse("und-u-va-posix")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ manager, err := New(ctx, []string{}, lang)
if err != nil {
t.Fatal(err)
}