fix: make hashing of remote config consistent in opAmp server (#1782)

odigos-io · Nov 18, 2024 · a6f1ad8 · a6f1ad8
1 parent c699098
commit a6f1ad8
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 4 deletions.
diff --git a/opampserver/go.mod b/opampserver/go.mod
@@ -5,7 +5,7 @@ go 1.22.0
 require (
 	github.com/go-logr/logr v1.4.2
 	github.com/odigos-io/odigos/api v0.0.0
-	github.com/odigos-io/odigos/common v1.0.70
+	github.com/odigos-io/odigos/common v0.0.0
 	github.com/odigos-io/odigos/k8sutils v0.0.0
 	go.opentelemetry.io/otel v1.29.0
 	google.golang.org/grpc v1.65.0
@@ -17,6 +17,8 @@ require (
 	sigs.k8s.io/controller-runtime v0.19.0
 )
 
+require github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
+
 require (
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
@@ -37,6 +39,7 @@ require (
 	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
 	github.com/google/uuid v1.6.0 // indirect
+	github.com/hashicorp/go-version v1.7.0 // indirect
 	github.com/imdario/mergo v0.3.6 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
@@ -52,6 +55,7 @@ require (
 	github.com/prometheus/common v0.55.0 // indirect
 	github.com/prometheus/procfs v0.15.1 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
+	github.com/stretchr/testify v1.9.0
 	github.com/x448/float16 v0.8.4 // indirect
 	go.opentelemetry.io/otel/trace v1.29.0 // indirect
 	go.uber.org/zap v1.27.0 // indirect
@@ -79,5 +83,6 @@ require (
 
 replace (
 	github.com/odigos-io/odigos/api => ../api
+	github.com/odigos-io/odigos/common => ../common
 	github.com/odigos-io/odigos/k8sutils => ../k8sutils
 )
diff --git a/opampserver/go.sum b/opampserver/go.sum
@@ -58,6 +58,8 @@ github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af h1:kmjWCqn2qkEml422C2
 github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af/go.mod h1:K1liHPHnj73Fdn/EKuT8nrFqBihUSKXoLYU0BuatOYo=
 github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
+github.com/hashicorp/go-version v1.7.0 h1:5tqGy27NaOTB8yJKUZELlFAS/LTKJkrmONwQKeRZfjY=
+github.com/hashicorp/go-version v1.7.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
 github.com/imdario/mergo v0.3.6 h1:xTNEAn+kxVO7dTZGu0CegyqKZmoWFI0rF8UxjlB2d28=
 github.com/imdario/mergo v0.3.6/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
 github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
@@ -89,8 +91,6 @@ github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9G
 github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
-github.com/odigos-io/odigos/common v1.0.70 h1:MsVswq8sELiCvkZJSA0w/F6PBXCBbVUXOA/qiLfq+7A=
-github.com/odigos-io/odigos/common v1.0.70/go.mod h1:ab+iH/sTKO9n4Mv614lAt6/fQnzNcoWLrc6uDSuZNxU=
 github.com/onsi/ginkgo/v2 v2.19.0 h1:9Cnnf7UHo57Hy3k6/m5k3dRfGTMXGvxhHFvkDTCTpvA=
 github.com/onsi/ginkgo/v2 v2.19.0/go.mod h1:rlwLi9PilAFJ8jCg9UE1QP6VBpd6/xj3SRC0d6TU0To=
 github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk=

diff --git a/opampserver/pkg/connection/remoteconfig.go b/opampserver/pkg/connection/remoteconfig.go
@@ -2,15 +2,27 @@ package connection
 
 import (
 	"crypto/sha256"
+	"slices"
 
 	"github.com/odigos-io/odigos/opampserver/protobufs"
 )
 
 func CalcRemoteConfigHash(remoteConfig *protobufs.AgentConfigMap) []byte {
+	// sort the entries by key
+	// then hash the body of each entry
+	// this is required to ensure that the hash is consistent
+	sortedKeys := make([]string, 0, len(remoteConfig.ConfigMap))
+	for key := range remoteConfig.ConfigMap {
+		sortedKeys = append(sortedKeys, key)
+	}
+
+	slices.Sort(sortedKeys)
 
 	hash := sha256.New()
-	for _, configEntry := range remoteConfig.ConfigMap {
+	for _, key := range sortedKeys {
+		configEntry := remoteConfig.ConfigMap[key]
 		hash.Write(configEntry.Body)
 	}
+
 	return hash.Sum(nil)
 }
diff --git a/opampserver/pkg/connection/remoteconfig_test.go b/opampserver/pkg/connection/remoteconfig_test.go
@@ -0,0 +1,35 @@
+package connection
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/odigos-io/odigos/opampserver/protobufs"
+)
+
+func TestCalcRemoteConfigHashConsistent(t *testing.T) {
+	remoteConfig := protobufs.AgentConfigMap{
+		ConfigMap: map[string]*protobufs.AgentConfigFile{
+			"key1": {
+				Body: []byte("value1"),
+			},
+			"key2": {
+				Body: []byte("value2"),
+			},
+			"key3": {
+				Body: []byte("value3"),
+			},
+			"key4": {
+				Body: []byte("value4"),
+			},
+			"key5": {
+				Body: []byte("value5"),
+			},
+		},
+	}
+
+	hash1 := CalcRemoteConfigHash(&remoteConfig)
+	hash2 := CalcRemoteConfigHash(&remoteConfig)
+	assert.Equal(t, hash1, hash2)
+}