Feature request: Eventlistener should support customized podTemplate

[MartinForReal]

Expected Behavior

Event listener should accept customizable default pod template as what tekton pipeline did.
If we can add a field in CRD that would be better

Actual Behavior

Event listener is created using the default deployment template. But it doesn't comply with our schedule policy.

Additional Info

tektoncd/pipeline#1901

[MartinForReal]

/kind feature

[MartinForReal]

@bobcatfish @dibyom @vdemeester @ckadner @Tomcli

[MartinForReal]

/area api

[ncskier]

This issue sounds similar to #440. I wonder if we can address both issues in the same way?

[dibyom]

Yeah this seems like a very reasonable feature request. I'm for adding this

[dibyom]

I wonder if a podTemplate would be enough though....is there a need for customizing things such as number of replicas etc.?
Also, perhaps we might need to customize fields within the Service spec as well?

[vdemeester]

This would be really nice to have this to be able to configure "resource request" for the event-listener pod 👼

[vdemeester]

I wonder if a podTemplate would be enough though....is there a need for customizing things such as number of replicas etc.?
Also, perhaps we might need to customize fields within the Service spec as well?

That's a good point, maybe having a DeploymentTemplate that would be merged with the default (and be validated a bit differently than the default Deployment spec would work).

That said, I think we might want to consider Service and Deployment as implementation detail and try as much as we can to not make those info bubble up. This would mean, having a replicas field, and maybe a podTemplate (because in the end, it's a pod running) or maybe only part of what we are interested in the podTemplate, like limits, …

[dibyom]

From the WG discussion today:

1. We should think what adding something like a PodTemplate would mean for api compatibility...I know in Pipelines we have been trying to be careful about not adding Kubernetes primitives into the Tekton API (cc @imjasonh )

2. Also worth considering, what the exact use cases are. Do we need all of the knobs present in the Pipeline PodTemplate? Maybe we just need resourceLimits and maybe replicas?

[vdemeester]

From the WG discussion today:

1. We should think what adding something like a `PodTemplate` would mean for api compatibility...I know in Pipelines we have been trying to be careful about not adding Kubernetes primitives into the Tekton API (cc @ImJasonH )

2. Also worth considering, what the exact use cases are. Do we need all of the knobs present in the Pipeline PodTemplate? Maybe we just need `resourceLimits` and maybe `replicas`?

I do agree on both. I do prefer the approach on having the least kubernetes primitive in there. As far as I can, the only need we have right now are resource request/limits and replicas.

[MartinForReal]

We also need to add tolerations or nodeSelector

[MartinForReal]

Maybe we can add a config object for following configs

• imagePullSecrets
• imagePullPolicy
• customLabels
• tolerations
• tolerations
• nodeSelector
• replicas
• resourceLimits

and hard code following settings:

• livenessProbe
• readinessProbe

If this works, I would like to submit patches.

[dibyom]

Curious about imagePullSecrets and imagePullPolicy. What's the use case for adding those in Triggers? The others sound good to me!

[imjasonh]

Knative has a duck type for thing that should expect to look like a Pod: https://godoc.org/github.com/knative/pkg/apis/duck/v1#PodSpecable

The idea is that tools and libraries can operate on a number of Pod-alike things, e.g. Knative services and K8s Pods and Deployments.

@mattmoor in case I've mischaracterized or there's a better canonical description of this model.

[MartinForReal]

Imagepullpolicy is needed when we need to validate image before we start a pod.( In this case we need to set imagepullpolicy to Always ). One possible policy is: image doesn't contains vulnerabilities. This can be achieved by enforcing the security policy in image registries( harbor or jfrog artifacts)
Imagepullsecret is used when we need to ensure that only sys admin have access to these images
Both of options are optional, we can remove them from the list

[gabemontero]

@dibyom asked that I echo a concern I noted in recent triggers wg call around use of the entire podtemplate as it exposes the podsecuritycontext (https://github.com/kubernetes/api/blob/master/core/v1/types.go#L2960)

allowing controllers to set these on behalf of requesting users is a slippery slope

certainly tektoncd/pipelines is already allowing it as well with PodTemplate being embedded in the core types, but in general is using namespace containment as the workaround

SAR based checks by the controller on behalf of a requesting users for RBAC encapsulations around podsecuritypolicies (https://kubernetes.io/docs/concepts/policy/pod-security-policy/#via-rbac) should be a long term goal in both pipeline and triggers projects

I am aware that SAR checks has come up in some issue discussion with a few scenarios in both projects, but I'm not aware of a feature on a current or soon to be upcoming roadmap that introduces them at the levels I'm referring to. If I'm incorrect, please inform me. Otherwise, I'm looking to fill that gap.

In partcilar, I'll mention what @dibyom and the folks on the triggers WG are already aware of. I do have a tektoncd/pipelines prototype that introduces SAR checks, but is not quite ready for demo and public introduction. I hope to get there in the next couple of weeks, where I can at least demo. And then from there work on design proposals, engagement with pipeline and API WG's, etc.

It leverages the knative admission controller / webhook utilities already used by tekton, and introduces requesting user to TaskRun and PipelineRun

A similar approach could be employed by triggers for a full PodTemplate that exposes PodSecurityPolicy if that comes to pass.

[imjasonh]

(SAR stands for SubjectAccessReview described here: https://kubernetes.io/docs/reference/access-authn-authz/authorization/#checking-api-access)

[msjostrom]

I would like to be able to add a sidecar proxy to the event sink container, for security purposes. Maybe that's an additional use case for the custom podTemplate.

[gabemontero]

@dibyom asked that I echo a concern I noted in recent triggers wg call around use of the entire podtemplate as it exposes the podsecuritycontext (https://github.com/kubernetes/api/blob/master/core/v1/types.go#L2960)

allowing controllers to set these on behalf of requesting users is a slippery slope

certainly tektoncd/pipelines is already allowing it as well with PodTemplate being embedded in the core types, but in general is using namespace containment as the workaround

SAR based checks by the controller on behalf of a requesting users for RBAC encapsulations around podsecuritypolicies (https://kubernetes.io/docs/concepts/policy/pod-security-policy/#via-rbac) should be a long term goal in both pipeline and triggers projects

I am aware that SAR checks has come up in some issue discussion with a few scenarios in both projects, but I'm not aware of a feature on a current or soon to be upcoming roadmap that introduces them at the levels I'm referring to. If I'm incorrect, please inform me. Otherwise, I'm looking to fill that gap.

In partcilar, I'll mention what @dibyom and the folks on the triggers WG are already aware of. I do have a tektoncd/pipelines prototype that introduces SAR checks, but is not quite ready for demo and public introduction. I hope to get there in the next couple of weeks, where I can at least demo. And then from there work on design proposals, engagement with pipeline and API WG's, etc.

It leverages the knative admission controller / webhook utilities already used by tekton, and introduces requesting user to TaskRun and PipelineRun

My prototype is at least demoable / presentable for community consumption.

My hope is to show it / talk to it during the trigger/pipeline WG meetings next Tuesday and Wednesday, May 12/13

A similar approach could be employed by triggers for a full PodTemplate that exposes PodSecurityPolicy if that comes to pass.

[dibyom]

Imagepullpolicy is needed when we need to validate image before we start a pod.( In this case we need to set imagepullpolicy to Always ). One possible policy is: image doesn't contains vulnerabilities. This can be achieved by enforcing the security policy in image registries( harbor or jfrog artifacts)
Imagepullsecret is used when we need to ensure that only sys admin have access to these images
Both of options are optional, we can remove them from the list

Those all sound like good use cases. The only image that this would apply to is the EventListener sink so I'm wondering if one alternative here is that the operator passes in the right image to the Triggers controller (currently this is passed via a flag, the operator could update the flag to use a blessed image.

[dibyom]

@gabemontero thanks for the write up and looking forward to the demo!! One thing about podTemplate in Triggers is that it only applies to the EventListener sink image which only processes HTTP calls whereas in pipelines it applies to Tasks that can be executing arbitrary commands. So, in my mind, Triggers is at less of a risk than Pipelines.

I would like to be able to add a sidecar proxy to the event sink container, for security purposes. Maybe that's an additional use case for the custom podTemplate.

@msjostrom interesting...would this be service mesh like Istio that can be automatically injected via a admission controller?

[gabemontero]

@gabemontero thanks for the write up and looking forward to the demo!! One thing about podTemplate in Triggers is that it only applies to the EventListener sink image which only processes HTTP calls whereas in pipelines it applies to Tasks that can be executing arbitrary commands. So, in my mind, Triggers is at less of a risk than Pipelines.

yep if the use of PodTemplate is narrowed at that level I would concur wrt the relative risk assessment @dibyom

I would like to be able to add a sidecar proxy to the event sink container, for security purposes. Maybe that's an additional use case for the custom podTemplate.

@msjostrom interesting...would this be service mesh like Istio that can be automatically injected via a admission controller?

[msjostrom]

@msjostrom interesting...would this be service mesh like Istio that can be automatically injected via a admission controller?

I guess it could be, but in my current case I want to use the openshift oauth-proxy sidecar to force the event producer to authenticate using an openshift issued token.

To really make it work, I would need the possibility to tell tektoncd-triggers-eventlistenersink image to only listen to localhost as well, to prevent bypassing of the sidecar proxy.

[tekton-robot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

[tekton-robot]

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

/lifecycle stale

Send feedback to tektoncd/plumbing.

[tekton-robot]

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

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[tekton-robot]

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

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.

[dibyom]

We now support a minimal customized podTemplate. We can add more fields if we need to or TEP-008 proposes a way to include a full customl PodSpec if needed.

[robermar23]

I am confused on why this issue was closed.

The documentation mentions podTemplate only support nodeSelector and tolerations?

Is that the "minimal customized podTemptate" mentioned?

I have a need to be able to define the resources for the pod's that are spun up.

Am I missing another means by which this is accomplished or is everyone just using their namespaced defaults?

[dibyom]

@robermar23 sorry for the delay. I opened a new feature request to track setting resource requirements in #792