Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry-pick to 1.5: Fix issue that harbor tile can not save customized settings #5233

Merged
merged 2 commits into from
Jul 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions make/common/templates/adminserver/env
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions make/harbor.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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.*)
12 changes: 10 additions & 2 deletions make/prepare
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"),
Expand Down
37 changes: 29 additions & 8 deletions src/adminserver/systemcfg/systemcfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package systemcfg
import (
"fmt"
"os"
"regexp"
"strconv"
"strings"

Expand Down Expand Up @@ -159,6 +160,7 @@ var (
env: "READ_ONLY",
parse: parseStringToBool,
},
common.ReloadKey: "RELOAD_KEY",
}

// configurations need read from environment variables
Expand Down Expand Up @@ -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
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down
76 changes: 76 additions & 0 deletions src/adminserver/systemcfg/systemcfg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/common/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,5 @@ const (
DefaultUIEndpoint = "http://ui:8080"
DefaultNotaryEndpoint = "http://notary-server:4443"
LdapGroupType = 1
ReloadKey = "reload_key"
)
1 change: 0 additions & 1 deletion tests/common

This file was deleted.