fix: task params nil params panic (#25699)

Co-authored-by: Qiu Jian <qiujian@yunionyun.com>

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

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=

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 {

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)
+	}
+}

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)

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

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])
+				}
+				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])
 					}
-				for i := range fields[idx].Info.Aliases {
-					fields[idx].Info.Aliases[i] = fmt.Sprintf("%s%s", amPrefix, fields[idx].Info.Aliases[i])
 				}
 				prefixed[idx] = true
 				changed = true

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)
 			}
 		}