Enable internal traffic for workloads exposed by v2alpha1 APIRule (#1666

) * Enable internal traffic for workloads exposed by v2alpha1 APIRule * Remove with labels * Add RN * Update desired_test.go * Revert changes of get service selector * Switch logic to allow instead of disallow * Update docs/release-notes/2.12.0.md Co-authored-by: Natalia Sitko <[email protected]> * Pass down podSelector from top to bottom --------- Co-authored-by: Natalia Sitko <[email protected]>
kyma-project · Feb 7, 2025 · a97fae9 · a97fae9
1 parent d02aaa2
commit a97fae9
Show file tree

Hide file tree

Showing 11 changed files with 225 additions and 55 deletions.
diff --git a/docs/release-notes/2.12.0.md b/docs/release-notes/2.12.0.md
@@ -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.
diff --git a/internal/builders/istio.go b/internal/builders/istio.go
@@ -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

diff --git a/internal/processing/processors/v2alpha1/authorizationpolicy/audience_test.go b/internal/processing/processors/v2alpha1/authorizationpolicy/audience_test.go
@@ -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)
@@ -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)

diff --git a/internal/processing/processors/v2alpha1/authorizationpolicy/creator.go b/internal/processing/processors/v2alpha1/authorizationpolicy/creator.go
@@ -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)
 		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)
@@ -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
@@ -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
 }
 

diff --git a/internal/processing/processors/v2alpha1/authorizationpolicy/ext_auth_test.go b/internal/processing/processors/v2alpha1/authorizationpolicy/ext_auth_test.go
@@ -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)
@@ -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)

diff --git a/internal/processing/processors/v2alpha1/authorizationpolicy/internal_access_test.go b/internal/processing/processors/v2alpha1/authorizationpolicy/internal_access_test.go
@@ -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"))
+			}
+		}
+	})
+})
diff --git a/internal/processing/processors/v2alpha1/authorizationpolicy/jwt_rules_test.go b/internal/processing/processors/v2alpha1/authorizationpolicy/jwt_rules_test.go
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)

diff --git a/internal/processing/processors/v2alpha1/authorizationpolicy/no_auth_rules_test.go b/internal/processing/processors/v2alpha1/authorizationpolicy/no_auth_rules_test.go
@@ -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)

diff --git a/internal/processing/processors/v2alpha1/authorizationpolicy/processor.go b/internal/processing/processors/v2alpha1/authorizationpolicy/processor.go
@@ -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,
 	}
 }
 
@@ -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,
 	}
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.