rgw: es: support username and password for ES #22030

theanalyst · 2018-05-16T12:48:47Z

For ES endpoints terminated with a username and password, either via xpack or
fronted by another webserver with http basic auth, we now support "username" and
"password" configurable which should be capable of doing HTTP basic
authentication

Fixes: https://tracker.ceph.com/issues/23655

This is built atop of #21852 and basically first adds support for headers in get http crs, and then adds these headers as a part of all the ES requests

theanalyst · 2018-06-12T15:18:47Z

ran a suite at http://pulpito.ceph.com/abhi-2018-06-06_14:42:46-rgw-wip-abhi-testing-3-distro-basic-smithi/ where we had some failures due to centos mirrors and re-run at http://pulpito.ceph.com/abhi-2018-06-11_09:09:36-rgw-wip-abhi-testing-3-distro-basic-smithi/

yuriw · 2018-07-23T21:21:37Z

@theanalyst pls rebase

--- pr 22030 --- pulling https://github.com/theanalyst/ceph.git branch wip-es6-auth
remote: Counting objects: 32, done.
remote: Total 32 (delta 26), reused 26 (delta 26), pack-reused 6
Unpacking objects: 100% (32/32), done.
From https://github.com/theanalyst/ceph

branch wip-es6-auth -> FETCH_HEAD
Auto-merging src/rgw/rgw_sync_module_es.cc
CONFLICT (file/directory): There is a directory with name src/dmclock in 67fe083. Adding src/dmclock as src/dmclock~HEAD
Automatic merge failed; fix conflicts and then commit the result.
Traceback (most recent call last):
File "/home/yuriw/wip_master/src/script/build-integration-branch", line 62, in
assert not r
AssertionError

theanalyst · 2018-08-07T15:40:16Z

@yuriw updated..

theanalyst · 2018-08-15T09:03:22Z

@yehudasa can you review this once again, this was the same as the fixes for ES versions > 5.0 + support for ES deployments with a username + pw

theanalyst · 2018-09-14T07:08:54Z

@yehudasa ping, this contains the ES6 support + creating a username/pw for setups that have X-pack configured. I closed the earlier approved #21852, this contains that pr + one commit for username/pw support

yehudasa

@theanalyst looks great, see my comment

yehudasa · 2018-09-14T13:40:55Z

src/rgw/rgw_sync_module_es.cc

+  if (v1.major_ver == v2.major_ver)
+    return v1.minor_ver >= v2.minor_ver;
+
+  return v1.major_ver > v2.major_ver;


shouldn't it be >= here?

I guess the first equality should catch this, having said that this does look non intuitive, I'll collapse this to a one liner

theanalyst · 2018-10-10T08:47:28Z

@yehudasa updated to use a std::pair as the ESVersion, and just a wrapper struct to parse the nested version json.

chardan · 2018-10-10T23:18:00Z

src/rgw/rgw_sync_module_es.cc

