Merge pull request #5233 from stonezdj/release_1.5.2

Fix issue that harbor tile can not save customized settings

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

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.*)

make/prepare

@@ -266,6 +266,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
@@ -320,7 +324,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,
@@ -376,7 +382,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"), 

src/adminserver/systemcfg/systemcfg.go

@@ -17,6 +17,7 @@ package systemcfg
 import (
 	"fmt"
 	"os"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -159,6 +160,7 @@ var (
 			env:   "READ_ONLY",
 			parse: parseStringToBool,
 		},
+		common.ReloadKey: "RELOAD_KEY",
 	}
 
 	// configurations need read from environment variables
@@ -240,17 +242,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
@@ -265,6 +265,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")
@@ -358,13 +362,30 @@ func LoadFromEnv(cfgs map[string]interface{}, all bool) error {
 		}
 	}
 
+	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 all && skipMatcher.MatchString(str) {
+				continue
+			}
 			cfgs[k] = os.Getenv(str)
 			continue
 		}
 
 		if parser, ok := v.(*parser); ok {
+			if all && skipMatcher.MatchString(parser.env) {
+				continue
+			}
 			i, err := parser.parse(os.Getenv(parser.env))
 			if err != nil {
 				return err

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:  "mysql",

src/common/const.go

@@ -107,4 +107,5 @@ const (
 	DefaultUIEndpoint           = "http://ui:8080"
 	DefaultNotaryEndpoint       = "http://notary-server:4443"
 	LdapGroupType               = 1
+	ReloadKey                   = "reload_key"
 )