Make Hash field calculations internal responsibility (#73)

The Hash is now calculated internally by the client instead of requiring
the caller to use the helper function to calculate the Hash before
passing the message to the client.

client/client.go

@@ -47,6 +47,10 @@ type OpAMPClient interface {
 	// May be also called after Start(), in which case the attributes will be included
 	// in the next outgoing status report. This is typically used by Agents which allow
 	// their AgentDescription to change dynamically while the OpAMPClient is started.
+	//
+	// The Hash field will be calculated and updated from the content of the rest of
+	// the fields.
+	//
 	// nil values are not allowed and will return an error.
 	SetAgentDescription(descr *protobufs.AgentDescription) error
 

client/clientimpl_test.go

@@ -3,7 +3,6 @@ package client
 import (
 	"context"
 	"errors"
-	"log"
 	"math/rand"
 	"net/http"
 	"net/url"
@@ -33,10 +32,6 @@ func createAgentDescr() *protobufs.AgentDescription {
 			},
 		},
 	}
-	err := CalcHashAgentDescription(agentDescr)
-	if err != nil {
-		log.Fatalln(err)
-	}
 	return agentDescr
 }
 
@@ -414,7 +409,6 @@ func createEffectiveConfig() *protobufs.EffectiveConfig {
 			},
 		},
 	}
-	CalcHashEffectiveConfig(cfg)
 	return cfg
 }
 
@@ -523,7 +517,6 @@ func TestSetAgentDescription(t *testing.T) {
 				Value: &protobufs.AnyValue{Value: &protobufs.AnyValue_StringValue{StringValue: "linux"}},
 			},
 		}
-		assert.NoError(t, CalcHashAgentDescription(clientAgentDescr))
 		assert.NoError(t, client.SetAgentDescription(clientAgentDescr))
 
 		// Verify change is delivered.

client/internal/clientcommon.go

