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 keyring http endpoints #2509

Merged
merged 5 commits into from
Nov 23, 2016
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions api/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,26 @@ type RaftConfiguration struct {
Index uint64
}

// KeyringOpts is used for performing Keyring operations
type KeyringOpts struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd make this an unexported struct keyringRequest since it's not part of your interface for the actual API functions.

Key string `json:",omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need the omitempty for anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember why I added this - it's the only field in the struct and we only ever decode to it, so there's no reason for it to be in there

}

// KeyringResponse is returned when listing the gossip encryption keys
type KeyringResponse struct {
// Whether this response is for a WAN ring
WAN bool

// The datacenter name this request corresponds to
Datacenter string

// A map of the encryption keys to the number of nodes they're installed on
Keys map[string]int

// The total number of nodes in this ring
NumNodes int
}

// RaftGetConfiguration is used to query the current Raft peer set.
func (op *Operator) RaftGetConfiguration(q *QueryOptions) (*RaftConfiguration, error) {
r := op.c.newRequest("GET", "/v1/operator/raft/configuration")
Expand Down Expand Up @@ -79,3 +99,61 @@ func (op *Operator) RaftRemovePeerByAddress(address string, q *WriteOptions) err
resp.Body.Close()
return nil
}

// KeyringInstall is used to install a new gossip encryption key into the cluster
func (op *Operator) KeyringInstall(key string) error {
Copy link
Contributor Author

@kyhavlov kyhavlov Nov 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these functions take an argument for overriding the ACL token used? Some of the other api functions do, but some don't so I wasn't sure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but I'd q *QueryOptions and q *WriteOptionsarguments to these methods and get the token from there. It kind of sucks that there's some extra fields in those that don't matter here, but that's kind of the common signature for these. We could add a helper that errors if there are things that are not supported, but we can just document for now.

r := op.c.newRequest("PUT", "/v1/operator/keyring/install")
r.obj = KeyringOpts{
Key: key,
}
_, resp, err := requireOK(op.c.doRequest(r))
if err != nil {
return err
}
resp.Body.Close()
return nil
}

// KeyringList is used to list the gossip keys installed in the cluster
func (op *Operator) KeyringList() ([]*KeyringResponse, error) {
r := op.c.newRequest("GET", "/v1/operator/keyring/list")
_, resp, err := requireOK(op.c.doRequest(r))
if err != nil {
return nil, err
}
defer resp.Body.Close()

var out []*KeyringResponse
if err := decodeBody(resp, &out); err != nil {
return nil, err
}
return out, nil
}

// KeyringRemove is used to remove a gossip encryption key from the cluster
func (op *Operator) KeyringRemove(key string) error {
r := op.c.newRequest("DELETE", "/v1/operator/keyring/remove")
r.obj = KeyringOpts{
Key: key,
}
_, resp, err := requireOK(op.c.doRequest(r))
if err != nil {
return err
}
resp.Body.Close()
return nil
}

// KeyringUse is used to change the active gossip encryption key
func (op *Operator) KeyringUse(key string) error {
r := op.c.newRequest("PUT", "/v1/operator/keyring/use")
r.obj = KeyringOpts{
Key: key,
}
_, resp, err := requireOK(op.c.doRequest(r))
if err != nil {
return err
}
resp.Body.Close()
return nil
}
68 changes: 68 additions & 0 deletions api/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package api
import (
"strings"
"testing"

"github.com/hashicorp/consul/testutil"
)

func TestOperator_RaftGetConfiguration(t *testing.T) {
Expand Down Expand Up @@ -36,3 +38,69 @@ func TestOperator_RaftRemovePeerByAddress(t *testing.T) {
t.Fatalf("err: %v", err)
}
}

func TestOperator_KeyringInstallListPutRemove(t *testing.T) {
oldKey := "d8wu8CSUrqgtjVsvcBPmhQ=="
newKey := "qxycTi/SsePj/TZzCBmNXw=="
t.Parallel()
c, s := makeClientWithConfig(t, nil, func(c *testutil.TestServerConfig) {
c.Encrypt = oldKey
})
defer s.Stop()

operator := c.Operator()
if err := operator.KeyringInstall(newKey); err != nil {
t.Fatalf("err: %v", err)
}

listResponses, err := operator.KeyringList()
if err != nil {
t.Fatalf("err %v", err)
}

// Make sure the new key is installed
if len(listResponses) != 2 {
t.Fatalf("bad: %v", len(listResponses))
}
for _, response := range listResponses {
if len(response.Keys) != 2 {
t.Fatalf("bad: %v", len(response.Keys))
}
if _, ok := response.Keys[oldKey]; !ok {
t.Fatalf("bad: %v", ok)
}
if _, ok := response.Keys[newKey]; !ok {
t.Fatalf("bad: %v", ok)
}
}

// Switch the primary to the new key
if err := operator.KeyringUse(newKey); err != nil {
t.Fatalf("err: %v", err)
}

if err := operator.KeyringRemove(oldKey); err != nil {
t.Fatalf("err: %v", err)
}

listResponses, err = operator.KeyringList()
if err != nil {
t.Fatalf("err %v", err)
}

// Make sure the old key is removed
if len(listResponses) != 2 {
t.Fatalf("bad: %v", len(listResponses))
}
for _, response := range listResponses {
if len(response.Keys) != 1 {
t.Fatalf("bad: %v", len(response.Keys))
}
if _, ok := response.Keys[oldKey]; ok {
t.Fatalf("bad: %v", ok)
}
if _, ok := response.Keys[newKey]; !ok {
t.Fatalf("bad: %v", ok)
}
}
}
4 changes: 4 additions & 0 deletions command/agent/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,10 @@ func (s *HTTPServer) registerHandlers(enableDebug bool) {
s.handleFuncMetrics("/v1/kv/", s.wrap(s.KVSEndpoint))
s.handleFuncMetrics("/v1/operator/raft/configuration", s.wrap(s.OperatorRaftConfiguration))
s.handleFuncMetrics("/v1/operator/raft/peer", s.wrap(s.OperatorRaftPeer))
s.handleFuncMetrics("/v1/operator/keyring/install", s.wrap(s.OperatorKeyringInstall))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking we can get rid of these /verb suffixes in the URLs and use HTTP verbs for everything:

GET /v1/operator/keyring -> list keys
POST /v1/operator/keyring -> Install a key
PUT /v1/operator/keyring -> use a key
DELETE /v1/operator/keyring -> remove a key

Still not super REST-y since we don't have the key in the path, but looks more REST-y and keeps the API a little smaller.

s.handleFuncMetrics("/v1/operator/keyring/list", s.wrap(s.OperatorKeyringList))
s.handleFuncMetrics("/v1/operator/keyring/remove", s.wrap(s.OperatorKeyringRemove))
s.handleFuncMetrics("/v1/operator/keyring/use", s.wrap(s.OperatorKeyringUse))
s.handleFuncMetrics("/v1/query", s.wrap(s.PreparedQueryGeneral))
s.handleFuncMetrics("/v1/query/", s.wrap(s.PreparedQuerySpecific))
s.handleFuncMetrics("/v1/session/create", s.wrap(s.SessionCreate))
Expand Down
108 changes: 108 additions & 0 deletions command/agent/operator_endpoint.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package agent

import (
"fmt"
"net/http"

"github.com/hashicorp/consul/consul/structs"
Expand Down Expand Up @@ -55,3 +56,110 @@ func (s *HTTPServer) OperatorRaftPeer(resp http.ResponseWriter, req *http.Reques
}
return nil, nil
}

// OperatorKeyringInstall is used to install a new gossip encryption key into the cluster
func (s *HTTPServer) OperatorKeyringInstall(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
if req.Method != "PUT" {
resp.WriteHeader(http.StatusMethodNotAllowed)
return nil, nil
}

var args structs.KeyringRequest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has extra fields that we wouldn't ever want to plumb directly (there's currently a panic() in the internal endpoint handler that we should change to an error if the Op isn't understood), so I'd make a simple unexported struct (define in here is fine) and a local string for the token and use those. There's stuff we don't use in this big struct so it's confusing to accept that here.

if err := decodeBody(req, &args, nil); err != nil {
resp.WriteHeader(400)
resp.Write([]byte(fmt.Sprintf("Request decode failed: %v", err)))
return nil, nil
}
s.parseToken(req, &args.Token)

responses, err := s.agent.InstallKey(args.Key, args.Token)
if err != nil {
return nil, err
}
for _, response := range responses.Responses {
if response.Error != "" {
return nil, fmt.Errorf(response.Error)
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is just returning the first error fine for these KeyringResponses? I thought about using multierror but it didn't seem useful because you'd just get a bunch of duplicate errors ("key must be 16/24/32 length" and the like)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes sense. We could dedup them into a multierror maybe so you can tell if you have a handful of problems vs. having to run several times, since a short list would be super useful.


return nil, nil
}

// OperatorKeyringList is used to list the keys installed in the cluster
func (s *HTTPServer) OperatorKeyringList(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
if req.Method != "GET" {
resp.WriteHeader(http.StatusMethodNotAllowed)
return nil, nil
}

var token string
s.parseToken(req, &token)

responses, err := s.agent.ListKeys(token)
if err != nil {
return nil, err
}
for _, response := range responses.Responses {
if response.Error != "" {
return nil, fmt.Errorf(response.Error)
}
}

return responses.Responses, nil
}

// OperatorKeyringRemove is used to list the keys installed in the cluster
func (s *HTTPServer) OperatorKeyringRemove(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
if req.Method != "DELETE" {
resp.WriteHeader(http.StatusMethodNotAllowed)
return nil, nil
}

var args structs.KeyringRequest
if err := decodeBody(req, &args, nil); err != nil {
resp.WriteHeader(400)
resp.Write([]byte(fmt.Sprintf("Request decode failed: %v", err)))
return nil, nil
}
s.parseToken(req, &args.Token)

responses, err := s.agent.RemoveKey(args.Key, args.Token)
if err != nil {
return nil, err
}
for _, response := range responses.Responses {
if response.Error != "" {
return nil, fmt.Errorf(response.Error)
}
}

return nil, nil
}

// OperatorKeyringUse is used to change the primary gossip encryption key
func (s *HTTPServer) OperatorKeyringUse(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
if req.Method != "PUT" {
resp.WriteHeader(http.StatusMethodNotAllowed)
return nil, nil
}

var args structs.KeyringRequest
if err := decodeBody(req, &args, nil); err != nil {
resp.WriteHeader(400)
resp.Write([]byte(fmt.Sprintf("Request decode failed: %v", err)))
return nil, nil
}
s.parseToken(req, &args.Token)

responses, err := s.agent.UseKey(args.Key, args.Token)
if err != nil {
return nil, err
}
for _, response := range responses.Responses {
if response.Error != "" {
return nil, fmt.Errorf(response.Error)
}
}

return nil, nil
}
Loading