Skip to content

Commit

Permalink
[release-1.22] Use label istio.io/dataplane-mode=disabled for pod-lev…
Browse files Browse the repository at this point in the history 
…el ambient opt-out (#50850)

* Push some local test cleanups (#50653)

* Use `Label` suffixes

Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>

* Tidy test annotation handling

Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>

* Lints

Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>

---------

Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>

* Use label istio.io/dataplane-mode=disabled for pod-level ambient opt-out (#50804)

* Use label for opt-out

Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>

* Resync with master

Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>

* Fix tests

Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>

* Remove comment

Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>

---------

Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>

---------

Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
  • Loading branch information
@bleggett
bleggett committed May 4, 2024
1 parent d8b1810 commit 291cba8
Show file tree
Hide file tree
Showing 89 changed files with 387 additions and 570 deletions.
4 changes: 2 additions & 2 deletions cni/README.md
View file
Expand Up @@ -62,9 +62,9 @@ Istio CNI injection is currently based on the same Pod annotations used in init-

- plugin config "exclude namespaces" applies first
- ambient is enabled if:
- namespace label "istio.io/dataplane-mode" == "ambient" is required (may change for 'on-by-default' mode)
- namespace label "istio.io/dataplane-mode" == "ambient", and/or pod label "istio.io/dataplane-mode" == "ambient"
- "sidecar.istio.io/status" annotation is not present on the pod (created by injection of sidecar)
- "ambient.istio.io/redirection" is not "disabled"
- pod label "istio.io/dataplane-mode" is not "none"
- sidecar interception is enabled if:
- "istio-init" container is not present in the pod.
- istio-proxy container exists and
Expand Down
4 changes: 2 additions & 2 deletions cni/pkg/nodeagent/cni-watcher_test.go
View file
Expand Up @@ -118,7 +118,7 @@ func TestCNIPluginServer(t *testing.T) {

// label the namespace
labelsPatch := []byte(fmt.Sprintf(`{"metadata":{"labels":{"%s":"%s"}}}`,
constants.DataplaneMode, constants.DataplaneModeAmbient))
constants.DataplaneModeLabel, constants.DataplaneModeAmbient))
_, err := client.Kube().CoreV1().Namespaces().Patch(ctx, ns.Name,
types.MergePatchType, labelsPatch, metav1.PatchOptions{})
assert.NoError(t, err)
Expand Down Expand Up @@ -194,7 +194,7 @@ func TestCNIPluginServerPrefersCNIProvidedPodIP(t *testing.T) {

// label the namespace
labelsPatch := []byte(fmt.Sprintf(`{"metadata":{"labels":{"%s":"%s"}}}`,
constants.DataplaneMode, constants.DataplaneModeAmbient))
constants.DataplaneModeLabel, constants.DataplaneModeAmbient))
_, err := client.Kube().CoreV1().Namespaces().Patch(ctx, ns.Name,
types.MergePatchType, labelsPatch, metav1.PatchOptions{})
assert.NoError(t, err)
Expand Down
7 changes: 2 additions & 5 deletions cni/pkg/nodeagent/informers.go
View file
Expand Up @@ -119,7 +119,7 @@ func (s *InformerHandlers) GetAmbientPods() []*corev1.Pod {
func (s *InformerHandlers) enqueueNamespace(o controllers.Object) {
namespace := o.GetName()
labels := o.GetLabels()
matchAmbient := labels[constants.DataplaneMode] == constants.DataplaneModeAmbient
matchAmbient := labels[constants.DataplaneModeLabel] == constants.DataplaneModeAmbient
if matchAmbient {
log.Infof("Namespace %s is enabled in ambient mesh", namespace)
} else {
Expand Down Expand Up @@ -177,7 +177,7 @@ func getModeLabel(m map[string]string) string {
if m == nil {
return ""
}
return m[constants.DataplaneMode]
return m[constants.DataplaneModeLabel]
}

func (s *InformerHandlers) reconcilePod(input any) error {
Expand Down Expand Up @@ -207,13 +207,10 @@ func (s *InformerHandlers) reconcilePod(input any) error {
}
wasAnnotated := oldPod.Annotations != nil && oldPod.Annotations[constants.AmbientRedirection] == constants.AmbientRedirectionEnabled
isAnnotated := newPod.Annotations != nil && newPod.Annotations[constants.AmbientRedirection] == constants.AmbientRedirectionEnabled
isOpOut := newPod.Annotations != nil && newPod.Annotations[constants.AmbientRedirection] == constants.AmbientRedirectionDisabled
shouldBeEnabled := util.PodRedirectionEnabled(ns, newPod)

// We should check the latest annotation vs desired status
changeNeeded := isAnnotated != shouldBeEnabled
// Also need change for case user manually rolls out the pods
changeNeeded = changeNeeded || (wasAnnotated && isOpOut && !shouldBeEnabled)

log.Debugf("Pod %s events: wasAnnotated(%v), isAnnotated(%v), shouldBeEnabled(%v), changeNeeded(%v), oldPod(%+v), newPod(%+v)",
pod.Name, wasAnnotated, isAnnotated, shouldBeEnabled, changeNeeded, oldPod, newPod)
Expand Down
50 changes: 25 additions & 25 deletions cni/pkg/nodeagent/informers_test.go
View file
Expand Up @@ -76,7 +76,7 @@ func TestExistingPodAddedWhenNsLabeled(t *testing.T) {

// label the namespace
labelsPatch := []byte(fmt.Sprintf(`{"metadata":{"labels":{"%s":"%s"}}}`,
constants.DataplaneMode, constants.DataplaneModeAmbient))
constants.DataplaneModeLabel, constants.DataplaneModeAmbient))
_, err := client.Kube().CoreV1().Namespaces().Patch(ctx, ns.Name,
types.MergePatchType, labelsPatch, metav1.PatchOptions{})
assert.NoError(t, err)
Expand Down Expand Up @@ -139,7 +139,7 @@ func TestExistingPodAddedWhenDualStack(t *testing.T) {

// label the namespace
labelsPatch := []byte(fmt.Sprintf(`{"metadata":{"labels":{"%s":"%s"}}}`,
constants.DataplaneMode, constants.DataplaneModeAmbient))
constants.DataplaneModeLabel, constants.DataplaneModeAmbient))
_, err := client.Kube().CoreV1().Namespaces().Patch(ctx, ns.Name,
types.MergePatchType, labelsPatch, metav1.PatchOptions{})
assert.NoError(t, err)
Expand Down Expand Up @@ -190,7 +190,7 @@ func TestExistingPodNotAddedIfNoIPInAnyStatusField(t *testing.T) {

// label the namespace
labelsPatch := []byte(fmt.Sprintf(`{"metadata":{"labels":{"%s":"%s"}}}`,
constants.DataplaneMode, constants.DataplaneModeAmbient))
constants.DataplaneModeLabel, constants.DataplaneModeAmbient))
_, err := client.Kube().CoreV1().Namespaces().Patch(ctx, ns.Name,
types.MergePatchType, labelsPatch, metav1.PatchOptions{})
assert.NoError(t, err)
Expand Down Expand Up @@ -227,7 +227,7 @@ func TestExistingPodRemovedWhenNsUnlabeled(t *testing.T) {
ns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
// TODO: once we if the add pod bug, re-enable this and remove the patch below
// Labels: map[string]string{constants.DataplaneMode: constants.DataplaneModeAmbient},
// Labels: map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient},

}

Expand Down Expand Up @@ -258,7 +258,7 @@ func TestExistingPodRemovedWhenNsUnlabeled(t *testing.T) {
log.Debug("labeling namespace")
_, err := client.Kube().CoreV1().Namespaces().Patch(ctx, ns.Name,
types.MergePatchType, []byte(fmt.Sprintf(`{"metadata":{"labels":{"%s":"%s"}}}`,
constants.DataplaneMode, constants.DataplaneModeAmbient)), metav1.PatchOptions{})
constants.DataplaneModeLabel, constants.DataplaneModeAmbient)), metav1.PatchOptions{})
assert.NoError(t, err)

