Skip to content

Commit

Permalink
chore(kuma-dp) move token validation to server side
Browse files Browse the repository at this point in the history
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
  • Loading branch information
jakubdyszkiewicz committed Oct 23, 2020
1 parent b521023 commit 6d5f397
Show file tree
Hide file tree
Showing 9 changed files with 147 additions and 74 deletions.
16 changes: 5 additions & 11 deletions app/kuma-dp/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,6 @@ func newRunCmd() *cobra.Command {
if err != nil {
return err
}
if catalog.Apis.DataplaneToken.Enabled() {
if cfg.DataplaneRuntime.TokenPath == "" && cfg.DataplaneRuntime.Token == "" {
return errors.New("Kuma CP is configured with Dataplane Token Server therefore the Dataplane Token is required. " +
"Generate token using 'kumactl generate dataplane-token > /path/file' and provide it via --dataplane-token-file=/path/file argument to Kuma DP")
}
if cfg.DataplaneRuntime.TokenPath != "" {
if err := kumadp_config.ValidateTokenPath(cfg.DataplaneRuntime.TokenPath); err != nil {
return err
}
}
}

dp, err := readDataplaneResource(cmd, &cfg)
if err != nil {
Expand Down Expand Up @@ -111,6 +100,11 @@ func newRunCmd() *cobra.Command {
runLog.Info("generated Envoy configuration will be stored in a temporary directory", "dir", tmpDir)
}

if cfg.DataplaneRuntime.TokenPath != "" {
if err := kumadp_config.ValidateTokenPath(cfg.DataplaneRuntime.TokenPath); err != nil {
return err
}
}
if cfg.DataplaneRuntime.Token != "" {
path := filepath.Join(cfg.DataplaneRuntime.ConfigDir, cfg.Dataplane.Name)
if err := writeFile(path, []byte(cfg.DataplaneRuntime.Token), 0600); err != nil {
Expand Down
22 changes: 0 additions & 22 deletions app/kuma-dp/cmd/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,28 +332,6 @@ var _ = Describe("run", func() {
}),
)

It("should fail when dataplane token server is enabled but token is not provided", func() {
// setup
catalogClientFactory = catalogDataplaneTokenServerEnabledFn

// given
cmd := newRootCmd()
cmd.SetArgs([]string{
"run",
"--cp-address", "http://localhost:1234",
"--name", "example",
"--admin-port", fmt.Sprintf("%d", port),
"--binary-path", filepath.Join("testdata", "envoy-mock.sleep.sh"),
})

// when
err := cmd.Execute()

// then
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError("Kuma CP is configured with Dataplane Token Server therefore the Dataplane Token is required. Generate token using 'kumactl generate dataplane-token > /path/file' and provide it via --dataplane-token-file=/path/file argument to Kuma DP"))
})

It("should fail when there are no free ports in the port range chosen for Envoy Admin API", func() {

By("simulating another Envoy instance that already uses this port")
Expand Down
7 changes: 6 additions & 1 deletion pkg/xds/bootstrap/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import (

func RegisterBootstrap(rt core_runtime.Runtime, mux *http.ServeMux) {
bootstrapHandler := BootstrapHandler{
Generator: NewDefaultBootstrapGenerator(rt.ResourceManager(), rt.Config().BootstrapServer.Params, rt.Config().DpServer.TlsCertFile),
Generator: NewDefaultBootstrapGenerator(
rt.ResourceManager(),
rt.Config().BootstrapServer.Params,
rt.Config().DpServer.TlsCertFile,
rt.Config().AdminServer.Apis.DataplaneToken.Enabled,
),
}
log.Info("registering Bootstrap in Dataplane Server")
mux.HandleFunc("/bootstrap", bootstrapHandler.Handle)
Expand Down
31 changes: 24 additions & 7 deletions pkg/xds/bootstrap/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,29 @@ type BootstrapGenerator interface {
func NewDefaultBootstrapGenerator(
resManager core_manager.ResourceManager,
config *bootstrap_config.BootstrapParamsConfig,
cacertFile string) BootstrapGenerator {
cacertFile string,
dpAuthEnabled bool,
) BootstrapGenerator {
return &bootstrapGenerator{
resManager: resManager,
config: config,
xdsCertFile: cacertFile,
resManager: resManager,
config: config,
xdsCertFile: cacertFile,
dpAuthEnabled: dpAuthEnabled,
}
}

type bootstrapGenerator struct {
resManager core_manager.ResourceManager
config *bootstrap_config.BootstrapParamsConfig
xdsCertFile string
resManager core_manager.ResourceManager
config *bootstrap_config.BootstrapParamsConfig
dpAuthEnabled bool
xdsCertFile string
}

func (b *bootstrapGenerator) Generate(ctx context.Context, request types.BootstrapRequest) (proto.Message, error) {
if err := b.validateRequest(request); err != nil {
return nil, err
}

proxyId, err := core_xds.BuildProxyId(request.Mesh, request.Name)
if err != nil {
return nil, err
Expand All @@ -64,6 +72,15 @@ func (b *bootstrapGenerator) Generate(ctx context.Context, request types.Bootstr
return bootstrapCfg, nil
}

var DpTokenRequired = errors.New("Dataplane Token is required. Generate token using 'kumactl generate dataplane-token > /path/file' and provide it via --dataplane-token-file=/path/file argument to Kuma DP")

func (b *bootstrapGenerator) validateRequest(request types.BootstrapRequest) error {
if b.dpAuthEnabled && request.DataplaneTokenPath == "" {
return DpTokenRequired
}
return nil
}

// dataplaneFor returns dataplane for two flows
// 1) Dataplane is passed to kuma-dp run, in this case we just read DP from the BootstrapRequest
// 2) Dataplane is created before kuma-dp run, in this case we access storage to fetch it (ex. Kubernetes)
Expand Down
11 changes: 8 additions & 3 deletions pkg/xds/bootstrap/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,14 @@ var _ = Describe("bootstrapGenerator", func() {

type testCase struct {
config func() *bootstrap_config.BootstrapParamsConfig
dpAuthEnabled bool
request types.BootstrapRequest
expectedConfigFile string
}
DescribeTable("should generate bootstrap configuration",
func(given testCase) {
// setup
generator := NewDefaultBootstrapGenerator(resManager, given.config(), filepath.Join("..", "..", "..", "test", "certs", "server-cert.pem"))
generator := NewDefaultBootstrapGenerator(resManager, given.config(), filepath.Join("..", "..", "..", "test", "certs", "server-cert.pem"), given.dpAuthEnabled)

// when
bootstrapConfig, err := generator.Generate(context.Background(), given.request)
Expand All @@ -96,6 +97,7 @@ var _ = Describe("bootstrapGenerator", func() {
Expect(actual).To(MatchYAML(expected))
},
Entry("default config with minimal request", testCase{
dpAuthEnabled: false,
config: func() *bootstrap_config.BootstrapParamsConfig {
cfg := bootstrap_config.DefaultBootstrapParamsConfig()
cfg.XdsHost = "127.0.0.1"
Expand All @@ -109,6 +111,7 @@ var _ = Describe("bootstrapGenerator", func() {
expectedConfigFile: "generator.default-config-minimal-request.golden.yaml",
}),
Entry("default config", testCase{
dpAuthEnabled: true,
config: func() *bootstrap_config.BootstrapParamsConfig {
cfg := bootstrap_config.DefaultBootstrapParamsConfig()
cfg.XdsHost = "127.0.0.1"
Expand All @@ -124,6 +127,7 @@ var _ = Describe("bootstrapGenerator", func() {
expectedConfigFile: "generator.default-config.golden.yaml",
}),
Entry("custom config with minimal request", testCase{
dpAuthEnabled: false,
config: func() *bootstrap_config.BootstrapParamsConfig {
return &bootstrap_config.BootstrapParamsConfig{
AdminAddress: "192.168.0.1", // by default, Envoy Admin interface should listen on loopback address
Expand All @@ -141,6 +145,7 @@ var _ = Describe("bootstrapGenerator", func() {
expectedConfigFile: "generator.custom-config-minimal-request.golden.yaml",
}),
Entry("custom config", testCase{
dpAuthEnabled: true,
config: func() *bootstrap_config.BootstrapParamsConfig {
return &bootstrap_config.BootstrapParamsConfig{
AdminAddress: "192.168.0.1", // by default, Envoy Admin interface should listen on loopback address
Expand Down Expand Up @@ -225,7 +230,7 @@ var _ = Describe("bootstrapGenerator", func() {
params.XdsHost = "127.0.0.1"
params.XdsPort = 5678

generator := NewDefaultBootstrapGenerator(resManager, params, "")
generator := NewDefaultBootstrapGenerator(resManager, params, "", false)
request := types.BootstrapRequest{
Mesh: "mesh",
Name: "name-1.namespace",
Expand Down Expand Up @@ -289,7 +294,7 @@ var _ = Describe("bootstrapGenerator", func() {
params.XdsHost = "127.0.0.1"
params.XdsPort = 5678

generator := NewDefaultBootstrapGenerator(resManager, params, "")
generator := NewDefaultBootstrapGenerator(resManager, params, "", false)
request := types.BootstrapRequest{
Mesh: "mesh",
Name: "name-3.namespace",
Expand Down
65 changes: 39 additions & 26 deletions pkg/xds/bootstrap/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"io/ioutil"
"net/http"

"github.com/go-logr/logr"

"github.com/kumahq/kuma/pkg/core"
"github.com/kumahq/kuma/pkg/core/resources/store"
"github.com/kumahq/kuma/pkg/core/validators"
Expand Down Expand Up @@ -32,38 +34,16 @@ func (b *BootstrapHandler) Handle(resp http.ResponseWriter, req *http.Request) {
return
}

logger := log.WithValues("params", reqParams)
config, err := b.Generator.Generate(req.Context(), reqParams)
if err != nil {
if store.IsResourceNotFound(err) {
resp.WriteHeader(http.StatusNotFound)
return
}
if store.IsResourcePreconditionFailed(err) {
resp.WriteHeader(http.StatusUnprocessableEntity)
_, err = resp.Write([]byte(err.Error()))
if err != nil {
log.WithValues("params", reqParams).Error(err, "Error while writing the response")
return
}
return
}
if validators.IsValidationError(err) {
resp.WriteHeader(http.StatusUnprocessableEntity)
_, err = resp.Write([]byte(err.Error()))
if err != nil {
log.WithValues("params", reqParams).Error(err, "Error while writing the response")
return
}
return
}
log.WithValues("params", reqParams).Error(err, "Could not generate a bootstrap configuration")
resp.WriteHeader(http.StatusInternalServerError)
handleError(resp, err, logger)
return
}

bytes, err = proto.ToYAML(config)
if err != nil {
log.WithValues("params", reqParams).Error(err, "Could not convert to json")
logger.Error(err, "Could not convert to json")
resp.WriteHeader(http.StatusInternalServerError)
return
}
Expand All @@ -72,7 +52,40 @@ func (b *BootstrapHandler) Handle(resp http.ResponseWriter, req *http.Request) {
resp.Header().Set("content-type", "text/x-yaml")
_, err = resp.Write(bytes)
if err != nil {
log.WithValues("params", reqParams).Error(err, "Error while writing the response")
logger.Error(err, "Error while writing the response")
return
}
}

func handleError(resp http.ResponseWriter, err error, logger logr.Logger) {
if err == DpTokenRequired {
resp.WriteHeader(http.StatusUnprocessableEntity)
_, err = resp.Write([]byte(err.Error()))
if err != nil {
logger.Error(err, "Error while writing the response")
}
return
}
if store.IsResourceNotFound(err) {
resp.WriteHeader(http.StatusNotFound)
return
}
if store.IsResourcePreconditionFailed(err) {
resp.WriteHeader(http.StatusUnprocessableEntity)
_, err = resp.Write([]byte(err.Error()))
if err != nil {
logger.Error(err, "Error while writing the response")
}
return
}
if validators.IsValidationError(err) {
resp.WriteHeader(http.StatusUnprocessableEntity)
_, err = resp.Write([]byte(err.Error()))
if err != nil {
logger.Error(err, "Error while writing the response")
}
return
}
logger.Error(err, "Could not generate a bootstrap configuration")
resp.WriteHeader(http.StatusInternalServerError)
}
49 changes: 45 additions & 4 deletions pkg/xds/bootstrap/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ var _ = Describe("Bootstrap Server", func() {
dpServer := dp_server.NewDpServer(dpServerCfg, metrics)

bootstrapHandler := bootstrap.BootstrapHandler{
Generator: bootstrap.NewDefaultBootstrapGenerator(resManager, config, ""),
Generator: bootstrap.NewDefaultBootstrapGenerator(resManager, config, "", true),
}
dpServer.HTTPMux().HandleFunc("/bootstrap", bootstrapHandler.Handle)

Expand Down Expand Up @@ -139,12 +139,12 @@ var _ = Describe("Bootstrap Server", func() {
},
Entry("minimal data provided (universal)", testCase{
dataplaneName: "dp-1",
body: `{ "mesh": "default", "name": "dp-1" }`,
body: `{ "mesh": "default", "name": "dp-1", "dataplaneTokenPath": "/tmp/token" }`,
expectedConfigFile: "bootstrap.universal.golden.yaml",
}),
Entry("minimal data provided (k8s)", testCase{
dataplaneName: "dp-1.default",
body: `{ "mesh": "default", "name": "dp-1.default" }`,
body: `{ "mesh": "default", "name": "dp-1.default", "dataplaneTokenPath": "/tmp/token" }`,
expectedConfigFile: "bootstrap.k8s.golden.yaml",
}),
Entry("full data provided", testCase{
Expand All @@ -159,7 +159,8 @@ var _ = Describe("Bootstrap Server", func() {
json := `
{
"mesh": "default",
"name": "dp-1.default"
"name": "dp-1.default",
"dataplaneTokenPath": "/tmp/token"
}
`

Expand All @@ -170,6 +171,46 @@ var _ = Describe("Bootstrap Server", func() {
Expect(resp.StatusCode).To(Equal(404))
})

It("should return 422 for the lack of the dataplane token", func() {
// given
res := mesh.DataplaneResource{
Spec: mesh_proto.Dataplane{
Networking: &mesh_proto.Dataplane_Networking{
Address: "8.8.8.8",
Inbound: []*mesh_proto.Dataplane_Networking_Inbound{
{
Port: 443,
ServicePort: 8443,
Tags: map[string]string{
"kuma.io/service": "backend",
},
},
},
},
},
}
err := resManager.Create(context.Background(), &res, store.CreateByKey("dp-1", "default"))
Expect(err).ToNot(HaveOccurred())

// when
json := `
{
"mesh": "default",
"name": "dp-1"
}
`

resp, err := httpClient.Post(baseUrl+"/bootstrap", "application/json", strings.NewReader(json))
// then
Expect(err).ToNot(HaveOccurred())
bytes, err := ioutil.ReadAll(resp.Body)
Expect(err).ToNot(HaveOccurred())
Expect(resp.Body.Close()).To(Succeed())
Expect(resp.StatusCode).To(Equal(422))
Expect(string(bytes)).To(Equal("Dataplane Token is required. Generate token using 'kumactl generate dataplane-token > /path/file' and provide it via --dataplane-token-file=/path/file argument to Kuma DP"))

})

It("should publish metrics", func() {
// given
res := mesh.DataplaneResource{
Expand Down
10 changes: 10 additions & 0 deletions pkg/xds/bootstrap/testdata/bootstrap.k8s.golden.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ dynamicResources:
apiType: GRPC
grpcServices:
- googleGrpc:
callCredentials:
- fromPlugin:
name: envoy.grpc_credentials.file_based_metadata
typedConfig:
'@type': type.googleapis.com/envoy.config.grpc_credential.v2alpha.FileBasedMetadataConfig
secretData:
filename: /tmp/token
credentialsFactoryName: envoy.grpc_credentials.file_based_metadata
statPrefix: ads
targetUri: 127.0.0.1:5678
cdsConfig:
Expand All @@ -12,6 +20,8 @@ dynamicResources:
node:
cluster: backend
id: default.dp-1.default
metadata:
dataplaneTokenPath: /tmp/token
staticResources:
clusters:
- connectTimeout: 1s
Expand Down
Loading

0 comments on commit 6d5f397

Please sign in to comment.