Skip to content

Commit

Permalink
v2alpha1 CORS and Hosts (#1157)
Browse files Browse the repository at this point in the history
* CORS policy in v2alpha1

* Revert to v1beta1

* Tests

* Tests

* Add test for multiple hosts

* Update API

* Revert "Update API"

This reverts commit 94e4eb0.

* review changes

* Fix unit test

* Add unit tests

* Adapt the doc

* Update docs/user/custom-resources/apirule/v2alpha1/04-10-apirule-custom-resource.md

Co-authored-by: Natalia Sitko <[email protected]>

---------

Co-authored-by: Tim Riffer <[email protected]>
Co-authored-by: Natalia Sitko <[email protected]>
  • Loading branch information
3 people authored Jul 15, 2024
1 parent 9467685 commit 34e5a44
Show file tree
Hide file tree
Showing 15 changed files with 903 additions and 44 deletions.
9 changes: 9 additions & 0 deletions apis/gateway/v2alpha1/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,12 @@ func (r *Rule) ContainsAccessStrategyJwt() bool {
func (r *Rule) ContainsNoAuth() bool {
return r.NoAuth != nil
}

func ConvertHttpMethodsToStrings(methods []HttpMethod) []string {
strings := make([]string, len(methods))
for i, method := range methods {
strings[i] = string(method)
}

return strings
}
10 changes: 5 additions & 5 deletions controllers/gateway/api_controller_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ var _ = Describe("APIRule Controller", Serial, func() {
g.Expect(len(vs.Name) > len(apiRuleName)).To(BeTrue())

expectedSpec := builders.VirtualServiceSpec().
Host(serviceHost).
AddHost(serviceHost).
Gateway(testGatewayURL).
HTTP(builders.HTTPRoute().
Match(builders.MatchRequest().Uri().Regex(testPath)).
Expand Down Expand Up @@ -591,7 +591,7 @@ var _ = Describe("APIRule Controller", Serial, func() {
vs := vsList.Items[0]

expectedSpec := builders.VirtualServiceSpec().
Host(serviceHost).
AddHost(serviceHost).
Gateway(testGatewayURL).
HTTP(builders.HTTPRoute().
Match(builders.MatchRequest().Uri().Regex("/img")).
Expand Down Expand Up @@ -733,7 +733,7 @@ var _ = Describe("APIRule Controller", Serial, func() {
vs := vsList.Items[0]

expectedSpec := builders.VirtualServiceSpec().
Host(serviceHost).
AddHost(serviceHost).
Gateway(testGatewayURL).
HTTP(builders.HTTPRoute().
Match(builders.MatchRequest().Uri().Regex("/img").MethodRegEx(http.MethodGet)).
Expand Down Expand Up @@ -1042,7 +1042,7 @@ var _ = Describe("APIRule Controller", Serial, func() {
vs := vsList.Items[0]

expectedSpec := builders.VirtualServiceSpec().
Host(serviceHost).
AddHost(serviceHost).
Gateway(testGatewayURL).
HTTP(builders.HTTPRoute().
Match(builders.MatchRequest().Uri().Regex("/img")).
Expand Down Expand Up @@ -1174,7 +1174,7 @@ var _ = Describe("APIRule Controller", Serial, func() {
vs := vsList.Items[0]

expectedSpec := builders.VirtualServiceSpec().
Host(serviceHost).
AddHost(serviceHost).
Gateway(testGatewayURL).
HTTP(builders.HTTPRoute().
Match(builders.MatchRequest().Uri().Regex("/favicon")).
Expand Down

Large diffs are not rendered by default.

48 changes: 47 additions & 1 deletion internal/builders/virtual_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package builders
import (
"fmt"
apirulev1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1"
apirulev2alpha1 "github.com/kyma-project/api-gateway/apis/gateway/v2alpha1"
"google.golang.org/protobuf/types/known/durationpb"
"google.golang.org/protobuf/types/known/wrapperspb"
"istio.io/api/networking/v1beta1"
Expand Down Expand Up @@ -80,7 +81,7 @@ func (vss *virtualServiceSpec) From(val *v1beta1.VirtualService) *virtualService
return vss
}

func (vss *virtualServiceSpec) Host(val string) *virtualServiceSpec {
func (vss *virtualServiceSpec) AddHost(val string) *virtualServiceSpec {
vss.value.Hosts = append(vss.value.Hosts, val)
return vss
}
Expand Down Expand Up @@ -155,6 +156,20 @@ func (mr *matchRequest) Uri() *stringMatch {
return &stringMatch{mr.value.Uri, func() *matchRequest { return mr }}
}

// MethodRegExV2Alpha1 sets the HTTP method regex in the HTTPMatchRequest for the given HTTP methods in the format "^(PUT|POST|GET)$".
func (mr *matchRequest) MethodRegExV2Alpha1(httpMethods ...apirulev2alpha1.HttpMethod) *matchRequest {
methodStrings := apirulev2alpha1.ConvertHttpMethodsToStrings(httpMethods)
methodsWithSeparator := strings.Join(methodStrings, "|")

mr.value.Method = &v1beta1.StringMatch{
MatchType: &v1beta1.StringMatch_Regex{
Regex: fmt.Sprintf("^(%s)$", methodsWithSeparator),
},
}

return mr
}

// MethodRegEx sets the HTTP method regex in the HTTPMatchRequest for the given HTTP methods in the format "^(PUT|POST|GET)$".
func (mr *matchRequest) MethodRegEx(httpMethods ...apirulev1beta1.HttpMethod) *matchRequest {

Expand Down Expand Up @@ -257,6 +272,37 @@ func (cp *corsPolicy) AllowOrigins(val ...*v1beta1.StringMatch) *corsPolicy {
return cp
}

func (cp *corsPolicy) FromV2Alpha1ApiRuleCorsPolicy(corsPolicy apirulev2alpha1.CorsPolicy) *corsPolicy {
if len(corsPolicy.AllowOrigins) == 0 {
cp.value.AllowOrigins = nil
} else {
matchers := corsPolicy.AllowOrigins.ToIstioStringMatchArray()
cp.value.AllowOrigins = append(cp.value.AllowOrigins, matchers...)
}

if len(corsPolicy.AllowHeaders) > 0 {
cp.value.AllowHeaders = corsPolicy.AllowHeaders
}

if len(corsPolicy.AllowMethods) > 0 {
cp.value.AllowMethods = corsPolicy.AllowMethods
}

if len(corsPolicy.ExposeHeaders) > 0 {
cp.value.ExposeHeaders = corsPolicy.ExposeHeaders
}

if corsPolicy.AllowCredentials != nil {
cp.value.AllowCredentials = &wrapperspb.BoolValue{Value: *corsPolicy.AllowCredentials}
}

if corsPolicy.MaxAge != nil {
cp.value.MaxAge = durationpb.New(time.Duration(*corsPolicy.MaxAge) * time.Second)
}

return cp
}

func (cp *corsPolicy) FromApiRuleCorsPolicy(corsPolicy apirulev1beta1.CorsPolicy) *corsPolicy {
if len(corsPolicy.AllowOrigins) == 0 {
cp.value.AllowOrigins = nil
Expand Down
8 changes: 4 additions & 4 deletions internal/builders/virtual_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var _ = Describe("Builder for", func() {
vs := VirtualService().From(&initialVs).GenerateName(name).Namespace(namespace).
Spec(
VirtualServiceSpec().
Host(host).
AddHost(host).
Gateway(gateway)).
Get()
Expect(vs.Name).To(BeEmpty())
Expand All @@ -60,8 +60,8 @@ var _ = Describe("Builder for", func() {
timeout := time.Second * 60

result := VirtualServiceSpec().
Host(host).
Host(host2).
AddHost(host).
AddHost(host2).
Gateway(gateway).
Gateway(gateway2).
HTTP(HTTPRoute().
Expand Down Expand Up @@ -132,7 +132,7 @@ var _ = Describe("Builder for", func() {
}

result := VirtualServiceSpec().
Host(host).
AddHost(host).
Gateway(gateway).
HTTP(HTTPRoute().
CorsPolicy(CorsPolicy().FromApiRuleCorsPolicy(corsPolicy)).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (r virtualServiceCreator) Create(api *gatewayv1beta1.APIRule) (*networkingv
virtualServiceNamePrefix := fmt.Sprintf("%s-", api.ObjectMeta.Name)

vsSpecBuilder := builders.VirtualServiceSpec()
vsSpecBuilder.Host(default_domain.GetHostWithDomain(*api.Spec.Host, r.defaultDomainName))
vsSpecBuilder.AddHost(default_domain.GetHostWithDomain(*api.Spec.Host, r.defaultDomainName))
vsSpecBuilder.Gateway(*api.Spec.Gateway)
filteredRules := processing.FilterDuplicatePaths(api.Spec.Rules)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (r virtualServiceCreator) Create(api *gatewayv1beta1.APIRule) (*networkingv
virtualServiceNamePrefix := fmt.Sprintf("%s-", api.ObjectMeta.Name)

vsSpecBuilder := builders.VirtualServiceSpec()
vsSpecBuilder.Host(default_domain.GetHostWithDomain(*api.Spec.Host, r.defaultDomainName))
vsSpecBuilder.AddHost(default_domain.GetHostWithDomain(*api.Spec.Host, r.defaultDomainName))
vsSpecBuilder.Gateway(*api.Spec.Gateway)
filteredRules := processing.FilterDuplicatePaths(api.Spec.Rules)

Expand Down
77 changes: 77 additions & 0 deletions internal/processing/processors/v2alpha1/cors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package v2alpha1_test

import (
gatewayv2alpha1 "github.com/kyma-project/api-gateway/apis/gateway/v2alpha1"
"github.com/kyma-project/api-gateway/internal/builders"
processors "github.com/kyma-project/api-gateway/internal/processing/processors/v2alpha1"
istioapiv1beta1 "istio.io/api/networking/v1beta1"
networkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"

. "github.com/kyma-project/api-gateway/internal/processing/processing_test"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("CORS", func() {
var client client.Client
var processor processors.VirtualServiceProcessor
BeforeEach(func() {
client = GetFakeClient()
})

DescribeTable("CORS",
func(apiRule *gatewayv2alpha1.APIRule, verifiers []verifier, expectedActions ...string) {
processor = processors.NewVirtualServiceProcessor(GetTestConfig(), apiRule)
checkVirtualServices(client, processor, verifiers, expectedActions...)
},

Entry("should set default empty values in VirtualService CORSPolicy when no CORS configuration is set in APIRule",
newAPIRuleBuilderWithDummyDataWithNoAuthRule().Build(),
[]verifier{
func(vs *networkingv1beta1.VirtualService) {
Expect(vs.Spec.Http[0].CorsPolicy).To(BeNil())

Expect(vs.Spec.Http[0].Headers.Response.Remove).To(ConsistOf([]string{
builders.ExposeHeadersName,
builders.MaxAgeName,
builders.AllowHeadersName,
builders.AllowCredentialsName,
builders.AllowMethodsName,
builders.AllowOriginName,
}))
},
}, "create"),

Entry("should apply all CORSPolicy headers correctly",
newAPIRuleBuilderWithDummyDataWithNoAuthRule().WithCORSPolicy(
newCorsPolicyBuilder().
WithAllowOrigins([]map[string]string{{"exact": "example.com"}}).
WithAllowMethods([]string{"GET", "POST"}).
WithAllowHeaders([]string{"header1", "header2"}).
WithExposeHeaders([]string{"header3", "header4"}).
WithAllowCredentials(true).
WithMaxAge(600).
Build()).
Build(),
[]verifier{func(vs *networkingv1beta1.VirtualService) {
Expect(vs.Spec.Http[0].CorsPolicy).NotTo(BeNil())
Expect(vs.Spec.Http[0].CorsPolicy.AllowOrigins).To(HaveLen(1))
Expect(vs.Spec.Http[0].CorsPolicy.AllowOrigins[0]).To(Equal(&istioapiv1beta1.StringMatch{MatchType: &istioapiv1beta1.StringMatch_Exact{Exact: "example.com"}}))
Expect(vs.Spec.Http[0].CorsPolicy.AllowMethods).To(ConsistOf("GET", "POST"))
Expect(vs.Spec.Http[0].CorsPolicy.AllowHeaders).To(ConsistOf("header1", "header2"))
Expect(vs.Spec.Http[0].CorsPolicy.ExposeHeaders).To(ConsistOf("header3", "header4"))
Expect(vs.Spec.Http[0].CorsPolicy.AllowCredentials.GetValue()).To(BeTrue())
Expect(vs.Spec.Http[0].CorsPolicy.MaxAge.Seconds).To(Equal(int64(600)))

Expect(vs.Spec.Http[0].Headers.Response.Remove).To(ConsistOf([]string{
builders.ExposeHeadersName,
builders.MaxAgeName,
builders.AllowHeadersName,
builders.AllowCredentialsName,
builders.AllowMethodsName,
builders.AllowOriginName,
}))
}}, "create"),
)
})
43 changes: 43 additions & 0 deletions internal/processing/processors/v2alpha1/hosts_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package v2alpha1_test

import (
gatewayv2alpha1 "github.com/kyma-project/api-gateway/apis/gateway/v2alpha1"
processors "github.com/kyma-project/api-gateway/internal/processing/processors/v2alpha1"
networkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"

. "github.com/kyma-project/api-gateway/internal/processing/processing_test"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("Hosts", func() {
var client client.Client
var processor processors.VirtualServiceProcessor
BeforeEach(func() {
client = GetFakeClient()
})

DescribeTable("Hosts",
func(apiRule *gatewayv2alpha1.APIRule, verifiers []verifier, expectedActions ...string) {
processor = processors.NewVirtualServiceProcessor(GetTestConfig(), apiRule)
checkVirtualServices(client, processor, verifiers, expectedActions...)
},

Entry("should set the host correctly",
newAPIRuleBuilder().WithGateway("example/example").WithHost("example.com").Build(),
[]verifier{
func(vs *networkingv1beta1.VirtualService) {
Expect(vs.Spec.Hosts).To(ConsistOf("example.com"))
},
}, "create"),

Entry("should set multiple hosts correctly",
newAPIRuleBuilder().WithGateway("example/example").WithHosts("example.com", "goat.com").Build(),
[]verifier{
func(vs *networkingv1beta1.VirtualService) {
Expect(vs.Spec.Hosts).To(ConsistOf("example.com", "goat.com"))
},
}, "create"),
)
})
51 changes: 51 additions & 0 deletions internal/processing/processors/v2alpha1/http_matching_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package v2alpha1_test

import (
gatewayv2alpha1 "github.com/kyma-project/api-gateway/apis/gateway/v2alpha1"
processors "github.com/kyma-project/api-gateway/internal/processing/processors/v2alpha1"
networkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1"
"net/http"
"sigs.k8s.io/controller-runtime/pkg/client"

. "github.com/kyma-project/api-gateway/internal/processing/processing_test"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("HTTP matching", func() {
var client client.Client
var processor processors.VirtualServiceProcessor
BeforeEach(func() {
client = GetFakeClient()
})
var _ = DescribeTable("Different methods on same path",
func(apiRule *gatewayv2alpha1.APIRule, verifiers []verifier, expectedActions ...string) {
processor = processors.NewVirtualServiceProcessor(GetTestConfig(), apiRule)
checkVirtualServices(client, processor, verifiers, expectedActions...)
},
Entry("from two rules with different methods on the same path should create two HTTP routes with different methods",
newAPIRuleBuilderWithDummyData().
WithRules(newRuleBuilder().WithMethods(http.MethodGet).WithPath("/").NoAuth().Build(),
newRuleBuilder().WithMethods(http.MethodPut).WithPath("/").WithJWTAuthn("example.com", "https://jwks.example.com", nil, nil).Build()).Build(),
[]verifier{
func(vs *networkingv1beta1.VirtualService) {
Expect(vs.Spec.Http).To(HaveLen(2))

Expect(vs.Spec.Http[0].Match[0].Method.GetRegex()).To(Equal("^(GET)$"))
Expect(vs.Spec.Http[1].Match[0].Method.GetRegex()).To(Equal("^(PUT)$"))
},
}, "create"),

Entry("from one rule with two methods on the same path should create one HTTP route with regex matching both methods",
newAPIRuleBuilderWithDummyData().
WithRules(newRuleBuilder().WithMethods(http.MethodGet, http.MethodPut).WithPath("/").NoAuth().Build()).
Build(),
[]verifier{
func(vs *networkingv1beta1.VirtualService) {
Expect(vs.Spec.Http).To(HaveLen(1))

Expect(vs.Spec.Http[0].Match[0].Method.GetRegex()).To(Equal("^(GET|PUT)$"))
},
}, "create"),
)
})
16 changes: 16 additions & 0 deletions internal/processing/processors/v2alpha1/reconciliation.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,19 @@ func NewReconciliation(apiRuleV2alpha1 *gatewayv2alpha1.APIRule, apiRuleV1beta1
config: config,
}
}

func findServiceNamespace(api *gatewayv2alpha1.APIRule, rule *gatewayv2alpha1.Rule) string {
// Fallback direction for the upstream service namespace: Rule.Service > Spec.Service > APIRule
if rule != nil && rule.Service != nil && rule.Service.Namespace != nil {
return *rule.Service.Namespace
}
if api != nil && api.Spec.Service != nil && api.Spec.Service.Namespace != nil {
return *api.Spec.Service.Namespace
}

if api != nil {
return api.Namespace
} else {
return ""
}
}
Loading

0 comments on commit 34e5a44

Please sign in to comment.