+
+  ESVersion get_version() {
+      int _major,_minor;
+      sscanf(s.c_str(), "%d.%d", &_major, &_minor);


Is there any possibility of parse error (including empty input) here? Remember to check the result of sscanf(), if you'd like to use scanf() here and there's any possibility of error. Or, consider using std::stoi(), which also has error detection (note that the older C atoi() doesn't give you a way to do this) and supports std::string directly.

@theanalyst should at least dump error and set defaults to _major and _minor.

stale · 2018-12-10T18:41:58Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

theanalyst · 2018-12-12T16:02:13Z

rebased against current master, also have made the es_version_decoder throw a JSONDecoder error in case of a failed scanf, since the error will be caught by the rest coroutine which will ultimately raise an -EINVAL.

theanalyst · 2018-12-12T17:42:20Z

Passes local testing, @yehudasa can you take a look

This commit introduces an enum mapping various ES types. Also encode_json now has a new member for string type so that this can be changed easily when upgrading ES versions for eg. wherein new ES engines do not support the string type anymore. Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

Adding another constructor overload for cases when we need to override the default headers Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

Getting the basic information from elasticsearch, while we're only currently interested in elasic search version, other data such as the ES cluster name etc may be useful in the future as well. Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

from Elasticsearch 5.0+ the type string is deprecated, while it is still allowed in 5.0, it returns a 400 error on 6.0 clusters as this type is fully removed. We now determine the es version while initializing the cluster from elasticsearch's default endpoint and use that to determine what string type to use. This way we support both 2.x and 5.x,6.x es versions as we default to string type for clusters < 5.0 Fixes: http://tracker.ceph.com/issues/22877 Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

Adding support to add additional headers as a part of GET and READ requests. Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

For ES endpoints terminated with a username and password, either via xpack or fronted by another webserver with http basic auth, we now support "username" and "password" configurable which should be capable of doing HTTP basic authentication Fixes: https://tracker.ceph.com/issues/23655 Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

using std::pair as the version struct. Another decoder class is currently introduced as we have a nested json type for version right now. Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

theanalyst · 2019-01-07T17:03:55Z

@yehudasa updated against the current master, and tested locally, can you take a look

yehudasa · 2019-01-23T19:06:54Z

@theanalyst still looks good

yehudasa · 2019-01-23T19:07:38Z

@theanalyst feel free to merge if passing tests. We'll want to backport it to mimic.

yehudasa · 2019-01-23T19:10:14Z

@theanalyst though note comment by @chardan

Need to revise string to text conversion for ES 5.x+

yehudasa · 2019-01-25T19:17:03Z

src/rgw/rgw_sync_module_es.cc


        es_index_settings settings(conf->num_replicas, conf->num_shards);
        es_index_mappings mappings;
+        if (es_info.version >= ESVersion(5,0)) {
+          ldout(sync_env->cct, 0) << "elasticsearch: using text type for string index mappings " << dendl;
+          mappings.string_type = ESType::Text;


@theanalyst note migration instructions here:
https://www.elastic.co/blog/strings-are-dead-long-live-strings
string converts into either text or keyword, depending on the analyzed state. We also need to make sure the index field is being created correctly (as code is written now, it doesn't create it).

Thanks for the find, I'll modify dump_type to handle these cases

I'm seeing the same behaviour even without the patch in ES 5.6, we get the double mapping to both text and keyword. I assume name fields should be text types and most of the fields like IDs should be keywords

So reading more into this it seems even mapping types are already deprecated and going away in the next ES version. https://www.elastic.co/guide/en/elasticsearch/reference/current/removal-of-types.html, not sure how exactly we're supposed to be mapping types then. For now I'll decide based on "analyzed" field whether we dump a keyword or a text if the type set is not a string already.

yehudasa · 2019-01-28T15:38:18Z

@theanalyst I did some work on this, see here:
#26106

theanalyst · 2019-02-04T10:37:51Z

taken up in #26106

theanalyst requested a review from yehudasa May 16, 2018 12:48

batrick added rgw needs-review labels May 16, 2018

theanalyst force-pushed the wip-es6-auth branch from 51b3972 to 67fe083 Compare May 18, 2018 11:51

theanalyst added the wip-abhi2-testing label Jun 5, 2018

theanalyst added needs-qa and removed wip-abhi2-testing labels Jul 18, 2018

yuriw added the wip-yuri2-testing label Jul 23, 2018

yuriw added wip-abhi2-testing and removed needs-qa wip-yuri2-testing labels Jul 23, 2018

theanalyst force-pushed the wip-es6-auth branch from 67fe083 to c146104 Compare August 7, 2018 15:39

theanalyst mentioned this pull request Aug 7, 2018

rgw: elastic: fixes for supporting Elasticsearch versions >= 5.0 #21852

Closed

yehudasa previously approved these changes Sep 14, 2018

View reviewed changes

chardan reviewed Oct 10, 2018

View reviewed changes

stale bot added the stale label Dec 10, 2018

tchaikov added the needs-qa label Dec 11, 2018

stale bot removed the stale label Dec 11, 2018

theanalyst force-pushed the wip-es6-auth branch 3 times, most recently from bdffb4f to 44ae56b Compare December 12, 2018 15:56

theanalyst added 7 commits January 7, 2019 15:43

rgw: rgw_cr_rest: allow headers to be specified in Put CRs

c5415d5

Adding another constructor overload for cases when we need to override the default headers Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

rgw_cr_rest: support extra headers for get resource crs

2dc64fd

Adding support to add additional headers as a part of GET and READ requests. Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

rgw: ES sync module: drop ESVersion struct

53d1685

using std::pair as the version struct. Another decoder class is currently introduced as we have a nested json type for version right now. Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

theanalyst force-pushed the wip-es6-auth branch from 44ae56b to 53d1685 Compare January 7, 2019 14:44

yehudasa removed the needs-review label Jan 23, 2019

yehudasa reviewed Jan 25, 2019

View reviewed changes

theanalyst closed this Feb 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rgw: es: support username and password for ES #22030

rgw: es: support username and password for ES #22030

theanalyst commented May 16, 2018

theanalyst commented Jun 12, 2018

yuriw commented Jul 23, 2018

theanalyst commented Aug 7, 2018

theanalyst commented Aug 15, 2018

theanalyst commented Sep 14, 2018

yehudasa left a comment

yehudasa Sep 14, 2018

theanalyst Sep 14, 2018

theanalyst commented Oct 10, 2018

chardan Oct 10, 2018

yehudasa Jan 23, 2019

stale bot commented Dec 10, 2018

theanalyst commented Dec 12, 2018

theanalyst commented Dec 12, 2018

theanalyst commented Jan 7, 2019

yehudasa commented Jan 23, 2019

yehudasa commented Jan 23, 2019

yehudasa commented Jan 23, 2019

yehudasa Jan 25, 2019

theanalyst Jan 28, 2019

theanalyst Jan 28, 2019

theanalyst Jan 28, 2019

yehudasa commented Jan 28, 2019

theanalyst commented Feb 4, 2019

rgw: es: support username and password for ES #22030

rgw: es: support username and password for ES #22030

Conversation

theanalyst commented May 16, 2018

theanalyst commented Jun 12, 2018

yuriw commented Jul 23, 2018

theanalyst commented Aug 7, 2018

theanalyst commented Aug 15, 2018

theanalyst commented Sep 14, 2018

yehudasa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theanalyst commented Oct 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Dec 10, 2018

theanalyst commented Dec 12, 2018

theanalyst commented Dec 12, 2018

theanalyst commented Jan 7, 2019

yehudasa commented Jan 23, 2019

yehudasa commented Jan 23, 2019

yehudasa commented Jan 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yehudasa commented Jan 28, 2019

theanalyst commented Feb 4, 2019