New version integrating comments

core/serror/serror.go

@@ -19,6 +19,14 @@ limitations under the License.
 
 package serror
 
+import (
+	"errors"
+)
+
+var (
+	ErrBadAddress = errors.New("Address for binding can not contain comma")
+)
+
 type SnapError interface {
 	error
 	Fields() map[string]interface{}

examples/configs/snap-config-sample.json

@@ -70,7 +70,7 @@
         "rest_certificate": "/path/to/cert/file",
         "rest_key": "/path/to/private/key",
         "port": 8282,
-        "bind_addresses": "127.0.0.1:12345"
+        "addr": "127.0.0.1:12345"
     },
     "tribe": {
         "enable": true,

examples/configs/snap-config-sample.yaml

@@ -115,8 +115,8 @@ restapi:
   # port sets the port to start the REST API server on. Default is 8181
   port: 8282
 
-  #
-  bind_addresses: 127.0.0.1:12345
+  # REST API in address[:port] format
+  addr: 127.0.0.1:12345
 
 # tribe section contains all configuration items for the tribe module
 tribe:

mgmt/rest/client/client_func_test.go

@@ -91,7 +91,7 @@ func startAPI() string {
 		}
 		log.Fatal(err)
 	}(r.Err())
-	r.SetAddresses("127.0.0.1", 0)
+	r.SetAddress("127.0.0.1", 0)
 	r.Start()
 	time.Sleep(100 * time.Millisecond)
 	return fmt.Sprintf("http://localhost:%d", r.Port())

mgmt/rest/client/client_tribe_func_test.go

@@ -200,7 +200,7 @@ func startTribes(count int) []int {
 		r.BindMetricManager(c)
 		r.BindTaskManager(s)
 		r.BindTribeManager(t)
-		r.SetAddresses("", mgtPort)
+		r.SetAddress("", mgtPort)
 		r.Start()
 		wg.Add(1)
 		timer := time.After(10 * time.Second)

mgmt/rest/rest_func_test.go

@@ -488,7 +488,7 @@ func startAPI(cfg *mockConfig) *restAPIInstance {
 		}
 		log.Fatal(err)
 	}(r.Err())