// wait for an update event
Expand All @@ -279,7 +279,7 @@ func TestExistingPodRemovedWhenNsUnlabeled(t *testing.T) {

// unlabel the namespace
labelsPatch := []byte(fmt.Sprintf(`{"metadata":{"labels":{"%s":null}}}`,
constants.DataplaneMode))
constants.DataplaneModeLabel))
_, err = client.Kube().CoreV1().Namespaces().Patch(ctx, ns.Name,
types.MergePatchType, labelsPatch, metav1.PatchOptions{})
assert.NoError(t, err)
Expand Down Expand Up @@ -319,7 +319,7 @@ func TestExistingPodRemovedWhenPodAnnotated(t *testing.T) {
ns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
// TODO: once we if the add pod bug, re-enable this and remove the patch below
// Labels: map[string]string{constants.DataplaneMode: constants.DataplaneModeAmbient},
// Labels: map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient},

}

Expand Down Expand Up @@ -350,7 +350,7 @@ func TestExistingPodRemovedWhenPodAnnotated(t *testing.T) {
log.Debug("labeling namespace")
_, err := client.Kube().CoreV1().Namespaces().Patch(ctx, ns.Name,
types.MergePatchType, []byte(fmt.Sprintf(`{"metadata":{"labels":{"%s":"%s"}}}`,
constants.DataplaneMode, constants.DataplaneModeAmbient)), metav1.PatchOptions{})
constants.DataplaneModeLabel, constants.DataplaneModeAmbient)), metav1.PatchOptions{})
assert.NoError(t, err)

