Add missing 'https://' to cifmw_ceph_rgw_keystone_ep #1795
Conversation
|
Thanks for the PR! ❤️ |
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/500203bcc1954e6dbc258b2a57e2614c ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 02m 22s |
|
recheck |
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/389777511f8648849cb21bb7596bd34c ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 05m 33s |
| @@ -66,7 +66,7 @@ cifmw_cephadm_pacific_filter: "16.*" | |||
| # The path of the rendered rgw spec file | |||
| cifmw_ceph_rgw_spec_path: /tmp/ceph_rgw.yml | |||
| cifmw_ceph_mds_spec_path: /tmp/ceph_mds.yml | |||
| cifmw_ceph_rgw_keystone_ep: "keystone-internal.openstack.svc:5000" | |||
| cifmw_ceph_rgw_keystone_ep: "https://keystone-internal.openstack.svc:5000" | |||
There was a problem hiding this comment.
https here is added to the internal keystone endpoint: if tls is not enabled at Pod level (podLevel: true in the ctlplane) but the ingress is configured as an edge router [1] this will fail.
While I think https should be the default, we should isolate the prefix in it's own variable.
I'm ok to follow up on that, but I would try something like:
cifmw_ceph_rgw_keystone_ep: "{{ cifmw_ceph_rgw_keystone_urischeme | default("https") }}:keystone-internal.openstack.svc:5000"
so we can make it configurable w/o introducing extra complexity.
[1] https://www.redhat.com/architect/encryption-secure-routes-openshift
There was a problem hiding this comment.
However, considering you can override the entire variable, my comment is not a blocker for the patch, more of a consideration.
So let me know if you want to keep it as it is and I can approve.
|
Let's rebase/recheck and merge this patch to and see how CI behaves. I'm not sure it's enough to solve the current problem but given we have jobs w/ tls enabled at pod level I think we need this. |
The default value for the cifmw_ceph_rgw_keystone_ep variable in the role cifmw_cephadm was missing the https:// part of the keystone endpoint URL so RGW was not getting configured with a correct keystone endpoint. Signed-off-by: John Fulton <fulton@redhat.com>
fultonj
force-pushed
the
ceph_rgw_keystone_http
branch
from
7839e2f to
6b0ad23
Compare
|
/lgtm |
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/e47e0526cb45484d9840ac1b7591edbd ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 35m 49s |
|
I have tested this patch in my local environment and I confirm it allows me to use RGW with recheck |
|
recheck |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rebtoor The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit d638f7d
into
openstack-k8s-operators:main
The default value for the
cifmw_ceph_rgw_keystone_epvariable in the rolecifmw_cephadmwas missing thehttps://part of the keystone endpoint URL so RGW was not getting configured with a correct keystone endpoint.As a pull request owner and reviewers, I checked that: