Browse Source

review fixes

pull/20453/head
skpratt 10 months ago
parent
commit
96bf768213
  1. 8
      agent/xdsv2/rbac_resources.go
  2. 2
      internal/auth/internal/types/errors.go
  3. 15
      internal/auth/internal/types/traffic_permissions.go
  4. 31
      internal/auth/internal/types/traffic_permissions_test.go
  5. 2
      test-integ/catalogv2/traffic_permissions_test.go

8
agent/xdsv2/rbac_resources.go

@ -363,8 +363,8 @@ func translateRule(dr *pbproxystate.DestinationRule) *envoy_rbac_v3.Permission {
func permissionsFromDestinationRules(drs []*pbproxystate.DestinationRule) []*envoy_rbac_v3.Permission {
var perms []*envoy_rbac_v3.Permission
for _, dr := range drs {
var subPerms []*envoy_rbac_v3.Permission
for _, er := range dr.Exclude {
subPerms := make([]*envoy_rbac_v3.Permission, len(dr.Exclude))
for i, er := range dr.Exclude {
translated := translateRule(&pbproxystate.DestinationRule{
PathExact: er.PathExact,
PathPrefix: er.PathPrefix,
@ -372,9 +372,9 @@ func permissionsFromDestinationRules(drs []*pbproxystate.DestinationRule) []*env
Methods: er.Methods,
DestinationRuleHeader: er.Headers,
})
subPerms = append(subPerms, &envoy_rbac_v3.Permission{
subPerms[i] = &envoy_rbac_v3.Permission{
Rule: &envoy_rbac_v3.Permission_NotRule{NotRule: translated},
})
}
}
subPerms = append([]*envoy_rbac_v3.Permission{translateRule(dr)}, subPerms...)
perms = append(perms, combineAndPermissions(subPerms))

2
internal/auth/internal/types/errors.go

@ -7,7 +7,7 @@ import "errors"
var (
errSourcesTenancy = errors.New("permissions sources may not specify partitions, peers, and sameness_groups together")
errSourceWildcards = errors.New("permission sources may not have wildcard namespaces and explicit names.")
errSourceWildcards = errors.New("permission sources may not have wildcard namespaces and explicit names")
errSourceExcludes = errors.New("must be defined on wildcard sources")
errInvalidPrefixValues = errors.New("prefix values, regex values, and explicit names must not combined")
errInvalidRule = errors.New("rules must contain path, method, header, or port fields")

15
internal/auth/internal/types/traffic_permissions.go

@ -4,6 +4,8 @@
package types
import (
"golang.org/x/exp/slices"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/consul/acl"
@ -293,7 +295,7 @@ func validatePermission(p *pbauth.Permission, id *pbresource.ID, wrapErr func(er
}
}
for _, m := range excl.Methods {
if len(dest.Methods) != 0 && !listContains(dest.Methods, m) {
if len(dest.Methods) != 0 && !slices.Contains(dest.Methods, m) {
merr = multierror.Append(merr, wrapExclPermRuleErr(resource.ErrInvalidListElement{
Name: "exclude_permission_header_rule",
Wrapped: errExclValuesMustBeSubset,
@ -301,7 +303,7 @@ func validatePermission(p *pbauth.Permission, id *pbresource.ID, wrapErr func(er
}
}
for _, port := range excl.PortNames {
if len(dest.PortNames) != 0 && !listContains(dest.PortNames, port) {
if len(dest.PortNames) != 0 && !slices.Contains(dest.PortNames, port) {
merr = multierror.Append(merr, wrapExclPermRuleErr(resource.ErrInvalidListElement{
Name: "exclude_permission_header_rule",
Wrapped: errExclValuesMustBeSubset,
@ -321,15 +323,6 @@ func validatePermission(p *pbauth.Permission, id *pbresource.ID, wrapErr func(er
return merr
}
func listContains(list []string, str string) bool {
for _, item := range list {
if item == str {
return true
}
}
return false
}
func sourceHasIncompatibleTenancies(src pbauth.SourceToSpiffe, id *pbresource.ID) bool {
if id.Tenancy == nil {
id.Tenancy = &pbresource.Tenancy{}

31
internal/auth/internal/types/traffic_permissions_test.go

@ -42,6 +42,37 @@ func TestValidateTrafficPermissions(t *testing.T) {
Action: pbauth.Action_ACTION_ALLOW,
},
},
"ok-permissions": {
tp: &pbauth.TrafficPermissions{
Destination: &pbauth.Destination{IdentityName: "wi-1"},
Action: pbauth.Action_ACTION_ALLOW,
Permissions: []*pbauth.Permission{
{
Sources: []*pbauth.Source{
{
IdentityName: "wi-2",
Namespace: "default",
Partition: "default",
},
{
IdentityName: "wi-1",
Namespace: "default",
Partition: "ap1",
},
},
DestinationRules: []*pbauth.DestinationRule{
{
PathPrefix: "/",
Methods: []string{"GET"},
Headers: []*pbauth.DestinationRuleHeader{{Name: "X-Consul-Token", Present: false, Invert: true}},
PortNames: []string{"https"},
Exclude: []*pbauth.ExcludePermissionRule{{PathExact: "/admin"}},
},
},
},
},
},
},
"unspecified-action": {
// Any type other than the TrafficPermissions type would work
// to cause the error we are expecting

2
test-integ/catalogv2/traffic_permissions_test.go

@ -284,7 +284,7 @@ func TestL7TrafficPermissions(t *testing.T) {
}
t.Log(initialTrafficPerms)
// Wait for the resource updates to go through and Envoy to be ready
time.Sleep(500 * time.Millisecond)
time.Sleep(1 * time.Second)
// Check the default server workload envoy config for RBAC filters matching testcase criteria
serverWorkload := cluster.WorkloadsByID(topology.ID{
Partition: "default",

Loading…
Cancel
Save