Skip to content

Commit

Permalink
Honor Default Traffic Permissions in V2 (#18886)
Browse files Browse the repository at this point in the history
wire up v2 default traffic permissions
  • Loading branch information
erichaberkorn authored Sep 19, 2023
1 parent 9b497f8 commit 170417a
Show file tree
Hide file tree
Showing 14 changed files with 87 additions and 27 deletions.
9 changes: 2 additions & 7 deletions agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -657,13 +657,8 @@ func (a *Agent) Start(ctx context.Context) error {
// Create proxy config manager now because it is a dependency of creating the proxyWatcher
// which will be passed to consul.NewServer so that it is then passed to the
// controller registration for the XDS controller in v2 mode, and the xds server in v1 and v2 mode.
var intentionDefaultAllow bool
switch a.config.ACLResolverSettings.ACLDefaultPolicy {
case "allow":
intentionDefaultAllow = true
case "deny":
intentionDefaultAllow = false
default:
intentionDefaultAllow, err := a.config.ACLResolverSettings.IsDefaultAllow()
if err != nil {
return fmt.Errorf("unexpected ACL default policy value of %q", a.config.ACLResolverSettings.ACLDefaultPolicy)
}

Expand Down
11 changes: 11 additions & 0 deletions agent/consul/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,17 @@ type ACLResolverSettings struct {
ACLDefaultPolicy string
}

func (s ACLResolverSettings) IsDefaultAllow() (bool, error) {
switch s.ACLDefaultPolicy {
case "allow":
return true, nil
case "deny":
return false, nil
default:
return false, fmt.Errorf("unexpected ACL default policy value of %q", s.ACLDefaultPolicy)
}
}

// ACLResolver is the type to handle all your token and policy resolution needs.
//
// Supports:
Expand Down
15 changes: 13 additions & 2 deletions agent/consul/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,9 @@ func NewServer(config *Config, flat Deps, externalGRPCServer *grpc.Server,
s.insecureResourceServiceClient,
logger.Named(logging.ControllerRuntime),
)
s.registerControllers(flat, proxyUpdater)
if err := s.registerControllers(flat, proxyUpdater); err != nil {
return nil, err
}
go s.controllerManager.Run(&lib.StopChannelContext{StopCh: shutdownCh})

go s.trackLeaderChanges()
Expand Down Expand Up @@ -895,9 +897,15 @@ func NewServer(config *Config, flat Deps, externalGRPCServer *grpc.Server,
return s, nil
}

func (s *Server) registerControllers(deps Deps, proxyUpdater ProxyUpdater) {
func (s *Server) registerControllers(deps Deps, proxyUpdater ProxyUpdater) error {
if stringslice.Contains(deps.Experiments, CatalogResourceExperimentName) {
catalog.RegisterControllers(s.controllerManager, catalog.DefaultControllerDependencies())

defaultAllow, err := s.config.ACLResolverSettings.IsDefaultAllow()
if err != nil {
return err
}

mesh.RegisterControllers(s.controllerManager, mesh.ControllerDependencies{
TrustBundleFetcher: func() (*pbproxystate.TrustBundle, error) {
var bundle pbproxystate.TrustBundle
Expand All @@ -923,6 +931,7 @@ func (s *Server) registerControllers(deps Deps, proxyUpdater ProxyUpdater) {

LeafCertManager: deps.LeafCertManager,
LocalDatacenter: s.config.Datacenter,
DefaultAllow: defaultAllow,
ProxyUpdater: proxyUpdater,
})
}
Expand All @@ -932,6 +941,8 @@ func (s *Server) registerControllers(deps Deps, proxyUpdater ProxyUpdater) {
if s.config.DevMode {
demo.RegisterControllers(s.controllerManager)
}

return nil
}

func newGRPCHandlerFromConfig(deps Deps, config *Config, s *Server) connHandler {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

const (
ComputedTrafficPermissionsKind = "ComputedTrafficPermission"
ComputedTrafficPermissionsKind = "ComputedTrafficPermissions"
)

var (
Expand Down
3 changes: 2 additions & 1 deletion internal/mesh/internal/controllers/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
type Dependencies struct {
TrustDomainFetcher sidecarproxy.TrustDomainFetcher
LocalDatacenter string
DefaultAllow bool
TrustBundleFetcher xds.TrustBundleFetcher
ProxyUpdater xds.ProxyUpdater
LeafCertManager *leafcert.Manager
Expand All @@ -44,7 +45,7 @@ func Register(mgr *controller.Manager, deps Dependencies) {
m = sidecarproxymapper.New(destinationsCache, proxyCfgCache, computedRoutesCache, identitiesCache)
)
mgr.Register(
sidecarproxy.Controller(destinationsCache, proxyCfgCache, computedRoutesCache, identitiesCache, m, deps.TrustDomainFetcher, deps.LocalDatacenter),
sidecarproxy.Controller(destinationsCache, proxyCfgCache, computedRoutesCache, identitiesCache, m, deps.TrustDomainFetcher, deps.LocalDatacenter, deps.DefaultAllow),
)

mgr.Register(routes.Controller())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,22 @@ type Builder struct {
proxyCfg *pbmesh.ProxyConfiguration
trustDomain string
localDatacenter string
defaultAllow bool
}

func New(
id *pbresource.ID,
identity *pbresource.Reference,
trustDomain string,
dc string,
defaultAllow bool,
proxyCfg *pbmesh.ProxyConfiguration,
) *Builder {
return &Builder{
id: id,
trustDomain: trustDomain,
localDatacenter: dc,
defaultAllow: defaultAllow,
proxyCfg: proxyCfg,
proxyStateTemplate: &pbmesh.ProxyStateTemplate{
ProxyState: &pbmesh.ProxyState{
Expand All @@ -55,6 +58,7 @@ func (b *Builder) Build() *pbmesh.ProxyStateTemplate {
b.proxyStateTemplate.RequiredTrustBundles[b.id.Tenancy.PeerName] = &pbproxystate.TrustBundleRef{
Peer: b.id.Tenancy.PeerName,
}
b.proxyStateTemplate.ProxyState.TrafficPermissionDefaultAllow = b.defaultAllow

return b.proxyStateTemplate
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func TestBuildMultiportImplicitDestinations(t *testing.T) {

for name, c := range cases {
t.Run(name, func(t *testing.T) {
proxyTmpl := New(testProxyStateTemplateID(), testIdentityRef(), trustDomain, datacenter, proxyCfg).
proxyTmpl := New(testProxyStateTemplateID(), testIdentityRef(), trustDomain, datacenter, false, proxyCfg).
BuildDestinations(c.getDestinations()).
Build()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func TestBuildExplicitDestinations(t *testing.T) {

for name, c := range cases {
t.Run(name, func(t *testing.T) {
proxyTmpl := New(testProxyStateTemplateID(), testIdentityRef(), "foo.consul", "dc1", nil).
proxyTmpl := New(testProxyStateTemplateID(), testIdentityRef(), "foo.consul", "dc1", false, nil).
BuildDestinations(c.destinations).
Build()

Expand Down Expand Up @@ -360,7 +360,7 @@ func TestBuildImplicitDestinations(t *testing.T) {

for name, c := range cases {
t.Run(name, func(t *testing.T) {
proxyTmpl := New(testProxyStateTemplateID(), testIdentityRef(), "foo.consul", "dc1", proxyCfg).
proxyTmpl := New(testProxyStateTemplateID(), testIdentityRef(), "foo.consul", "dc1", false, proxyCfg).
BuildDestinations(c.destinations).
Build()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestBuildLocalApp_Multiport(t *testing.T) {

for name, c := range cases {
t.Run(name, func(t *testing.T) {
proxyTmpl := New(testProxyStateTemplateID(), testIdentityRef(), "foo.consul", "dc1", nil).
proxyTmpl := New(testProxyStateTemplateID(), testIdentityRef(), "foo.consul", "dc1", false, nil).
BuildLocalApp(c.workload, nil).
Build()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ import (

func TestBuildLocalApp(t *testing.T) {
cases := map[string]struct {
workload *pbcatalog.Workload
ctp *pbauth.ComputedTrafficPermissions
workload *pbcatalog.Workload
ctp *pbauth.ComputedTrafficPermissions
defaultAllow bool
}{
"source/l4-single-workload-address-without-ports": {
workload: &pbcatalog.Workload{
Expand Down Expand Up @@ -83,12 +84,13 @@ func TestBuildLocalApp(t *testing.T) {
},
},
},
defaultAllow: true,
},
}

for name, c := range cases {
t.Run(name, func(t *testing.T) {
proxyTmpl := New(testProxyStateTemplateID(), testIdentityRef(), "foo.consul", "dc1", nil).
proxyTmpl := New(testProxyStateTemplateID(), testIdentityRef(), "foo.consul", "dc1", c.defaultAllow, nil).
BuildLocalApp(c.workload, c.ctp).
Build()
actual := protoToJSON(t, proxyTmpl)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@
}
]
}
]
],
"trafficPermissionDefaultAllow": true
},
"requiredLeafCertificates": {
"test-identity": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ func Controller(
mapper *sidecarproxymapper.Mapper,
trustDomainFetcher TrustDomainFetcher,
dc string,
defaultAllow bool,
) controller.Controller {
if destinationsCache == nil || proxyCfgCache == nil || computedRoutesCache == nil || identitiesCache == nil || mapper == nil || trustDomainFetcher == nil {
panic("destinations cache, proxy configuration cache, computed routes cache, identities cache, mapper, and trust domain fetcher are required")
Expand Down Expand Up @@ -99,6 +100,7 @@ func Controller(
identitiesCache: identitiesCache,
getTrustDomain: trustDomainFetcher,
dc: dc,
defaultAllow: defaultAllow,
})
}

Expand All @@ -108,6 +110,7 @@ type reconciler struct {
computedRoutesCache *sidecarproxycache.ComputedRoutesCache
identitiesCache *sidecarproxycache.IdentitiesCache
getTrustDomain TrustDomainFetcher
defaultAllow bool
dc string
}

Expand Down Expand Up @@ -194,7 +197,7 @@ func (r *reconciler) Reconcile(ctx context.Context, rt controller.Runtime, req c
ctp = trafficPermissions.Data
}

b := builder.New(req.ID, identityRefFromWorkload(workload), trustDomain, r.dc, proxyCfg).
b := builder.New(req.ID, identityRefFromWorkload(workload), trustDomain, r.dc, r.defaultAllow, proxyCfg).
BuildLocalApp(workload.Data, ctp)

// Get all destinationsData.
Expand Down
40 changes: 37 additions & 3 deletions internal/mesh/internal/controllers/sidecarproxy/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func (suite *meshControllerTestSuite) SetupTest() {
Tenancy: suite.apiWorkloadID.Tenancy,
}

suite.proxyStateTemplate = builder.New(suite.apiWorkloadID, identityRef, "test.consul", "dc1", nil).
suite.proxyStateTemplate = builder.New(suite.apiWorkloadID, identityRef, "test.consul", "dc1", false, nil).
BuildLocalApp(suite.apiWorkload, suite.computedTrafficPermissionsData).
Build()
}
Expand Down Expand Up @@ -357,7 +357,7 @@ func (suite *meshControllerTestSuite) TestController() {
)
trustDomainFetcher := func() (string, error) { return "test.consul", nil }

mgr.Register(Controller(destinationsCache, proxyCfgCache, computedRoutesCache, identitiesCache, m, trustDomainFetcher, "dc1"))
mgr.Register(Controller(destinationsCache, proxyCfgCache, computedRoutesCache, identitiesCache, m, trustDomainFetcher, "dc1", false))
mgr.SetRaftLeader(true)
go mgr.Run(suite.ctx)

Expand Down Expand Up @@ -554,7 +554,10 @@ func (suite *meshControllerTestSuite) TestController() {
requireImplicitDestinationsFound(t, "db", webProxyStateTemplate)
})

testutil.RunStep(suite.T(), "computed traffic permissions force regeneration", func(t *testing.T) {
testutil.RunStep(suite.T(), "traffic permissions", func(t *testing.T) {
dec := resourcetest.MustDecode[*pbmesh.ProxyStateTemplate](t, apiProxyStateTemplate)
require.False(t, dec.Data.ProxyState.TrafficPermissionDefaultAllow)

suite.runtime.Logger.Trace("deleting computed traffic permissions")
_, err := suite.client.Delete(suite.ctx, &pbresource.DeleteRequest{Id: suite.computedTrafficPermissions.Id})
require.NoError(t, err)
Expand Down Expand Up @@ -622,6 +625,37 @@ func (suite *meshControllerTestSuite) TestController() {
})
}

func (suite *meshControllerTestSuite) TestControllerDefaultAllow() {
// Run the controller manager
mgr := controller.NewManager(suite.client, suite.runtime.Logger)

// Initialize controller dependencies.
var (
destinationsCache = sidecarproxycache.NewDestinationsCache()
proxyCfgCache = sidecarproxycache.NewProxyConfigurationCache()
computedRoutesCache = sidecarproxycache.NewComputedRoutesCache()
identitiesCache = sidecarproxycache.NewIdentitiesCache()
m = sidecarproxymapper.New(destinationsCache, proxyCfgCache, computedRoutesCache, identitiesCache)
)
trustDomainFetcher := func() (string, error) { return "test.consul", nil }

mgr.Register(Controller(destinationsCache, proxyCfgCache, computedRoutesCache, identitiesCache, m, trustDomainFetcher, "dc1", true))
mgr.SetRaftLeader(true)
go mgr.Run(suite.ctx)

var (
// Create proxy state template IDs to check against in this test.
webProxyStateTemplateID = resourcetest.Resource(types.ProxyStateTemplateType, "web-def").ID()
)

retry.Run(suite.T(), func(r *retry.R) {
suite.client.RequireResourceExists(r, webProxyStateTemplateID)
webProxyStateTemplate := suite.client.RequireResourceExists(r, webProxyStateTemplateID)
dec := resourcetest.MustDecode[*pbmesh.ProxyStateTemplate](r, webProxyStateTemplate)
require.True(r, dec.Data.ProxyState.TrafficPermissionDefaultAllow)
})
}

func TestMeshController(t *testing.T) {
suite.Run(t, new(meshControllerTestSuite))
}
Expand Down
6 changes: 2 additions & 4 deletions internal/resource/resourcetest/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,15 @@
package resourcetest

import (
"testing"

"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"

"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/proto-public/pbresource"
)

func MustDecode[T proto.Message](t *testing.T, res *pbresource.Resource) *resource.DecodedResource[T] {
dec, err := resource.Decode[T](res)
func MustDecode[Tp proto.Message](t T, res *pbresource.Resource) *resource.DecodedResource[Tp] {
dec, err := resource.Decode[Tp](res)
require.NoError(t, err)
return dec
}

0 comments on commit 170417a

Please sign in to comment.