Skip to content

Commit

Permalink
Remove the HTTP endpoint for Notary Signer, as it's not used by anyth…
Browse files Browse the repository at this point in the history
…ing.

This removes the maintenance burden of keeping them in sync.

Signed-off-by: Ying Li <[email protected]>
  • Loading branch information
cyli committed Jul 23, 2016
1 parent 75321dd commit a487935
Show file tree
Hide file tree
Showing 11 changed files with 11 additions and 537 deletions.
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
7 changes: 0 additions & 7 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)
}
}

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

0 comments on commit a487935

Please sign in to comment.