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 support for custom ports #38
Changes from 35 commits
050939f
01bf854
23c512f
59aa205
eea9d5e
99cd26d
f91df23
42af42a
740d068
81d01c5
9d6defb
b7b3c71
bfa7193
1295970
2144329
bf8d525
aa07654
05c94b7
677c1f7
51b443a
9fac310
1c81ed7
91272a8
208cf90
638c75a
487f868
80c4b01
7530e3f
1327743
71644f7
a01a3bb
a9aff14
2cd1a28
1a26448
cd3fb90
efb96d3
6f22e1c
c112230
344462b
b515a68
ee98762
0b32b1e
0dbc506
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
--- | ||
collections: | ||
- name: fedora.linux_system_roles |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
--- | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are changing the way we use system roles from other system roles - https://linux-system-roles.github.io/documentation/role-requirements.html The short answer is
collections:
- fedora.linux_system_roles
- name: Allow the custom port for tangd_port_t in SELinux
import_role:
name: fedora.linux_system_roles.selinux
vars:
... |
||
# This task tells SELinux for the port that we want the tangd | ||
# service to use when distribution Fedora | ||
- name: Allow the custom port for tangd_port_t in SELinux | ||
import_role: | ||
name: fedora.linux_system_roles.selinux | ||
vars: | ||
selinux_ports: | ||
- ports: "{{ nbde_server_port }}" | ||
proto: tcp | ||
setype: tangd_port_t | ||
state: present | ||
|
||
# This block creates the override file for systemd with the new | ||
# port that we have requested | ||
- name: Create override file | ||
block: | ||
# This task checks if the directory /etc/systemd/system/tangd.socket.d | ||
# exist and registers the value in systemd__system__tangd_socket | ||
- name: Check if directory /etc/systemd/system/tangd.socket.d exist | ||
stat: | ||
path: /etc/systemd/system/tangd.socket.d | ||
register: systemd_system_tangd_socket | ||
|
||
# This tasks Create the /etc/systemd/system/tangd.socket.d directory if | ||
# it does not exist | ||
- name: Create a directory if it does not exist | ||
file: | ||
path: /etc/systemd/system/tangd.socket.d | ||
state: directory | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will already create the directory if it does not exist, no? There is no need to check/register in the previous task and run this conditionally here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
mode: '0755' | ||
when: systemd_system_tangd_socket.stat.exists | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We want to create the directory when it does not exist?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ops ... you are correct. I've fixed the problems and I'm pushing it right now. |
||
|
||
# This tasks creates the file with the port entry that we want tangd to | ||
# listen to | ||
- name: Creates the file with the port entry that we want tangd to listen to | ||
template: | ||
src: tangd_socket_override.conf.j2 | ||
dest: /etc/systemd/system/tangd.socket.d/override.conf | ||
backup: true | ||
mode: '0644' | ||
register: __nbde_server_daemon_reload | ||
|
||
# This tasks reload the daemons so the new changes take effect | ||
- name: Reload the daemons so the new changes take effect | ||
systemd: | ||
daemon_reload: true | ||
when: __nbde_server_daemon_reload is changed | ||
|
||
- name: Ensure the desired port is added to firewalld | ||
import_role: | ||
name: fedora.linux_system_roles.firewall | ||
vars: | ||
firewall: | ||
- port: "{{ nbde_server_port }}/tcp" | ||
zone: "{{ nbde_server_firewall_zone }}" | ||
state: enabled | ||
immediate: true | ||
permanent: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{{ ansible_managed | comment }} | ||
[Socket] | ||
ListenStream= | ||
ListenStream={{ nbde_server_port }} |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,48 @@ | ||||||||
--- | ||||||||
- name: Test tangd_custom_port | ||||||||
hosts: all | ||||||||
vars: | ||||||||
nbde_server_port: 7500 | ||||||||
nbde_server_firewall_zone: public | ||||||||
tasks: | ||||||||
- name: install with custom port and firewall zone | ||||||||
import_role: | ||||||||
name: fedora.linux-system-roles.nbde_server | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd think
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right - should be |
||||||||
|
||||||||
- name: check if port is open | ||||||||
shell: | ||||||||
cmd: |- | ||||||||
set -euo pipefail | ||||||||
ss -tulpn | grep {{ nbde_server_port }} | awk -F' ' '{print $5}' | ||||||||
register: __open_ports_output | ||||||||
failed_when: __open_ports_output.stdout != "*:{{ nbde_server_port }}" | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sergio-correia, @lessfoobar, @richm, how must this right-hand side be exactly matched?
Can we consider it is successful if the ports match? Please note that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
|
||||||||
- name: check if port TCP is open | ||||||||
shell: | ||||||||
cmd: |- | ||||||||
set -euo pipefail | ||||||||
ss -tulpn | grep {{ nbde_server_port }} | awk -F' ' '{print $1}' | ||||||||
register: __open_ports_output | ||||||||
failed_when: __open_ports_output.stdout != "tcp" | ||||||||
|
||||||||
- name: check if port is opened in firewall | ||||||||
shell: | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Then you don't need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah ... really not sure why I didn't went for the firewall-cmd command ... glad you found that out. Way more simpler than my solution. |
||||||||
cmd: |- | ||||||||
set -euo pipefail | ||||||||
firewall-cmd --list-all | grep {{ nbde_server_port }}/tcp | \ | ||||||||
awk -F':' '{print $2}' | \ | ||||||||
awk '{gsub(/^[ \t]+| [ \t]+$/,""); print$0}' | ||||||||
register: __firewall_output | ||||||||
failed_when: __firewall_output.stdout != "{{ nbde_server_port }}/tcp" | ||||||||
|
||||||||
- name: check if firewall zone is set | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think you need this check at all - the |
||||||||
shell: | ||||||||
cmd: |- | ||||||||
set -euo pipefail | ||||||||
firewall-cmd --list-all | grep {{ nbde_server_firewall_zone }} | \ | ||||||||
awk -F' ' '{print $1}' | ||||||||
register: __firewall_output_zone | ||||||||
failed_when: >- | ||||||||
__firewall_output_zone.stdout != "{{ nbde_server_firewall_zone }}" | ||||||||
|
||||||||
# vim:set ts=2 sw=2 et: |
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.
(nbde_server_port | int != 80)
nbde_server_port
can be a string or an int - ifnbde_server_port
is an int e.g.nbde_server_port: 80
then80
is not equal to"80"
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've had so much problems with mode and always quoting it, so for me by default all numbers should be quoted. But sure I can fix that also.
ansible/ansible#9196
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.
If you want to keep everything a string, you can do that too:
(nbde_server_port | string != "80")
But in the examples you added, you are using
int
instead ofstring
e.g. https://github.com/linux-system-roles/nbde_server/pull/38/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R37 , https://github.com/linux-system-roles/nbde_server/pull/38/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R124, etc.So whatever you decide to do, please be consistent