fix: returning a validation error in case of wrong configuration. (#157)

* fix: returning a validation error in case of wrong configuration.

As of now, while using deck to create a
route-scoped plugin with a `service` field
defined, the sync goes through the first
time but errors out subsequently. Under a
route, we do not want the user to add the
`service` field for a plugin. Thus, we are
checking for this specific scenario and
erroring out, instead of letting the sync
action go through.

For: Kong/deck#1353

* fix: added checks for all conflicting nested configs in plugins.

A foreign key nested under a plugin of a different
scope would error out. This would make sure that a
sync does not go through when wrong configurations
are passed via deck. An example would be adding a
`service` field in a plugin, nested under a route.
This change ensures more such issues do not come up.

* test: added integration test for validations related to scoped plugins

* chore: removed redundant code lines

* chore: removed stray code lines from test

* test: fixed tests and added more for kong <3.0

* chore: addressed PR comments

pkg/file/builder.go

@@ -19,6 +19,13 @@ import (
 
 const ratelimitingAdvancedPluginName = "rate-limiting-advanced"
 
+const (
+	primaryRelationConsumer      = "consumer"
+	primaryRelationConsumerGroup = "consumer-group"
+	primaryRelationRoute         = "route"
+	primaryRelationService       = "service"
+)
+
 type stateBuilder struct {
 	targetContent   *Content
 	rawState        *utils.KongRawState
@@ -488,6 +495,10 @@ func (b *stateBuilder) consumers() {
 		// plugins for the Consumer
 		var plugins []FPlugin
 		for _, p := range c.Plugins {
+			if err := checkForNestedForeignKeys(p.Plugin, primaryRelationConsumer); err != nil {
+				b.err = err
+				return
+			}
 			p.Consumer = utils.GetConsumerReference(c.Consumer)
 			plugins = append(plugins, *p)
 		}
@@ -920,6 +931,9 @@ func (b *stateBuilder) ingestService(s *FService) error {
 	// plugins for the service
 	var plugins []FPlugin
 	for _, p := range s.Plugins {
+		if err := checkForNestedForeignKeys(p.Plugin, primaryRelationService); err != nil {
+			return err
+		}
 		p.Service = utils.GetServiceReference(s.Service)
 		plugins = append(plugins, *p)
 	}
@@ -1436,6 +1450,9 @@ func (b *stateBuilder) ingestRoute(r FRoute) error {
 	// plugins for the route
 	var plugins []FPlugin
 	for _, p := range r.Plugins {
+		if err := checkForNestedForeignKeys(p.Plugin, primaryRelationRoute); err != nil {
+			return err
+		}
 		p.Route = utils.GetRouteReference(r.Route)
 		plugins = append(plugins, *p)
 	}
@@ -1570,3 +1587,25 @@ func defaulter(
 	}
 	return defaulter, nil
 }
+
+func checkForNestedForeignKeys(plugin kong.Plugin, primary string) error {
+	var errs []error
+
+	if primary != primaryRelationConsumer && plugin.Consumer != nil && !utils.Empty(plugin.Consumer.ID) {
+		errs = append(errs, fmt.Errorf("nesting consumer (%v) under %v-scoped plugin plugin (%v) is not allowed",
+			*plugin.Consumer.ID, primary, *plugin.Name))
+	}
+	if primary != primaryRelationRoute && plugin.Route != nil && !utils.Empty(plugin.Route.ID) {
+		errs = append(errs, fmt.Errorf("nesting route (%v) under %v-scoped plugin (%v) is not allowed",
+			*plugin.Route.ID, primary, *plugin.Name))
+	}
+	if primary != primaryRelationService && plugin.Service != nil && !utils.Empty(plugin.Service.ID) {
+		errs = append(errs, fmt.Errorf("nesting service (%v) under %v-scoped plugin (%v) is not allowed",
+			*plugin.Service.ID, primary, *plugin.Name))
+	}
+	if primary != primaryRelationConsumerGroup && plugin.ConsumerGroup != nil && !utils.Empty(plugin.ConsumerGroup.ID) {
+		errs = append(errs, fmt.Errorf("nesting consumer-group (%v) under %v-scoped plugin (%v) is not allowed",
+			*plugin.ConsumerGroup.ID, primary, *plugin.Name))
+	}
+	return errors.Join(errs...)
+}

tests/integration/sync_test.go

@@ -6114,3 +6114,71 @@ func Test_Sync_PluginDeprecatedFields38x(t *testing.T) {
 		})
 	}
 }
+
+func Test_Sync_Scoped_Plugins(t *testing.T) {
+	runWhen(t, "kong", "<3.0.0")
+	setup(t)
+
+	tests := []struct {
+		name          string
+		file          string
+		errorExpected string
+	}{
+		{
+			name:          "syncing route-scoped plugin with service field set",
+			file:          "testdata/sync/036-scoped-plugins-validation/route-plugins.yaml",
+			errorExpected: "building state: nesting service (example-service) under route-scoped plugin (request-transformer) is not allowed",
+		},
+		{
+			name:          "syncing service-scoped plugin with route and consumer field set",
+			file:          "testdata/sync/036-scoped-plugins-validation/service-plugins.yaml",
+			errorExpected: "building state: nesting consumer (foo) under service-scoped plugin plugin (request-transformer) is not allowed\nnesting route (example-route) under service-scoped plugin (request-transformer) is not allowed",
+		},
+		{
+			name:          "syncing consumer-scoped plugin with service and route field set",
+			file:          "testdata/sync/036-scoped-plugins-validation/consumer-plugins.yaml",
+			errorExpected: "building state: nesting route (example-route) under consumer-scoped plugin (request-transformer) is not allowed\nnesting service (example-service) under consumer-scoped plugin (request-transformer) is not allowed",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			err := sync(tc.file)
+			require.Equal(t, tc.errorExpected, err.Error())
+		})
+	}
+}
+
+func Test_Sync_Scoped_Plugins_3x(t *testing.T) {
+	runWhen(t, "kong", ">=3.0.0")
+	setup(t)
+
+	tests := []struct {
+		name          string
+		file          string
+		errorExpected string
+	}{
+		{
+			name:          "syncing route-scoped plugin with service field set",
+			file:          "testdata/sync/036-scoped-plugins-validation/3x/route-plugins.yaml",
+			errorExpected: "building state: nesting service (example-service) under route-scoped plugin (request-transformer) is not allowed",
+		},
+		{
+			name:          "syncing service-scoped plugin with route and consumer field set",
+			file:          "testdata/sync/036-scoped-plugins-validation/3x/service-plugins.yaml",
+			errorExpected: "building state: nesting consumer (foo) under service-scoped plugin plugin (request-transformer) is not allowed\nnesting route (example-route) under service-scoped plugin (request-transformer) is not allowed",
+		},
+		{
+			name:          "syncing consumer-scoped plugin with service and route field set",
+			file:          "testdata/sync/036-scoped-plugins-validation/3x/consumer-plugins.yaml",
+			errorExpected: "building state: nesting route (example-route) under consumer-scoped plugin (request-transformer) is not allowed\nnesting service (example-service) under consumer-scoped plugin (request-transformer) is not allowed",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			err := sync(tc.file)
+			require.Equal(t, tc.errorExpected, err.Error())
+		})
+	}
+}

