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

refactor(utils): add hasher pkg and add more uts for toml #5992

Merged
merged 3 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 2 additions & 1 deletion pkg/controllers/pd/tasks/cm.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/pingcap/tidb-operator/apis/core/v1alpha1"
"github.com/pingcap/tidb-operator/pkg/client"
pdcfg "github.com/pingcap/tidb-operator/pkg/configs/pd"
"github.com/pingcap/tidb-operator/pkg/utils/hasher"
maputil "github.com/pingcap/tidb-operator/pkg/utils/map"
"github.com/pingcap/tidb-operator/pkg/utils/task/v3"
"github.com/pingcap/tidb-operator/pkg/utils/toml"
Expand All @@ -46,7 +47,7 @@ func TaskConfigMap(ctx *ReconcileContext, _ logr.Logger, c client.Client) task.T
return task.Fail().With("pd config cannot be encoded: %v", err)
}

hash, err := toml.GenerateHash(ctx.PD.Spec.Config)
hash, err := hasher.GenerateHash(ctx.PD.Spec.Config)
if err != nil {
return task.Fail().With("failed to generate hash for `pd.spec.config`: %v", err)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/controllers/tidb/tasks/cm.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/pingcap/tidb-operator/apis/core/v1alpha1"
"github.com/pingcap/tidb-operator/pkg/client"
tidbcfg "github.com/pingcap/tidb-operator/pkg/configs/tidb"
"github.com/pingcap/tidb-operator/pkg/utils/hasher"
maputil "github.com/pingcap/tidb-operator/pkg/utils/map"
"github.com/pingcap/tidb-operator/pkg/utils/task/v2"
"github.com/pingcap/tidb-operator/pkg/utils/toml"
Expand Down Expand Up @@ -61,7 +62,7 @@ func (t *TaskConfigMap) Sync(ctx task.Context[ReconcileContext]) task.Result {
return task.Fail().With("tidb config cannot be encoded: %w", err)
}

rtx.ConfigHash, err = toml.GenerateHash(rtx.TiDB.Spec.Config)
rtx.ConfigHash, err = hasher.GenerateHash(rtx.TiDB.Spec.Config)
if err != nil {
return task.Fail().With("failed to generate hash for `tidb.spec.config`: %w", err)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/controllers/tiflash/tasks/cm.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/pingcap/tidb-operator/apis/core/v1alpha1"
"github.com/pingcap/tidb-operator/pkg/client"
tiflashcfg "github.com/pingcap/tidb-operator/pkg/configs/tiflash"
"github.com/pingcap/tidb-operator/pkg/utils/hasher"
maputil "github.com/pingcap/tidb-operator/pkg/utils/map"
"github.com/pingcap/tidb-operator/pkg/utils/task/v2"
"github.com/pingcap/tidb-operator/pkg/utils/toml"
Expand Down Expand Up @@ -72,7 +73,7 @@ func (t *TaskConfigMap) Sync(ctx task.Context[ReconcileContext]) task.Result {
return task.Fail().With("tiflash proxy config cannot be encoded: %w", err)
}

rtx.ConfigHash, err = toml.GenerateHash(rtx.TiFlash.Spec.Config)
rtx.ConfigHash, err = hasher.GenerateHash(rtx.TiFlash.Spec.Config)
if err != nil {
return task.Fail().With("failed to generate hash for `tiflash.spec.config`: %w", err)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/controllers/tikv/tasks/cm.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/pingcap/tidb-operator/apis/core/v1alpha1"
"github.com/pingcap/tidb-operator/pkg/client"
tikvcfg "github.com/pingcap/tidb-operator/pkg/configs/tikv"
"github.com/pingcap/tidb-operator/pkg/utils/hasher"
maputil "github.com/pingcap/tidb-operator/pkg/utils/map"
"github.com/pingcap/tidb-operator/pkg/utils/task/v2"
"github.com/pingcap/tidb-operator/pkg/utils/toml"
Expand Down Expand Up @@ -60,7 +61,7 @@ func (t *TaskConfigMap) Sync(ctx task.Context[ReconcileContext]) task.Result {
return task.Fail().With("tikv config cannot be encoded: %w", err)
}

rtx.ConfigHash, err = toml.GenerateHash(rtx.TiKV.Spec.Config)
rtx.ConfigHash, err = hasher.GenerateHash(rtx.TiKV.Spec.Config)
if err != nil {
return task.Fail().With("failed to generate hash for `tikv.spec.config`: %w", err)
}
Expand Down
27 changes: 27 additions & 0 deletions pkg/utils/hasher/hasher.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package hasher

import (
"bytes"
"fmt"
"hash/fnv"

"github.com/pelletier/go-toml/v2"
"k8s.io/apimachinery/pkg/util/rand"

"github.com/pingcap/tidb-operator/apis/core/v1alpha1"
hashutil "github.com/pingcap/tidb-operator/third_party/kubernetes/pkg/util/hash"
)

// GenerateHash takes a TOML string as input, unmarshals it into a map,
// and generates a hash of the resulting configuration. The hash is then
// encoded into a safe string format and returned.
// If the order of keys in the TOML string is different, the hash will be the same.
func GenerateHash(tomlStr v1alpha1.ConfigFile) (string, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this function only works for TOML, should we include toml in the name?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but may add it when a new similar function is added?

var config map[string]any
if err := toml.NewDecoder(bytes.NewReader([]byte(tomlStr))).Decode(&config); err != nil {
return "", fmt.Errorf("failed to unmarshal toml string %s: %w", tomlStr, err)
}
hasher := fnv.New32a()
hashutil.DeepHashObject(hasher, config)
return rand.SafeEncodeString(fmt.Sprint(hasher.Sum32())), nil
}
128 changes: 128 additions & 0 deletions pkg/utils/hasher/hasher_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
package hasher

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/pingcap/tidb-operator/apis/core/v1alpha1"
)

func TestGenerateHash(t *testing.T) {
tests := []struct {
name string
tomlStr v1alpha1.ConfigFile
semanticallyEquivalentStr v1alpha1.ConfigFile
wantHash string
wantError bool
}{
{
name: "Valid TOML string",
tomlStr: v1alpha1.ConfigFile(`foo = 'bar'
[log]
k1 = 'v1'
k2 = 'v2'`),
semanticallyEquivalentStr: v1alpha1.ConfigFile(`foo = 'bar'
[log]
k2 = 'v2'
k1 = 'v1'`),
wantHash: "5dbbcf4574",
wantError: false,
},
{
name: "Different config value",
tomlStr: v1alpha1.ConfigFile(`foo = 'foo'
[log]
k2 = 'v2'
k1 = 'v1'`),
wantHash: "f5bc46cb9",
wantError: false,
},
{
name: "multiple sections with blank line",
tomlStr: v1alpha1.ConfigFile(`[a]
k1 = 'v1'
[b]
k2 = 'v2'`),
semanticallyEquivalentStr: v1alpha1.ConfigFile(`[a]
k1 = 'v1'
[b]

k2 = 'v2'`),
wantHash: "79598d5977",
wantError: false,
},
{
name: "Empty TOML string",
tomlStr: v1alpha1.ConfigFile(``),
wantHash: "7d6fc488b7",
wantError: false,
},
{
name: "Invalid TOML string",
tomlStr: v1alpha1.ConfigFile(`key1 = "value1"
key2 = value2`), // Missing quotes around value2
wantHash: "",
wantError: true,
},
{
name: "Nested tables",
tomlStr: v1alpha1.ConfigFile(`[parent]
child1 = "value1"
child2 = "value2"
[parent.child]
grandchild1 = "value3"
grandchild2 = "value4"`),
semanticallyEquivalentStr: v1alpha1.ConfigFile(`[parent]
child2 = "value2"
child1 = "value1"
[parent.child]
grandchild2 = "value4"
grandchild1 = "value3"`),
wantHash: "7bf645ccb4",
wantError: false,
},
{
name: "Array of tables",
tomlStr: v1alpha1.ConfigFile(`[[products]]
name = "Hammer"
sku = 738594937

[[products]]
name = "Nail"
sku = 284758393

color = "gray"`),
semanticallyEquivalentStr: v1alpha1.ConfigFile(`[[products]]
sku = 738594937
name = "Hammer"

[[products]]
sku = 284758393
name = "Nail"

color = "gray"`),
wantHash: "7549cf87f4",
wantError: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotHash, err := GenerateHash(tt.tomlStr)
if tt.wantError {
assert.Error(t, err)
} else {
require.NoError(t, err)
assert.Equal(t, tt.wantHash, gotHash)

if string(tt.semanticallyEquivalentStr) != "" {
reorderedHash, err := GenerateHash(tt.semanticallyEquivalentStr)
require.NoError(t, err)
assert.Equal(t, tt.wantHash, reorderedHash)
}
}
})
}
}
42 changes: 15 additions & 27 deletions pkg/utils/toml/toml.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,11 @@ package toml
import (
"bytes"
"fmt"
"hash/fnv"
"reflect"
"strings"

"github.com/mitchellh/mapstructure"
"github.com/pelletier/go-toml/v2"
"k8s.io/apimachinery/pkg/util/rand"

"github.com/pingcap/tidb-operator/apis/core/v1alpha1"
hashutil "github.com/pingcap/tidb-operator/third_party/kubernetes/pkg/util/hash"
)

type Decoder[T any, PT *T] interface {
Expand Down Expand Up @@ -57,6 +53,7 @@ func (c *codec[T, PT]) Decode(data []byte, obj PT) error {
Result: obj,
})
if err != nil {
// unreachable
return err
}
if err := decoder.Decode(raw); err != nil {
Expand All @@ -69,7 +66,7 @@ func (c *codec[T, PT]) Decode(data []byte, obj PT) error {
}

func (c *codec[T, PT]) Encode(obj PT) ([]byte, error) {
if err := Overwrite(obj, c.raw); err != nil {
if err := overwrite(obj, c.raw); err != nil {
return nil, err
}

Expand All @@ -81,25 +78,30 @@ func (c *codec[T, PT]) Encode(obj PT) ([]byte, error) {
return buf.Bytes(), nil
}

func Overwrite(obj any, m map[string]any) error {
func overwrite(obj any, m map[string]any) error {
structVal := reflect.ValueOf(obj).Elem()
fieldTypes := reflect.VisibleFields(structVal.Type())
for _, fieldType := range fieldTypes {
if !fieldType.IsExported() {
continue
}

name := fieldType.Tag.Get("toml")
src := structVal.FieldByIndex(fieldType.Index)

for src.Kind() == reflect.Pointer {
src = src.Elem()
tag := fieldType.Tag.Get("toml")
parts := strings.Split(tag, ",")
name := fieldType.Name
if len(parts) != 0 {
name = parts[0]
}
src := structVal.FieldByIndex(fieldType.Index)

if src.IsZero() {
continue
}

for src.Kind() == reflect.Pointer {
src = src.Elem()
}

v, ok := m[name]
if !ok {
m[name] = src.Interface()
Expand All @@ -123,7 +125,7 @@ func getField(src reflect.Value, dst any) (any, error) {
if !ok {
return nil, fmt.Errorf("type mismatched, expected map, actual %T", dst)
}
if err := Overwrite(src.Addr().Interface(), vm); err != nil {
if err := overwrite(src.Addr().Interface(), vm); err != nil {
return nil, err
}

Expand Down Expand Up @@ -154,17 +156,3 @@ func getField(src reflect.Value, dst any) (any, error) {
return src.Interface(), nil
}
}

// GenerateHash takes a TOML string as input, unmarshals it into a map,
// and generates a hash of the resulting configuration. The hash is then
// encoded into a safe string format and returned.
// If the order of keys in the TOML string is different, the hash will be the same.
func GenerateHash(tomlStr v1alpha1.ConfigFile) (string, error) {
var config map[string]any
if err := toml.NewDecoder(bytes.NewReader([]byte(tomlStr))).Decode(&config); err != nil {
return "", fmt.Errorf("failed to unmarshal toml string %s: %w", tomlStr, err)
}
hasher := fnv.New32a()
hashutil.DeepHashObject(hasher, config)
return rand.SafeEncodeString(fmt.Sprint(hasher.Sum32())), nil
}
Loading
Loading