From 02d8a11ff00cf3a1a85c8997580c2a8b46fd69c0 Mon Sep 17 00:00:00 2001 From: Eric Date: Mon, 28 Mar 2022 13:56:44 -0400 Subject: [PATCH] remove gogo from acl protobufs --- agent/consul/state/acl.go | 10 +++---- agent/consul/state/acl_test.go | 8 ++--- agent/http.go | 36 ++++++++++++++++++++--- agent/structs/protobuf_compat.go | 28 ------------------ agent/structs/structs.go | 18 ------------ api/namespace.go | 46 +++++++++++++++++++++++++++++ proto/pbacl/acl.go | 19 ++++++++++++ proto/pbacl/acl.pb.go | 50 +++++++++++++++++++++++--------- proto/pbacl/acl.proto | 19 ++---------- proto/pbcommon/common.go | 29 ++++++++++++++++++ proto/prototest/testing.go | 14 +++++++++ 11 files changed, 188 insertions(+), 89 deletions(-) create mode 100644 proto/pbacl/acl.go create mode 100644 proto/prototest/testing.go diff --git a/agent/consul/state/acl.go b/agent/consul/state/acl.go index 9b84c6c16d..877037fe26 100644 --- a/agent/consul/state/acl.go +++ b/agent/consul/state/acl.go @@ -126,7 +126,7 @@ func (s *Store) CanBootstrapACLToken() (bool, uint64, error) { // to update the name. Unlike the older functions to operate specifically on role or policy links // this function does not itself handle the case where the id cannot be found. Instead the // getName function should handle that and return an error if necessary -func resolveACLLinks(tx ReadTxn, links []pbacl.ACLLink, getName func(ReadTxn, string) (string, error)) (int, error) { +func resolveACLLinks(tx ReadTxn, links []*pbacl.ACLLink, getName func(ReadTxn, string) (string, error)) (int, error) { var numValid int for linkIndex, link := range links { if link.ID != "" { @@ -152,12 +152,12 @@ func resolveACLLinks(tx ReadTxn, links []pbacl.ACLLink, getName func(ReadTxn, st // associated with the ID of the link. Ideally this will be a no-op if the names are already correct // however if a linked resource was renamed it might be stale. This function will treat the incoming // links with copy-on-write semantics and its output will indicate whether any modifications were made. -func fixupACLLinks(tx ReadTxn, original []pbacl.ACLLink, getName func(ReadTxn, string) (string, error)) ([]pbacl.ACLLink, bool, error) { +func fixupACLLinks(tx ReadTxn, original []*pbacl.ACLLink, getName func(ReadTxn, string) (string, error)) ([]*pbacl.ACLLink, bool, error) { owned := false links := original - cloneLinks := func(l []pbacl.ACLLink, copyNumLinks int) []pbacl.ACLLink { - clone := make([]pbacl.ACLLink, copyNumLinks) + cloneLinks := func(l []*pbacl.ACLLink, copyNumLinks int) []*pbacl.ACLLink { + clone := make([]*pbacl.ACLLink, copyNumLinks) copy(clone, l[:copyNumLinks]) return clone } @@ -183,7 +183,7 @@ func fixupACLLinks(tx ReadTxn, original []pbacl.ACLLink, getName func(ReadTxn, s } // append the corrected link - links = append(links, pbacl.ACLLink{ID: link.ID, Name: name}) + links = append(links, &pbacl.ACLLink{ID: link.ID, Name: name}) } else if owned { links = append(links, link) } diff --git a/agent/consul/state/acl_test.go b/agent/consul/state/acl_test.go index c86527cd1d..358b1dea8c 100644 --- a/agent/consul/state/acl_test.go +++ b/agent/consul/state/acl_test.go @@ -4110,7 +4110,7 @@ func TestStateStore_resolveACLLinks(t *testing.T) { tx := s.db.Txn(false) defer tx.Abort() - links := []pbacl.ACLLink{ + links := []*pbacl.ACLLink{ { Name: "foo", }, @@ -4133,7 +4133,7 @@ func TestStateStore_resolveACLLinks(t *testing.T) { tx := s.db.Txn(false) defer tx.Abort() - links := []pbacl.ACLLink{ + links := []*pbacl.ACLLink{ { ID: "b985e082-25d3-45a9-9dd8-fd1a41b83b0d", }, @@ -4166,7 +4166,7 @@ func TestStateStore_resolveACLLinks(t *testing.T) { tx := s.db.Txn(false) defer tx.Abort() - links := []pbacl.ACLLink{ + links := []*pbacl.ACLLink{ { ID: "b985e082-25d3-45a9-9dd8-fd1a41b83b0d", }, @@ -4186,7 +4186,7 @@ func TestStateStore_resolveACLLinks(t *testing.T) { func TestStateStore_fixupACLLinks(t *testing.T) { t.Parallel() - links := []pbacl.ACLLink{ + links := []*pbacl.ACLLink{ { ID: "40b57f86-97ea-40e4-a99a-c399cc81f4dd", Name: "foo", diff --git a/agent/http.go b/agent/http.go index e039c2c7cf..ba0067b620 100644 --- a/agent/http.go +++ b/agent/http.go @@ -843,7 +843,7 @@ func serveHandlerWithHeaders(h http.Handler, headers map[string]string) http.Han // parseWait is used to parse the ?wait and ?index query params // Returns true on error -func parseWait(resp http.ResponseWriter, req *http.Request, b structs.QueryOptionsCompat) bool { +func parseWait(resp http.ResponseWriter, req *http.Request, b QueryOptionsCompat) bool { query := req.URL.Query() if wait := query.Get("wait"); wait != "" { dur, err := time.ParseDuration(wait) @@ -868,7 +868,7 @@ func parseWait(resp http.ResponseWriter, req *http.Request, b structs.QueryOptio // parseCacheControl parses the CacheControl HTTP header value. So far we only // support maxage directive. -func parseCacheControl(resp http.ResponseWriter, req *http.Request, b structs.QueryOptionsCompat) bool { +func parseCacheControl(resp http.ResponseWriter, req *http.Request, b QueryOptionsCompat) bool { raw := strings.ToLower(req.Header.Get("Cache-Control")) if raw == "" { @@ -926,7 +926,7 @@ func parseCacheControl(resp http.ResponseWriter, req *http.Request, b structs.Qu // parseConsistency is used to parse the ?stale and ?consistent query params. // Returns true on error -func (s *HTTPHandlers) parseConsistency(resp http.ResponseWriter, req *http.Request, b structs.QueryOptionsCompat) bool { +func (s *HTTPHandlers) parseConsistency(resp http.ResponseWriter, req *http.Request, b QueryOptionsCompat) bool { query := req.URL.Query() defaults := true if _, ok := query["stale"]; ok { @@ -1130,7 +1130,7 @@ func parseMetaPair(raw string) (string, string) { // parse is a convenience method for endpoints that need to use both parseWait // and parseDC. -func (s *HTTPHandlers) parse(resp http.ResponseWriter, req *http.Request, dc *string, b structs.QueryOptionsCompat) bool { +func (s *HTTPHandlers) parse(resp http.ResponseWriter, req *http.Request, dc *string, b QueryOptionsCompat) bool { s.parseDC(req, dc) var token string s.parseTokenWithDefault(req, &token) @@ -1190,3 +1190,31 @@ func getPathSuffixUnescaped(path string, prefixToTrim string) (string, error) { return suffixUnescaped, nil } + +func setMetaProtobuf(resp http.ResponseWriter, queryMeta *pbcommon.QueryMeta) { + qm := new(structs.QueryMeta) + pbcommon.QueryMetaToStructs(queryMeta, qm) + setMeta(resp, qm) +} + +type QueryOptionsCompat interface { + GetAllowStale() bool + SetAllowStale(bool) + + GetRequireConsistent() bool + SetRequireConsistent(bool) + + GetUseCache() bool + SetUseCache(bool) + + SetFilter(string) + SetToken(string) + + SetMustRevalidate(bool) + SetMaxAge(time.Duration) + SetMaxStaleDuration(time.Duration) + SetStaleIfError(time.Duration) + + SetMaxQueryTime(time.Duration) + SetMinQueryIndex(uint64) +} diff --git a/agent/structs/protobuf_compat.go b/agent/structs/protobuf_compat.go index 143bd97e3e..860d971c35 100644 --- a/agent/structs/protobuf_compat.go +++ b/agent/structs/protobuf_compat.go @@ -4,34 +4,6 @@ import ( "time" ) -// QueryOptionsCompat is the interface that both the structs.QueryOptions -// and the proto/pbcommongogo.QueryOptions structs need to implement so that they -// can be operated on interchangeably -type QueryOptionsCompat interface { - GetToken() string - SetToken(string) - GetMinQueryIndex() uint64 - SetMinQueryIndex(uint64) - GetMaxQueryTime() (time.Duration, error) - SetMaxQueryTime(time.Duration) - GetAllowStale() bool - SetAllowStale(bool) - GetRequireConsistent() bool - SetRequireConsistent(bool) - GetUseCache() bool - SetUseCache(bool) - GetMaxStaleDuration() (time.Duration, error) - SetMaxStaleDuration(time.Duration) - GetMaxAge() (time.Duration, error) - SetMaxAge(time.Duration) - GetMustRevalidate() bool - SetMustRevalidate(bool) - GetStaleIfError() (time.Duration, error) - SetStaleIfError(time.Duration) - GetFilter() string - SetFilter(string) -} - // GetToken helps implement the QueryOptionsCompat interface // Copied from proto/pbcommongogo/common.pb.go func (m *QueryOptions) GetToken() string { diff --git a/agent/structs/structs.go b/agent/structs/structs.go index ca4a7c8490..96af7c4719 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -2575,10 +2575,6 @@ type ProtoMarshaller interface { } func EncodeProtoInterface(t MessageType, message interface{}) ([]byte, error) { - if marshaller, ok := message.(ProtoMarshaller); ok { - return EncodeProtoGogo(t, marshaller) - } - if marshaller, ok := message.(proto.Message); ok { return EncodeProto(t, marshaller) } @@ -2586,15 +2582,6 @@ func EncodeProtoInterface(t MessageType, message interface{}) ([]byte, error) { return nil, fmt.Errorf("message does not implement the ProtoMarshaller interface: %T", message) } -func EncodeProtoGogo(t MessageType, message ProtoMarshaller) ([]byte, error) { - data := make([]byte, message.Size()+1) - data[0] = uint8(t) - if _, err := message.MarshalTo(data[1:]); err != nil { - return nil, err - } - return data, nil -} - func EncodeProto(t MessageType, pb proto.Message) ([]byte, error) { data := make([]byte, proto.Size(pb)+1) data[0] = uint8(t) @@ -2612,11 +2599,6 @@ func DecodeProto(buf []byte, pb proto.Message) error { return proto.Unmarshal(buf, pb) } -func DecodeProtoGogo(buf []byte, out ProtoMarshaller) error { - // Note that this assumes the leading byte indicating the type as already been stripped off. - return out.Unmarshal(buf) -} - // CompoundResponse is an interface for gathering multiple responses. It is // used in cross-datacenter RPC calls where more than 1 datacenter is // expected to reply. diff --git a/api/namespace.go b/api/namespace.go index bfc5aff17d..65cc6f3f3b 100644 --- a/api/namespace.go +++ b/api/namespace.go @@ -1,6 +1,7 @@ package api import ( + "encoding/json" "fmt" "time" ) @@ -38,6 +39,25 @@ type Namespace struct { ModifyIndex uint64 `json:"ModifyIndex,omitempty"` } +func (n *Namespace) UnmarshalJSON(data []byte) error { + type Alias Namespace + aux := struct { + DeletedAtSnake *time.Time `json:"deleted_at"` + *Alias + }{ + Alias: (*Alias)(n), + } + if err := json.Unmarshal(data, &aux); err != nil { + return err + } + + if n.DeletedAt == nil && aux.DeletedAtSnake != nil { + n.DeletedAt = aux.DeletedAtSnake + } + + return nil +} + // NamespaceACLConfig is the Namespace specific ACL configuration container type NamespaceACLConfig struct { // PolicyDefaults is the list of policies that should be used for the parent authorizer @@ -48,6 +68,32 @@ type NamespaceACLConfig struct { RoleDefaults []ACLLink `json:"RoleDefaults" alias:"role_defaults"` } +func (n *NamespaceACLConfig) UnmarshalJSON(data []byte) error { + type Alias NamespaceACLConfig + aux := struct { + PolicyDefaultsSnake []ACLLink `json:"policy_defaults"` + RoleDefaultsSnake []ACLLink `json:"role_defaults"` + *Alias + }{ + Alias: (*Alias)(n), + } + if err := json.Unmarshal(data, &aux); err != nil { + return err + } + + if n.PolicyDefaults == nil { + for _, pd := range aux.PolicyDefaultsSnake { + n.PolicyDefaults = append(n.PolicyDefaults, pd) + } + } + if n.RoleDefaults == nil { + for _, pd := range aux.RoleDefaultsSnake { + n.RoleDefaults = append(n.RoleDefaults, pd) + } + } + return nil +} + // Namespaces can be used to manage Namespaces in Consul Enterprise.. type Namespaces struct { c *Client diff --git a/proto/pbacl/acl.go b/proto/pbacl/acl.go new file mode 100644 index 0000000000..ec64d59299 --- /dev/null +++ b/proto/pbacl/acl.go @@ -0,0 +1,19 @@ +package pbacl + +import ( + "github.com/hashicorp/consul/api" +) + +func (a *ACLLink) ToAPI() api.ACLLink { + return api.ACLLink{ + ID: a.ID, + Name: a.Name, + } +} + +func ACLLinkFromAPI(a api.ACLLink) *ACLLink { + return &ACLLink{ + ID: a.ID, + Name: a.Name, + } +} diff --git a/proto/pbacl/acl.pb.go b/proto/pbacl/acl.pb.go index 6c52c86159..d91f00ed58 100644 --- a/proto/pbacl/acl.pb.go +++ b/proto/pbacl/acl.pb.go @@ -5,7 +5,6 @@ package pbacl import ( fmt "fmt" - _ "github.com/gogo/protobuf/gogoproto" proto "github.com/golang/protobuf/proto" io "io" math "math" @@ -24,8 +23,12 @@ var _ = math.Inf const _ = proto.ProtoPackageIsVersion3 // please upgrade the proto package type ACLLink struct { - ID string `protobuf:"bytes,1,opt,name=ID,proto3" json:"ID,omitempty"` - Name string `protobuf:"bytes,2,opt,name=Name,proto3" json:"Name,omitempty" hash:"ignore"` + ID string `protobuf:"bytes,1,opt,name=ID,proto3" json:"ID,omitempty"` + // @gotags: hash:ignore-" + Name string `protobuf:"bytes,2,opt,name=Name,proto3" json:"Name,omitempty"` + XXX_NoUnkeyedLiteral struct{} `json:"-"` + XXX_unrecognized []byte `json:"-"` + XXX_sizecache int32 `json:"-"` } func (m *ACLLink) Reset() { *m = ACLLink{} } @@ -61,6 +64,20 @@ func (m *ACLLink) XXX_DiscardUnknown() { var xxx_messageInfo_ACLLink proto.InternalMessageInfo +func (m *ACLLink) GetID() string { + if m != nil { + return m.ID + } + return "" +} + +func (m *ACLLink) GetName() string { + if m != nil { + return m.Name + } + return "" +} + func init() { proto.RegisterType((*ACLLink)(nil), "acl.ACLLink") } @@ -68,19 +85,16 @@ func init() { func init() { proto.RegisterFile("proto/pbacl/acl.proto", fileDescriptor_ad2d2c73a6a0d8b5) } var fileDescriptor_ad2d2c73a6a0d8b5 = []byte{ - // 193 bytes of a gzipped FileDescriptorProto + // 145 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe2, 0x12, 0x2d, 0x28, 0xca, 0x2f, 0xc9, 0xd7, 0x2f, 0x48, 0x4a, 0x4c, 0xce, 0xd1, 0x4f, 0x4c, 0xce, 0xd1, 0x03, 0xf3, 0x85, 0x98, - 0x13, 0x93, 0x73, 0xa4, 0x44, 0xd2, 0xf3, 0xd3, 0xf3, 0x21, 0xf2, 0x20, 0x16, 0x44, 0x4a, 0xc9, - 0x81, 0x8b, 0xdd, 0xd1, 0xd9, 0xc7, 0x27, 0x33, 0x2f, 0x5b, 0x88, 0x8f, 0x8b, 0xc9, 0xd3, 0x45, - 0x82, 0x51, 0x81, 0x51, 0x83, 0x33, 0x88, 0xc9, 0xd3, 0x45, 0x48, 0x95, 0x8b, 0xc5, 0x2f, 0x31, - 0x37, 0x55, 0x82, 0x09, 0x24, 0xe2, 0x24, 0xf8, 0xe9, 0x9e, 0x3c, 0x6f, 0x46, 0x62, 0x71, 0x86, - 0x95, 0x52, 0x66, 0x7a, 0x5e, 0x7e, 0x51, 0xaa, 0x52, 0x10, 0x58, 0xda, 0xc9, 0xf3, 0xc4, 0x43, - 0x39, 0x86, 0x13, 0x8f, 0xe4, 0x18, 0x2f, 0x3c, 0x92, 0x63, 0x7c, 0xf0, 0x48, 0x8e, 0x71, 0xc2, - 0x63, 0x39, 0x86, 0x19, 0x8f, 0xe5, 0x18, 0x2e, 0x3c, 0x96, 0x63, 0xb8, 0xf1, 0x58, 0x8e, 0x21, - 0x4a, 0x3d, 0x3d, 0xb3, 0x24, 0xa3, 0x34, 0x49, 0x2f, 0x39, 0x3f, 0x57, 0x1f, 0x64, 0x42, 0x66, - 0x72, 0x7e, 0x51, 0x81, 0x7e, 0x72, 0x7e, 0x5e, 0x71, 0x69, 0x8e, 0x3e, 0x92, 0x8b, 0x93, 0xd8, - 0xc0, 0x1c, 0x63, 0x40, 0x00, 0x00, 0x00, 0xff, 0xff, 0x29, 0x0f, 0xd4, 0xf1, 0xc7, 0x00, 0x00, + 0x13, 0x93, 0x73, 0x94, 0x74, 0xb9, 0xd8, 0x1d, 0x9d, 0x7d, 0x7c, 0x32, 0xf3, 0xb2, 0x85, 0xf8, + 0xb8, 0x98, 0x3c, 0x5d, 0x24, 0x18, 0x15, 0x18, 0x35, 0x38, 0x83, 0x98, 0x3c, 0x5d, 0x84, 0x84, + 0xb8, 0x58, 0xfc, 0x12, 0x73, 0x53, 0x25, 0x98, 0xc0, 0x22, 0x60, 0xb6, 0x93, 0xe5, 0x89, 0x47, + 0x72, 0x8c, 0x17, 0x1e, 0xc9, 0x31, 0x3e, 0x78, 0x24, 0xc7, 0x38, 0xe3, 0xb1, 0x1c, 0x43, 0x94, + 0x7a, 0x7a, 0x66, 0x49, 0x46, 0x69, 0x92, 0x5e, 0x72, 0x7e, 0xae, 0x7e, 0x46, 0x62, 0x71, 0x46, + 0x66, 0x72, 0x7e, 0x51, 0x81, 0x7e, 0x72, 0x7e, 0x5e, 0x71, 0x69, 0x8e, 0x3e, 0x92, 0xc5, 0x49, + 0x6c, 0x60, 0x8e, 0x31, 0x20, 0x00, 0x00, 0xff, 0xff, 0x1b, 0xe6, 0xfd, 0xff, 0x8e, 0x00, 0x00, 0x00, } @@ -104,6 +118,10 @@ func (m *ACLLink) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l + if m.XXX_unrecognized != nil { + i -= len(m.XXX_unrecognized) + copy(dAtA[i:], m.XXX_unrecognized) + } if len(m.Name) > 0 { i -= len(m.Name) copy(dAtA[i:], m.Name) @@ -146,6 +164,9 @@ func (m *ACLLink) Size() (n int) { if l > 0 { n += 1 + l + sovAcl(uint64(l)) } + if m.XXX_unrecognized != nil { + n += len(m.XXX_unrecognized) + } return n } @@ -260,6 +281,7 @@ func (m *ACLLink) Unmarshal(dAtA []byte) error { if (iNdEx + skippy) > l { return io.ErrUnexpectedEOF } + m.XXX_unrecognized = append(m.XXX_unrecognized, dAtA[iNdEx:iNdEx+skippy]...) iNdEx += skippy } } diff --git a/proto/pbacl/acl.proto b/proto/pbacl/acl.proto index 60cfefb3da..58fa65ae12 100644 --- a/proto/pbacl/acl.proto +++ b/proto/pbacl/acl.proto @@ -4,21 +4,8 @@ package acl; option go_package = "github.com/hashicorp/consul/proto/pbacl"; - -// Go Modules now includes the version in the filepath for packages within GOPATH/pkg/mode -// Therefore unless we want to hardcode a version here like -// github.com/gogo/protobuf@v1.3.0/gogoproto/gogo.proto then the only other choice is to -// have a more relative import and pass the right import path to protoc. I don't like it -// but its necessary. -import "gogoproto/gogo.proto"; - -option (gogoproto.goproto_unkeyed_all) = false; -option (gogoproto.goproto_unrecognized_all) = false; -option (gogoproto.goproto_getters_all) = false; -option (gogoproto.goproto_sizecache_all) = false; - message ACLLink { string ID = 1; - string Name = 2 - [(gogoproto.moretags) = "hash:\"ignore\""]; -} \ No newline at end of file + // @gotags: hash:ignore-" + string Name = 2; +} diff --git a/proto/pbcommon/common.go b/proto/pbcommon/common.go index 713089e48c..f8211604d4 100644 --- a/proto/pbcommon/common.go +++ b/proto/pbcommon/common.go @@ -147,3 +147,32 @@ func (r *ReadRequest) HasTimedOut(start time.Time, rpcHoldTimeout, maxQueryTime, func (td TargetDatacenter) RequestDatacenter() string { return td.Datacenter } + +// SetLastContact is needed to implement the structs.QueryMetaCompat interface +func (q *QueryMeta) SetLastContact(lastContact time.Duration) { + q.LastContact = structs.DurationToProto(lastContact) +} + +// SetKnownLeader is needed to implement the structs.QueryMetaCompat interface +func (q *QueryMeta) SetKnownLeader(knownLeader bool) { + q.KnownLeader = knownLeader +} + +// SetIndex is needed to implement the structs.QueryMetaCompat interface +func (q *QueryMeta) SetIndex(index uint64) { + q.Index = index +} + +// SetConsistencyLevel is needed to implement the structs.QueryMetaCompat interface +func (q *QueryMeta) SetConsistencyLevel(consistencyLevel string) { + q.ConsistencyLevel = consistencyLevel +} + +func (q *QueryMeta) GetBackend() structs.QueryBackend { + return structs.QueryBackend(0) +} + +// SetResultsFilteredByACLs is needed to implement the structs.QueryMetaCompat interface +func (q *QueryMeta) SetResultsFilteredByACLs(v bool) { + q.ResultsFilteredByACLs = v +} diff --git a/proto/prototest/testing.go b/proto/prototest/testing.go new file mode 100644 index 0000000000..5b5468ea6b --- /dev/null +++ b/proto/prototest/testing.go @@ -0,0 +1,14 @@ +package prototest + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func AssertDeepEqual(t *testing.T, x, y interface{}, opts ...cmp.Option) { + t.Helper() + if diff := cmp.Diff(x, y, opts...); diff != "" { + t.Fatalf("assertion failed: values are not equal\n--- expected\n+++ actual\n%v", diff) + } +}