// wait for an update event
Expand All @@ -369,11 +369,11 @@ func TestExistingPodRemovedWhenPodAnnotated(t *testing.T) {
mock.Anything,
).Once().Return(nil)

// annotate the pod
annotationsPatch := []byte(fmt.Sprintf(`{"metadata":{"annotations":{"%s":"%s"}}}`,
constants.AmbientRedirection, constants.AmbientRedirectionDisabled))
// label the pod for exclusion
labelsPatch := []byte(fmt.Sprintf(`{"metadata":{"labels":{"%s":"%s"}}}`,
constants.DataplaneModeLabel, constants.DataplaneModeNone))
_, err = client.Kube().CoreV1().Pods(pod.Namespace).Patch(ctx, pod.Name,
types.MergePatchType, annotationsPatch, metav1.PatchOptions{})
types.MergePatchType, labelsPatch, metav1.PatchOptions{})
assert.NoError(t, err)

// wait for an update events
Expand All @@ -384,7 +384,7 @@ func TestExistingPodRemovedWhenPodAnnotated(t *testing.T) {

assertPodNotAnnotated(t, client, pod)

// patch a test label to emulate a non-annotation POD update event
// patch a test label to emulate a POD update event
_, err = client.Kube().CoreV1().Pods(pod.Namespace).Patch(ctx, pod.Name,
types.MergePatchType, []byte(`{"metadata":{"labels":{"test":"update"}}}`), metav1.PatchOptions{})
assert.NoError(t, err)
Expand Down Expand Up @@ -421,7 +421,7 @@ func TestAmbientEnabledReturnsPodIfEnabled(t *testing.T) {
ns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{constants.DataplaneMode: constants.DataplaneModeAmbient},
Labels: map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient},
},
}

