From c0dea5e31af856ed96b8257b5caa952161c8a05b Mon Sep 17 00:00:00 2001 From: Brian Pursley Date: Tue, 29 Nov 2022 23:09:57 -0500 Subject: [PATCH] i18n: Fix bug where package-level variables are not translated. Change i18n.T() to load translations if they have not yet been loaded. Added new integration tests to test help output translation. --- staging/src/k8s.io/kubectl/pkg/cmd/cmd.go | 7 - .../src/k8s.io/kubectl/pkg/util/i18n/i18n.go | 67 ++++++++- .../k8s.io/kubectl/pkg/util/i18n/i18n_test.go | 131 ++++++++++++++++++ test/cmd/help.sh | 52 +++++++ test/cmd/legacy-script.sh | 7 + 5 files changed, 256 insertions(+), 8 deletions(-) create mode 100644 test/cmd/help.sh diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go index 88e3a583deb..8313179b007 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go @@ -331,13 +331,6 @@ func NewKubectlCommand(o KubectlOptions) *cobra.Command { f := cmdutil.NewFactory(matchVersionKubeConfigFlags) - // Sending in 'nil' for the getLanguageFn() results in using - // the LANG environment variable. - // - // TODO: Consider adding a flag or file preference for setting - // the language, instead of just loading from the LANG env. variable. - i18n.LoadTranslations("kubectl", nil) - // Proxy command is incompatible with CommandHeaderRoundTripper, so // clear the WrapConfigFn before running proxy command. proxyCmd := proxy.NewCmdProxy(f, o.IOStreams) diff --git a/staging/src/k8s.io/kubectl/pkg/util/i18n/i18n.go b/staging/src/k8s.io/kubectl/pkg/util/i18n/i18n.go index 4aebe31812d..d850b283db1 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/i18n/i18n.go +++ b/staging/src/k8s.io/kubectl/pkg/util/i18n/i18n.go @@ -24,8 +24,10 @@ import ( "fmt" "os" "strings" + "sync" + + "github.com/chai2010/gettext-go" - gettext "github.com/chai2010/gettext-go" "k8s.io/klog/v2" ) @@ -52,6 +54,56 @@ var knownTranslations = map[string][]string{ }, } +var ( + lazyLoadTranslationsOnce sync.Once + LoadTranslationsFunc = func() error { + return LoadTranslations("kubectl", nil) + } + translationsLoaded bool +) + +// SetLoadTranslationsFunc sets the function called to lazy load translations. +// It must be called in an init() func that occurs BEFORE any i18n.T() calls are made by any package. You can +// accomplish this by creating a separate package containing your init() func, and then importing that package BEFORE +// any other packages that call i18n.T(). +// +// Example Usage: +// +// package myi18n +// +// import "k8s.io/kubectl/pkg/util/i18n" +// +// func init() { +// if err := i18n.SetLoadTranslationsFunc(loadCustomTranslations); err != nil { +// panic(err) +// } +// } +// +// func loadCustomTranslations() error { +// // Load your custom translations here... +// } +// +// And then in your main or root command package, import your custom package like this: +// +// import ( +// // Other imports that don't need i18n... +// _ "example.com/myapp/myi18n" +// // Other imports that do need i18n... +// ) +func SetLoadTranslationsFunc(f func() error) error { + if translationsLoaded { + return errors.New("translations have already been loaded") + } + LoadTranslationsFunc = func() error { + if err := f(); err != nil { + return err + } + translationsLoaded = true + return nil + } + return nil +} + func loadSystemLanguage() string { // Implements the following locale priority order: LC_ALL, LC_MESSAGES, LANG // Similarly to: https://www.gnu.org/software/gettext/manual/html_node/Locale-Environment-Variables.html @@ -128,13 +180,26 @@ func LoadTranslations(root string, getLanguageFn func() string) error { gettext.BindLocale(gettext.New("k8s", root+".zip", buf.Bytes())) gettext.SetDomain("k8s") gettext.SetLanguage(langStr) + translationsLoaded = true return nil } +func lazyLoadTranslations() { + lazyLoadTranslationsOnce.Do(func() { + if translationsLoaded { + return + } + if err := LoadTranslationsFunc(); err != nil { + klog.Warning("Failed to load translations") + } + }) +} + // T translates a string, possibly substituting arguments into it along // the way. If len(args) is > 0, args1 is assumed to be the plural value // and plural translation is used. func T(defaultValue string, args ...int) string { + lazyLoadTranslations() if len(args) == 0 { return gettext.PGettext("", defaultValue) } diff --git a/staging/src/k8s.io/kubectl/pkg/util/i18n/i18n_test.go b/staging/src/k8s.io/kubectl/pkg/util/i18n/i18n_test.go index eff6188d5b7..3098e6db572 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/i18n/i18n_test.go +++ b/staging/src/k8s.io/kubectl/pkg/util/i18n/i18n_test.go @@ -18,7 +18,10 @@ package i18n import ( "os" + "sync" "testing" + + "github.com/chai2010/gettext-go" ) var knownTestLocale = "en_US.UTF-8" @@ -155,3 +158,131 @@ func TestTranslationUsingEnvVar(t *testing.T) { }) } } + +// resetLazyLoading allows multiple tests to test translation lazy loading by resetting the state +func resetLazyLoading() { + translationsLoaded = false + lazyLoadTranslationsOnce = sync.Once{} +} + +func TestLazyLoadTranslationFuncIsCalled(t *testing.T) { + resetLazyLoading() + + timesCalled := 0 + err := SetLoadTranslationsFunc(func() error { + timesCalled++ + return LoadTranslations("test", func() string { return "en_US" }) + }) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if translationsLoaded { + t.Errorf("expected translationsLoaded to be false, but it was true") + } + + // Translation should succeed and use the lazy loaded translations + result := T("test_string") + if result != "baz" { + t.Errorf("expected: %s, saw: %s", "baz", result) + } + if timesCalled != 1 { + t.Errorf("expected LoadTranslationsFunc to have been called 1 time, but it was called %d times", timesCalled) + } + if !translationsLoaded { + t.Errorf("expected translationsLoaded to be true, but it was false") + } + + // Call T() again, and timesCalled should remain 1 + T("test_string") + if timesCalled != 1 { + t.Errorf("expected LoadTranslationsFunc to have been called 1 time, but it was called %d times", timesCalled) + } +} + +func TestLazyLoadTranslationFuncOnlyCalledIfTranslationsNotLoaded(t *testing.T) { + resetLazyLoading() + + // Set a custom translations func + timesCalled := 0 + err := SetLoadTranslationsFunc(func() error { + timesCalled++ + return LoadTranslations("test", func() string { return "en_US" }) + }) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if translationsLoaded { + t.Errorf("expected translationsLoaded to be false, but it was true") + } + + // Explicitly load translations before lazy loading can occur + err = LoadTranslations("test", func() string { return "default" }) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if !translationsLoaded { + t.Errorf("expected translationsLoaded to be true, but it was false") + } + + // Translation should succeed, and use the explicitly loaded translations, not the lazy loaded ones + result := T("test_string") + if result != "foo" { + t.Errorf("expected: %s, saw: %s", "foo", result) + } + if timesCalled != 0 { + t.Errorf("expected LoadTranslationsFunc to have not been called, but it was called %d times", timesCalled) + } +} + +func TestSetCustomLoadTranslationsFunc(t *testing.T) { + resetLazyLoading() + + // Set a custom translations func that loads translations from a directory + err := SetLoadTranslationsFunc(func() error { + gettext.BindLocale(gettext.New("k8s", "./translations/test")) + gettext.SetDomain("k8s") + gettext.SetLanguage("en_US") + return nil + }) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if translationsLoaded { + t.Errorf("expected translationsLoaded to be false, but it was true") + } + + // Translation should succeed + result := T("test_string") + if result != "baz" { + t.Errorf("expected: %s, saw: %s", "baz", result) + } + if !translationsLoaded { + t.Errorf("expected translationsLoaded to be true, but it was false") + } +} + +func TestSetCustomLoadTranslationsFuncAfterTranslationsLoadedShouldFail(t *testing.T) { + resetLazyLoading() + + // Explicitly load translations + err := LoadTranslations("test", func() string { return "en_US" }) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if !translationsLoaded { + t.Errorf("expected translationsLoaded to be true, but it was false") + } + + // This should fail because translations have already been loaded, and the custom function should not be called. + timesCalled := 0 + err = SetLoadTranslationsFunc(func() error { + timesCalled++ + return nil + }) + if err == nil { + t.Errorf("expected error, but it did not occur") + } + if timesCalled != 0 { + t.Errorf("expected LoadTranslationsFunc to have not been called, but it was called %d times", timesCalled) + } +} diff --git a/test/cmd/help.sh b/test/cmd/help.sh new file mode 100644 index 00000000000..72cca6c949f --- /dev/null +++ b/test/cmd/help.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env bash + +# Copyright 2022 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -o errexit +set -o nounset +set -o pipefail + +run_kubectl_help_tests() { + set -o nounset + set -o errexit + + # The purpose of this test is to exercise the translation functionality in a simple way. + # If the strings used by this test are changed, they will need to be updated here so that this test will pass. + + kube::test::if_has_string "$(kubectl help)" "Modify kubeconfig files" + kube::test::if_has_string "$(LANG=de_DE.UTF8 kubectl help)" "Verändere kubeconfig Dateien" + kube::test::if_has_string "$(LANG=en_US.UTF8 kubectl help)" "Modify kubeconfig files" + kube::test::if_has_string "$(LANG=fr_FR.UTF8 kubectl help)" "Modifier des fichiers kubeconfig" + kube::test::if_has_string "$(LANG=it_IT.UTF8 kubectl help)" "Modifica i file kubeconfig" + kube::test::if_has_string "$(LANG=ja_JP.UTF8 kubectl help)" "kubeconfigを変更する" + kube::test::if_has_string "$(LANG=ko_KR.UTF8 kubectl help)" "kubeconfig 파일을 수정합니다" + kube::test::if_has_string "$(LANG=pt_BR.UTF8 kubectl help)" "Edita o arquivo kubeconfig" + kube::test::if_has_string "$(LANG=zh_CN.UTF8 kubectl help)" "修改 kubeconfig 文件" + kube::test::if_has_string "$(LANG=zh_TW.UTF8 kubectl help)" "修改 kubeconfig 檔案" + + kube::test::if_has_string "$(kubectl uncordon --help)" "Mark node as schedulable." + kube::test::if_has_string "$(LANG=de_DE.UTF-8 kubectl uncordon --help)" "Markiere Knoten als schedulable." + kube::test::if_has_string "$(LANG=en_US.UTF-8 kubectl uncordon --help)" "Mark node as schedulable." + kube::test::if_has_string "$(LANG=fr_FR.UTF-8 kubectl uncordon --help)" "Mark node as schedulable." + kube::test::if_has_string "$(LANG=it_IT.UTF-8 kubectl uncordon --help)" "Contrassegna il nodo come programmabile." + kube::test::if_has_string "$(LANG=ja_JP.UTF-8 kubectl uncordon --help)" "Mark node as schedulable." + kube::test::if_has_string "$(LANG=ko_KR.UTF-8 kubectl uncordon --help)" "Mark node as schedulable." + kube::test::if_has_string "$(LANG=pt_BR.UTF-8 kubectl uncordon --help)" "Remove a restrição de execução de workloads no node." + kube::test::if_has_string "$(LANG=zh_CN.UTF-8 kubectl uncordon --help)" "标记节点为可调度。" + kube::test::if_has_string "$(LANG=zh_TW.UTF-8 kubectl uncordon --help)" "Mark node as schedulable." + + set +o nounset + set +o errexit +} diff --git a/test/cmd/legacy-script.sh b/test/cmd/legacy-script.sh index 6fff7b11ae9..19935394481 100755 --- a/test/cmd/legacy-script.sh +++ b/test/cmd/legacy-script.sh @@ -45,6 +45,7 @@ source "${KUBE_ROOT}/test/cmd/events.sh" source "${KUBE_ROOT}/test/cmd/exec.sh" source "${KUBE_ROOT}/test/cmd/generic-resources.sh" source "${KUBE_ROOT}/test/cmd/get.sh" +source "${KUBE_ROOT}/test/cmd/help.sh" source "${KUBE_ROOT}/test/cmd/kubeconfig.sh" source "${KUBE_ROOT}/test/cmd/node-management.sh" source "${KUBE_ROOT}/test/cmd/plugins.sh" @@ -555,6 +556,12 @@ runTests() { record_command run_kubectl_get_tests fi + ################ + # Kubectl help # + ################ + + record_command run_kubectl_help_tests + ################## # Kubectl events # ##################