Fixed: OXUIB-1632 Multiple ui-middleware-nodes might end up in different configurations

Root cause: configuration was only reloaded on those nodes, that did the version update. The other nodes might not notice that. Solution: On propagated version changes, the nodes always check for config changes

Fixed: OXUIB-1632 Multiple ui-middleware-nodes might end up in different configurations
Root cause: configuration was only reloaded on those nodes, that did the version update. The other nodes might not notice that. Solution: On propagated version changes, the nodes always check for config changes
1ee37611 · richard.petersen · 49465a32 · 1ee37611 · 1ee37611 · 1ee37611
Commit 1ee37611 authored 2 years ago by richard.petersen
--- a/integration/config_test.js
+++ b/integration/config_test.js
+import request from 'supertest'
+import { expect } from 'chai'
+import { generateSimpleViteManifest, mockApp, mockConfig, mockFetch } from '../spec/util.js'
+import { Response } from 'node-fetch'
+import { client, closeQueue, getQueues, pubClient } from '../src/redis.js'
+import * as td from 'testdouble'
+import { getRedisKey } from '../src/util.js'
+
+describe('Configuration', function () {
+  let app
+  let config
+
+  beforeEach(async function () {
+    // need to set the redis-prefix. Otherwise, the bull workers will interfere
+    process.env.REDIS_PREFIX = Math.random()
+    await client.flushdb()
+    mockConfig(config = { urls: ['http://ui-server/'] })
+    mockFetch({
+      'http://ui-server': {
+        '/manifest.json': generateSimpleViteManifest({
+          'index.html': {}
+        }),
+        '/index.html': () => new Response('<html><head></head><body>it\'s me</body></html>', { headers: { 'content-type': 'text/html' } }),
+        '/meta.json': td.when(td.func()(td.matchers.anything())).thenReturn(
+          new Response(JSON.stringify({ commitSha: '1' }), { headers: { 'Content-Type': 'application/json' } }),
+          new Response(JSON.stringify({ commitSha: '2' }), { headers: { 'Content-Type': 'application/json' } })
+        )
+      }
+    })
+    app = await mockApp()
+  })
+
+  afterEach(async function () {
+    td.reset()
+    for (const queue of getQueues()) {
+      await closeQueue(queue.name)
+    }
+    // reset, after the queues were removed
+    process.env.REDIS_PREFIX = 'ui-middleware'
+  })
+
+  it('updates the configuration when updated on a different node', async function () {
+    // need to do this with dynamic import such that the mocked config is used
+    await import('../src/create-queues.js').then(({ default: createQueues }) => createQueues())
+
+    const response = await request(app).get('/meta')
+    expect(response.body).to.have.length(2)
+
+    config.urls = []
+    pubClient.publish(getRedisKey({ name: 'updateLatestVersion' }), '1234')
+    await new Promise(resolve => setTimeout(resolve, 200))
+
+    const response2 = await request(app).get('/meta')
+    expect(response2.body).to.have.length(1)
+  })
+})
--- a/spec/meta_test.js
+++ b/spec/meta_test.js
@@ -7,10 +7,11 @@ import RedisMock from 'ioredis-mock'

 describe('Responses contain custom headers', function () {
  let fetchConfig
+  let config
  let app

-  before(async function () {
-    mockConfig({ urls: ['http://ui-server/'] })
+  beforeEach(async function () {
+    mockConfig(config = { urls: ['http://ui-server/'] })
    mockRedis()
    mockFetch(fetchConfig = {
      'http://ui-server': {
@@ -24,15 +25,12 @@ describe('Responses contain custom headers', function () {
    app = await mockApp()
  })

-  after(function () {
-    td.reset()
-  })
-
  afterEach(async function () {
    await new RedisMock().flushdb()
    delete process.env.APP_VERSION
    delete process.env.BUILD_TIMESTAMP
    delete process.env.CI_COMMIT_SHA
+    td.reset()
  })

  it('has own metadata', async function () {
@@ -60,18 +58,22 @@ describe('Responses contain custom headers', function () {
    })
  })

-  describe('without service avaible', function () {
-    let prevConfig
+  it('has updated metadata if config is updated', async function () {
+    const response = await request(app).get('/meta')
+    expect(response.body).to.have.length(2)

+    config.urls = []
+    await import('../src/version.js').then(({ updateVersionProcessor }) => updateVersionProcessor())
+
+    const response2 = await request(app).get('/meta')
+    expect(response2.body).to.have.length(1)
+  })
+
+  describe('without service avaible', function () {
    beforeEach(function () {
-      prevConfig = fetchConfig['http://ui-server']
      delete fetchConfig['http://ui-server']
    })

-    afterEach(function () {
-      fetchConfig['http://ui-server'] = prevConfig
-    })
-
    it('does not have metadata from ui service when unavailable', async function () {
      await import('../src/cache.js').then(({ clear }) => clear())
      const response = await request(app).get('/meta')
@@ -84,8 +86,6 @@ describe('Responses contain custom headers', function () {
  })

  describe('without redis disabled', function () {
-    let prevConfig
-
    beforeEach(async function () {
      td.reset()
      mockConfig({ urls: ['http://ui-server/'] })
@@ -101,10 +101,6 @@ describe('Responses contain custom headers', function () {
      app = await mockApp()
    })

-    afterEach(function () {
-      fetchConfig['http://ui-server'] = prevConfig
-    })
-
    it('has metadata', async function () {
      const response = await request(app).get('/meta')
      expect(response.statusCode).to.equal(200)

--- a/spec/util.js
+++ b/spec/util.js
@@ -16,10 +16,10 @@ export function generateSimpleViteManifest (mapping) {
  return viteManifest
 }

-export function mockConfig ({ urls = [] } = {}) {
+export function mockConfig (obj = {}) {
  td.replaceEsm('fs/promises', {}, {
    readFile () {
-      return `baseUrls:\n${urls.map(u => `  - ${u}`).join('\n')}`
+      return `baseUrls:\n${obj.urls.map(u => `  - ${u}`).join('\n')}`
    }
  })
 }

--- a/src/files.js
+++ b/src/files.js
@@ -37,8 +37,6 @@ export async function fetchFileWithHeadersFromBaseUrl (path, baseUrl, version) {
 }

 export async function fetchFileWithHeaders ({ path, version }) {
-  if (config.urls.length === 0) await config.load()
-
  const viteManifests = await getViteManifests({ version })
  const module = viteManifests[path.substr(1)]
  if (module?.meta?.baseUrl) {

--- a/src/manifests.js
+++ b/src/manifests.js
@@ -8,7 +8,6 @@ export async function getViteManifests ({ version }) {
  const manifests = await cache.get(getRedisKey({ version, name: 'viteManifests' }))
  if (manifests) return JSON.parse(manifests)

-  await config.load()
  // vite manifests contains a set of objects with the vite-manifests
  // from the corresponding registered services
  const viteManifests = await Promise.all(config.urls.map(async baseUrl => {

--- a/src/meta.js
+++ b/src/meta.js
@@ -7,7 +7,6 @@ export async function getMergedMetadata ({ version }) {
  const metadata = await cache.get(getRedisKey({ version, name: 'mergedMetadata' }))
  if (metadata) return JSON.parse(metadata)

-  await config.load()
  const newMetadata = await Promise.all(config.urls.map(async url => {
    const { origin } = new URL(url)
    try {

--- a/src/routes/health.js
+++ b/src/routes/health.js
@@ -3,6 +3,7 @@ import health from '@cloudnative/health-connect'
 import * as redis from '../redis.js'
 import { getLatestVersion } from '../version.js'
 import { once } from '../util.js'
+import { config } from '../config.js'

 const router = Router()
 const healthCheck = new health.HealthChecker()
@@ -12,10 +13,12 @@ if (redis.isEnabled()) {
  healthCheck.registerReadinessCheck(redisReady)
 }

-const startupCheck = new health.StartupCheck('check latest version', once(async function () {
+const checkLatestVersion = new health.StartupCheck('check latest version', once(async function () {
+  // config is required before the first version check
+  await config.load()
  await getLatestVersion()
 }))
-healthCheck.registerStartupCheck(startupCheck)
+healthCheck.registerStartupCheck(checkLatestVersion)

 router.use('/live', health.LivenessEndpoint(healthCheck))
 router.use('/ready', health.ReadinessEndpoint(healthCheck))

--- a/src/version.js
+++ b/src/version.js
@@ -7,7 +7,6 @@ import * as redis from './redis.js'
 let latestVersion

 export const fetchLatestVersion = async () => {
-  await config.load()
  const infos = await Promise.all(config.urls.map(async baseUrl => {
    try {
      const response = await fetch(new URL('meta.json', baseUrl))
@@ -53,13 +52,16 @@ export function registerLatestVersionListener (client) {
  if (redis.isEnabled()) {
    const key = getRedisKey({ name: 'updateLatestVersion' })
    client.subscribe(key, (errs, count) => logger.info(`Subscribed to ${key}.`))
-    client.on('message', (channel, message) => {
-      if (channel === key) latestVersion = message
+    client.on('message', async (channel, message) => {
+      if (channel !== key) return
+      await config.load()
+      latestVersion = message
    })
  }
 }

 export async function updateVersionProcessor () {
+  await config.load()
  const [storedVersion, fetchedVersion] = await Promise.all([
    getLatestVersion(),
    fetchLatestVersion()
@@ -69,5 +71,5 @@ export async function updateVersionProcessor () {
    redis.pubClient.publish(getRedisKey({ name: 'updateLatestVersion' }), fetchedVersion)
    await redis.client.set(getRedisKey({ name: 'latestVersion' }), fetchedVersion)
  }
-  return fetchedVersion
+  return (latestVersion = fetchedVersion)
 }