Skip to content

Commit

Permalink
Merge branch 'main' into NET-4135
Browse files Browse the repository at this point in the history
  • Loading branch information
absolutelightning authored Sep 20, 2023
2 parents eaed33c + 3a2e620 commit 5d366e8
Show file tree
Hide file tree
Showing 90 changed files with 1,523 additions and 602 deletions.
3 changes: 3 additions & 0 deletions .changelog/18573.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
xds: Use downstream protocol when connecting to local app
```
3 changes: 3 additions & 0 deletions .changelog/18797.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
command: Adds -since flag in consul debug command which internally calls hcdiag for debug information in the past.
```
1 change: 1 addition & 0 deletions .github/workflows/reusable-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ jobs:
- "envoyextensions"
- "troubleshoot"
- "test/integration/consul-container"
- "test-integ"
- "testing/deployer"
fail-fast: true
name: lint ${{ matrix.directory }}
Expand Down
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: 10 additions & 1 deletion agent/config/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1134,6 +1134,15 @@ func (b *builder) build() (rt RuntimeConfig, err error) {
return RuntimeConfig{}, fmt.Errorf("cache.entry_fetch_rate must be strictly positive, was: %v", rt.Cache.EntryFetchRate)
}

// TODO(CC-6389): Remove once resource-apis is no longer considered experimental and is supported by HCP
if stringslice.Contains(rt.Experiments, consul.CatalogResourceExperimentName) && rt.IsCloudEnabled() {
// Allow override of this check for development/testing purposes. Should not be used in production
override, err := strconv.ParseBool(os.Getenv("CONSUL_OVERRIDE_HCP_RESOURCE_APIS_CHECK"))
if err != nil || !override {
return RuntimeConfig{}, fmt.Errorf("`experiments` cannot include 'resource-apis' when HCP `cloud` configuration is set")
}
}

