Skip to content

Commit 534a9f5

Browse files
rboyerrboyer
committed
making changes to API/RPC structure and adding tests
* removed some unnecessary compiler output * remove configurability of OverprovisioningFactor
1 parent cf26893 commit 534a9f5

19 files changed

+576
-384
lines changed

‎agent/cache-types/discovery_chain_test.go

-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ func TestCompiledDiscoveryChain(t *testing.T) {
1616
typ := &CompiledDiscoveryChain{RPC: rpc}
1717

1818
// just do the default chain
19-
entries := structs.NewDiscoveryChainConfigEntries()
2019
chain := discoverychain.TestCompileConfigEntries(t, "web", "default", "dc1", nil)
2120

2221
// Expect the proper RPC call. This also sets the expected value
@@ -31,7 +30,6 @@ func TestCompiledDiscoveryChain(t *testing.T) {
3130

3231
reply := args.Get(2).(*structs.DiscoveryChainResponse)
3332
reply.Chain = chain
34-
reply.Entries = entries.Flatten()
3533
reply.QueryMeta.Index = 48
3634
resp = reply
3735
})

‎agent/consul/acl_endpoint_test.go

+22-1
Original file line numberDiff line numberDiff line change
@@ -5255,6 +5255,22 @@ func upsertTestToken(codec rpc.ClientCodec, masterToken string, datacenter strin
52555255
return &out, nil
52565256
}
52575257

