From 83e2ea55bfa00a3566f8729504bd89dd817ed98f Mon Sep 17 00:00:00 2001 From: Nancy Butler <42977925+mantis-toboggan-md@users.noreply.github.com> Date: Mon, 3 Jun 2024 08:10:53 -0700 Subject: [PATCH 1/6] sync upstream eks spec on edit --- pkg/eks/components/CruEKS.vue | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/eks/components/CruEKS.vue b/pkg/eks/components/CruEKS.vue index 0b1c69aaa57..0d808af92bc 100644 --- a/pkg/eks/components/CruEKS.vue +++ b/pkg/eks/components/CruEKS.vue @@ -5,7 +5,7 @@ import { defineComponent } from 'vue'; import { removeObject } from '@shell/utils/array'; import { _CREATE, _EDIT, _VIEW } from '@shell/config/query-params'; import { NORMAN } from '@shell/config/types'; -import { diffUpstreamSpec } from '@shell/utils/kontainer'; +import { diffUpstreamSpec, syncUpstreamConfig } from '@shell/utils/kontainer'; import CreateEditView from '@shell/mixins/create-edit-view'; import FormValidation from '@shell/mixins/form-validation'; @@ -119,6 +119,10 @@ export default defineComponent({ const liveNormanCluster = await this.value.findNormanCluster(); this.normanCluster = await store.dispatch(`rancher/clone`, { resource: liveNormanCluster }); + // ensure any fields editable through this UI that have been altered in azure portal are shown here - see syncUpstreamConfig jsdoc for details + if (!this.isNewOrUnprovisioned) { + syncUpstreamConfig('aks', this.normanCluster); + } // track original version on edit to ensure we don't offer k8s downgrades this.originalVersion = this.normanCluster?.eksConfig?.kubernetesVersion || ''; } else { From 0083f25faa9c6182f00a7ad7a78c1f23055e3062 Mon Sep 17 00:00:00 2001 From: Nancy Butler <42977925+mantis-toboggan-md@users.noreply.github.com> Date: Mon, 24 Jun 2024 15:20:20 -0700 Subject: [PATCH 2/6] allow for nil nodeGroups fix networking subnet mode display --- pkg/eks/components/CruEKS.vue | 10 ++++-- pkg/eks/components/Logging.vue | 6 ++-- pkg/eks/components/Networking.vue | 2 +- shell/utils/__tests__/kontainer.test.ts | 46 ++++++++++++++++++++++++- shell/utils/kontainer.ts | 6 +++- 5 files changed, 61 insertions(+), 9 deletions(-) diff --git a/pkg/eks/components/CruEKS.vue b/pkg/eks/components/CruEKS.vue index 0d808af92bc..f6c66f300b7 100644 --- a/pkg/eks/components/CruEKS.vue +++ b/pkg/eks/components/CruEKS.vue @@ -121,7 +121,7 @@ export default defineComponent({ this.normanCluster = await store.dispatch(`rancher/clone`, { resource: liveNormanCluster }); // ensure any fields editable through this UI that have been altered in azure portal are shown here - see syncUpstreamConfig jsdoc for details if (!this.isNewOrUnprovisioned) { - syncUpstreamConfig('aks', this.normanCluster); + syncUpstreamConfig('eks', this.normanCluster); } // track original version on edit to ensure we don't offer k8s downgrades this.originalVersion = this.normanCluster?.eksConfig?.kubernetesVersion || ''; @@ -140,7 +140,7 @@ export default defineComponent({ } this.config = this.normanCluster.eksConfig as EKSConfig; - if (!this.config.nodeGroups || !this.config.nodeGroups.length) { + if ((!this.config.nodeGroups || !this.config.nodeGroups.length) && this.mode === _CREATE) { this.$set(this.config, 'nodeGroups', [{ ...DEFAULT_NODE_GROUP_CONFIG, nodegroupName: 'group1' }]); } if (this.config.nodeGroups) { @@ -382,7 +382,7 @@ export default defineComponent({ }); return out; - } + }, }, methods: { @@ -413,6 +413,9 @@ export default defineComponent({ }, async actuallySave(): Promise { + if (!this.isNewOrUnprovisioned && !this.nodeGroups.length) { + delete this.normanCluster.eksConfig.nodeGroups; + } await this.normanCluster.save(); return await this.normanCluster.waitForCondition('InitialRolesPopulated'); @@ -527,6 +530,7 @@ export default defineComponent({ :done-route="doneRoute" :errors="fvUnreportedValidationErrors" :validation-passed="fvFormIsValid" + data-testid="eks-cruresource" @error="e=>errors=e" @finish="save" @cancel="done" diff --git a/pkg/eks/components/Logging.vue b/pkg/eks/components/Logging.vue index 0e1da076552..6e854a095f0 100644 --- a/pkg/eks/components/Logging.vue +++ b/pkg/eks/components/Logging.vue @@ -33,13 +33,13 @@ export default defineComponent({ methods: { typeEnabled(type: string) { - return this.loggingTypes.includes(type); + return (this.loggingTypes || []).includes(type); }, toggleType(type:string) { - const out = [...this.loggingTypes]; + const out = [...(this.loggingTypes || [])]; - if (this.loggingTypes.includes(type)) { + if (out.includes(type)) { removeObject(out, type); } else { out.push(type); diff --git a/pkg/eks/components/Networking.vue b/pkg/eks/components/Networking.vue index 65678b5cf52..3d2466d2b12 100644 --- a/pkg/eks/components/Networking.vue +++ b/pkg/eks/components/Networking.vue @@ -89,7 +89,7 @@ export default defineComponent({ loadingVpcs: false, vpcInfo: {} as {Vpcs: AWS.VPC[]}, subnetInfo: {} as {Subnets: AWS.Subnet[]}, - chooseSubnet: this.subnets && !!this.subnets.length + chooseSubnet: !!this.subnets && !!this.subnets.length }; }, diff --git a/shell/utils/__tests__/kontainer.test.ts b/shell/utils/__tests__/kontainer.test.ts index 6764978bd74..bc3bb84a609 100644 --- a/shell/utils/__tests__/kontainer.test.ts +++ b/shell/utils/__tests__/kontainer.test.ts @@ -1,4 +1,4 @@ -import { diffUpstreamSpec } from '@shell/utils/kontainer'; +import { diffUpstreamSpec, syncUpstreamConfig } from '@shell/utils/kontainer'; describe('fx: diffUpstreamSpec', () => { it.each([ @@ -90,3 +90,47 @@ describe('fx: diffUpstreamSpec', () => { expect(diffUpstreamSpec(upstream, local)).toStrictEqual(diff); }); }); + +describe('fx: syncUpstreamSpec', () => { + it('should set any fields defined in upstream spec and not local spec', () => { + const upstream = { + string: 'def', + 'other-string': '123', + emptyArray: [], + emptyObject: {}, + nonEmptyArray: [1, 2, 3], + nonEmptyObject: { foo: 'bar' }, + falseBoolean: false, + trueBoolean: true, + alreadySet: 'abc', + alreadySetArray: [2, 3, 4], + alreadySetBooleanFalse: false, + alreadySetBooleanTrue: true + }; + const local = { + alreadySet: 'def', + alreadySetArray: [1, 2, 3], + alreadySetBooleanFalse: false, + alreadySetBooleanTrue: true + }; + + const expected = { + string: 'def', + 'other-string': '123', + nonEmptyArray: [1, 2, 3], + nonEmptyObject: { foo: 'bar' }, + falseBoolean: false, + trueBoolean: true, + alreadySet: 'def', + alreadySetArray: [1, 2, 3], + alreadySetBooleanFalse: false, + alreadySetBooleanTrue: true + }; + + const testCluster = { eksConfig: local, eksStatus: { upstreamSpec: upstream } }; + + syncUpstreamConfig( 'eks', testCluster); + + expect(testCluster.eksConfig).toStrictEqual(expected); + }); +}); diff --git a/shell/utils/kontainer.ts b/shell/utils/kontainer.ts index 1f81bacbc14..7fcc3e42847 100644 --- a/shell/utils/kontainer.ts +++ b/shell/utils/kontainer.ts @@ -17,7 +17,11 @@ export function syncUpstreamConfig(configPrefix: string, normanCluster: {[key: s if (!isEmpty(upstreamConfig)) { Object.keys(upstreamConfig).forEach((key) => { - if (isEmpty(rancherConfig[key]) && !isEmpty(upstreamConfig[key])) { + if (typeof upstreamConfig[key] === 'object') { + if (isEmpty(rancherConfig[key]) && !isEmpty(upstreamConfig[key])) { + set(rancherConfig, key, upstreamConfig[key]); + } + } else if ((rancherConfig[key] === null || rancherConfig[key] === undefined) && upstreamConfig[key] !== null && upstreamConfig[key] !== undefined) { set(rancherConfig, key, upstreamConfig[key]); } }); From 02e7796807071f27a1620b724da353d0bf63669c Mon Sep 17 00:00:00 2001 From: Nancy Butler <42977925+mantis-toboggan-md@users.noreply.github.com> Date: Tue, 25 Jun 2024 11:20:50 -0700 Subject: [PATCH 3/6] split up syncUpstreamConfig test --- pkg/eks/components/CruEKS.vue | 1 - shell/utils/__tests__/kontainer.test.ts | 64 +++++++++++++++++++++---- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/pkg/eks/components/CruEKS.vue b/pkg/eks/components/CruEKS.vue index f6c66f300b7..b06f51be75e 100644 --- a/pkg/eks/components/CruEKS.vue +++ b/pkg/eks/components/CruEKS.vue @@ -530,7 +530,6 @@ export default defineComponent({ :done-route="doneRoute" :errors="fvUnreportedValidationErrors" :validation-passed="fvFormIsValid" - data-testid="eks-cruresource" @error="e=>errors=e" @finish="save" @cancel="done" diff --git a/shell/utils/__tests__/kontainer.test.ts b/shell/utils/__tests__/kontainer.test.ts index bc3bb84a609..1864c034a07 100644 --- a/shell/utils/__tests__/kontainer.test.ts +++ b/shell/utils/__tests__/kontainer.test.ts @@ -96,12 +96,6 @@ describe('fx: syncUpstreamSpec', () => { const upstream = { string: 'def', 'other-string': '123', - emptyArray: [], - emptyObject: {}, - nonEmptyArray: [1, 2, 3], - nonEmptyObject: { foo: 'bar' }, - falseBoolean: false, - trueBoolean: true, alreadySet: 'abc', alreadySetArray: [2, 3, 4], alreadySetBooleanFalse: false, @@ -117,10 +111,6 @@ describe('fx: syncUpstreamSpec', () => { const expected = { string: 'def', 'other-string': '123', - nonEmptyArray: [1, 2, 3], - nonEmptyObject: { foo: 'bar' }, - falseBoolean: false, - trueBoolean: true, alreadySet: 'def', alreadySetArray: [1, 2, 3], alreadySetBooleanFalse: false, @@ -133,4 +123,58 @@ describe('fx: syncUpstreamSpec', () => { expect(testCluster.eksConfig).toStrictEqual(expected); }); + + it('should not set empty objects or arrays from upstream spec', () => { + const upstream = { + emptyArray: [], + emptyObject: {}, + nonEmptyArray: [1, 2, 3], + nonEmptyObject: { foo: 'bar' }, + alreadySet: 'abc', + alreadySetArray: [2, 3, 4], + }; + const local = { + alreadySet: 'def', + alreadySetArray: [1, 2, 3], + }; + + const expected = { + nonEmptyArray: [1, 2, 3], + nonEmptyObject: { foo: 'bar' }, + alreadySet: 'def', + alreadySetArray: [1, 2, 3], + }; + + const testCluster = { eksConfig: local, eksStatus: { upstreamSpec: upstream } }; + + syncUpstreamConfig( 'eks', testCluster); + + expect(testCluster.eksConfig).toStrictEqual(expected); + }); + + it('should not overwrite boolean values explicitly set false', () => { + const upstream = { + falseBoolean: false, + trueBoolean: true, + alreadySetBooleanFalse: true, + alreadySetBooleanTrue: false + }; + const local = { + alreadySetBooleanFalse: false, + alreadySetBooleanTrue: true + }; + + const expected = { + falseBoolean: false, + trueBoolean: true, + alreadySetBooleanFalse: false, + alreadySetBooleanTrue: true + }; + + const testCluster = { eksConfig: local, eksStatus: { upstreamSpec: upstream } }; + + syncUpstreamConfig( 'eks', testCluster); + + expect(testCluster.eksConfig).toStrictEqual(expected); + }); }); From d3ae4a9bf7637dc49d28a6955689174e9f9252fd Mon Sep 17 00:00:00 2001 From: Nancy Butler <42977925+mantis-toboggan-md@users.noreply.github.com> Date: Tue, 25 Jun 2024 11:51:06 -0700 Subject: [PATCH 4/6] update subnets display for imported clusters and clusters that had been configured to create a vpc and subnet automatically --- pkg/eks/components/CruEKS.vue | 6 ++++ pkg/eks/components/Networking.vue | 30 +++++++++++++++---- .../components/__tests__/Networking.test.ts | 22 +++++++++++++- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/pkg/eks/components/CruEKS.vue b/pkg/eks/components/CruEKS.vue index b06f51be75e..f8a06e5d1c0 100644 --- a/pkg/eks/components/CruEKS.vue +++ b/pkg/eks/components/CruEKS.vue @@ -307,6 +307,11 @@ export default defineComponent({ return this.value?.id || null; }, + // used to display VPC/subnet information in the networking tab for imported clusters and clusters with the 'create a vpc and subnets automatically' option selected + statusSubnets(): string[] { + return this.normanCluster?.eksStatus?.subnets || []; + }, + canManageMembers(): boolean { return canViewClusterMembershipEditor(this.$store); }, @@ -665,6 +670,7 @@ export default defineComponent({ :mode="mode" :region="config.region" :amazon-credential-secret="config.amazonCredentialSecret" + :status-subnets="statusSubnets" :rules="{subnets:fvGetAndReportPathRules('subnets')}" /> diff --git a/pkg/eks/components/Networking.vue b/pkg/eks/components/Networking.vue index 3d2466d2b12..0891a1752e5 100644 --- a/pkg/eks/components/Networking.vue +++ b/pkg/eks/components/Networking.vue @@ -1,6 +1,6 @@