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.

Sign up for GitHub

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

Jump to bottom

[BUG] GET /_cat/snapshots incorrectly requires a repository #16648

Open
dblock opened this issue Nov 14, 2024 · 4 comments · May be fixed by #16960
Open

[BUG] GET /_cat/snapshots incorrectly requires a repository #16648

dblock opened this issue Nov 14, 2024 · 4 comments · May be fixed by #16960
Labels
bug Something isn't working Storage:Snapshots

Comments

@dblock
Copy link
Member

dblock commented Nov 14, 2024 •
edited
Loading

Describe the bug

According to https://opensearch.org/docs/latest/api-reference/cat/cat-snapshots/ GET /_cat/snapshots should return all snapshots for a repository, but there's no way to specify a repository (/_cat/snapshots/{repository} works).

Instead it returns a 400 requiring a repository name.

400 (application/json) | {
  "error": {
    "root_cause": [
      {
        "type": "action_request_validation_exception",
        "reason": "Validation Failed: 1: repository is missing;"
      }
    ],
    "type": "action_request_validation_exception",
    "reason": "Validation Failed: 1: repository is missing;"
  },
  "status": 400
}

Either the doc is incorrect and this endpoint should not be exposed without a required repository parameter, or the code is broken for /_cat/snapshots.

Related component

Storage:Snapshots

To Reproduce

curl ...

Expected behavior

Behave as described in https://opensearch.org/docs/latest/api-reference/cat/cat-snapshots/.

Additional Details

Vanilla 2.18 with path.repo=/tmp/opensearch/repo.
@dblock dblock added bug Something isn't working untriaged labels Nov 14, 2024
@dblock dblock mentioned this issue Nov 14, 2024
Closed
@finnegancarroll finnegancarroll mentioned this issue Nov 29, 2024
Closed
1 task
@finnegancarroll
Copy link
Contributor

It looks like 'repository' can be supplied as a query param for /_cat/snapshots which is not reflected by the documentation.
However It does seem redundant to support /_cat/snapshots and /_cat/snapshots/<repository>. What are your thoughts on removing /_cat/snapshots from code and docs and only supporting /_cat/snapshots/<repository>?

@dblock
Copy link
Member Author

dblock commented Nov 29, 2024 •
edited
Loading

It looks like 'repository' can be supplied as a query param for /_cat/snapshots which is not reflected by the documentation. However It does seem redundant to support /_cat/snapshots and /_cat/snapshots/<repository>. What are your thoughts on removing /_cat/snapshots from code and docs and only supporting /_cat/snapshots/<repository>?

It's a breaking change now, so I'd deprecate it.

@dblock dblock mentioned this issue Nov 29, 2024
Merged
@dblock
Copy link
Member Author

dblock commented Nov 29, 2024

I added the missing parameter to the spec in opensearch-project/opensearch-api-specification#700, so it will make its way to the documentation via opensearch-project/documentation-website#8692.

@dblock dblock removed the untriaged label Dec 2, 2024
@dblock
Copy link
Member Author

dblock commented Dec 2, 2024

[Catch All Triage - 1, 2, 3]

@finnegancarroll finnegancarroll linked a pull request Jan 6, 2025 that will close this issue
Draft
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working Storage:Snapshots
Projects
Storage Project Board
Status: 🆕 New
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Deprecate /_cat/snapshots in favor of /_cat/snapshots/{repository} finnegancarroll/OpenSearch
2 participants
@dblock @finnegancarroll