introduce switch to disable ipv6 on nginx #1024
Conversation
|
Any update on this issue? |
|
Not from my side. @shanemcd asked me to open a PR and I did. If I something is missing from my side I am happy to add it. |
|
Hey guys, what is the status of this request? Really interested to have it merged. Thanks! |
|
Can this be bumped in importance or something alike? I think the default IPV6 listening is breaking AWX in a lot of non-ipv6 environments. Thanks! |
The maintainers of awx are all Redhat employees, they really don't check pull requests and usually take 6-12months if ever. This is an open source project controlled by RedHat so it is just the code that is open source everything else is done by RedHat/IBM rules and times. Sorry for the rant, but just so you don't waste to much time waiting for a fix and just fix it on your side because this PR would probable never come out and they probably won't care unless Tower clients demand it. |
Thanks for the reply. No worries about the rant, I understand your frustration. We have worked around it in our environment for now, but still a shame to see that this Pull Request just has to wait in a void. |
| @@ -6,6 +6,9 @@ api_version: '{{ deployment_type }}.ansible.com/v1beta1' | |||
| database_name: "{{ deployment_type }}" | |||
| database_username: "{{ deployment_type }}" | |||
|
|
|||
| # Enable nginx IPv6 binding on default | |||
| ipv6_disabled: no | |||
There was a problem hiding this comment.
This would need to be configurable via the AWX spec. In it's current state, I don't see how this PR would allow users to toggle this setting (for example if they have an environment that only allows ipv6).
@digifuchsi To do so, you can add the field to the AWX Custom Resource Definition. Here is an example of a similar boolean setting:
You'll also want to add an entry to the UI param fields so that this shows up correctly when installing from OperatorHub, for example:
There was a problem hiding this comment.
You'll also need to rebase to resolve the conflicts on this PR before merging.
@digifuchsi @MaxVolum3 @koshrf I am sorry this went so long without a review. We now have weekly triage meetings for community PR's, so contributions like this will not sit for so long in the future.
There was a problem hiding this comment.
@digifuchsi aside from that, this PR looks good. With those changes we should be able to get this merged. Please drop me a ping here when you've added those changes.
There was a problem hiding this comment.
I would like to see this change made, so I opened a separate PR with the changes you requested. Apologies if this is bad practice
See: #1135
There was a problem hiding this comment.
@dale-mittleman thank you for fixing conflicts and adding the entries to the CRD and CSV template. This is now merged via #1135
|
The necessary updates were made and merged in this PR: #1135 Closing as this is now fixed. Thanks! |
SUMMARY
Introduces a switch to disable IPv6 in nginx if you have an environment without IPv6 enabled
Fixes #1017
ISSUE TYPE
ADDITIONAL INFORMATION