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

Disallow non-file characters in pipeline names #108648

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

parkertimmins
Copy link
Contributor

@parkertimmins parkertimmins commented May 14, 2024

Pipeline names may currently contain several non-alpha characters, such as comma and asterisk. Pipelines with such names cannot be fetched by id from the GET _ingest/pipeline/ api as the comma is interpreted to mean multiple id queries, and the asterisk is treated as a star query. Thus these characters, along with other characters which are not allowed in index or alias names, should be disallowed from pipeline names.

closes #104411

Pipeline names may currently contain several characters non-alpah
characters, such as comma and asterisk. Pipelines with such name cannot
be fetched by id from the `GET _ingest/pipeline/` api as the comma is
interpreted to mean multiple id queries, and the asterisk is treated as
a star query. Thus these characters, along with other characters which
are not allowed in index or alias names, should be disallowed from
pipeline names.
@parkertimmins parkertimmins marked this pull request as ready for review May 15, 2024 14:48
@parkertimmins parkertimmins self-assigned this May 15, 2024
@elasticsearchmachine elasticsearchmachine added the needs:triage Requires assignment of a team area label label May 15, 2024
@parkertimmins parkertimmins added >bug :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP and removed needs:triage Requires assignment of a team area label labels May 15, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label May 15, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

Pipeline.create can be called on existing pipeline. We want existing
pipelines with names that contains special characters to continue to
function. Thus only validation in put request before making pipeline.
@nielsbauman
Copy link
Contributor

Should we consult the Backwards Compatability Committee (BCC) on this?

cc @dakrone

@dakrone
Copy link
Member

dakrone commented May 15, 2024

This is definitely something we'd want to consider as breaking before merging (and should consult the BCC). Also (separately), I wouldn't consider this large enough to introduce a brand new exception, we could likely use IllegalArgumentException for this. We can use MetadataCreateIndexService.validateIndexOrAliasName for the validation and construction of the IllegalArgumentException (we do this in a few places in the code).

@parkertimmins
Copy link
Contributor Author

I was thinking to discuss with the team whether or not this seemed breaking changy, but in that case I'll go ahead and open a BCC ticket.

Validating pipeline names is not worth creating a new exception, just use IllegalArgumentException .
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.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PUT _ingest/pipeline allows creating a pipeline with special characters in its name
4 participants