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

Remove the HTTP endpoint for Notary Signer #870

Merged
merged 1 commit into from
Jul 25, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
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
27 changes: 5 additions & 22 deletions cmd/notary-signer/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"errors"
"fmt"
"net"
"net/http"
"os"
"strings"
"time"
Expand Down Expand Up @@ -62,7 +61,7 @@ func parseSignerConfig(configFilePath string) (signer.Config, error) {
utils.SetUpBugsnag(bugsnagConf)

// parse server config
httpAddr, grpcAddr, tlsConfig, err := getAddrAndTLSConfig(config)
grpcAddr, tlsConfig, err := getAddrAndTLSConfig(config)
if err != nil {
return signer.Config{}, err
}
Expand All @@ -74,7 +73,6 @@ func parseSignerConfig(configFilePath string) (signer.Config, error) {
}

return signer.Config{
HTTPAddr: httpAddr,
GRPCAddr: grpcAddr,
TLSConfig: tlsConfig,
CryptoServices: cryptoServices,
Expand Down Expand Up @@ -213,33 +211,18 @@ func setupGRPCServer(grpcAddr string, tlsConfig *tls.Config,
return grpcServer, lis, nil
}

func setupHTTPServer(httpAddr string, tlsConfig *tls.Config,
cryptoServices signer.CryptoServiceIndex) *http.Server {

return &http.Server{
Addr: httpAddr,
Handler: api.Handlers(cryptoServices),
TLSConfig: tlsConfig,
}
}

func getAddrAndTLSConfig(configuration *viper.Viper) (string, string, *tls.Config, error) {
func getAddrAndTLSConfig(configuration *viper.Viper) (string, *tls.Config, error) {
tlsConfig, err := utils.ParseServerTLS(configuration, true)
if err != nil {
return "", "", nil, fmt.Errorf("unable to set up TLS: %s", err.Error())
return "", nil, fmt.Errorf("unable to set up TLS: %s", err.Error())
}

grpcAddr := configuration.GetString("server.grpc_addr")
if grpcAddr == "" {
return "", "", nil, fmt.Errorf("grpc listen address required for server")
}

httpAddr := configuration.GetString("server.http_addr")
if httpAddr == "" {
return "", "", nil, fmt.Errorf("http listen address required for server")
return "", nil, fmt.Errorf("grpc listen address required for server")
}

return httpAddr, grpcAddr, tlsConfig, nil
return grpcAddr, tlsConfig, nil
}

func bootstrap(s interface{}) error {
Expand Down
9 changes: 1 addition & 8 deletions cmd/notary-signer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,11 @@ func main() {
logrus.Fatal(err.Error())
}

httpServer := setupHTTPServer(signerConfig.HTTPAddr, signerConfig.TLSConfig, signerConfig.CryptoServices)

if debug {
log.Println("RPC server listening on", signerConfig.GRPCAddr)
log.Println("HTTP server listening on", signerConfig.HTTPAddr)
}

go grpcServer.Serve(lis)
err = httpServer.ListenAndServeTLS("", "")
if err != nil {
log.Fatal("HTTPS server failed to start:", err)
}
grpcServer.Serve(lis)
}

func usage() {
Expand Down
31 changes: 4 additions & 27 deletions cmd/notary-signer/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,26 +40,24 @@ func configure(jsonConfig string) *viper.Viper {
// error is propagated.
func TestGetAddrAndTLSConfigInvalidTLS(t *testing.T) {
invalids := []string{
`{"server": {"http_addr": ":1234", "grpc_addr": ":2345"}}`,
`{"server": {"grpc_addr": ":2345"}}`,
`{"server": {
"http_addr": ":1234",
"grpc_addr": ":2345",
"tls_cert_file": "nope",
"tls_key_file": "nope"
}}`,
}
for _, configJSON := range invalids {
_, _, _, err := getAddrAndTLSConfig(configure(configJSON))
_, _, err := getAddrAndTLSConfig(configure(configJSON))
require.Error(t, err)
require.Contains(t, err.Error(), "unable to set up TLS")
}
}

// If a GRPC address is not provided, an error is returned.
func TestGetAddrAndTLSConfigNoGRPCAddr(t *testing.T) {
_, _, _, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{
_, _, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{
"server": {
"http_addr": ":1234",
"tls_cert_file": "%s",
"tls_key_file": "%s"
}
Expand All @@ -68,31 +66,16 @@ func TestGetAddrAndTLSConfigNoGRPCAddr(t *testing.T) {
require.Contains(t, err.Error(), "grpc listen address required for server")
}

// If an HTTP address is not provided, an error is returned.
func TestGetAddrAndTLSConfigNoHTTPAddr(t *testing.T) {
_, _, _, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{
"server": {
"grpc_addr": ":1234",
"tls_cert_file": "%s",
"tls_key_file": "%s"
}
}`, Cert, Key)))
require.Error(t, err)
require.Contains(t, err.Error(), "http listen address required for server")
}

// Success parsing a valid TLS config, HTTP address, and GRPC address.
func TestGetAddrAndTLSConfigSuccess(t *testing.T) {
httpAddr, grpcAddr, tlsConf, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{
grpcAddr, tlsConf, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{
"server": {
"http_addr": ":2345",
"grpc_addr": ":1234",
"tls_cert_file": "%s",
"tls_key_file": "%s"
}
}`, Cert, Key)))
require.NoError(t, err)
require.Equal(t, ":2345", httpAddr)
require.Equal(t, ":1234", grpcAddr)
require.NotNil(t, tlsConf)
}
Expand Down Expand Up @@ -241,12 +224,6 @@ func TestSetupCryptoServicesInvalidStore(t *testing.T) {
require.Equal(t, err.Error(), fmt.Sprintf("%s is not an allowed backend, must be one of: %s", "invalid_backend", []string{notary.SQLiteBackend, notary.MemoryBackend, notary.RethinkDBBackend}))
}

func TestSetupHTTPServer(t *testing.T) {
httpServer := setupHTTPServer(":4443", nil, make(signer.CryptoServiceIndex))
require.Equal(t, ":4443", httpServer.Addr)
require.Nil(t, httpServer.TLSConfig)
}

func TestSetupGRPCServerInvalidAddress(t *testing.T) {
_, _, err := setupGRPCServer("nope", nil, make(signer.CryptoServiceIndex))
require.Error(t, err)
Expand Down
22 changes: 2 additions & 20 deletions docs/reference/signer-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ learn more about the configuration section corresponding to that key:

<pre><code class="language-json">{
<a href="#server-section-required">"server"</a>: {
"http_addr": ":4444",
"grpc_addr": ":7899",
"tls_cert_file": "./fixtures/notary-signer.crt",
"tls_key_file": "./fixtures/notary-signer.key",
Expand Down Expand Up @@ -57,7 +56,6 @@ Example:

```json
"server": {
"http_addr": ":4444",
"grpc_addr": ":7899",
"tls_cert_file": "./fixtures/notary-signer.crt",
"tls_key_file": "./fixtures/notary-signer.key",
Expand All @@ -71,22 +69,6 @@ Example:
<th>Required</th>
<th>Description</th>
</tr>
<tr>
<td valign="top"><code>http_addr</code></td>
<td valign="top">yes</td>
<td valign="top">The TCP address (IP and port) to listen for HTTP
traffic on. Examples:
<ul>
<li><code>":4444"</code> means listen on port 4444 on all IPs (and
hence all interfaces, such as those listed when you run
<code>ifconfig</code>)</li>
<li><code>"127.0.0.1:4444"</code> means listen on port 4444 on
localhost only. That means that the server will not be
accessible except locally (via SSH tunnel, or just on a local
terminal)</li>
</ul>
</td>
</tr>
<tr>
<td valign="top"><code>grpc_addr</code></td>
<td valign="top">yes</td>
Expand All @@ -107,14 +89,14 @@ Example:
<td valign="top"><code>tls_key_file</code></td>
<td valign="top">yes</td>
<td valign="top">The path to the private key to use for
HTTPS. The path is relative to the directory of the
GRPC TLS. The path is relative to the directory of the
configuration file.</td>
</tr>
<tr>
<td valign="top"><code>tls_cert_file</code></td>
<td valign="top">yes</td>
<td valign="top">The path to the certificate to use for
HTTPS. The path is relative to the directory of the
GRPC TLS. The path is relative to the directory of the
configuration file.</td>
</tr>
<tr>
Expand Down
1 change: 0 additions & 1 deletion fixtures/signer-config-local.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{
"server": {
"http_addr": ":4444",
"grpc_addr": ":7899",
"tls_cert_file": "./notary-signer.crt",
"tls_key_file": "./notary-signer.key",
Expand Down
1 change: 0 additions & 1 deletion fixtures/signer-config.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{
"server": {
"http_addr": ":4444",
"grpc_addr": ":7899",
"tls_cert_file": "./notary-signer.crt",
"tls_key_file": "./notary-signer.key",
Expand Down
1 change: 0 additions & 1 deletion fixtures/signer-config.rethink.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{
"server": {
"http_addr": ":4444",
"grpc_addr": ":7899",
"tls_cert_file": "./notary-signer.crt",
"tls_key_file": "./notary-signer.key",
Expand Down
2 changes: 0 additions & 2 deletions signer.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ ENV SERVICE_NAME=notary_signer
ENV NOTARY_SIGNER_DEFAULT_ALIAS="timestamp_1"
ENV NOTARY_SIGNER_TIMESTAMP_1="testpassword"

EXPOSE 4444

# Install notary-signer
RUN go install \
-tags pkcs11 \
Expand Down
Loading