Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add flag to allow /operator/keyring requests to only hit local servers #6279

Merged
merged 9 commits into from
Aug 12, 2019
18 changes: 13 additions & 5 deletions agent/consul/internal_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,19 @@ func (m *Internal) KeyringOperation(
}
}

// Only perform WAN keyring querying and RPC forwarding once
if !args.Forwarded && m.srv.serfWAN != nil {
args.Forwarded = true
m.executeKeyringOp(args, reply, true)
return m.srv.globalRPC("Internal.KeyringOperation", args, reply)
// Validate use of local-only
if args.LocalOnly && args.Operation != structs.KeyringList {
// Error aggressively to be clear about LocalOnly behavior
return fmt.Errorf("argument error: LocalOnly can only be used for List operations")
}

if !args.LocalOnly { // LocalOnly should always be false for non-GET requests
mkeeler marked this conversation as resolved.
Show resolved Hide resolved
// Only perform WAN keyring querying and RPC forwarding once
if !args.Forwarded && m.srv.serfWAN != nil {
args.Forwarded = true
m.executeKeyringOp(args, reply, true)
return m.srv.globalRPC("Internal.KeyringOperation", args, reply)
}
}

// Query the LAN keyring of this node's DC
Expand Down
119 changes: 117 additions & 2 deletions agent/consul/internal_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ package consul
import (
"encoding/base64"
"os"
"strings"
"testing"

"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/testrpc"
"github.com/hashicorp/net-rpc-msgpackrpc"
msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"

"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -287,7 +288,7 @@ func TestInternal_KeyringOperation(t *testing.T) {

// 3 responses (one from each DC LAN, one from WAN) in two-node cluster
if len(out2.Responses) != 3 {
t.Fatalf("bad: %#v", out)
t.Fatalf("bad: %#v", out2)
}
wanResp, lanResp = 0, 0
for _, resp := range out2.Responses {
Expand All @@ -302,6 +303,120 @@ func TestInternal_KeyringOperation(t *testing.T) {
}
}

func TestInternal_KeyringOperationList_LocalOnly(t *testing.T) {
t.Parallel()
key1 := "H1dfkSZOVnP/JUnaBfTzXg=="
keyBytes1, err := base64.StdEncoding.DecodeString(key1)
if err != nil {
t.Fatalf("err: %s", err)
}
dir1, s1 := testServerWithConfig(t, func(c *Config) {
c.SerfLANConfig.MemberlistConfig.SecretKey = keyBytes1
c.SerfWANConfig.MemberlistConfig.SecretKey = keyBytes1
})
defer os.RemoveAll(dir1)
defer s1.Shutdown()
codec := rpcClient(t, s1)
defer codec.Close()

testrpc.WaitForLeader(t, s1.RPC, "dc1")

// Start a second agent to test cross-dc queries
dir2, s2 := testServerWithConfig(t, func(c *Config) {
c.SerfLANConfig.MemberlistConfig.SecretKey = keyBytes1
c.SerfWANConfig.MemberlistConfig.SecretKey = keyBytes1
c.Datacenter = "dc2"
})
defer os.RemoveAll(dir2)
defer s2.Shutdown()

// Try to join
joinWAN(t, s2, s1)

// --
// Try request with `LocalOnly` set to true
var out structs.KeyringResponses
req := structs.KeyringRequest{
Operation: structs.KeyringList,
LocalOnly: true,
}
if err := msgpackrpc.CallWithCodec(codec, "Internal.KeyringOperation", &req, &out); err != nil {
t.Fatalf("err: %v", err)
mkeeler marked this conversation as resolved.
Show resolved Hide resolved
}

// 1 response (from this DC LAN)
if len(out.Responses) != 1 {
t.Fatalf("expected num responses to be 1, got %d; out is: %#v", len(out.Responses), out)
}
wanResp, lanResp := 0, 0
for _, resp := range out.Responses {
if resp.WAN {
wanResp++
} else {
lanResp++
}
}
if lanResp != 1 || wanResp != 0 {
t.Fatalf("should have 1 lan and 0 wan response, got (lan=%d) (wan=%d)", lanResp, wanResp)
}

// --
// Try same request again but with `LocalOnly` set to false
req.LocalOnly = false
if err := msgpackrpc.CallWithCodec(codec, "Internal.KeyringOperation", &req, &out); err != nil {
t.Fatalf("err: %v", err)
}

// 3 responses (one from each DC LAN, one from WAN)
if len(out.Responses) != 3 {
t.Fatalf("expected num responses to be 3, got %d; out is: %#v", len(out.Responses), out)
}
wanResp, lanResp = 0, 0
for _, resp := range out.Responses {
if resp.WAN {
wanResp++
} else {
lanResp++
}
}
if lanResp != 2 || wanResp != 1 {
t.Fatalf("should have 2 lan and 1 wan response, got (lan=%d) (wan=%d)", lanResp, wanResp)
}
}

func TestInternal_KeyringOperationWrite_LocalOnly(t *testing.T) {
t.Parallel()
key1 := "H1dfkSZOVnP/JUnaBfTzXg=="
keyBytes1, err := base64.StdEncoding.DecodeString(key1)
if err != nil {
t.Fatalf("err: %s", err)
}
dir1, s1 := testServerWithConfig(t, func(c *Config) {
c.SerfLANConfig.MemberlistConfig.SecretKey = keyBytes1
c.SerfWANConfig.MemberlistConfig.SecretKey = keyBytes1
})
defer os.RemoveAll(dir1)
defer s1.Shutdown()
codec := rpcClient(t, s1)
defer codec.Close()

testrpc.WaitForLeader(t, s1.RPC, "dc1")

// Try request with `LocalOnly` set to true
var out structs.KeyringResponses
req := structs.KeyringRequest{
Operation: structs.KeyringRemove,
LocalOnly: true,
}
err = msgpackrpc.CallWithCodec(codec, "Internal.KeyringOperation", &req, &out)
if err == nil {
t.Fatalf("expected an error")
}
if !strings.Contains(err.Error(), "LocalOnly") {
t.Fatalf("expected error to contain string 'LocalOnly'. Got: %v", err)
}
}

func TestInternal_NodeInfo_FilterACL(t *testing.T) {
t.Parallel()
dir, token, srv, codec := testACLFilterServer(t)
Expand Down
12 changes: 12 additions & 0 deletions agent/operator_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ type keyringArgs struct {
Key string
Token string
RelayFactor uint8
LocalOnly bool // ?local-only; only used for GET requests
}

// OperatorKeyringEndpoint handles keyring operations (install, list, use, remove)
Expand Down Expand Up @@ -104,6 +105,17 @@ func (s *HTTPServer) OperatorKeyringEndpoint(resp http.ResponseWriter, req *http
}
}

// Parse local-only. local-only will only be used in GET requests.
if localOnly := req.URL.Query().Get("local-only"); localOnly != "" {
var err error
args.LocalOnly, err = strconv.ParseBool(localOnly)
if err != nil {
resp.WriteHeader(http.StatusBadRequest)
s-mang marked this conversation as resolved.
Show resolved Hide resolved
fmt.Fprintf(resp, "Error parsing local-only: %v", err)
return nil, nil
}
}

// Switch on the method
switch req.Method {
case "GET":
Expand Down
1 change: 1 addition & 0 deletions agent/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,7 @@ type KeyringRequest struct {
Datacenter string
Forwarded bool
RelayFactor uint8
LocalOnly bool
QueryOptions
}

Expand Down
3 changes: 3 additions & 0 deletions website/source/api/operator/keyring.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ The table below shows this endpoint's support for
non-zero value will cause nodes to relay their responses through this many
randomly-chosen other nodes in the cluster. The maximum allowed value is `5`.
This is specified as part of the URL as a query parameter.
- `local-only` `(bool: false)` - Settng `local-only` to true will force the keyring
mkeeler marked this conversation as resolved.
Show resolved Hide resolved
query to only hit local servers (no WAN traffic). This is specified as part of
the URL as a query parameter.

### Sample Request

Expand Down