Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Jan 13, 2025
Merged
39 changes: 39 additions & 0 deletions pkg/file/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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...)
}
68 changes: 68 additions & 0 deletions tests/integration/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
}
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Loading