Skip to content

Commit

Permalink
Refactor the agentpb package
Browse files Browse the repository at this point in the history
First move the whole thing to the top-level proto package name.

Secondly change some things around internally to have sub-packages.
  • Loading branch information
mkeeler committed Jul 22, 2020
1 parent abbe171 commit 6d5e062
Show file tree
Hide file tree
Showing 26 changed files with 592 additions and 253 deletions.
10 changes: 5 additions & 5 deletions agent/auto-config/auto_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import (
"strings"
"time"

"github.com/hashicorp/consul/agent/agentpb"
"github.com/hashicorp/consul/agent/config"
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/logging"
"github.com/hashicorp/consul/proto/autoconf"
"github.com/hashicorp/consul/tlsutil"
"github.com/hashicorp/go-discover"
discoverk8s "github.com/hashicorp/go-discover/provider/k8s"
Expand Down Expand Up @@ -234,7 +234,7 @@ func (ac *AutoConfig) restorePersistedAutoConfig() (bool, error) {
}

// InitialConfiguration will perform a one-time RPC request to the configured servers
// to retrieve various cluster wide configurations. See the agent/agentpb/auto_config.proto
// to retrieve various cluster wide configurations. See the proto/auto-config/auto_config.proto
// file for a complete reference of what configurations can be applied in this manner.
// The returned configuration will be the new configuration with any auto-config settings
// already applied. If AutoConfig is not enabled this method will just parse any
Expand Down Expand Up @@ -391,7 +391,7 @@ func (ac *AutoConfig) resolveHost(hostPort string) []net.TCPAddr {
// recordAutoConfigReply takes an AutoConfig RPC reply records it with the agent
// This will persist the configuration to disk (unless in dev mode running without
// a data dir) and will reload the configuration.
func (ac *AutoConfig) recordAutoConfigReply(reply *agentpb.AutoConfigResponse) error {
func (ac *AutoConfig) recordAutoConfigReply(reply *autoconf.AutoConfigResponse) error {
// overwrite the auto encrypt DNS SANs with the ones specified in the auto_config stanza
if len(ac.config.AutoConfig.DNSSANs) > 0 && reply.Config.AutoEncrypt != nil {
reply.Config.AutoEncrypt.DNSSAN = ac.config.AutoConfig.DNSSANs
Expand Down Expand Up @@ -441,14 +441,14 @@ func (ac *AutoConfig) getInitialConfigurationOnce(ctx context.Context) (bool, er
return false, err
}

request := agentpb.AutoConfigRequest{
request := autoconf.AutoConfigRequest{
Datacenter: ac.config.Datacenter,
Node: ac.config.NodeName,
Segment: ac.config.SegmentName,
JWT: token,
}

var reply agentpb.AutoConfigResponse
var reply autoconf.AutoConfigResponse

servers, err := ac.autoConfigHosts()
if err != nil {
Expand Down
26 changes: 13 additions & 13 deletions agent/auto-config/auto_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import (
"testing"
"time"

"github.com/hashicorp/consul/agent/agentpb"
pbconfig "github.com/hashicorp/consul/agent/agentpb/config"
"github.com/hashicorp/consul/agent/config"
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/proto/autoconf"
pbconfig "github.com/hashicorp/consul/proto/config"
"github.com/hashicorp/consul/sdk/testutil"
"github.com/hashicorp/consul/tlsutil"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -201,7 +201,7 @@ func TestInitialConfiguration_cancelled(t *testing.T) {

directRPC := mockDirectRPC{}

expectedRequest := agentpb.AutoConfigRequest{
expectedRequest := autoconf.AutoConfigRequest{
Datacenter: "dc1",
Node: "autoconf",
JWT: "blarg",
Expand Down Expand Up @@ -271,14 +271,14 @@ func TestInitialConfiguration_success(t *testing.T) {
directRPC := mockDirectRPC{}

populateResponse := func(val interface{}) {
resp, ok := val.(*agentpb.AutoConfigResponse)
resp, ok := val.(*autoconf.AutoConfigResponse)
require.True(t, ok)
resp.Config = &pbconfig.Config{
PrimaryDatacenter: "primary",
}
}

expectedRequest := agentpb.AutoConfigRequest{
expectedRequest := autoconf.AutoConfigRequest{
Datacenter: "dc1",
Node: "autoconf",
JWT: "blarg",
Expand All @@ -291,7 +291,7 @@ func TestInitialConfiguration_success(t *testing.T) {
&net.TCPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 8300},
"AutoConfig.InitialConfiguration",
&expectedRequest,
&agentpb.AutoConfigResponse{}).Return(populateResponse)
&autoconf.AutoConfigResponse{}).Return(populateResponse)

ac, err := New(WithBuilderOpts(builderOpts), WithTLSConfigurator(&tlsutil.Configurator{}), WithDirectRPC(&directRPC))
require.NoError(t, err)
Expand Down Expand Up @@ -323,14 +323,14 @@ func TestInitialConfiguration_retries(t *testing.T) {
directRPC := mockDirectRPC{}

populateResponse := func(val interface{}) {
resp, ok := val.(*agentpb.AutoConfigResponse)
resp, ok := val.(*autoconf.AutoConfigResponse)
require.True(t, ok)
resp.Config = &pbconfig.Config{
PrimaryDatacenter: "primary",
}
}

expectedRequest := agentpb.AutoConfigRequest{
expectedRequest := autoconf.AutoConfigRequest{
Datacenter: "dc1",
Node: "autoconf",
JWT: "blarg",
Expand All @@ -346,39 +346,39 @@ func TestInitialConfiguration_retries(t *testing.T) {
&net.TCPAddr{IP: net.IPv4(198, 18, 0, 1), Port: 8300},
"AutoConfig.InitialConfiguration",
&expectedRequest,
&agentpb.AutoConfigResponse{}).Return(fmt.Errorf("injected failure")).Times(0)
&autoconf.AutoConfigResponse{}).Return(fmt.Errorf("injected failure")).Times(0)
directRPC.On(
"RPC",
"dc1",
"autoconf",
&net.TCPAddr{IP: net.IPv4(198, 18, 0, 2), Port: 8398},
"AutoConfig.InitialConfiguration",
&expectedRequest,
&agentpb.AutoConfigResponse{}).Return(fmt.Errorf("injected failure")).Times(0)
&autoconf.AutoConfigResponse{}).Return(fmt.Errorf("injected failure")).Times(0)
directRPC.On(
"RPC",
"dc1",
"autoconf",
&net.TCPAddr{IP: net.IPv4(198, 18, 0, 3), Port: 8399},
"AutoConfig.InitialConfiguration",
&expectedRequest,
&agentpb.AutoConfigResponse{}).Return(fmt.Errorf("injected failure")).Times(0)
&autoconf.AutoConfigResponse{}).Return(fmt.Errorf("injected failure")).Times(0)
directRPC.On(
"RPC",
"dc1",
"autoconf",
&net.TCPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 1234},
"AutoConfig.InitialConfiguration",
&expectedRequest,
&agentpb.AutoConfigResponse{}).Return(fmt.Errorf("injected failure")).Once()
&autoconf.AutoConfigResponse{}).Return(fmt.Errorf("injected failure")).Once()
directRPC.On(
"RPC",
"dc1",
"autoconf",
&net.TCPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 1234},
"AutoConfig.InitialConfiguration",
&expectedRequest,
&agentpb.AutoConfigResponse{}).Return(populateResponse)
&autoconf.AutoConfigResponse{}).Return(populateResponse)

waiter := lib.NewRetryWaiter(2, 0, 1*time.Millisecond, nil)
ac, err := New(WithBuilderOpts(builderOpts), WithTLSConfigurator(&tlsutil.Configurator{}), WithDirectRPC(&directRPC), WithRetryWaiter(waiter))
Expand Down
6 changes: 3 additions & 3 deletions agent/auto-config/config_translate.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
package autoconf

import (
pbconfig "github.com/hashicorp/consul/agent/agentpb/config"
pbconfig "github.com/hashicorp/consul/proto/config"
)

// translateAgentConfig is meant to take in a agent/agentpb/config.Config type
// translateAgentConfig is meant to take in a proto/config.Config type
// and craft the corresponding agent/config.Config type. The need for this function
// should eventually be removed with the protobuf and normal version converging.
// In the meantime, its not desirable to have the flatter Config struct in protobufs
// as in the long term we want a configuration with more nested groupings.
//
// Why is this function not in the agent/agentpb/config package? The answer, that
// Why is this function not in the proto/config package? The answer, that
// package cannot import the agent/config package without running into import cycles.
//
// If this function is meant to output an agent/config.Config then why does it output
Expand Down
2 changes: 1 addition & 1 deletion agent/auto-config/config_translate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import (
"encoding/json"
"testing"

pbconfig "github.com/hashicorp/consul/agent/agentpb/config"
"github.com/hashicorp/consul/agent/config"
pbconfig "github.com/hashicorp/consul/proto/config"
"github.com/stretchr/testify/require"
)

Expand Down
12 changes: 6 additions & 6 deletions agent/consul/auto_config_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import (
"fmt"

"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/agentpb"
"github.com/hashicorp/consul/agent/agentpb/config"
"github.com/hashicorp/consul/agent/consul/authmethod/ssoauth"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/lib/template"
"github.com/hashicorp/consul/proto/autoconf"
"github.com/hashicorp/consul/proto/config"
"github.com/hashicorp/consul/tlsutil"
bexpr "github.com/hashicorp/go-bexpr"
)
Expand All @@ -23,12 +23,12 @@ type AutoConfigOptions struct {
type AutoConfigAuthorizer interface {
// Authorizes the request and returns a struct containing the various
// options for how to generate the configuration.
Authorize(*agentpb.AutoConfigRequest) (AutoConfigOptions, error)
Authorize(*autoconf.AutoConfigRequest) (AutoConfigOptions, error)
}

type disabledAuthorizer struct{}

func (_ *disabledAuthorizer) Authorize(_ *agentpb.AutoConfigRequest) (AutoConfigOptions, error) {
func (_ *disabledAuthorizer) Authorize(_ *autoconf.AutoConfigRequest) (AutoConfigOptions, error) {
return AutoConfigOptions{}, fmt.Errorf("Auto Config is disabled")
}

Expand All @@ -38,7 +38,7 @@ type jwtAuthorizer struct {
claimAssertions []string
}

func (a *jwtAuthorizer) Authorize(req *agentpb.AutoConfigRequest) (AutoConfigOptions, error) {
func (a *jwtAuthorizer) Authorize(req *autoconf.AutoConfigRequest) (AutoConfigOptions, error) {
// perform basic JWT Authorization
identity, err := a.validator.ValidateLogin(context.Background(), req.JWT)
if err != nil {
Expand Down Expand Up @@ -257,7 +257,7 @@ var (

// AgentAutoConfig will authorize the incoming request and then generate the configuration
// to push down to the client
func (ac *AutoConfig) InitialConfiguration(req *agentpb.AutoConfigRequest, resp *agentpb.AutoConfigResponse) error {
func (ac *AutoConfig) InitialConfiguration(req *autoconf.AutoConfigRequest, resp *autoconf.AutoConfigResponse) error {
// default the datacenter to our datacenter - agents do not have to specify this as they may not
// yet know the datacenter name they are going to be in.
if req.Datacenter == "" {
Expand Down
24 changes: 12 additions & 12 deletions agent/consul/auto_config_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import (
"testing"
"time"

"github.com/hashicorp/consul/agent/agentpb"
"github.com/hashicorp/consul/agent/agentpb/config"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/internal/go-sso/oidcauth/oidcauthtest"
"github.com/hashicorp/consul/proto/autoconf"
"github.com/hashicorp/consul/proto/config"
"github.com/hashicorp/consul/sdk/testutil"
"github.com/hashicorp/consul/tlsutil"
"github.com/hashicorp/memberlist"
Expand Down Expand Up @@ -80,9 +80,9 @@ func signJWTWithStandardClaims(t *testing.T, privKey string, claims interface{})
// require running test servers
func TestAutoConfigInitialConfiguration(t *testing.T) {
type testCase struct {
request agentpb.AutoConfigRequest
expected agentpb.AutoConfigResponse
patchResponse func(t *testing.T, srv *Server, resp *agentpb.AutoConfigResponse)
request autoconf.AutoConfigRequest
expected autoconf.AutoConfigResponse
patchResponse func(t *testing.T, srv *Server, resp *autoconf.AutoConfigResponse)
err string
}

Expand All @@ -107,32 +107,32 @@ func TestAutoConfigInitialConfiguration(t *testing.T) {

cases := map[string]testCase{
"wrong-datacenter": {
request: agentpb.AutoConfigRequest{
request: autoconf.AutoConfigRequest{
Datacenter: "no-such-dc",
},
err: `invalid datacenter "no-such-dc" - agent auto configuration cannot target a remote datacenter`,
},
"unverifiable": {
request: agentpb.AutoConfigRequest{
request: autoconf.AutoConfigRequest{
Node: "test-node",
// this is signed using an incorrect private key
JWT: signJWTWithStandardClaims(t, altpriv, map[string]interface{}{"consul_node_name": "test-node"}),
},
err: "Permission denied: Failed JWT authorization: no known key successfully validated the token signature",
},
"claim-assertion-failed": {
request: agentpb.AutoConfigRequest{
request: autoconf.AutoConfigRequest{
Node: "test-node",
JWT: signJWTWithStandardClaims(t, priv, map[string]interface{}{"wrong_claim": "test-node"}),
},
err: "Permission denied: Failed JWT claim assertion",
},
"good": {
request: agentpb.AutoConfigRequest{
request: autoconf.AutoConfigRequest{
Node: "test-node",
JWT: signJWTWithStandardClaims(t, priv, map[string]interface{}{"consul_node_name": "test-node"}),
},
expected: agentpb.AutoConfigResponse{
expected: autoconf.AutoConfigResponse{
Config: &config.Config{
Datacenter: "dc1",
PrimaryDatacenter: "dc1",
Expand Down Expand Up @@ -167,7 +167,7 @@ func TestAutoConfigInitialConfiguration(t *testing.T) {
},
},
},
patchResponse: func(t *testing.T, srv *Server, resp *agentpb.AutoConfigResponse) {
patchResponse: func(t *testing.T, srv *Server, resp *autoconf.AutoConfigResponse) {
// we are expecting an ACL token but cannot check anything for equality
// so here we check that it was set and overwrite it
require.NotNil(t, resp.Config)
Expand Down Expand Up @@ -250,7 +250,7 @@ func TestAutoConfigInitialConfiguration(t *testing.T) {

for testName, tcase := range cases {
t.Run(testName, func(t *testing.T) {
var reply agentpb.AutoConfigResponse
var reply autoconf.AutoConfigResponse
err := msgpackrpc.CallWithCodec(codec, "AutoConfig.InitialConfiguration", &tcase.request, &reply)
if tcase.err != "" {
testutil.RequireErrorContains(t, err, tcase.err)
Expand Down
12 changes: 6 additions & 6 deletions agent/consul/state/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import (
"fmt"
"time"

"github.com/hashicorp/consul/agent/agentpb"
"github.com/hashicorp/consul/agent/structs"
pbacl "github.com/hashicorp/consul/proto/acl"
memdb "github.com/hashicorp/go-memdb"
)

Expand Down Expand Up @@ -339,7 +339,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 *txn, links []agentpb.ACLLink, getName func(*txn, string) (string, error)) (int, error) {
func resolveACLLinks(tx *txn, links []pbacl.ACLLink, getName func(*txn, string) (string, error)) (int, error) {
var numValid int
for linkIndex, link := range links {
if link.ID != "" {
Expand All @@ -365,12 +365,12 @@ func resolveACLLinks(tx *txn, links []agentpb.ACLLink, getName func(*txn, string
// 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 *txn, original []agentpb.ACLLink, getName func(*txn, string) (string, error)) ([]agentpb.ACLLink, bool, error) {
func fixupACLLinks(tx *txn, original []pbacl.ACLLink, getName func(*txn, string) (string, error)) ([]pbacl.ACLLink, bool, error) {
owned := false
links := original

cloneLinks := func(l []agentpb.ACLLink, copyNumLinks int) []agentpb.ACLLink {
clone := make([]agentpb.ACLLink, copyNumLinks)
cloneLinks := func(l []pbacl.ACLLink, copyNumLinks int) []pbacl.ACLLink {
clone := make([]pbacl.ACLLink, copyNumLinks)
copy(clone, l[:copyNumLinks])
return clone
}
Expand All @@ -396,7 +396,7 @@ func fixupACLLinks(tx *txn, original []agentpb.ACLLink, getName func(*txn, strin
}

// append the corrected link
links = append(links, agentpb.ACLLink{ID: link.ID, Name: name})
links = append(links, pbacl.ACLLink{ID: link.ID, Name: name})
} else if owned {
links = append(links, link)
}
Expand Down
Loading

0 comments on commit 6d5e062

Please sign in to comment.