From 89cb0e8a41caf684a46c6180a9fc3ad518f78a5b Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Mon, 21 Nov 2022 09:37:40 +0000 Subject: actually follow the spec for java "properties" format * filter out various "interactive shell" variables * refactor to get the effective environment --- client/go/jvm/application_container.go | 6 +- client/go/jvm/container.go | 2 + client/go/jvm/jdisc_options.go | 17 ------ client/go/jvm/mem_avail_test.go | 2 +- client/go/jvm/properties.go | 106 +++++++++++++++++++++++++++++++++ client/go/jvm/properties_test.go | 27 +++++++++ client/go/jvm/standalone_container.go | 8 +-- client/go/prog/run.go | 2 +- client/go/prog/spec_env.go | 2 +- 9 files changed, 140 insertions(+), 32 deletions(-) create mode 100644 client/go/jvm/properties.go create mode 100644 client/go/jvm/properties_test.go (limited to 'client') diff --git a/client/go/jvm/application_container.go b/client/go/jvm/application_container.go index e1ce45f890c..1cbc8877aa3 100644 --- a/client/go/jvm/application_container.go +++ b/client/go/jvm/application_container.go @@ -53,11 +53,7 @@ func (a *ApplicationContainer) addJdiscProperties() { propsFile := fmt.Sprintf("%s/%s.properties", containerHomeDir, "jdisc") opts.fixSpec.FixDir(containerHomeDir) opts.fixSpec.FixDir(bCacheDir) - trace.Trace("write props file:", propsFile) - err := os.WriteFile(propsFile, selectedEnv(), 0600) - if err != nil { - util.JustExitWith(err) - } + a.propsFile = propsFile opts.AddOption("-Djdisc.config.file=" + propsFile) opts.AddOption("-Djdisc.cache.path=" + bCacheDir) opts.AddOption("-Djdisc.logger.tag=" + cfgId) diff --git a/client/go/jvm/container.go b/client/go/jvm/container.go index 3a66a2d37c4..53ff9f9a809 100644 --- a/client/go/jvm/container.go +++ b/client/go/jvm/container.go @@ -26,6 +26,7 @@ type containerBase struct { configId string serviceName string jvmArgs *Options + propsFile string } func (cb *containerBase) ServiceName() string { @@ -67,6 +68,7 @@ func (cb *containerBase) Exec() { p := prog.NewSpec(argv) p.ConfigureNumaCtl() cb.JvmOptions().exportEnvSettings(p) + writeEnvAsProperties(p.EffectiveEnv(), cb.propsFile) trace.Info("starting container; env:", readableEnv(p.Env)) trace.Info("starting container; exec:", argv) err := p.Run() diff --git a/client/go/jvm/jdisc_options.go b/client/go/jvm/jdisc_options.go index 32cfb840336..84348bbda63 100644 --- a/client/go/jvm/jdisc_options.go +++ b/client/go/jvm/jdisc_options.go @@ -4,10 +4,6 @@ package jvm import ( - "bytes" - "os" - "strings" - "github.com/vespa-engine/vespa/client/go/defaults" ) @@ -16,19 +12,6 @@ const ( DEFAULT_CP_FILE = "lib/jars/jdisc_core-jar-with-dependencies.jar" ) -func selectedEnv() []byte { - var buf bytes.Buffer - for _, vv := range os.Environ() { - varName := strings.Split(vv, "=")[0] - if strings.Contains(vv, "\\u") || strings.Contains(vv, "\n") || strings.Contains(varName, "%%") { - continue - } - buf.WriteString(vv) - buf.WriteString("\n") - } - return buf.Bytes() -} - func (opts *Options) AddCommonJdiscProperties() { logCtlDir := defaults.UnderVespaHome("var/db/vespa/logcontrol") opts.fixSpec.FixDir(logCtlDir) diff --git a/client/go/jvm/mem_avail_test.go b/client/go/jvm/mem_avail_test.go index 34366759523..e85f2876d3a 100644 --- a/client/go/jvm/mem_avail_test.go +++ b/client/go/jvm/mem_avail_test.go @@ -9,7 +9,7 @@ import ( ) func TestCg2Get(t *testing.T) { - trace.AdjustVerbosity(2) + trace.AdjustVerbosity(0) const MM = "memory.max" res, err := vespa_cg2get(MM) diff --git a/client/go/jvm/properties.go b/client/go/jvm/properties.go new file mode 100644 index 00000000000..6d944c82c2a --- /dev/null +++ b/client/go/jvm/properties.go @@ -0,0 +1,106 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Author: arnej + +package jvm + +import ( + "bytes" + "fmt" + "os" + "sort" + "strings" + + "github.com/vespa-engine/vespa/client/go/trace" + "github.com/vespa-engine/vespa/client/go/util" +) + +// quote as specified in JDK source file java.base/share/classes/java/util/Properties.java +func propQuote(s string, buf *bytes.Buffer) { + inKey := true + inVal := false + for _, ch := range s { + needQ := false + switch { + case ch == ' ': + needQ = !inVal + case ch == ':': + needQ = inKey + case ch < ' ': + needQ = true + case ch > '~': + needQ = true + case ch == '\\': + buf.WriteString("\\") + case ch == '=': + inKey = false + default: + inVal = !inKey + } + if needQ { + fmt.Fprintf(buf, "\\u%04X", ch) + } else { + buf.WriteRune(ch) + } + } +} + +func envAsProperties(envv []string) []byte { + suppress := map[string]bool{ + "_": true, + "HISTCONTROL": true, + "HISTSIZE": true, + "IFS": true, + "LESSOPEN": true, + "LOADEDMODULES": true, + "LS_COLORS": true, + "MAIL": true, + "MODULEPATH": true, + "MODULEPATH_modshare": true, + "MODULESHOME": true, + "MODULES_CMD": true, + "MODULES_RUN_QUARANTINE": true, + "OLDPWD": true, + "PCP_DIR": true, + "PS1": true, + "PWD": true, + "SHLVL": true, + "SSH_AUTH_SOCK": true, + "SSH_CLIENT": true, + "SSH_CONNECTION": true, + "SSH_TTY": true, + "S_COLORS": true, + "which_declare": true, + "": true, + } + var buf bytes.Buffer + buf.WriteString("# properties converted from environment variables\n") + sort.Strings(envv) + for _, env := range envv { + parts := strings.Split(env, "=") + if len(parts) >= 2 { + varName := parts[0] + if suppress[varName] || strings.Contains(varName, "%%") { + continue + } + if strings.Contains(env, "\n") { + continue + } + propQuote(env, &buf) + buf.WriteRune('\n') + } else { + trace.Warning("environment value without '=':", env) + } + } + return buf.Bytes() +} + +func writeEnvAsProperties(envv []string, propsFile string) { + if propsFile == "" { + panic("missing propsFile") + } + trace.Trace("write props file:", propsFile) + err := os.WriteFile(propsFile, envAsProperties(envv), 0600) + if err != nil { + util.JustExitWith(err) + } +} diff --git a/client/go/jvm/properties_test.go b/client/go/jvm/properties_test.go new file mode 100644 index 00000000000..c417319a51e --- /dev/null +++ b/client/go/jvm/properties_test.go @@ -0,0 +1,27 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package jvm + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSomeQuoting(t *testing.T) { + var buf bytes.Buffer + propQuote("a=b", &buf) + assert.Equal(t, "a=b", buf.String()) + buf.Reset() + propQuote("foobar", &buf) + assert.Equal(t, "foobar", buf.String()) + buf.Reset() + propQuote("x y = foo bar ", &buf) + assert.Equal(t, `x\u0020y\u0020=\u0020foo bar `, buf.String()) + buf.Reset() + propQuote("x:y=foo:bar", &buf) + assert.Equal(t, "x\\u003Ay=foo:bar", buf.String()) + buf.Reset() + propQuote(`PS1=\[\e[0;32m\]AWS-US\[\e[0m\] [\u@\[\e[1m\]\h\[\e[0m\]:\w]$ `, &buf) + assert.Equal(t, `PS1=\\[\\e[0;32m\\]AWS-US\\[\\e[0m\\] [\\u@\\[\\e[1m\\]\\h\\[\\e[0m\\]:\\w]$ `, buf.String()) +} diff --git a/client/go/jvm/standalone_container.go b/client/go/jvm/standalone_container.go index edaea346889..33007862f58 100644 --- a/client/go/jvm/standalone_container.go +++ b/client/go/jvm/standalone_container.go @@ -5,12 +5,10 @@ package jvm import ( "fmt" - "os" "github.com/vespa-engine/vespa/client/go/defaults" "github.com/vespa-engine/vespa/client/go/envvars" "github.com/vespa-engine/vespa/client/go/prog" - "github.com/vespa-engine/vespa/client/go/trace" "github.com/vespa-engine/vespa/client/go/util" ) @@ -70,11 +68,7 @@ func (a *StandaloneContainer) addJdiscProperties() { opts.fixSpec.FixDir(containerParentDir) opts.fixSpec.FixDir(bCacheParentDir) opts.fixSpec.FixDir(bCacheDir) - trace.Trace("write props file:", propsFile) - err := os.WriteFile(propsFile, selectedEnv(), 0600) - if err != nil { - util.JustExitWith(err) - } + a.propsFile = propsFile opts.AddOption("-Djdisc.export.packages=") opts.AddOption("-Djdisc.config.file=" + propsFile) opts.AddOption("-Djdisc.cache.path=" + bCacheDir) diff --git a/client/go/prog/run.go b/client/go/prog/run.go index 29f35837bf5..0d11cef4d30 100644 --- a/client/go/prog/run.go +++ b/client/go/prog/run.go @@ -21,6 +21,6 @@ func (spec *Spec) Run() error { if spec.shouldUseVespaMalloc { spec.Setenv(envvars.LD_PRELOAD, spec.vespaMallocPreload) } - envv := spec.effectiveEnv() + envv := spec.EffectiveEnv() return util.Execvpe(prog, args, envv) } diff --git a/client/go/prog/spec_env.go b/client/go/prog/spec_env.go index ffdafe7676b..b40b3f172fd 100644 --- a/client/go/prog/spec_env.go +++ b/client/go/prog/spec_env.go @@ -21,7 +21,7 @@ func (p *Spec) Getenv(k string) string { return os.Getenv(k) } -func (spec *Spec) effectiveEnv() []string { +func (spec *Spec) EffectiveEnv() []string { envMap := make(map[string]string) addToMap := func(kv string) { for idx, elem := range kv { -- cgit v1.2.3