From 72e9b22e10932d8b74b043bf0bc9e9f3a3383b5c Mon Sep 17 00:00:00 2001 From: stonezdj Date: Wed, 27 Jun 2018 19:26:15 +0800 Subject: [PATCH] Fix issue that harbor tile can not save customized settings --- make/common/templates/adminserver/env | 2 + make/harbor.cfg | 4 ++ make/prepare | 12 +++- src/adminserver/systemcfg/systemcfg.go | 38 ++++++++--- src/adminserver/systemcfg/systemcfg_test.go | 76 +++++++++++++++++++++ src/common/const.go | 1 + tests/common | 1 - 7 files changed, 123 insertions(+), 11 deletions(-) delete mode 120000 tests/common diff --git a/make/common/templates/adminserver/env b/make/common/templates/adminserver/env index f591c04ed..d991be6cd 100644 --- a/make/common/templates/adminserver/env +++ b/make/common/templates/adminserver/env @@ -59,3 +59,5 @@ CLAIR_URL=$clair_url NOTARY_URL=$notary_url REGISTRY_STORAGE_PROVIDER_NAME=$storage_provider_name READ_ONLY=false +SKIP_RELOAD_ENV_PATTERN=$skip_reload_env_pattern +RELOAD_KEY=$reload_key diff --git a/make/harbor.cfg b/make/harbor.cfg index ae2223326..f702ea1a2 100644 --- a/make/harbor.cfg +++ b/make/harbor.cfg @@ -179,3 +179,7 @@ registry_storage_provider_name = filesystem #Refer to https://docs.docker.com/registry/configuration/#storage for all available configuration. registry_storage_provider_config = +#If reload_config=true, all settings which present in harbor.cfg take effect after prepare and restart harbor, it overwrites exsiting settings. +#reload_config=true +#Regular expression to match skipped environment variables +#skip_reload_env_pattern=(^EMAIL.*)|(^LDAP.*) diff --git a/make/prepare b/make/prepare index 4896f8798..bc8989d2b 100755 --- a/make/prepare +++ b/make/prepare @@ -275,6 +275,10 @@ if rcp.has_option("configuration", "redis_url"): else: redis_url = "" +if rcp.has_option("configuration", "skip_reload_env_pattern"): + skip_reload_env_pattern = rcp.get("configuration", "skip_reload_env_pattern") +else: + skip_reload_env_pattern = "$^" storage_provider_name = rcp.get("configuration", "registry_storage_provider_name").strip() storage_provider_config = rcp.get("configuration", "registry_storage_provider_config").strip() # yaml requires 1 or more spaces between the key and value @@ -337,7 +341,9 @@ if protocol == "https": else: render(os.path.join(templates_dir, "nginx", "nginx.http.conf"), nginx_conf) - +#Use reload_key to avoid reload config after restart harbor +reload_key = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(6)) if reload_config == "true" else "" + render(os.path.join(templates_dir, "adminserver", "env"), adminserver_conf_env, reload_config=reload_config, @@ -393,7 +399,9 @@ render(os.path.join(templates_dir, "adminserver", "env"), token_service_url=token_service_url, jobservice_url=jobservice_url, clair_url=clair_url, - notary_url=notary_url + notary_url=notary_url, + reload_key=reload_key, + skip_reload_env_pattern=skip_reload_env_pattern ) render(os.path.join(templates_dir, "ui", "env"), diff --git a/src/adminserver/systemcfg/systemcfg.go b/src/adminserver/systemcfg/systemcfg.go index 00dd43b2b..85ff58e9d 100644 --- a/src/adminserver/systemcfg/systemcfg.go +++ b/src/adminserver/systemcfg/systemcfg.go @@ -17,6 +17,7 @@ package systemcfg import ( "fmt" "os" + "regexp" "strconv" "strings" @@ -162,6 +163,7 @@ var ( env: "READ_ONLY", parse: parseStringToBool, }, + common.ReloadKey: "RELOAD_KEY", } // configurations need read from environment variables @@ -245,17 +247,15 @@ func Init() (err error) { if err = initCfgStore(); err != nil { return err } - - loadAll := false cfgs := map[string]interface{}{} - - if os.Getenv("RESET") == "true" { - log.Info("RESET is set, will load all configurations from environment variables") - loadAll = true + //Use reload key to avoid reset customed setting after restart + curCfgs, err := CfgStore.Read() + if err != nil { + return err } - + loadAll := isLoadAll(curCfgs[common.ReloadKey]) if !loadAll { - cfgs, err = CfgStore.Read() + cfgs = curCfgs if cfgs == nil { log.Info("configurations read from storage driver are null, will load them from environment variables") loadAll = true @@ -270,6 +270,10 @@ func Init() (err error) { return CfgStore.Write(cfgs) } +func isLoadAll(curReloadKey interface{}) bool { + return strings.EqualFold(os.Getenv("RESET"), "true") && os.Getenv("RELOAD_KEY") != curReloadKey +} + func initCfgStore() (err error) { drivertype := os.Getenv("CFG_DRIVER") @@ -363,13 +367,31 @@ func LoadFromEnv(cfgs map[string]interface{}, all bool) error { } } + reloadCfg := os.Getenv("RESET") + skipPattern := os.Getenv("SKIP_RELOAD_ENV_PATTERN") + skipPattern = strings.TrimSpace(skipPattern) + if len(skipPattern) == 0 { + skipPattern = "$^" // doesn't match any string by default + } + skipMatcher, err := regexp.Compile(skipPattern) + if err != nil { + log.Errorf("Regular express parse error, skipPattern:%v", skipPattern) + skipMatcher = regexp.MustCompile("$^") + } + for k, v := range envs { if str, ok := v.(string); ok { + if skipMatcher.MatchString(str) && strings.EqualFold(reloadCfg, "true") { + continue + } cfgs[k] = os.Getenv(str) continue } if parser, ok := v.(*parser); ok { + if skipMatcher.MatchString(parser.env) && strings.EqualFold(reloadCfg, "true") { + continue + } i, err := parser.parse(os.Getenv(parser.env)) if err != nil { return err diff --git a/src/adminserver/systemcfg/systemcfg_test.go b/src/adminserver/systemcfg/systemcfg_test.go index 76173f03a..581725c78 100644 --- a/src/adminserver/systemcfg/systemcfg_test.go +++ b/src/adminserver/systemcfg/systemcfg_test.go @@ -124,8 +124,84 @@ func TestLoadFromEnv(t *testing.T) { assert.Equal(t, extEndpoint, cfgs[common.ExtEndpoint]) assert.Equal(t, "ldap_url", cfgs[common.LDAPURL]) assert.Equal(t, true, cfgs[common.LDAPVerifyCert]) + } +func TestIsLoadAll(t *testing.T) { + os.Clearenv() + if err := os.Setenv("RELOAD_KEY", "123456"); err != nil { + t.Fatalf("failed to set env: %v", err) + } + if err := os.Setenv("RESET", "True"); err != nil { + t.Fatalf("failed to set env: %v", err) + } + assert.False(t, isLoadAll("123456")) + assert.True(t, isLoadAll("654321")) +} + +func TestLoadFromEnvWithReloadConfigInvalidSkipPattern(t *testing.T) { + os.Clearenv() + ldapURL := "ldap://ldap.com" + extEndpoint := "http://harbor.com" + cfgsReload := map[string]interface{}{ + common.LDAPURL: "ldap_url", + } + if err := os.Setenv("LDAP_URL", ldapURL); err != nil { + t.Fatalf("failed to set env: %v", err) + } + if err := os.Setenv("EXT_ENDPOINT", extEndpoint); err != nil { + t.Fatalf("failed to set env: %v", err) + } + + if err := os.Setenv("LDAP_VERIFY_CERT", "false"); err != nil { + t.Fatalf("failed to set env: %v", err) + } + + if err := os.Setenv("SKIP_RELOAD_ENV_PATTERN", "a(b"); err != nil { + t.Fatalf("failed to set env: %v", err) + } + err := LoadFromEnv(cfgsReload, true) + if err != nil { + t.Fatalf("failed to load From env: %v", err) + } + assert.Equal(t, ldapURL, cfgsReload[common.LDAPURL]) + + os.Clearenv() + +} + +func TestLoadFromEnvWithReloadConfigSkipPattern(t *testing.T) { + os.Clearenv() + ldapURL := "ldap://ldap.com" + extEndpoint := "http://harbor.com" + cfgsReload := map[string]interface{}{ + common.LDAPURL: "ldap_url", + } + if err := os.Setenv("LDAP_URL", ldapURL); err != nil { + t.Fatalf("failed to set env: %v", err) + } + if err := os.Setenv("EXT_ENDPOINT", extEndpoint); err != nil { + t.Fatalf("failed to set env: %v", err) + } + + if err := os.Setenv("LDAP_VERIFY_CERT", "false"); err != nil { + t.Fatalf("failed to set env: %v", err) + } + if err := os.Setenv("SKIP_RELOAD_ENV_PATTERN", "^LDAP.*"); err != nil { + t.Fatalf("failed to set env: %v", err) + } + if err := os.Setenv("RESET", "true"); err != nil { + t.Fatalf("failed to set env: %v", err) + } + err := LoadFromEnv(cfgsReload, false) + if err != nil { + t.Fatalf("failed to load From env: %v", err) + } + assert.Equal(t, "ldap_url", cfgsReload[common.LDAPURL]) //env value ignored + + os.Clearenv() + +} func TestGetDatabaseFromCfg(t *testing.T) { cfg := map[string]interface{}{ common.DatabaseType: "postgresql", diff --git a/src/common/const.go b/src/common/const.go index 6b085dc40..b53ec35ea 100644 --- a/src/common/const.go +++ b/src/common/const.go @@ -107,4 +107,5 @@ const ( DefaultUIEndpoint = "http://ui:8080" DefaultNotaryEndpoint = "http://notary-server:4443" LdapGroupType = 1 + ReloadKey = "reload_key" ) diff --git a/tests/common b/tests/common deleted file mode 120000 index 94570ef83..000000000 --- a/tests/common +++ /dev/null @@ -1 +0,0 @@ -../make/common \ No newline at end of file