diff --git a/agent/agent.go b/agent/agent.go index 87a1da86cea4..8354320ad670 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -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) } diff --git a/agent/consul/acl.go b/agent/consul/acl.go index a73372a040ad..ddba359d949f 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -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: diff --git a/agent/consul/server.go b/agent/consul/server.go index d2fd0472b90b..39d5b0a543a7 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -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() @@ -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 @@ -923,6 +931,7 @@ func (s *Server) registerControllers(deps Deps, proxyUpdater ProxyUpdater) { LeafCertManager: deps.LeafCertManager, LocalDatacenter: s.config.Datacenter, + DefaultAllow: defaultAllow, ProxyUpdater: proxyUpdater, }) } @@ -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 { diff --git a/internal/auth/internal/types/computed_traffic_permissions.go b/internal/auth/internal/types/computed_traffic_permissions.go index 753d875f8b1e..8d30c1f0200e 100644 --- a/internal/auth/internal/types/computed_traffic_permissions.go +++ b/internal/auth/internal/types/computed_traffic_permissions.go @@ -10,7 +10,7 @@ import ( ) const ( - ComputedTrafficPermissionsKind = "ComputedTrafficPermission" + ComputedTrafficPermissionsKind = "ComputedTrafficPermissions" ) var ( diff --git a/internal/mesh/internal/controllers/register.go b/internal/mesh/internal/controllers/register.go index cc62da83dec0..b9272e58053d 100644 --- a/internal/mesh/internal/controllers/register.go +++ b/internal/mesh/internal/controllers/register.go @@ -21,6 +21,7 @@ import ( type Dependencies struct { TrustDomainFetcher sidecarproxy.TrustDomainFetcher LocalDatacenter string + DefaultAllow bool TrustBundleFetcher xds.TrustBundleFetcher ProxyUpdater xds.ProxyUpdater LeafCertManager *leafcert.Manager @@ -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()) diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/builder.go b/internal/mesh/internal/controllers/sidecarproxy/builder/builder.go index 0c0cd0661d17..ba600f4fe044 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/builder.go +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/builder.go @@ -16,6 +16,7 @@ type Builder struct { proxyCfg *pbmesh.ProxyConfiguration trustDomain string localDatacenter string + defaultAllow bool } func New( @@ -23,12 +24,14 @@ func New( 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{ @@ -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 } diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder_multiport_test.go b/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder_multiport_test.go index 1443acc63255..a8a8e0820147 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder_multiport_test.go +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder_multiport_test.go @@ -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() diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder_test.go b/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder_test.go index a2381da05d6c..9af0853bef33 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder_test.go +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder_test.go @@ -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() @@ -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() diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/local_app_multiport_test.go b/internal/mesh/internal/controllers/sidecarproxy/builder/local_app_multiport_test.go index 96b795109f53..e1d4cb664a08 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/local_app_multiport_test.go +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/local_app_multiport_test.go @@ -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() diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/local_app_test.go b/internal/mesh/internal/controllers/sidecarproxy/builder/local_app_test.go index 66985c74b4e8..ba372efe68de 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/local_app_test.go +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/local_app_test.go @@ -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{ @@ -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) diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/source/l4-multiple-workload-addresses-with-specific-ports.golden b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/source/l4-multiple-workload-addresses-with-specific-ports.golden index d9320a445ac8..f34753025900 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/source/l4-multiple-workload-addresses-with-specific-ports.golden +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/source/l4-multiple-workload-addresses-with-specific-ports.golden @@ -74,7 +74,8 @@ } ] } - ] + ], + "trafficPermissionDefaultAllow": true }, "requiredLeafCertificates": { "test-identity": { diff --git a/internal/mesh/internal/controllers/sidecarproxy/controller.go b/internal/mesh/internal/controllers/sidecarproxy/controller.go index 416d77261066..73ce3c6c3f02 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/controller.go +++ b/internal/mesh/internal/controllers/sidecarproxy/controller.go @@ -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") @@ -99,6 +100,7 @@ func Controller( identitiesCache: identitiesCache, getTrustDomain: trustDomainFetcher, dc: dc, + defaultAllow: defaultAllow, }) } @@ -108,6 +110,7 @@ type reconciler struct { computedRoutesCache *sidecarproxycache.ComputedRoutesCache identitiesCache *sidecarproxycache.IdentitiesCache getTrustDomain TrustDomainFetcher + defaultAllow bool dc string } @@ -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. diff --git a/internal/mesh/internal/controllers/sidecarproxy/controller_test.go b/internal/mesh/internal/controllers/sidecarproxy/controller_test.go index 902c9f47de6b..6aab1931e873 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/controller_test.go +++ b/internal/mesh/internal/controllers/sidecarproxy/controller_test.go @@ -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() } @@ -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) @@ -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) @@ -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)) } diff --git a/internal/resource/resourcetest/decode.go b/internal/resource/resourcetest/decode.go index d68fff865517..109ad39ceb75 100644 --- a/internal/resource/resourcetest/decode.go +++ b/internal/resource/resourcetest/decode.go @@ -4,8 +4,6 @@ package resourcetest import ( - "testing" - "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" @@ -13,8 +11,8 @@ import ( "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 }