Skip to content

Commit

Permalink
[Session Index] Attach alias to index when index name changes (elasti…
Browse files Browse the repository at this point in the history
…c#210176)

Closes elastic#210179

## Summary

While attempting to attach an alias to the session index, we were using
`.kibana_security_session_1` as the index name. However, the session
index, if upgraded using the Upgrade assistant gets renamed to
`.kibana_security_session_1-reindexed-for-9` and
`.kibana_security_session_1` is set as an alias pointing to this index.
When we try to reattach the alias using this as the index name, ES
throws an error. This doesn't affect Kibana functionality but it
increases the number of errors thrown in the logs.

**This PR corrects this issue by attaching the alias to the index only
when alias isn't already present. We now only assign the alias if not
present - and not during creation of the index as it is created with the
alias in it's settings.**

### Release note
Fixes the assignment of the Session index alias by only attaching it if
not already present.

## How to test

To see the error in the logs, you'll need at least 7.x and 8.x checked
out locally. Once done, run bootstrap.

#### Step 1: On 7.17
- Start ES with `yarn es snapshot --license trial -E
path.data=/tmp/esdata`
- Start Kibana and login with elastic user
- You can check the contents of Kibana session index:

```
GET .kibana_security_session_1/_search
{
  "query": {
    "match_all": {}
  }
}
```
Should return a single document
- You can now shut kibana and ES


#### Step 2: on 8.x
- Make a backup of the esdata above `cp -r /tmp/esdata /tmp/esdatabkp`
- Start ES as above `yarn es snapshot --license trial -E
path.data=/tmp/esdata`
- Start kibana and login
- Navigate to Upgrade assistant. You should see at least 2 System
indices that require migration (Security and Kibana)
- Start the migrate index process (Step 2 in the UA interface)
- Once done, trigger a local restart of kibana either by restarting
using the start script or just triggering a file save on any file in
your IDE
- You should start seeing the error described above in 100ms increments
till it reaches 10000ms and then it's every 10 seconds
- Navigate to Dev tools and run
```
GET .kibana_security_session/_alias
```
You should see the index as 
```
.kibana_security_session_reindexed-for-9 {
    aliases: {
        // aliases of the index including kibana_security_session_1
    }
}
```

#### Verify the fix
To verify, we have a couple of options - either clone the PR and go
through the same steps as Step 2 above but for 9.0. The easier option is
replace the code of function `attachAliasToIndex` in `session_index.ts`
in 8.x with the changes in this PR. This should restart your kibana
server and you will no longer see the error in the logs.


### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
SiddharthMantri and elasticmachine authored Feb 12, 2025
1 parent 0897d08 commit 45d9fa0
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,6 @@ describe('Session index', () => {

expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
index: indexName,
name: aliasName,
});
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
});

Expand All @@ -114,13 +110,10 @@ describe('Session index', () => {

expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledWith({ index: aliasName });
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.putAlias).not.toHaveBeenCalled();

expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
index: indexName,
name: aliasName,
});
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
});

Expand All @@ -139,10 +132,6 @@ describe('Session index', () => {

expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
index: indexName,
name: aliasName,
});
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
});

Expand All @@ -156,14 +145,46 @@ describe('Session index', () => {
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledWith({ index: aliasName });
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledWith({ index: indexName });
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledTimes(2);
expect(mockElasticsearchClient.indices.putAlias).not.toHaveBeenCalled();

expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putIndexTemplate).not.toHaveBeenCalled();

expect(mockElasticsearchClient.indices.create).toHaveBeenCalled();
});

it('attaches alias if no alias present', async () => {
mockElasticsearchClient.indices.existsTemplate.mockResponse(false);
mockElasticsearchClient.indices.existsIndexTemplate.mockResponse(true);
mockElasticsearchClient.indices.exists.mockImplementation(
async ({ index }) => index === indexName
);
mockElasticsearchClient.indices.existsAlias.mockResponse(false);

await sessionIndex.initialize();

expect(mockElasticsearchClient.indices.existsAlias).toHaveBeenCalledWith({ name: aliasName });
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
index: indexName,
name: aliasName,
});
expect(mockElasticsearchClient.indices.create).toHaveBeenCalled();

expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
});

it('does not attach alias if alias present', async () => {
mockElasticsearchClient.indices.existsTemplate.mockResponse(false);
mockElasticsearchClient.indices.existsIndexTemplate.mockResponse(true);
mockElasticsearchClient.indices.exists.mockResponse(true);
mockElasticsearchClient.indices.existsAlias.mockResponse(true);

await sessionIndex.initialize();

expect(mockElasticsearchClient.indices.existsAlias).toHaveBeenCalledWith({ name: aliasName });
expect(mockElasticsearchClient.indices.putAlias).not.toHaveBeenCalled();

expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
});

