Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix geoip databases index access after system feature migration (again) #122938

Merged
merged 8 commits into from
Feb 21, 2025

Conversation

joegallo
Copy link
Contributor

@joegallo joegallo commented Feb 19, 2025

This is a follow up to #121196.

Once again I've added a failing integration test to capture the actual failing workflow that we noticed during testing, then I wrote a failing unit test to try to keep things a little tighter. Finally there's a small change to our index expression resolution code.

As before, the true root issue is that we're doing index resolution in two passes when X-Pack Security is enabled -- in the first pass we resolve the wildcard to whatever (in a security-aware manner) and then in the second pass the wildcards have been removed, so it's like we're asking for things literally by name. The error in this case was because the wildcard resolution was including the alias (only!), so the second round of resolution was blowing up and saying "don't ask for an alias here".

I'm fine with the tests I've written here, but I think it's obvious from my comments that I'm not entirely thrilled with my actual "solution" to this bug.

@joegallo joegallo added >bug :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP Team:Data Management Meta label for data/management team v9.0.0 v8.18.1 v8.19.0 v9.1.0 labels Feb 19, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @joegallo, I've created a changelog YAML for you.

After migrating the system indices, the having .geoip_databases as an
index-behind-an-alias ends up resulting in `GET _data_stream` blowing
up.
such that a net new alias is visible if the net new index that it
points to is visible.
@joegallo joegallo force-pushed the fix-geoip-access-after-reindex-repeat branch from a16bc2f to ffc82ff Compare February 20, 2025 00:14
@joegallo joegallo marked this pull request as ready for review February 20, 2025 16:19
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@joegallo
Copy link
Contributor Author

This PR will close elastic/kibana#211102.

Copy link
Member

@masseyke masseyke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@gmarouli gmarouli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, +1 for the comment :)

@joegallo joegallo merged commit a895875 into elastic:main Feb 21, 2025
17 checks passed
@joegallo joegallo deleted the fix-geoip-access-after-reindex-repeat branch February 21, 2025 13:00
@joegallo
Copy link
Contributor Author

joegallo commented Feb 21, 2025

Dammit! I forgot the auto-backport label!!!1

joegallo added a commit to joegallo/elasticsearch that referenced this pull request Feb 21, 2025
joegallo added a commit to joegallo/elasticsearch that referenced this pull request Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP Team:Data Management Meta label for data/management team v8.18.1 v8.19.0 v9.0.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants