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

Enable PROXY procotol for proxy hosts and streams #3618

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

snordmann
Copy link

@snordmann snordmann commented Mar 10, 2024

As I see not too much progress in #3537, I decided to copy most of the code and (try to) fix the tests.
In this PR I also included the code necessary to enable the PROXY protocol for Streams.

Thank you @jwklijnsma for the previous work

@snordmann snordmann changed the title Enable PROXY procotol for proxy hosts Enable PROXY procotol for proxy hosts and streams Mar 10, 2024
@jc21
Copy link
Member

jc21 commented Mar 10, 2024

CI is reporting:

Integration Tests / Mysql / Hosts endpoints.Should be able to create a http host

Error Message:
data should NOT have additional properties, data should have required property 'proxy_support_enabled'

Cypress output:

[validateSwaggerSchema DEBUG] Endpoint: /nginx/proxy-hosts
[validateSwaggerSchema DEBUG] Response Schema: {
  "id": 1,
  "created_on": "2024-03-10T10:06:30.000Z",
  "modified_on": "2024-03-10T10:06:30.000Z",
  "owner_user_id": 1,
  "domain_names": [
    "test.example.com"
  ],
  "forward_host": "1.1.1.1",
  "forward_port": 80,
  "access_list_id": 0,
  "certificate_id": 0,
  "ssl_forced": 0,
  "caching_enabled": 0,
  "block_exploits": 0,
  "advanced_config": "",
  "meta": {
    "letsencrypt_agree": false,
    "dns_challenge": false
  },
  "allow_websocket_upgrade": 0,
  "http2_support": 0,
  "forward_scheme": "http",
  "enabled": 1,
  "locations": [],
  "hsts_enabled": 0,
  "hsts_subdomains": 0,
  "enable_proxy_protocol": 0,
  "load_balancer_ip": "",
  "certificate": null,
  "owner": {
    "id": 1,
    "created_on": "2024-03-10T10:06:18.000Z",
    "modified_on": "2024-03-10T10:06:18.000Z",
    "is_deleted": 0,
    "is_disabled": 0,
    "email": "admin@example.com",
    "name": "Administrator",
    "nickname": "Admin",
    "avatar": "",
    "roles": [
      "admin"
    ]
  },
  "access_list": null,
  "use_default_location": true,
  "ipv6": true
}
[validateSwaggerSchema ERROR] data should NOT have additional properties, data should have required property 'proxy_support_enabled'

@snordmann
Copy link
Author

One question I do have: How should we show the user, that udp and proxy_protocol are exclusive options on a listener? Currently I just silently drop the proxy_protocol option for the UDP listener, but I am unsure that this is a good implementation.

I imagine that we could also disable the UDP button, when "Enable Proxy Protocol" is pressed.

@nginxproxymanagerci
Copy link

Docker Image for build 6 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-3618

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

@jwklijnsma
Copy link

jwklijnsma commented Mar 10, 2024

As I see not too much progress in #3537, I decided to copy most of the code and (try to) fix the tests. In this PR I also included the code necessary to enable the PROXY protocol for Streams.

Thank you @jwklijnsma for the previous work but thnx for it

The plan was to fix the test next week

@jwklijnsma
Copy link

what needs be done for mr @jc21

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.

None yet

3 participants