Serving: Unable to set EnableServiceLinks in PodSpec

Created on 20 Nov 2019 · 10Comments · Source: knative/serving

/area API

What version of Knative?

0.0.10

Expected Behavior

Ability to disable service links by setting PodSpec.EnableServiceLinks to false.

Actual Behavior

kubectl apply fails with:

admission webhook "webhook.serving.knative.dev" denied the request: mutation failed: must not set the field(s): spec.template.spec.enableServiceLinks

Steps to Reproduce the Problem

1) Create a KNative service with enableServiceLinks: true set in the pod spec.

# Example truncated for brevity
apiVersion: serving.knative.dev/v1
kind: Service
spec:
  template:
    spec:
      enableServiceLinks: false

2) kubectl apply -f <yourfile.yaml>

I assume there was some reasoning behind this, but this flag was added for very good reasons as folks found it difficult to debug their own env vars. See https://github.com/kubernetes/kubernetes/issues/60099 for detailed rationale.

areAPI kinbug

Source

tshak

Most helpful comment

To follow up - we discussed service links at the API WG group and the consensus was

1) We'll allow service links to be set (hence the PR)
2) We don't want to deviate from the K8s defaults in the PodSpec (this could affect portability of workloads)

A future concern would be _could_ Knative operators change these default - but that's a different issue

dprotaso on 23 Jun 2020

🎉1 👍1

All 10 comments

We whitelist PodSpec values rather than blacklist them, so I assume this hasn't been considered yet. It does sound harmless to add though I'd say.

/cc @dgerd

markusthoemmes on 20 Nov 2019

👍1

Happy New Year! While this is not a critical issue, it seems like it's a simple change. Is there anything that I can do to help resolve this?

tshak on 9 Jan 2020

Upon further reflection, even though it's a breaking change, I'd set this to false and not even expose it as a setting. This setting's default in k8s is just for backward compat with the deprecated docker --links feature, something Knative users are probably not even aware of. It exposes things that we wouldn't want the apps to have access to (such as the IP addresses of other services in the namespace). This seems like a good opportunity for Knative to reduce the surface area. Either way please let me know if I can help.

tshak on 16 Feb 2020

👍1

Issues go stale after 90 days of inactivity.
Mark the issue as fresh by adding the comment /remove-lifecycle stale.
Stale issues rot after an additional 30 days of inactivity and eventually close.
If this issue is safe to close now please do so by adding the comment /close.

Send feedback to Knative Productivity Slack channel or file an issue in knative/test-infra.

/lifecycle stale

knative-housekeeping-robot on 17 May 2020

Stale issues rot after 30 days of inactivity.
Mark the issue as fresh by adding the comment /remove-lifecycle rotten.
Rotten issues close after an additional 30 days of inactivity.
If this issue is safe to close now please do so by adding the comment /close.

Send feedback to Knative Productivity Slack channel or file an issue in knative/test-infra.

/lifecycle rotten

knative-housekeeping-robot on 16 Jun 2020

/remove-lifecycle rotten

dprotaso on 17 Jun 2020

/assign @dprotaso

dprotaso on 23 Jun 2020

To follow up - we discussed service links at the API WG group and the consensus was

1) We'll allow service links to be set (hence the PR)
2) We don't want to deviate from the K8s defaults in the PodSpec (this could affect portability of workloads)

A future concern would be _could_ Knative operators change these default - but that's a different issue

dprotaso on 23 Jun 2020

🎉1 👍1

Possibly related, but I need to confirm: https://knative.slack.com/archives/CA4DNJ9A4/p1593267884035600