SPKI: Use RunE in cobra commands (#3102)

This PR is in preparation to the scion-pki testing pipeline.
Instead of calling os.Exit() the tests now use cobra to return errors.
With this change, unit tests can check the exit codes of the application
without building and running the binary by directly invoking the
commands.

go/lib/keyconf/keyconf.go

@@ -54,7 +54,7 @@ const (
 	// FIXME(roosd): removed unused keys above.
 
 	ASSigKeyFile = "as-signing.key"
-	ASDecKeyFile = "as-decrypt.ley"
+	ASDecKeyFile = "as-decrypt.key"
 	ASRevKeyFile = "as-revocation.key"
 
 	IssuerRevKeyFile  = "issuer-revocation.key"

go/tools/scion-pki/internal/cmd/BUILD.bazel

@@ -6,6 +6,7 @@ go_library(
     importpath = "github.com/scionproto/scion/go/tools/scion-pki/internal/cmd",
     visibility = ["//go/tools/scion-pki:__subpackages__"],
     deps = [
+        "//go/lib/common:go_default_library",
         "//go/tools/scion-pki/internal/certs:go_default_library",
         "//go/tools/scion-pki/internal/keys:go_default_library",
         "//go/tools/scion-pki/internal/pkicmn:go_default_library",

go/tools/scion-pki/internal/cmd/cmd.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/tools/scion-pki/internal/certs"
 	"github.com/scionproto/scion/go/tools/scion-pki/internal/keys"
 	"github.com/scionproto/scion/go/tools/scion-pki/internal/pkicmn"
@@ -41,6 +42,8 @@ root configuration files used in the SCION control plane PKI.`,
 			pkicmn.OutDir = pkicmn.RootDir
 		}
 	},
+	SilenceErrors: true,
+	SilenceUsage:  true,
 }
 
 const (
@@ -82,8 +85,14 @@ var autoCompleteCmd = &cobra.Command{
 }
 
 func Execute() {
-	if err := RootCmd.Execute(); err != nil {
-		fmt.Fprintf(os.Stderr, "%s\n", err)
+	err := RootCmd.Execute()
+	switch err.(type) {
+	case common.BasicError:
+		fmt.Fprintf(os.Stderr, "Error: %s\n", err)
+		os.Exit(2)
+	case error:
+		fmt.Fprintf(os.Stderr, "Error: %s\n", err)
+		RootCmd.Usage()
 		os.Exit(1)
 	}
 }

go/tools/scion-pki/internal/v2/cmd/BUILD.bazel

@@ -8,7 +8,7 @@ go_library(
     deps = [
         "//go/tools/scion-pki/internal/v2/keys:go_default_library",
         "//go/tools/scion-pki/internal/v2/tmpl:go_default_library",
-        "//go/tools/scion-pki/internal/v2/trc:go_default_library",
+        "//go/tools/scion-pki/internal/v2/trcs:go_default_library",
         "@com_github_spf13_cobra//:go_default_library",
     ],
 )

go/tools/scion-pki/internal/v2/cmd/cmd.go

@@ -19,7 +19,7 @@ import (
 
 	"github.com/scionproto/scion/go/tools/scion-pki/internal/v2/keys"
 	"github.com/scionproto/scion/go/tools/scion-pki/internal/v2/tmpl"
-	"github.com/scionproto/scion/go/tools/scion-pki/internal/v2/trc"
+	"github.com/scionproto/scion/go/tools/scion-pki/internal/v2/trcs"
 )
 
 var Cmd = &cobra.Command{
@@ -30,5 +30,5 @@ var Cmd = &cobra.Command{
 func init() {
 	Cmd.AddCommand(tmpl.Cmd)
 	Cmd.AddCommand(keys.Cmd)
-	Cmd.AddCommand(trc.Cmd)
+	Cmd.AddCommand(trcs.Cmd)
 }

go/tools/scion-pki/internal/v2/conf/as.go

@@ -106,8 +106,7 @@ func NewTemplateASCfg(subject addr.IA, trcVer uint64, voting, issuing bool) *ASC
 
 // LoadASCfg loads the AS configuration from a directory.
 func LoadASCfg(dir string) (*ASCfg, error) {
-	cname := filepath.Join(dir, ASConfFileName)
-	cfg, err := ini.Load(cname)
+	cfg, err := ini.Load(ASCfgPath(dir))
 	if err != nil {
 		return nil, err
 	}
@@ -121,6 +120,11 @@ func LoadASCfg(dir string) (*ASCfg, error) {
 	return as, nil
 }
 
+// ASCfgPath returns the path to the AS config file given a directory.
+func ASCfgPath(dir string) string {
+	return filepath.Join(dir, ASConfFileName)
+}
+
 // Validate parses the raw values and validates that the AS config is correct.
 func (a *ASCfg) Validate() error {
 	if a.AS == nil {

go/tools/scion-pki/internal/v2/conf/isd.go

@@ -58,8 +58,7 @@ func NewTemplateISDCfg() *ISDCfg {
 
 // LoadISDCfg loads the ISD configuration from a directory.
 func LoadISDCfg(dir string) (*ISDCfg, error) {
-	cname := filepath.Join(dir, ISDCfgFileName)
-	cfg, err := ini.Load(cname)
+	cfg, err := ini.Load(ISDCfgPath(dir))
 	if err != nil {
 		return nil, err
 	}
@@ -76,6 +75,11 @@ func LoadISDCfg(dir string) (*ISDCfg, error) {
 	return i, nil
 }
 
+// ISDCfgPath returns the path to the ISD config file given a directory.
+func ISDCfgPath(dir string) string {
+	return filepath.Join(dir, ISDCfgFileName)
+}
+
 // Write writes the ISD config to the provided path.
 func (i *ISDCfg) Write(path string, force bool) error {
 	// Check if file exists and do not override without -f

go/tools/scion-pki/internal/v2/keys/cmd.go

@@ -18,6 +18,8 @@ import (
 	"fmt"
 
 	"github.com/spf13/cobra"
+
+	"github.com/scionproto/scion/go/lib/common"
 )
 
 var Cmd = &cobra.Command{
@@ -40,9 +42,12 @@ Selector:
 var genCmd = &cobra.Command{
 	Use:   "gen",
 	Short: "Generate new keys",
-	Args:  cobra.MinimumNArgs(1),
-	Run: func(cmd *cobra.Command, args []string) {
-		runGenKey(args)
+	Args:  cobra.ExactArgs(1),
+	RunE: func(cmd *cobra.Command, args []string) error {
+		if err := runGenKey(args[0]); err != nil {
+			return common.NewBasicError("unable to generate keys", err)
+		}
+		return nil
 	},
 }
 

go/tools/scion-pki/internal/v2/keys/gen.go

@@ -30,20 +30,20 @@ import (
 	"github.com/scionproto/scion/go/tools/scion-pki/internal/v2/conf"
 )
 
-func runGenKey(args []string) {
-	asMap, err := pkicmn.ProcessSelector(args[0])
+func runGenKey(selector string) error {
+	asMap, err := pkicmn.ProcessSelector(selector)
 	if err != nil {
-		pkicmn.ErrorAndExit("Error: %s\n", err)
+		return err
 	}
 	for isd, ases := range asMap {
 		isdCfg, err := conf.LoadISDCfg(pkicmn.GetIsdPath(pkicmn.RootDir, isd))
 		if err != nil {
-			pkicmn.ErrorAndExit("Error reading isd.ini: %s\n", err)
+			return common.NewBasicError("unable to read isd.ini", err, "isd", isd)
 		}
 		for _, ia := range ases {
 			asCfg, err := conf.LoadASCfg(pkicmn.GetAsPath(pkicmn.RootDir, ia))
 			if err != nil {
-				pkicmn.ErrorAndExit("Error reading as.ini for %s: %s", ia, err)
+				return common.NewBasicError("unable to read as.ini", err, "ia", ia)
 			}
 			as := as{
 				cfg:     asCfg,
@@ -53,11 +53,11 @@ func runGenKey(args []string) {
 			}
 			pkicmn.QuietPrint("Generating keys for %s\n", ia)
 			if err = as.gen(); err != nil {
-				pkicmn.ErrorAndExit("Error generating keys: %s\n", err)
+				return common.NewBasicError("unable to generate keys", err, "ia", ia)
 			}
 		}
 	}
-	os.Exit(0)
+	return nil
 }
 
 type as struct {
@@ -90,7 +90,7 @@ func (a *as) gen() error {
 		})
 	}
 	if err := os.MkdirAll(a.outDir, 0700); err != nil {
-		return nil
+		return err
 	}
 	for file, keyType := range keys {
 		if err := a.genKey(file, keyType); err != nil {
@@ -103,7 +103,7 @@ func (a *as) gen() error {
 func (a *as) genKey(fname, keyType string) error {
 	privKey, err := genKey(keyType)
 	if err != nil {
-		return common.NewBasicError("Error generating keys", err, "key", fname)
+		return common.NewBasicError("error generating key", err, "key", fname)
 	}
 	// Skip keys that should not be generated.
 	if privKey == nil {
@@ -113,7 +113,7 @@ func (a *as) genKey(fname, keyType string) error {
 	privKeyPath := filepath.Join(a.outDir, fname)
 	privKeyEnc := base64.StdEncoding.EncodeToString(privKey)
 	if err = pkicmn.WriteToFile([]byte(privKeyEnc), privKeyPath, 0600); err != nil {
-		return common.NewBasicError("Cannot write key file", err, "key", fname)
+		return common.NewBasicError("cannot write key file", err, "key", fname)
 	}
 	return nil
 }
@@ -143,7 +143,7 @@ func genMasterKey() ([]byte, error) {
 		return nil, err
 	}
 	if n != 16 {
-		return nil, common.NewBasicError("Not enough random bytes", nil)
+		return nil, common.NewBasicError("not enough random bytes", nil)
 	}
 	return key, nil
 }

go/tools/scion-pki/internal/v2/tmpl/as.go

@@ -18,28 +18,29 @@ import (
 	"path/filepath"
 
 	"github.com/scionproto/scion/go/lib/addr"
+	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/tools/scion-pki/internal/pkicmn"
 	"github.com/scionproto/scion/go/tools/scion-pki/internal/v2/conf"
 )
 
-func runGenAsTmpl(args []string) {
-	asMap, err := pkicmn.ProcessSelector(args[0])
+func runGenASTmpl(selector string) error {
+	asMap, err := pkicmn.ProcessSelector(selector)
 	if err != nil {
-		pkicmn.ErrorAndExit("Error: %s\n", err)
+		return err
 	}
-	pkicmn.QuietPrint("Generating cert config templates.\n")
+	pkicmn.QuietPrint("Generating AS config templates.\n")
 	for isd, ases := range asMap {
-		iconf, err := conf.LoadISDCfg(pkicmn.GetIsdPath(pkicmn.RootDir, isd))
+		isdCfg, err := conf.LoadISDCfg(pkicmn.GetIsdPath(pkicmn.RootDir, isd))
 		if err != nil {
-			pkicmn.ErrorAndExit("Error reading %s: %s\n", conf.ISDCfgFileName, err)
+			return common.NewBasicError("unable to read isd.ini", err, "isd", isd)
 		}
 		for _, ia := range ases {
-			if err = genAndWriteASTmpl(ia, iconf); err != nil {
-				pkicmn.ErrorAndExit("Error generating %s template for %s: %s\n",
-					conf.ASConfFileName, ia, err)
+			if err = genAndWriteASTmpl(ia, isdCfg); err != nil {
+				return common.NewBasicError("error generating as.ini template", err, "ia", ia)
 			}
 		}
 	}
+	return nil
 }
 
 func genAndWriteASTmpl(ia addr.IA, isd *conf.ISDCfg) error {

go/tools/scion-pki/internal/v2/tmpl/cmd.go

@@ -16,6 +16,8 @@ package tmpl
 
 import (
 	"github.com/spf13/cobra"
+
+	"github.com/scionproto/scion/go/lib/common"
 )
 
 var Cmd = &cobra.Command{
@@ -31,7 +33,11 @@ var topo = &cobra.Command{
 	Short: "Generate isd.ini and as.ini templates for the provided topo",
 	Args:  cobra.ExactArgs(1),
 	RunE: func(cmd *cobra.Command, args []string) error {
-		return runGenTopoTmpl(args)
+		if err := runGenTopoTmpl(args[0]); err != nil {
+			return common.NewBasicError("unable to generate templates from topo", err,
+				"file", args[0])
+		}
+		return nil
 	},
 }
 
@@ -45,9 +51,13 @@ Selector:
 	X
 		A specific ISD X.
 `,
-	Args: cobra.MinimumNArgs(1),
-	Run: func(cmd *cobra.Command, args []string) {
-		runGenIsdTmpl(args)
+	Args: cobra.ExactArgs(1),
+	RunE: func(cmd *cobra.Command, args []string) error {
+		if err := runGenISDTmpl(args[0]); err != nil {
+			return common.NewBasicError("unable to generate ISD templates", err,
+				"selector", args[0])
+		}
+		return nil
 	},
 }
 
@@ -63,9 +73,12 @@ Selector:
 	X-Y
 		A specific AS X-Y, e.g. AS 1-ff00:0:300
 `,
-	Args: cobra.MinimumNArgs(1),
-	Run: func(cmd *cobra.Command, args []string) {
-		runGenAsTmpl(args)
+	Args: cobra.ExactArgs(1),
+	RunE: func(cmd *cobra.Command, args []string) error {
+		if err := runGenASTmpl(args[0]); err != nil {
+			return common.NewBasicError("unable to generate AS templates", err, "selector", args[0])
+		}
+		return nil
 	},
 }
 

go/tools/scion-pki/internal/v2/tmpl/isd.go

@@ -19,22 +19,26 @@ import (
 	"path/filepath"
 
 	"github.com/scionproto/scion/go/lib/addr"
+	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/tools/scion-pki/internal/pkicmn"
 	"github.com/scionproto/scion/go/tools/scion-pki/internal/v2/conf"
 )
 
-func runGenIsdTmpl(args []string) {
-	asMap, err := pkicmn.ProcessSelector(args[0])
+func runGenISDTmpl(selector string) error {
+	asMap, err := pkicmn.ProcessSelector(selector)
 	if err != nil {
-		pkicmn.ErrorAndExit("Error: %s\n", err)
+		return err
 	}
 	pkicmn.QuietPrint("Generating trc config templates.\n")
 	for isd := range asMap {
-		genIsdTmpl(isd)
+		if err := genISDTmpl(isd); err != nil {
+			return common.NewBasicError("error generating isd.ini template", err, "isd", isd)
+		}
 	}
+	return nil
 }
 
-func genIsdTmpl(isd addr.ISD) error {
+func genISDTmpl(isd addr.ISD) error {
 	dir := pkicmn.GetIsdPath(pkicmn.RootDir, isd)
 	pkicmn.QuietPrint("Generating configuration template for ISD%d\n", isd)
 	i := conf.NewTemplateISDCfg()

go/tools/scion-pki/internal/v2/tmpl/topo.go

@@ -35,18 +35,18 @@ var (
 	rawValidity string
 )
 
-func runGenTopoTmpl(args []string) error {
-	raw, err := ioutil.ReadFile(args[0])
+func runGenTopoTmpl(path string) error {
+	raw, err := ioutil.ReadFile(path)
 	if err != nil {
-		return common.NewBasicError("unable to read file", err, "file", args[0])
+		return common.NewBasicError("unable to read file", err)
 	}
 	val, err := validityFromFlags()
 	if err != nil {
 		return err
 	}
 	var topo topoFile
 	if err := yaml.Unmarshal(raw, &topo); err != nil {
-		return common.NewBasicError("unable to parse topo", err, "file", args[0])
+		return common.NewBasicError("unable to parse topo", err)
 	}
 	isdCfgs := make(map[addr.ISD]*conf.ISDCfg)
 	for isd := range topo.ISDs() {
@@ -70,7 +70,6 @@ func runGenTopoTmpl(args []string) error {
 			return common.NewBasicError("unable to write AS config", err, "ia", ia)
 		}
 	}
-
 	return nil
 }
 

go/tools/scion-pki/internal/v2/trc/BUILD.bazel → go/tools/scion-pki/internal/v2/trcs/BUILD.bazel

@@ -10,7 +10,7 @@ go_library(
         "sign.go",
         "util.go",
     ],
-    importpath = "github.com/scionproto/scion/go/tools/scion-pki/internal/v2/trc",
+    importpath = "github.com/scionproto/scion/go/tools/scion-pki/internal/v2/trcs",
     visibility = ["//go/tools/scion-pki:__subpackages__"],
     deps = [
         "//go/lib/addr:go_default_library",

go/tools/scion-pki/internal/v2/trc/ases.go → go/tools/scion-pki/internal/v2/trcs/ases.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package trc
+package trcs
 
 import (
 	"path/filepath"