it('does not delete legacy index template if the legacy template API is not available (410)', async () => {
Expand All @@ -182,7 +203,6 @@ describe('Session index', () => {

expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.getMapping).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putMapping).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.create).toHaveBeenCalledWith(
Expand All @@ -206,7 +226,6 @@ describe('Session index', () => {

expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.getMapping).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putMapping).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.create).toHaveBeenCalledWith(
Expand All @@ -228,7 +247,6 @@ describe('Session index', () => {
expect(mockElasticsearchClient.indices.create).toHaveBeenCalledWith(
getSessionIndexSettings({ indexName, aliasName })
);
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);
});

it('deletes legacy & modern index templates if needed and creates index if it does not exist', async () => {
Expand All @@ -248,7 +266,6 @@ describe('Session index', () => {
expect(mockElasticsearchClient.indices.create).toHaveBeenCalledWith(
getSessionIndexSettings({ indexName, aliasName })
);
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);
});

it('deletes modern index template if needed and creates index if it does not exist', async () => {
Expand All @@ -263,7 +280,7 @@ describe('Session index', () => {
expect(mockElasticsearchClient.indices.deleteIndexTemplate).toHaveBeenCalledWith({
name: indexTemplateName,
});
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);

expect(mockElasticsearchClient.indices.create).toHaveBeenCalledWith(
getSessionIndexSettings({ indexName, aliasName })
);
Expand All @@ -281,11 +298,6 @@ describe('Session index', () => {
expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();

expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
index: indexName,
name: aliasName,
});
});

it('updates mappings for existing index without version in the meta', async () => {
Expand All @@ -306,11 +318,6 @@ describe('Session index', () => {
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();

expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
index: indexName,
name: aliasName,
});

expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledWith({ index: aliasName });
expect(mockElasticsearchClient.indices.putMapping).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -338,11 +345,6 @@ describe('Session index', () => {
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();

expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
index: indexName,
name: aliasName,
});

expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledWith({ index: aliasName });
expect(mockElasticsearchClient.indices.putMapping).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -370,11 +372,6 @@ describe('Session index', () => {
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();

expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
index: indexName,
name: aliasName,
});

expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledWith({ index: aliasName });
expect(mockElasticsearchClient.indices.putMapping).not.toHaveBeenCalled();
Expand All @@ -390,7 +387,6 @@ describe('Session index', () => {
assertExistenceChecksPerformed();
expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.getMapping).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putMapping).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.create).toHaveBeenCalledWith(
Expand Down Expand Up @@ -527,7 +523,6 @@ describe('Session index', () => {

expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledTimes(2);
expect(mockElasticsearchClient.indices.create).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); // since we attempted to delete sessions, we still refresh the index
Expand Down Expand Up @@ -1510,7 +1505,6 @@ describe('Session index', () => {

expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledTimes(2);
expect(mockElasticsearchClient.indices.create).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);

expect(mockElasticsearchClient.create).toHaveBeenCalledTimes(2);
expect(mockElasticsearchClient.create).toHaveBeenNthCalledWith(
Expand Down Expand Up @@ -1567,7 +1561,6 @@ describe('Session index', () => {

expect(mockElasticsearchClient.indices.exists).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
expect(mockElasticsearchClient.indices.putAlias).not.toHaveBeenCalled();

expect(mockElasticsearchClient.create).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.create).toHaveBeenCalledWith(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -681,17 +681,25 @@ export class SessionIndex {
}
}

await this.attachAliasToIndex();

return;
}

const isIndexNameAlias = await this.options.elasticsearchClient.indices.existsAlias({
name: this.aliasName,
});

if (!isIndexNameAlias) {
this.options.logger.debug(
'Session index already exists with no alias. Attaching alias to the index.'
);

await this.attachAliasToIndex();
}

this.options.logger.debug(
'Session index already exists. Attaching alias to the index and ensuring up-to-date mappings...'
'Session index already exists. Ensuring up-to-date index mappings...'
);

await this.attachAliasToIndex();

let indexMappingsVersion: string | undefined;
try {
const indexMappings = await this.options.elasticsearchClient.indices.getMapping({
Expand Down

0 comments on commit 45d9fa0

Please sign in to comment.