-
Notifications
You must be signed in to change notification settings - Fork 15
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
work in progress: ssl configuration for Tendrl [blocked] #46
Conversation
lineinfile: | ||
path: /etc/httpd/conf.d/tendrl.conf | ||
regexp: ' *#? *Redirect permanent / https://.*/' | ||
line: " Redirect permanent / https://{{ httpd_ip_address }}/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, there should be FQDN, instead of IP address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, let's discuss it here: Tendrl/api#302
- name: Validate ansible variables tweaking apache configuration | ||
assert: | ||
that: | ||
- lookup('dig', ansible_fqdn) == httpd_ip_address |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dahorak here it is
- name: Configure VirtualHost ServerName in tendrl-ssl.conf | ||
lineinfile: | ||
path: /etc/httpd/conf.d/tendrl-ssl.conf | ||
insertafter: '<VirtualHost .*:443>' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason not to use just <VirtualHost *:443>
(or <VirtualHost _default_:443>
) - simply do not specify any IP address?
From my point of view, it make sense to serve Tendrl related pages on all available IPs/interfaces on the Tendrl server and if somebody want's to restrict it, he or she should know why and how to do it... But default could be with _default_
(or *
) and it might avoid some unexpected issues with Tendrl server connected into multiple networks or with access from localhost...
If it make sense, it might be worth to change also the default in the tendrl-api package (https://github.com/Tendrl/api/blob/master/config/apache.vhost-ssl.sample#L8).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brainfunked @anivargi see previous comment from @dahorak
@dahorak your solution is correct. Ideally, we should use the default vhost for ssl. However, that requires the vhost in the ssl configuration file shipped with the package to be modified. In the long run, this would involve tracking changes to the file in every rpm update. The idea behind the sample configuration files is that they could be dropped into an existing apache setup without having to modify any of the default configuration files shipped in various packages. |
@dahorak I rechecked your suggestion and when I modify the
The appache starts serving me testing page instead of tendrl web ui ... so I don't think that is a good idea. |
@mbukatov yep, I understand it from the previous comment. To be able to use this approach, we should modify the default config file shipped with mod_ssl package... which is not good idea. |
Are you still working on this ? |
@r0h4n I'm not working on this right now, and I don't plan to work on this during the time our team is focused on dowsntream work. When I have time on the upstream work again, I will need to test multiple possible configurations to test which one makes sense, rebase this pull request and restart the upstream review. |
Status update: this is not part of short term work plan (next 2 milestones) right now. |
Implementing #30