-	r.SetAddresses("127.0.0.1", 0)
+	r.SetAddress("127.0.0.1", 0)
 	r.Start()
 	time.Sleep(time.Millisecond * 100)
 	return &restAPIInstance{

mgmt/rest/server.go

@@ -52,7 +52,7 @@ const (
 const (
 	defaultEnable          bool   = true
 	defaultPort            int    = 8181
-	defaultBindAddresses   string = ""
+	defaultAddress         string = ""
 	defaultHTTPS           bool   = false
 	defaultRestCertificate string = ""
 	defaultRestKey         string = ""
@@ -74,7 +74,7 @@ var (
 type Config struct {
 	Enable           bool   `json:"enable,omitempty"yaml:"enable,omitempty"`
 	Port             int    `json:"port,omitempty"yaml:"port,omitempty"`
-	BindAddresses    string `json:"bind_addresses,omitempty"yaml:"bind_addresses,omitempty"`
+	Address          string `json:"addr,omitempty"yaml:"addr,omitempty"`
 	HTTPS            bool   `json:"https,omitempty"yaml:"https,omitempty"`
 	RestCertificate  string `json:"rest_certificate,omitempty"yaml:"rest_certificate,omitempty"`
 	RestKey          string `json:"rest_key,omitempty"yaml:"rest_key,omitempty"`
@@ -110,7 +110,7 @@ const (
 						"minimum": 0,
 						"maximum": 65535
 					},
-					"bind_addresses" : {
+					"addr" : {
 						"type": "string"
 					}
 				},
@@ -215,7 +215,7 @@ func GetDefaultConfig() *Config {
 	return &Config{
 		Enable:           defaultEnable,
 		Port:             defaultPort,
-		BindAddresses:    defaultBindAddresses,
+		Address:          defaultAddress,
 		HTTPS:            defaultHTTPS,
 		RestCertificate:  defaultRestCertificate,
 		RestKey:          defaultRestKey,
@@ -246,9 +246,9 @@ func (c *Config) UnmarshalJSON(data []byte) error {
 			if err := json.Unmarshal(v, &(c.Port)); err != nil {
 				return fmt.Errorf("%v (while parsing 'restapi::port')", err)
 			}
-		case "bind_addresses":
-			if err := json.Unmarshal(v, &(c.BindAddresses)); err != nil {
-				return fmt.Errorf("%v (while parsing 'restapi::bind_addresses')", err)
+		case "addr":
+			if err := json.Unmarshal(v, &(c.Address)); err != nil {
+				return fmt.Errorf("%v (while parsing 'restapi::addr')", err)
 			}
 		case "https":
 			if err := json.Unmarshal(v, &(c.HTTPS)); err != nil {
@@ -309,19 +309,20 @@ func (s *Server) Name() string {
 	return "REST"
 }
 
-func (s *Server) SetAddresses(addrString string, dfltPort int) {
+func (s *Server) SetAddress(addrString string, dfltPort int) error {
 	restLogger.Info(fmt.Sprintf("Setting address to: [%v] Default port: %v", addrString, dfltPort))
 	// In the future, we could extend this to support multiple comma separated IP[:port] values
 	if strings.Index(addrString, ",") != -1 {
-		panic("Not supported address value for binding")
+		return serror.ErrBadAddress
 	}
 	// If no port is specified, use default port
 	if strings.Index(addrString, ":") != -1 {
 		s.addrString = addrString
 	} else {
 		s.addrString = fmt.Sprintf("%s:%d", addrString, dfltPort)
 	}
-	restLogger.Info(fmt.Sprintf("Address used for binding: [%v]", addrString))
+	restLogger.Info(fmt.Sprintf("Address used for binding: [%v]", s.addrString))
+	return nil
 }
 
 func (s *Server) Start() error {

mgmt/rest/server_test.go

@@ -53,8 +53,8 @@ func TestRestAPIConfigJSON(t *testing.T) {
 		Convey("Port should be 8282", func() {
 			So(cfg.Port, ShouldEqual, 8282)
 		})
-		Convey("BindAddresses should equal 127.0.0.1:12345", func() {
-			So(cfg.BindAddresses, ShouldEqual, "127.0.0.1:12345")
+		Convey("Address should equal 127.0.0.1:12345", func() {
+			So(cfg.Address, ShouldEqual, "127.0.0.1:12345")
 		})
 		Convey("RestAuth should be true", func() {
 			So(cfg.RestAuth, ShouldEqual, true)
@@ -95,8 +95,8 @@ func TestRestAPIConfigYaml(t *testing.T) {
 		Convey("Port should be 8282", func() {
 			So(cfg.Port, ShouldEqual, 8282)
 		})
-		Convey("BindAddresses should equal 127.0.0.1:12345", func() {
-			So(cfg.BindAddresses, ShouldEqual, "127.0.0.1:12345")
+		Convey("Address should equal 127.0.0.1:12345", func() {
+			So(cfg.Address, ShouldEqual, "127.0.0.1:12345")
 		})
 		Convey("RestAuth should be true", func() {
 			So(cfg.RestAuth, ShouldEqual, true)
@@ -126,8 +126,8 @@ func TestRestAPIDefaultConfig(t *testing.T) {
 		Convey("Port should be 8181", func() {
 			So(cfg.Port, ShouldEqual, 8181)
 		})
-		Convey("BindAddresses should equal empty string", func() {
-			So(cfg.BindAddresses, ShouldEqual, "")
+		Convey("Address should equal empty string", func() {
+			So(cfg.Address, ShouldEqual, "")
 		})
 		Convey("RestAuth should be false", func() {
 			So(cfg.RestAuth, ShouldEqual, false)

mgmt/rest/tribe_test.go

@@ -700,7 +700,7 @@ func startTribes(count int, seed string) ([]int, int) {
 		r.BindMetricManager(c)
 		r.BindTaskManager(s)
 		r.BindTribeManager(t)
-		r.SetAddresses("", mgtPort)
+		r.SetAddress("", mgtPort)
 		r.Start()
 		wg.Add(1)
 		timer := time.After(10 * time.Second)

snapd.go

@@ -38,6 +38,7 @@ import (
 	"github.com/codegangsta/cli"
 	"github.com/vrischmann/jsonutil"
 
+	"errors"
 	"github.com/intelsdi-x/snap/control"
 	"github.com/intelsdi-x/snap/core"
 	"github.com/intelsdi-x/snap/core/serror"
@@ -54,14 +55,14 @@ var (
 		Usage: "Disable the agent REST API",
 	}
 	flAPIPort = cli.IntFlag{
-		Name:  "api-port, p",
-		Usage: "API port (Default: 8181)",
+		Name:   "api-port, p",
+		Usage:  "API port (Default: 8181)",
 		EnvVar: "SNAP_PORT",
 	}
-	flAPIBindAddresses = cli.StringFlag{
-		Name:   "bind-addresses, b",
-		Usage:  "Address[:port] to bind to.",
-		EnvVar: "SNAP_BIND_ADDRESSES",
+	flAPIAddr = cli.StringFlag{
+		Name:   "api-addr, b",
+		Usage:  "API Address[:port] to bind to/listen on. Default: empty string => listen on all interfaces",
+		EnvVar: "SNAP_ADDR",
 	}
 	flMaxProcs = cli.IntFlag{
 		Name:   "max-procs, c",
@@ -233,7 +234,7 @@ func main() {
 	app.Flags = []cli.Flag{
 		flAPIDisabled,
 		flAPIPort,
-		flAPIBindAddresses,
+		flAPIAddr,
 		flLogLevel,
 		flLogPath,
 		flMaxProcs,
@@ -361,7 +362,7 @@ func action(ctx *cli.Context) {
 			r.BindTribeManager(tr)
 		}
 		go monitorErrors(r.Err())
-		r.SetAddresses(cfg.RestAPI.BindAddresses, cfg.RestAPI.Port)
+		r.SetAddress(cfg.RestAPI.Address, cfg.RestAPI.Port)
 		coreModules = append(coreModules, r)
 		log.Info("REST API is enabled")
 	} else {
@@ -652,9 +653,37 @@ func setDurationVal(field time.Duration, ctx *cli.Context, flagName string) time
 	return field
 }
 
+//
+func checkCmdLineFlags(ctx *cli.Context) error {
+	// Bind address is specified
+	if ctx.IsSet("api-addr") {
+		addr := ctx.String("api-addr")
+		// Contains a comma
+		if strings.Index(addr, ",") != -1 {
+			return serror.ErrBadAddress
+		}
+		idx := strings.Index(addr, ":")
+		// Port is specified in address string
+		if idx != -1 {
+			// Port is also specified on command line
+			if ctx.IsSet("api-port") {
+				return errors.New("Port can not be specified in both --api-addr and --port")
+			}
+			if idx == (len(addr) - 1) {
+				return errors.New("Empty port specified")
+			}
+		}
+	}
+	return nil
+}
+
 // Apply the command line flags set (if any) to override the values
 // in the input configuration
 func applyCmdLineFlags(cfg *Config, ctx *cli.Context) {
+	err := checkCmdLineFlags(ctx)
+	if err != nil {
+		panic(err)
+	}
 	invertBoolean := true
 	// apply any command line flags that might have been set, first for the
 	// snapd-related flags
@@ -670,7 +699,7 @@ func applyCmdLineFlags(cfg *Config, ctx *cli.Context) {
 	// next for the RESTful server related flags
 	cfg.RestAPI.Enable = setBoolVal(cfg.RestAPI.Enable, ctx, "disable-api", invertBoolean)
 	cfg.RestAPI.Port = setIntVal(cfg.RestAPI.Port, ctx, "api-port")
-	cfg.RestAPI.BindAddresses = setStringVal(cfg.RestAPI.BindAddresses, ctx, "bind-addresses")
+	cfg.RestAPI.Address = setStringVal(cfg.RestAPI.Address, ctx, "ip-addr")
 	cfg.RestAPI.HTTPS = setBoolVal(cfg.RestAPI.HTTPS, ctx, "rest-https")
 	cfg.RestAPI.RestCertificate = setStringVal(cfg.RestAPI.RestCertificate, ctx, "rest-cert")
 	cfg.RestAPI.RestKey = setStringVal(cfg.RestAPI.RestKey, ctx, "rest-key")