5258+
func upsertTestTokenWithPolicyRules(codec rpc.ClientCodec, masterToken string, datacenter string, rules string) (*structs.ACLToken, error) {
5259+
policy, err := upsertTestPolicyWithRules(codec, masterToken, datacenter, rules)
5260+
if err != nil {
5261+
return nil, err
5262+
}
5263+
5264+
token, err := upsertTestToken(codec, masterToken, datacenter, func(token *structs.ACLToken) {
5265+
token.Policies = []structs.ACLTokenPolicyLink{{ID: policy.ID}}
5266+
})
5267+
if err != nil {
5268+
return nil, err
5269+
}
5270+
5271+
return token, nil
5272+
}
5273+
52585274
func retrieveTestTokenAccessorForSecret(codec rpc.ClientCodec, masterToken string, datacenter string, id string) (string, error) {
52595275
arg := structs.ACLTokenGetRequest{
52605276
TokenID: "root",
@@ -5312,6 +5328,10 @@ func deleteTestPolicy(codec rpc.ClientCodec, masterToken string, datacenter stri
53125328

53135329
// upsertTestPolicy creates a policy for testing purposes
53145330
func upsertTestPolicy(codec rpc.ClientCodec, masterToken string, datacenter string) (*structs.ACLPolicy, error) {
5331+
return upsertTestPolicyWithRules(codec, masterToken, datacenter, "")
5332+
}
5333+
5334+
func upsertTestPolicyWithRules(codec rpc.ClientCodec, masterToken string, datacenter string, rules string) (*structs.ACLPolicy, error) {
53155335
// Make sure test policies can't collide
53165336
policyUnq, err := uuid.GenerateUUID()
53175337
if err != nil {
@@ -5321,7 +5341,8 @@ func upsertTestPolicy(codec rpc.ClientCodec, masterToken string, datacenter stri
53215341
arg := structs.ACLPolicySetRequest{
53225342
Datacenter: datacenter,
53235343
Policy: structs.ACLPolicy{
5324-
Name: fmt.Sprintf("test-policy-%s", policyUnq),
5344+
Name: fmt.Sprintf("test-policy-%s", policyUnq),
5345+
Rules: rules,
53255346
},
53265347
WriteRequest: structs.WriteRequest{Token: masterToken},
53275348
}

‎agent/consul/discovery_chain_endpoint.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func (c *DiscoveryChain) Get(args *structs.DiscoveryChainRequest, reply *structs
2020
if done, err := c.srv.forward("DiscoveryChain.Get", args, args, reply); done {
2121
return err
2222
}
23-
defer metrics.MeasureSince([]string{"discoverychain", "get"}, time.Now())
23+
defer metrics.MeasureSince([]string{"discovery_chain", "get"}, time.Now())
2424

2525
// Fetch the ACL token, if any.
2626
rule, err := c.srv.ResolveToken(args.Token)
@@ -70,7 +70,6 @@ func (c *DiscoveryChain) Get(args *structs.DiscoveryChainRequest, reply *structs
7070
}
7171

7272
reply.Index = index
73-
reply.Entries = entries.Flatten()
7473
reply.Chain = chain
7574

7675
return nil

‎agent/consul/discovery_chain_endpoint_test.go

+212
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
package consul
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"testing"
7+
"time"
8+
9+
"github.com/hashicorp/consul/acl"
10+
"github.com/hashicorp/consul/agent/structs"
11+
"github.com/hashicorp/consul/testrpc"
12+
msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
func TestDiscoveryChainEndpoint_Get(t *testing.T) {
17+
t.Parallel()
18+
19+
dir1, s1 := testServerWithConfig(t, func(c *Config) {
20+
c.PrimaryDatacenter = "dc1"
21+
c.ACLDatacenter = "dc1"
22+
c.ACLsEnabled = true
23+
c.ACLMasterToken = "root"
24+
c.ACLDefaultPolicy = "deny"
25+
})
26+
defer os.RemoveAll(dir1)
27+
defer s1.Shutdown()
28+
codec := rpcClient(t, s1)
29+
defer codec.Close()
30+
31+
testrpc.WaitForTestAgent(t, s1.RPC, "dc1")
32+
testrpc.WaitForLeader(t, s1.RPC, "dc1")
33+
34+
denyToken, err := upsertTestTokenWithPolicyRules(codec, "root", "dc1", "")
35+
require.NoError(t, err)
36+
37+
allowToken, err := upsertTestTokenWithPolicyRules(codec, "root", "dc1", `service "web" { policy = "read" }`)
38+
require.NoError(t, err)
39+
40+
getChain := func(args *structs.DiscoveryChainRequest) (*structs.DiscoveryChainResponse, error) {
41+
resp := structs.DiscoveryChainResponse{}
42+
err := msgpackrpc.CallWithCodec(codec, "DiscoveryChain.Get", &args, &resp)
43+
if err != nil {
44+
return nil, err
45+
}
46+
// clear fields that we don't care about
47+
resp.QueryMeta = structs.QueryMeta{}
48+
return &resp, nil
49+
}
50+
51+
// ==== compiling the default chain (no config entries)
52+
53+
{ // no token
54+
_, err := getChain(&structs.DiscoveryChainRequest{
55+
Name: "web",
56+
EvaluateInDatacenter: "dc1",
57+
EvaluateInNamespace: "default",
58+
Datacenter: "dc1",
59+
})
60+
if !acl.IsErrPermissionDenied(err) {
61+
t.Fatalf("err: %v", err)
62+
}
63+
}
64+
65+
{ // wrong token
66+
_, err := getChain(&structs.DiscoveryChainRequest{
67+
Name: "web",
68+
EvaluateInDatacenter: "dc1",
69+
EvaluateInNamespace: "default",
70+
Datacenter: "dc1",
71+
QueryOptions: structs.QueryOptions{Token: denyToken.SecretID},
72+
})
73+
if !acl.IsErrPermissionDenied(err) {
74+
t.Fatalf("err: %v", err)
75+
}
76+
}
77+
78+
expectDefaultResponse_DC1_Default := &structs.DiscoveryChainResponse{
79+
Chain: &structs.CompiledDiscoveryChain{
80+
ServiceName: "web",
81+
Namespace: "default",
82+
Datacenter: "dc1",
83+
Protocol: "tcp",
84+
StartNode: "resolver:web,,,dc1",
85+
Nodes: map[string]*structs.DiscoveryGraphNode{
86+
"resolver:web,,,dc1": &structs.DiscoveryGraphNode{
87+
Type: structs.DiscoveryGraphNodeTypeResolver,
88+
Name: "web,,,dc1",
89+
Resolver: &structs.DiscoveryResolver{
90+
Default: true,
91+
ConnectTimeout: 5 * time.Second,
92+
Target: structs.DiscoveryTarget{
93+
Service: "web",
94+
Namespace: "default",
95+
Datacenter: "dc1",
96+
},
97+
},
98+
},
99+
},
100+
Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{
101+
structs.DiscoveryTarget{
102+
Service: "web",
103+
Namespace: "default",
104+
Datacenter: "dc1",
105+
}: {},
106+
},
107+
},
108+
}
109+
110+
// various ways with good token
111+
for _, tc := range []struct {
112+
evalDC string
113+
evalNS string
114+
expect *structs.DiscoveryChainResponse
115+
}{
116+
{
117+
evalDC: "dc1",
118+
evalNS: "default",
119+
expect: expectDefaultResponse_DC1_Default,
120+
},
121+
{
122+
evalDC: "",
123+
evalNS: "default",
124+
expect: expectDefaultResponse_DC1_Default,
125+
},
126+
{
127+
evalDC: "dc1",
128+
evalNS: "",
129+
expect: expectDefaultResponse_DC1_Default,
130+
},
131+
{
132+
evalDC: "",
133+
evalNS: "",
134+
expect: expectDefaultResponse_DC1_Default,
135+
},
136+
} {
137+
tc := tc
138+
name := fmt.Sprintf("dc=%q ns=%q", tc.evalDC, tc.evalNS)
139+
require.True(t, t.Run(name, func(t *testing.T) {
140+
resp, err := getChain(&structs.DiscoveryChainRequest{
141+
Name: "web",
142+
EvaluateInDatacenter: tc.evalDC,
143+
EvaluateInNamespace: tc.evalNS,
144+
Datacenter: "dc1",
145+
QueryOptions: structs.QueryOptions{Token: allowToken.SecretID},
146+
})
147+
require.NoError(t, err)
148+
149+
require.Equal(t, tc.expect, resp)
150+
}))
151+
}
152+
153+
{ // Now create one config entry.
154+
out := false
155+
require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConfigEntry.Apply",
156+
&structs.ConfigEntryRequest{
157+
Datacenter: "dc1",
158+
Entry: &structs.ServiceResolverConfigEntry{
159+
Kind: structs.ServiceResolver,
160+
Name: "web",
161+
ConnectTimeout: 33 * time.Second,
162+
},
163+
WriteRequest: structs.WriteRequest{Token: "root"},
164+
}, &out))
165+
require.True(t, out)
166+
}
167+
168+
// ==== compiling a chain with config entries
169+
170+
{ // good token
171+
resp, err := getChain(&structs.DiscoveryChainRequest{
172+
Name: "web",
173+
EvaluateInDatacenter: "dc1",
174+
EvaluateInNamespace: "default",
175+
Datacenter: "dc1",
176+
QueryOptions: structs.QueryOptions{Token: allowToken.SecretID},
177+
})
178+
require.NoError(t, err)
179+
180+
expect := &structs.DiscoveryChainResponse{
181+
Chain: &structs.CompiledDiscoveryChain{
182+
ServiceName: "web",
183+
Namespace: "default",
184+
Datacenter: "dc1",
185+
Protocol: "tcp",
186+
StartNode: "resolver:web,,,dc1",
187+
Nodes: map[string]*structs.DiscoveryGraphNode{
188+
"resolver:web,,,dc1": &structs.DiscoveryGraphNode{
189+
Type: structs.DiscoveryGraphNodeTypeResolver,
190+
Name: "web,,,dc1",
191+
Resolver: &structs.DiscoveryResolver{
192+
ConnectTimeout: 33 * time.Second,
193+
Target: structs.DiscoveryTarget{
194+
Service: "web",
195+
Namespace: "default",
196+
Datacenter: "dc1",
197+
},
198+
},
199+
},
200+
},
201+
Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{
202+
structs.DiscoveryTarget{
203+
Service: "web",
204+
Namespace: "default",
205+
Datacenter: "dc1",
206+
}: {},
207+
},
208+
},
209+
}
210+
require.Equal(t, expect, resp)
211+
}
212+
}

‎agent/consul/discoverychain/compile.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -755,7 +755,6 @@ RESOLVE_AGAIN:
755755
Type: structs.DiscoveryGraphNodeTypeResolver,
756756
Name: target.Identifier(),
757757
Resolver: &structs.DiscoveryResolver{
758-
Definition: resolver,
759758
Default: resolver.IsDefault(),
760759
Target: target,
761760
ConnectTimeout: connectTimeout,
@@ -837,9 +836,7 @@ RESOLVE_AGAIN:
837836

838837
// If we filtered everything out then no point in having a failover.
839838
if len(failoverTargets) > 0 {
840-
df := &structs.DiscoveryFailover{
841-
Definition: &failover,
842-
}
839+
df := &structs.DiscoveryFailover{}
843840
node.Resolver.Failover = df
844841

845842
// Convert the targets into targets by cheating a bit and

0 commit comments

Comments
 (0)