Skip to content

Conversation

@gmarouli
Copy link
Contributor

@gmarouli gmarouli commented Feb 6, 2025

To simplify the failure store API we remove the wildcard selector ::*.

When a user wants to access both data and failure components they can write my-ds,my-ds::failures or my-ds::data,my-ds::failures.

This should simplify the security model as well.

@gmarouli gmarouli requested a review from jbaiera February 7, 2025 13:44
@gmarouli gmarouli marked this pull request as ready for review February 7, 2025 13:44
@elasticsearchmachine elasticsearchmachine added the needs:triage Requires assignment of a team area label label Feb 7, 2025
@gmarouli gmarouli added >non-issue :Data Management/Data streams Data streams and their lifecycles auto-backport Automatically create backport pull requests when merged v9.0.0 v8.18.1 v8.19.0 Team:Data Management Meta label for data/management team and removed needs:triage Requires assignment of a team area label labels Feb 7, 2025
@elasticsearchmachine
Copy link
Collaborator

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

public static IndexComponentSelector read(StreamInput in) throws IOException {
return getById(in.readByte());
byte id = in.readByte();
if (in.getTransportVersion().onOrAfter(TransportVersions.REMOVE_ALL_APPLICABLE_SELECTOR)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

More versions will be added here when it gets backported.

Copy link
Member

@jbaiera jbaiera left a comment

Choose a reason for hiding this comment

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

LGTM! Left one question re: bwc and serialization, but otherwise good to go

DATA("data", (byte) 0),
FAILURES("failures", (byte) 1),
ALL_APPLICABLE("*", (byte) 2);
FAILURES("failures", (byte) 1);
Copy link
Member

Choose a reason for hiding this comment

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

This is an unlikely scenario but could we ever end up in a place where a new selector added here could collide with the old ::* value in a mixed cluster?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm.... Interesting thought. Thinking out loud because this is a tough cookie.

If we implement this the way it's now and backport it back to 8.18 and 8.x.
We can remove this backwards compatibility code from 9.x; because you always have to go through the 8.last version. 8.last will never propagate this selector because it matches it to ::data and this selector is never persisted anywhere, I think. Furthermore, there is no CCR or CCS supported yet. Right?

This means that if we ever introduced ::* in 9+ we could distinguish the two. Right?

@gmarouli gmarouli merged commit a7cf0f6 into elastic:main Feb 12, 2025
18 checks passed
@gmarouli gmarouli deleted the failure-store/remove-star-selector branch February 12, 2025 20:27
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Feb 12, 2025

Status Branch Result
9.0
8.18
8.x

gmarouli added a commit to gmarouli/elasticsearch that referenced this pull request Feb 13, 2025
@gmarouli gmarouli added v9.0.1 and removed v9.0.0 labels Feb 13, 2025
gmarouli added a commit to gmarouli/elasticsearch that referenced this pull request Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Data Management/Data streams Data streams and their lifecycles >non-issue Team:Data Management Meta label for data/management team v8.18.1 v8.19.0 v9.0.1 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants