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.

Sign up for GitHub

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

Enable internal traffic for workloads exposed by v2alpha1 APIRule #1666

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
24af56c
Enable internal traffic for workloads exposed by v2alpha1 APIRule
barchw Feb 4, 2025
7a9c4bc
Remove with labels
barchw Feb 4, 2025
e4e5560
Add RN
barchw Feb 4, 2025
4a10ef8
Update desired_test.go
barchw Feb 6, 2025
44baff7
Revert changes of get service selector
barchw Feb 7, 2025
882f894
Switch logic to allow instead of disallow
barchw Feb 7, 2025
0417ce0
Update docs/release-notes/2.12.0.md
barchw Feb 7, 2025
d6db66d
Pass down podSelector from top to bottom
barchw Feb 7, 2025
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
27 changes: 15 additions & 12 deletions apis/gateway/v2alpha1/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,28 +33,31 @@ type PodSelector struct {
Namespace string
}

func GetSelectorFromService(ctx context.Context, client client.Client, apiRule *APIRule, rule Rule) (PodSelector, error) {

var service *Service
func getServiceFromRule(apiRule APIRule, rule Rule) *Service {
if rule.Service != nil {
service = rule.Service
} else {
service = apiRule.Spec.Service
return rule.Service
}
return apiRule.Spec.Service
}

func GetSelectorForRule(ctx context.Context, client client.Client, apiRule *APIRule, rule Rule) (PodSelector, error) {
service := getServiceFromRule(*apiRule, rule)
serviceNamespace, err := FindServiceNamespace(apiRule, rule)
if err != nil {
return PodSelector{}, fmt.Errorf("finding service namespace: %w", err)
}
return GetSelectorFromService(ctx, client, service, serviceNamespace)
}

func GetSelectorFromService(ctx context.Context, client client.Client, service *Service, defaultNamespace string) (PodSelector, error) {
if service == nil || service.Name == nil {
return PodSelector{}, fmt.Errorf("service name is required but missing")
}
serviceNamespacedName := types.NamespacedName{Name: *service.Name}
if service.Namespace != nil {
serviceNamespacedName.Namespace = *service.Namespace
} else {
ns, err := FindServiceNamespace(apiRule, rule)
if err != nil {
return PodSelector{}, fmt.Errorf("finding service namespace: %w", err)
}

serviceNamespacedName.Namespace = ns
serviceNamespacedName.Namespace = defaultNamespace
}

if serviceNamespacedName.Namespace == "" {
Expand Down
22 changes: 11 additions & 11 deletions apis/gateway/v2alpha1/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,24 +63,24 @@ var _ = Describe("Service", func() {
),
)

Context("GetSelectorFromService", func() {
Context("GetSelectorForRule", func() {

It("should return error when service is not set", func() {
_, err := v2alpha1.GetSelectorFromService(context.Background(), createFakeClient(), &v2alpha1.APIRule{}, v2alpha1.Rule{})
_, err := v2alpha1.GetSelectorForRule(context.Background(), createFakeClient(), &v2alpha1.APIRule{}, v2alpha1.Rule{})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal("service name is required but missing"))
})

It("should return error when service name is not set", func() {
_, err := v2alpha1.GetSelectorFromService(context.Background(), createFakeClient(), &v2alpha1.APIRule{}, v2alpha1.Rule{
_, err := v2alpha1.GetSelectorForRule(context.Background(), createFakeClient(), &v2alpha1.APIRule{}, v2alpha1.Rule{
Service: &v2alpha1.Service{},
})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal("service name is required but missing"))
})

It("should return error when service is not found", func() {
_, err := v2alpha1.GetSelectorFromService(context.Background(), createFakeClient(), &v2alpha1.APIRule{}, v2alpha1.Rule{
_, err := v2alpha1.GetSelectorForRule(context.Background(), createFakeClient(), &v2alpha1.APIRule{}, v2alpha1.Rule{
Service: &v2alpha1.Service{
Name: ptr.To("test-service"),
},
Expand All @@ -96,7 +96,7 @@ var _ = Describe("Service", func() {
build()

fakeClient := createFakeClient(s)
selector, err := v2alpha1.GetSelectorFromService(context.Background(), fakeClient, &v2alpha1.APIRule{}, v2alpha1.Rule{
selector, err := v2alpha1.GetSelectorForRule(context.Background(), fakeClient, &v2alpha1.APIRule{}, v2alpha1.Rule{
Service: &v2alpha1.Service{
Name: ptr.To("test-service"),
Namespace: ptr.To("test-namespace"),
Expand All @@ -114,7 +114,7 @@ var _ = Describe("Service", func() {
build()

fakeClient := createFakeClient(s)
selector, err := v2alpha1.GetSelectorFromService(context.Background(), fakeClient, &v2alpha1.APIRule{}, v2alpha1.Rule{
selector, err := v2alpha1.GetSelectorForRule(context.Background(), fakeClient, &v2alpha1.APIRule{}, v2alpha1.Rule{
Service: &v2alpha1.Service{
Name: ptr.To("test-service"),
Namespace: ptr.To("test-namespace"),
Expand All @@ -133,7 +133,7 @@ var _ = Describe("Service", func() {
build()

fakeClient := createFakeClient(s)
selector, err := v2alpha1.GetSelectorFromService(context.Background(), fakeClient,
selector, err := v2alpha1.GetSelectorForRule(context.Background(), fakeClient,
&v2alpha1.APIRule{
ObjectMeta: v1.ObjectMeta{
Namespace: "apirule-namespace",
Expand Down Expand Up @@ -163,7 +163,7 @@ var _ = Describe("Service", func() {
build()

fakeClient := createFakeClient(s)
selector, err := v2alpha1.GetSelectorFromService(context.Background(), fakeClient,
selector, err := v2alpha1.GetSelectorForRule(context.Background(), fakeClient,
&v2alpha1.APIRule{
ObjectMeta: v1.ObjectMeta{
Namespace: "apirule-namespace",
Expand All @@ -188,7 +188,7 @@ var _ = Describe("Service", func() {
build()

fakeClient := createFakeClient(s)
selector, err := v2alpha1.GetSelectorFromService(context.Background(), fakeClient,
selector, err := v2alpha1.GetSelectorForRule(context.Background(), fakeClient,
&v2alpha1.APIRule{
ObjectMeta: v1.ObjectMeta{
Namespace: "apirule-namespace",
Expand All @@ -213,7 +213,7 @@ var _ = Describe("Service", func() {
build()

fakeClient := createFakeClient(s)
selector, err := v2alpha1.GetSelectorFromService(context.Background(), fakeClient,
selector, err := v2alpha1.GetSelectorForRule(context.Background(), fakeClient,
&v2alpha1.APIRule{
ObjectMeta: v1.ObjectMeta{
Namespace: "apirule-namespace",
Expand Down Expand Up @@ -242,7 +242,7 @@ var _ = Describe("Service", func() {
build()

fakeClient := createFakeClient(s)
selector, err := v2alpha1.GetSelectorFromService(context.Background(), fakeClient,
selector, err := v2alpha1.GetSelectorForRule(context.Background(), fakeClient,
&v2alpha1.APIRule{
ObjectMeta: v1.ObjectMeta{
Namespace: "apirule-namespace",
Expand Down
3 changes: 3 additions & 0 deletions docs/release-notes/2.12.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## Bugfixes

Fixed an [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.
barchw marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 6 additions & 0 deletions internal/builders/istio.go
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion internal/processing/hashbasedstate/desired_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ var _ = Describe("Desired state", func() {
ap := securityv1beta1.AuthorizationPolicy{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"gateway.kyma-project.io/hash": "56fggdf4",
"gateway.kyma-project.io/Hash": "56fggdf4",
},
},
}
Expand Down
4 changes: 2 additions & 2 deletions internal/processing/processors/v2alpha1/authorizationpolicy/audience_test.go
View file
Open in desktop
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
66 changes: 61 additions & 5 deletions internal/processing/processors/v2alpha1/authorizationpolicy/creator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,32 @@ 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
disallowInternalTraffic bool
mluk-sap marked this conversation as resolved.
Show resolved Hide resolved
}

// 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.GetSelectorForRule(ctx, client, apiRule, rule)
if err != nil {
return state, err
}
var aps *securityv1beta1.AuthorizationPolicyList
if _, ok := selectorAllowed[selector]; ok || r.disallowInternalTraffic {
aps, err = r.generateAuthorizationPolicies(ctx, client, apiRule, rule, false)
if err != nil {
return state, err
}
} else {
aps, err = r.generateAuthorizationPolicies(ctx, client, apiRule, rule, true)
if err != nil {
return state, err
}
selectorAllowed[selector] = true
}

for _, ap := range aps.Items {
h := hashbasedstate.NewAuthorizationPolicy(ap)
Expand All @@ -53,7 +68,33 @@ 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(ctx context.Context, k8sClient client.Client, api *gatewayv2alpha1.APIRule, rule gatewayv2alpha1.Rule) (*securityv1beta1.AuthorizationPolicy, error) {
podSelector, err := gatewayv2alpha1.GetSelectorForRule(ctx, k8sClient, api, rule)
if err != nil {
return nil, err
}

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, api *gatewayv2alpha1.APIRule, rule gatewayv2alpha1.Rule, allowInternalTraffic bool) (*securityv1beta1.AuthorizationPolicyList, error) {
authorizationPolicyList := securityv1beta1.AuthorizationPolicyList{}

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

if allowInternalTraffic {
internalTrafficAp, err := r.generateAllowForInternalTraffic(ctx, client, 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 Expand Up @@ -191,7 +247,7 @@ func (r creator) generateAuthorizationPolicy(ctx context.Context, client client.
}

func (r creator) generateExtAuthAuthorizationPolicySpec(ctx context.Context, client client.Client, api *gatewayv2alpha1.APIRule, rule gatewayv2alpha1.Rule, providerName string) (*v1beta1.AuthorizationPolicy, error) {
podSelector, err := gatewayv2alpha1.GetSelectorFromService(ctx, client, api, rule)
podSelector, err := gatewayv2alpha1.GetSelectorForRule(ctx, client, api, rule)
if err != nil {
return nil, err
}
Expand All @@ -205,7 +261,7 @@ func (r creator) generateExtAuthAuthorizationPolicySpec(ctx context.Context, cli
}

func (r creator) generateAuthorizationPolicySpec(ctx context.Context, client client.Client, api *gatewayv2alpha1.APIRule, rule gatewayv2alpha1.Rule, authorization *gatewayv2alpha1.JwtAuthorization) (*v1beta1.AuthorizationPolicy, error) {
podSelector, err := gatewayv2alpha1.GetSelectorFromService(ctx, client, api, rule)
podSelector, err := gatewayv2alpha1.GetSelectorForRule(ctx, client, api, rule)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions internal/processing/processors/v2alpha1/authorizationpolicy/ext_auth_test.go
View file
Open in desktop
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
Loading