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

HEAD /index/_alias/ always returns 200 OK even alias is not set #24644

Closed
jbfewo opened this issue May 12, 2017 · 13 comments · Fixed by #25043

Comments

@jbfewo
Copy link

commented May 12, 2017

Elasticsearch version: 5.4.0

Plugins installed: []

JVM version: java version "1.8.0_131"

OS version: Debian 3.16.39-1+deb8u2

Description of the problem including expected versus actual behavior:

Making a HEAD request on an existing index to check the existance of a given alias always returns 200 OK, even if the alias doesn't exist (for that index).

I expect a 404 status like in previous versions of ES.

Steps to reproduce:

  1. create an index (i.e. foo_index)
  2. curl -I -XHEAD 'localhost:9200/foo_index/_alias/nonexisting_alias'
HTTP/1.1 200 OK
content-type: application/json; charset=UTF-8
content-length: 2
@jasontedor

This comment has been minimized.

Copy link
Member

commented May 12, 2017

We aligned the HEAD and GET methods (as per the HTTP specification). The problem here is that it appears that GET does not return a 404 on a non-existing alias, instead it returns an empty body. I think the reason for this is in case you specify multiple indices (GET /i/_alias/a,b) and a exists but b doesn't we would return 200 OK and a and so this devolves to an empty body on GET /i/_alias/b). This seems wrong to me. I will solicit the thoughts of others.

@jasontedor

This comment has been minimized.

Copy link
Member

commented May 12, 2017

@javanna @clintongormley Thoughts here?

@javanna

This comment has been minimized.

Copy link
Member

commented May 12, 2017

With indices, you would be able to control how to treat unavailable indices through the ignore_unavailable parameter. But when resolving aliases, we never throw exception if the provided names don't exist and there is no option to change that. Hence we return 200 all the time. Maybe we should apply the indices options to the alias side as well and allow for throwing exception hence returning 404 when one or more aliases is missing? Otherwise, I am not sure we should always throw whenever an alias is not found as that becomes problematic when one alias is there but others aren't.

Unrelated, but I would prefer to have get alias return something that indicates more clearly that the index exists but the alias doesn't, like:

{
  "foo_index" : {}
}

rather than completely skipping the index object in the response and returning an empty body. But that is a different concern and doesn't address this issue.

@jasontedor

This comment has been minimized.

Copy link
Member

commented May 12, 2017

Otherwise, I am not sure we should always throw whenever an alias is not found as that becomes problematic when one alias is there but others aren't.

Sure, I meant to question only the behavior when none are found.

@javanna

This comment has been minimized.

Copy link
Member

commented May 12, 2017

right, we could also throw error in get alias and 404 in alias exists when nothing is found. That would be ok I guess.

@jasontedor

This comment has been minimized.

Copy link
Member

commented May 15, 2017

@clintongormley After spelunking through git, it appears the behavior here might have originated with an older comment. Do you still think we should not 404 if there are no matching aliases?

@clintongormley

This comment has been minimized.

Copy link
Member

commented May 15, 2017

After spelunking through git, it appears the behavior here might have originated with an older comment. Do you still think we should not 404 if there are no matching aliases?

It is hard to know the right thing to do here. I think the problem comes down to the API itself. The {index} portion is really {index-or-alias} as either can be provided. The {alias} part means "filter the result to only show this alias". I'm not even sure that this filtering functionality is practically useful. When would you ask for filtered aliases rather than all the aliases for an index?

On top of that, aligning the GET and HEAD behaviours doesn't make much sense here either as they're asking for different things. The HEAD request in our API is defined as "does this alias exist on this index?".

The only way I can think to make these two consistent is to throw a 404 if any of the listed aliases doesn't exist. Maybe that's OK. It's not terribly useful for GET, but it is useful for HEAD. Of course, this would be a breaking change

@m1kola

This comment has been minimized.

Copy link

commented Jun 2, 2017

I think this should be mentioned in the Breaking changes in 5.4 document and/or fixed, because HEAD request returns 404 for this endpoint in Elasticsearch 5.3 (I guess it works in the same way in all 5.* versions except 5.4 and higher). As far as remember versions 2.* and in 1.* also return 404.

Also the official python client doesn't know how to handle new behaviour (see the exists_alias method of elasticsearch.client.IndicesClient in elasticsearch>=5.0.0,<6.0.0), so I guess update to 5.4 breaks a lot of applications that use this package.

People should know where to look. I've spent almost whole day on debugging. I thought it was an error in my application.

@javanna

This comment has been minimized.

Copy link
Member

commented Jun 2, 2017

