use https for package sources

[koenpunt]

No description provided.

[elasticsearch-release]

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

[jakommo]

Good catch @koenpunt , thanks for the PR.
IIRC at least for ubuntu this would require apt-transport-https to be installed, which, for some reason, isn't by default. Mind adding this as well?
Not sure about yum though.

[koenpunt]

Hadn't had any trouble without apt-transport-https on Ubuntu > 14.04. But it is installed, so maybe as a dependency of something else. I'll add it ;)

[jakommo]

Awesome, thanks.

[jakommo]

Jenkins, test it!

[koenpunt]

Added, and from what I found thus far is that Yum supports https out of the box.

[koenpunt]

I believe the failed run has nothing to do with these changes..?

[jakommo]

Yeah looks like some issue with kitchen tests.
Thanks for contributing 😄

[koenpunt]

Sidenote, I think anyone now updating the role will end up with 2 separate source definitions in /etc/apt/sources.list.d/packages_elastic_co_elasticsearch_2_x_debian.list, one for http and 1 for https. Not sure if that's going to give issues on installation?

[jakommo]

Yeah, just tested, results in a duplicate warning:

W: Duplicate sources.list entry https://packages.elastic.co/elasticsearch/2.x/debian/ stable/main amd64 Packages (/var/lib/apt/lists/packages.elastic.co_elasticsearch_2.x_debian_dists_stable_main_binary-amd64_Packages)
W: You may want to run apt-get update to correct these problems

Only affects Debian/Ubuntu as RHEL/CentOS uses: https://github.com/elastic/ansible-elasticsearch/blob/master/templates/elasticsearch.repo

Currently I can only think of adding something like {{ es_apt_url_old }} and add another task to remove it prior to installing the new one.

@gingerwizard @jpcarey @koenpunt any better ideas?

[koenpunt]

We can just add an extra line to the task I guess:

apt_repository: repo="{{ es_apt_url_old }}" state=absent

[koenpunt]

I've created a PR if that's the change we want: #172

[koenpunt]

Or we can add a additional variable like es_apt_use_https, with a default of true and add some logic around that. But I can imagine that https is something you want to enforce sometime in the future, so probably best to move away from it.

[jpcarey]

@jakommo would there be harm to switching packages_elastic_co_elasticsearch_2_x_debian.list to a template? It seems like the current situation creates extra logic that could be confusing (without some comments that explain it).

[jakommo]

See my comment #172 (comment).
I think it makes sense to use a more generic filename for this, so every major version is in the same file.
For this to work we need tow tasks, one removing the old repo file and one to add the https version into a new repo file with a generic filename.

About switching to template. Not sure if this brings any advantage. I think staying with apt_repository at least has the advantage of using something like update_cache.

Looks like there is a yum_repository now and it has a lot more options than the apt one. Maybe we should think about switching from the template to the yum_repository and then use there repo modules for both.
Could profit from that in the future if more features are added to the modules.

[koenpunt]

apt_repository has a filename option, but only since Ansible 2.1.

☞ I do think the apt_repository should be more flexible, as in specifying type (deb, deb-src) distro and endpoint separately, for which then the url could be matched on both https and http. But even if this change is going to happen in Ansible, it will probably not before Ansible 2.2.

[jakommo]

apt_repository has a filename option, but only since Ansible 2.1.

That shouldn't be a problem since we require > 2.1.2 already

[jakommo]

Created #173 to fix this for now.
Had a quick look at the 5.x repo and the URL has changed, so I think it makes sense to look in more depth into a solution that works for all versions then.