Expand Down Expand Up @@ -449,10 +449,10 @@ func TestAmbientEnabledReturnsNoPodIfNotEnabled(t *testing.T) {

pod := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "test",
UID: "1234",
Annotations: map[string]string{constants.AmbientRedirection: constants.AmbientRedirectionDisabled},
Name: "test",
Namespace: "test",
UID: "1234",
Labels: map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeNone},
},
Spec: corev1.PodSpec{
NodeName: NodeName,
Expand All @@ -464,7 +464,7 @@ func TestAmbientEnabledReturnsNoPodIfNotEnabled(t *testing.T) {
ns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{constants.DataplaneMode: constants.DataplaneModeAmbient},
Labels: map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient},
},
}

Expand Down Expand Up @@ -493,10 +493,10 @@ func TestAmbientEnabledReturnsErrorIfBogusNS(t *testing.T) {

pod := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "test",
UID: "1234",
Annotations: map[string]string{constants.AmbientRedirection: constants.AmbientRedirectionDisabled},
Name: "test",
Namespace: "test",
UID: "1234",
Labels: map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeNone},
},
Spec: corev1.PodSpec{
NodeName: NodeName,
Expand All @@ -508,7 +508,7 @@ func TestAmbientEnabledReturnsErrorIfBogusNS(t *testing.T) {
ns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{constants.DataplaneMode: constants.DataplaneModeAmbient},
Labels: map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient},
},
}

Expand Down Expand Up @@ -552,7 +552,7 @@ func TestExistingPodAddedWhenItPreExists(t *testing.T) {
ns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{constants.DataplaneMode: constants.DataplaneModeAmbient},
Labels: map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient},
},
}

Expand Down
24 changes: 11 additions & 13 deletions cni/pkg/plugin/plugin_test.go
View file
Expand Up @@ -232,7 +232,7 @@ func TestCmdAddAmbientEnabledOnNS(t *testing.T) {
cniConf := buildMockConf(true, url)

pod, ns := buildFakePodAndNSForClient()
ns.ObjectMeta.Labels = map[string]string{constants.DataplaneMode: constants.DataplaneModeAmbient}
ns.ObjectMeta.Labels = map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient}

testDoAddRun(t, cniConf, testNSName, pod, ns)

Expand All @@ -247,7 +247,7 @@ func TestCmdAddAmbientEnabledOnNSServerFails(t *testing.T) {
cniConf := buildMockConf(true, url)

pod, ns := buildFakePodAndNSForClient()
ns.ObjectMeta.Labels = map[string]string{constants.DataplaneMode: constants.DataplaneModeAmbient}
ns.ObjectMeta.Labels = map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient}

testCmdAddExpectFail(t, cniConf, pod, ns)

Expand All @@ -268,7 +268,7 @@ func TestCmdAddPodWithProxySidecarAmbientEnabledNS(t *testing.T) {

pod.Spec.Containers = []corev1.Container{app, proxy}
pod.ObjectMeta.Annotations = map[string]string{annotation.SidecarStatus.Name: "true"}
ns.ObjectMeta.Labels = map[string]string{constants.DataplaneMode: constants.DataplaneModeAmbient}
ns.ObjectMeta.Labels = map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient}

testDoAddRun(t, cniConf, testNSName, pod, ns)

Expand All @@ -288,7 +288,7 @@ func TestCmdAddPodWithGenericSidecar(t *testing.T) {
app := corev1.Container{Name: "app"}

pod.Spec.Containers = []corev1.Container{app, proxy}
ns.ObjectMeta.Labels = map[string]string{constants.DataplaneMode: constants.DataplaneModeAmbient}
ns.ObjectMeta.Labels = map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient}

testDoAddRun(t, cniConf, testNSName, pod, ns)

Expand All @@ -297,22 +297,22 @@ func TestCmdAddPodWithGenericSidecar(t *testing.T) {
assert.Equal(t, wasCalled, true)
}

