fix: add validator to the final config struct

.gitignore

@@ -28,4 +28,5 @@ vendor/
 .ignore*
 local/
 .deps/
-.cache/
+.cache/
+*.env

aws-cli-auth.go

@@ -2,29 +2,33 @@ package main
 
 import (
 	"context"
-	"log"
 	"os"
 	"os/signal"
 	"syscall"
+	"time"
 
 	"github.com/DevLabFoundry/aws-cli-auth/cmd"
+	"github.com/rs/zerolog"
 )
 
 func main() {
 	ctx, stop := signal.NotifyContext(context.Background(), []os.Signal{os.Interrupt, syscall.SIGTERM, os.Kill}...)
 	defer stop()
+	logger := zerolog.New(zerolog.ConsoleWriter{Out: os.Stderr, TimeFormat: time.RFC3339}).
+		Level(zerolog.ErrorLevel).
+		With().Timestamp().
+		Logger()
 
 	go func() {
 		<-ctx.Done()
 		stop()
-		// log.Printf("\x1b[31minterrupted: %s\x1b[0m", ctx.Err())
-		os.Exit(0)
+		logger.Fatal().Msgf("\x1b[31minterrupted: %s\x1b[0m", ctx.Err())
 	}()
 
-	c := cmd.New()
+	c := cmd.New(logger)
 	c.WithSubCommands(cmd.SubCommands()...)
 
 	if err := c.Execute(ctx); err != nil {
-		log.Fatalf("\x1b[31m%s\x1b[0m", err)
+		logger.Fatal().Msgf("\x1b[31m%s\x1b[0m", err)
 	}
 }

cmd/awscliauth.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/DevLabFoundry/aws-cli-auth/internal/credentialexchange"
 	"github.com/Ensono/eirctl/selfupdate"
+	"github.com/rs/zerolog"
 	"github.com/spf13/cobra"
 )
 
@@ -23,6 +24,7 @@ type Root struct {
 	// ChannelErr io.Writer
 	// viperConf  *viper.Viper
 	rootFlags *RootCmdFlags
+	logger    zerolog.Logger
 	Datadir   string
 }
 
@@ -35,9 +37,10 @@ type RootCmdFlags struct {
 	CustomIniLocation string
 }
 
