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

Fix handling of REST gateway options and add an integration test #493

Merged
merged 2 commits into from
Mar 22, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
74 changes: 74 additions & 0 deletions cmd/spicedb/restgateway_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
//go:build docker
// +build docker

package main

import (
"fmt"
"io/ioutil"
"net/http"
"testing"

"github.com/ory/dockertest/v3"
"github.com/stretchr/testify/require"
)

func TestRESTGateway(t *testing.T) {
require := require.New(t)

tester, err := newTester(t,
&dockertest.RunOptions{
Repository: "authzed/spicedb",
Tag: "latest",
Cmd: []string{"serve", "--log-level", "debug", "--grpc-preshared-key", "somerandomkeyhere", "--http-enabled"},
ExposedPorts: []string{"50051/tcp", "8443/tcp"},
},
"somerandomkeyhere",
)
require.NoError(err)
defer tester.cleanup()

resp, err := http.Get(fmt.Sprintf("http://localhost:%s", tester.httpPort))
require.NoError(err)

body, err := ioutil.ReadAll(resp.Body)
require.NoError(err)
require.Equal(`{"code":5,"message":"Not Found","details":[]}`, string(body))
josephschorr marked this conversation as resolved.
Show resolved Hide resolved

// Attempt to read schema without a valid Auth header.
readUrl := fmt.Sprintf("http://localhost:%s/v1/schema/read", tester.httpPort)
resp, err = http.Post(readUrl, "", nil)
require.NoError(err)

body, err = ioutil.ReadAll(resp.Body)
require.NoError(err)

require.Equal(500, resp.StatusCode)
require.Contains(string(body), "Unauthenticated")

// Attempt to read schema with an invalid Auth header.
req, err := http.NewRequest("POST", readUrl, nil)
req.Header.Add("Authorization", "Bearer notcorrect")

resp, err = http.DefaultClient.Do(req)
require.NoError(err)

body, err = ioutil.ReadAll(resp.Body)
require.NoError(err)

require.Equal(500, resp.StatusCode)
require.Contains(string(body), "invalid preshared key: invalid token")

// Read with the correct token.
req, err = http.NewRequest("POST", readUrl, nil)
req.Header.Add("Authorization", "Bearer somerandomkeyhere")

resp, err = http.DefaultClient.Do(req)
require.NoError(err)

body, err = ioutil.ReadAll(resp.Body)
require.NoError(err)

require.Equal(200, resp.StatusCode)
require.Contains(string(body), "definition user {")
}
13 changes: 10 additions & 3 deletions cmd/spicedb/servetesting_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func TestTestServer(t *testing.T) {
Cmd: []string{"serve-testing", "--log-level", "debug"},
ExposedPorts: []string{"50051/tcp", "50052/tcp"},
},
"",
)
require.NoError(err)
defer tester.cleanup()
Expand Down Expand Up @@ -136,10 +137,11 @@ func TestTestServer(t *testing.T) {
type spicedbHandle struct {
port string
readonlyPort string
httpPort string
cleanup func()
}

func newTester(t *testing.T, containerOpts *dockertest.RunOptions) (*spicedbHandle, error) {
func newTester(t *testing.T, containerOpts *dockertest.RunOptions, token string) (*spicedbHandle, error) {
pool, err := dockertest.NewPool("")
if err != nil {
return nil, fmt.Errorf("Could not connect to docker: %w", err)
Expand All @@ -152,6 +154,7 @@ func newTester(t *testing.T, containerOpts *dockertest.RunOptions) (*spicedbHand

port := resource.GetPort("50051/tcp")
readonlyPort := resource.GetPort("50052/tcp")
httpPort := resource.GetPort("8443/tcp")

cleanup := func() {
// When you're done, kill and remove the container
Expand All @@ -162,7 +165,11 @@ func newTester(t *testing.T, containerOpts *dockertest.RunOptions) (*spicedbHand

// Give the service time to boot.
require.Eventually(t, func() bool {
conn, err := grpc.Dial(fmt.Sprintf("localhost:%s", port), grpc.WithInsecure())
conn, err := grpc.Dial(
fmt.Sprintf("localhost:%s", port),
grpc.WithInsecure(),
grpcutil.WithInsecureBearerToken(token),
)
if err != nil {
return false
}
Expand All @@ -185,5 +192,5 @@ func newTester(t *testing.T, containerOpts *dockertest.RunOptions) (*spicedbHand
return err == nil
}, 1*time.Second, 10*time.Millisecond, "could not start test server")

return &spicedbHandle{port: port, readonlyPort: readonlyPort, cleanup: cleanup}, nil
return &spicedbHandle{port: port, readonlyPort: readonlyPort, httpPort: httpPort, cleanup: cleanup}, nil
}
16 changes: 16 additions & 0 deletions pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,18 @@ func (c *Config) Complete() (RunnableServer, error) {
}

// Configure the gateway to serve HTTP
if len(c.HTTPGatewayUpstreamAddr) == 0 {
c.HTTPGatewayUpstreamAddr = c.GRPCServer.Address
} else {
log.Info().Str("upstream", c.HTTPGatewayUpstreamAddr).Msg("Overriding REST gateway upstream")
}

if len(c.HTTPGatewayUpstreamTLSCertPath) == 0 {
c.HTTPGatewayUpstreamTLSCertPath = c.GRPCServer.TLSCertPath
} else {
log.Info().Str("cert-path", c.HTTPGatewayUpstreamTLSCertPath).Msg("Overriding REST gateway upstream TLS")
}

gatewayHandler, err := gateway.NewHandler(context.TODO(), c.HTTPGatewayUpstreamAddr, c.HTTPGatewayUpstreamTLSCertPath)
if err != nil {
log.Fatal().Err(err).Msg("failed to initialize rest gateway")
Expand All @@ -229,6 +241,10 @@ func (c *Config) Complete() (RunnableServer, error) {
}).Handler(gatewayHandler)
}

if c.HTTPGateway.Enabled {
log.Info().Str("upstream", c.HTTPGatewayUpstreamAddr).Msg("starting REST gateway")
}

Comment on lines +244 to +247
Copy link
Contributor

Choose a reason for hiding this comment

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

this logging is already covered by the httpserver object (and is misleading anyway - it doesn't start running here, it starts running down in Run)

{"level":"warn","addr":":8443","prefix":"http","time":"2022-03-22T13:30:05Z","message":"http server serving plaintext"}

Copy link
Member Author

Choose a reason for hiding this comment

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

But it doesn't show the upstream address, which was the issue in the first place

gatewayServer, err := c.HTTPGateway.Complete(zerolog.InfoLevel, gatewayHandler)
if err != nil {
return nil, fmt.Errorf("failed to initialize rest gateway: %w", err)
Expand Down