[FAB-3458] Remove global vars in server command

Removed the global variables from the fabric-ca-server package and encapsulated them in a struct. Not having global values is a good practice as well as in this case fix issues with unit test cases. Change-Id: Iea761eadb6aa115b680a313ca5cb4026d2f09e5c Signed-off-by: Anil Ambati <[email protected]>
hyperledger · Aug 21, 2017 · f77203e · f77203e
1 parent 3418fe2
commit f77203e
Show file tree

Hide file tree

Showing 9 changed files with 240 additions and 299 deletions.
diff --git a/cmd/fabric-ca-client/main.go b/cmd/fabric-ca-client/main.go
@@ -18,14 +18,12 @@ package main
 
 import (
 	"os"
-
-	"github.com/hyperledger/fabric-ca/util"
 )
 
 // The fabric-ca client main
 func main() {
 	if err := RunMain(os.Args); err != nil {
-		util.Fatal("%s", err)
+		os.Exit(1)
 	}
 }
 
@@ -36,7 +34,11 @@ func RunMain(args []string) error {
 	os.Args = args
 
 	// Execute the command
-	ccmd := NewCommand(args[1])
+	cmdName := ""
+	if len(args) > 1 {
+		cmdName = args[1]
+	}
+	ccmd := NewCommand(cmdName)
 	err := ccmd.Execute()
 
 	// Restore original os.Args

diff --git a/cmd/fabric-ca-client/main_test.go b/cmd/fabric-ca-client/main_test.go
@@ -123,6 +123,12 @@ type TestData struct {
 	input []string // input
 }
 
+func TestNoArguments(t *testing.T) {
+	err := RunMain([]string{cmdName})
+	if err == nil {
+		assert.Error(t, errors.New("Should have resulted in an error as no agruments provided"))
+	}
+}
 func TestExtraArguments(t *testing.T) {
 	errCases := []TestData{
 		{[]string{cmdName, "enroll", "extraArg", "extraArg2"}},

diff --git a/cmd/fabric-ca-server/config.go b/cmd/fabric-ca-server/config.go
@@ -375,37 +375,36 @@ intermediate:
 )
 
 var (
-	// cfgFileName is the name of the config file
-	cfgFileName string
-	// serverCfg is the server's config
-	serverCfg      *lib.ServerConfig
 	extraArgsError = "Unrecognized arguments found: %v\n\n%s"
 )
 
 // Initialize config
-func configInit(cmd string) (err error) {
+func (s *ServerCmd) configInit() (err error) {
+	if !s.configRequired() {
+		return nil
+	}
 	// Make the config file name absolute
-	if !filepath.IsAbs(cfgFileName) {
-		cfgFileName, err = filepath.Abs(cfgFileName)
+	if !filepath.IsAbs(s.cfgFileName) {
+		s.cfgFileName, err = filepath.Abs(s.cfgFileName)
 		if err != nil {
 			return fmt.Errorf("Failed to get full path of config file: %s", err)
 		}
 	}
 
 	// If the config file doesn't exist, create a default one
-	if !util.FileExists(cfgFileName) {
-		err = createDefaultConfigFile()
+	if !util.FileExists(s.cfgFileName) {
+		err = s.createDefaultConfigFile()
 		if err != nil {
 			return fmt.Errorf("Failed to create default configuration file: %s", err)
 		}
-		log.Infof("Created default configuration file at %s", cfgFileName)
+		log.Infof("Created default configuration file at %s", s.cfgFileName)
 	} else {
-		log.Infof("Configuration file location: %s", cfgFileName)
+		log.Infof("Configuration file location: %s", s.cfgFileName)
 	}
 
 	// Read the config
 	viper.AutomaticEnv() // read in environment variables that match
-	err = lib.UnmarshalConfig(serverCfg, viper.GetViper(), cfgFileName, true, true)
+	err = lib.UnmarshalConfig(s.cfg, viper.GetViper(), s.cfgFileName, true, true)
 	if err != nil {
 		return err
 	}
@@ -415,20 +414,20 @@ func configInit(cmd string) (err error) {
 	// true as CFSSL expects.
 	pl := "csr.ca.pathlength"
 	if viper.IsSet(pl) && viper.GetInt(pl) == 0 {
-		serverCfg.CAcfg.CSR.CA.PathLenZero = true
+		s.cfg.CAcfg.CSR.CA.PathLenZero = true
 	}
 	// The maxpathlen field controls how deep the CA hierarchy when issuing
 	// a CA certificate. If it is explicitly set to 0, set the PathLenZero
 	// field to true as CFSSL expects.
 	pl = "signing.profiles.ca.caconstraint.maxpathlen"
 	if viper.IsSet(pl) && viper.GetInt(pl) == 0 {
-		serverCfg.CAcfg.Signing.Profiles["ca"].CAConstraint.MaxPathLenZero = true
+		s.cfg.CAcfg.Signing.Profiles["ca"].CAConstraint.MaxPathLenZero = true
 	}
 
 	return nil
 }
 
-func createDefaultConfigFile() error {
+func (s *ServerCmd) createDefaultConfigFile() error {
 	var user, pass string
 	// If LDAP is enabled, authentication of enrollment requests are performed
 	// by using LDAP authentication; therefore, no bootstrap username and password
@@ -486,32 +485,12 @@ func createDefaultConfigFile() error {
 	}
 
 	// Now write the file
-	cfgDir := filepath.Dir(cfgFileName)
+	cfgDir := filepath.Dir(s.cfgFileName)
 	err = os.MkdirAll(cfgDir, 0755)
 	if err != nil {
 		return err
 	}
 
 	// Now write the file
-	return ioutil.WriteFile(cfgFileName, []byte(cfg), 0644)
-}
-
-// getCAName returns CA Name
-// If ca.name property is specified (via the environment variable
-// 'FABRIC_CA_SERVER_CA_NAME' or the command line option '--ca.name' or
-// in the configuration file), then its value is returned
-// If ca.name property is not specified, domain is extracted from the hostname and is
-// returned
-// If domain is empty, then hostname is returned
-func getCAName(hostname string) (caName string) {
-	caName = viper.GetString("ca.name")
-	if caName != "" {
-		return caName
-	}
-
-	caName = strings.Join(strings.Split(hostname, ".")[1:], ".")
-	if caName == "" {
-		caName = hostname
-	}
-	return caName
+	return ioutil.WriteFile(s.cfgFileName, []byte(cfg), 0644)
 }
diff --git a/cmd/fabric-ca-server/init.go b/cmd/fabric-ca-server/init.go
diff --git a/cmd/fabric-ca-server/main.go b/cmd/fabric-ca-server/main.go
@@ -16,63 +16,12 @@ limitations under the License.
 
 package main
 
-import (
-	"os"
-	"path/filepath"
-	"strings"
+import "os"
 
-	"github.com/hyperledger/fabric-ca/lib"
-	"github.com/hyperledger/fabric-ca/util"
-	"github.com/spf13/cobra"
-	"github.com/spf13/viper"
-)
-
-// rootCmd is the base command for the Hyerledger Fabric CA server
 var (
-	rootCmd = &cobra.Command{
-		Use:   cmdName,
-		Short: longName,
-		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
-			util.CmdRunBegin()
-			cmd.SilenceUsage = true
-			return nil
-		},
-	}
 	blockingStart = true
 )
 
-func init() {
-	// Get the default config file path
-	cfg := util.GetDefaultConfigFile(cmdName)
-
-	// All env variables must be prefixed
-	viper.SetEnvPrefix(envVarPrefix)
-	viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
-
-	// Set specific global flags used by all commands
-	pflags := rootCmd.PersistentFlags()
-	pflags.StringVarP(&cfgFileName, "config", "c", cfg, "Configuration file")
-	util.FlagString(pflags, "boot", "b", "",
-		"The user:pass for bootstrap admin; it is required to build default config file when ldap.enabled is false")
-
-	// Register flags for all tagged and exported fields in the config
-	serverCfg = &lib.ServerConfig{}
-	tags := map[string]string{
-		"help.csr.cn":           "The common name field of the certificate signing request to a parent fabric-ca-server",
-		"skip.csr.serialnumber": "true",
-		"help.csr.hosts":        "A list of comma-separated host names in a certificate signing request to a parent fabric-ca-server",
-	}
-	err := util.RegisterFlags(pflags, serverCfg, nil)
-	if err != nil {
-		panic(err)
-	}
-	caCfg := &lib.CAConfig{}
-	err = util.RegisterFlags(pflags, caCfg, tags)
-	if err != nil {
-		panic(err)
-	}
-}
-
 // The fabric-ca server main
 func main() {
 	if err := RunMain(os.Args); err != nil {
@@ -82,29 +31,21 @@ func main() {
 
 // RunMain is the fabric-ca server main
 func RunMain(args []string) error {
-
 	// Save the os.Args
 	saveOsArgs := os.Args
 	os.Args = args
 
+	cmdName := ""
+	if len(args) > 1 {
+		cmdName = args[1]
+	}
+	scmd := NewCommand(cmdName, blockingStart)
+
 	// Execute the command
-	err := rootCmd.Execute()
+	err := scmd.Execute()
 
 	// Restore original os.Args
 	os.Args = saveOsArgs
 
 	return err
 }
-
-// Get a server for the init and start commands
-func getServer() *lib.Server {
-	return &lib.Server{
-		HomeDir:       filepath.Dir(cfgFileName),
-		Config:        serverCfg,
-		BlockingStart: blockingStart,
-		CA: lib.CA{
-			Config:         &serverCfg.CAcfg,
-			ConfigFilePath: cfgFileName,
-		},
-	}
-}