[Dynamic Config] Fix bug when attempting dynamic config index creation

changelogs/fragments/8184.yml

@@ -0,0 +1,2 @@
+fix:
+- Fix bug when dynamic config index and alias are checked ([#8184](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8184))

src/core/server/config/service/config_store_client/opensearch_config_store.test.ts

@@ -5,12 +5,16 @@
 
 import { SearchResponse } from '../../../opensearch';
 import { opensearchClientMock } from '../../../opensearch/client/mocks';
-import { DYNAMIC_APP_CONFIG_ALIAS } from '../../utils/constants';
+import { DYNAMIC_APP_CONFIG_ALIAS, DYNAMIC_APP_CONFIG_INDEX_PREFIX } from '../../utils/constants';
 import { OpenSearchConfigStoreClient } from './opensearch_config_store_client';
 import { ConfigDocument } from './types';
 import _ from 'lodash';
 import { ConfigBlob } from '../../types';
-import { BulkOperationContainer } from '@opensearch-project/opensearch/api/types';
+import {
+  BulkOperationContainer,
+  CatIndicesResponse,
+  IndicesGetAliasResponse,
+} from '@opensearch-project/opensearch/api/types';
 import { getDynamicConfigIndexName } from '../../utils/utils';
 
 describe('OpenSearchConfigStoreClient', () => {
@@ -32,10 +36,12 @@ describe('OpenSearchConfigStoreClient', () => {
     isListConfig: boolean;
     configDocuments: ConfigDocument[];
     existsAliasResult: boolean;
+    getAliasIndicesResult: IndicesGetAliasResponse;
+    catIndicesResult: CatIndicesResponse;
   }
 
   /**
-   * Creates a new OpenSearch client mock complete with a mock for existsAlias() and search() results
+   * Creates a new OpenSearch client mock complete with a mock for existsAlias(), cat.indices(), and search() results
    *
    * @param param0
    * @returns
@@ -44,6 +50,8 @@ describe('OpenSearchConfigStoreClient', () => {
     isListConfig,
     configDocuments,
     existsAliasResult,
+    getAliasIndicesResult,
+    catIndicesResult,
   }: OpenSearchClientMockProps) => {
     const mockClient = opensearchClientMock.createOpenSearchClient();
 
@@ -53,6 +61,18 @@ describe('OpenSearchConfigStoreClient', () => {
       })
     );
 
+    mockClient.cat.indices.mockResolvedValue(
+      opensearchClientMock.createApiResponse<CatIndicesResponse>({
+        body: catIndicesResult,
+      })
+    );
+
+    mockClient.indices.getAlias.mockResolvedValue(
+      opensearchClientMock.createApiResponse<IndicesGetAliasResponse>({
+        body: getAliasIndicesResult,
+      })
+    );
+
     // @ts-expect-error
     mockClient.search.mockImplementation((request, options) => {
       // Filters out results when the request is for getting/bulk getting configs
@@ -83,6 +103,49 @@ describe('OpenSearchConfigStoreClient', () => {
     return mockClient;
   };
 
+  const noDynamicConfigIndexResults: CatIndicesResponse = [
+    {
+      index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_`,
+    },
+    {
+      index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_foo`,
+    },
+    {
+      index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_foo_2`,
+    },
+  ];
+
+  const oneDynamicConfigIndexResult: CatIndicesResponse = [
+    {
+      index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_1`,
+    },
+  ];
+
+  const multipleDynamicConfigIndexResults: CatIndicesResponse = [
+    {
+      index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_2`,
+    },
+    {
+      index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_4`,
+    },
+    {
+      index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_800`,
+    },
+  ];
+
+  const validAliasIndicesResponse: IndicesGetAliasResponse = {
+    [`${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_4`]: { aliases: { DYNAMIC_APP_CONFIG_ALIAS: {} } },
+  };
+
+  const multipleAliasIndicesResponse: IndicesGetAliasResponse = {
+    [`${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_4`]: { aliases: { DYNAMIC_APP_CONFIG_ALIAS: {} } },
+    [`${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_2`]: { aliases: { DYNAMIC_APP_CONFIG_ALIAS: {} } },
+  };
+
+  const invalidAliasIndicesResponse: IndicesGetAliasResponse = {
+    [`.some_random_index_8`]: { aliases: { DYNAMIC_APP_CONFIG_ALIAS: {} } },
+  };
+
   const configDocument: ConfigDocument = {
     config_name: 'some_config_name',
     config_blob: {
@@ -143,26 +206,88 @@ describe('OpenSearchConfigStoreClient', () => {
     it.each([
       {
         existsAliasResult: false,
+        catIndicesResult: noDynamicConfigIndexResults,
+        getAliasIndicesResult: validAliasIndicesResponse,
         numCreateCalls: 1,
+        numUpdateCalls: 0,
+        errorThrown: false,
+      },
+      {
+        existsAliasResult: false,
+        catIndicesResult: multipleDynamicConfigIndexResults,
+        getAliasIndicesResult: validAliasIndicesResponse,
+        numCreateCalls: 0,
+        numUpdateCalls: 1,
+        errorThrown: false,
+      },
+      {
+        existsAliasResult: false,
+        catIndicesResult: oneDynamicConfigIndexResult,
+        getAliasIndicesResult: validAliasIndicesResponse,
+        numCreateCalls: 0,
+        numUpdateCalls: 1,
+        errorThrown: false,
       },
       {
         existsAliasResult: true,
+        catIndicesResult: multipleDynamicConfigIndexResults,
+        getAliasIndicesResult: {},
         numCreateCalls: 0,
+        numUpdateCalls: 0,
+        errorThrown: true,
+      },
+      {
+        existsAliasResult: true,
+        catIndicesResult: multipleDynamicConfigIndexResults,
+        getAliasIndicesResult: multipleAliasIndicesResponse,
+        numCreateCalls: 0,
+        numUpdateCalls: 0,
+        errorThrown: true,
+      },
+      {
+        existsAliasResult: true,
+        catIndicesResult: multipleDynamicConfigIndexResults,
+        getAliasIndicesResult: invalidAliasIndicesResponse,
+        numCreateCalls: 0,
+        numUpdateCalls: 0,
+        errorThrown: true,
+      },
+      {
+        existsAliasResult: true,
+        catIndicesResult: multipleDynamicConfigIndexResults,
+        getAliasIndicesResult: validAliasIndicesResponse,
+        numCreateCalls: 0,
+        numUpdateCalls: 0,
+        errorThrown: false,
       },
     ])(
-      'should create config index $numCreateCalls times when existsAlias() is $existsAliasResult',
-      async ({ existsAliasResult, numCreateCalls }) => {
+      'should throw error should be $errorThrown, create() should be called $numCreateCalls times, and update() should be called $numUpdateCalls times',
+      async ({
+        existsAliasResult,
+        catIndicesResult,
+        getAliasIndicesResult,
+        numCreateCalls,
+        numUpdateCalls,
+        errorThrown,
+      }) => {
         const mockClient = createOpenSearchClientMock({
           isListConfig: false,
           configDocuments: [],
           existsAliasResult,
+          getAliasIndicesResult,
+          catIndicesResult,
         });
         const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
-        await configStoreClient.createDynamicConfigIndex();
+
+        if (errorThrown) {
+          expect(configStoreClient.createDynamicConfigIndex()).rejects.toThrowError();
+        } else {
+          await configStoreClient.createDynamicConfigIndex();
+        }
 
         expect(mockClient.indices.existsAlias).toBeCalled();
         expect(mockClient.indices.create).toBeCalledTimes(numCreateCalls);
-        expect(mockClient.indices.updateAliases).toBeCalledTimes(numCreateCalls);
+        expect(mockClient.indices.updateAliases).toBeCalledTimes(numUpdateCalls);
       }
     );
   });
@@ -175,6 +300,8 @@ describe('OpenSearchConfigStoreClient', () => {
         isListConfig: false,
         configDocuments: [configDocument],
         existsAliasResult: false,
+        catIndicesResult: oneDynamicConfigIndexResult,
+        getAliasIndicesResult: validAliasIndicesResponse,
       });
       const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
 
@@ -210,6 +337,8 @@ describe('OpenSearchConfigStoreClient', () => {
         isListConfig: false,
         configDocuments,
         existsAliasResult: false,
+        catIndicesResult: oneDynamicConfigIndexResult,
+        getAliasIndicesResult: validAliasIndicesResponse,
       });
       const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
 
@@ -274,6 +403,8 @@ describe('OpenSearchConfigStoreClient', () => {
           isListConfig: true,
           configDocuments: allConfigDocuments,
           existsAliasResult: false,
+          catIndicesResult: oneDynamicConfigIndexResult,
+          getAliasIndicesResult: validAliasIndicesResponse,
         });
         const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
         const actualMap = await configStoreClient.listConfigs();
@@ -342,6 +473,8 @@ describe('OpenSearchConfigStoreClient', () => {
           isListConfig: false,
           configDocuments: newConfigDocuments,
           existsAliasResult: false,
+          catIndicesResult: oneDynamicConfigIndexResult,
+          getAliasIndicesResult: validAliasIndicesResponse,
         });
         const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
         await configStoreClient.createConfig({
@@ -540,6 +673,8 @@ describe('OpenSearchConfigStoreClient', () => {
           isListConfig: false,
           configDocuments: existingConfigs,
           existsAliasResult: false,
+          catIndicesResult: oneDynamicConfigIndexResult,
+          getAliasIndicesResult: validAliasIndicesResponse,
         });
         const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
         await configStoreClient.bulkCreateConfigs({
@@ -565,6 +700,8 @@ describe('OpenSearchConfigStoreClient', () => {
         isListConfig: false,
         configDocuments: [],
         existsAliasResult: false,
+        catIndicesResult: oneDynamicConfigIndexResult,
+        getAliasIndicesResult: validAliasIndicesResponse,
       });
       const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
       await configStoreClient.deleteConfig({ name: 'some_config_name' });
@@ -604,6 +741,8 @@ describe('OpenSearchConfigStoreClient', () => {
         isListConfig: false,
         configDocuments: [],
         existsAliasResult: false,
+        catIndicesResult: oneDynamicConfigIndexResult,
+        getAliasIndicesResult: validAliasIndicesResponse,
       });
       const configStoreClient = new OpenSearchConfigStoreClient(mockClient);
       await configStoreClient.bulkDeleteConfigs({

src/core/server/config/service/config_store_client/opensearch_config_store_client.ts

@@ -20,9 +20,15 @@
 } from '../../types';
 import {
   DYNAMIC_APP_CONFIG_ALIAS,
+  DYNAMIC_APP_CONFIG_INDEX_PREFIX,
   DYNAMIC_APP_CONFIG_MAX_RESULT_SIZE,
 } from '../../utils/constants';
-import { pathToString, getDynamicConfigIndexName } from '../../utils/utils';
+import {
+  pathToString,
+  getDynamicConfigIndexName,
+  isDynamicConfigIndex,
+  extractVersionFromDynamicConfigIndex,
+} from '../../utils/utils';
 import { ConfigDocument } from './types';
 
 interface ConfigMapEntry {
@@ -48,25 +54,52 @@
    * TODO Add migration logic
    */
   public async createDynamicConfigIndex() {
-    const existsResponse = await this.#openSearchClient.indices.existsAlias({
+    const existsAliasResponse = await this.#openSearchClient.indices.existsAlias({
       name: DYNAMIC_APP_CONFIG_ALIAS,
     });
-    if (!existsResponse.body) {
-      await this.#openSearchClient.indices.create({
-        index: getDynamicConfigIndexName(1),
-      });
-      await this.#openSearchClient.indices.updateAliases({
-        body: {
-          actions: [
-            {
-              add: {
-                index: getDynamicConfigIndexName(1),
-                alias: DYNAMIC_APP_CONFIG_ALIAS,
+
+    if (!existsAliasResponse.body) {
+      const latestVersion = await this.searchLatestConfigIndex();
+      if (latestVersion < 1) {
+        await this.#openSearchClient.indices.create({
+          index: getDynamicConfigIndexName(1),
+          body: {
+            aliases: { [DYNAMIC_APP_CONFIG_ALIAS]: {} },
+          },
+        });
+      } else {
+        await this.#openSearchClient.indices.updateAliases({
+          body: {
+            actions: [
+              {
+                add: {
+                  index: getDynamicConfigIndexName(latestVersion),
+                  alias: DYNAMIC_APP_CONFIG_ALIAS,
+                },
               },
-            },
-          ],
-        },
+            ],
+          },
+        });
+      }
+    } else {
+      const results = await this.#openSearchClient.indices.getAlias({
+        name: DYNAMIC_APP_CONFIG_ALIAS,
       });