func TestCmdAddPodDisabledAnnotation(t *testing.T) {
func TestCmdAddPodDisabledLabel(t *testing.T) {
url, serverClose := setupCNIEventClientWithMockServer(false)

cniConf := buildMockConf(true, url)

pod, ns := buildFakePodAndNSForClient()

app := corev1.Container{Name: "app"}
ns.ObjectMeta.Labels = map[string]string{constants.DataplaneMode: constants.AmbientRedirectionEnabled}
pod.ObjectMeta.Annotations = map[string]string{constants.DataplaneMode: constants.AmbientRedirectionDisabled}
ns.ObjectMeta.Labels = map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient}
pod.ObjectMeta.Labels = map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeNone}
pod.Spec.Containers = []corev1.Container{app}

testDoAddRun(t, cniConf, testNSName, pod, ns)

wasCalled := serverClose()
// Pod has an explicit opt-out annotation, should not be added to ambient mesh
// Pod has an explicit opt-out label, should not be added to ambient mesh
assert.Equal(t, wasCalled, false)
}

Expand All @@ -324,15 +324,13 @@ func TestCmdAddPodEnabledNamespaceDisabled(t *testing.T) {
pod, ns := buildFakePodAndNSForClient()

app := corev1.Container{Name: "app"}
pod.ObjectMeta.Annotations = map[string]string{constants.DataplaneMode: constants.AmbientRedirectionEnabled}
pod.ObjectMeta.Labels = map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient}
pod.Spec.Containers = []corev1.Container{app}

testDoAddRun(t, cniConf, testNSName, pod, ns)

wasCalled := serverClose()
// Currently, we do not allow individual pod opt-in to ambient if namespace is not labeled, so pod
// shouls not be added to ambient
assert.Equal(t, wasCalled, false)
assert.Equal(t, wasCalled, true)
}

func TestCmdAddPodInExcludedNamespace(t *testing.T) {
Expand All @@ -345,7 +343,7 @@ func TestCmdAddPodInExcludedNamespace(t *testing.T) {

app := corev1.Container{Name: "app"}
ns.ObjectMeta.Name = excludedNS
ns.ObjectMeta.Labels = map[string]string{constants.DataplaneMode: constants.AmbientRedirectionEnabled}
ns.ObjectMeta.Labels = map[string]string{constants.DataplaneModeLabel: constants.AmbientRedirectionEnabled}

pod.ObjectMeta.Namespace = excludedNS
pod.Spec.Containers = []corev1.Container{app}
Expand Down
11 changes: 4 additions & 7 deletions cni/pkg/util/podutil.go
View file
Expand Up @@ -40,23 +40,20 @@ var annotationRemovePatch = []byte(fmt.Sprintf(
constants.AmbientRedirection,
))

// TODO: we should use the upstream istio version of this function.
// PodRedirectionEnabled determines if a pod should or should not be configured
// to have traffic redirected thru the node proxy.
func PodRedirectionEnabled(namespace *corev1.Namespace, pod *corev1.Pod) bool {
if !(namespace.GetLabels()[constants.DataplaneMode] == constants.DataplaneModeAmbient ||
pod.GetLabels()[constants.DataplaneMode] == constants.DataplaneModeAmbient) {
if !(namespace.GetLabels()[constants.DataplaneModeLabel] == constants.DataplaneModeAmbient ||
pod.GetLabels()[constants.DataplaneModeLabel] == constants.DataplaneModeAmbient) {
// Neither namespace nor pod has ambient mode enabled
return false
}
if podHasSidecar(pod) {
// Ztunnel and sidecar for a single pod is currently not supported; opt out.
return false
}
// TODO: delete this block when there is a way for users to signal the intent to exclude
// a pod from ambient that does not require the use of this annotation
if pod.Annotations[constants.AmbientRedirection] == constants.AmbientRedirectionDisabled {
// Pod explicitly asked to not have redirection enabled
if pod.GetLabels()[constants.DataplaneModeLabel] == constants.DataplaneModeNone {
// Pod explicitly asked to not have ambient redirection enabled
return false
}
return true
Expand Down

0 comments on commit 291cba8

Please sign in to comment.