-func New() *Root {
+func New(logger zerolog.Logger) *Root {
 	rf := &RootCmdFlags{}
 	r := &Root{
+		logger:    logger,
 		rootFlags: rf,
 		Cmd: &cobra.Command{
 			Use:   "aws-cli-auth",

cmd/awscliauth_test.go

@@ -12,13 +12,14 @@ import (
 	"github.com/DevLabFoundry/aws-cli-auth/cmd"
 	"github.com/DevLabFoundry/aws-cli-auth/internal/credentialexchange"
 	"github.com/DevLabFoundry/aws-cli-auth/internal/web"
+	"github.com/rs/zerolog"
 )
 
 func cmdHelperExecutor(t *testing.T, args []string) (stdOut *bytes.Buffer, errOut *bytes.Buffer, err error) {
 	t.Helper()
 	errOut = new(bytes.Buffer)
 	stdOut = new(bytes.Buffer)
-	c := cmd.New()
+	c := cmd.New(zerolog.New(io.Discard))
 	c.WithSubCommands(cmd.SubCommands()...)
 	c.Cmd.SetArgs(args)
 	c.Cmd.SetErr(errOut)

cmd/saml.go

@@ -13,12 +13,15 @@ import (
 	"github.com/DevLabFoundry/aws-cli-auth/internal/web"
 	"github.com/aws/aws-sdk-go-v2/config"
 	"github.com/aws/aws-sdk-go-v2/service/sts"
+	validator "github.com/rezakhademix/govalidator/v2"
+	"github.com/rs/zerolog"
 	"github.com/spf13/cobra"
 	"gopkg.in/ini.v1"
 )
 
 var (
 	ErrUnableToCreateSession = errors.New("sts - cannot start a new session")
+	ErrValidationFailed      = errors.New("missing values")
 )
 
 const (
@@ -64,17 +67,24 @@ func newSamlCmd(r *Root) {
 			if err != nil {
 				return err
 			}
-
+			if r.rootFlags.Verbose {
+				r.logger = r.logger.Level(zerolog.DebugLevel)
+			}
+			r.logger.Debug().Str("CustomIniLocation", r.rootFlags.CustomIniLocation).Msg("if empty using default ~/.aws-cli-auth.ini")
 			iniFile, err := samlInitConfig(r.rootFlags.CustomIniLocation)
 			if err != nil {
 				return err
 			}
 
+			r.logger.Debug().Msgf("iniFile: %+v", iniFile)
+
 			conf, err := credentialexchange.LoadCliConfig(iniFile, r.rootFlags.CfgSectionName)
 			if err != nil {
 				return err
 			}
 
+			r.logger.Debug().Str("section", r.rootFlags.CfgSectionName).Msgf("loaded section: %+v", conf)
+
 			if err := ConfigFromFlags(conf, r.rootFlags, flags, user.Username); err != nil {
 				return err
 			}
@@ -95,25 +105,28 @@ func newSamlCmd(r *Root) {
 				saveRole = allRoles[len(allRoles)-1]
 			}
 
+			r.logger.Debug().Str("saveRole", saveRole).
+				Str("SsoEndpoint", conf.SsoUserEndpoint).
+				Str("SsoCredFedEndpoint", conf.SsoCredFedEndpoint).
+				Msg("")
+
 			secretStore, err := credentialexchange.NewSecretStore(saveRole,
 				fmt.Sprintf("%s-%s", credentialexchange.SELF_NAME, credentialexchange.RoleKeyConverter(saveRole)),
 				os.TempDir(), user.Username)
 			if err != nil {
 				return err
 			}
 
-			// we want to remove any AWS_* env vars that could interfere with the default config
-			// for _, envVar := range []string{"AWS_PROFILE", "AWS_ACCESS_KEY_ID",
-			// 	"AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN"} {
-			// 	os.Unsetenv(envVar)
-			// }
-
-			awsConf, err := config.LoadDefaultConfig(ctx)
+			cfg, err := config.LoadDefaultConfig(ctx)
 			if err != nil {
 				return fmt.Errorf("failed to create session %s, %w", err, ErrUnableToCreateSession)
 			}
 
-			svc := sts.NewFromConfig(awsConf)
+			if cfg.Region == "" {
+				return fmt.Errorf("unable to deduce AWS region, AWS_REGION, AWS_DEFAULT_REGION, ~/.aws/config default or profile level region must be set")
+			}
+
+			svc := sts.NewFromConfig(cfg)
 			webConfig := web.NewWebConf(r.Datadir).
 				WithTimeout(flags.SamlTimeout).
 				WithCustomExecutable(conf.BaseConfig.BrowserExecutablePath)
@@ -165,7 +178,7 @@ If this flag is specified the --sso-role must also be specified.`)
 	// sc.cmd.MarkFlagsRequiredTogether("principal", "role")
 	// SSO flow for SAML
 	sc.cmd.MarkFlagsRequiredTogether("is-sso", "sso-role", "sso-region")
-	sc.cmd.PersistentFlags().Int32VarP(&flags.SamlTimeout, "saml-timeout", "", 120, "Timeout in seconds, before the operation of waiting for a response is cancelled via the chrome driver")
+	sc.cmd.PersistentFlags().Int32VarP(&flags.SamlTimeout, "saml-timeout", "", 120, "Timeout in seconds, before the operation of waiting for a response is cancelled via CDP (ChromeDeubgProto)")
 	// Add subcommand to root command
 	r.Cmd.AddCommand(sc.cmd)
 }
@@ -219,5 +232,24 @@ func ConfigFromFlags(fileConfig *credentialexchange.CredentialConfig, rf *RootCm
 
 	fileConfig.BaseConfig = baseConf
 	fileConfig.Duration = d
+
+	return configValid(fileConfig)
+}
+
+func configValid(config *credentialexchange.CredentialConfig) error {
+	v := validator.New()
+	ssoVal := !config.IsSso
+	if config.IsSso {
+		ssoVal = len(config.SsoRole) > 0 && len(config.SsoRegion) > 0
+	}
+	v.RequiredString(config.ProviderUrl, "provider-url", "provider url must be specified").
+		// RequiredString(config.BaseConfig.Role, "role", "role must be provided").
+		RequiredString(config.PrincipalArn, "principal-arn", "principal ARN must be provided").
+		CustomRule(ssoVal, "is-sso", "sso-role must be specified when is-sso is set").
+		CustomRule((len(config.BaseConfig.Role) > 1 && len(config.SsoRole) < 1) || (len(config.BaseConfig.Role) < 1 && len(config.SsoRole) > 1), "role", "sso-role cannot be specified when role is also set")
+
+	if v.IsFailed() {
+		return fmt.Errorf("%w %#q", ErrValidationFailed, v.Errors())
+	}
 	return nil
 }

cmd/saml_test.go

@@ -1,21 +1,23 @@
 package cmd_test
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/DevLabFoundry/aws-cli-auth/cmd"
 	"github.com/DevLabFoundry/aws-cli-auth/internal/credentialexchange"
 	"github.com/go-test/deep"
 )
 
-func Test_ConfigMerge(t *testing.T) {
+func Test_ConfigMerge_succeeds(t *testing.T) {
 	conf := &credentialexchange.CredentialConfig{
 		BaseConfig: credentialexchange.BaseConfig{
 			BrowserExecutablePath: "/foo/path",
 			Role:                  "role1",
 			RoleChain:             []string{"role-123"},
 		},
-		ProviderUrl: "https://my-idp.com/?app_id=testdd",
+		PrincipalArn: "aw:arn:....123",
+		ProviderUrl:  "https://my-idp.com/?app_id=testdd",
 	}
 	if err := cmd.ConfigFromFlags(conf, &cmd.RootCmdFlags{}, &cmd.SamlCmdFlags{Role: "role-overridden-from-flags"}, "me"); err != nil {
 		t.Fatal(err)
@@ -28,8 +30,79 @@ func Test_ConfigMerge(t *testing.T) {
 			RoleChain:             []string{"role-123"},
 			Username:              "me",
 		},
+		PrincipalArn: "aw:arn:....123",
 	}
 	if diff := deep.Equal(conf, want); len(diff) > 0 {
 		t.Errorf("diff: %v", diff)
 	}
 }
+
+func Test_ConfigMerge_fails_with_missing(t *testing.T) {
+	t.Run("provider not provided", func(t *testing.T) {
+
+		conf := &credentialexchange.CredentialConfig{
+			BaseConfig: credentialexchange.BaseConfig{
+				BrowserExecutablePath: "/foo/path",
+				Role:                  "",
+				RoleChain:             []string{"role-123"},
+			},
+			ProviderUrl: "",
+		}
+		err := cmd.ConfigFromFlags(conf, &cmd.RootCmdFlags{}, &cmd.SamlCmdFlags{Role: "role-overridden-from-flags"}, "me")
+		if !errors.Is(err, cmd.ErrValidationFailed) {
+			t.Error(err)
+		}
+	})
+	t.Run("role not provided", func(t *testing.T) {
+
+		conf := &credentialexchange.CredentialConfig{
+			BaseConfig: credentialexchange.BaseConfig{
+				BrowserExecutablePath: "/foo/path",
+				Role:                  "",
+				RoleChain:             []string{"role-123"},
+			},
+			ProviderUrl: "https://my-idp.com/?app_id=testdd",
+		}
+		err := cmd.ConfigFromFlags(conf, &cmd.RootCmdFlags{}, &cmd.SamlCmdFlags{}, "me")
+		if !errors.Is(err, cmd.ErrValidationFailed) {
+			t.Error(err)
+		}
+	})
+	t.Run("is-sso set but sso-role not set", func(t *testing.T) {
+
+		conf := &credentialexchange.CredentialConfig{
+			BaseConfig: credentialexchange.BaseConfig{
+				BrowserExecutablePath: "/foo/path",
+				Role:                  "",
+				RoleChain:             []string{"role-123"},
+			},
+			PrincipalArn: "some-arn",
+			IsSso:        true,
+			SsoRegion:    "",
+			SsoRole:      "foo:bar",
+			ProviderUrl:  "https://my-idp.com/?app_id=testdd",
+		}
+		err := cmd.ConfigFromFlags(conf, &cmd.RootCmdFlags{}, &cmd.SamlCmdFlags{}, "me")
+		if !errors.Is(err, cmd.ErrValidationFailed) {
+			t.Error(err)
+		}
+	})
+	t.Run("role and sso-role both provided", func(t *testing.T) {
+
+		conf := &credentialexchange.CredentialConfig{
+			BaseConfig: credentialexchange.BaseConfig{
+				BrowserExecutablePath: "/foo/path",
+				Role:                  "",
+				RoleChain:             []string{"role-123"},
+			},
+			PrincipalArn: "some-arn",
+			SsoRegion:    "foo",
+			SsoRole:      "foo:bar",
+			ProviderUrl:  "https://my-idp.com/?app_id=testdd",
+		}
+		err := cmd.ConfigFromFlags(conf, &cmd.RootCmdFlags{}, &cmd.SamlCmdFlags{Role: "wrong-role"}, "me")
+		if !errors.Is(err, cmd.ErrValidationFailed) {
+			t.Error(err)
+		}
+	})
+}

eirctl.yaml

@@ -1,5 +1,5 @@
 import:
-  - https://raw.githubusercontent.com/Ensono/eirctl/e71dd9d66293e27e70fd0620e63a6d627579c060/shared/build/go/eirctl.yaml
+  - https://raw.githubusercontent.com/Ensono/eirctl/refs/tags/v0.9.7/shared/build/go/eirctl.yaml
 
 contexts:
   unit:test:
@@ -12,14 +12,16 @@ contexts:
         - GO
 
 pipelines:
-  build: 
+  build:
     - task: build:unix
     - task: build:win
       depends_on: build:unix
 
   unit:test:run:
     - task: unit:test:prereqs
     - task: unit:test
+      env:
+        ROOT_PKG_NAME: github.com/DevLabFoundry
       depends_on: unit:test:prereqs
 
   bin:release:
@@ -34,7 +36,7 @@ pipelines:
 
 tasks:
   tag:
-    command: 
+    command:
       - |
         git tag -a ${VERSION} -m "ci tag release" ${REVISION}
         git push origin ${VERSION}
@@ -48,6 +50,7 @@ tasks:
     description: |
       Unit test runner needs a bit of extra care in this case to ensure we have all the dependencies
     command: |
+      unset GOTOOLCHAIN
       export GOPATH=$PWD/.deps GOBIN=$PWD/.deps/bin
       CGO_ENABLED=1 go test ./... -v -coverpkg=github.com/DevLabFoundry/... -race -mod=readonly -timeout=1m -shuffle=on -buildvcs=false -coverprofile=.coverage/out -count=1 -run=$GO_TEST_RUN_ARGS | tee .coverage/test.out
       cat .coverage/test.out | .deps/bin/go-junit-report > .coverage/report-junit.xml
@@ -56,7 +59,7 @@ tasks:
   unit:test:prereqs:
     description: Installs coverage and junit tools
     context: unit:test
-    command: 
+    command:
       - |
         mkdir -p .coverage
         export GOPATH="${PWD}/.deps" GOBIN="${PWD}/.deps/bin"
@@ -65,13 +68,14 @@ tasks:
         go install github.com/AlekSi/gocov-xml@v1.0.0
 
   clean:dir:
-    command: 
+    command:
       - |
         rm -rf dist/
 
   build:win:
     context: go1x
     description: Builds Go binary
+    reset_context: true
     command:
       - |
         mkdir -p .deps