Skip to content

Commit

Permalink
new approach for hiding Node.SecretID in the API, using json tag
Browse files Browse the repository at this point in the history
documented this approach in the contributing guide
refactored the JSON handlers with extensions
modified event stream encoding to use the go-msgpack encoders with the extensions
  • Loading branch information
cgbaker committed Mar 20, 2021
1 parent 53123fb commit 379281c
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 21 deletions.
4 changes: 2 additions & 2 deletions command/agent/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,13 +495,13 @@ func (s *HTTPServer) wrap(handler func(resp http.ResponseWriter, req *http.Reque
if obj != nil {
var buf bytes.Buffer
if prettyPrint {
enc := codec.NewEncoder(&buf, registerExtensions(structs.JsonHandlePretty))
enc := codec.NewEncoder(&buf, structs.JsonHandlePretty)
err = enc.Encode(obj)
if err == nil {
buf.Write([]byte("\n"))
}
} else {
enc := codec.NewEncoder(&buf, registerExtensions(structs.JsonHandle))
enc := codec.NewEncoder(&buf, structs.JsonHandleWithExtensions)
err = enc.Encode(obj)
}
if err != nil {
Expand Down
5 changes: 4 additions & 1 deletion contributing/checklist-jobspec.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@
* Implement and test other logical methods
* [ ] Add conversion between `api/` and `nomad/structs` in `command/agent/job_endpoint.go`
* Add test for conversion
* msgpack [encoding](http://ugorji.net/blog/go-codec-primer#drop-in-replacement-for-encoding-json-json-key-in
-struct-tag-supported) only uses the [`codec` tag](https://github.com/hashicorp/nomad/blob/v1.0.0/nomad/structs/structs.go#L10557-L10558);
the `json` tag is available for customizing API output when encoding `structs` objects
* [ ] Implement diff logic for new structs/fields in `nomad/structs/diff.go`
* Note that fields must be listed in alphabetical order in `FieldDiff` slices in `nomad/structs/diff_test.go`
* Add test for diff of new structs/fields
* [ ] Add change detection for new structs/feilds in `scheduler/util.go/tasksUpdated`
* [ ] Add change detection for new structs/fields in `scheduler/util.go/tasksUpdated`
* Might be covered by `.Equals` but might not be, check.
* Should return true if the task must be replaced as a result of the change.

Expand Down
10 changes: 7 additions & 3 deletions nomad/stream/ndjson.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package stream

import (
"bytes"
"context"
"encoding/json"
"fmt"
"time"

"github.com/hashicorp/go-msgpack/codec"

"github.com/hashicorp/nomad/nomad/structs"
)

Expand Down Expand Up @@ -71,15 +73,17 @@ func (n *JsonStream) Send(v interface{}) error {
return n.ctx.Err()
}

buf, err := json.Marshal(v)
var buf bytes.Buffer
enc := codec.NewEncoder(&buf, structs.JsonHandleWithExtensions)
err := enc.Encode(v)
if err != nil {
return fmt.Errorf("error marshaling json for stream: %w", err)
}

select {
case <-n.ctx.Done():
return fmt.Errorf("error stream is no longer running: %w", err)
case n.outCh <- &structs.EventJson{Data: buf}:
case n.outCh <- &structs.EventJson{Data: buf.Bytes()}:
}

return nil
Expand Down
19 changes: 7 additions & 12 deletions command/agent/json_encoding.go → nomad/structs/json_encoding.go
Original file line number Diff line number Diff line change
@@ -1,37 +1,32 @@
package agent
package structs

import (
"reflect"

"github.com/hashicorp/go-msgpack/codec"

"github.com/hashicorp/nomad/nomad/structs"
)

// Special encoding for structs.Node, to perform the following:
// 1. ensure that Node.SecretID is zeroed out
// 2. provide backwards compatibility for the following fields:
// 1. provide backwards compatibility for the following fields:
// * Node.Drain
type nodesExt struct{}

// ConvertExt converts a structs.Node to an anonymous struct with the extra field, Drain
func (n nodesExt) ConvertExt(v interface{}) interface{} {
node := v.(*structs.Node)
copy := node.Copy()
copy.SecretID = ""
node := v.(*Node)
return struct {
*structs.Node
*Node
Drain bool
}{
Node: copy,
Node: node,
Drain: node.DrainStrategy != nil,
}
}

// UpdateExt is not used
func (n nodesExt) UpdateExt(_ interface{}, _ interface{}) {}

func registerExtensions(h *codec.JsonHandle) *codec.JsonHandle {
h.SetInterfaceExt(reflect.TypeOf(structs.Node{}), 1, nodesExt{})
func RegisterJSONEncodingExtensions(h *codec.JsonHandle) *codec.JsonHandle {
h.SetInterfaceExt(reflect.TypeOf(Node{}), 1, nodesExt{})
return h
}
11 changes: 8 additions & 3 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1811,7 +1811,7 @@ type Node struct {
// SecretID is an ID that is only known by the Node and the set of Servers.
// It is not accessible via the API and is used to authenticate nodes
// conducting privileged activities.
SecretID string
SecretID string `json:"-"`

// Datacenter for this node
Datacenter string
Expand Down Expand Up @@ -10576,13 +10576,18 @@ var MsgpackHandle = func() *codec.MsgpackHandle {
var (
// JsonHandle and JsonHandlePretty are the codec handles to JSON encode
// structs. The pretty handle will add indents for easier human consumption.
// JsonHandleWithExtensions and JsonHandlePretty include extensions for
// encoding structs objects with API-specific fields
JsonHandle = &codec.JsonHandle{
HTMLCharsAsIs: true,
}
JsonHandlePretty = &codec.JsonHandle{
JsonHandleWithExtensions = RegisterJSONEncodingExtensions(&codec.JsonHandle{
HTMLCharsAsIs: true,
})
JsonHandlePretty = RegisterJSONEncodingExtensions(&codec.JsonHandle{
HTMLCharsAsIs: true,
Indent: 4,
}
})
)

// Decode is used to decode a MsgPack encoded object
Expand Down

0 comments on commit 379281c

Please sign in to comment.