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

Enable internal traffic for workloads exposed by v2alpha1 APIRule #1666

Merged
merged 8 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/release-notes/2.12.0.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## Bugfixes

We've fixed the [issue](https://github.com/kyma-project/api-gateway/issues/1632) where a workload exposed via a `v2alpha1` APIRule was not accessible from within the cluster.
6 changes: 6 additions & 0 deletions internal/builders/istio.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ func (rf *FromBuilder) WithForcedJWTAuthorizationV2alpha1(authentications []*gat
return rf
}

func (rf *FromBuilder) ExcludingIngressGatewaySource() *FromBuilder {
source := v1beta1.Source{NotPrincipals: []string{istioIngressGatewayPrincipal}}
rf.value.Source = &source
return rf
}

func (rf *FromBuilder) WithIngressGatewaySource() *FromBuilder {
source := v1beta1.Source{Principals: []string{istioIngressGatewayPrincipal}}
rf.value.Source = &source
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var _ = Describe("JwtAuthorization Policy Processor", func() {

svc := newServiceBuilderWithDummyData().build()
client := getFakeClient(svc)
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
result, err := processor.EvaluateReconciliation(context.Background(), client)
Expand Down Expand Up @@ -54,7 +54,7 @@ var _ = Describe("JwtAuthorization Policy Processor", func() {
build()
svc := newServiceBuilderWithDummyData().build()
client := getFakeClient(svc)
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
result, err := processor.EvaluateReconciliation(context.Background(), client)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,26 @@ type Creator interface {
type creator struct {
// Controls that requests to Ory Oathkeeper are also permitted when
// migrating from APIRule v1beta1 to v2alpha1.
oryPassthrough bool
oryPassthrough bool
allowInternalTraffic bool
}

// Create returns the AuthorizationPolicy using the configuration of the APIRule.
func (r creator) Create(ctx context.Context, client client.Client, apiRule *gatewayv2alpha1.APIRule) (hashbasedstate.Desired, error) {
state := hashbasedstate.NewDesired()
selectorAllowed := make(map[gatewayv2alpha1.PodSelector]bool)
for _, rule := range apiRule.Spec.Rules {
aps, err := r.generateAuthorizationPolicies(ctx, client, apiRule, rule)
selector, err := gatewayv2alpha1.GetSelectorFromService(ctx, client, apiRule, rule)
mluk-sap marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return state, err
}
var aps *securityv1beta1.AuthorizationPolicyList
_, selectorAlreadyAllowed := selectorAllowed[selector]
aps, err = r.generateAuthorizationPolicies(ctx, client, selector, apiRule, rule, !selectorAlreadyAllowed && r.allowInternalTraffic)
if err != nil {
return state, err
}
selectorAllowed[selector] = true

for _, ap := range aps.Items {
h := hashbasedstate.NewAuthorizationPolicy(ap)
Expand All @@ -53,7 +62,28 @@ func (r creator) Create(ctx context.Context, client client.Client, apiRule *gate
return state, nil
}

func (r creator) generateAuthorizationPolicies(ctx context.Context, client client.Client, api *gatewayv2alpha1.APIRule, rule gatewayv2alpha1.Rule) (*securityv1beta1.AuthorizationPolicyList, error) {
func (r creator) generateAllowForInternalTraffic(podSelector gatewayv2alpha1.PodSelector, api *gatewayv2alpha1.APIRule, rule gatewayv2alpha1.Rule) (*securityv1beta1.AuthorizationPolicy, error) {
apBuilder, err := baseAuthorizationPolicyBuilder(api, rule)
if err != nil {
return nil, fmt.Errorf("error creating base AuthorizationPolicy builder: %w", err)
}

apBuilder.WithSpec(
builders.NewAuthorizationPolicySpecBuilder().
WithSelector(podSelector.Selector).
WithAction(v1beta1.AuthorizationPolicy_ALLOW).
WithRule(builders.NewRuleBuilder().
WithFrom(
builders.NewFromBuilder().
ExcludingIngressGatewaySource().
Get()).
Get()).
Get())

return apBuilder.Get(), nil
}

func (r creator) generateAuthorizationPolicies(ctx context.Context, client client.Client, podSelector gatewayv2alpha1.PodSelector, api *gatewayv2alpha1.APIRule, rule gatewayv2alpha1.Rule, allowInternalTraffic bool) (*securityv1beta1.AuthorizationPolicyList, error) {
authorizationPolicyList := securityv1beta1.AuthorizationPolicyList{}

var jwtAuthorizations []*gatewayv2alpha1.JwtAuthorization
Expand Down Expand Up @@ -103,6 +133,21 @@ func (r creator) generateAuthorizationPolicies(ctx context.Context, client clien
}
}

if allowInternalTraffic {
internalTrafficAp, err := r.generateAllowForInternalTraffic(podSelector, api, rule)
if err != nil {
return &authorizationPolicyList, err
}

if internalTrafficAp != nil {
err = hashbasedstate.AddLabelsToAuthorizationPolicy(internalTrafficAp, baseHashIndex+1+len(jwtAuthorizations))
if err != nil {
return &authorizationPolicyList, err
}
authorizationPolicyList.Items = append(authorizationPolicyList.Items, internalTrafficAp)
}
}

return &authorizationPolicyList, nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var _ = Describe("Processing ExtAuth rules", func() {

svc := newServiceBuilderWithDummyData().build()
client := getFakeClient(svc)
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
results, err := processor.EvaluateReconciliation(context.Background(), client)
Expand Down Expand Up @@ -84,7 +84,7 @@ var _ = Describe("Processing ExtAuth rules", func() {
build()
svc := newServiceBuilderWithDummyData().build()
client := getFakeClient(svc)
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
results, err := processor.EvaluateReconciliation(context.Background(), client)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package authorizationpolicy_test

import (
"context"
"github.com/kyma-project/api-gateway/internal/processing/processors/v2alpha1/authorizationpolicy"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
securityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
)

var _ = Describe("Internal access from cluster APs", func() {
It("should create an AP for internal access, one if only one service is used", func() {
// given
ruleJwt := newNoAuthRuleBuilderWithDummyData().
withPath("/abc").
build()

noAuthRule := newNoAuthRuleBuilderWithDummyData().
withPath("/def").
build()

apiRule := newAPIRuleBuilderWithDummyData().
withRules(ruleJwt, noAuthRule).
build()
svc := newServiceBuilderWithDummyData().build()
client := getFakeClient(svc)
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)

// when
result, err := processor.EvaluateReconciliation(context.Background(), client)

Expect(err).To(BeNil())
Expect(result).To(HaveLen(3))

goodAps := 0
for i := 0; i < 3; i++ {
ap := result[i].Obj.(*securityv1beta1.AuthorizationPolicy)
if len(ap.Spec.Rules[0].To) == 0 {
Expect(len(ap.Spec.Rules[0].From)).To(Equal(1))
Expect(ap.Spec.Rules[0].From[0].Source.NotPrincipals).To(ConsistOf("cluster.local/ns/istio-system/sa/istio-ingressgateway-service-account"))
goodAps++
} else if len(ap.Spec.Rules[0].To) == 1 {
if ap.Spec.Rules[0].To[0].Operation.Paths[0] == "/abc" {
goodAps++
} else if ap.Spec.Rules[0].To[0].Operation.Paths[0] == "/def" {
goodAps++
}
}
}
Expect(goodAps).To(Equal(3))
})

It("should create an AP for internal access, two if two services are used", func() {
// given
ruleJwt := newNoAuthRuleBuilderWithDummyData().
withPath("/abc").
build()

noAuthRule := newNoAuthRuleBuilderWithDummyData().
withPath("/def").
withServiceName("different-service").
build()

apiRule := newAPIRuleBuilderWithDummyData().
withRules(ruleJwt, noAuthRule).
build()
svc := newServiceBuilderWithDummyData().build()
differentSvc := newServiceBuilderWithDummyData().withName("different-service").addSelector("a", "b").build()
client := getFakeClient(svc, differentSvc)
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)

// when
result, err := processor.EvaluateReconciliation(context.Background(), client)

Expect(err).To(BeNil())
Expect(result).To(HaveLen(4))

for _, apResult := range result {
ap := apResult.Obj.(*securityv1beta1.AuthorizationPolicy)

if len(ap.Spec.Rules[0].To) > 0 {
Expect(len(ap.Spec.Rules[0].To[0].Operation.Paths)).To(Equal(1))
if ap.Spec.Rules[0].To[0].Operation.Paths[0] == "/abc" {
Expect(ap.Spec.Rules[0].To[0].Operation.Paths).To(ContainElement("/abc"))
} else if ap.Spec.Rules[0].To[0].Operation.Paths[0] == "/def" {
Expect(ap.Spec.Rules[0].To[0].Operation.Paths).To(ContainElement("/def"))
}
} else {
Expect(len(ap.Spec.Rules[0].From)).To(Equal(1))
Expect(ap.Spec.Rules[0].From[0].Source.NotPrincipals).To(ConsistOf("cluster.local/ns/istio-system/sa/istio-ingressgateway-service-account"))
}
}
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var _ = Describe("Processing JWT rules", func() {
svc := newServiceBuilderWithDummyData().build()

client := getFakeClient(svc)
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
result, err := processor.EvaluateReconciliation(context.Background(), client)
Expand Down Expand Up @@ -106,7 +106,7 @@ var _ = Describe("Processing JWT rules", func() {
apiRule := newAPIRuleBuilderWithDummyData().withRules(jwtRule).build()
svc := newServiceBuilderWithDummyData().build()
client := getFakeClient(svc)
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
result, err := processor.EvaluateReconciliation(context.Background(), client)
Expand Down Expand Up @@ -175,7 +175,7 @@ var _ = Describe("Processing JWT rules", func() {
apiRule := newAPIRuleBuilderWithDummyData().withRules(ruleJwt).build()
svc := newServiceBuilderWithDummyData().build()
client := getFakeClient(svc)
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
result, err := processor.EvaluateReconciliation(context.Background(), client)
Expand Down Expand Up @@ -256,7 +256,7 @@ var _ = Describe("Processing JWT rules", func() {
withRules(rules...).
build()

processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
result, err := processor.EvaluateReconciliation(context.Background(), ctrlClient)
Expand Down Expand Up @@ -313,7 +313,7 @@ var _ = Describe("Processing JWT rules", func() {
apiRule := newAPIRuleBuilderWithDummyData().
withServiceName(serviceName).
withRules(jwtRule).build()
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
result, err := processor.EvaluateReconciliation(context.Background(), ctrlClient)
Expand Down Expand Up @@ -369,7 +369,7 @@ var _ = Describe("Processing JWT rules", func() {
build()

apiRule := newAPIRuleBuilderWithDummyData().withRules(jwtRule).build()
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
result, err := processor.EvaluateReconciliation(context.Background(), ctrlClient)
Expand Down Expand Up @@ -424,7 +424,7 @@ var _ = Describe("Processing JWT rules", func() {
build()

apiRule := newAPIRuleBuilderWithDummyData().withRules(jwtRule).build()
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
result, err := processor.EvaluateReconciliation(context.Background(), ctrlClient)
Expand Down Expand Up @@ -491,7 +491,7 @@ var _ = Describe("Processing JWT rules", func() {
build()

apiRule := newAPIRuleBuilderWithDummyData().withRules(jwtRule).build()
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
result, err := processor.EvaluateReconciliation(context.Background(), ctrlClient)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ var _ = Describe("Processing NoAuth rules", func() {
build()
svc := newServiceBuilderWithDummyData().build()
client := getFakeClient(svc)
processor := authorizationpolicy.NewProcessor(&testLogger, apiRule)
processor := authorizationpolicy.NewProcessorWithoutInternalTraffic(&testLogger, apiRule)

// when
results, err := processor.EvaluateReconciliation(context.Background(), client)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,22 @@ import (
func NewProcessor(log *logr.Logger, rule *gatewayv2alpha1.APIRule) Processor {
return Processor{
apiRule: rule,
creator: creator{},
Log: log,
creator: creator{
allowInternalTraffic: true,
},
Log: log,
}
}

// NewProcessorWithoutInternalTraffic returns a Processor with the desired state handling for AuthorizationPolicy.
// This processor will not create AuthorizationPolicy for internal traffic.
func NewProcessorWithoutInternalTraffic(log *logr.Logger, rule *gatewayv2alpha1.APIRule) Processor {
return Processor{
apiRule: rule,
creator: creator{
allowInternalTraffic: false,
},
Log: log,
}
}

Expand All @@ -25,7 +39,8 @@ func NewMigrationProcessor(log *logr.Logger, rule *gatewayv2alpha1.APIRule, oryP
return Processor{
apiRule: rule,
creator: creator{
oryPassthrough: oryPassthrough,
oryPassthrough: oryPassthrough,
allowInternalTraffic: true,
},
Log: log,
}
Expand Down
Loading