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

1 files changed, 55 insertions, 10 deletions

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)
+}