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

feat: Ubuntu noble #290

Merged
merged 31 commits into from
Jun 21, 2024
Merged

feat: Ubuntu noble #290

merged 31 commits into from
Jun 21, 2024

Conversation

mattwillsher
Copy link
Member

@mattwillsher mattwillsher commented Jun 3, 2024

Enhancement:

  • Added Ubuntu 24.04/Noble support
  • Removed CentOS 6 & 8 tests due to them being EOL
  • Fix a bunch of test issues

Reason:

Result:

Issue Tracker Tickets (Jira or BZ if any):

@mattwillsher mattwillsher changed the title Feature: Ubuntu noble feat: Ubuntu noble Jun 4, 2024
@mattwillsher
Copy link
Member Author

Hi @Jakuje

Current failure is down to the sockets systemd unit file. Ubuntu 24.04 default is:

[Unit]
Description=OpenBSD Secure Shell server socket
Before=sockets.target ssh.service
ConditionPathExists=!/etc/ssh/sshd_not_to_be_run

[Socket]
ListenStream=22
Accept=no
FreeBind=yes

[Install]
WantedBy=sockets.target
RequiredBy=ssh.service

Our template is

[Unit]
Description=OpenBSD Secure Shell server socket
Documentation=man:sshd(8) man:sshd_config(5)
{% if __sshd_socket_accept %}
Conflicts={{ sshd_service }}.service
{% else %}
Before=sockets.target
{% endif %}

[Socket]
ListenStream=22
{% if __sshd_socket_accept %}
Accept=yes
{% else %}
Accept=no
{% endif %}

[Install]
WantedBy=sockets.target

Options I can see are:

  • Add conditions for Ubuntu 24.04 to the template
  • Convert code to clone vendor provided sshd.socket and use lineinfile to replace the ones we want to control
  • Generate a systemd override file rather than shadowing the entire file - I've only had a cursory look at this, so maybe this isn't a viable option?

What are your thoughts?

(P.S. No more PRs. the misspelling of noble on the branch was annoying me and source branch can't be changed on a PR)

@Jakuje
Copy link
Collaborator

Jakuje commented Jun 6, 2024

Hi @Jakuje

Current failure is down to the sockets systemd unit file. Ubuntu 24.04 default is:

Thank you for having a look into this!

[Unit]
Description=OpenBSD Secure Shell server socket
Before=sockets.target ssh.service
ConditionPathExists=!/etc/ssh/sshd_not_to_be_run

[Socket]
ListenStream=22
Accept=no
FreeBind=yes

[Install]
WantedBy=sockets.target
RequiredBy=ssh.service

I am wondering how it even works ... Its already some time since I was maintaining the OpenSSH, but this sounds like both ssh.service and ssh.socket is enabled which in the past caused random failures (whether systemd or openssh got hold of the port 22 first). But this seems to be solved by ordering the socket first with RequiredBy and also the FreeBind which will allow binding to the addresses that are not up yet ...

Our template is

[Unit]
Description=OpenBSD Secure Shell server socket
Documentation=man:sshd(8) man:sshd_config(5)
{% if __sshd_socket_accept %}
Conflicts={{ sshd_service }}.service
{% else %}
Before=sockets.target
{% endif %}

[Socket]
ListenStream=22
{% if __sshd_socket_accept %}
Accept=yes
{% else %}
Accept=no
{% endif %}

[Install]
WantedBy=sockets.target

Options I can see are:

* Add conditions for Ubuntu 24.04 to the template

I think that would be preferred. If I see right, we need ConditionPathExists, RequiredBy, FreeBind as well as adjusting the Before.

* Convert code to clone vendor provided sshd.socket and use lineinfile to replace the ones we want to control

This could do as we are writing the files in /etc/ and the systemd has the package-installed service files in /usr/lib/systemd/system/ssh.service. But we would have to depend on these being intact on the target system, which is something I wanted to avoid by keeping the results independent on the potentially/partially broken systems.

* Generate a systemd override file rather than shadowing the entire file - I've only had a cursory look at this, so maybe this isn't a viable option?

The override works ok when we want to modify some features in existing service, but if we want to set up separate sshd service with different name and different configuration, we still need to generate the full unit file anyway (I think). So we would need to have to implement both options anyway.

@Jakuje
Copy link
Collaborator

Jakuje commented Jun 6, 2024

Regarding the CentOS 8, I got roles-ansible/check-ansible-centos-centos8-action#7 merged so it should keep working with the archive repositories. Certainly not for production use, but it should be good enough for CI use.

Your PR for the ubuntu was also merged to the actions so you can revert that / drop that patch too when you will be doing next changes.

@mattwillsher mattwillsher marked this pull request as ready for review June 11, 2024 01:34
@mattwillsher mattwillsher requested a review from Jakuje June 11, 2024 01:47
@mattwillsher
Copy link
Member Author

@Jakuje I think this is about there. I've disable the tests for the ssh@.service as 24.04 doesn't have the file in it's package. Please review.

README.md Outdated Show resolved Hide resolved
meta/main.yml Outdated Show resolved Hide resolved
templates/sshd.socket.j2 Outdated Show resolved Hide resolved
tests/tests_systemd_services.yml Outdated Show resolved Hide resolved
tests/tests_systemd_services.yml Outdated Show resolved Hide resolved
mattwillsher and others added 5 commits June 11, 2024 17:46
Co-authored-by: Jakub Jelen <jjelen@redhat.com>
Co-authored-by: Jakub Jelen <jjelen@redhat.com>
vars/Ubuntu_24.yml Outdated Show resolved Hide resolved
@mattwillsher mattwillsher marked this pull request as draft June 13, 2024 21:31
@mattwillsher mattwillsher self-assigned this Jun 13, 2024
Copy link
Collaborator

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

lgtm!

@mattwillsher mattwillsher marked this pull request as ready for review June 21, 2024 08:08
@mattwillsher mattwillsher merged commit d4eae95 into main Jun 21, 2024
30 checks passed
@mattwillsher mattwillsher deleted the feature/ubuntu-noble branch June 21, 2024 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants