Validate AKS Machine goal responses

pkg/aksmachine/client_armapi.go

@@ -77,9 +77,12 @@ func (c *armMachineClient) Create(ctx context.Context, desired GoalState) (*Mach
 	if err := c.validateMachineIdentity(resp.Machine); err != nil {
 		return nil, err
 	}
-	result := machineFromARM(resp.Machine, desired)
+	result := machineFromARM(resp.Machine)
 	result.ID = c.machineID.String()
 	result.Name = c.machineID.Name
+	if err := result.Validate(); err != nil {
+		return nil, fmt.Errorf("validate create machine response: %w", err)
+	}
 	return result, nil
 }
 
@@ -103,9 +106,12 @@ func (c *armMachineClient) Get(ctx context.Context) (*Machine, error) {
 	if err := c.validateMachineIdentity(resp.Machine); err != nil {
 		return nil, err
 	}
-	result := machineFromARM(resp.Machine, GoalState{})
+	result := machineFromARM(resp.Machine)
 	result.ID = c.machineID.String()
 	result.Name = c.machineID.Name
+	if err := result.Validate(); err != nil {
+		return nil, fmt.Errorf("validate get machine response: %w", err)
+	}
 	return result, nil
 }
 
@@ -256,8 +262,8 @@ func (c *armMachineClient) validateMachineIdentity(machine armcontainerservice.M
 	return nil
 }
 
-func machineFromARM(machine armcontainerservice.Machine, fallback GoalState) *Machine {
-	result := &Machine{Goal: fallback}
+func machineFromARM(machine armcontainerservice.Machine) *Machine {
+	result := &Machine{}
 	if machine.ID != nil {
 		result.ID = *machine.ID
 	}
@@ -274,9 +280,6 @@ func machineFromARM(machine armcontainerservice.Machine, fallback GoalState) *Ma
 		if kubernetes.OrchestratorVersion != nil {
 			result.Goal.KubernetesVersion = *kubernetes.OrchestratorVersion
 		}
-		if result.Goal.KubernetesVersion == "" && kubernetes.CurrentOrchestratorVersion != nil {
-			result.Goal.KubernetesVersion = *kubernetes.CurrentOrchestratorVersion
-		}
 		if kubernetes.MaxPods != nil {
 			result.Goal.MaxPods = int(*kubernetes.MaxPods)
 		}
@@ -298,9 +301,6 @@ func machineFromARM(machine armcontainerservice.Machine, fallback GoalState) *Ma
 	if properties.ETag != nil {
 		result.Goal.SettingsVersion = *properties.ETag
 	}
-	if result.Goal.SettingsVersion == "" {
-		result.Goal.SettingsVersion = result.Goal.KubernetesVersion
-	}
 	if properties.ProvisioningState != nil {
 		result.Status.ProvisioningState = ProvisioningState(*properties.ProvisioningState)
 	}

pkg/aksmachine/client_armapi_test.go

@@ -317,7 +317,7 @@ func TestMachineFromARM(t *testing.T) {
 			},
 			ProvisioningState: ptr("Succeeded"),
 		},
-	}, GoalState{SettingsVersion: "fallback-settings"})
+	})
 
 	if machine.ID != "machine-id" || machine.Name != "node1" {
 		t.Fatalf("machine identity = %#v", machine)
@@ -398,7 +398,7 @@ func TestValidateMachineIdentity(t *testing.T) {
 	}
 }
 
-func TestMachineFromARMUsesCurrentOrchestratorVersionFallback(t *testing.T) {
+func TestMachineFromARMDoesNotUseCurrentOrchestratorVersionAsGoal(t *testing.T) {
 	t.Parallel()
 
 	currentVersion := "1.35.2"
@@ -408,13 +408,26 @@ func TestMachineFromARMUsesCurrentOrchestratorVersionFallback(t *testing.T) {
 				CurrentOrchestratorVersion: &currentVersion,
 			},
 		},
-	}, GoalState{})
+	})
 
