From 1ee376117a8468a267f2f2297694fc4d9003458f Mon Sep 17 00:00:00 2001
From: Richard Petersen <richard.petersen@open-xchange.com>
Date: Mon, 9 May 2022 16:24:36 +0200
Subject: [PATCH] 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
---
integration/config_test.js | 56 ++++++++++++++++++++++++++++++++++++++
spec/meta_test.js | 34 ++++++++++-------------
spec/util.js | 4 +--
src/files.js | 2 --
src/manifests.js | 1 -
src/meta.js | 1 -
src/routes/health.js | 7 +++--
src/version.js | 10 ++++---
8 files changed, 84 insertions(+), 31 deletions(-)
create mode 100644 integration/config_test.js
diff --git a/integration/config_test.js b/integration/config_test.js
new file mode 100644
index 0000000..b5f6d27
--- /dev/null
+++ b/integration/config_test.js
@@ -0,0 +1,56 @@
+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)
+ })
+})
diff --git a/spec/meta_test.js b/spec/meta_test.js
index 4ff9221..cbf94fe 100644
--- 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)
diff --git a/spec/util.js b/spec/util.js
index 4667576..a7419c6 100644
--- 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')}`
}
})
}
diff --git a/src/files.js b/src/files.js
index 3cee604..d193ffc 100644
--- 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) {
diff --git a/src/manifests.js b/src/manifests.js
index 87ca688..f70c176 100644
--- 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 => {
diff --git a/src/meta.js b/src/meta.js
index 3bc63ab..06a56c7 100644
--- 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 {
diff --git a/src/routes/health.js b/src/routes/health.js
index edd0baf..9e64e08 100644
--- 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))
diff --git a/src/version.js b/src/version.js
index 652877c..0496d32 100644
--- 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)
}
--
GitLab