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

add sysctl net.ipv4.ip_unprivileged_port_start 53 #171

Closed
wants to merge 1 commit into from

Conversation

pacoxu
Copy link
Contributor

@pacoxu pacoxu commented Aug 6, 2024

ref coredns/deployment#298

kubernetes/kubernetes#103326 marked it as safe sysctl since Kubernetes v1.22.

Kernel 4.11 add this: torvalds/linux@4548b68 which is per namespaced.

xref coredns/coredns#6716 and kubernetes/kubernetes#125226.

Signed-off-by: Paco Xu <paco.xu@daocloud.io>
@pacoxu
Copy link
Contributor Author

pacoxu commented Aug 6, 2024

/assign @chrisohaver

@hagaibarel hagaibarel self-requested a review August 6, 2024 06:46
@hagaibarel
Copy link
Collaborator

@pacoxu thanks for the PR. Can you explain why should this be in the default values? We already expose this so individuals can override the setting using helm's set or an external values file.

Seems like this change is for a specific use case, which can be handled without any changes, and I'm not sure this should be globally set as the default

@pacoxu
Copy link
Contributor Author

pacoxu commented Aug 6, 2024

BTW, this needs kernel 4.11+

Or coredns pod will fail with below error:

Warning FailedCreatePodSandBox 2s (x13 over 43s) kubelet Failed to create pod sandbox: rpc error: code = Unknown desc = failed to create containerd task: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: open /proc/sys/net/ipv4/ip_unprivileged_port_start: no such file or directory: unknown

For more context, see kubernetes/kubernetes#105309 (comment).

@pacoxu
Copy link
Contributor Author

pacoxu commented Aug 6, 2024

/hold
for kernel version 4.11 requirement

@hagaibarel
Copy link
Collaborator

BTW, this should already be covered with https://github.com/coredns/helm/blob/master/charts/coredns/values.yaml#L94C1-L97C25

Comment on lines +90 to +93
podSecurityContext:
sysctls:
- name: net.ipv4.ip_unprivileged_port_start
value: "53"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an alter way and with the new sysctl, we can remove the NET_BIND_SERVICE capability.

          capabilities:
            add:
            - NET_BIND_SERVICE
            drop:
            - ALL

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we already have a solution in place for this, I don't think we should make any changes, this works as intended and there's no real value in replacing the capabilities with the sysctls

@hagaibarel
Copy link
Collaborator

Closing this, this is already covered by using the NET_BIND_SERVICE capabilities

@hagaibarel hagaibarel closed this Aug 6, 2024
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.

2 participants