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 http2 support in template.yml #405

Closed
wants to merge 3 commits into from
Closed

Add http2 support in template.yml #405

wants to merge 3 commits into from

Conversation

orlovmyk
Copy link

@orlovmyk orlovmyk commented Feb 27, 2024

This works, I've tested, resulting config will be

server {
    listen 443 ssl http2;
}

with http2: true

Proposed changes

Describe the use case and detail of the change. If this PR addresses an issue on GitHub, make sure to include a link to that issue using one of the supported keywords here in this description (not in the title of the PR).

Checklist

Before creating a PR, run through this checklist and mark each as complete:

  • I have read the CONTRIBUTING document.
  • I have added Molecule tests that prove my fix is effective or that my feature works.
  • I have checked that any relevant Molecule tests pass after adding my changes.
  • I have updated any relevant documentation (defaults/main/*.yml, README.md and CHANGELOG.md).

This works, I've tested, resulting config will be

```
server {
    listen 443 ssl http2;
}
```
with `http2: true`
@alessfg alessfg added the enhancement Enhance an existing feature label Feb 29, 2024
@alessfg alessfg added this to the 0.7.2 milestone Feb 29, 2024
Copy link
Collaborator

@alessfg alessfg left a comment

Choose a reason for hiding this comment

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

Hey @orlovmyk!

The http2 parameter on the listen directive is deprecated (see https://nginx.org/en/docs/http/ngx_http_core_module.html#listen), and it was removed in #346. That being said, it's true that the change has only made it's way to the mainline branch so far, so it makes sense to bring it back for the time being.

That being said, I don't quite think the parameter will work as is. If you see the current template on main, you'll see the http2 parameter for the listen directive is not there anymore https://github.com/nginxinc/ansible-role-nginx-config/blob/main/templates/http/core.j2#L139-L159. You can add it back with a couple changes by merging the change here https://github.com/nginxinc/ansible-role-nginx-config/blob/main/templates/http/core.j2#L139-L159 with the quic parameter (you can configure either http2 or quic). Once you do that, can you also add the parameter to the molecule test (you can easily revert the change made here to do that https://github.com/nginxinc/ansible-role-nginx-config/pull/346/files#diff-bc4d0c4551c2773138cb727ed62a97e7e37f37731b8f6a2c8fe715ae136fdfb6L551). Finally, I'd appreciate if you could update the changelog.

Thanks!

P.S: The CI/CD pipeline is failing atm but I'll hopefully have a fix for that early next week 😄

@orlovmyk
Copy link
Author

orlovmyk commented Mar 5, 2024

@alessfg You can close it as far it's deprectaed. I guess someone else struggling will just find this issue
Thanks

@marcelaraujo
Copy link

marcelaraujo commented Mar 15, 2024

This is not working on Ubuntu 20.04. If I enabled the https2 directive, the ansible will generate the http2 on instead of adding the https2 into the listen directive.

The given error

unknown directive \"http2\" in /etc/nginx/sites-enabled/default:61"

I installed with Nginx role

- name: Install Nginx
  hosts: all
  become: yes
  gather_facts: true
  collections:
    - nginxinc.nginx
  roles:
    - role: nginx
      vars:
        nginx_install_from: os_repository
        nginx_modules: []
> uname -a

> Linux proxy 6.2.0-1017-aws #17~22.04.1-Ubuntu SMP Fri Nov 17 21:07:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
apt info nginx

Package: nginx
Version: 1.18.0-6ubuntu14.4
Priority: optional
Section: web
Origin: Ubuntu
Maintainer: Ubuntu Developers <[email protected]>
Original-Maintainer: Debian Nginx Maintainers <[email protected]>
Bugs: https://bugs.launchpad.net/ubuntu/+filebug
Installed-Size: 50.2 kB
Depends: nginx-core (<< 1.18.0-6ubuntu14.4.1~) | nginx-full (<< 1.18.0-6ubuntu14.4.1~) | nginx-light (<< 1.18.0-6ubuntu14.4.1~) | nginx-extras (<< 1.18.0-6ubuntu14.4.1~), nginx-core (>= 1.18.0-6ubuntu14.4) | nginx-full (>= 1.18.0-6ubuntu14.4) | nginx-light (>= 1.18.0-6ubuntu14.4) | nginx-extras (>= 1.18.0-6ubuntu14.4)
Breaks: libnginx-mod-http-lua (<< 1.18.0-6ubuntu5)
Homepage: https://nginx.net
Download-Size: 3872 B
APT-Manual-Installed: yes
APT-Sources: http://us-west-2.ec2.archive.ubuntu.com/ubuntu jammy-updates/main amd64 Packages
Description: small, powerful, scalable web/proxy server
 Nginx ("engine X") is a high-performance web and reverse proxy server
 created by Igor Sysoev. It can be used both as a standalone web server
 and as a proxy to reduce the load on back-end HTTP or mail servers.
 .
 This is a dependency package to install either nginx-core (by default),
 nginx-full, nginx-light or nginx-extras.

N: There are 2 additional records. Please use the '-a' switch to see them.
nginx -V

nginx version: nginx/1.18.0 (Ubuntu)
built with OpenSSL 3.0.2 15 Mar 2022
TLS SNI support enabled
configure arguments: --with-cc-opt='-g -O2 -ffile-prefix-map=/build/nginx-zctdR4/nginx-1.18.0=. -flto=auto -ffat-lto-objects -flto=auto -ffat-lto-objects -fstack-protector-strong -Wformat -Werror=format-security -fPIC -Wdate-time -D_FORTIFY_SOURCE=2' --with-ld-opt='-Wl,-Bsymbolic-functions -flto=auto -ffat-lto-objects -flto=auto -Wl,-z,relro -Wl,-z,now -fPIC' --prefix=/usr/share/nginx --conf-path=/etc/nginx/nginx.conf --http-log-path=/var/log/nginx/access.log --error-log-path=/var/log/nginx/error.log --lock-path=/var/lock/nginx.lock --pid-path=/run/nginx.pid --modules-path=/usr/lib/nginx/modules --http-client-body-temp-path=/var/lib/nginx/body --http-fastcgi-temp-path=/var/lib/nginx/fastcgi --http-proxy-temp-path=/var/lib/nginx/proxy --http-scgi-temp-path=/var/lib/nginx/scgi --http-uwsgi-temp-path=/var/lib/nginx/uwsgi --with-compat --with-debug --with-pcre-jit --with-http_ssl_module --with-http_stub_status_module --with-http_realip_module --with-http_auth_request_module --with-http_v2_module --with-http_dav_module --with-http_slice_module --with-threads --add-dynamic-module=/build/nginx-zctdR4/nginx-1.18.0/debian/modules/http-geoip2 --with-http_addition_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_sub_module

@alessfg
Copy link
Collaborator

alessfg commented Mar 15, 2024

You are using the os_repository option which in turn is installing a very outdated version of nginx, 1.18.0. The latest version is 1.25.4 (or 1.24.0 if you are using the stable NGINX branch). You'd have to use the old style of http2 directive, which is not supported by this role until the changes discussed in this PR are reverted 😄

@marcelaraujo
Copy link

Let me check this @alessfg

@k0ste
Copy link

k0ste commented May 17, 2024

Currently, the CentOS stream have maximum the 1.24 version

[root@host]# dnf module list nginx
Last metadata expiration check: 0:01:33 ago on Fri 17 May 2024 07:19:53 PM +07.
CentOS Stream 8 - AppStream
Name                           Stream                            Profiles                           Summary
nginx                          1.14 [d]                          common [d]                         nginx webserver
nginx                          1.16                              common [d]                         nginx webserver
nginx                          1.18                              common [d]                         nginx webserver
nginx                          1.20                              common [d]                         nginx webserver
nginx                          1.22                              common                             nginx webserver
nginx                          1.24 [e]                          common [d]                         nginx webserver

Hint: [d]efault, [e]nabled, [x]disabled, [i]nstalled

I was resolved this issue with custom_directives

---
nginx_config_http_template_enable: true
nginx_config_http_template:
- template_file: 'http/default.conf.j2'
  deployment_location: '/etc/nginx/conf.d/http_vhosts.conf'
  backup: false
  config:
    servers:
      - core:
          root: '/srv/http'
          server_name: "{{ inventory_hostname }}"
        custom_directives:
          - "{{ 'listen ' + ansible_host + ':80 reuseport http2;' }}"

@alessfg
Copy link
Collaborator

alessfg commented May 27, 2024

If I might suggest, and unless you 100% need to use CentOS 8, I would suggest switching to one of the officially supported distros and using the latest release (1.25.5 for mainline, 1.26.0 for stable) 😄

Copy link

github-actions bot commented May 29, 2024

✅ All required contributors have signed the F5 CLA for this PR. Thank you!
Posted by the CLA Assistant Lite bot.

@orlovmyk
Copy link
Author

I have hereby read the F5 CLA and agree to its terms

@orlovmyk orlovmyk closed this by deleting the head repository Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhance an existing feature
Projects
Status: Review in progress
Development

Successfully merging this pull request may close these issues.

4 participants