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

Proposal - Deprecate AJP support #10098

Closed
rikatz opened this issue Jun 18, 2023 · 6 comments · Fixed by #10158
Closed

Proposal - Deprecate AJP support #10098

rikatz opened this issue Jun 18, 2023 · 6 comments · Fixed by #10158
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@rikatz
Copy link
Contributor

rikatz commented Jun 18, 2023

AJP has been on Ingress NGINX since 2018. For those who don't know it, it is a binary protocol usually used to communicate with Java backends (Tomcat, Jboss, etc).

The fact is that while AJP is really useful on some specific scenarios (binary communications, huge headers, etc) it is not a really used feature (I couldn't see people doing comments on it while using ingress).

To use AJP we have a dependency on a module that is written in C, not updated since 2021. It is a security concern for the project. Also, usually the Java backends that rely on AJP can also use HTTP without any big issues, unless you really need some specific AJP feature.

My proposal is to remove the AJP support on the next minor release (1.9) and reduce our compilation scope, the amount of annotation and codes, etc.

Please add a +1 or -1 but also leave your comment on why!

Thanks!

@rikatz rikatz added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 18, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jun 18, 2023
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@diogomurta
Copy link

I agree to remove the feature from ajp. I don't see many users in the community using it and keeping unused functionality is not worth keeping in the project.

@mimmus
Copy link

mimmus commented Jun 18, 2023

Used a lot in the past, with Tomcat backends, but no more actually used on Kubernetes.

@rikatz rikatz self-assigned this Jun 18, 2023
@vojtechmares
Copy link

Hello,
until today, I had no idea a such feature is supported by Ingress NGINX or even nginx itself (I come from a Javaless world). For me it is completely fine to remove it.

Given your arguments and the general concern over the C library, it is more then reasonable to remove it IMHO.

@tao12345666333
Copy link
Member

As I wrote in the comment, we used a fork version of the AJP module because the original module has not yet added support for NGINX v1.21 and the project is inactive. yaoweibin/nginx_ajp_module#52

# Check for recent changes: https://github.com/msva/nginx_ajp_module/compare/fcbb2ccca4901d317ecd7a9dabb3fec9378ff40f...master
# This is a fork from https://github.com/yaoweibin/nginx_ajp_module
# Since it has not been updated and is not compatible with NGINX 1.21
export NGINX_AJP_VERSION=fcbb2ccca4901d317ecd7a9dabb3fec9378ff40f

I'm +1 for removing this feature.

@aimuz
Copy link
Contributor

aimuz commented Jun 26, 2023

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants