Prevent using deployment in more than one subset/consumer (#22)

api/v1alpha1/dynamicenv_webhook.go

@@ -18,6 +18,7 @@ package v1alpha1
 
 import (
 	"fmt"
+	"k8s.io/utils/strings/slices"
 	"reflect"
 
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -142,6 +143,10 @@ func (de *DynamicEnv) validateSubsetsProperties() error {
 			return field.Invalid(field.NewPath("spec").Child("Subsets").Key(s.Name).Child("initContainers"), s.InitContainers, msg)
 		}
 	}
+	if err := validateNoDuplicateDeployment(subsets); err != nil {
+		msg := "It seems that the following deployment appears in more then one subset/consumer: " + err.Error()
+		return field.Invalid(field.NewPath("spec"), de.Spec, msg)
+	}
 	return nil
 }
 
@@ -234,3 +239,15 @@ func validateUniqueContainerNames(containers []ContainerOverrides) bool {
 	}
 	return len(m) == len(containers)
 }
+
+func validateNoDuplicateDeployment(subsets []Subset) error {
+	var uniques []string
+	for _, s := range subsets {
+		combined := fmt.Sprintf("%s/%s", s.Namespace, s.Name)
+		if slices.Contains(uniques, combined) {
+			return fmt.Errorf("%s", combined)
+		}
+		uniques = append(uniques, combined)
+	}
+	return nil
+}

api/v1alpha1/dynamicenv_webhook_test.go

@@ -28,6 +28,7 @@ import (
 )
 
 var zeroReplicas int32 = 0
+var oneReplica int32 = 1
 
 func mkDynamicEnvFromYamlFile(fileName string) (de DynamicEnv, err error) {
 	sourceFile, err := os.Open(fileName)
@@ -354,6 +355,151 @@ var _ = Describe("Validating Webhook", func() {
 				},
 				"0 replicas",
 			),
+			Entry(
+				"duplicate deployment in subsets",
+				&DynamicEnv{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "my-de",
+						Namespace: "default",
+					},
+					Spec: DynamicEnvSpec{
+						IstioMatches: []IstioMatch{
+							{
+								Headers: map[string]StringMatch{
+									"name": {
+										Exact: "my_name",
+									},
+								},
+							},
+						},
+						Consumers: nil,
+						Subsets: []Subset{
+							{
+								Name:           "somename",
+								Namespace:      "ns",
+								Replicas:       &oneReplica,
+								DefaultVersion: "version",
+								Containers: []ContainerOverrides{
+									{
+										ContainerName: "main",
+										Image:         "image",
+									},
+								},
+							},
+							{
+								Name:           "somename",
+								Namespace:      "ns",
+								Replicas:       &oneReplica,
+								DefaultVersion: "version",
+								Containers: []ContainerOverrides{
+									{
+										ContainerName: "main",
+										Image:         "image",
+									},
+								},
+							},
+						},
+					},
+				},
+				"appears in more then one subset/consumer",
+			),
+			Entry(
+				"duplicate deployment in consumers",
+				&DynamicEnv{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "my-de",
+						Namespace: "default",
+					},
+					Spec: DynamicEnvSpec{
+						IstioMatches: []IstioMatch{
+							{
+								Headers: map[string]StringMatch{
+									"name": {
+										Exact: "my_name",
+									},
+								},
+							},
+						},
+						Subsets: nil,
+						Consumers: []Subset{
+							{
+								Name:           "somename",
+								Namespace:      "ns",
+								Replicas:       &oneReplica,
+								DefaultVersion: "version",
+								Containers: []ContainerOverrides{
+									{
+										ContainerName: "main",
+										Image:         "image",
+									},
+								},
+							},
+							{
+								Name:           "somename",
+								Namespace:      "ns",
+								Replicas:       &oneReplica,
+								DefaultVersion: "version",
+								Containers: []ContainerOverrides{
+									{
+										ContainerName: "main",
+										Image:         "image",
+									},
+								},
+							},
+						},
+					},
+				},
+				"appears in more then one subset/consumer",
+			),
+			Entry(
+				"duplicate deployment between subsets and consumers",
+				&DynamicEnv{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "my-de",
+						Namespace: "default",
+					},
+					Spec: DynamicEnvSpec{
+						IstioMatches: []IstioMatch{
+							{
+								Headers: map[string]StringMatch{
+									"name": {
+										Exact: "my_name",
+									},
+								},
+							},
+						},
+						Subsets: []Subset{
+							{
+								Name:           "somename",
+								Namespace:      "ns",
+								Replicas:       &oneReplica,
+								DefaultVersion: "version",
+								Containers: []ContainerOverrides{
+									{
+										ContainerName: "main",
+										Image:         "image",
+									},
+								},
+							},
+						},
+						Consumers: []Subset{
+							{
+								Name:           "somename",
+								Namespace:      "ns",
+								Replicas:       &oneReplica,
+								DefaultVersion: "version",
+								Containers: []ContainerOverrides{
+									{
+										ContainerName: "main",
+										Image:         "image",
+									},
+								},
+							},
+						},
+					},
+				},
+				"appears in more then one subset/consumer",
+			),
 		)
 
 		DescribeTable(
@@ -461,6 +607,12 @@ var _ = Describe("Validating Webhook", func() {
 				"fixtures/disallowed-modifications-modifying-to-zero-replicas-new.yaml",
 				"0 replicas",
 			),
+			Entry(
+				"Sharing deployment between subsets/consumers",
+				"fixtures/disallowed-modifications-using-same-deployment-in-subset-and-consumer-old.yaml",
+				"fixtures/disallowed-modifications-using-same-deployment-in-subset-and-consumer-new.yaml",
+				"appears in more then one subset/consumer",
+			),
 		)
 	})
 })

api/v1alpha1/fixtures/disallowed-modifications-using-same-deployment-in-subset-and-consumer-new.yaml

@@ -0,0 +1,26 @@
+---
+apiVersion: "riskified.com/v1alpha1"
+kind: "DynamicEnv"
+metadata:
+  name: "dynamicenv-vs-with-multiple-services"
+  namespace: "default"
+spec:
+  istioMatches:
+    - headers:
+        end-user:
+          prefix: "jason"
+  subsets:
+    - name: "details"
+      namespace: "vs-with-multiple-services"
+      containers:
+        - containerName: "details"
+          image: "docker.io/istio/examples-bookinfo-details-v2:1.16.2"
+          env:
+            - name: "TOPIC_NAME"
+              value: "test"
+  consumers:
+    - name: "details"
+      namespace: "vs-with-multiple-services"
+      containers:
+        - containerName: "details"
+          image: "alternative-image:v1"

api/v1alpha1/fixtures/disallowed-modifications-using-same-deployment-in-subset-and-consumer-old.yaml

@@ -0,0 +1,26 @@
+---
+apiVersion: "riskified.com/v1alpha1"
+kind: "DynamicEnv"
+metadata:
+  name: "dynamicenv-vs-with-multiple-services"
+  namespace: "default"
+spec:
+  istioMatches:
+    - headers:
+        end-user:
+          prefix: "jason"
+  subsets:
+    - name: "details"
+      namespace: "vs-with-multiple-services"
+      containers:
+        - containerName: "details"
+          image: "docker.io/istio/examples-bookinfo-details-v2:1.16.2"
+          env:
+            - name: "TOPIC_NAME"
+              value: "test"
+  consumers:
+    - name: "details-work"
+      namespace: "vs-with-multiple-services"
+      containers:
+        - containerName: "details"
+          image: "alternative-image:v1"