Ensure required options are only enforced for relevant subcommands in…

… SDK (#231)

* Move schema validation right on the first usage of `v`

* Fix sub-test

* Add unit test that reproduces the bug

* Set normal flags instead of persistent ones

Persistent flags work for *all* sub-commands, so if they are set as
needed, then they will be needed by any subcommand

* Fix check in unit test

* Rename test and add 2 test cases

* Visit and set the flags for some subcommands

* Add new field Persistent

* Make all default fields persistent

* Visit persistent flags and set their value from Viper

* Make relationships from default fields public

* Don't combine default/persistent fields with developer provided ones

We don't want all fields to be persistent, meaning, to be defined as
if they were defined for all subcommands. Connectors will define
required fields, and making these persistent means that they will be
required in every subcommand, even in those that they make no sense
like `help` or `completions`.

So, anyway, this also implied some changes in design, like not mixing
fields defined by the developer of the connector. The reason for this
is that right now those fields that are persistent are the default
ones brought by the SDK, and Cobra has problems making relationships
between fields if some of them are defined on the subcommand while
others are not. Besides, only the main command should get defined
those fields that are persistent.

* Make function to define persistent fields only

* Add test case

* Be specific about the error for a test case

* Allow for persistent and non-persistent flags in connector configuration

* Delete rendundant function

advice from @CodeRabbit 🤷

internal/tests/notrequireargsinsubcommands_test.go

@@ -0,0 +1,36 @@
+package tests
+
+import (
+	"context"
+	"testing"
+
+	"github.com/conductorone/baton-sdk/pkg/field"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCallSubCommand(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), timeoutIn)
+	defer cancel()
+
+	requiredField := field.StringField("name", field.WithRequired(true))
+	carrier := field.NewConfiguration(
+		[]field.SchemaField{
+			requiredField,
+		},
+	)
+
+	t.Run("should run «help» sub-command successfully", func(t *testing.T) {
+		_, err := entrypoint(ctx, carrier, "help")
+		require.NoError(t, err)
+	})
+
+	t.Run("should run «capabilities» sub-command without success", func(t *testing.T) {
+		_, err := entrypoint(ctx, carrier, "capabilities")
+		require.EqualError(t, err, "(Cobra) Execute failed: required flag(s) \"name\" not set")
+	})
+
+	t.Run("should run «completion zsh» sub-command successfully", func(t *testing.T) {
+		_, err := entrypoint(ctx, carrier, "completion", "zsh")
+		require.NoError(t, err)
+	})
+}

pkg/cli/commands.go

@@ -34,11 +34,6 @@ func MakeMainCommand(
 	opts ...connectorrunner.Option,
 ) func(*cobra.Command, []string) error {
 	return func(*cobra.Command, []string) error {
-		// validate required fields and relationship constraints
-		if err := field.Validate(confschema, v); err != nil {
-			return err
-		}
-
 		runCtx, err := initLogger(
 			ctx,
 			name,
@@ -59,6 +54,11 @@ func MakeMainCommand(
 			}
 		}
 
+		// validate required fields and relationship constraints
+		if err := field.Validate(confschema, v); err != nil {
+			return err
+		}
+
 		c, err := getconnector(runCtx, v)
 		if err != nil {
 			return err
@@ -177,11 +177,6 @@ func MakeGRPCServerCommand(
 	getconnector GetConnectorFunc,
 ) func(*cobra.Command, []string) error {
 	return func(*cobra.Command, []string) error {
-		// validate required fields and relationship constraints
-		if err := field.Validate(confschema, v); err != nil {
-			return err
-		}
-
 		runCtx, err := initLogger(
 			ctx,
 			name,
@@ -192,6 +187,11 @@ func MakeGRPCServerCommand(
 			return err
 		}
 
+		// validate required fields and relationship constraints
+		if err := field.Validate(confschema, v); err != nil {
+			return err
+		}
+
 		c, err := getconnector(runCtx, v)
 		if err != nil {
 			return err
@@ -278,6 +278,7 @@ func MakeCapabilitiesCommand(
 	ctx context.Context,
 	name string,
 	v *viper.Viper,
+	confschema field.Configuration,
 	getconnector GetConnectorFunc,
 ) func(*cobra.Command, []string) error {
 	return func(*cobra.Command, []string) error {
@@ -291,6 +292,11 @@ func MakeCapabilitiesCommand(
 			return err
 		}
 
+		// validate required fields and relationship constraints
+		if err := field.Validate(confschema, v); err != nil {
+			return err
+		}
+
 		c, err := getconnector(runCtx, v)
 		if err != nil {
 			return err