luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

secrets/consul: Use consistent parameter names (#15400) 

* Add "consul_policies" parameter and deprecate "policies" parameter

* Update tests and remove superfluous log statements
This commit is contained in:
Robert 2022-05-19 14:43:54 -05:00 committed by GitHub
parent e6fb16be9c
commit 6425999ff2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 91 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
builtin/logical/consul/backend_test.go
View File

@ -99,7 +99,18 @@ func TestBackend_Renew_Revoke(t *testing.T) {
testBackendRenewRevoke(t, "1.4.4")
})
testBackendRenewRevoke14(t, "")
t.Run("param-policies", func(t *testing.T) {
t.Parallel()
testBackendRenewRevoke14(t, "", "policies")
})
t.Run("param-consul_policies", func(t *testing.T) {
t.Parallel()
testBackendRenewRevoke14(t, "", "consul_policies")
})
t.Run("both-params", func(t *testing.T) {
t.Parallel()
testBackendRenewRevoke14(t, "", "both")
})
})
})
}
@ -163,7 +174,6 @@ func testBackendRenewRevoke(t *testing.T, version string) {
if err := mapstructure.Decode(resp.Data, &d); err != nil {
t.Fatal(err)
}
t.Logf("Generated token: %s", d.Token)
// Build a client and verify that the credentials work
consulapiConfig := consulapi.DefaultConfig()
@ -174,7 +184,6 @@ func testBackendRenewRevoke(t *testing.T, version string) {
t.Fatal(err)
}
t.Logf("Verifying that the generated token works...")
_, err = client.KV().Put(&consulapi.KVPair{
Key: "foo",
Value: []byte("bar"),
@ -199,7 +208,6 @@ func testBackendRenewRevoke(t *testing.T, version string) {
t.Fatal(err)
}
t.Logf("Verifying that the generated token does not work...")
_, err = client.KV().Put(&consulapi.KVPair{
Key: "foo",
Value: []byte("bar"),
@ -209,7 +217,7 @@ func testBackendRenewRevoke(t *testing.T, version string) {
}
}
func testBackendRenewRevoke14(t *testing.T, version string) {
func testBackendRenewRevoke14(t *testing.T, version string, policiesParam string) {
config := logical.TestBackendConfig()
config.StorageView = &logical.InmemStorage{}
b, err := Factory(context.Background(), config)
@ -238,14 +246,36 @@ func testBackendRenewRevoke14(t *testing.T, version string) {
req.Path = "roles/test"
req.Data = map[string]interface{}{
"policies": []string{"test"},
"lease": "6h",
}
if policiesParam == "both" {
req.Data["policies"] = []string{"wrong-name"}
req.Data["consul_policies"] = []string{"test"}
} else {
req.Data[policiesParam] = []string{"test"}
}
_, err = b.HandleRequest(context.Background(), req)
if err != nil {
t.Fatal(err)
}
read := &logical.Request{
Storage: config.StorageView,
Operation: logical.ReadOperation,
Path: "roles/test",
Data: connData,
}
roleResp, err := b.HandleRequest(context.Background(), read)
expectExtract := roleResp.Data["consul_policies"]
respExtract := roleResp.Data[policiesParam]
if respExtract != nil {
if expectExtract.([]string)[0] != respExtract.([]string)[0] {
t.Errorf("mismatch: response consul_policies '%s' does not match '[test]'", roleResp.Data["consul_policies"])
}
}
req.Operation = logical.ReadOperation
req.Path = "creds/test"
resp, err := b.HandleRequest(context.Background(), req)
@ -269,7 +299,6 @@ func testBackendRenewRevoke14(t *testing.T, version string) {
if err := mapstructure.Decode(resp.Data, &d); err != nil {
t.Fatal(err)
}
t.Logf("Generated token: %s with accessor %s", d.Token, d.Accessor)
// Build a client and verify that the credentials work
consulapiConfig := consulapi.DefaultNonPooledConfig()
@ -280,7 +309,6 @@ func testBackendRenewRevoke14(t *testing.T, version string) {
t.Fatal(err)
}
t.Log("Verifying that the generated token works...")
_, err = client.Catalog(), nil
if err != nil {
t.Fatal(err)
@ -314,7 +342,6 @@ func testBackendRenewRevoke14(t *testing.T, version string) {
Datacenter: "DC1",
}
t.Log("Verifying that the generated token does not exist...")
_, _, err = mgmtclient.ACL().TokenRead(d.Accessor, q)
if err == nil {
t.Fatal("err: expected error")
@ -350,7 +377,7 @@ func TestBackend_LocalToken(t *testing.T) {
req.Path = "roles/test"
req.Data = map[string]interface{}{
"policies": []string{"test"},
"consul_policies": []string{"test"},
"ttl": "6h",
"local": false,
}
@ -361,7 +388,7 @@ func TestBackend_LocalToken(t *testing.T) {
req.Path = "roles/test_local"
req.Data = map[string]interface{}{
"policies": []string{"test"},
"consul_policies": []string{"test"},
"ttl": "6h",
"local": true,
}
@ -391,7 +418,6 @@ func TestBackend_LocalToken(t *testing.T) {
if err := mapstructure.Decode(resp.Data, &d); err != nil {
t.Fatal(err)
}
t.Logf("Generated token: %s with accessor %s", d.Token, d.Accessor)
if d.Local {
t.Fatalf("requested global token, got local one")
@ -406,7 +432,6 @@ func TestBackend_LocalToken(t *testing.T) {
t.Fatal(err)
}
t.Log("Verifying that the generated token works...")
_, err = client.Catalog(), nil
if err != nil {
t.Fatal(err)
@ -428,7 +453,6 @@ func TestBackend_LocalToken(t *testing.T) {
if err := mapstructure.Decode(resp.Data, &d); err != nil {
t.Fatal(err)
}
t.Logf("Generated token: %s with accessor %s", d.Token, d.Accessor)
if !d.Local {
t.Fatalf("requested local token, got global one")
@ -443,7 +467,6 @@ func TestBackend_LocalToken(t *testing.T) {
t.Fatal(err)
}
t.Log("Verifying that the generated token works...")
_, err = client.Catalog(), nil
if err != nil {
t.Fatal(err)
@ -771,7 +794,6 @@ func TestBackend_Roles(t *testing.T) {
if err := mapstructure.Decode(resp.Data, &d); err != nil {
t.Fatal(err)
}
t.Logf("Generated consul_roles token: %s with accessor %s", d.Token, d.Accessor)
// Build a client and verify that the credentials work
consulapiConfig := consulapi.DefaultNonPooledConfig()
@ -782,7 +804,6 @@ func TestBackend_Roles(t *testing.T) {
t.Fatal(err)
}
t.Log("Verifying that the generated token works...")
_, err = client.Catalog(), nil
if err != nil {
t.Fatal(err)
@ -816,7 +837,6 @@ func TestBackend_Roles(t *testing.T) {
Datacenter: "DC1",
}
t.Log("Verifying that the generated token does not exist...")
_, _, err = mgmtclient.ACL().TokenRead(d.Accessor, q)
if err == nil {
t.Fatal("err: expected error")
@ -869,7 +889,7 @@ func testBackendEntNamespace(t *testing.T) {
// Create the role in namespace "ns1"
req.Path = "roles/test-ns"
req.Data = map[string]interface{}{
"policies": []string{"ns-test"},
"consul_policies": []string{"ns-test"},
"lease": "6h",
"consul_namespace": "ns1",
}
@ -902,7 +922,6 @@ func testBackendEntNamespace(t *testing.T) {
if err := mapstructure.Decode(resp.Data, &d); err != nil {
t.Fatal(err)
}
t.Logf("Generated namespace '%s' token: %s with accessor %s", d.ConsulNamespace, d.Token, d.Accessor)
if d.ConsulNamespace != "ns1" {
t.Fatalf("Failed to access namespace")
@ -917,7 +936,6 @@ func testBackendEntNamespace(t *testing.T) {
t.Fatal(err)
}
t.Log("Verifying that the generated token works...")
_, err = client.Catalog(), nil
if err != nil {
t.Fatal(err)
@ -952,7 +970,6 @@ func testBackendEntNamespace(t *testing.T) {
Namespace: "ns1",
}
t.Log("Verifying that the generated token does not exist...")
_, _, err = mgmtclient.ACL().TokenRead(d.Accessor, q)
if err == nil {
t.Fatal("err: expected error")
@ -989,7 +1006,7 @@ func testBackendEntPartition(t *testing.T) {
// Create the role in partition "part1"
req.Path = "roles/test-part"
req.Data = map[string]interface{}{
"policies": []string{"part-test"},
"consul_policies": []string{"part-test"},
"lease": "6h",
"partition": "part1",
}
@ -1022,7 +1039,6 @@ func testBackendEntPartition(t *testing.T) {
if err := mapstructure.Decode(resp.Data, &d); err != nil {
t.Fatal(err)
}
t.Logf("Generated partition '%s' token: %s with accessor %s", d.Partition, d.Token, d.Accessor)
if d.Partition != "part1" {
t.Fatalf("Failed to access partition")
@ -1037,7 +1053,6 @@ func testBackendEntPartition(t *testing.T) {
t.Fatal(err)
}
t.Log("Verifying that the generated token works...")
_, err = client.Catalog(), nil
if err != nil {
t.Fatal(err)
@ -1072,7 +1087,6 @@ func testBackendEntPartition(t *testing.T) {
Partition: "test1",
}
t.Log("Verifying that the generated token does not exist...")
_, _, err = mgmtclient.ACL().TokenRead(d.Accessor, q)
if err == nil {
t.Fatal("err: expected error")
@ -1120,7 +1134,7 @@ func TestBackendRenewRevokeRolesAndIdentities(t *testing.T) {
"role and policies": {
"rp",
map[string]interface{}{
"policies": []string{"test"},
"consul_policies": []string{"test"},
"consul_roles": []string{"role-test"},
"lease": "6h",
},
@ -1135,7 +1149,7 @@ func TestBackendRenewRevokeRolesAndIdentities(t *testing.T) {
"service identity and policies": {
"sip",
map[string]interface{}{
"policies": []string{"test"},
"consul_policies": []string{"test"},
"service_identities": "service1",
"lease": "6h",
},
@ -1151,7 +1165,7 @@ func TestBackendRenewRevokeRolesAndIdentities(t *testing.T) {
"service identity and role and policies": {
"sirp",
map[string]interface{}{
"policies": []string{"test"},
"consul_policies": []string{"test"},
"consul_roles": []string{"role-test"},
"service_identities": "service1",
"lease": "6h",
@ -1167,7 +1181,7 @@ func TestBackendRenewRevokeRolesAndIdentities(t *testing.T) {
"node identity and policies": {
"nip",
map[string]interface{}{
"policies": []string{"test"},
"consul_policies": []string{"test"},
"node_identities": []string{"node1:dc1"},
"lease": "6h",
},
@ -1183,8 +1197,8 @@ func TestBackendRenewRevokeRolesAndIdentities(t *testing.T) {
"node identity and role and policies": {
"nirp",
map[string]interface{}{
"consul_policies": []string{"test"},
"consul_roles": []string{"role-test"},
"service_identities": "service1",
"node_identities": []string{"node1:dc1"},
"lease": "6h",
},
@ -1200,7 +1214,7 @@ func TestBackendRenewRevokeRolesAndIdentities(t *testing.T) {
"node identity and service identity and policies": {
"nisip",
map[string]interface{}{
"policies": []string{"test"},
"consul_policies": []string{"test"},
"service_identities": "service1",
"node_identities": []string{"node1:dc1"},
"lease": "6h",
@ -1218,7 +1232,7 @@ func TestBackendRenewRevokeRolesAndIdentities(t *testing.T) {
"node identity and service identity and role and policies": {
"nisirp",
map[string]interface{}{
"policies": []string{"test"},
"consul_policies": []string{"test"},
"consul_roles": []string{"role-test"},
"service_identities": "service1",
"node_identities": []string{"node1:dc1"},
@ -1261,7 +1275,6 @@ func TestBackendRenewRevokeRolesAndIdentities(t *testing.T) {
if err := mapstructure.Decode(resp.Data, &d); err != nil {
t.Fatal(err)
}
t.Logf("Generated token: %s with accessor %s", d.Token, d.Accessor)
// Build a client and verify that the credentials work
consulapiConfig := consulapi.DefaultNonPooledConfig()
@ -1272,7 +1285,6 @@ func TestBackendRenewRevokeRolesAndIdentities(t *testing.T) {
t.Fatal(err)
}
t.Log("Verifying that the generated token works...")
_, err = client.Catalog(), nil
if err != nil {
t.Fatal(err)
@ -1304,7 +1316,6 @@ func TestBackendRenewRevokeRolesAndIdentities(t *testing.T) {
Datacenter: "DC1",
}
t.Log("Verifying that the generated token does not exist...")
_, _, err = mgmtclient.ACL().TokenRead(d.Accessor, q)
if err == nil {
t.Fatal("err: expected error")

41
builtin/logical/consul/path_roles.go
View File

@ -29,16 +29,32 @@ func pathRoles(b *backend) *framework.Path {
Description: "Name of the role.",
},
// The "policy" and "token_type" parameters were deprecated in Consul back in version 1.4.
// They have been removed from Consul as of version 1.11. Consider removing them here in the future.
"policy": {
Type: framework.TypeString,
Description: `Policy document, base64 encoded. Required
for 'client' tokens. Required for Consul pre-1.4.`,
},
"token_type": {
Type: framework.TypeString,
Default: "client",
Description: `Which type of token to create: 'client' or 'management'. If
a 'management' token, the "policy", "policies", and "consul_roles" parameters are not
required. Defaults to 'client'.`,
},
"policies": {
Type: framework.TypeCommaStringSlice,
Description: `List of policies to attach to the token. Either "policies"
or "consul_roles" are required for Consul 1.5 and above, or just "policies" if
Description: `Use "consul_policies" instead.`,
Deprecated: true,
},
"consul_policies": {
Type: framework.TypeCommaStringSlice,
Description: `List of policies to attach to the token. Either "consul_policies"
or "consul_roles" are required for Consul 1.5 and above, or just "consul_policies" if
using Consul 1.4.`,
},
@ -54,14 +70,6 @@ or "consul_roles" are required for Consul 1.5 and above.`,
and instead be local to the current datacenter. Available in Consul 1.4 and above.`,
},
"token_type": {
Type: framework.TypeString,
Default: "client",
Description: `Which type of token to create: 'client' or 'management'. If
a 'management' token, the "policy", "policies", and "consul_roles" parameters are not
required. Defaults to 'client'.`,
},
"ttl": {
Type: framework.TypeDurationSecond,
Description: "TTL for the Consul token created from the role.",
@ -156,7 +164,7 @@ func (b *backend) pathRolesRead(ctx context.Context, req *logical.Request, d *fr
resp.Data["policy"] = base64.StdEncoding.EncodeToString([]byte(roleConfigData.Policy))
}
if len(roleConfigData.Policies) > 0 {
resp.Data["policies"] = roleConfigData.Policies
resp.Data["consul_policies"] = roleConfigData.Policies
}
if len(roleConfigData.ConsulRoles) > 0 {
resp.Data["consul_roles"] = roleConfigData.ConsulRoles
@ -174,6 +182,7 @@ func (b *backend) pathRolesRead(ctx context.Context, req *logical.Request, d *fr
func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
tokenType := d.Get("token_type").(string)
policy := d.Get("policy").(string)
consulPolicies := d.Get("consul_policies").([]string)
policies := d.Get("policies").([]string)
roles := d.Get("consul_roles").([]string)
serviceIdentities := d.Get("service_identities").([]string)
@ -181,8 +190,8 @@ func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *f
switch tokenType {
case "client":
if policy == "" && len(policies) == 0 && len(roles) == 0 &&
len(serviceIdentities) == 0 && len(nodeIdentities) == 0 {
if policy == "" && len(policies) == 0 && len(consulPolicies) == 0 &&
len(roles) == 0 && len(serviceIdentities) == 0 && len(nodeIdentities) == 0 {
return logical.ErrorResponse(
"Use either a policy document, a list of policies or roles, or a set of service or node identities, depending on your Consul version"), nil
}
@ -191,6 +200,10 @@ func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *f
return logical.ErrorResponse("token_type must be \"client\" or \"management\""), nil
}
if len(consulPolicies) == 0 {
consulPolicies = policies
}
policyRaw, err := base64.StdEncoding.DecodeString(policy)
if err != nil {
return logical.ErrorResponse(fmt.Sprintf(
@ -220,7 +233,7 @@ func (b *backend) pathRolesWrite(ctx context.Context, req *logical.Request, d *f
partition := d.Get("partition").(string)
entry, err := logical.StorageEntryJSON("policy/"+name, roleConfig{
Policy: string(policyRaw),
Policies: policies,
Policies: consulPolicies,
ConsulRoles: roles,
ServiceIdentities: serviceIdentities,
NodeIdentities: nodeIdentities,

4
builtin/logical/consul/path_token_test.go
View File

@ -7,7 +7,7 @@ import (
"github.com/hashicorp/consul/api"
)
func Test_parseServiceIdentities(t *testing.T) {
func TestToken_parseServiceIdentities(t *testing.T) {
tests := []struct {
name string
args []string
@ -58,7 +58,7 @@ func Test_parseServiceIdentities(t *testing.T) {
}
}
func Test_parseNodeIdentities(t *testing.T) {
func TestToken_parseNodeIdentities(t *testing.T) {
tests := []struct {
name string
args []string

3
changelog/15400.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:deprecation
secrets/consul: Deprecate parameter "policies" in favor of "consul_policies" for consistency
```