Merge pull request #14777 from stonezdj/21apr15_declarative_config

Implement declarative configure feature

src/controller/config/controller.go

@@ -18,6 +18,8 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"os"
+
 	"github.com/goharbor/harbor/src/common"
 	"github.com/goharbor/harbor/src/lib/config"
 	"github.com/goharbor/harbor/src/lib/config/metadata"
@@ -28,9 +30,14 @@ import (
 	"github.com/goharbor/harbor/src/pkg/user"
 )
 
+const (
+	configOverwriteJSON = "CONFIG_OVERWRITE_JSON"
+)
+
 var (
 	// Ctl Global instance of the config controller
 	Ctl            = NewController()
+	readOnlyForAll = false
 )
 
 // Controller define operations related to configures
@@ -39,10 +46,12 @@ type Controller interface {
 	UserConfigs(ctx context.Context) (map[string]*models.Value, error)
 	// UpdateUserConfigs update the user scope configurations
 	UpdateUserConfigs(ctx context.Context, conf map[string]interface{}) error
-	// GetAll get all configurations, used by internal, should include the system config items
+	// AllConfigs get all configurations, used by internal, should include the system config items
 	AllConfigs(ctx context.Context) (map[string]interface{}, error)
 	// ConvertForGet - delete sensitive attrs and add editable field to every attr
 	ConvertForGet(ctx context.Context, cfg map[string]interface{}, internal bool) (map[string]*models.Value, error)
+	// OverwriteConfig overwrite config in the database and set all configure read only when CONFIG_OVERWRITE_JSON is provided
+	OverwriteConfig(ctx context.Context) error
 }
 
 type controller struct {
@@ -67,17 +76,16 @@ func (c *controller) AllConfigs(ctx context.Context) (map[string]interface{}, er
 }
 
 func (c *controller) UpdateUserConfigs(ctx context.Context, conf map[string]interface{}) error {
+	if readOnlyForAll {
+		return errors.ForbiddenError(nil).WithMessage("current config is init by env variable: CONFIG_OVERWRITE_JSON, it cannot be updated")
+	}
 	mgr := config.GetCfgManager(ctx)
 	err := mgr.Load(ctx)
 	if err != nil {
 		return err
 	}
-	isSysErr, err := c.validateCfg(ctx, conf)
+	err = c.validateCfg(ctx, conf)
 	if err != nil {
-		if isSysErr {
-			log.Errorf("failed to validate configurations: %v", err)
-			return fmt.Errorf("failed to validate configuration")
-		}
 		return err
 	}
 	if err := mgr.UpdateConfig(ctx, conf); err != nil {
@@ -87,39 +95,28 @@ func (c *controller) UpdateUserConfigs(ctx context.Context, conf map[string]inte
 	return nil
 }
 
-func (c *controller) validateCfg(ctx context.Context, cfgs map[string]interface{}) (bool, error) {
+func (c *controller) validateCfg(ctx context.Context, cfgs map[string]interface{}) error {
 	mgr := config.GetCfgManager(ctx)
-	flag, err := c.authModeCanBeModified(ctx)
+
+	// check if auth can be modified
+	if nv, ok := cfgs[common.AUTHMode]; ok {
+		if nv.(string) != mgr.Get(ctx, common.AUTHMode).GetString() {
+			canBeModified, err := c.authModeCanBeModified(ctx)
 			if err != nil {
-		return true, err
+				return err
 			}
-	if !flag {
+			if !canBeModified {
-		if failedKeys := c.checkUnmodifiable(ctx, cfgs, common.AUTHMode); len(failedKeys) > 0 {
+				return errors.BadRequestError(nil).
-			return false, errors.BadRequestError(nil).
+					WithMessage(fmt.Sprintf("the auth mode cannot be modified as new users have been inserted into database"))
-				WithMessage(fmt.Sprintf("the keys %v can not be modified as new users have been inserted into database", failedKeys))
 			}
 		}
-	err = mgr.ValidateCfg(ctx, cfgs)
-	if err != nil {
-		return false, errors.BadRequestError(err)
-	}
-	return false, nil
 	}
 
-func (c *controller) checkUnmodifiable(ctx context.Context, cfgs map[string]interface{}, keys ...string) (failed []string) {
+	err := mgr.ValidateCfg(ctx, cfgs)
-	mgr := config.GetCfgManager(ctx)
+	if err != nil {
-	if mgr == nil || cfgs == nil || keys == nil {
+		return errors.BadRequestError(err)
-		return
 	}
-	for _, k := range keys {
+	return nil
-		v := mgr.Get(ctx, k).GetString()
-		if nv, ok := cfgs[k]; ok {
-			if v != fmt.Sprintf("%v", nv) {
-				failed = append(failed, k)
-			}
-		}
-	}
-	return
 }
 
 // ScanAllPolicy is represent the json request and object for scan all policy
@@ -129,7 +126,6 @@ type ScanAllPolicy struct {
 	Param map[string]interface{} `json:"parameter,omitempty"`
 }
 
-// ConvertForGet - delete sensitive attrs and add editable field to every attr
 func (c *controller) ConvertForGet(ctx context.Context, cfg map[string]interface{}, internal bool) (map[string]*models.Value, error) {
 	result := map[string]*models.Value{}
 
@@ -159,7 +155,7 @@ func (c *controller) ConvertForGet(ctx context.Context, cfg map[string]interface
 		}
 		result[item.Name] = &models.Value{
 			Val:      val,
-			Editable: true,
+			Editable: !readOnlyForAll,
 		}
 	}
 
@@ -169,19 +165,35 @@ func (c *controller) ConvertForGet(ctx context.Context, cfg map[string]interface
 	}
 
 	// set value for auth_mode
-	flag, err := c.authModeCanBeModified(ctx)
+	canBeModified, err := c.authModeCanBeModified(ctx)
 	if err != nil {
 		return nil, err
 	}
-	result[common.AUTHMode].Editable = flag
+	result[common.AUTHMode].Editable = canBeModified && !readOnlyForAll
 
 	return result, nil
 }
 
+func (c *controller) OverwriteConfig(ctx context.Context) error {
+	cfgMap := map[string]interface{}{}
+	if v, ok := os.LookupEnv(configOverwriteJSON); ok {
+		err := json.Unmarshal([]byte(v), &cfgMap)
+		if err != nil {
+			return err
+		}
+		err = c.UpdateUserConfigs(ctx, cfgMap)
+		if err != nil {
+			return err
+		}
+		readOnlyForAll = true
+	}
+	return nil
+}
+
 func (c *controller) authModeCanBeModified(ctx context.Context) (bool, error) {
-	users, err := c.userManager.List(ctx, &q.Query{})
+	cnt, err := c.userManager.Count(ctx, &q.Query{})
 	if err != nil {
 		return false, err
 	}
-	return len(users) == 0, nil
+	return cnt == 1, nil // admin user only
 }

src/core/main.go

@@ -26,6 +26,8 @@ import (
 	"syscall"
 	"time"
 
+	configCtl "github.com/goharbor/harbor/src/controller/config"
+
 	"github.com/astaxie/beego"
 	_ "github.com/astaxie/beego/session/redis"
 	_ "github.com/astaxie/beego/session/redis_sentinel"
@@ -190,10 +192,13 @@ func main() {
 	if err = migration.Migrate(database); err != nil {
 		log.Fatalf("failed to migrate: %v", err)
 	}
-	if err := config.Load(orm.Context()); err != nil {
+	ctx := orm.Context()
+	if err := config.Load(ctx); err != nil {
 		log.Fatalf("failed to load config: %v", err)
 	}
-
+	if err := configCtl.Ctl.OverwriteConfig(ctx); err != nil {
+		log.Fatalf("failed to init config from CONFIG_OVERWRITE_JSON, error %v", err)
+	}
 	password, err := config.InitialAdminPassword()
 	if err != nil {
 		log.Fatalf("failed to get admin's initial password: %v", err)

src/lib/config/config.go

@@ -17,6 +17,8 @@ package config
 import (
 	"context"
 	"errors"
+	"sync"
+
 	"github.com/goharbor/harbor/src/common"
 	comModels "github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/lib/config/metadata"
@@ -24,7 +26,6 @@ import (
 	"github.com/goharbor/harbor/src/lib/encrypt"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/lib/orm"
-	"sync"
 )
 
 const (

src/pkg/config/db/dao/dao_test.go

@@ -1,132 +0,0 @@
-//  Copyright Project Harbor 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.
-
-package dao
-
-import (
-	"context"
-	"github.com/goharbor/harbor/src/common"
-	"github.com/goharbor/harbor/src/common/utils/test"
-	"github.com/goharbor/harbor/src/lib/config/models"
-	"github.com/goharbor/harbor/src/lib/orm"
-	"os"
-	"testing"
-)
-
-var testCtx context.Context
-
-func TestMain(m *testing.M) {
-	test.InitDatabaseFromEnv()
-	testCtx = orm.Context()
-	os.Exit(m.Run())
-}
-
-func TestSaveConfigEntries(t *testing.T) {
-	dao := New()
-	configEntries := []models.ConfigEntry{
-		{
-			Key:   "teststringkey",
-			Value: "192.168.111.211",
-		},
-		{
-			Key:   "testboolkey",
-			Value: "true",
-		},
-		{
-			Key:   "testnumberkey",
-			Value: "5",
-		},
-		{
-			Key:   common.CfgDriverDB,
-			Value: "db",
-		},
-	}
-	err := dao.SaveConfigEntries(testCtx, configEntries)
-	if err != nil {
-		t.Fatalf("failed to save configuration to database %v", err)
-	}
-	readEntries, err := dao.GetConfigEntries(testCtx)
-	if err != nil {
-		t.Fatalf("Failed to get configuration from database %v", err)
-	}
-	findItem := 0
-	for _, entry := range readEntries {
-		switch entry.Key {
-		case "teststringkey":
-			if "192.168.111.211" == entry.Value {
-				findItem++
-			}
-		case "testnumberkey":
-			if "5" == entry.Value {
-				findItem++
-			}
-		case "testboolkey":
-			if "true" == entry.Value {
-				findItem++
-			}
-		default:
-		}
-	}
-	if findItem != 3 {
-		t.Fatalf("Should update 3 configuration but only update %d", findItem)
-	}
-
-	configEntries = []models.ConfigEntry{
-		{
-			Key:   "teststringkey",
-			Value: "192.168.111.215",
-		},
-		{
-			Key:   "testboolkey",
-			Value: "false",
-		},
-		{
-			Key:   "testnumberkey",
-			Value: "7",
-		},
-		{
-			Key:   common.CfgDriverDB,
-			Value: "db",
-		},
-	}
-	err = dao.SaveConfigEntries(testCtx, configEntries)
-	if err != nil {
-		t.Fatalf("failed to save configuration to database %v", err)
-	}
-	readEntries, err = dao.GetConfigEntries(testCtx)
-	if err != nil {
-		t.Fatalf("Failed to get configuration from database %v", err)
-	}
-	findItem = 0
-	for _, entry := range readEntries {
-		switch entry.Key {
-		case "teststringkey":
-			if "192.168.111.215" == entry.Value {
-				findItem++
-			}
-		case "testnumberkey":
-			if "7" == entry.Value {
-				findItem++
-			}
-		case "testboolkey":
-			if "false" == entry.Value {
-				findItem++
-			}
-		default:
-		}
-	}
-	if findItem != 3 {
-		t.Fatalf("Should update 3 configuration but only update %d", findItem)
-	}
-}

src/pkg/config/manager.go

@@ -21,6 +21,7 @@ import (
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
 	"github.com/goharbor/harbor/src/lib/config/metadata"
+	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/pkg/config/store"
 	"github.com/goharbor/harbor/src/pkg/config/validate"
@@ -177,10 +178,17 @@ func (c *CfgManager) UpdateConfig(ctx context.Context, cfgs map[string]interface
 // ValidateCfg validate config by metadata. return the first error if exist.
 func (c *CfgManager) ValidateCfg(ctx context.Context, cfgs map[string]interface{}) error {
 	for key, value := range cfgs {
+		item, exist := metadata.Instance().GetByName(key)
+		if !exist {
+			return errors.New(fmt.Sprintf("invalid config, item not defined in metadatalist, %v", key))
+		}
+		if item.Scope == metadata.SystemScope {
+			return errors.New(fmt.Sprintf("system config items cannot be updated, item: %v", key))
+		}
 		strVal := utils.GetStrValueOfAnyType(value)
 		_, err := metadata.NewCfgValue(key, strVal)
 		if err != nil {
-			return fmt.Errorf("%v, item name: %v", err, key)
+			return errors.Wrap(err, "item name "+key)
 		}
 	}
 

src/testing/controller/config/controller.go

@@ -60,6 +60,20 @@ func (_m *Controller) ConvertForGet(ctx context.Context, cfg map[string]interfac
 	return r0, r1
 }
 
+// OverwriteConfig provides a mock function with given fields: ctx
+func (_m *Controller) OverwriteConfig(ctx context.Context) error {
+	ret := _m.Called(ctx)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context) error); ok {
+		r0 = rf(ctx)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // UpdateUserConfigs provides a mock function with given fields: ctx, conf
 func (_m *Controller) UpdateUserConfigs(ctx context.Context, conf map[string]interface{}) error {
 	ret := _m.Called(ctx, conf)