From 3825220ca61229dd2738a856b4479b5c7376d7e3 Mon Sep 17 00:00:00 2001 From: stonezdj Date: Fri, 16 Apr 2021 15:13:35 +0800 Subject: [PATCH] Implement declarative configure feature Add env CONFIG_OVERWRITE_JSON for declarative config Init config with the json in CONFIG_OVERWRITE_JSON in main.go Signed-off-by: stonezdj --- src/controller/config/controller.go | 94 ++++++++------ src/core/main.go | 9 +- src/lib/config/config.go | 3 +- src/pkg/config/db/dao/dao_test.go | 132 -------------------- src/pkg/config/manager.go | 10 +- src/testing/controller/config/controller.go | 14 +++ 6 files changed, 85 insertions(+), 177 deletions(-) delete mode 100644 src/pkg/config/db/dao/dao_test.go diff --git a/src/controller/config/controller.go b/src/controller/config/controller.go index 4fa847c53..2635c9ed7 100644 --- a/src/controller/config/controller.go +++ b/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() + 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) - if err != nil { - return true, err - } - if !flag { - if failedKeys := c.checkUnmodifiable(ctx, cfgs, common.AUTHMode); len(failedKeys) > 0 { - return false, errors.BadRequestError(nil). - 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) { - mgr := config.GetCfgManager(ctx) - if mgr == nil || cfgs == nil || keys == nil { - return - } - for _, k := range keys { - v := mgr.Get(ctx, k).GetString() - if nv, ok := cfgs[k]; ok { - if v != fmt.Sprintf("%v", nv) { - failed = append(failed, k) + // 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 err + } + if !canBeModified { + return errors.BadRequestError(nil). + WithMessage(fmt.Sprintf("the auth mode cannot be modified as new users have been inserted into database")) } } } - return + + err := mgr.ValidateCfg(ctx, cfgs) + if err != nil { + return errors.BadRequestError(err) + } + return nil } // 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 } diff --git a/src/core/main.go b/src/core/main.go index 44f4d9c7b..05ad52fd4 100755 --- a/src/core/main.go +++ b/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) diff --git a/src/lib/config/config.go b/src/lib/config/config.go index 0f51594d4..2b4203914 100644 --- a/src/lib/config/config.go +++ b/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 ( diff --git a/src/pkg/config/db/dao/dao_test.go b/src/pkg/config/db/dao/dao_test.go deleted file mode 100644 index 02fcf45ce..000000000 --- a/src/pkg/config/db/dao/dao_test.go +++ /dev/null @@ -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) - } -} diff --git a/src/pkg/config/manager.go b/src/pkg/config/manager.go index 26d837dcf..759ea3be6 100644 --- a/src/pkg/config/manager.go +++ b/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) } } diff --git a/src/testing/controller/config/controller.go b/src/testing/controller/config/controller.go index 54c3408e5..91f72d8c2 100644 --- a/src/testing/controller/config/controller.go +++ b/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)