Allow user defined endpoint to host action for Canal #3272
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/assign @justinsb |
KashifSaadat
force-pushed
the
canal-custom-endpoint-to-host-action
branch
from
280f92e
to
2fd2db2
Compare
|
LGTM ... Side issue we need to look at getting it applied in a pre-existing cluster (even if only on delete, given it's a daemonet), I thought protokube unit applied the channels but maybe i'm mistaken .. |
k8s-ci-robot
added
size/M
KashifSaadat
force-pushed
the
canal-custom-endpoint-to-host-action
branch
from
222879b
to
d48b269
Compare
|
@gambol99 I've added validation to the Canal spec, please could you have another peek? |
pkg/apis/kops/validation/legacy.go
Outdated
| // Check Canal Networking Spec if used | ||
| if c.Spec.Networking.Canal != nil { | ||
| action := c.Spec.Networking.Canal.DefaultEndpointToHostAction | ||
| if action != "" { |
There was a problem hiding this comment.
we could probably shorten this with a case statement instead
switch action {
case "", "ACCEPT", "DROP", "RETURN":
default:
return field.Invalid(fieldSpec.Child("Networking", "Canal", "DefaultEndpointToHostAction"), action, fmt.Sprintf("Unsupported value: %s, supports ACCEPT, DROP or RETURN", action))
}There was a problem hiding this comment.
Yup good idea, will update.
KashifSaadat
force-pushed
the
canal-custom-endpoint-to-host-action
branch
from
d48b269
to
a627a83
Compare
|
other than the above comment ... lgtm |
KashifSaadat
force-pushed
the
canal-custom-endpoint-to-host-action
branch
from
a627a83
to
f254a06
Compare
| @@ -119,7 +119,7 @@ spec: | |||
| fieldPath: spec.nodeName | |||
| # Set Felix endpoint to host default action to ACCEPT. | |||
| - name: FELIX_DEFAULTENDPOINTTOHOSTACTION | |||
| value: "ACCEPT" | |||
| value: "{{- if eq .Networking.Canal.DefaultEndpointToHostAction "" }}ACCEPT{{- else -}}{{ .Networking.Canal.DefaultEndpointToHostAction }}{{- end -}}" | |||
There was a problem hiding this comment.
I think you can also use {{- or .Networking.Canal.DefaultEndpointToHostAction "ACCEPT" }} but this works :-)
There was a problem hiding this comment.
Yep that works and cleaner, will update. :) Cheers!
|
/ok-to-test /lgtm |
k8s-ci-robot
added
the
lgtm
|
There's a gotcha here for applying in an existing cluster ... we only apply if the version is different currently. But this is tricky, because this is a change that isn't described by a version. The "k8s way" would be to set a configmap, but we're still figuring that all out TBH. A trick would be to delete the calico manifest annotation on the kube-system namespace, then the manifest will be reapplied. |
KashifSaadat
force-pushed
the
canal-custom-endpoint-to-host-action
branch
from
f254a06
to
2ffc790
Compare
k8s-github-robot
added
the
approved
|
/lgtm cancel //PR changed after LGTM, removing LGTM. @KashifSaadat @justinsb |
k8s-github-robot
removed
the
lgtm
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: KashifSaadat, justinsb The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Test failure filed as kubernetes/kubernetes#51429 |
|
/test pull-kops-e2e-kubernetes-aws |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue |
Adds ability to define
Networking.Canal.DefaultEndpointToHostActionin the Cluster Spec. This allows you to customise the behaviour of traffic routing from a pod to the host (after calico iptables chains have been processed).ACCEPTis the default value and is left as-is.If you want to allow some or all traffic from endpoint to host, set this parameter to “RETURN” or “ACCEPT”. Use “RETURN” if you have your own rules in the iptables “INPUT” chain; Calico will insert its rules at the top of that chain, then “RETURN” packets to the “INPUT” chain once it has completed processing workload endpoint egress policy.