Skip to content

Commit

Permalink
Support API Rules on the same path and different methods (#27)
Browse files Browse the repository at this point in the history
* Rule validation for same path

* Fix panic

* Fix test

* Support for same paths

* Calculate actual state paths

* Refactor

* Add tests and refactor code
  • Loading branch information
cnvergence authored Sep 23, 2022
1 parent 9bd5d18 commit 953625e
Show file tree
Hide file tree
Showing 10 changed files with 510 additions and 31 deletions.
4 changes: 2 additions & 2 deletions controllers/api_controller_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ var _ = Describe("APIRule Controller", func() {
Expect(err).NotTo(HaveOccurred())
Expect(created.Status.APIRuleStatus.Code).To(Equal(gatewayv1beta1.StatusError))
Expect(created.Status.APIRuleStatus.Description).To(ContainSubstring("Multiple validation errors:"))
Expect(created.Status.APIRuleStatus.Description).To(ContainSubstring("Attribute \".spec.rules\": Multiple rules defined for the same path"))
Expect(created.Status.APIRuleStatus.Description).To(ContainSubstring("Attribute \".spec.rules\": multiple rules defined for the same path and method"))
Expect(created.Status.APIRuleStatus.Description).To(ContainSubstring("Attribute \".spec.rules[0].accessStrategies[0].config\": strategy: noop does not support configuration"))
Expect(created.Status.APIRuleStatus.Description).To(ContainSubstring("Attribute \".spec.rules[1].accessStrategies[0].config\": strategy: noop does not support configuration"))
Expect(created.Status.APIRuleStatus.Description).To(ContainSubstring("1 more error(s)..."))
Expand Down Expand Up @@ -660,7 +660,7 @@ func noConfigHandler(name string) *gatewayv1beta1.Handler {
}
}

//Converts a []interface{} to a string slice. Panics if given object is of other type.
// Converts a []interface{} to a string slice. Panics if given object is of other type.
func asStringSlice(in interface{}) []string {

inSlice := in.([]interface{})
Expand Down
36 changes: 36 additions & 0 deletions internal/processing/helpers.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package processing

import (
"fmt"

gatewayv1beta1 "github.com/kyma-incubator/api-gateway/api/v1beta1"
"github.com/kyma-incubator/api-gateway/internal/builders"
rulev1alpha1 "github.com/ory/oathkeeper-maester/api/v1alpha1"
k8sMeta "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand All @@ -18,6 +21,39 @@ func isSecured(rule gatewayv1beta1.Rule) bool {
return false
}

func hasPathDuplicates(rules []gatewayv1beta1.Rule) bool {
duplicates := map[string]bool{}
for _, rule := range rules {
if duplicates[rule.Path] {
return true
}
duplicates[rule.Path] = true
}

return false
}

func filterDuplicatePaths(rules []gatewayv1beta1.Rule) []gatewayv1beta1.Rule {
duplicates := make(map[string]bool)
var filteredRules []gatewayv1beta1.Rule
for _, rule := range rules {
if _, exists := duplicates[rule.Path]; !exists {
duplicates[rule.Path] = true
filteredRules = append(filteredRules, rule)
}
}

return filteredRules
}

func setAccessRuleKey(hasPathDuplicates bool, rule rulev1alpha1.Rule) string {
if hasPathDuplicates {
return fmt.Sprintf("%s:%s", rule.Spec.Match.URL, rule.Spec.Match.Methods)
}

return rule.Spec.Match.URL
}

func generateOwnerRef(api *gatewayv1beta1.APIRule) k8sMeta.OwnerReference {
return *builders.OwnerReference().
Name(api.ObjectMeta.Name).
Expand Down
9 changes: 5 additions & 4 deletions internal/processing/processing.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ type CorsConfig struct {
// CalculateRequiredState returns required state of all objects related to given api
func (f *Factory) CalculateRequiredState(api *gatewayv1beta1.APIRule) *State {
var res State

pathDuplicates := hasPathDuplicates(api.Spec.Rules)
res.accessRules = make(map[string]*rulev1alpha1.Rule)
for _, rule := range api.Spec.Rules {
if isSecured(rule) {
ar := generateAccessRule(api, rule, rule.AccessStrategies, f.additionalLabels, f.defaultDomainName)
res.accessRules[ar.Spec.Match.URL] = ar
res.accessRules[setAccessRuleKey(pathDuplicates, *ar)] = ar
}
}

Expand All @@ -83,9 +83,10 @@ func (f *Factory) GetActualState(ctx context.Context, api *gatewayv1beta1.APIRul
labels := make(map[string]string)
labels[OwnerLabelv1alpha1] = fmt.Sprintf("%s.%s", api.ObjectMeta.Name, api.ObjectMeta.Namespace)

pathDuplicates := hasPathDuplicates(api.Spec.Rules)
var state State

var vsList networkingv1beta1.VirtualServiceList

if err := f.client.List(ctx, &vsList, client.MatchingLabels(labels)); err != nil {
return nil, err
}
Expand All @@ -105,7 +106,7 @@ func (f *Factory) GetActualState(ctx context.Context, api *gatewayv1beta1.APIRul

for i := range arList.Items {
obj := arList.Items[i]
state.accessRules[obj.Spec.Match.URL] = &obj
state.accessRules[setAccessRuleKey(pathDuplicates, obj)] = &obj
}
return &state, nil
}
Expand Down
Loading

0 comments on commit 953625e

Please sign in to comment.