+
+      const indices = Object.keys(results.body);
+      if (indices.length !== 1) {
+        throw new Error(
+          `Alias ${DYNAMIC_APP_CONFIG_ALIAS} is pointing to 0 or multiple indices. Please remove the alias(es) and restart the server`
+        );
+      }
+      const numNonDynamicConfigIndices = indices.filter((index) => !isDynamicConfigIndex(index))
+        .length;
+
+      if (numNonDynamicConfigIndices > 0) {
+        throw new Error(
+          `Alias ${DYNAMIC_APP_CONFIG_ALIAS} is pointing to a non dynamic config index. Please remove the alias and restart the server`
+        );
+      }
     }
   }
 
@@ -342,4 +375,34 @@
       },
     });
   }
+
+  /**
+   * Finds the most updated dynamic config index
+   *
+   * @returns the latest version number or 0 if not found
+   */
+  private async searchLatestConfigIndex(): Promise<number> {
+    const configIndices = await this.#openSearchClient.cat.indices({
+      index: `${DYNAMIC_APP_CONFIG_INDEX_PREFIX}_*`,
+      format: 'json',
+    });
+
+    if (configIndices.body.length < 1) {
+      return 0;
+    }
+
+    const validIndices = configIndices.body
+      .map((hit) => hit.index?.toString())
+      .filter((index) => index && isDynamicConfigIndex(index));
+
+    return validIndices.length === 0
+      ? 0
+      : validIndices
+          .map((configIndex) => {
+            return configIndex ? extractVersionFromDynamicConfigIndex(configIndex) : 0;
+          })
+          .reduce((currentMax, currentNum) => {
+            return currentMax && currentNum && currentMax > currentNum ? currentMax : currentNum;
+          });
+  }
 }