Add agent hash to check for detecting upgrade rollbacks (#3842)

* Add agent hash to check for detecting upgrade rollbacks
elastic · Nov 30, 2023 · 4a8661d · 4a8661d
1 parent 8a8abd0
commit 4a8661d
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 5 deletions.
diff --git a/internal/pkg/agent/application/upgrade/marker_watcher.go b/internal/pkg/agent/application/upgrade/marker_watcher.go
@@ -91,18 +91,18 @@ func (mfw *MarkerFileWatcher) Run(ctx context.Context) error {
 				case e.Op&(fsnotify.Create|fsnotify.Write) != 0:
 					// Upgrade marker file was created or updated; read its contents
 					// and send them over the update channel.
-					mfw.processMarker(version.GetAgentPackageVersion())
+					mfw.processMarker(version.GetAgentPackageVersion(), version.Commit())
 				}
 			case <-doInitialRead:
-				mfw.processMarker(version.GetAgentPackageVersion())
+				mfw.processMarker(version.GetAgentPackageVersion(), version.Commit())
 			}
 		}
 	}()
 
 	return nil
 }
 
-func (mfw *MarkerFileWatcher) processMarker(currentVersion string) {
+func (mfw *MarkerFileWatcher) processMarker(currentVersion string, commit string) {
 	marker, err := loadMarker(mfw.markerFilePath)
 	if err != nil {
 		mfw.logger.Error(err)
@@ -120,7 +120,7 @@ func (mfw *MarkerFileWatcher) processMarker(currentVersion string) {
 	// been recorded in the marker's upgrade details field but, in case it
 	// isn't for some reason, we fallback to explicitly setting that state as
 	// part of the upgrade details in the marker.
-	if marker.PrevVersion == currentVersion {
+	if marker.PrevVersion == currentVersion && marker.PrevHash == commit {
 		if marker.Details == nil {
 			marker.Details = details.NewDetails("unknown", details.StateRollback, marker.GetActionID())
 		} else {

diff --git a/internal/pkg/agent/application/upgrade/marker_watcher_test.go b/internal/pkg/agent/application/upgrade/marker_watcher_test.go
@@ -84,6 +84,9 @@ func TestProcessMarker(t *testing.T) {
 	cases := map[string]struct {
 		markerFileContents string
 
+		currentAgentVersion string
+		currentAgentHash    string
+
 		expectedErrLogMsg bool
 		expectedDetails   *details.Details
 	}{
@@ -146,6 +149,51 @@ details:
 				State:         details.StateWatching,
 			},
 		},
+		"same_version_different_hash": {
+			markerFileContents: `
+prev_version: 8.9.2
+prev_hash: aaaaaa
+details:
+  target_version: 8.9.2
+  state: UPG_WATCHING
+`,
+			currentAgentVersion: "8.9.2",
+			currentAgentHash:    "bbbbbb",
+			expectedErrLogMsg:   false,
+			expectedDetails: &details.Details{
+				TargetVersion: "8.9.2",
+				State:         details.StateWatching,
+			},
+		},
+		"same_version_same_hash": {
+			markerFileContents: `
+prev_version: 8.9.2
+prev_hash: aaaaaa
+details:
+  target_version: 8.9.2
+  state: UPG_WATCHING
+`,
+			currentAgentVersion: "8.9.2",
+			currentAgentHash:    "aaaaaa",
+			expectedErrLogMsg:   false,
+			expectedDetails: &details.Details{
+				TargetVersion: "8.9.2",
+				State:         details.StateRollback,
+			},
+		},
+		"same_version_same_hash_no_details": {
+			markerFileContents: `
+prev_version: 8.9.2
+prev_hash: aaaaaa
+`,
+			currentAgentVersion: "8.9.2",
+			currentAgentHash:    "aaaaaa",
+			expectedErrLogMsg:   false,
+			expectedDetails: &details.Details{
+				TargetVersion: "unknown",
+				State:         details.StateRollback,
+			},
+		},
 	}
 
 	for name, test := range cases {
@@ -182,7 +230,19 @@ details:
 				}
 			}()
 
-			mfw.processMarker("8.9.2")
+			// default values for version and hash
+			currentVersion := "8.9.2"
+			currentCommit := ""
+
+			// apply overrides from testcase
+			if test.currentAgentVersion != "" {
+				currentVersion = test.currentAgentVersion
+			}
+			if test.currentAgentHash != "" {
+				currentCommit = test.currentAgentHash
+			}
+
+			mfw.processMarker(currentVersion, currentCommit)
 
 			// error loading marker
 			if test.expectedErrLogMsg {