diff --git a/go.mod b/go.mod index 270374028e..6159be128f 100644 --- a/go.mod +++ b/go.mod @@ -122,7 +122,7 @@ require ( yunion.io/x/jsonutils v1.0.1-0.20260917025845-3108cd9a32ea yunion.io/x/log v1.0.1-0.20240305175729-7cf2d6cd5a91 yunion.io/x/ovsdb v0.0.0-20230306173834-f164f413a900 - yunion.io/x/pkg v1.10.4-0.20260916163305-b7743fa5e758 + yunion.io/x/pkg v1.10.4-0.20260918012554-27cd9d2e093b yunion.io/x/s3cli v0.0.0-20241221171442-1c11599d28e1 yunion.io/x/sqlchemy v1.1.3-0.20260917035232-d682485b3a12 yunion.io/x/structarg v0.0.0-20260917033311-96c7653334ac diff --git a/go.sum b/go.sum index 4bc049bfea..efff286a78 100644 --- a/go.sum +++ b/go.sum @@ -1989,8 +1989,9 @@ yunion.io/x/ovsdb v0.0.0-20230306173834-f164f413a900 h1:Hu/4ERvoWaN6aiFs4h4/yvVB yunion.io/x/ovsdb v0.0.0-20230306173834-f164f413a900/go.mod h1:0vLkNEhlmA64HViPBAnSTUMrx5QP1CLsxXmxDKQ80tc= yunion.io/x/pkg v0.0.0-20190628082551-f4033ba2ea30/go.mod h1:t6rEGG2sQ4J7DhFxSZVOTjNd0YO/KlfWQyK1W4tog+E= yunion.io/x/pkg v0.0.0-20200814072949-4f1b541857d6/go.mod h1:t6rEGG2sQ4J7DhFxSZVOTjNd0YO/KlfWQyK1W4tog+E= -yunion.io/x/pkg v1.10.4-0.20260916163305-b7743fa5e758 h1:3p/GCOPvDfvlSfovOh9GKxsSa7PPqp//DUjGRX8Wyus= yunion.io/x/pkg v1.10.4-0.20260916163305-b7743fa5e758/go.mod h1:Crp6aUYB+Yq16DFjZPHhHq7ha5HL2DfyPvzCo2LJ+LU= +yunion.io/x/pkg v1.10.4-0.20260918012554-27cd9d2e093b h1:XUJLpD48685DE8hwxEC+ms1F4T6/g2KKAfGuVn8rZeo= +yunion.io/x/pkg v1.10.4-0.20260918012554-27cd9d2e093b/go.mod h1:S8ITTpENaL3qSXi8Y4L6wVbVlP8XZ5BvABO10DFqF7M= yunion.io/x/s3cli v0.0.0-20241221171442-1c11599d28e1 h1:1KJ3YYinydPHpDEQRXdr/T8SYcKZ5Er+m489H+PnaQ4= yunion.io/x/s3cli v0.0.0-20241221171442-1c11599d28e1/go.mod h1:0iFKpOs1y4lbCxeOmq3Xx/0AcQoewVPwj62eRluioEo= yunion.io/x/sqlchemy v1.1.3-0.20260917035232-d682485b3a12 h1:xU8Z/Zg+zc199Y58eB3fA8mvOHsA28UCDfd7OuK3QKA= diff --git a/pkg/cloudcommon/db/taskman/tasks.go b/pkg/cloudcommon/db/taskman/tasks.go index 4d898ff910..a9d1dc61b1 100644 --- a/pkg/cloudcommon/db/taskman/tasks.go +++ b/pkg/cloudcommon/db/taskman/tasks.go @@ -951,7 +951,22 @@ func (task *STask) ClearPendingUsage(index int) error { } func (task *STask) GetParams() *jsonutils.JSONDict { - return task.Params + result := jsonutils.NewDict() + if task.Params == nil { + return result + } + copied := task.Params.DeepCopy() + copyParams, ok := copied.(*jsonutils.JSONDict) + if !ok || copyParams == nil { + return result + } + paramsJsonMap, _ := copyParams.GetMap() + for k, v := range paramsJsonMap { + if !strings.HasPrefix(k, "__") { + result.Set(k, v) + } + } + return result } func (task *STask) GetUserCred() mcclient.TokenCredential { diff --git a/pkg/cloudcommon/db/taskman/tasks_test.go b/pkg/cloudcommon/db/taskman/tasks_test.go new file mode 100644 index 0000000000..e8f9a35f34 --- /dev/null +++ b/pkg/cloudcommon/db/taskman/tasks_test.go @@ -0,0 +1,144 @@ +// Copyright 2019 Yunion +// +// 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 taskman + +import ( + "testing" + + "yunion.io/x/jsonutils" +) + +func TestSTaskGetParams(t *testing.T) { + newParams := func(pairs map[string]jsonutils.JSONObject) *jsonutils.JSONDict { + params := jsonutils.NewDict() + for k, v := range pairs { + params.Set(k, v) + } + return params + } + + nested := jsonutils.NewDict() + nested.Set("ip", jsonutils.NewString("10.0.0.1")) + stages := jsonutils.NewArray( + jsonutils.NewDict(), + jsonutils.NewString("on_init"), + ) + + cases := []struct { + name string + params *jsonutils.JSONDict + want *jsonutils.JSONDict + }{ + { + name: "nil_params", + params: nil, + want: jsonutils.NewDict(), + }, + { + name: "empty_params", + params: jsonutils.NewDict(), + want: jsonutils.NewDict(), + }, + { + name: "keep_public_keys", + params: newParams(map[string]jsonutils.JSONObject{ + "parent_task_id": jsonutils.NewString("task-1"), + "auto_start": jsonutils.JSONTrue, + }), + want: newParams(map[string]jsonutils.JSONObject{ + "parent_task_id": jsonutils.NewString("task-1"), + "auto_start": jsonutils.JSONTrue, + }), + }, + { + name: "drop_internal_keys", + params: newParams(map[string]jsonutils.JSONObject{ + "__stages": stages, + "__pending_usage__": jsonutils.NewDict(), + "__request_context": jsonutils.NewDict(), + "__parent_task_notifyurl": jsonutils.NewString("http://notify"), + }), + want: jsonutils.NewDict(), + }, + { + name: "mixed_keys", + params: newParams(map[string]jsonutils.JSONObject{ + "desc": nested, + "__stages": stages, + "_private": jsonutils.NewString("keep"), + "__": jsonutils.NewString("drop"), + "guest_id": jsonutils.NewString("g-1"), + }), + want: newParams(map[string]jsonutils.JSONObject{ + "desc": nested, + "_private": jsonutils.NewString("keep"), + "guest_id": jsonutils.NewString("g-1"), + }), + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + task := &STask{} + task.Params = c.params + got := task.GetParams() + if got == nil { + t.Fatal("GetParams() returned nil, want empty dict") + } + if !got.Equals(c.want) { + t.Fatalf("GetParams() = %s, want %s", got, c.want) + } + }) + } +} + +func TestSTaskGetParamsDoesNotMutateOriginal(t *testing.T) { + orig := jsonutils.NewDict() + orig.Set("guest_id", jsonutils.NewString("g-1")) + orig.Set("__stages", jsonutils.NewArray(jsonutils.NewString("on_init"))) + nested := jsonutils.NewDict() + nested.Set("ip", jsonutils.NewString("10.0.0.1")) + orig.Set("desc", nested) + + task := &STask{STaskBase: STaskBase{Params: orig}} + got := task.GetParams() + got.Set("guest_id", jsonutils.NewString("g-2")) + got.Set("extra", jsonutils.JSONTrue) + desc, err := got.Get("desc") + if err != nil { + t.Fatalf("get desc: %v", err) + } + descDict, ok := desc.(*jsonutils.JSONDict) + if !ok { + t.Fatalf("desc type %T, want *jsonutils.JSONDict", desc) + } + descDict.Set("ip", jsonutils.NewString("10.0.0.2")) + + if got.Contains("__stages") { + t.Fatal("GetParams() leaked internal key __stages") + } + guestId, _ := orig.GetString("guest_id") + if guestId != "g-1" { + t.Fatalf("original guest_id = %s, want g-1", guestId) + } + if !orig.Contains("__stages") { + t.Fatal("original __stages was removed") + } + origIp, _ := orig.GetString("desc", "ip") + if origIp != "10.0.0.1" { + t.Fatalf("original nested ip = %s, want 10.0.0.1", origIp) + } +} diff --git a/pkg/compute/guestdrivers/baremetals.go b/pkg/compute/guestdrivers/baremetals.go index 42e77dc828..ef72712ae5 100644 --- a/pkg/compute/guestdrivers/baremetals.go +++ b/pkg/compute/guestdrivers/baremetals.go @@ -356,6 +356,7 @@ func (self *SBaremetalGuestDriver) RequestStartOnHost(ctx context.Context, guest if params.Length() > 0 { config.Add(params, "params") } + log.Debugf("RequestStartOnHost config: %s", config.String()) headers := task.GetTaskRequestHeader() url := fmt.Sprintf("/baremetals/%s/servers/%s/start", host.Id, guest.Id) _, err := host.BaremetalSyncRequest(ctx, "POST", url, headers, config) diff --git a/vendor/modules.txt b/vendor/modules.txt index 05b5c51fb9..2b3537e735 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2788,7 +2788,7 @@ yunion.io/x/log/hooks yunion.io/x/ovsdb/cli_util yunion.io/x/ovsdb/schema/ovn_nb yunion.io/x/ovsdb/types -# yunion.io/x/pkg v1.10.4-0.20260916163305-b7743fa5e758 +# yunion.io/x/pkg v1.10.4-0.20260918012554-27cd9d2e093b ## explicit; go 1.18 yunion.io/x/pkg/appctx yunion.io/x/pkg/errors diff --git a/vendor/yunion.io/x/pkg/util/reflectutils/ambiguous.go b/vendor/yunion.io/x/pkg/util/reflectutils/ambiguous.go index 61ea63b2dc..ec0afc83bb 100644 --- a/vendor/yunion.io/x/pkg/util/reflectutils/ambiguous.go +++ b/vendor/yunion.io/x/pkg/util/reflectutils/ambiguous.go @@ -57,7 +57,7 @@ func expandAmbiguousPrefix(fields SStructFieldValueSet) SStructFieldValueSet { if prefixed[idx] { continue } - amPrefix, ok := fields[idx].Info.Tags[TAG_AMBIGUOUS_PREFIX] + amPrefix, ok := fields[idx].Info.Tag(TAG_AMBIGUOUS_PREFIX) if !ok { continue } @@ -65,15 +65,25 @@ func expandAmbiguousPrefix(fields SStructFieldValueSet) SStructFieldValueSet { if takenByOther(fields, expanded, indexes) { continue } - fields[idx].Info.Name = expanded - if depBy, ok := fields[idx].Info.Tags[TAG_DEPRECATED_BY]; ok { - fields[idx].Info.Tags[TAG_DEPRECATED_BY] = fmt.Sprintf("%s%s", amPrefix, depBy) + info := fields[idx].Info + info.Name = expanded + _, newDepBy := info.tags[TAG_DEPRECATED_BY] + _, oldDepBy := info.tags[TAG_OLD_DEPRECATED_BY] + if newDepBy || oldDepBy { + // the tags may still be shared with other callers + info.copyTags() } - if depBy, ok := fields[idx].Info.Tags[TAG_OLD_DEPRECATED_BY]; ok { - fields[idx].Info.Tags[TAG_OLD_DEPRECATED_BY] = fmt.Sprintf("%s%s", amPrefix, depBy) + if newDepBy { + info.tags[TAG_DEPRECATED_BY] = fmt.Sprintf("%s%s", amPrefix, info.tags[TAG_DEPRECATED_BY]) } - for i := range fields[idx].Info.Aliases { - fields[idx].Info.Aliases[i] = fmt.Sprintf("%s%s", amPrefix, fields[idx].Info.Aliases[i]) + if oldDepBy { + info.tags[TAG_OLD_DEPRECATED_BY] = fmt.Sprintf("%s%s", amPrefix, info.tags[TAG_OLD_DEPRECATED_BY]) + } + if len(info.aliases) > 0 { + info.copyAliases() + for i := range info.aliases { + info.aliases[i] = fmt.Sprintf("%s%s", amPrefix, info.aliases[i]) + } } prefixed[idx] = true changed = true diff --git a/vendor/yunion.io/x/pkg/util/reflectutils/jsonfield.go b/vendor/yunion.io/x/pkg/util/reflectutils/jsonfield.go index 892d367314..a42aa439e1 100644 --- a/vendor/yunion.io/x/pkg/util/reflectutils/jsonfield.go +++ b/vendor/yunion.io/x/pkg/util/reflectutils/jsonfield.go @@ -30,9 +30,10 @@ import ( // package. Do not construct a literal or modify the exported fields in an // unmanaged way // -// The tags of a field are meant to be read through Tag and TagMap rather than -// off the Tags map, which is not handed out to stay writable by whoever read -// it; a caller may modify what TagMap returns. +// The tags of a field are read through Tag and TagMap. The map behind them +// is shared with the other callers of the fetch functions, which is what +// makes reading a field several times cheap, so a caller may only modify what +// TagMap hands out, which is a copy of its own. type SStructFieldInfo struct { // True if the field has json tag `json:"-"` Ignore bool @@ -72,40 +73,34 @@ type SStructFieldInfo struct { // value as a json string ForceString bool - // Tags holds the tags of the field keyed by tag name, a tag without a - // value being mapped to the empty string. Read it through Tag or - // TagMap rather than off here. - Tags map[string]string + // tags holds the tags of the field keyed by tag name, a tag without a + // value being mapped to the empty string. The map is shared with the + // other callers of the fetch functions, so it has to be copied with + // copyTags before being written to; read it through Tag or TagMap. + tags map[string]string - // Aliases are the other names the field is looked up by, taken from - // the "alias" tag - Aliases []string + // aliases are the other names the field is looked up by, taken from + // the "alias" tag. Like tags it is shared, and has to be copied with + // copyAliases before being written to. + aliases []string } -func (s *SStructFieldInfo) updateTags(k, v string) { - s.Tags[k] = v -} - -func (s SStructFieldInfo) deepCopy() *SStructFieldInfo { - scopy := SStructFieldInfo{ - Ignore: s.Ignore, - OmitEmpty: s.OmitEmpty, - OmitFalse: s.OmitFalse, - OmitZero: s.OmitZero, - Name: s.Name, - FieldName: s.FieldName, - ForceString: s.ForceString, - kebabFieldName: s.kebabFieldName, - } - tags := make(map[string]string, len(s.Tags)) - for k, v := range s.Tags { +// copyTags takes a private copy of the tags, so that they can be written to +// without touching the ones this info was read out of. +func (s *SStructFieldInfo) copyTags() { + tags := make(map[string]string, len(s.tags)+1) + for k, v := range s.tags { tags[k] = v } - scopy.Tags = tags - aliases := make([]string, len(s.Aliases)) - copy(aliases, s.Aliases) - scopy.Aliases = aliases - return &scopy + s.tags = tags +} + +// copyAliases takes a private copy of the aliases, so that they can be +// written to without touching the ones this info was read out of. +func (s *SStructFieldInfo) copyAliases() { + aliases := make([]string, len(s.aliases)) + copy(aliases, s.aliases) + s.aliases = aliases } func ParseStructFieldJsonInfo(sf reflect.StructField) SStructFieldInfo { @@ -120,8 +115,8 @@ func ParseFieldJsonInfo(name string, tag reflect.StructTag) SStructFieldInfo { info.OmitZero = false info.OmitFalse = false - info.Tags = utils.TagMap(tag) - if val, ok := info.Tags["json"]; ok { + info.tags = utils.TagMap(tag) + if val, ok := info.tags["json"]; ok { keys := strings.Split(val, ",") if len(keys) > 0 { if keys[0] == "-" { @@ -155,14 +150,14 @@ func ParseFieldJsonInfo(name string, tag reflect.StructTag) SStructFieldInfo { } } } - if val, ok := info.Tags["name"]; ok { + if val, ok := info.tags["name"]; ok { info.Name = val } if !info.Ignore && len(info.Name) == 0 { info.Name = info.kebabFieldName } - if val, ok := info.Tags["alias"]; !info.Ignore && ok { - info.Aliases = strings.Split(val, ",") + if val, ok := info.tags["alias"]; !info.Ignore && ok { + info.aliases = strings.Split(val, ",") } return info } @@ -179,15 +174,15 @@ func (info *SStructFieldInfo) MarshalName() string { // Tag returns the value of the tag named name and whether the field has it. // A tag without a value is reported as present with an empty value. func (info *SStructFieldInfo) Tag(name string) (string, bool) { - val, ok := info.Tags[name] + val, ok := info.tags[name] return val, ok } // TagMap returns a copy of the tags of the field, which the caller owns and // is free to modify. func (info *SStructFieldInfo) TagMap() map[string]string { - tags := make(map[string]string, len(info.Tags)) - for k, v := range info.Tags { + tags := make(map[string]string, len(info.tags)) + for k, v := range info.tags { tags[k] = v } return tags @@ -375,10 +370,10 @@ func fetchStructFieldValueSet3(dataValue reflect.Value, allocatePtr bool, tags m continue } } - fieldInfo := fieldInfos[sf.Name].deepCopy() + fieldInfo := fieldInfos[sf.Name] if !fieldInfo.Ignore || includeIgnore { structFieldVaule := SStructFieldValue{ - Info: fieldInfo, + Info: &fieldInfo, Value: fv, } if parent != nil { @@ -390,6 +385,7 @@ func fetchStructFieldValueSet3(dataValue reflect.Value, allocatePtr bool, tags m if len(tags) > 0 { for i := range fields { fieldName := fields[i].Info.MarshalName() + owned := false for k, v := range tags { target := "" pos := strings.Index(k, "->") @@ -400,7 +396,12 @@ func fetchStructFieldValueSet3(dataValue reflect.Value, allocatePtr bool, tags m if len(target) > 0 && target != fieldName { continue } - fields[i].Info.updateTags(k, v) + if !owned { + // the tags may still be shared with other callers + fields[i].Info.copyTags() + owned = true + } + fields[i].Info.tags[k] = v } } } @@ -445,7 +446,7 @@ func (fields SStructFieldValueSet) GetStructFieldIndexes2(name string, strictMod ret = append(ret, i) } else if info.FieldName == capName { ret = append(ret, i) - } else if len(info.Aliases) > 0 && utils.IsInArray(name, info.Aliases) { + } else if len(info.aliases) > 0 && utils.IsInArray(name, info.aliases) { ret = append(ret, i) } }