Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ishustava committed Oct 13, 2023
1 parent 9011dd2 commit 137cbc1
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 20 deletions.
10 changes: 10 additions & 0 deletions internal/catalog/internal/testhelpers/acl_hooks_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,16 @@ func RunWorkloadSelectingTypeACLsTests[T WorkloadSelecting](t *testing.T, typ *p
WriteOK: resourcetest.ALLOW,
ListOK: resourcetest.DEFAULT,
},
"service test write with prefixed selectors and a policy with a specific service": {
Rules: `service "test" { policy = "write" } service "workload" { policy = "read" }`,
Data: getData(&pbcatalog.WorkloadSelector{Prefixes: []string{"workload"}}),
Typ: typ,
ReadOK: resourcetest.ALLOW,
// TODO (ishustava): this is wrong and should be fixed in a follow up PR. We should not allow
// a policy for a specific service when only prefixes are specified in the selector.
WriteOK: resourcetest.ALLOW,
ListOK: resourcetest.DEFAULT,
},
}

for name, tc := range cases {
Expand Down
5 changes: 2 additions & 3 deletions internal/catalog/internal/types/dns_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ func TestDNSPolicyACLs(t *testing.T) {
Weights: &pbcatalog.Weights{Passing: 1, Warning: 0},
}
},
func(registry resource.Registry) {
RegisterDNSPolicy(registry)
})
RegisterDNSPolicy,
)
}
5 changes: 2 additions & 3 deletions internal/catalog/internal/types/health_checks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ func TestHealthChecksACLs(t *testing.T) {
func(selector *pbcatalog.WorkloadSelector) *pbcatalog.HealthChecks {
return &pbcatalog.HealthChecks{Workloads: selector}
},
func(registry resource.Registry) {
RegisterHealthChecks(registry)
})
RegisterHealthChecks,
)
}
18 changes: 18 additions & 0 deletions internal/catalog/internal/types/health_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,24 @@ func TestHealthStatusACLs(t *testing.T) {
WriteOK: resourcetest.ALLOW,
ListOK: resourcetest.DEFAULT,
},
"node test read with workload owner": {
Rules: `node "test" { policy = "read" }`,
Data: healthStatusData,
Owner: workload,
Typ: pbcatalog.HealthStatusType,
ReadOK: resourcetest.DENY,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
"node test write with workload owner": {
Rules: `node "test" { policy = "write" }`,
Data: healthStatusData,
Owner: workload,
Typ: pbcatalog.HealthStatusType,
ReadOK: resourcetest.DENY,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
}

for name, tc := range cases {
Expand Down
5 changes: 2 additions & 3 deletions internal/catalog/internal/types/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ func TestServiceACLs(t *testing.T) {
func(selector *pbcatalog.WorkloadSelector) *pbcatalog.Service {
return &pbcatalog.Service{Workloads: selector}
},
func(registry resource.Registry) {
RegisterService(registry)
})
RegisterService,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ func TestDestinationsConfigurationACLs(t *testing.T) {
func(selector *pbcatalog.WorkloadSelector) *pbmesh.DestinationsConfiguration {
return &pbmesh.DestinationsConfiguration{Workloads: selector}
},
func(registry resource.Registry) {
RegisterDestinationsConfiguration(registry)
})
RegisterDestinationsConfiguration,
)
}

func TestValidateDestinationsConfiguration(t *testing.T) {
Expand Down
5 changes: 2 additions & 3 deletions internal/mesh/internal/types/destinations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,6 @@ func TestDestinationsACLs(t *testing.T) {
func(selector *pbcatalog.WorkloadSelector) *pbmesh.Destinations {
return &pbmesh.Destinations{Workloads: selector}
},
func(registry resource.Registry) {
RegisterDestinations(registry)
})
RegisterDestinations,
)
}
10 changes: 6 additions & 4 deletions internal/mesh/internal/types/proxy_configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ import (
func TestProxyConfigurationACLs(t *testing.T) {
catalogtesthelpers.RunWorkloadSelectingTypeACLsTests[*pbmesh.ProxyConfiguration](t, pbmesh.ProxyConfigurationType,
func(selector *pbcatalog.WorkloadSelector) *pbmesh.ProxyConfiguration {
return &pbmesh.ProxyConfiguration{Workloads: selector}
return &pbmesh.ProxyConfiguration{
Workloads: selector,
DynamicConfig: &pbmesh.DynamicConfig{},
}
},
func(registry resource.Registry) {
RegisterProxyConfiguration(registry)
})
RegisterProxyConfiguration,
)
}

func TestMutateProxyConfiguration(t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion internal/resource/resourcetest/acls.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,15 @@ func RunACLTestCase(t *testing.T, tc ACLTestCase, registry resource.Registry) {
reg, ok := registry.Resolve(tc.Typ)
require.True(t, ok)

resolvedType, ok := registry.Resolve(tc.Typ)
require.True(t, ok)

res := Resource(tc.Typ, "test").
WithTenancy(resource.DefaultNamespacedTenancy()).
WithTenancy(DefaultTenancyForType(t, resolvedType)).
WithOwner(tc.Owner).
WithData(t, tc.Data).
Build()

ValidateAndNormalize(t, registry, res)

config := acl.Config{
Expand Down
16 changes: 16 additions & 0 deletions internal/resource/resourcetest/tenancy.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package resourcetest

import (
"strings"
"testing"

"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/proto-public/pbresource"
Expand Down Expand Up @@ -35,3 +36,18 @@ func Tenancy(s string) *pbresource.Tenancy {
return &pbresource.Tenancy{Partition: "BAD", Namespace: "BAD", PeerName: "BAD"}
}
}

func DefaultTenancyForType(t *testing.T, reg resource.Registration) *pbresource.Tenancy {
switch reg.Scope {
case resource.ScopeNamespace:
return resource.DefaultNamespacedTenancy()
case resource.ScopePartition:
return resource.DefaultPartitionedTenancy()
case resource.ScopeCluster:
return resource.DefaultClusteredTenancy()
default:
t.Fatalf("unsupported resource scope: %v", reg.Scope)
return nil
}
return nil

Check failure on line 52 in internal/resource/resourcetest/tenancy.go

View workflow job for this annotation

GitHub Actions / lint / lint

unreachable: unreachable code (govet)

Check failure on line 52 in internal/resource/resourcetest/tenancy.go

View workflow job for this annotation

GitHub Actions / lint-32bit / lint

unreachable: unreachable code (govet)
}

0 comments on commit 137cbc1

Please sign in to comment.