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

Expose a new endpoint for injection in the proxy auto-injector #2286

Closed
alpeb opened this issue Feb 14, 2019 · 4 comments
Closed

Expose a new endpoint for injection in the proxy auto-injector #2286

alpeb opened this issue Feb 14, 2019 · 4 comments

Comments

@alpeb
Copy link
Member

alpeb commented Feb 14, 2019

Expose a new endpoint in the control plane that performs the proxy injection, given the new stored config (#1999). This will be used by both linkerd install/inject (#2288) and the auto-injector (#2289).

It should receive the resource yaml in the request, and return the injected yaml, or the JSON patch.
A request param should determine the type of the response.
As a followup, this endpoint should take into account the annotations/labels of the namespace and the resource itself for overriding the config (#2287)

@alpeb
Copy link
Member Author

alpeb commented Feb 14, 2019

We can provide this as a new endpoint in the controller api, or as part of the auto-injector, in which case we need to lift the current restriction of only installing the auto-injector when --proxy-auto-inject is provided at install time.

@ihcsim
Copy link
Contributor

ihcsim commented Feb 14, 2019

My thought is to re-use the existing proxy-injector controller, and change the effect of the --proxy-auto-inject flag such that, if enabled, then the admission controller webhook is installed. Otherwise, it only serves the new endpoint.

@klingerf
Copy link
Contributor

👍 to @ihcsim's comment. Originally I was thinking we should add a separate endpoint that's called by both the CLI and the auto-injector, but I think we'll gain a lot by instead forcing the CLI to use the auto-inject endpoint directly (although not by way of the linkerd-proxy-injector Service that the webhook uses -- we'll instead need to expose that endpoint via the public API).

By forcing the CLI to use the auto-inject endpoint, it guarantees that CLI injection and auto-injection will maintain feature parity. We won't be able to add additional functionality to CLI inject without also making that functionality available to auto-inject users.

Also, since auto-inject requires modifications in JSON patch format but the CLI outputs fully-patched YAML, I feel like we'd end up with a lot of conditional formatting logic in an endpoint that provides both formats. Instead, if we only support JSON patch format, the CLI can leverage k8s.io code directly to apply those patches, and we won't need an inject implementation in our codebase that produces fully-patched YAML.

In terms of running an additional pod in non-auto-inject control plane installs, I think it's fine to include the proxy-injector pod to serve the inject endpoint, with a flag to control whether or not it creates the MWC, based on whether auto-inject is enabled or not. We could also investigate moving the proxy-inject container into the linkerd-controller pod, since it would be a required part of all control plane installs. I don't feel too strongly one way or another on that.

@alpeb alpeb self-assigned this Feb 14, 2019
@alpeb alpeb changed the title New control plane endpoint that consolidates proxy injection Expose a new endpoint for injection in the proxy auto-injector Feb 15, 2019
@ihcsim ihcsim assigned ihcsim and unassigned alpeb Feb 25, 2019
@ihcsim ihcsim removed their assignment Feb 26, 2019
@alpeb
Copy link
Member Author

alpeb commented Mar 22, 2019

This has been superseded by #2447

@alpeb alpeb closed this as completed Mar 22, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants