Skip to content

Commit

Permalink
Push some local test cleanups (istio#50653)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@bleggett
bleggett committed May 3, 2024
1 parent bbe00ab commit ba40f7f
Show file tree
Hide file tree
Showing 48 changed files with 382 additions and 396 deletions.
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
4 changes: 2 additions & 2 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
24 changes: 12 additions & 12 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 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 @@ -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 @@ -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
16 changes: 8 additions & 8 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 @@ -305,8 +305,8 @@ func TestCmdAddPodDisabledAnnotation(t *testing.T) {
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.AmbientRedirectionEnabled}
pod.ObjectMeta.Annotations = map[string]string{constants.DataplaneModeLabel: constants.AmbientRedirectionDisabled}
pod.Spec.Containers = []corev1.Container{app}

testDoAddRun(t, cniConf, testNSName, pod, ns)
Expand All @@ -324,7 +324,7 @@ 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.Annotations = map[string]string{constants.DataplaneModeLabel: constants.AmbientRedirectionEnabled}
pod.Spec.Containers = []corev1.Container{app}

testDoAddRun(t, cniConf, testNSName, pod, ns)
Expand All @@ -345,7 +345,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
7 changes: 5 additions & 2 deletions cni/pkg/util/podutil.go
View file
Expand Up @@ -44,14 +44,17 @@ var annotationRemovePatch = []byte(fmt.Sprintf(
// 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 {
// Namespace does not have ambient mode enabled
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
return false
Expand Down
157 changes: 157 additions & 0 deletions cni/pkg/util/podutil_test.go
View file
Expand Up @@ -20,6 +20,8 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"istio.io/api/annotation"
"istio.io/istio/pkg/config/constants"
"istio.io/istio/pkg/test/util/assert"
)

Expand Down Expand Up @@ -75,3 +77,158 @@ func TestGetPodIPsIfNoPodIPPresent(t *testing.T) {
podIPs := GetPodIPsIfPresent(pod)
assert.Equal(t, len(podIPs), 0)
}

func TestPodRedirectionEnabled(t *testing.T) {
var (
ambientEnabledLabel = map[string]string{constants.DataplaneModeLabel: constants.DataplaneModeAmbient}
ambientDisabledAnnotation = map[string]string{constants.AmbientRedirection: constants.AmbientRedirectionDisabled}
sidecarStatusAnnotation = map[string]string{annotation.SidecarStatus.Name: "test"}

namespaceWithAmbientEnabledLabel = &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: ambientEnabledLabel,
},
}

unlabelledNamespace = &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
}

podWithAmbientEnabledLabel = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "test",
Labels: ambientEnabledLabel,
},
}

unlabelledPod = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "test",
},
}

podWithSidecar = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "test",
Annotations: sidecarStatusAnnotation,
},
}

podWithAmbientDisabledAnnotation = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "test",
Annotations: ambientDisabledAnnotation,
},
}

podWithAmbientEnabledLabelAndAmbientDisabledAnnotation = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "test",
Labels: ambientEnabledLabel,
Annotations: ambientDisabledAnnotation,
},
}
podWithSidecarAndAmbientEnabledLabel = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "test",
Labels: ambientEnabledLabel,
Annotations: sidecarStatusAnnotation,
},
}
)

type args struct {
namespace *corev1.Namespace
pod *corev1.Pod
}
tests := []struct {
name string
args args
want bool
}{
{
name: "ambient mode enabled for namespace",
args: args{
namespace: namespaceWithAmbientEnabledLabel,
pod: unlabelledPod,
},
want: true,
},
{
name: "ambient mode enabled for pod",
args: args{
namespace: unlabelledNamespace,
pod: podWithAmbientEnabledLabel,
},
want: true,
},
{
name: "ambient mode enabled for both namespace and pod",
args: args{
namespace: namespaceWithAmbientEnabledLabel,
pod: podWithAmbientEnabledLabel,
},
want: true,
},
{
name: "ambient mode enabled for neither namespace nor pod",
args: args{
namespace: unlabelledNamespace,
pod: unlabelledPod,
},
want: false,
},
{
name: "pod has sidecar and namespace has ambient enabled",
args: args{
namespace: namespaceWithAmbientEnabledLabel,
pod: podWithSidecar,
},
want: false,
},
// TODO: when there exists a means for users to signal the intent to exclude a pod from ambient without requiring the use of
// the ambient redirection annotation, this annotation should no longer be checked by this function and this case should return 'true'
{
name: "pod has annotation to disable ambient redirection",
args: args{
namespace: namespaceWithAmbientEnabledLabel,
pod: podWithAmbientDisabledAnnotation,
},
want: false,
},
// TODO: when there exists a means for users to signal the intent to exclude a pod from ambient without requiring the use of
// the ambient redirection annotation, this annotation should no longer be checked by this function and this case should return 'true'
{
name: "pod has label to enable ambient mode and annotation to disable ambient redirection",
args: args{
namespace: namespaceWithAmbientEnabledLabel,
pod: podWithAmbientEnabledLabelAndAmbientDisabledAnnotation,
},
want: false,
},
{
name: "pod has sidecar, pod has ambient mode label",
args: args{
namespace: unlabelledNamespace,
pod: podWithSidecarAndAmbientEnabledLabel,
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := PodRedirectionEnabled(tt.args.namespace, tt.args.pod); got != tt.want {
t.Errorf("PodRedirectionEnabled() = %v, want %v", got, tt.want)
}
})
}
}

0 comments on commit ba40f7f

Please sign in to comment.