From 341d6ed9ccdb1d5237a9b000809c7d2ee6460455 Mon Sep 17 00:00:00 2001
From: Saverio Proto <saverioproto@microsoft.com>
Date: Thu, 15 Dec 2022 16:59:44 +0100
Subject: [PATCH] auto-upgrade: variable orchestrator_version to null

---
 locals.tf              | 18 +++++++---------
 main.tf                |  2 +-
 test/unit/unit_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/locals.tf b/locals.tf
index 4dbb4f5b..fca4e41d 100644
--- a/locals.tf
+++ b/locals.tf
@@ -1,15 +1,13 @@
 locals {
   # automatic upgrades are either:
-  # - null 
-  # - patch, but then the kubernetes_version musnt't specify a patch number
-  # - rapid/stable/node-image, but then the kubernetes_version must be null
-  automatic_channel_upgrade_check = var.automatic_channel_upgrade == null ? true : (
-    contains(["patch"], var.automatic_channel_upgrade) &&
-    can(regex("^[0-9]{1,}\\.[0-9]{1,}$", var.kubernetes_version))
-    ) ? true : (
-    contains(["rapid", "stable", "node-image"], var.automatic_channel_upgrade) &&
-    var.kubernetes_version == null
-  )
+  # - null
+  # - patch, but then the kubernetes_version must not specify a patch number and orchestrator_version must be null
+  # - rapid/stable/node-image, but then the kubernetes_version and the orchestrator_version must be null
+  automatic_channel_upgrade_check = var.automatic_channel_upgrade == null ? true : var.orchestrator_version == null && (
+    (contains(["patch"], var.automatic_channel_upgrade) && can(regex("^[0-9]{1,}\\.[0-9]{1,}$", var.kubernetes_version)))
+    || (contains(["rapid", "stable", "node-image"], var.automatic_channel_upgrade) && var.kubernetes_version == null
+  ))
+
   # Abstract the decision whether to create an Analytics Workspace or not.
   create_analytics_solution  = var.log_analytics_workspace_enabled && var.log_analytics_solution_id == null
   create_analytics_workspace = var.log_analytics_workspace_enabled && var.log_analytics_workspace == null
diff --git a/main.tf b/main.tf
index 9651200d..73a37433 100644
--- a/main.tf
+++ b/main.tf
@@ -254,7 +254,7 @@ resource "azurerm_kubernetes_cluster" "main" {
     }
     precondition {
       condition     = local.automatic_channel_upgrade_check
-      error_message = "Either disable automatic upgrades, only specify up to the minor version when using `automatic_channel_upgrade=patch` or don't specify `kubernetes_version` at all when using `automatic_channel_upgrade=stable|rapid|node-image`."
+      error_message = "Either disable automatic upgrades, or only specify up to the minor version when using `automatic_channel_upgrade=patch` or don't specify `kubernetes_version` at all when using `automatic_channel_upgrade=stable|rapid|node-image`. With automatic upgrades `orchestrator_version` must be set to `null`."
     }
   }
 }
diff --git a/test/unit/unit_test.go b/test/unit/unit_test.go
index dd867c35..a0d3a141 100644
--- a/test/unit/unit_test.go
+++ b/test/unit/unit_test.go
@@ -150,6 +150,55 @@ func TestAutomaticUpgrades(t *testing.T) {
 	}
 }
 
+func TestInvalidVarsForAutomaticUpgrades(t *testing.T) {
+	testCases := map[string]struct {
+		name string
+		vars map[string]interface{}
+	}{
+		"automatic_patches_with_patch_number_specified": {
+			vars: map[string]interface{}{
+				"prefix":                    "foo",
+				"resource_group_name":       "bar",
+				"automatic_channel_upgrade": "patch",
+				"kubernetes_version":        "1.25.3",
+				"orchestrator_version":      "1.25.3",
+			},
+		},
+		"automatic_upgrades_to_newest_version_with_fixed_versions": {
+			vars: map[string]interface{}{
+				"prefix":                    "foo",
+				"resource_group_name":       "bar",
+				"automatic_channel_upgrade": "rapid",
+				"kubernetes_version":        "1.25.3",
+				"orchestrator_version":      "1.24",
+			},
+		},
+		"automatic_upgrades_to_stable_version_with_orchestrator": {
+			vars: map[string]interface{}{
+				"prefix":                    "foo",
+				"resource_group_name":       "bar",
+				"automatic_channel_upgrade": "stable",
+				"orchestrator_version":      "1.24",
+			},
+		},
+	}
+
+	for name, tt := range testCases {
+		t.Run(name, func(t *testing.T) {
+			test_helper.RunE2ETest(t, "../../", "unit-test-fixture",
+				terraform.Options{
+					Upgrade: false,
+					Vars:    tt.vars,
+				},
+				func(t *testing.T, output test_helper.TerraformOutput) {
+					upgrades, ok := output["automatic_channel_upgrade_check"].(bool)
+					assert.True(t, ok)
+					assert.False(t, upgrades)
+				})
+		})
+	}
+}
+
 func dummyRequiredVariables() map[string]interface{} {
 	return map[string]interface{}{
 		"prefix":              "foo",