Update NGINX base image to v1.19

[tao12345666333]

Signed-off-by: Jintao Zhang [email protected]

What this PR does / why we need it:

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Which issue/s this PR fixes

How Has This Been Tested?

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I've read the CONTRIBUTION guide
• I have added tests to cover my changes.
• All new and existing tests passed.

[k8s-ci-robot]

@tao12345666333: 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.

[rikatz]

/lgtm
/approve
Thanks

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rikatz, tao12345666333

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rikatz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[BloodyIron]

Hey so why was there no justification put at all in the original merge, or any of the discussion here? Merging this actually reverted the NGINX base version from 1.20.1 to 1.19.1 and this has opened ingress-nginx back up to CVE-2021-23017 which was previously addressed in #7164 so again, why was this regression merged mainline? Users upgrading to 0.49.1 and newer now are actually less secure than before... was this even considered or evaluated??? I see no evidence of such.

[rikatz]

We wont move to nginx v1.20 right now, as this is not supported by Openresty (https://openresty.org/en/ann-1019009001.html) which we use a lot.

About the CVE, it's patched in our base NGINX image (https://github.com/kubernetes/ingress-nginx/blob/main/images/nginx/rootfs/patches/patch.2021.resolver.txt) and https://github.com/kubernetes/ingress-nginx/blob/main/images/nginx/rootfs/build.sh#L608 so this should not be a concern.

[BloodyIron]

@rikatz what is the currently anticipated timeline for bringing NGINX base v1.20.x (or higher) into ingress-nginx? (Based on what you say about openresty not supporting it).

[BloodyIron]

I would also like to add that back-porting the security code to address that CVE into your forked versions of NGINX base 1.19.x makes it rather hard to tell when the "secure" version of the code is in use, vs not-in-use and is just regular-old NGINX base 1.19.x. Having it clear-cut that ingress-nginx is using NGINX base 1.20.1 is orders of magnitude more simple to determine if the CVE is mitigated or not (from an "outsider's" perspective).