From 2c7eb4e3782845b89f7b59aeaa8b143937f0c500 Mon Sep 17 00:00:00 2001 From: Vijay Srinivas Date: Tue, 15 Aug 2023 12:12:26 +0530 Subject: [PATCH 1/6] fix for #18406 , non presence of consul-version meta --- .../consul-ui/app/serializers/application.js | 42 ++++++++++--------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/ui/packages/consul-ui/app/serializers/application.js b/ui/packages/consul-ui/app/serializers/application.js index 6230554f7caa..7494bde82fab 100644 --- a/ui/packages/consul-ui/app/serializers/application.js +++ b/ui/packages/consul-ui/app/serializers/application.js @@ -227,7 +227,7 @@ export default class ApplicationSerializer extends Serializer { // create a Set and add version with only major.minor : ex-1.24.6 as 1.24 let versionSet = new Set(); payload.forEach(function (item) { - if (item.Meta && item.Meta['consul-version'] !== '') { + if (item.Meta && item.Meta['consul-version'] && item.Meta['consul-version'] !== '') { const split = item.Meta['consul-version'].split('.'); versionSet.add(split[0] + '.' + split[1]); } @@ -235,28 +235,30 @@ export default class ApplicationSerializer extends Serializer { const versionArray = Array.from(versionSet); - // Sort the array in descending order using a custom comparison function - versionArray.sort((a, b) => { - // Split the versions into arrays of numbers - const versionA = a.split('.').map((part) => { - const number = Number(part); - return isNaN(number) ? 0 : number; - }); - const versionB = b.split('.').map((part) => { - const number = Number(part); - return isNaN(number) ? 0 : number; - }); + if (versionArray.length > 0) { + // Sort the array in descending order using a custom comparison function + versionArray.sort((a, b) => { + // Split the versions into arrays of numbers + const versionA = a.split('.').map((part) => { + const number = Number(part); + return isNaN(number) ? 0 : number; + }); + const versionB = b.split('.').map((part) => { + const number = Number(part); + return isNaN(number) ? 0 : number; + }); - const minLength = Math.min(versionA.length, versionB.length); + const minLength = Math.min(versionA.length, versionB.length); - // start with comparing major version num, if equal then compare minor - for (let i = 0; i < minLength; i++) { - if (versionA[i] !== versionB[i]) { - return versionB[i] - versionA[i]; + // start with comparing major version num, if equal then compare minor + for (let i = 0; i < minLength; i++) { + if (versionA[i] !== versionB[i]) { + return versionB[i] - versionA[i]; + } } - } - return versionB.length - versionA.length; - }); + return versionB.length - versionA.length; + }); + } return versionArray; //sorted array } From 3ebde50229c2b959e501ad1f8e3b4fddcecde645 Mon Sep 17 00:00:00 2001 From: Vijay Srinivas Date: Fri, 18 Aug 2023 15:41:34 +0530 Subject: [PATCH 2/6] removed redundant checks --- .../consul-ui/app/serializers/application.js | 42 +++++++++---------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/ui/packages/consul-ui/app/serializers/application.js b/ui/packages/consul-ui/app/serializers/application.js index 7494bde82fab..b7b1fa4847fe 100644 --- a/ui/packages/consul-ui/app/serializers/application.js +++ b/ui/packages/consul-ui/app/serializers/application.js @@ -227,7 +227,7 @@ export default class ApplicationSerializer extends Serializer { // create a Set and add version with only major.minor : ex-1.24.6 as 1.24 let versionSet = new Set(); payload.forEach(function (item) { - if (item.Meta && item.Meta['consul-version'] && item.Meta['consul-version'] !== '') { + if (item.Meta && item.Meta['consul-version']) { const split = item.Meta['consul-version'].split('.'); versionSet.add(split[0] + '.' + split[1]); } @@ -235,30 +235,28 @@ export default class ApplicationSerializer extends Serializer { const versionArray = Array.from(versionSet); - if (versionArray.length > 0) { - // Sort the array in descending order using a custom comparison function - versionArray.sort((a, b) => { - // Split the versions into arrays of numbers - const versionA = a.split('.').map((part) => { - const number = Number(part); - return isNaN(number) ? 0 : number; - }); - const versionB = b.split('.').map((part) => { - const number = Number(part); - return isNaN(number) ? 0 : number; - }); + // Sort the array in descending order using a custom comparison function + versionArray.sort((a, b) => { + // Split the versions into arrays of numbers + const versionA = a.split('.').map((part) => { + const number = Number(part); + return isNaN(number) ? 0 : number; + }); + const versionB = b.split('.').map((part) => { + const number = Number(part); + return isNaN(number) ? 0 : number; + }); - const minLength = Math.min(versionA.length, versionB.length); + const minLength = Math.min(versionA.length, versionB.length); - // start with comparing major version num, if equal then compare minor - for (let i = 0; i < minLength; i++) { - if (versionA[i] !== versionB[i]) { - return versionB[i] - versionA[i]; - } + // start with comparing major version num, if equal then compare minor + for (let i = 0; i < minLength; i++) { + if (versionA[i] !== versionB[i]) { + return versionB[i] - versionA[i]; } - return versionB.length - versionA.length; - }); - } + } + return versionB.length - versionA.length; + }); return versionArray; //sorted array } From 01142e8d907cc17d1de175667cb0df4c8f1d4dad Mon Sep 17 00:00:00 2001 From: Vijay Srinivas Date: Fri, 18 Aug 2023 15:49:01 +0530 Subject: [PATCH 3/6] updated mock-api to mimic api response for synthetic nodes --- ui/packages/consul-ui/mock-api/v1/internal/ui/nodes | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/packages/consul-ui/mock-api/v1/internal/ui/nodes b/ui/packages/consul-ui/mock-api/v1/internal/ui/nodes index de64b0bb8eb0..5cabfb42460d 100644 --- a/ui/packages/consul-ui/mock-api/v1/internal/ui/nodes +++ b/ui/packages/consul-ui/mock-api/v1/internal/ui/nodes @@ -13,6 +13,7 @@ function(item, i) { const peerNameString = i === 0 ? '"PeerName": "billing",' : '"PeerName": "",' + const isSyntheticNode = env('CONSUL_AGENTLESS_ENABLED') ? fake.helpers.randomize([true, false, false, false]) : false return ` { "ID":"${fake.random.uuid()}", @@ -25,8 +26,10 @@ }, "Meta": { "consul-network-segment":"", + ${isSyntheticNode ? `` : ` "consul-version": "${env('CONSUL_VERSION') ? fake.helpers.randomize([env('CONSUL_VERSION'),"1.10.4","1.15.2", "1.17.8","1.7.2","1.12.4", "1.17.2","1.0.9","2.0.2"]) : fake.helpers.randomize(["1.10.4","1.15.2", "1.17.8","1.7.2","1.12.4", "1.17.2","1.0.9","2.0.2"]) }", - "synthetic-node": ${env('CONSUL_AGENTLESS_ENABLED') ? fake.helpers.randomize([true, false, false, false]) : false} + `} + "synthetic-node": ${isSyntheticNode} }, "Services":[ ${ From 24ba5e93728ea5b77efe6d70ea5105e72873d03f Mon Sep 17 00:00:00 2001 From: Vijay Srinivas Date: Fri, 18 Aug 2023 21:56:38 +0530 Subject: [PATCH 4/6] added test to test getDistinctConsulVersions method with synthetic-node case --- .../unit/serializers/application-test.js | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/ui/packages/consul-ui/tests/unit/serializers/application-test.js b/ui/packages/consul-ui/tests/unit/serializers/application-test.js index d50bf81d3fae..542e313b667b 100644 --- a/ui/packages/consul-ui/tests/unit/serializers/application-test.js +++ b/ui/packages/consul-ui/tests/unit/serializers/application-test.js @@ -6,6 +6,7 @@ import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; import { HEADERS_SYMBOL as META } from 'consul-ui/utils/http/consul'; +import Node from 'consul-ui/models/node'; module('Unit | Serializer | application', function (hooks) { setupTest(hooks); @@ -122,4 +123,94 @@ module('Unit | Serializer | application', function (hooks) { assert.deepEqual(actual, expected); // assert.ok(adapter.uidForURL.calledTwice); }); + test('normalizeResponse for Node returns the expected meta in response', function (assert) { + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('application'); + serializer.timestamp = () => 1234567890; //mocks actual timestamp + serializer.primaryKey = 'primary-key-name'; + serializer.slugKey = 'Name'; + serializer.fingerprint = function (primary, slug, foreignValue) { + return function (item) { + return { + ...item, + ...{ + Datacenter: foreignValue, + [primary]: item[slug], + }, + }; + }; + }; + + const payload = [ + { + Node: 'node-0', + Meta: { 'consul-version': '1.7.2' }, + uid: '1234', + SyncTime: 1234567890, + }, + { + Node: 'node-1', + Meta: { 'consul-version': '1.18.0' }, + uid: '1235', + SyncTime: 1234567891, + }, + // synthetic-node with consul-version meta + { + Node: 'node-2', + Meta: { 'synthetic-node': true }, + uid: '1236', + SyncTime: 1234567891, + }, + ]; + + const expected = { + data: [ + { + attributes: { + Node: 'node-0', + Meta: { 'consul-version': '1.7.2' }, + SyncTime: 1234567890, + uid: '1234', + }, + id: '1234', + relationships: {}, + type: 'node', + }, + { + attributes: { + Node: 'node-1', + Meta: { 'consul-version': '1.18.0' }, + SyncTime: 1234567890, + uid: '1235', + }, + id: '1235', + relationships: {}, + type: 'node', + }, + { + attributes: { + Node: 'node-2', + Meta: { 'synthetic-node': true }, + SyncTime: 1234567890, + uid: '1236', + }, + id: '1236', + relationships: {}, + type: 'node', + }, + ], + included: [], + meta: { + versions: ['1.18', '1.7'], //expect distinct major versions sorted + cacheControl: undefined, + cursor: undefined, + date: 1234567890, + dc: undefined, + nspace: undefined, + partition: undefined, + }, + }; + const actual = serializer.normalizeResponse(store, Node, payload, '2', 'query'); + assert.deepEqual(actual, expected); + }); }); From 4cfb8e0b59bcd522f414ccb3a2899dffba7ada35 Mon Sep 17 00:00:00 2001 From: Vijay Srinivas Date: Fri, 18 Aug 2023 22:04:47 +0530 Subject: [PATCH 5/6] updated typo in comments --- .../consul-ui/tests/unit/serializers/application-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/packages/consul-ui/tests/unit/serializers/application-test.js b/ui/packages/consul-ui/tests/unit/serializers/application-test.js index 542e313b667b..180ae7617240 100644 --- a/ui/packages/consul-ui/tests/unit/serializers/application-test.js +++ b/ui/packages/consul-ui/tests/unit/serializers/application-test.js @@ -154,7 +154,7 @@ module('Unit | Serializer | application', function (hooks) { uid: '1235', SyncTime: 1234567891, }, - // synthetic-node with consul-version meta + // synthetic-node without consul-version meta { Node: 'node-2', Meta: { 'synthetic-node': true }, From b3f3d9dc059403209d7b83898c78dae7a6613c75 Mon Sep 17 00:00:00 2001 From: Vijay Srinivas Date: Sat, 19 Aug 2023 08:35:45 +0530 Subject: [PATCH 6/6] added change log --- .changelog/18464.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/18464.txt diff --git a/.changelog/18464.txt b/.changelog/18464.txt new file mode 100644 index 000000000000..b0c15cf0d552 --- /dev/null +++ b/.changelog/18464.txt @@ -0,0 +1,3 @@ +```release-note:bug +UI : Nodes list view was breaking for synthetic-nodes. Fix handles non existence of consul-version meta for node. +```