From 896d8f5ec563a73701536b29860142bdccbabb35 Mon Sep 17 00:00:00 2001 From: skpratt Date: Thu, 2 Nov 2023 13:16:08 -0500 Subject: [PATCH] temporarily disallow L7 traffic permissions (#19322) --- internal/auth/internal/types/errors.go | 1 + .../internal/types/traffic_permissions.go | 14 ++++ .../types/traffic_permissions_test.go | 77 +++++++++++++++++-- 3 files changed, 84 insertions(+), 8 deletions(-) diff --git a/internal/auth/internal/types/errors.go b/internal/auth/internal/types/errors.go index b79d7a3b98..b11fac6304 100644 --- a/internal/auth/internal/types/errors.go +++ b/internal/auth/internal/types/errors.go @@ -12,4 +12,5 @@ var ( errSourceExcludes = errors.New("must be defined on wildcard sources") errInvalidPrefixValues = errors.New("prefix values, regex values, and explicit names must not combined") ErrWildcardNotSupported = errors.New("traffic permissions without explicit destinations are not yet supported") + ErrL7NotSupported = errors.New("traffic permissions with L7 rules are not yet supported") ) diff --git a/internal/auth/internal/types/traffic_permissions.go b/internal/auth/internal/types/traffic_permissions.go index bf22fdb0b5..acf2655371 100644 --- a/internal/auth/internal/types/traffic_permissions.go +++ b/internal/auth/internal/types/traffic_permissions.go @@ -217,6 +217,13 @@ func validatePermission(p *pbauth.Permission, wrapErr func(error) error) error { Wrapped: err, }) } + // TODO: remove this when L7 traffic permissions are implemented + if len(dest.PathExact) > 0 || len(dest.PathPrefix) > 0 || len(dest.PathRegex) > 0 || len(dest.Methods) > 0 || dest.Header != nil { + merr = multierror.Append(merr, wrapDestRuleErr(resource.ErrInvalidListElement{ + Name: "destination_rule", + Wrapped: ErrL7NotSupported, + })) + } if (len(dest.PathExact) > 0 && len(dest.PathPrefix) > 0) || (len(dest.PathRegex) > 0 && len(dest.PathExact) > 0) || (len(dest.PathRegex) > 0 && len(dest.PathPrefix) > 0) { @@ -234,6 +241,13 @@ func validatePermission(p *pbauth.Permission, wrapErr func(error) error) error { Wrapped: err, }) } + // TODO: remove this when L7 traffic permissions are implemented + if len(excl.PathExact) > 0 || len(excl.PathPrefix) > 0 || len(excl.PathRegex) > 0 || len(excl.Methods) > 0 || excl.Header != nil { + merr = multierror.Append(merr, wrapDestRuleErr(resource.ErrInvalidListElement{ + Name: "exclude_permission_rules", + Wrapped: ErrL7NotSupported, + })) + } if (len(excl.PathExact) > 0 && len(excl.PathPrefix) > 0) || (len(excl.PathRegex) > 0 && len(excl.PathExact) > 0) || (len(excl.PathRegex) > 0 && len(excl.PathPrefix) > 0) { diff --git a/internal/auth/internal/types/traffic_permissions_test.go b/internal/auth/internal/types/traffic_permissions_test.go index 0948d240b6..94fb165c3b 100644 --- a/internal/auth/internal/types/traffic_permissions_test.go +++ b/internal/auth/internal/types/traffic_permissions_test.go @@ -65,10 +65,46 @@ func TestValidateTrafficPermissions(t *testing.T) { }, "no-destination": { tp: &pbauth.TrafficPermissions{ + Action: pbauth.Action_ACTION_ALLOW, + Permissions: nil, + }, + expectErr: `invalid "data.destination" field: cannot be empty`, + }, + "source-tenancy": { + tp: &pbauth.TrafficPermissions{ + Destination: &pbauth.Destination{ + IdentityName: "w1", + }, + Action: pbauth.Action_ACTION_ALLOW, + Permissions: []*pbauth.Permission{ + { + Sources: []*pbauth.Source{ + { + Partition: "ap1", + Peer: "cl1", + SamenessGroup: "sg1", + }, + }, + DestinationRules: nil, + }, + }, + }, + expectErr: `invalid element at index 0 of list "permissions": invalid element at index 0 of list "sources": invalid element at index 0 of list "source": permissions sources may not specify partitions, peers, and sameness_groups together`, + }, + // TODO: remove when L7 traffic permissions are implemented + "l7-fields-path": { + tp: &pbauth.TrafficPermissions{ + Destination: &pbauth.Destination{ + IdentityName: "w1", + }, Action: pbauth.Action_ACTION_ALLOW, Permissions: []*pbauth.Permission{ { - Sources: nil, + Sources: []*pbauth.Source{ + { + Partition: "ap1", + }, + }, DestinationRules: []*pbauth.DestinationRule{ { PathExact: "wi2", @@ -77,9 +113,9 @@ func TestValidateTrafficPermissions(t *testing.T) { }, }, }, - expectErr: `invalid "data.destination" field: cannot be empty`, + expectErr: `invalid element at index 0 of list "permissions": invalid element at index 0 of list "destination_rules": invalid element at index 0 of list "destination_rule": traffic permissions with L7 rules are not yet supported`, }, - "source-tenancy": { + "l7-fields-methods": { tp: &pbauth.TrafficPermissions{ Destination: &pbauth.Destination{ IdentityName: "w1", @@ -89,16 +125,41 @@ func TestValidateTrafficPermissions(t *testing.T) { { Sources: []*pbauth.Source{ { - Partition: "ap1", - Peer: "cl1", - SamenessGroup: "sg1", + Partition: "ap1", + }, + }, + DestinationRules: []*pbauth.DestinationRule{ + { + Methods: []string{"PUT"}, }, }, - DestinationRules: nil, }, }, }, - expectErr: `invalid element at index 0 of list "permissions": invalid element at index 0 of list "sources": invalid element at index 0 of list "source": permissions sources may not specify partitions, peers, and sameness_groups together`, + expectErr: `invalid element at index 0 of list "permissions": invalid element at index 0 of list "destination_rules": invalid element at index 0 of list "destination_rule": traffic permissions with L7 rules are not yet supported`, + }, + "l7-fields-header": { + tp: &pbauth.TrafficPermissions{ + Destination: &pbauth.Destination{ + IdentityName: "w1", + }, + Action: pbauth.Action_ACTION_ALLOW, + Permissions: []*pbauth.Permission{ + { + Sources: []*pbauth.Source{ + { + Partition: "ap1", + }, + }, + DestinationRules: []*pbauth.DestinationRule{ + { + Header: &pbauth.DestinationRuleHeader{Name: "foo"}, + }, + }, + }, + }, + }, + expectErr: `invalid element at index 0 of list "permissions": invalid element at index 0 of list "destination_rules": invalid element at index 0 of list "destination_rule": traffic permissions with L7 rules are not yet supported`, }, }