Skip to content

Commit 2ffe87b

Browse files
trujillo-adamtrujillo-adam
authored
Merge branch 'main' into docs/proxy-defaults-reformat-other-improvements
2 parents 0514ba9 + ac867d6 commit 2ffe87b

File tree

77 files changed

+704
-340
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

77 files changed

+704
-340
lines changed

‎.changelog/18625.txt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
```release-note:improvement
2+
Adds flag -append-filename (which works on values version, dc, node and status) to consul snapshot save command.
3+
Adding the flag -append-filename version,dc,node,status will add consul version, consul datacenter, node name and leader/follower
4+
(status) in the file name given in the snapshot save command before the file extension.
5+
```

‎.changelog/18636.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
connect: Fix issue where Envoy endpoints would not populate correctly after a snapshot restore.
3+
```

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
## 1.16.1 (August 8, 2023)
22

3+
KNOWN ISSUES:
4+
5+
* connect: Consul versions 1.16.0 and 1.16.1 may have issues when a snapshot restore is performed and the servers are hosting xDS streams. When this bug triggers, it will cause Envoy to incorrectly populate upstream endpoints. This bug only impacts agent-less service mesh and should be fixed in Consul 1.16.2 by [GH-18636](https://github.com/hashicorp/consul/pull/18636).
6+
37
SECURITY:
48

59
* Update `golang.org/x/net` to v0.13.0 to address [CVE-2023-3978](https://nvd.nist.gov/vuln/detail/CVE-2023-3978). [[GH-18358](https://github.com/hashicorp/consul/issues/18358)]
@@ -136,6 +140,10 @@ https://github.com/rboyer/safeio/pull/3 [[GH-18302](https://github.com/hashicorp
136140

137141
## 1.16.0 (June 26, 2023)
138142

143+
KNOWN ISSUES:
144+
145+
* connect: Consul versions 1.16.0 and 1.16.1 may have issues when a snapshot restore is performed and the servers are hosting xDS streams. When this bug triggers, it will cause Envoy to incorrectly populate upstream endpoints. This bug only impacts agent-less service mesh and should be fixed in Consul 1.16.2 by [GH-18636](https://github.com/hashicorp/consul/pull/18636).
146+
139147
BREAKING CHANGES:
140148

141149
* api: The `/v1/health/connect/` and `/v1/health/ingress/` endpoints now immediately return 403 "Permission Denied" errors whenever a token with insufficient `service:read` permissions is provided. Prior to this change, the endpoints returned a success code with an empty result list when a token with insufficient permissions was provided. [[GH-17424](https://github.com/hashicorp/consul/issues/17424)]

‎agent/agent.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4617,7 +4617,7 @@ func (a *Agent) proxyDataSources() proxycfg.DataSources {
46174617
// interact with ACLs and the streaming backend. See comments in `proxycfgglue.ServerHealthBlocking`
46184618
// for more details.
46194619
// sources.Health = proxycfgglue.ServerHealth(deps, proxycfgglue.ClientHealth(a.rpcClientHealth))
4620-
sources.Health = proxycfgglue.ServerHealthBlocking(deps, proxycfgglue.ClientHealth(a.rpcClientHealth), server.FSM().State())
4620+
sources.Health = proxycfgglue.ServerHealthBlocking(deps, proxycfgglue.ClientHealth(a.rpcClientHealth))
46214621
sources.HTTPChecks = proxycfgglue.ServerHTTPChecks(deps, a.config.NodeName, proxycfgglue.CacheHTTPChecks(a.cache), a.State)
46224622
sources.Intentions = proxycfgglue.ServerIntentions(deps)
46234623
sources.IntentionUpstreams = proxycfgglue.ServerIntentionUpstreams(deps)

‎agent/grpc-external/services/resource/testing/testing.go‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"context"
88
"testing"
99

10+
"github.com/stretchr/testify/mock"
1011
"github.com/stretchr/testify/require"
1112
"google.golang.org/grpc"
1213
"google.golang.org/grpc/credentials/insecure"
@@ -16,6 +17,7 @@ import (
1617
"github.com/hashicorp/consul/acl"
1718
"github.com/hashicorp/consul/acl/resolver"
1819
svc "github.com/hashicorp/consul/agent/grpc-external/services/resource"
20+
"github.com/hashicorp/consul/agent/grpc-external/testutils"
1921
internal "github.com/hashicorp/consul/agent/grpc-internal"
2022
"github.com/hashicorp/consul/agent/structs"
2123
"github.com/hashicorp/consul/internal/resource"
@@ -52,7 +54,27 @@ func AuthorizerFrom(t *testing.T, policyStrs ...string) resolver.Result {
5254
// returns a client to interact with it. ACLs will be disabled and only the
5355
// default partition and namespace are available.
5456
func RunResourceService(t *testing.T, registerFns ...func(resource.Registry)) pbresource.ResourceServiceClient {
55-
return RunResourceServiceWithACL(t, resolver.DANGER_NO_AUTH{}, registerFns...)
57+
// Provide a resolver which will default partition and namespace when not provided. This is similar to user
58+
// initiated requests.
59+
//
60+
// Controllers under test should be providing full tenancy since they will run with the DANGER_NO_AUTH.
61+
mockACLResolver := &svc.MockACLResolver{}
62+
mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
63+
Return(testutils.ACLsDisabled(t), nil).
64+
Run(func(args mock.Arguments) {
65+
// Caller expecting passed in tokenEntMeta and authorizerContext to be filled in.
66+
tokenEntMeta := args.Get(1).(*acl.EnterpriseMeta)
67+
if tokenEntMeta != nil {
68+
FillEntMeta(tokenEntMeta)
69+
}
70+
71+
authzContext := args.Get(2).(*acl.AuthorizerContext)
72+
if authzContext != nil {
73+
FillAuthorizerContext(authzContext)
74+
}
75+
})
76+
77+
return RunResourceServiceWithACL(t, mockACLResolver, registerFns...)
5678
}
5779

5880
func RunResourceServiceWithACL(t *testing.T, aclResolver svc.ACLResolver, registerFns ...func(resource.Registry)) pbresource.ResourceServiceClient {

‎agent/grpc-external/services/resource/testing/testing_ce.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: BUSL-1.1
3+
4+
//go:build !consulent
5+
// +build !consulent
6+
7+
package testing
8+
9+
import (
10+
"github.com/hashicorp/consul/acl"
11+
)
12+
13+
func FillEntMeta(entMeta *acl.EnterpriseMeta) {
14+
// nothing to to in CE.
15+
}
16+
17+
func FillAuthorizerContext(authzContext *acl.AuthorizerContext) {
18+
// nothing to to in CE.
19+
}

‎agent/proxycfg-glue/glue.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ type Store interface {
5252
PeeringTrustBundleList(ws memdb.WatchSet, entMeta acl.EnterpriseMeta) (uint64, []*pbpeering.PeeringTrustBundle, error)
5353
TrustBundleListByService(ws memdb.WatchSet, service, dc string, entMeta acl.EnterpriseMeta) (uint64, []*pbpeering.PeeringTrustBundle, error)
5454
VirtualIPsForAllImportedServices(ws memdb.WatchSet, entMeta acl.EnterpriseMeta) (uint64, []state.ServiceVirtualIP, error)
55+
CheckConnectServiceNodes(ws memdb.WatchSet, serviceName string, entMeta *acl.EnterpriseMeta, peerName string) (uint64, structs.CheckServiceNodes, error)
56+
CheckIngressServiceNodes(ws memdb.WatchSet, serviceName string, entMeta *acl.EnterpriseMeta) (uint64, structs.CheckServiceNodes, error)
57+
CheckServiceNodes(ws memdb.WatchSet, serviceName string, entMeta *acl.EnterpriseMeta, peerName string) (uint64, structs.CheckServiceNodes, error)
5558
}
5659

5760
// CacheCARoots satisfies the proxycfg.CARoots interface by sourcing data from

‎agent/proxycfg-glue/health_blocking.go‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/hashicorp/go-memdb"
1313

1414
"github.com/hashicorp/consul/acl"
15-
"github.com/hashicorp/consul/agent/consul/state"
1615
"github.com/hashicorp/consul/agent/consul/watch"
1716
"github.com/hashicorp/consul/agent/proxycfg"
1817
"github.com/hashicorp/consul/agent/structs"
@@ -39,14 +38,13 @@ import (
3938
// because proxycfg has a `req.Source.Node != ""` which prevents the `streamingEnabled` check from passing.
4039
// This means that while agents should technically have this same issue, they don't experience it with mesh health
4140
// watches.
42-
func ServerHealthBlocking(deps ServerDataSourceDeps, remoteSource proxycfg.Health, state *state.Store) *serverHealthBlocking {
43-
return &serverHealthBlocking{deps, remoteSource, state, 5 * time.Minute}
41+
func ServerHealthBlocking(deps ServerDataSourceDeps, remoteSource proxycfg.Health) *serverHealthBlocking {
42+
return &serverHealthBlocking{deps, remoteSource, 5 * time.Minute}
4443
}
4544

4645
type serverHealthBlocking struct {
4746
deps ServerDataSourceDeps
4847
remoteSource proxycfg.Health
49-
state *state.Store
5048
watchTimeout time.Duration
5149
}
5250

@@ -66,7 +64,7 @@ func (h *serverHealthBlocking) Notify(ctx context.Context, args *structs.Service
6664
}
6765

6866
// Determine the function we'll call
69-
var f func(memdb.WatchSet, *state.Store, *structs.ServiceSpecificRequest) (uint64, structs.CheckServiceNodes, error)
67+
var f func(memdb.WatchSet, Store, *structs.ServiceSpecificRequest) (uint64, structs.CheckServiceNodes, error)
7068
switch {
7169
case args.Connect:
7270
f = serviceNodesConnect
@@ -108,14 +106,20 @@ func (h *serverHealthBlocking) Notify(ctx context.Context, args *structs.Service
108106
// their data, rather than holding onto the last-known list of healthy nodes indefinitely.
109107
if hadResults {
110108
hadResults = false
109+
h.deps.Logger.Debug("serverHealthBlocking emitting zero check-service-nodes due to insufficient ACL privileges",
110+
"serviceName", structs.NewServiceName(args.ServiceName, &args.EnterpriseMeta),
111+
"correlationID", correlationID,
112+
"connect", args.Connect,
113+
"ingress", args.Ingress,
114+
)
111115
return 0, &structs.IndexedCheckServiceNodes{}, watch.ErrorACLResetData
112116
}
113117
return 0, nil, acl.ErrPermissionDenied
114118
}
115119
}
116120

117121
var thisReply structs.IndexedCheckServiceNodes
118-
thisReply.Index, thisReply.Nodes, err = f(ws, h.state, args)
122+
thisReply.Index, thisReply.Nodes, err = f(ws, store, args)
119123
if err != nil {
120124
return 0, nil, err
121125
}
@@ -134,6 +138,13 @@ func (h *serverHealthBlocking) Notify(ctx context.Context, args *structs.Service
134138
}
135139

136140
hadResults = true
141+
h.deps.Logger.Trace("serverHealthBlocking emitting check-service-nodes",
142+
"serviceName", structs.NewServiceName(args.ServiceName, &args.EnterpriseMeta),
143+
"correlationID", correlationID,
144+
"connect", args.Connect,
145+
"ingress", args.Ingress,
146+
"nodes", len(thisReply.Nodes),
147+
)
137148
return thisReply.Index, &thisReply, nil
138149
},
139150
dispatchBlockingQueryUpdate[*structs.IndexedCheckServiceNodes](ch),
@@ -151,14 +162,14 @@ func (h *serverHealthBlocking) filterACL(authz *acl.AuthorizerContext, token str
151162
return nil
152163
}
153164

154-
func serviceNodesConnect(ws memdb.WatchSet, s *state.Store, args *structs.ServiceSpecificRequest) (uint64, structs.CheckServiceNodes, error) {
165+
func serviceNodesConnect(ws memdb.WatchSet, s Store, args *structs.ServiceSpecificRequest) (uint64, structs.CheckServiceNodes, error) {
155166
return s.CheckConnectServiceNodes(ws, args.ServiceName, &args.EnterpriseMeta, args.PeerName)
156167
}
157168

158-
func serviceNodesIngress(ws memdb.WatchSet, s *state.Store, args *structs.ServiceSpecificRequest) (uint64, structs.CheckServiceNodes, error) {
169+
func serviceNodesIngress(ws memdb.WatchSet, s Store, args *structs.ServiceSpecificRequest) (uint64, structs.CheckServiceNodes, error) {
159170
return s.CheckIngressServiceNodes(ws, args.ServiceName, &args.EnterpriseMeta)
160171
}
161172

162-
func serviceNodesDefault(ws memdb.WatchSet, s *state.Store, args *structs.ServiceSpecificRequest) (uint64, structs.CheckServiceNodes, error) {
173+
func serviceNodesDefault(ws memdb.WatchSet, s Store, args *structs.ServiceSpecificRequest) (uint64, structs.CheckServiceNodes, error) {
163174
return s.CheckServiceNodes(ws, args.ServiceName, &args.EnterpriseMeta, args.PeerName)
164175
}

‎agent/proxycfg-glue/health_blocking_test.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ func TestServerHealthBlocking(t *testing.T) {
3030
remoteSource := newMockHealth(t)
3131
remoteSource.On("Notify", ctx, req, correlationID, ch).Return(result)
3232

33-
store := state.NewStateStore(nil)
34-
dataSource := ServerHealthBlocking(ServerDataSourceDeps{Datacenter: "dc1"}, remoteSource, store)
33+
dataSource := ServerHealthBlocking(ServerDataSourceDeps{Datacenter: "dc1"}, remoteSource)
3534
err := dataSource.Notify(ctx, req, correlationID, ch)
3635
require.Equal(t, result, err)
3736
})
@@ -52,7 +51,7 @@ func TestServerHealthBlocking(t *testing.T) {
5251
Datacenter: datacenter,
5352
ACLResolver: aclResolver,
5453
Logger: testutil.Logger(t),
55-
}, nil, store)
54+
}, nil)
5655
dataSource.watchTimeout = 1 * time.Second
5756

5857
// Watch for all events

‎agent/proxycfg/upstreams.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@ func (s *handlerUpstreams) handleUpdateUpstreams(ctx context.Context, u UpdateEv
136136

137137
uid := UpstreamIDFromString(uidString)
138138

139+
s.logger.Debug("upstream-target watch fired",
140+
"correlationID", correlationID,
141+
"nodes", len(resp.Nodes),
142+
)
139143
if _, ok := upstreamsSnapshot.WatchedUpstreamEndpoints[uid]; !ok {
140144
upstreamsSnapshot.WatchedUpstreamEndpoints[uid] = make(map[string]structs.CheckServiceNodes)
141145
}

0 commit comments

Comments
 (0)