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

proxy-injector: add default-inbound-policy annotation #6750

Merged
merged 5 commits into from
Aug 26, 2021

Conversation

kleimkuhler
Copy link
Contributor

The proxy injector now adds the config.linkerd.io/default-inbound-policy annotation to all injected pods.

Closes #6720.

If the pod has the annotation before injection then that value is used. If the pod does not have the annotation but the namespace does, then it inherits that. If both the pod and the namespace do not have the annotation, then it defaults to .Values.policyController.defaultAllowPolicy.

Upon injecting the sidecar container into the pod, this annotation value is used to set the LINKERD2_PROXY_INBOUND_DEFAULT_POLICY environment variable. Additionally, LINKERD2_PROXY_POLICY_CLUSTER_NETWORKS is also set to the value of .Values.clusterNetworks.

Signed-off-by: Kevin Leimkuhler kevin@kleimkuhler.com

Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
@kleimkuhler kleimkuhler requested a review from a team as a code owner August 25, 2021 21:09
pkg/inject/inject.go Outdated Show resolved Hide resolved
Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
Copy link
Contributor

@Pothulapati Pothulapati left a comment

Choose a reason for hiding this comment

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

Was able to verify it working, and the proxy doesn't seem to complain about the policy env anymore 🥳

pkg/k8s/labels.go Outdated Show resolved Hide resolved
@@ -66,6 +66,7 @@ var (
k8s.ProxyOutboundConnectTimeout,
k8s.ProxyInboundConnectTimeout,
k8s.ProxyAwait,
k8s.ProxyDefaultInboundPolicy,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also have a cmd flag in makeProxyFlags to make it easy to configure without having to manually add the annotation (especially useful when the manifests are somewhere and accessed using a URL)? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will make a follow-up issue to add this as polish for stable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -34,6 +34,10 @@ env:
value: {{ternary "localhost.:8090" (printf "linkerd-policy.%s.svc.%s.:8090" .Values.namespace .Values.clusterDomain) (eq (toString .Values.proxy.component) "linkerd-destination")}}
- name: LINKERD2_PROXY_POLICY_WORKLOAD
value: "$(_pod_ns):$(_pod_name)"
- name: LINKERD2_PROXY_INBOUND_DEFAULT_POLICY
value: {{.Values.proxy.defaultInboundPolicy | default .Values.policyController.defaultAllowPolicy}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if .Values.policyController.defaultAllowPolicy is useful in any manner for the user? It could be even be confusing in a way? proxy.defaultInboundPolicy totally makes sense as its a proxy level configuration. Maybe we can remove this and fall-back to the default value directly i.e all-unauthenticated? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I think we want to support a global default policy that isn't set per-workload

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's possible that the cluster's default allow policy is more restricted than all-unauthenticated which is why we want to fall back to that here.

@kleimkuhler kleimkuhler merged commit 152290e into main Aug 26, 2021
@kleimkuhler kleimkuhler deleted the kleimkuhler/annotate-policy branch August 26, 2021 18:46
ahmedalhulaibi added a commit to ahmedalhulaibi/linkerd2 that referenced this pull request Dec 7, 2021
PR linkerd#6750 adds the config.linkerd.io/default-inbound-policy annotation for setting the default inbound policy for an injected proxy.

This commit adds support for a default-inbound-policy flag in makeProxyFlags so that it can be set with the linkerd inject command.

Closes linkerd#6754

Signed-off-by: ahmedalhulaibi <ahmed.alhulaibi41@gmail.com>
ahmedalhulaibi added a commit to ahmedalhulaibi/linkerd2 that referenced this pull request Dec 8, 2021
PR linkerd#6750 adds the config.linkerd.io/default-inbound-policy annotation for setting the default inbound policy for an injected proxy.

This commit adds support for a default-inbound-policy flag in makeProxyFlags so that it can be set with the linkerd inject command.

Closes linkerd#6754

Signed-off-by: ahmedalhulaibi <ahmed.alhulaibi41@gmail.com>

tests: use all-authenticated constant
kleimkuhler pushed a commit that referenced this pull request Dec 9, 2021
PR #6750 adds the config.linkerd.io/default-inbound-policy annotation for setting the default inbound policy for an injected proxy.

This commit adds support for a default-inbound-policy flag in makeProxyFlags so that it can be set with the linkerd inject command.

Closes #6754

Signed-off-by: ahmedalhulaibi <ahmed.alhulaibi41@gmail.com>
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.

injector: Configure config.linkerd.io/default-inbound-policy
5 participants