@@ -2,6 +2,7 @@ package internal
 
 import (
 	"context"
+	"crypto/sha256"
 	"errors"
 	"fmt"
 	"sync"
@@ -62,7 +63,6 @@ func (c *ClientCommon) PrepareStart(_ context.Context, settings types.StartSetti
 		settings.RemoteConfigStatus = &protobufs.RemoteConfigStatus{
 			Status: protobufs.RemoteConfigStatus_UNSET,
 		}
-		calcHashRemoteConfigStatus(settings.RemoteConfigStatus)
 	}
 
 	if err := c.ClientSyncedState.SetRemoteConfigStatus(settings.RemoteConfigStatus); err != nil {
@@ -143,8 +143,8 @@ func (c *ClientCommon) PrepareFirstMessage(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	if cfg != nil && len(cfg.Hash) == 0 {
-		return errors.New("EffectiveConfig hash is empty")
+	if cfg != nil {
+		calcHashEffectiveConfig(cfg)
 	}
 
 	c.sender.NextMessage().Update(
@@ -188,6 +188,20 @@ func (c *ClientCommon) SetAgentDescription(descr *protobufs.AgentDescription) er
 	return nil
 }
 
+// calcHashEffectiveConfig calculates and sets the Hash field from the rest of the
+// fields in the message.
+func calcHashEffectiveConfig(msg *protobufs.EffectiveConfig) {
+	h := sha256.New()
+	if msg.ConfigMap != nil {
+		for k, v := range msg.ConfigMap.ConfigMap {
+			h.Write([]byte(k))
+			h.Write(v.Body)
+			h.Write([]byte(v.ContentType))
+		}
+	}
+	msg.Hash = h.Sum(nil)
+}
+
 // UpdateEffectiveConfig fetches the current local effective config using
 // GetEffectiveConfig callback and sends it to the Server using provided Sender.
 func (c *ClientCommon) UpdateEffectiveConfig(ctx context.Context) error {
@@ -196,8 +210,8 @@ func (c *ClientCommon) UpdateEffectiveConfig(ctx context.Context) error {
 	if err != nil {
 		return fmt.Errorf("GetEffectiveConfig failed: %w", err)
 	}
-	if cfg != nil && len(cfg.Hash) == 0 {
-		return errors.New("hash field must be set, use CalcHashEffectiveConfig")
+	if cfg != nil {
+		calcHashEffectiveConfig(cfg)
 	}
 	// Send it to the Server.
 	c.sender.NextMessage().UpdateStatus(func(statusReport *protobufs.StatusReport) {

client/internal/clientstate.go

@@ -2,10 +2,11 @@ package internal
 
 import (
 	"crypto/sha256"
-	"errors"
+	"hash"
 	"sync"
 
 	"github.com/open-telemetry/opamp-go/protobufs"
+	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/proto"
 )
 
@@ -50,7 +51,43 @@ func (s *ClientSyncedState) PackageStatuses() *protobufs.PackageStatuses {
 	return s.packageStatuses
 }
 
+func writeHashKV(hash hash.Hash, kv *protobufs.KeyValue) error {
+	// To keep the implementation simple we convert the data to an equivalent JSON
+	// string and calculate the hash from the string bytes.
+	b, err := protojson.Marshal(kv)
+	if err != nil {
+		return err
+	}
+	hash.Write(b)
+	return nil
+}
+
+func writeHashKVList(hash hash.Hash, kvl []*protobufs.KeyValue) error {
+	for _, kv := range kvl {
+		err := writeHashKV(hash, kv)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func calcHashAgentDescription(msg *protobufs.AgentDescription) error {
+	h := sha256.New()
+	err := writeHashKVList(h, msg.IdentifyingAttributes)
+	if err != nil {
+		return err
+	}
+	err = writeHashKVList(h, msg.NonIdentifyingAttributes)
+	if err != nil {
+		return err
+	}
+	msg.Hash = h.Sum(nil)
+	return nil
+}
+
 // SetAgentDescription sets the AgentDescription in the state.
+// Will calculate the Hash from the content of the other fields.
 func (s *ClientSyncedState) SetAgentDescription(descr *protobufs.AgentDescription) error {
 	if descr == nil {
 		return ErrAgentDescriptionMissing
@@ -60,12 +97,12 @@ func (s *ClientSyncedState) SetAgentDescription(descr *protobufs.AgentDescriptio
 		return ErrAgentDescriptionNoAttributes
 	}
 
-	clone := proto.Clone(descr).(*protobufs.AgentDescription)
-
-	if len(descr.Hash) == 0 {
-		return errors.New("hash field must be set, use CalcHashAgentDescription")
+	if err := calcHashAgentDescription(descr); err != nil {
+		return err
 	}
 
+	clone := proto.Clone(descr).(*protobufs.AgentDescription)
+
 	defer s.mutex.Unlock()
 	s.mutex.Lock()
 	s.agentDescription = clone
@@ -74,22 +111,21 @@ func (s *ClientSyncedState) SetAgentDescription(descr *protobufs.AgentDescriptio
 }
 
 func calcHashRemoteConfigStatus(status *protobufs.RemoteConfigStatus) {
-	// Calculate and set the Hash field from the rest of the fields in the message.
 	h := sha256.New()
 	h.Write(status.LastRemoteConfigHash)
 	h.Write([]byte(status.Status.String()))
 	h.Write([]byte(status.ErrorMessage))
 	status.Hash = h.Sum(nil)
 }
 
+// SetRemoteConfigStatus sets the RemoteConfigStatus in the state.
+// Will calculate the Hash from the content of the other fields.
 func (s *ClientSyncedState) SetRemoteConfigStatus(status *protobufs.RemoteConfigStatus) error {
 	if status == nil {
 		return errRemoteConfigStatusMissing
 	}
 
-	if len(status.Hash) == 0 {
-		return errors.New("hash field must be set, use CalcHashRemoteConfigStatus")
-	}
+	calcHashRemoteConfigStatus(status)
 
 	clone := proto.Clone(status).(*protobufs.RemoteConfigStatus)
 

client/internal/receivedprocessor.go

@@ -142,20 +142,19 @@ func (r *receivedProcessor) rcvRemoteConfig(
 
 	// Respond back with the hash of the config received from the Server.
 	cfgStatus.LastRemoteConfigHash = remoteCfg.ConfigHash
-	calcHashRemoteConfigStatus(cfgStatus)
 
 	// Get the hash of the status before we update it.
 	prevStatus := r.clientSyncedState.RemoteConfigStatus()
-	var cfgStatusHash []byte
+	var prevCfgStatusHash []byte
 	if prevStatus != nil {
-		cfgStatusHash = prevStatus.Hash
+		prevCfgStatusHash = prevStatus.Hash
 	}
 
 	// Remember the status for the future use.
 	_ = r.clientSyncedState.SetRemoteConfigStatus(cfgStatus)
 
 	// Check if the status has changed by comparing the hashes.
-	if !bytes.Equal(cfgStatusHash, cfgStatus.Hash) {
+	if !bytes.Equal(prevCfgStatusHash, cfgStatus.Hash) {
 		// Let the Agent know about the status.
 		r.callbacks.SaveRemoteConfigStatus(ctx, cfgStatus)
 

client/types/callbacks.go

@@ -45,6 +45,9 @@ type Callbacks interface {
 	// the Server while OnRemoteConfig call has not returned will be remembered.
 	// Once OnRemoteConfig call returns it will be called again with the most recent
 	// remote config received.
+	//
+	// The EffectiveConfig.Hash field will be calculated and updated from the content
+	// of the rest of the fields.
 	OnRemoteConfig(
 		ctx context.Context,
 		remoteConfig *protobufs.AgentRemoteConfig,
@@ -60,6 +63,9 @@ type Callbacks interface {
 	// GetEffectiveConfig returns the current effective config. Only one
 	// GetEffectiveConfig call can be active at any time. Until GetEffectiveConfig
 	// returns it will not be called again.
+	//
+	// The Hash field in the returned EffectiveConfig will be calculated and updated
+	// by the caller from the content of the rest of the fields.
 	GetEffectiveConfig(ctx context.Context) (*protobufs.EffectiveConfig, error)
 
 	// OnOpampConnectionSettings is called when the Agent receives an OpAMP

client/types/startsettings.go

@@ -30,6 +30,8 @@ type StartSettings struct {
 	// The remote config status. If nil is passed it will force
 	// the Server to send a remote config back. It is not required to set the Hash
 	// field, it will be calculated by Start() function.
+	// The Hash field will be calculated and updated from the content of the rest of
+	// the fields.
 	RemoteConfigStatus *protobufs.RemoteConfigStatus
 
 	LastConnectionSettingsHash []byte

internal/examples/agent/agent/agent.go

@@ -167,9 +167,6 @@ func (agent *Agent) createAgentIdentity() {
 			},
 		},
 	}
-	if err := client.CalcHashAgentDescription(agent.agentDescription); err != nil {
-		agent.logger.Errorf("Cannot calculate the hash: %v", err)
-	}
 }
 
 func (agent *Agent) updateAgentIdentity(instanceId ulid.ULID) {