-	if machine.Goal.KubernetesVersion != currentVersion {
-		t.Fatalf("KubernetesVersion = %q, want %q", machine.Goal.KubernetesVersion, currentVersion)
+	if machine.Goal.KubernetesVersion != "" {
+		t.Fatalf("KubernetesVersion = %q, want empty without desired orchestratorVersion", machine.Goal.KubernetesVersion)
 	}
-	if machine.Goal.SettingsVersion != currentVersion {
-		t.Fatalf("SettingsVersion = %q, want %q", machine.Goal.SettingsVersion, currentVersion)
+}
+
+func TestMachineFromARMDoesNotSynthesizeSettingsVersion(t *testing.T) {
+	t.Parallel()
+
+	machine := machineFromARM(armcontainerservice.Machine{
+		Properties: &armcontainerservice.MachineProperties{
+			Kubernetes: &armcontainerservice.MachineKubernetesProfile{
+				OrchestratorVersion: ptr("1.35.2"),
+			},
+		},
+	})
+
+	if machine.Goal.SettingsVersion != "" {
+		t.Fatalf("SettingsVersion = %q, want empty without ETag", machine.Goal.SettingsVersion)
 	}
 }
 

pkg/aksmachine/client_incluster.go

@@ -170,8 +170,8 @@ func (c *clusterEndpointClient) adoptExistingMachine(ctx context.Context, desire
 }
 
 func validateAdoptedMachine(machine *Machine, desired GoalState) error {
-	if machine == nil {
-		return fmt.Errorf("cluster endpoint returned nil machine")
+	if err := machine.Validate(); err != nil {
+		return fmt.Errorf("cluster endpoint returned invalid machine: %w", err)
 	}
 	if desired.KubernetesVersion != "" && machine.Goal.KubernetesVersion != "" && machine.Goal.KubernetesVersion != desired.KubernetesVersion {
 		return fmt.Errorf("pre-created machine Kubernetes version %q does not match desired %q", machine.Goal.KubernetesVersion, desired.KubernetesVersion)
@@ -279,7 +279,11 @@ func machineFromEndpointJSON(data []byte) (*Machine, error) {
 	if err := json.Unmarshal(data, &armMachine); err != nil {
 		return nil, fmt.Errorf("decode cluster endpoint machine response: %w", err)
 	}
-	return machineFromARM(armMachine, GoalState{}), nil
+	machine := machineFromARM(armMachine)
+	if err := machine.Validate(); err != nil {
+		return nil, fmt.Errorf("validate cluster endpoint machine response: %w", err)
+	}
+	return machine, nil
 }
 
 func joinEndpointURLPath(base string, elem ...string) string {

pkg/aksmachine/client_incluster_test.go

@@ -88,6 +88,19 @@ func TestMachineFromEndpointJSONUsesARMModel(t *testing.T) {
 	}
 }
 
+func TestMachineFromEndpointJSONRejectsMissingETag(t *testing.T) {
+	t.Parallel()
+
+	_, err := machineFromEndpointJSON([]byte(`{
+  "properties": {
+    "kubernetes": {"orchestratorVersion": "1.34.0"}
+  }
+}`))
+	if err == nil || !strings.Contains(err.Error(), "goal settings version is empty") {
+		t.Fatalf("machineFromEndpointJSON() error = %v, want missing settings version", err)
+	}
+}
+
 func TestClusterEndpointClientNotFound(t *testing.T) {
 	t.Parallel()
 

pkg/aksmachine/ensure.go

@@ -30,18 +30,18 @@ func (t *ensureMachineTask) Name() string { return "ensure-machine" }
 func (t *ensureMachineTask) Do(ctx context.Context) error {
 	machine, err := t.machines.Get(ctx)
 	if err == nil {
-		if machine != nil && machine.Goal.KubernetesVersion == t.goal.KubernetesVersion {
+		if err := machine.Validate(); err != nil {
+			return t.handleError("get machine", fmt.Errorf("AKS returned an invalid machine: %w", err))
+		}
+		if machine.Goal.KubernetesVersion == t.goal.KubernetesVersion {
 			t.logger.Info("machine already registered, skipping")
-			return t.adoptSettingsVersion(machine, "get machine")
+			t.goal.SettingsVersion = machine.Goal.SettingsVersion
+			return nil
 		}
 
-		remoteVersion := ""
-		if machine != nil {
-			remoteVersion = machine.Goal.KubernetesVersion
-		}
 		t.logger.Info(
 			"updating registered machine from local bootstrap config",
-			"remoteKubernetesVersion", remoteVersion,
+			"remoteKubernetesVersion", machine.Goal.KubernetesVersion,
 			"localKubernetesVersion", t.goal.KubernetesVersion,
 		)
 		machine, err = t.machines.Create(ctx, *t.goal)
@@ -63,8 +63,8 @@ func (t *ensureMachineTask) Do(ctx context.Context) error {
 }
 
 func (t *ensureMachineTask) adoptSettingsVersion(machine *Machine, operation string) error {
-	if machine == nil {
-		return t.handleError(operation, fmt.Errorf("AKS returned a nil machine"))
+	if err := machine.Validate(); err != nil {
+		return t.handleError(operation, fmt.Errorf("AKS returned an invalid machine: %w", err))
 	}
 	if machine.Goal.KubernetesVersion != t.goal.KubernetesVersion {
 		return t.handleError(
@@ -76,9 +76,7 @@ func (t *ensureMachineTask) adoptSettingsVersion(machine *Machine, operation str
 			),
 		)
 	}
-	if machine.Goal.SettingsVersion != "" {
-		t.goal.SettingsVersion = machine.Goal.SettingsVersion
-	}
+	t.goal.SettingsVersion = machine.Goal.SettingsVersion
 	return nil
 }
 

pkg/aksmachine/ensure_test.go

@@ -84,7 +84,7 @@ func TestEnsureMachineGetFailure(t *testing.T) {
 func TestEnsureMachineCreatesAndAdoptsSettingsVersion(t *testing.T) {
 	t.Parallel()
 
-	goal := GoalState{KubernetesVersion: "1.35.1", SettingsVersion: "1.35.1"}
+	goal := GoalState{KubernetesVersion: "1.35.1"}
 	client := &ensureMachineClient{createResult: &Machine{Goal: GoalState{
 		KubernetesVersion: "1.35.1",
 		SettingsVersion:   "etag-created",
@@ -107,7 +107,6 @@ func TestEnsureMachineAdoptsExistingSettingsVersionWithoutReplacingLocalGoal(t *
 
 	goal := GoalState{
 		KubernetesVersion: "1.35.1",
-		SettingsVersion:   "1.35.1",
 		MaxPods:           30,
 		NodeLabels:        map[string]string{"source": "local"},
 		NodeTaints:        []string{"local=true:NoSchedule"},
@@ -149,7 +148,7 @@ func TestEnsureMachineAdoptsExistingSettingsVersionWithoutReplacingLocalGoal(t *
 func TestEnsureMachineUpdatesMismatchedVersion(t *testing.T) {
 	t.Parallel()
 
-	goal := GoalState{KubernetesVersion: "1.35.1", SettingsVersion: "1.35.1"}
+	goal := GoalState{KubernetesVersion: "1.35.1"}
 	client := &ensureMachineClient{
 		machine: &Machine{Goal: GoalState{KubernetesVersion: "1.34.0", SettingsVersion: "etag-old"}},
 		createResult: &Machine{Goal: GoalState{
@@ -176,7 +175,7 @@ func TestEnsureMachineUpdatesMismatchedVersion(t *testing.T) {
 func TestEnsureMachineRejectsUnchangedRemoteVersionAfterUpdate(t *testing.T) {
 	t.Parallel()
 
-	goal := GoalState{KubernetesVersion: "1.35.1", SettingsVersion: "1.35.1"}
+	goal := GoalState{KubernetesVersion: "1.35.1"}
 	client := &ensureMachineClient{
 		machine: &Machine{Goal: GoalState{KubernetesVersion: "1.34.0", SettingsVersion: "etag-old"}},
 		createResult: &Machine{Goal: GoalState{
@@ -190,13 +189,63 @@ func TestEnsureMachineRejectsUnchangedRemoteVersionAfterUpdate(t *testing.T) {
 	if err == nil || !strings.Contains(err.Error(), `AKS machine Kubernetes version "1.34.0" does not match local bootstrap version "1.35.1"`) {
 		t.Fatalf("Do() error = %v, want version mismatch", err)
 	}
-	if goal.SettingsVersion != "1.35.1" {
-		t.Fatalf("SettingsVersion = %q, want local fallback", goal.SettingsVersion)
+	if goal.SettingsVersion != "" {
+		t.Fatalf("SettingsVersion = %q, want empty before a valid Machine response", goal.SettingsVersion)
+	}
+}
+
+func TestEnsureMachineRejectsInvalidExistingMachine(t *testing.T) {
+	t.Parallel()
+
+	tests := map[string]struct {
+		machine *Machine
+		require bool
+		wantErr string
+	}{
+		"best effort preserves local goal after missing settings version": {
+			machine: &Machine{Goal: GoalState{KubernetesVersion: "1.35.1"}},
+		},
+		"required rejects missing settings version": {
+			machine: &Machine{Goal: GoalState{KubernetesVersion: "1.35.1"}},
+			require: true,
+			wantErr: "goal settings version is empty",
+		},
+		"best effort preserves local goal after nil response": {},
+		"required rejects nil response": {
+			require: true,
+			wantErr: "machine is nil",
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			goal := GoalState{KubernetesVersion: "1.35.1", NodeLabels: map[string]string{"source": "local"}}
+			client := &ensureMachineClient{machine: tt.machine, getResultSet: true}
+			task := EnsureMachine(client, &goal, tt.require, slog.New(slog.NewTextHandler(io.Discard, nil)))
+
+			err := task.Do(t.Context())
+			if tt.wantErr != "" {
+				if err == nil || !strings.Contains(err.Error(), tt.wantErr) {
+					t.Fatalf("Do() error = %v, want containing %q", err, tt.wantErr)
+				}
+			} else if err != nil {
+				t.Fatalf("Do() error = %v", err)
+			}
+			if goal.SettingsVersion != "" || goal.NodeLabels["source"] != "local" {
+				t.Fatalf("local goal changed after invalid response: %#v", goal)
+			}
+			if client.createCalls != 0 {
+				t.Fatalf("Create() calls = %d, want 0", client.createCalls)
+			}
+		})
 	}
 }
 
 type ensureMachineClient struct {
 	machine      *Machine
+	getResultSet bool
 	createResult *Machine
 	getErr       error
 	createErr    error
@@ -208,7 +257,7 @@ func (c *ensureMachineClient) Get(context.Context) (*Machine, error) {
 	if c.getErr != nil {
 		return nil, c.getErr
 	}
-	if c.machine != nil {
+	if c.getResultSet || c.machine != nil {
 		return c.machine, nil
 	}
 	return nil, &NotFoundError{Resource: "machine"}

pkg/aksmachine/types.go

@@ -49,11 +49,8 @@ func (g GoalState) validate() error {
 // GoalStateFromConfig builds and validates the initial AKS machine goal state
 // from local agent configuration.
 func GoalStateFromConfig(cfg *config.Config) (GoalState, error) {
-	// Until the finalized Machine API exposes a settings version in all paths,
-	// use KubernetesVersion as the same stable fallback used by ARM reads.
 	goal := GoalState{
 		KubernetesVersion: cfg.Components.Kubernetes,
-		SettingsVersion:   cfg.Components.Kubernetes,
 		MaxPods:           cfg.Node.MaxPods,
 		NodeLabels:        maps.Clone(cfg.Node.Labels),
 		NodeTaints:        slices.Clone(cfg.Node.Taints),
@@ -93,6 +90,21 @@ type Machine struct {
 	Status Status    `json:"status"`
 }
 
+// Validate verifies that a Machine returned by AKS contains a goal suitable
+// for bootstrap or reconciliation.
+func (m *Machine) Validate() error {
+	if m == nil {
+		return fmt.Errorf("machine is nil")
+	}
+	if err := m.Goal.validate(); err != nil {
+		return fmt.Errorf("goal: %w", err)
+	}
+	if m.Goal.SettingsVersion == "" {
+		return fmt.Errorf("goal settings version is empty")
+	}
+	return nil
+}
+
 // MachineClient provides access to the AKS-side machine representation.
 // Production should use the official Azure SDK implementation once the public
 // SDK contains the finalized resource shape; tests can provide fake or remote