hi @m1kola which breaking change do you mean? You commented on an issue that is opened for discussion, hence no change directly related to this issue has been committed yet.

@jasontedor

This comment has been minimized.

Copy link
Member

commented Jun 2, 2017

@m1kola It's in the release notes for 5.4.0 as this is considered a bug fix (the fact that HEAD and GET had mismatched behavior is a specification violation, and therefore a bug). The line item is:

Fix alias HEAD requests #23094 (issue: #21125)

@m1kola

This comment has been minimized.

Copy link

commented Jun 2, 2017

hi @m1kola which breaking change do you mean? You commented on an issue that is opened for discussion, hence no change directly related to this issue has been committed yet.

Hi @javanna. I know that the issue is marked for discussion, but it looks like initially @jbfewo wanted to report a bug. See the first comment.

I expect a 404 status like in previous versions of ES.


@m1kola It's in the release notes for 5.4.0 as this is considered a bug fix (the fact that HEAD and GET had mismatched behavior is a specification violation, and therefore a bug). The line item is:

Fix alias HEAD requests #23094 (issue: #21125)

Hi @jasontedor. Yes, it fixes specification, but introduces incompatibility with 5.3 and older versions. See the demo below.

Steps to compare/reproduce

I compare the behaviour of ES 5.3.3 and 5.4.1 using the official Docker images.

ES 5.3.3:

docker run -p 9200:9200 -e "http.host=0.0.0.0" -e "transport.host=127.0.0.1" docker.elastic.co/elasticsearch/elasticsearch:5.3.3

ES 5.4.1:

docker run -p 9200:9200 -e "http.host=0.0.0.0" -e "transport.host=127.0.0.1" docker.elastic.co/elasticsearch/elasticsearch:5.4.1

Check an alias. Index doesn't exist.

Check the this_index_should_be_deleted alias exists. The my_index index doesn't exist, at the moment of a request.

Request:

curl -H "Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==" -H "Content-Type: application/json; charset=UTF-8" -I 'http://localhost:9200/my_index/_alias/this_index_should_be_deleted?pretty'

Response. Both 5.3.3 and 5.4.1 return 404:

HTTP/1.1 404 Not Found
content-type: text/plain; charset=UTF-8
content-length: 0

Create the my_index index

Request:

curl -sD - -H "Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==" -H "Content-Type: application/json; charset=UTF-8" -XPUT 'http://localhost:9200/my_index?pretty' -d '{}'

Response. Same on both versions:

HTTP/1.1 200 OK
content-type: application/json; charset=UTF-8
content-length: 60

{
  "acknowledged" : true,
  "shards_acknowledged" : true
}

Check an alias. Alias doesn't really exist, but index exists

Request:

curl -H "Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==" -H "Content-Type: application/json; charset=UTF-8" -I 'http://localhost:9200/my_index/_alias/this_index_should_be_deleted?pretty'

Response on 5.3.3:

HTTP/1.1 404 Not Found
content-type: text/plain; charset=UTF-8
content-length: 0

🔴 Response on 5.4.1:

HTTP/1.1 200 OK
content-type: application/json; charset=UTF-8
content-length: 4

Create an alias

Request:

curl -sD - -H "Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==" -H "Content-Type: application/json; charset=UTF-8" -XPUT 'http://localhost:9200/my_index/_alias/alias_for_my_index?pretty' -d '{}'

Response. Same on both versions:

HTTP/1.1 200 OK
content-type: application/json; charset=UTF-8
content-length: 28

{
  "acknowledged" : true
}

Check the alias. Both index and alias are exist

Request:

curl -sD - -H "Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==" -H "Content-Type: application/json; charset=UTF-8" -XPUT 'http://localhost:9200/my_index/_alias/alias_for_my_index?pretty' -d '{}'

Response. Same on both versions:

HTTP/1.1 200 OK
content-type: application/json; charset=UTF-8
content-length: 28

{
  "acknowledged" : true
}

Hope this helps.

@jasontedor

This comment has been minimized.

Copy link
Member

commented Jun 2, 2017

It's just mislabeled, we are going to fix this.

@jasontedor

This comment has been minimized.

Copy link
Member

commented Jun 3, 2017

I opened #25043.

gasman added a commit to gasman/wagtail that referenced this issue Jun 7, 2017
5.4 has a bug in testing existence of aliases which causes tests to fail: elastic/elasticsearch#24644
gasman added a commit to wagtail/wagtail that referenced this issue Jun 7, 2017
5.4 has a bug in testing existence of aliases which causes tests to fail: elastic/elasticsearch#24644
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.