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.

Sign up for GitHub

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

Jump to bottom

add namespace toggle to tproxy #510

Merged
merged 10 commits into from
May 6, 2021
Merged

add namespace toggle to tproxy #510

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c82e491
add namespace toggling to tproxy
kschoche May 4, 2021
76068c0
fix up the tests
kschoche May 4, 2021
f013726
Merge branch 'master' into tproxy_add_namespace_toggle_support
kschoche May 5, 2021
128050f
do not modify the annotation and update the changelog
kschoche May 5, 2021
ecb1604
fix typo in comment
kschoche May 5, 2021
b3f00c8
fix test after merge
kschoche May 5, 2021
cdc40ea
plumb the client through the command
kschoche May 5, 2021
2f81238
a couple cleanups
kschoche May 5, 2021
cfdb238
review comments
kschoche May 5, 2021
b87fd97
Apply suggestions from code review
kschoche May 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions connect-inject/annotations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ const (
// annotationConsulNamespace is the Consul namespace the service is registered into.
annotationConsulNamespace = "consul.hashicorp.com/consul-namespace"

// annotationTransparentProxy enables or disables transparent proxy mode for a given pod.
// annotationTransparentProxy enables or disables default transparent proxy behaviour for a given pod or namespace.
// This annotation takes a boolean value (true/false).
annotationTransparentProxy = "consul.hashicorp.com/transparent-proxy"
annotationTransparentProxy = "consul.hashicorp.com/transparent-proxy-default"
kschoche marked this conversation as resolved.
Show resolved Hide resolved

// annotationTProxyExcludeInboundPorts is a comma-separated list of inbound ports to exclude from traffic redirection.
annotationTProxyExcludeInboundPorts = "consul.hashicorp.com/transparent-proxy-exclude-inbound-ports"
Expand Down
18 changes: 12 additions & 6 deletions connect-inject/container_init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,16 @@ func (h *Handler) containerInitCopyContainer() corev1.Container {

// containerInit returns the init container spec for registering the Consul
// service, setting up the Envoy bootstrap, etc.
func (h *Handler) containerInit(pod corev1.Pod, k8sNamespace string) (corev1.Container, error) {
func (h *Handler) containerInit(namespace *corev1.Namespace, pod corev1.Pod) (corev1.Container, error) {
kschoche marked this conversation as resolved.
Show resolved Hide resolved
// Check if tproxy is enabled on this pod.
tproxyEnabled, err := transparentProxyEnabled(pod, h.EnableTransparentProxy)
tproxyEnabled, err := transparentProxyEnabled(namespace, pod, h.EnableTransparentProxy)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We now pass the namespace into containerInit from the handler, this allows us to (1) re-use the transparentProxyEnabled() function between containerInit/endpointsController and (2) makes testing simpler so we do not need to load up a k8s clientset for every containerInit test.

if err != nil {
return corev1.Container{}, err
}

data := initContainerCommandData{
AuthMethod: h.AuthMethod,
ConsulNamespace: h.consulNamespace(k8sNamespace),
ConsulNamespace: h.consulNamespace(namespace.Name),
NamespaceMirroringEnabled: h.EnableK8SNSMirroring,
ConsulCACert: h.ConsulCACert,
EnableTransparentProxy: tproxyEnabled,
Expand Down Expand Up @@ -214,12 +214,18 @@ func (h *Handler) containerInit(pod corev1.Pod, k8sNamespace string) (corev1.Con
}

// transparentProxyEnabled returns true if transparent proxy should be enabled for this pod.
// It returns an error when the annotation value cannot be parsed by strconv.ParseBool.
func transparentProxyEnabled(pod corev1.Pod, globalEnabled bool) (bool, error) {
// It returns an error when the annotation value cannot be parsed by strconv.ParseBool or if we are unable
// to read the pod's namespace label when it exists.
func transparentProxyEnabled(namespace *corev1.Namespace, pod corev1.Pod, globalEnabled bool) (bool, error) {
// First check to see if the pod annotation exists to override the namespace or global settings.
if raw, ok := pod.Annotations[annotationTransparentProxy]; ok {
return strconv.ParseBool(raw)
}

// Next see if the namespace has been defaulted.
if raw, ok := namespace.Labels[annotationTransparentProxy]; ok {
return strconv.ParseBool(raw)
}
// Else fall back to the global default.
return globalEnabled, nil
}

Expand Down
83 changes: 56 additions & 27 deletions connect-inject/container_init_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ consul-k8s connect-init -pod-name=${POD_NAME} -pod-namespace=${POD_NAMESPACE} \

h := tt.Handler
pod := *tt.Pod(minimal())
container, err := h.containerInit(pod, k8sNamespace)
container, err := h.containerInit(&testNS, pod)
require.NoError(err)
actual := strings.Join(container.Command, " ")
require.Contains(actual, tt.Cmd)
Expand All @@ -146,64 +146,63 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
annotations map[string]string
expectedContainsCmd string
expectedNotContainsCmd string
namespaceLabel map[string]string
}{
"enabled globally, annotation not provided": {
"enabled globally, ns not set, annotation not provided": {
true,
nil,
`/consul/connect-inject/consul connect redirect-traffic \
-proxy-id="$(cat /consul/connect-inject/proxyid)" \
-proxy-uid=5995`,
"",
nil,
},
"enabled globally, annotation is false": {
"enabled globally, ns not set, annotation is false": {
true,
map[string]string{
annotationTransparentProxy: "false",
},
map[string]string{annotationTransparentProxy: "false"},
"",
`/consul/connect-inject/consul connect redirect-traffic \
-proxy-id="$(cat /consul/connect-inject/proxyid)" \
-proxy-uid=5995`,
nil,
},
"enabled globally, annotation is true": {
"enabled globally, ns not set, annotation is true": {
true,
map[string]string{
annotationTransparentProxy: "true",
},
map[string]string{annotationTransparentProxy: "true"},
`/consul/connect-inject/consul connect redirect-traffic \
-proxy-id="$(cat /consul/connect-inject/proxyid)" \
-proxy-uid=5995`,
"",
nil,
},
"disabled globally, annotation not provided": {
"disabled globally, ns not set, annotation not provided": {
false,
nil,
"",
`/consul/connect-inject/consul connect redirect-traffic \
-proxy-id="$(cat /consul/connect-inject/proxyid)" \
-proxy-uid=5995`,
nil,
},
"disabled globally, annotation is false": {
"disabled globally, ns not set, annotation is false": {
false,
map[string]string{
annotationTransparentProxy: "false",
},
map[string]string{annotationTransparentProxy: "false"},
"",
`/consul/connect-inject/consul connect redirect-traffic \
-proxy-id="$(cat /consul/connect-inject/proxyid)" \
-proxy-uid=5995`,
nil,
},
"disabled globally, annotation is true": {
"disabled globally, ns not set, annotation is true": {
false,
map[string]string{
annotationTransparentProxy: "true",
},
map[string]string{annotationTransparentProxy: "true"},
`/consul/connect-inject/consul connect redirect-traffic \
-proxy-id="$(cat /consul/connect-inject/proxyid)" \
-proxy-uid=5995`,
"",
nil,
},
"exclude-inbound-ports annotation is provided": {
"exclude-inbound-ports, ns is not set, annotation is provided": {
true,
map[string]string{
annotationTransparentProxy: "true",
Expand All @@ -215,8 +214,9 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
-proxy-id="$(cat /consul/connect-inject/proxyid)" \
-proxy-uid=5995`,
"",
nil,
},
"exclude-outbound-ports annotation is provided": {
"exclude-outbound-ports, ns is not set, annotation is provided": {
true,
map[string]string{
annotationTransparentProxy: "true",
Expand All @@ -228,6 +228,7 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
-proxy-id="$(cat /consul/connect-inject/proxyid)" \
-proxy-uid=5995`,
"",
nil,
},
"exclude-outbound-cidrs annotation is provided": {
true,
Expand All @@ -241,8 +242,9 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
-proxy-id="$(cat /consul/connect-inject/proxyid)" \
-proxy-uid=5995`,
"",
nil,
},
"exclude-uids annotation is provided": {
"exclude-uids annotation is provided, ns is not set": {
true,
map[string]string{
annotationTransparentProxy: "true",
Expand All @@ -254,6 +256,25 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
-proxy-id="$(cat /consul/connect-inject/proxyid)" \
-proxy-uid=5995`,
"",
nil,
},
"disabled globally, ns enabled, annotation not set": {
false,
nil,
`/consul/connect-inject/consul connect redirect-traffic \
-proxy-id="$(cat /consul/connect-inject/proxyid)" \
-proxy-uid=5995`,
"",
map[string]string{annotationTransparentProxy: "true"},
},
"enabled globally, ns disabled, annotation not set": {
true,
nil,
"",
`/consul/connect-inject/consul connect redirect-traffic \
-proxy-id="$(cat /consul/connect-inject/proxyid)" \
-proxy-uid=5995`,
map[string]string{annotationTransparentProxy: "false"},
},
}
for name, c := range cases {
Expand All @@ -270,7 +291,9 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) {
},
RunAsNonRoot: pointerToBool(false),
}
container, err := h.containerInit(*pod, k8sNamespace)
ns := testNS
ns.Labels = c.namespaceLabel
container, err := h.containerInit(&ns, *pod)
require.NoError(t, err)
actualCmd := strings.Join(container.Command, " ")

Expand Down Expand Up @@ -529,7 +552,7 @@ consul-k8s connect-init -pod-name=${POD_NAME} -pod-namespace=${POD_NAMESPACE} \
require := require.New(t)

h := tt.Handler
container, err := h.containerInit(*tt.Pod(minimal()), k8sNamespace)
container, err := h.containerInit(&testNS, *tt.Pod(minimal()))
require.NoError(err)
actual := strings.Join(container.Command, " ")
require.Equal(tt.Cmd, actual)
Expand Down Expand Up @@ -565,7 +588,7 @@ func TestHandlerContainerInit_authMethod(t *testing.T) {
ServiceAccountName: "foo",
},
}
container, err := h.containerInit(*pod, k8sNamespace)
container, err := h.containerInit(&testNS, *pod)
require.NoError(err)
actual := strings.Join(container.Command, " ")
require.Contains(actual, `
Expand Down Expand Up @@ -602,7 +625,7 @@ func TestHandlerContainerInit_WithTLS(t *testing.T) {
},
},
}
container, err := h.containerInit(*pod, k8sNamespace)
container, err := h.containerInit(&testNS, *pod)
require.NoError(err)
actual := strings.Join(container.Command, " ")
require.Contains(actual, `
Expand Down Expand Up @@ -646,7 +669,7 @@ func TestHandlerContainerInit_Resources(t *testing.T) {
},
},
}
container, err := h.containerInit(*pod, k8sNamespace)
container, err := h.containerInit(&testNS, *pod)
require.NoError(err)
require.Equal(corev1.ResourceRequirements{
Limits: corev1.ResourceList{
Expand Down Expand Up @@ -675,3 +698,9 @@ func TestHandlerContainerInitCopyContainer(t *testing.T) {
actual := strings.Join(container.Command, " ")
require.Contains(actual, `cp /bin/consul /consul/connect-inject/consul`)
}

var testNS = corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: k8sNamespace,
},
}
9 changes: 8 additions & 1 deletion connect-inject/endpoints_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,14 @@ func (r *EndpointsController) createServiceRegistrations(pod corev1.Pod, service
proxyService.Tags = tags
}

tproxyEnabled, err := transparentProxyEnabled(pod, r.EnableTransparentProxy)
// A user can enable/disable tproxy for an entire namespace.
var ns corev1.Namespace
err = r.Client.Get(r.Context, types.NamespacedName{Name: pod.Namespace, Namespace: ""}, &ns)
if err != nil {
return nil, nil, err
}

tproxyEnabled, err := transparentProxyEnabled(&ns, pod, r.EnableTransparentProxy)
if err != nil {
return nil, nil, err
}
Expand Down
69 changes: 64 additions & 5 deletions connect-inject/endpoints_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,8 +798,11 @@ func TestReconcileCreateEndpoint(t *testing.T) {
fakeClientPod := createPod("fake-consul-client", "127.0.0.1", false)
fakeClientPod.Labels = map[string]string{"component": "client", "app": "consul", "release": "consul"}

// Add the default namespace.
ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}}
// Create fake k8s client
k8sObjects := append(tt.k8sObjects(), fakeClientPod)
k8sObjects := append(tt.k8sObjects(), fakeClientPod, &ns)

fakeClient := fake.NewClientBuilder().WithRuntimeObjects(k8sObjects...).Build()

// Create test consul server
Expand Down Expand Up @@ -1613,8 +1616,10 @@ func TestReconcileUpdateEndpoint(t *testing.T) {
fakeClientPod := createPod("fake-consul-client", "127.0.0.1", false)
fakeClientPod.Labels = map[string]string{"component": "client", "app": "consul", "release": "consul"}

// Add the default namespace.
ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}}
// Create fake k8s client
k8sObjects := append(tt.k8sObjects(), fakeClientPod)
k8sObjects := append(tt.k8sObjects(), fakeClientPod, ns)
fakeClient := fake.NewClientBuilder().WithRuntimeObjects(k8sObjects...).Build()

masterToken := "b78d37c7-0ca7-5f4d-99ee-6d9975ce4586"
Expand Down Expand Up @@ -1787,8 +1792,10 @@ func TestReconcileDeleteEndpoint(t *testing.T) {
fakeClientPod := createPod("fake-consul-client", "127.0.0.1", false)
fakeClientPod.Labels = map[string]string{"component": "client", "app": "consul", "release": "consul"}

// Add the default namespace.
ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}}
// Create fake k8s client
fakeClient := fake.NewClientBuilder().WithRuntimeObjects(fakeClientPod).Build()
fakeClient := fake.NewClientBuilder().WithRuntimeObjects(fakeClientPod, ns).Build()

// Create test consul server
consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) {
Expand Down Expand Up @@ -2576,6 +2583,7 @@ func TestEndpointsController_createServiceRegistrations_withTransparentProxy(t *
service *corev1.Service
expTaggedAddresses map[string]api.ServiceAddress
proxyMode api.ProxyMode
namespaceLabels map[string]string
expErr string
}{
"enabled globally, annotation not provided": {
Expand Down Expand Up @@ -2719,6 +2727,53 @@ func TestEndpointsController_createServiceRegistrations_withTransparentProxy(t *
},
expErr: "",
},
"disabled globally, namespace enabled, no annotation": {
globalEnabled: false,
service: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: serviceName,
Namespace: "default",
},
Spec: corev1.ServiceSpec{
ClusterIP: "10.0.0.1",
Ports: []corev1.ServicePort{
{
Port: 80,
},
},
},
},
proxyMode: api.ProxyModeTransparent,
expTaggedAddresses: map[string]api.ServiceAddress{
"virtual": {
Address: "10.0.0.1",
Port: 80,
},
},
namespaceLabels: map[string]string{annotationTransparentProxy: "true"},
expErr: "",
},
"enabled globally, namespace disabled, no annotation": {
globalEnabled: true,
service: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: serviceName,
Namespace: "default",
},
Spec: corev1.ServiceSpec{
ClusterIP: "10.0.0.1",
Ports: []corev1.ServicePort{
{
Port: 80,
},
},
},
},
proxyMode: api.ProxyModeDefault,
expTaggedAddresses: nil,
namespaceLabels: map[string]string{annotationTransparentProxy: "false"},
expErr: "",
},
// This case is impossible since we're always passing an endpoints object to this function,
// and Kubernetes will ensure that there is only an endpoints object if there is a service object.
// However, we're testing this case to check that we return an error in case we cannot get the service from k8s.
Expand Down Expand Up @@ -2927,11 +2982,15 @@ func TestEndpointsController_createServiceRegistrations_withTransparentProxy(t *
},
},
}
// Add the pods namespace.
ns := corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: pod.Namespace, Labels: c.namespaceLabels},
}
var fakeClient client.Client
if c.service != nil {
fakeClient = fake.NewClientBuilder().WithRuntimeObjects(pod, endpoints, c.service).Build()
fakeClient = fake.NewClientBuilder().WithRuntimeObjects(&ns, pod, endpoints, c.service).Build()
} else {
fakeClient = fake.NewClientBuilder().WithRuntimeObjects(pod, endpoints).Build()
fakeClient = fake.NewClientBuilder().WithRuntimeObjects(&ns, pod, endpoints).Build()
}

epCtrl := EndpointsController{
Expand Down
Loading