[TEP-0102] HTTPS Connection to Triggers ClusterInterceptor #662
Conversation
|
/kind tep |
| * No inputs required from user to run ClusterInterceptor server as `HTTPS` as everything is handled internally by Triggers. | ||
|
|
||
| ## Implementation Details | ||
| At high level below are few implementation details |
There was a problem hiding this comment.
I think we also require changes in interceptor SDK so that custom interceptor can also be used in tls mode only.
So this work should start after tektoncd/triggers#1312
There was a problem hiding this comment.
Good catch but I don't think we need to block on it. I can rebase that PR/update it to use TLS once the implementation for this is done.
| At high level below are few implementation details | ||
| * Port and ENV changes in [core-interceptors-deployment.yaml](https://github.com/tektoncd/triggers/blob/main/config/interceptors/core-interceptors-deployment.yaml). | ||
| * Add new secret file to [config/interceptors](https://github.com/tektoncd/triggers/tree/main/config/interceptors) folder. | ||
| * Update roles, clusterroles. |
There was a problem hiding this comment.
what will be the changes? the Cluster role for interceptor would need secret access to write that specific secret?
| * Add new secret file to [config/interceptors](https://github.com/tektoncd/triggers/tree/main/config/interceptors) folder. | ||
| * Update roles, clusterroles. | ||
| * Changes to ClusterInterceptor server to run as `HTTPS`. | ||
| * Changes to EventListener in order to connect with ClusterInterceptor securely. |
There was a problem hiding this comment.
How will the EventListener find the cacert? In admission controllers, the WebhookConfiguration CR has (I think) a cacert field? Do we need to add something similar to the ClusterInterceptor CRD?
| * Changes to EventListener in order to connect with ClusterInterceptor securely. | ||
|
|
||
| ## A look into the future | ||
| * Providing a way to user to pass their own certificate to run ClusterInterceptor server. |
There was a problem hiding this comment.
If we add a caCert or something similar to the ClusterInterceptor CRD, this might be as simple as a user filling out this field. If it is missing, and the knative/pkg cert controller can generate the cert
@dibyom addressed review comments
I like the way we do it for |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dibyom, vdemeester 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:
Approvers can indicate their approval by writing |
Great, could we open a GH issue to track this feature? I have one small comment on moving some text to the Motivation section instead. I think that can also be done in a new PR where we change the status to implementable given that we already have tektoncd/triggers#1333 open /lgtm |
|
|
||
| ### User Stories | ||
|
|
||
| * ClusterInterceptor calls are done using `HTTP` instead of `HTTPS` which is considered a security problem because |
There was a problem hiding this comment.
I'd say this makes more sense in the Motivation section than in the User Stories section :)
Issue: tektoncd/triggers#871
As part of this TEP proposing to make default secure connection between ClusterInterceptor and EventListener.