Fix _alias/<alias> returning non-matching data streams #104145

mattc58 · 2024-01-09T16:27:17Z

This fixes a bug where GET _alias/<alias> would return non-matching data streams. This happens when security is enabled and when the index pattern is changed to ["*',"-*"] and the data streams don't properly process that.

Fixes #96589.

elasticsearchmachine · 2024-01-09T16:27:41Z

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

mattc58 · 2024-01-09T16:36:37Z

The new REST test can be run with: ./gradlew ':modules:data-streams:yamlRestTest' --tests "org.elasticsearch.datastreams.DataStreamsClientYamlTestSuiteIT.test {p0=data_stream/140_data_stream_aliases/Test get alias with non-matching data streams}" --info

It will fail with the current code with:

      - is_false: ds-first.aliases.my-alias

    [2024-01-10T03:34:49,505][INFO ][o.e.d.DataStreamsClientYamlTestSuiteIT] [test] Stash dump on test failure [{
      "stash" : {
        "body" : {
          "ds-first" : {
            "aliases" : {
              "my-alias" : { }
            }
          }
        }
      }
    }]

[snip]

        Caused by:
        java.lang.AssertionError: field [ds-first.aliases.my-alias] has a true value but it shouldn't
        Expected: ("" or a string equal to "false" ignoring case or "0")
             but: was "{}"
            at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
            at org.junit.Assert.assertThat(Assert.java:956)
            at org.elasticsearch.test.rest.yaml.section.IsFalseAssertion.doAssert(IsFalseAssertion.java:48)
            at org.elasticsearch.test.rest.yaml.section.Assertion.execute(Assertion.java:65)
            at org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.executeSection(ESClientYamlSuiteTestCase.java:558)
            ... 38 more

elasticsearchmachine · 2024-01-09T19:20:21Z

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

elasticsearchmachine · 2024-01-09T19:36:58Z

Hi @mattc58, I've updated the changelog YAML for you.

…aStreamAlias

…o fix-96589-alias-datastreams

…astreams

masseyke · 2024-01-22T17:20:50Z

server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java

+        }
+
+        {
+            GetAliasesRequest request = new GetAliasesRequest();


Should we have a test in here that covers the exclusions case (like testFindAliasWithExclusion but for findDataStreamAliases? I don't know that we even claim to support it, but we get it virtually for free with what you've done.

…aStream

…astreams

masseyke · 2024-01-26T14:09:18Z

...rc/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java

-                .toList();
-            if (aliases.isEmpty() == false) {
-                result.put(requestedDataStream, aliases);
+            if (dsAliases.containsKey(requestedDataStream)) {


I think that this check is redundant now, right? In Metadata, the setter only puts things in the map that's returned if they are the things you passed in as the second argument to findDataStreamAliases.

That is, can you just return the result of findDataStreamAliases from this method?

masseyke · 2024-01-26T14:13:35Z

server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java

+     */
+    public Map<String, List<DataStreamAlias>> findDataStreamAliases(final String[] aliases, final String[] dataStreams) {
+        ImmutableOpenMap.Builder<String, List<DataStreamAlias>> mapBuilder = ImmutableOpenMap.builder();
+        Map<String, List<DataStreamAlias>> dataStreamAliases = dataStreamAliasesByDataStream();


I'm wondering if at some point we'll need to start computing this outside of this method if users have a lot of data stream aliases. But doing it once per request doesn't seem all that bad.

I wonder if doing the analysis and guidelines suggested in #104456 would help guide us here. Maybe we could store aliases differently in cluster state.

masseyke

LGTM. Seems like a big improvement over what we had.

masseyke · 2024-01-26T14:17:10Z

...treams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml

+---
+"Test get alias with non-matching data streams":
+  - skip:
+      version: " - 8.12.1"


This is currently pointed at main (8.13.0) and the PR isn't labeled with 8.12.1. We'll need to make sure to remember backport it, or change this version if this doesn't get backported in time for 8.12.1

…astreams

mattc58 · 2024-01-29T20:03:22Z

@elasticsearchmachine run elasticsearch-ci/part-2

elasticsearchmachine · 2024-01-29T21:23:29Z

💚 Backport successful

Status	Branch	Result
✅	8.12

This fixes a bug where GET _alias/<alias> would return non-matching data streams

… (#104880) * Fix _alias/<alias> returning non-matching data streams (#104145) This fixes a bug where GET _alias/<alias> would return non-matching data streams * Remove problematic assertions * Change to `containsInAnyOrder` in assertions for lists

mattc58 added 3 commits January 9, 2024 11:21

Add test that fails on current code

8840d5b

Change the way data streams are found when getting aliases

48dc55d

Change optimistic GA notice to "was available" in tests

ddad4d5

mattc58 added >bug :Data Management/Data streams Data streams and their lifecycles Team:Data Management Meta label for data/management team v8.13.0 labels Jan 9, 2024

Update docs/changelog/104145.yaml

67ba648

Apply spotless fixes

a0394c6

mattc58 added auto-backport Automatically create backport pull requests when merged and removed auto-backport Automatically create backport pull requests when merged labels Jan 9, 2024

mattc58 marked this pull request as ready for review January 9, 2024 19:19

Update docs/changelog/104145.yaml

6c2f36b

mattc58 marked this pull request as draft January 10, 2024 00:29

mattc58 added 4 commits January 16, 2024 15:58

Refactor Metadata.findAliases to work with both AliasMetadata and Dat…

ad527e7

…aStreamAlias

Merge remote-tracking branch 'origin/fix-96589-alias-datastreams' int…

8e4899f

…o fix-96589-alias-datastreams

Merge remote-tracking branch 'upstream/main' into fix-96589-alias-dat…

997ed07

…astreams

Remove debugging code

cdb5715

mattc58 marked this pull request as ready for review January 17, 2024 14:28

mattc58 requested a review from masseyke January 17, 2024 14:28

masseyke reviewed Jan 22, 2024

View reviewed changes

mattc58 added 3 commits January 23, 2024 08:41

Use a map of data stream aliases by data stream name

b8a3b5f

Add tests for alias exclusion and new function dataStreamAliasesByDat…

7e0dd21

…aStream

Merge remote-tracking branch 'upstream/main' into fix-96589-alias-dat…

3ba3a55

…astreams

mattc58 requested a review from masseyke January 23, 2024 16:53

masseyke reviewed Jan 26, 2024

View reviewed changes

masseyke approved these changes Jan 26, 2024

View reviewed changes

masseyke reviewed Jan 26, 2024

View reviewed changes

mattc58 added 2 commits January 29, 2024 09:55

Merge remote-tracking branch 'upstream/main' into fix-96589-alias-dat…

ab9a20e

…astreams

Add PR feedback

f06f842

mattc58 added v8.12.1 auto-backport-and-merge Automatically create backport pull requests and merge when ready labels Jan 29, 2024

mattc58 merged commit de68d91 into elastic:main Jan 29, 2024
15 checks passed

mattc58 deleted the fix-96589-alias-datastreams branch January 29, 2024 21:22

mattc58 mentioned this pull request Jan 29, 2024

[8.12] Fix _alias/<alias> returning non-matching data streams (#104145) #104880

Merged

mattc58 added a commit to mattc58/elasticsearch that referenced this pull request Jan 29, 2024

Fix _alias/<alias> returning non-matching data streams (elastic#104145)

75c8068

This fixes a bug where GET _alias/<alias> would return non-matching data streams

mattc58 mentioned this pull request Jan 29, 2024

Fix problematic assertions in unit tests #104885

Merged

mattc58 mentioned this pull request Feb 7, 2024

Failing ES Promotion: FTR Configs #48 / Alerts APIs - Trial License/Complete Tier @ess Alerts Compatibility Query should generate a signal-on-legacy-signal with AAD index pattern elastic/kibana#176105

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix _alias/<alias> returning non-matching data streams #104145

Fix _alias/<alias> returning non-matching data streams #104145

mattc58 commented Jan 9, 2024 •

edited

Loading

elasticsearchmachine commented Jan 9, 2024

mattc58 commented Jan 9, 2024

elasticsearchmachine commented Jan 9, 2024

elasticsearchmachine commented Jan 9, 2024

masseyke Jan 22, 2024

masseyke Jan 26, 2024

masseyke Jan 26, 2024

masseyke Jan 26, 2024

mattc58 Jan 29, 2024

masseyke left a comment

masseyke Jan 26, 2024

mattc58 commented Jan 29, 2024

elasticsearchmachine commented Jan 29, 2024

Fix _alias/<alias> returning non-matching data streams #104145

Fix _alias/<alias> returning non-matching data streams #104145

Conversation

mattc58 commented Jan 9, 2024 • edited Loading

elasticsearchmachine commented Jan 9, 2024

mattc58 commented Jan 9, 2024

elasticsearchmachine commented Jan 9, 2024

elasticsearchmachine commented Jan 9, 2024

masseyke Jan 22, 2024

Choose a reason for hiding this comment

masseyke Jan 26, 2024

Choose a reason for hiding this comment

masseyke Jan 26, 2024

Choose a reason for hiding this comment

masseyke Jan 26, 2024

Choose a reason for hiding this comment

mattc58 Jan 29, 2024

Choose a reason for hiding this comment

masseyke left a comment

Choose a reason for hiding this comment

masseyke Jan 26, 2024

Choose a reason for hiding this comment

mattc58 commented Jan 29, 2024

elasticsearchmachine commented Jan 29, 2024

💚 Backport successful

mattc58 commented Jan 9, 2024 •

edited

Loading