diff --git a/api/v1alpha1/dynamicenv_webhook.go b/api/v1alpha1/dynamicenv_webhook.go index 08cf70a..9d276c8 100644 --- a/api/v1alpha1/dynamicenv_webhook.go +++ b/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 +} diff --git a/api/v1alpha1/dynamicenv_webhook_test.go b/api/v1alpha1/dynamicenv_webhook_test.go index a9566f8..3574477 100644 --- a/api/v1alpha1/dynamicenv_webhook_test.go +++ b/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", + ), ) }) }) diff --git a/api/v1alpha1/fixtures/disallowed-modifications-using-same-deployment-in-subset-and-consumer-new.yaml b/api/v1alpha1/fixtures/disallowed-modifications-using-same-deployment-in-subset-and-consumer-new.yaml new file mode 100644 index 0000000..4911faf --- /dev/null +++ b/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" diff --git a/api/v1alpha1/fixtures/disallowed-modifications-using-same-deployment-in-subset-and-consumer-old.yaml b/api/v1alpha1/fixtures/disallowed-modifications-using-same-deployment-in-subset-and-consumer-old.yaml new file mode 100644 index 0000000..f1c9400 --- /dev/null +++ b/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"