if rt.UIConfig.MetricsProvider == "prometheus" {
// Handle defaulting for the built-in version of prometheus.
if len(rt.UIConfig.MetricsProxy.PathAllowlist) == 0 {
Expand Down Expand Up @@ -2550,7 +2559,7 @@ func (b *builder) cloudConfigVal(v Config) hcpconfig.CloudConfig {
val := hcpconfig.CloudConfig{
ResourceID: os.Getenv("HCP_RESOURCE_ID"),
}
// Node id might get overriden in setup.go:142
// Node id might get overridden in setup.go:142
nodeID := stringVal(v.NodeID)
val.NodeID = types.NodeID(nodeID)
val.NodeName = b.nodeName(v.NodeName)
Expand Down
69 changes: 69 additions & 0 deletions agent/config/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -575,3 +575,72 @@ func TestBuidler_hostMetricsWithCloud(t *testing.T) {
require.NotNil(t, cfg)
require.True(t, cfg.Telemetry.EnableHostMetrics)
}

func TestBuilder_WarnCloudConfigWithResourceApis(t *testing.T) {
tests := []struct {
name string
hcl string
expectErr bool
override bool
}{
{
name: "base_case",
hcl: ``,
},
{
name: "resource-apis_no_cloud",
hcl: `experiments = ["resource-apis"]`,
},
{
name: "cloud-config_no_experiments",
hcl: `cloud{ resource_id = "abc" client_id = "abc" client_secret = "abc"}`,
},
{
name: "cloud-config_resource-apis_experiment",
hcl: `
experiments = ["resource-apis"]
cloud{ resource_id = "abc" client_id = "abc" client_secret = "abc"}`,
expectErr: true,
},
{
name: "cloud-config_other_experiment",
hcl: `
experiments = ["test"]
cloud{ resource_id = "abc" client_id = "abc" client_secret = "abc"}`,
},
{
name: "cloud-config_resource-apis_experiment_override",
hcl: `
experiments = ["resource-apis"]
cloud{ resource_id = "abc" client_id = "abc" client_secret = "abc"}`,
override: true,
},
}
for _, tc := range tests {
// using dev mode skips the need for a data dir
devMode := true
builderOpts := LoadOpts{
DevMode: &devMode,
Overrides: []Source{
FileSource{
Name: "overrides",
Format: "hcl",
Data: tc.hcl,
},
},
}
if tc.override {
os.Setenv("CONSUL_OVERRIDE_HCP_RESOURCE_APIS_CHECK", "1")
}
_, err := Load(builderOpts)
if tc.override {
os.Unsetenv("CONSUL_OVERRIDE_HCP_RESOURCE_APIS_CHECK")
}
if tc.expectErr {
require.Error(t, err)
require.Contains(t, err.Error(), "cannot include 'resource-apis' when HCP")
} else {
require.NoError(t, err)
}
}
}
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 @@ -59,7 +59,6 @@ func (s *Server) GetEnvoyBootstrapParams(ctx context.Context, req *pbdataplane.G
Tenancy: &pbresource.Tenancy{
Namespace: req.Namespace,
Partition: req.Partition,
PeerName: "local",
},
Type: catalog.WorkloadType,
}
Expand All @@ -69,6 +68,7 @@ func (s *Server) GetEnvoyBootstrapParams(ctx context.Context, req *pbdataplane.G
if err != nil {
// This error should already include the gRPC status code and so we don't need to wrap it
// in status.Error.
logger.Error("Error looking up workload", "error", err)
return nil, err
}
var workload pbcatalog.Workload
Expand All @@ -93,6 +93,7 @@ func (s *Server) GetEnvoyBootstrapParams(ctx context.Context, req *pbdataplane.G
Type: mesh.ProxyConfigurationType,
})
if err != nil {
logger.Error("Error looking up proxyConfiguration", "error", err)
return nil, err
}

Expand Down
33 changes: 31 additions & 2 deletions agent/xds/clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -1096,8 +1096,14 @@ func (s *ResourceGenerator) makeAppCluster(cfgSnap *proxycfg.ConfigSnapshot, nam
protocol = cfg.Protocol
}
if protocol == "http2" || protocol == "grpc" {
if err := s.setHttp2ProtocolOptions(c); err != nil {
return c, err
if name == xdscommon.LocalAppClusterName {
if err := s.setLocalAppHttpProtocolOptions(c); err != nil {
return c, err
}
} else {
if err := s.setHttp2ProtocolOptions(c); err != nil {
return c, err
}
}
}
if cfg.MaxInboundConnections > 0 {
Expand Down Expand Up @@ -2016,6 +2022,29 @@ func (s *ResourceGenerator) setHttp2ProtocolOptions(c *envoy_cluster_v3.Cluster)
return nil
}

// Allows forwarding either HTTP/1.1 or HTTP/2 traffic to the local application.
// The protocol used depends on the protocol received from the downstream service
// on the public listener.
func (s *ResourceGenerator) setLocalAppHttpProtocolOptions(c *envoy_cluster_v3.Cluster) error {
cfg := &envoy_upstreams_v3.HttpProtocolOptions{
UpstreamProtocolOptions: &envoy_upstreams_v3.HttpProtocolOptions_UseDownstreamProtocolConfig{
UseDownstreamProtocolConfig: &envoy_upstreams_v3.HttpProtocolOptions_UseDownstreamHttpConfig{
HttpProtocolOptions: &envoy_core_v3.Http1ProtocolOptions{},
Http2ProtocolOptions: &envoy_core_v3.Http2ProtocolOptions{},
},
},
}
any, err := anypb.New(cfg)
if err != nil {
return err
}
c.TypedExtensionProtocolOptions = map[string]*anypb.Any{
"envoy.extensions.upstreams.http.v3.HttpProtocolOptions": any,
}

return nil
}

// generatePeeredClusterName returns an SNI-like cluster name which mimics PeeredServiceSNI
// but excludes partition information which could be ambiguous (local vs remote partition).
func generatePeeredClusterName(uid proxycfg.UpstreamID, tb *pbpeering.PeeringTrustBundle) string {
Expand Down
11 changes: 8 additions & 3 deletions agent/xds/delta.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,13 @@ func getEnvoyConfiguration(proxySnapshot proxysnapshot.ProxySnapshot, logger hcl
)
c := proxySnapshot.(*proxytracker.ProxyState)
logger.Trace("ProxyState", c)
return generator.AllResourcesFromIR(c)
resources, err := generator.AllResourcesFromIR(c)
if err != nil {
logger.Error("error generating resources from proxy state template", "err", err)
return nil, err
}
logger.Trace("generated resources from proxy state template", "resources", resources)
return resources, nil
default:
return nil, errors.New("proxysnapshot must be of type ProxyState or ConfigSnapshot")
}
Expand Down Expand Up @@ -428,9 +434,8 @@ func newResourceIDFromEnvoyNode(node *envoy_config_core_v3.Node) *pbresource.ID
Tenancy: &pbresource.Tenancy{
Namespace: entMeta.NamespaceOrDefault(),
Partition: entMeta.PartitionOrDefault(),
PeerName: "local",
},
Type: mesh.ProxyStateTemplateV1AlphaType,
Type: mesh.ProxyStateTemplateType,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@
"typedExtensionProtocolOptions": {
"envoy.extensions.upstreams.http.v3.HttpProtocolOptions": {
"@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions",
"explicitHttpConfig": {
"http2ProtocolOptions": {}
"useDownstreamProtocolConfig": {
"http2ProtocolOptions": {},
"httpProtocolOptions": {}
}
}
}
Expand Down
34 changes: 33 additions & 1 deletion agent/xdsv2/cluster_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,13 @@ func (pr *ProxyResources) makeEnvoyStaticCluster(name string, protocol string, s
if ok {
cluster.LoadAssignment = makeEnvoyClusterLoadAssignment(name, endpointList.Endpoints)
}
err := addHttpProtocolOptions(protocol, cluster)

var err error
if name == xdscommon.LocalAppClusterName {
err = addLocalAppHttpProtocolOptions(protocol, cluster)
} else {
err = addHttpProtocolOptions(protocol, cluster)
}
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -243,6 +249,30 @@ func (pr *ProxyResources) makeEnvoyAggregateCluster(name string, protocol string
return clusters, nil
}

func addLocalAppHttpProtocolOptions(protocol string, c *envoy_cluster_v3.Cluster) error {
if !(protocol == "http2" || protocol == "grpc") {
// do not error. returning nil means it won't get set.
return nil
}
cfg := &envoy_upstreams_v3.HttpProtocolOptions{
UpstreamProtocolOptions: &envoy_upstreams_v3.HttpProtocolOptions_UseDownstreamProtocolConfig{
UseDownstreamProtocolConfig: &envoy_upstreams_v3.HttpProtocolOptions_UseDownstreamHttpConfig{
HttpProtocolOptions: &envoy_core_v3.Http1ProtocolOptions{},
Http2ProtocolOptions: &envoy_core_v3.Http2ProtocolOptions{},
},
},
}
any, err := anypb.New(cfg)
if err != nil {
return err
}
c.TypedExtensionProtocolOptions = map[string]*anypb.Any{
"envoy.extensions.upstreams.http.v3.HttpProtocolOptions": any,
}

return nil
}

func addHttpProtocolOptions(protocol string, c *envoy_cluster_v3.Cluster) error {
if !(protocol == "http2" || protocol == "grpc") {
// do not error. returning nil means it won't get set.
Expand Down Expand Up @@ -346,6 +376,8 @@ func addEnvoyLBToCluster(dynamicConfig *pbproxystate.DynamicEndpointGroupConfig,
}

// TODO(proxystate): In a future PR this will create clusters and add it to ProxyResources.proxyState
// Currently, we do not traverse the listener -> endpoint paths and instead just generate each resource by iterating
// through its top level map. In the future we want to traverse these paths to ensure each listener has a cluster, etc.
func (pr *ProxyResources) makeEnvoyClusterFromL4Destination(l4 *pbproxystate.L4Destination) error {
return nil
}
Loading

0 comments on commit 5d366e8

Please sign in to comment.