tests/integration/testdata/sync/036-scoped-plugins-validation/3x/consumer-plugins.yaml

@@ -0,0 +1,19 @@
+_format_version: "3.0"
+services:
+- name: example-service
+  port: 3200
+  protocol: http
+  routes:
+  - name: example-route
+    paths:
+    - ~/r1
+consumers:
+- username: foo
+  plugins:
+  - name: request-transformer
+    route: example-route
+    service: example-service
+    config:
+        add:
+          querystring:
+          - test

tests/integration/testdata/sync/036-scoped-plugins-validation/3x/route-plugins.yaml

@@ -0,0 +1,16 @@
+_format_version: "3.0"
+services:
+- name: example-service
+  port: 3200
+  protocol: http
+  routes:
+  - name: example-route
+    paths:
+    - ~/r1
+    plugins:
+    - name: request-transformer
+      service: example-service
+      config:
+        add:
+          querystring:
+          - test

tests/integration/testdata/sync/036-scoped-plugins-validation/3x/service-plugins.yaml

@@ -0,0 +1,17 @@
+_format_version: "3.0"
+services:
+- name: example-service
+  port: 3200
+  protocol: http
+  plugins:
+    - name: request-transformer
+      route: example-route
+      consumer: foo
+      config:
+        add:
+          querystring:
+          - test
+  routes:
+  - name: example-route
+    paths:
+    - ~/r1

tests/integration/testdata/sync/036-scoped-plugins-validation/consumer-plugins.yaml

@@ -0,0 +1,19 @@
+_format_version: "1.0"
+services:
+- name: example-service
+  port: 3200
+  protocol: http
+  routes:
+  - name: example-route
+    paths:
+    - ~/r1
+consumers:
+- username: foo
+  plugins:
+  - name: request-transformer
+    route: example-route
+    service: example-service
+    config:
+        add:
+          querystring:
+          - test

tests/integration/testdata/sync/036-scoped-plugins-validation/route-plugins.yaml

@@ -0,0 +1,16 @@
+_format_version: "1.0"
+services:
+- name: example-service
+  port: 3200
+  protocol: http
+  routes:
+  - name: example-route
+    paths:
+    - ~/r1
+    plugins:
+    - name: request-transformer
+      service: example-service
+      config:
+        add:
+          querystring:
+          - test

tests/integration/testdata/sync/036-scoped-plugins-validation/service-plugins.yaml

@@ -0,0 +1,17 @@
+_format_version: "1.0"
+services:
+- name: example-service
+  port: 3200
+  protocol: http
+  plugins:
+    - name: request-transformer
+      route: example-route
+      consumer: foo
+      config:
+        add:
+          querystring:
+          - test
+  routes:
+  - name: example-route
+    paths:
+    - ~/r1