opensource/podman
1
0
Fork 0
mirror of https://github.com/containers/podman.git synced 2025-07-04 10:10:32 +08:00
Code Issues Packages Projects Releases Wiki Activity

Potentially breaking: Make hooks sort order locale-independent

Browse Source 
Don't sort OCI hooks using the locale collation order; it does not
make sense for the same system-wide directory to be interpreted differently
depending on the user's LC_COLLATE setting, and the language-specific
collation order can even change over time.

Besides, the current collation order determination code has never worked
with the most common LC_COLLATE values like en_US.UTF-8.

Ideally, we would like to just order based on Unicode code points
to be reliably stable, but the existing implementation is case-insensitive,
so we are forced to rely on the unicode case mapping tables at least.

(This gives up on canonicalization and width-insensitivity, potentially
breaking users who rely on these previously documented properties.)

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This commit is contained in:
Miloslav Trmač
2019-03-02 06:36:44 +01:00
parent fe79bdd07e
commit 97c9115c02
6 changed files with 15 additions and 154 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
libpod/container_internal.go
View File

@ -25,7 +25,6 @@ import (
opentracing "github.com/opentracing/opentracing-go"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"golang.org/x/text/language"
kwait "k8s.io/apimachinery/pkg/util/wait"
)
@ -34,15 +33,6 @@ const (
artifactsDir = "artifacts"
)
var (
// localeToLanguageMap maps from locale values to language tags.
localeToLanguageMap = 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
@ -1283,48 +1273,15 @@ func (c *Container) saveSpec(spec *spec.Spec) error {
return nil
}
// localeToLanguage translates POSIX locale strings to BCP 47 language tags.
func localeToLanguage(locale string) string {
locale = strings.Replace(strings.SplitN(locale, ".", 2)[0], "_", "-", 1)
langString, ok := localeToLanguageMap[strings.ToLower(locale)]
if !ok {
langString = locale
}
return langString
}
// Warning: precreate hooks may alter 'config' in place.
func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (extensionStageHooks map[string][]spec.Hook, err error) {
var locale string
var ok bool
for _, envVar := range []string{
"LC_ALL",
"LC_COLLATE",
"LANG",
} {
locale, ok = os.LookupEnv(envVar)
if ok {
break
}
}
langString := localeToLanguage(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 nil, err
}
}
allHooks := make(map[string][]spec.Hook)
if c.runtime.config.HooksDir == nil {
if rootless.IsRootless() {
return nil, nil
}
for _, hDir := range []string{hooks.DefaultDir, hooks.OverrideDir} {
manager, err := hooks.New(ctx, []string{hDir}, []string{"precreate", "poststop"}, lang)
manager, err := hooks.New(ctx, []string{hDir}, []string{"precreate", "poststop"})
if err != nil {
if os.IsNotExist(err) {
continue
@ -1343,7 +1300,7 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (exten
}
}
} else {
manager, err := hooks.New(ctx, c.runtime.config.HooksDir, []string{"precreate", "poststop"}, lang)
manager, err := hooks.New(ctx, c.runtime.config.HooksDir, []string{"precreate", "poststop"})
if err != nil {
if os.IsNotExist(err) {
logrus.Warnf("Requested OCI hooks directory %q does not exist", c.runtime.config.HooksDir)

48
libpod/container_internal_test.go
View File

@ -17,54 +17,6 @@ import (
// hookPath is the path to an example hook executable.
var hookPath string
func TestLocaleToLanguage(t *testing.T) {
for _, testCase := range []struct {
locale string
language string
}{
{
locale: "",
language: "und-u-va-posix",
},
{
locale: "C",
language: "und-u-va-posix",
},
{
locale: "POSIX",
language: "und-u-va-posix",
},
{
locale: "c",
language: "und-u-va-posix",
},
{
locale: "en",
language: "en",
},
{
locale: "en_US",
language: "en-US",
},
{
locale: "en.UTF-8",
language: "en",
},
{
locale: "en_US.UTF-8",
language: "en-US",
},
{
locale: "does-not-exist",
language: "does-not-exist",
},
} {
t.Run(testCase.locale, func(t *testing.T) {
assert.Equal(t, testCase.language, localeToLanguage(testCase.locale))
})
}
}
func TestPostDeleteHooks(t *testing.T) {
ctx := context.Background()
dir, err := ioutil.TempDir("", "libpod_test_")

7
pkg/hooks/docs/oci-hooks.5.md
View File

@ -23,10 +23,9 @@ If multiple directories are configured, a JSON filename in a preferred directory
Tools consuming this format may also opt to monitor the hook directries for changes, in which case they will notice additions, changes, and removals to JSON files without needing to be restarted or otherwise signaled. When the tool monitors multiple hooks directories, the precedence discussed in the previous paragraph still applies. For example, if a consuming tool watches for hooks in `/etc/containers/oci/hooks.d` and `/usr/share/containers/oci/hooks.d` (in order of decreasing precedence), then writing a new hook definition to `/etc/containers/oci/hooks.d/01-my-hook.json` will mask the hook previously loaded from `/usr/share/containers/oci/hooks.d/01-my-hook.json`. Subsequent changes to `/usr/share/containers/oci/hooks.d/01-my-hook.json` will have no effect on the consuming tool as long as `/etc/containers/oci/hooks.d/01-my-hook.json` exists. Removing `/etc/containers/oci/hooks.d/01-my-hook.json` will reload the hook from `/usr/share/containers/oci/hooks.d/01-my-hook.json`.
Hooks are injected in the JSON filename case- and width-insensitive collation order.
Collation order depends on your locale, as set by `LC_ALL`, `LC_COLLATE`, or `LANG` (in order of decreasing precedence).
For more information, see `locale(7)`.
For example, in the POSIX locale, 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`.
Hooks are injected in the order obtained by sorting the JSON file names, after converting them to lower case, based on their Unicode code points.
For example, 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`.
It is strongly recommended to make the sort oder unambiguous depending on an ASCII-only prefix (like the `01`/`02` above).
Each JSON file should contain an object with one of the following schemas.

30
pkg/hooks/hooks.go
View File

@ -5,14 +5,14 @@ import (
"context"
"fmt"
"path/filepath"
"sort"
"strings"
"sync"
current "github.com/containers/libpod/pkg/hooks/1.0.0"
rspec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"golang.org/x/text/collate"
"golang.org/x/text/language"
)
// Version is the current hook configuration version.
@ -31,7 +31,6 @@ type Manager struct {
hooks map[string]*current.Hook
directories []string
extensionStages []string
language language.Tag
lock sync.Mutex
}
@ -40,8 +39,6 @@ type namedHook struct {
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
@ -51,12 +48,11 @@ type namedHooks []*namedHook
// those specified in the OCI Runtime Specification and to control
// OCI-defined stages instead of delagating to the OCI runtime. See
// Hooks() for more information.
func New(ctx context.Context, directories []string, extensionStages []string, lang language.Tag) (manager *Manager, err error) {
func New(ctx context.Context, directories []string, extensionStages []string) (manager *Manager, err error) {
manager = &Manager{
hooks: map[string]*current.Hook{},
directories: directories,
extensionStages: extensionStages,
language: lang,
}
for _, dir := range directories {
@ -94,15 +90,14 @@ func (m *Manager) namedHooks() (hooks []*namedHook) {
// extensionStageHooks. This takes precedence over their inclusion in
// the OCI configuration. For example:
//
// manager, err := New(ctx, []string{DefaultDir}, []string{"poststop"}, lang)
// manager, err := New(ctx, []string{DefaultDir}, []string{"poststop"})
// extensionStageHooks, err := manager.Hooks(config, annotations, hasBindMounts)
//
// will have any matching post-stop hooks in extensionStageHooks and
// will not insert them into config.Hooks.Poststop.
func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBindMounts bool) (extensionStageHooks map[string][]rspec.Hook, err error) {
hooks := m.namedHooks()
collator := collate.New(m.language, collate.IgnoreCase, collate.IgnoreWidth)
collator.Sort(namedHooks(hooks))
sort.Slice(hooks, func(i, j int) bool { return strings.ToLower(hooks[i].name) < strings.ToLower(hooks[j].name) })
localStages := map[string]bool{} // stages destined for extensionStageHooks
for _, stage := range m.extensionStages {
localStages[stage] = true
@ -166,18 +161,3 @@ 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)
}

15
pkg/hooks/hooks_test.go
View File

@ -12,7 +12,6 @@ import (
current "github.com/containers/libpod/pkg/hooks/1.0.0"
rspec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/stretchr/testify/assert"
"golang.org/x/text/language"
)
// path is the path to an example hook executable.
@ -43,12 +42,7 @@ func TestGoodNew(t *testing.T) {
}
}
lang, err := language.Parse("und-u-va-posix")
if err != nil {
t.Fatal(err)
}
manager, err := New(ctx, []string{dir}, []string{}, lang)
manager, err := New(ctx, []string{dir}, []string{})
if err != nil {
t.Fatal(err)
}
@ -110,12 +104,7 @@ func TestBadNew(t *testing.T) {
t.Fatal(err)
}
lang, err := language.Parse("und-u-va-posix")
if err != nil {
t.Fatal(err)
}
_, err = New(ctx, []string{dir}, []string{}, lang)
_, err = New(ctx, []string{dir}, []string{})
if err == nil {
t.Fatal("unexpected success")
}

22
pkg/hooks/monitor_test.go
View File

@ -11,7 +11,6 @@ import (
rspec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/stretchr/testify/assert"
"golang.org/x/text/language"
)
func TestMonitorOneDirGood(t *testing.T) {
@ -22,12 +21,7 @@ func TestMonitorOneDirGood(t *testing.T) {
}
defer os.RemoveAll(dir)
lang, err := language.Parse("und-u-va-posix")
if err != nil {
t.Fatal(err)
}
manager, err := New(ctx, []string{dir}, []string{}, lang)
manager, err := New(ctx, []string{dir}, []string{})
if err != nil {
t.Fatal(err)
}
@ -132,12 +126,7 @@ func TestMonitorTwoDirGood(t *testing.T) {
}
defer os.RemoveAll(fallbackDir)
lang, err := language.Parse("und-u-va-posix")
if err != nil {
t.Fatal(err)
}
manager, err := New(ctx, []string{fallbackDir, primaryDir}, []string{}, lang)
manager, err := New(ctx, []string{fallbackDir, primaryDir}, []string{})
if err != nil {
t.Fatal(err)
}
@ -312,12 +301,7 @@ func TestMonitorTwoDirGood(t *testing.T) {
func TestMonitorBadWatcher(t *testing.T) {
ctx := context.Background()
lang, err := language.Parse("und-u-va-posix")
if err != nil {
t.Fatal(err)
}
manager, err := New(ctx, []string{}, []string{}, lang)
manager, err := New(ctx, []string{}, []string{})
if err != nil {
t.Fatal(err)
}