Remove default security context from gateway injection

[ethanchowell]

Please provide a description of this PR:

I recently tried upgrading from Istio 1.19.4 to 1.20.3 and was unable to because of the reasons outlined in #49549. This PR removes the default incompatible kernel settings so users still on 3.x based kernels can deploy gateways.

[istio-policy-bot]

😊 Welcome @ethanchowell! This is either your first contribution to the Istio istio repo, or it's been
a while since you've been here.

You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

[istio-testing]

Hi @ethanchowell. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[hanxiaop]

IIRC, the PR that added this was merged recently. Deleting the security context will introduce a regression I think. Instead, we may be able to make this optional in case you have your own settings.

[ethanchowell]

If it were optional, I think it would solve my specific use case since I'm already providing a securityContext but wouldn't benefit anyone else limited by a 3.x based kernel who isn't already specifying a securityContext, which IMO makes this setting a regression either way. Then to make Istio usable for those users, they either still have to upgrade their OS, or have something in the docs roughly equivalent to "just set something so the gateway doesn't take privileges on your kernel settings". On the other hand by removing this, you could already provide a securityContext through existing installation methods, so users that needed to directly use port 80 or 443 for example could set this as needed, or alternatively map a port > 1024 in the service overrides

[hanxiaop]

cc related folks to take a look @hemendrateli @howardjohn

[hemendrateli]

I am ok with both ways.
Kernel version 4.11 was released around 7 years back (which added net.ipv4.ip_unprivileged_port_start feature). I am not sure how many users still are on kernel < 4.11

If we want to merge this PR then these kind of users will be broken if they were using port < 1024.

so in any way I think we should update documentation also about broken users.

[howardjohn]

so users that needed to directly use port 80 or 443 for example could set this as needed, or alternatively map a port > 1024 in the service overrides.

I would very much prefer to make people binding to port 80/443 (99.99% of users) have a smoother experience than users running >7 year old kernels, if we have to make that tradeoff.

[bleggett]

From the original issue:

I'm currently setting my securityContext anyway, which I would expect the chart to honor vs merging the two.

☝️ that is also what I would expect to happen, and that is how I would prefer this to work, we cannot remove securityContext for everyone.

[nicole-lihui]

IMO， These configurations should be the same as down：

istio/manifests/charts/gateway/templates/deployment.yaml

Lines 38 to 45 in f653b34

	{{- if .Values.securityContext }}
	{{- toYaml .Values.securityContext | nindent 8 }}
	{{- else }}
	# Safe since 1.22: https://github.com/kubernetes/kubernetes/pull/103326
	sysctls:
	- name: net.ipv4.ip_unprivileged_port_start
	value: "0"
	{{- end }}

Instead of forcing defaults everywhere, users can't use valuese support it at all

[hanxiaop]

/ok-to-test

[istio-testing]

@ethanchowell: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
release-notes_istio	b13160d	link	true	/test release-notes
unit-tests-arm64_istio	b13160d	link	true	/test unit-tests-arm64
unit-tests_istio	b13160d	link	true	/test unit-tests
gencheck_istio	b13160d	link	true	/test gencheck

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. I understand the commands that are listed here.

[nicole-lihui]

Is there any other solution?

[howardjohn]

Yes, make it conditional but default to the current set and I will approve

[nicole-lihui]

We have a customer scenario that requires addressing the same issue urgently.
Since there hasn't been much progress on the current PR, I've opened another PR to actively tackle this problem.
🏃 PR: #51234