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

Add radosgw_frontend_ssl_certificate parameter #4130

Merged
merged 4 commits into from Jul 2, 2019

Conversation

gfidente
Copy link
Contributor

This is necessary when configuring RGW/civetweb with SSL because
in addition to passing a specific frontend option, we need to
append the 's' character to the binding port.

Signed-off-by: Giulio Fidente gfidente@redhat.com

Copy link
Contributor

@dsavineau dsavineau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add the Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1721914 statement in the commit body ?
Also I'm wondering if we could do both ssl implementation in the same time instead of civetweb only. wdyt ?

roles/ceph-config/templates/ceph.conf.j2 Outdated Show resolved Hide resolved
@gfidente gfidente force-pushed the rgw_ssl branch 2 times, most recently from 3c231bb to 9417b0f Compare June 20, 2019 12:45
@dsavineau
Copy link
Contributor

The jinja template still fails.
Also for beast you need the ssl key in a different file so there's another option for that.

http://docs.ceph.com/docs/master/radosgw/frontends/#options

@dsavineau
Copy link
Contributor

Nevermind, I didn't read the documentation correctly. So it's fine with only one combined file for both civetweb and beast

@dsavineau dsavineau changed the title Add radosgw_civetweb_ssl_certificate parameter Add radosgw_frontend_ssl_certificate parameter Jun 20, 2019
@ktdreyer
Copy link
Member

Do we need to backport this to stable-4.0 for OSP, @gfidente ?

@dsavineau
Copy link
Contributor

@dsavineau
Copy link
Contributor

@gfidente the code looks good but there's some part missing

  • The ssl file needs to be available within the radosgw container. So we probably need to add a bind mount around there [1]. Note that we should change ansible_distribution by ansible_os_family.
  • The radosgw handler needs to be updated to reflect the protocol [2].

[1] https://github.com/ceph/ceph-ansible/blob/master/roles/ceph-rgw/templates/ceph-radosgw.service.j2#L31-L33
[2] https://github.com/ceph/ceph-ansible/blob/master/roles/ceph-handler/templates/restart_rgw_daemon.sh.j2#L60

@gfidente
Copy link
Contributor Author

@dsavineau thanks for the review, will do

roles/ceph-rgw/templates/ceph-radosgw.service.j2 Outdated Show resolved Hide resolved
roles/ceph-rgw/templates/ceph-radosgw.service.j2 Outdated Show resolved Hide resolved
roles/ceph-handler/templates/restart_rgw_daemon.sh.j2 Outdated Show resolved Hide resolved
This is necessary when configuring RGW with SSL because
in addition to passing specific frontend options, civetweb
appends the 's' character to the binding port and beast uses
ssl_endpoint instead of endpoint.

Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1722071
Signed-off-by: Giulio Fidente <gfidente@redhat.com>
@dsavineau
Copy link
Contributor

jenkins test pipeline

@dsavineau
Copy link
Contributor

mergify failed to merge, doing it manually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants