From ba26cf5ebd8205b30122600704a6e1edfb6c12a1 Mon Sep 17 00:00:00 2001
From: Nancy <42977925+mantis-toboggan-md@users.noreply.github.com>
Date: Thu, 18 Jul 2024 16:06:18 -0700
Subject: [PATCH] do not allow cluster upgrades if node pools would fall more
 than one minor version behind (#11489)

---
 pkg/eks/components/Config.vue                 | 39 ++++++++++-
 pkg/eks/components/CruEKS.vue                 |  1 +
 pkg/eks/components/NodeGroup.vue              |  7 ++
 pkg/eks/components/__tests__/Config.test.ts   | 64 +++++++++++++++++++
 .../components/__tests__/NodeGroup.test.ts    | 57 +++++++++++++++++
 pkg/eks/l10n/en-us.yaml                       |  3 +-
 pkg/eks/types/index.d.ts                      |  1 +
 7 files changed, 169 insertions(+), 3 deletions(-)

diff --git a/pkg/eks/components/Config.vue b/pkg/eks/components/Config.vue
index 92f8367d7f9..1eed140aaae 100644
--- a/pkg/eks/components/Config.vue
+++ b/pkg/eks/components/Config.vue
@@ -1,13 +1,14 @@
 <script lang="ts">
 import { defineComponent, PropType } from 'vue';
 import { EKSConfig, AWS } from '../types';
-import { _EDIT, _VIEW } from '@shell/config/query-params';
+import { _EDIT, _VIEW, _CREATE } from '@shell/config/query-params';
 import semver from 'semver';
 import { Store, mapGetters } from 'vuex';
 
 import { MANAGEMENT } from '@shell/config/types';
 import { SETTING } from '@shell/config/settings';
 import RadioGroup from '@components/Form/Radio/RadioGroup.vue';
+import Banner from '@components/Banner/Banner.vue';
 
 import LabeledSelect from '@shell/components/form/LabeledSelect.vue';
 import KeyValue from '@shell/components/form/KeyValue.vue';
@@ -23,7 +24,8 @@ export default defineComponent({
     RadioGroup,
     KeyValue,
     Checkbox,
-    LabeledInput
+    LabeledInput,
+    Banner
   },
 
   props: {
@@ -31,6 +33,7 @@ export default defineComponent({
       type:    String,
       default: _EDIT
     },
+
     isNewOrUnprovisioned: {
       type:    Boolean,
       default: true
@@ -40,6 +43,7 @@ export default defineComponent({
       type:    Boolean,
       default: false
     },
+
     eksRoles: {
       type:    Array,
       default: () => []
@@ -49,26 +53,32 @@ export default defineComponent({
       type:    Object,
       default: () => {}
     },
+
     kmsKey: {
       type:    String,
       default: ''
     },
+
     serviceRole: {
       type:    String,
       default: ''
     },
+
     kubernetesVersion: {
       type:    String,
       default: ''
     },
+
     enableNetworkPolicy: {
       type:    Boolean,
       default: false
     },
+
     ebsCSIDriver: {
       type:    Boolean,
       default: false
     },
+
     config: {
       type:     Object as PropType<EKSConfig>,
       required: true
@@ -120,6 +130,7 @@ export default defineComponent({
       },
       immediate: true
     },
+
     'encryptSecrets'(neu) {
       if (!neu) {
         this.$emit('update:kmsKey', '');
@@ -146,6 +157,23 @@ export default defineComponent({
   computed: {
     ...mapGetters({ t: 'i18n/t' }),
 
+    // the control plane k8s version can't be more than one minor version ahead of any node pools
+    // verify that all nodepools are on the same version as the control plane before showing upgrade optiopns
+    canUpgrade(): boolean {
+      if (this.mode === _CREATE) {
+        return true;
+      }
+      const nodeGroups = this.config?.nodeGroups || [];
+
+      const needsUpgrade = nodeGroups.filter((group) => semver.gt(semver.coerce(this.originalVersion), semver.coerce(group.version)) || group._isUpgrading);
+
+      return !needsUpgrade.length;
+    },
+
+    hasUpgradesAvailable() {
+      return this.versionOptions.filter((opt) => !opt.disabled).length > 1;
+    },
+
     versionOptions(): {value: string, label: string, disabled?:boolean}[] {
       return this.allKubernetesVersions.reduce((versions, v: string) => {
         const coerced = semver.coerce(v);
@@ -238,6 +266,12 @@ export default defineComponent({
 
 <template>
   <div>
+    <Banner
+      v-if="!canUpgrade && hasUpgradesAvailable"
+      color="info"
+      label-key="eks.version.upgradeDisallowed"
+      data-testid="eks-version-upgrade-disallowed-banner"
+    />
     <div
       :style="{'display':'flex',
                'align-items':'center'}"
@@ -253,6 +287,7 @@ export default defineComponent({
           :taggable="true"
           :searchable="true"
           data-testid="eks-version-dropdown"
+          :disabled="!canUpgrade && hasUpgradesAvailable"
           @input="$emit('update:kubernetesVersion', $event)"
         />
       </div>
diff --git a/pkg/eks/components/CruEKS.vue b/pkg/eks/components/CruEKS.vue
index c1ce1e39bcc..a044d1c073c 100644
--- a/pkg/eks/components/CruEKS.vue
+++ b/pkg/eks/components/CruEKS.vue
@@ -656,6 +656,7 @@ export default defineComponent({
             :request-spot-instances.sync="node.requestSpotInstances"
             :labels.sync="node.labels"
             :version.sync="node.version"
+            :pool-is-upgrading.sync="node._isUpgrading"
             :cluster-version="config.kubernetesVersion"
             :original-cluster-version="originalVersion"
             :region="config.region"
diff --git a/pkg/eks/components/NodeGroup.vue b/pkg/eks/components/NodeGroup.vue
index 52d3a804756..675c1236078 100644
--- a/pkg/eks/components/NodeGroup.vue
+++ b/pkg/eks/components/NodeGroup.vue
@@ -199,6 +199,11 @@ export default defineComponent({
       default: null
     },
 
+    poolIsUpgrading: {
+      type:    Boolean,
+      default: false
+    },
+
     loadingInstanceTypes: {
       type:    Boolean,
       default: false
@@ -408,8 +413,10 @@ export default defineComponent({
       set(neu: boolean) {
         if (neu) {
           this.$emit('update:version', this.clusterVersion);
+          this.$emit('update:poolIsUpgrading', true);
         } else {
           this.$emit('update:version', this.originalNodeVersion);
+          this.$emit('update:poolIsUpgrading', false);
         }
       }
     },
diff --git a/pkg/eks/components/__tests__/Config.test.ts b/pkg/eks/components/__tests__/Config.test.ts
index 699a6fc4e80..aa7a1f00b49 100644
--- a/pkg/eks/components/__tests__/Config.test.ts
+++ b/pkg/eks/components/__tests__/Config.test.ts
@@ -243,4 +243,68 @@ describe('eKS K8s configuration', () => {
 
     expect(versionOptions.filter((opt: {label: string, value: string, disabled?:boolean}) => !opt.disabled).map((opt: {label: string, value: string, disabled?:boolean}) => opt.value)).toStrictEqual(enabledVersions);
   });
+
+  it('should not allow the user to upgrade if any node groups are of a lower version', async() => {
+    const setup = requiredSetup({ value: '>1.24' });
+    const wrapper = shallowMount(Config, {
+      propsData: {
+        config: {
+          amazonCredentialSecret: '', region: '', nodeGroups: [{ version: '1.26' }, { version: '1.25' }]
+        },
+        originalVersion: '1.26',
+      },
+      ...setup
+    });
+
+    await setCredential(wrapper);
+
+    const versionDropdown = wrapper.find('[data-testid="eks-version-dropdown"]');
+    const upgradesDisallowedBanner = wrapper.find('[data-testid="eks-version-upgrade-disallowed-banner"]');
+
+    expect(versionDropdown.props().disabled).toBe(true);
+    expect(upgradesDisallowedBanner.isVisible()).toBe(true);
+  });
+
+  // setting/unsetting _isUpgrading is tested in ./NodeGroup.test.ts
+  it('should not allow the user to upgrade if any node groups are currently being upgraded', async() => {
+    const setup = requiredSetup({ value: '>1.24' });
+    const wrapper = shallowMount(Config, {
+      propsData: {
+        config: {
+          amazonCredentialSecret: '', region: '', nodeGroups: [{ version: '1.26' }, { version: '1.26', _isUpgrading: true }]
+        },
+        originalVersion: '1.26',
+      },
+      ...setup
+    });
+
+    await setCredential(wrapper);
+
+    const versionDropdown = wrapper.find('[data-testid="eks-version-dropdown"]');
+    const upgradesDisallowedBanner = wrapper.find('[data-testid="eks-version-upgrade-disallowed-banner"]');
+
+    expect(versionDropdown.props().disabled).toBe(true);
+    expect(upgradesDisallowedBanner.isVisible()).toBe(true);
+  });
+
+  it('should allow the user to upgrade if all node groups are the same version as the current cluster version', async() => {
+    const setup = requiredSetup({ value: '>1.24' });
+    const wrapper = shallowMount(Config, {
+      propsData: {
+        config: {
+          amazonCredentialSecret: '', region: '', nodeGroups: [{ version: '1.26' }, { version: '1.26' }]
+        },
+        originalVersion: '1.26',
+      },
+      ...setup
+    });
+
+    await setCredential(wrapper);
+
+    const versionDropdown = wrapper.find('[data-testid="eks-version-dropdown"]');
+    const upgradesDisallowedBanner = wrapper.find('[data-testid="eks-version-upgrade-disallowed-banner"]');
+
+    expect(versionDropdown.props().disabled).toBe(false);
+    expect(upgradesDisallowedBanner.exists()).toBe(false);
+  });
 });
diff --git a/pkg/eks/components/__tests__/NodeGroup.test.ts b/pkg/eks/components/__tests__/NodeGroup.test.ts
index 33ddd81fe9b..8ace0bb668a 100644
--- a/pkg/eks/components/__tests__/NodeGroup.test.ts
+++ b/pkg/eks/components/__tests__/NodeGroup.test.ts
@@ -351,6 +351,63 @@ describe('eks node groups: edit', () => {
     expect(wrapper.emitted('update:version')?.[0]?.[0]).toBe('1.24');
   });
 
+  it('should report that a pool is being upgraded when the upgrade checkbox is checked', async() => {
+    const setup = requiredSetup();
+
+    const wrapper = shallowMount(NodeGroup, {
+      propsData: {
+        launchTemplate:         {},
+        region:                 'foo',
+        amazonCredentialSecret: 'bar',
+        version:                '1.23',
+        clusterVersion:         '1.24',
+        originalClusterVersion: '1.24',
+        isNewOrUnprovisioned:   false
+      },
+      ...setup
+    });
+
+    expect(wrapper.emitted('update:poolIsUpgrading')).toBeUndefined();
+
+    const upgradeVersionCheckbox = wrapper.find('[data-testid="eks-version-upgrade-checkbox"]');
+
+    upgradeVersionCheckbox.vm.$emit('input', true);
+    await wrapper.vm.$nextTick();
+
+    expect(wrapper.emitted('update:poolIsUpgrading')?.[0]?.[0]).toBe(true);
+  });
+
+  it('should report that a pool is NOT being upgraded when the upgrade checkbox is unchecked', async() => {
+    const setup = requiredSetup();
+
+    const wrapper = shallowMount(NodeGroup, {
+      propsData: {
+        launchTemplate:         {},
+        region:                 'foo',
+        amazonCredentialSecret: 'bar',
+        version:                '1.23',
+        clusterVersion:         '1.24',
+        originalClusterVersion: '1.24',
+        isNewOrUnprovisioned:   false
+      },
+      ...setup
+    });
+
+    expect(wrapper.emitted('update:poolIsUpgrading')).toBeUndefined();
+
+    const upgradeVersionCheckbox = wrapper.find('[data-testid="eks-version-upgrade-checkbox"]');
+
+    wrapper.setProps({ version: '1.24' });
+    await wrapper.vm.$nextTick();
+
+    expect(upgradeVersionCheckbox.props().value).toBe(true);
+
+    upgradeVersionCheckbox.vm.$emit('input', false);
+    await wrapper.vm.$nextTick();
+
+    expect(wrapper.emitted('update:poolIsUpgrading')?.[0]?.[0]).toBe(false);
+  });
+
   it('should revert the node version to its original version when the upgrade checkbox is unchecked', async() => {
     const setup = requiredSetup();
 
diff --git a/pkg/eks/l10n/en-us.yaml b/pkg/eks/l10n/en-us.yaml
index f0dc7066117..b3e450af084 100644
--- a/pkg/eks/l10n/en-us.yaml
+++ b/pkg/eks/l10n/en-us.yaml
@@ -118,6 +118,7 @@ eks:
     label: Tags
   version:
     label: Kubernetes Version
-    upgradeWarning: (minor versio n >1 not allowed by EKS)
+    upgradeWarning: (minor version >1 not allowed by EKS)
+    upgradeDisallowed: "EKS only allows upgrades of one minor version: you must upgrade all node group versions before upgrading the cluster version."
   vpcSubnet:
     label: VPC and Subnet
\ No newline at end of file
diff --git a/pkg/eks/types/index.d.ts b/pkg/eks/types/index.d.ts
index 52fb27204e9..545ff9f1259 100644
--- a/pkg/eks/types/index.d.ts
+++ b/pkg/eks/types/index.d.ts
@@ -36,6 +36,7 @@ export interface EKSNodeGroup {
   version?: string
   __nameUnique?: boolean
   _isNew?: boolean,
+  _isUpgrading: boolean
 }
 
 export interface EKSConfig {