feat: Fail on invalid config. (#8295)

Signed-off-by: Alex Collins <alex_collins@intuit.com>

config/config.go

@@ -105,6 +105,9 @@ type Config struct {
 
 	// NavColor is an ui navigation bar background color
 	NavColor string `json:"navColor,omitempty"`
+
+	// SSO in settings for single-sign on
+	SSO SSOConfig `json:"sso,omitempty"`
 }
 
 func (c Config) GetContainerRuntimeExecutor(labels labels.Labels) (string, error) {

config/controller.go

@@ -7,14 +7,13 @@ import (
 
 	log "github.com/sirupsen/logrus"
 	apiv1 "k8s.io/api/core/v1"
-	apierr "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"sigs.k8s.io/yaml"
 )
 
 type Controller interface {
-	Get(context.Context, interface{}) error
+	Get(context.Context) (*Config, error)
 }
 
 type controller struct {
@@ -33,7 +32,7 @@ func NewController(namespace, name string, kubeclientset kubernetes.Interface) C
 	}
 }
 
-func parseConfigMap(cm *apiv1.ConfigMap, config interface{}) error {
+func parseConfigMap(cm *apiv1.ConfigMap, config *Config) error {
 	// The key in the configmap to retrieve workflow configuration from.
 	// Content encoding is expected to be YAML.
 	rawConfig, ok := cm.Data["config"]
@@ -50,15 +49,16 @@ func parseConfigMap(cm *apiv1.ConfigMap, config interface{}) error {
 			}
 		}
 	}
-	err := yaml.Unmarshal([]byte(rawConfig), config)
+	err := yaml.UnmarshalStrict([]byte(rawConfig), config)
 	return err
 }
 
-func (cc *controller) Get(ctx context.Context, config interface{}) error {
+func (cc *controller) Get(ctx context.Context) (*Config, error) {
+	config := &Config{}
 	cmClient := cc.kubeclientset.CoreV1().ConfigMaps(cc.namespace)
 	cm, err := cmClient.Get(ctx, cc.configMap, metav1.GetOptions{})
-	if err != nil && !apierr.IsNotFound(err) {
-		return err
+	if err != nil {
+		return nil, err
 	}
-	return parseConfigMap(cm, config)
+	return config, parseConfigMap(cm, config)
 }

config/controller_test.go

@@ -38,9 +38,9 @@ func Test_parseConfigMap(t *testing.T) {
 			assert.NotEmpty(t, c.ArtifactRepository)
 		}
 	})
-	t.Run("IgnoreGarbage", func(t *testing.T) {
+	t.Run("Garbage", func(t *testing.T) {
 		c := &Config{}
 		err := parseConfigMap(&apiv1.ConfigMap{Data: map[string]string{"garbage": "garbage"}}, c)
-		assert.NoError(t, err)
+		assert.Error(t, err)
 	})
 }

config/rbac.go

@@ -0,0 +1,9 @@
+package config
+
+type RBACConfig struct {
+	Enabled bool `json:"enabled,omitempty"`
+}
+
+func (c *RBACConfig) IsEnabled() bool {
+	return c != nil && c.Enabled
+}

config/sso.go

@@ -0,0 +1,31 @@
+package config
+
+import (
+	"time"
+
+	apiv1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+type SSOConfig struct {
+	Issuer       string                  `json:"issuer"`
+	IssuerAlias  string                  `json:"issuerAlias,omitempty"`
+	ClientID     apiv1.SecretKeySelector `json:"clientId"`
+	ClientSecret apiv1.SecretKeySelector `json:"clientSecret"`
+	RedirectURL  string                  `json:"redirectUrl"`
+	RBAC         *RBACConfig             `json:"rbac,omitempty"`
+	// additional scopes (on top of "openid")
+	Scopes        []string        `json:"scopes,omitempty"`
+	SessionExpiry metav1.Duration `json:"sessionExpiry,omitempty"`
+	// customGroupClaimName will override the groups claim name
+	CustomGroupClaimName string `json:"customGroupClaimName,omitempty"`
+	UserInfoPath         string `json:"userInfoPath,omitempty"`
+	InsecureSkipVerify   bool   `json:"insecureSkipVerify,omitempty"`
+}
+
+func (c SSOConfig) GetSessionExpiry() time.Duration {
+	if c.SessionExpiry.Duration > 0 {
+		return c.SessionExpiry.Duration
+	}
+	return 10 * time.Hour
+}

server/apiserver/argoserver.go

@@ -127,8 +127,7 @@ func NewArgoServer(ctx context.Context, opts ArgoServerOpts) (*argoServer, error
 	var resourceCache *cache.ResourceCache = nil
 	ssoIf := sso.NullSSO
 	if opts.AuthModes[auth.SSO] {
-		c := &Config{}
-		err := configController.Get(ctx, c)
+		c, err := configController.Get(ctx)
 		if err != nil {
 			return nil, err
 		}
@@ -173,8 +172,7 @@ var backoff = wait.Backoff{
 }
 
 func (as *argoServer) Run(ctx context.Context, port int, browserOpenFunc func(string)) {
-	config := &Config{}
-	err := as.configController.Get(ctx, config)
+	config, err := as.configController.Get(ctx)
 	if err != nil {
 		log.Fatal(err)
 	}

server/auth/sso/sso.go

@@ -12,6 +12,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/argoproj/argo-workflows/v3/config"
+
 	pkgrand "github.com/argoproj/pkg/rand"
 	"github.com/coreos/go-oidc/v3/oidc"
 	"github.com/go-jose/go-jose/v3"
@@ -23,7 +25,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 
-	"github.com/argoproj/argo-workflows/v3/server/auth/rbac"
 	"github.com/argoproj/argo-workflows/v3/server/auth/types"
 )
 
@@ -45,6 +46,8 @@ type Interface interface {
 
 var _ Interface = &sso{}
 
+type Config = config.SSOConfig
+
 type sso struct {
 	config          *oauth2.Config
 	issuer          string
@@ -54,7 +57,7 @@ type sso struct {
 	secure          bool
 	privateKey      crypto.PrivateKey
 	encrypter       jose.Encrypter
-	rbacConfig      *rbac.Config
+	rbacConfig      *config.RBACConfig
 	expiry          time.Duration
 	customClaimName string
 	userInfoPath    string
@@ -64,29 +67,6 @@ func (s *sso) IsRBACEnabled() bool {
 	return s.rbacConfig.IsEnabled()
 }
 
-type Config struct {
-	Issuer       string                  `json:"issuer"`
-	IssuerAlias  string                  `json:"issuerAlias,omitempty"`
-	ClientID     apiv1.SecretKeySelector `json:"clientId"`
-	ClientSecret apiv1.SecretKeySelector `json:"clientSecret"`
-	RedirectURL  string                  `json:"redirectUrl"`
-	RBAC         *rbac.Config            `json:"rbac,omitempty"`
-	// additional scopes (on top of "openid")
-	Scopes        []string        `json:"scopes,omitempty"`
-	SessionExpiry metav1.Duration `json:"sessionExpiry,omitempty"`
-	// customGroupClaimName will override the groups claim name
-	CustomGroupClaimName string `json:"customGroupClaimName,omitempty"`
-	UserInfoPath         string `json:"userInfoPath,omitempty"`
-	InsecureSkipVerify   bool   `json:"insecureSkipVerify,omitempty"`
-}
-
-func (c Config) GetSessionExpiry() time.Duration {
-	if c.SessionExpiry.Duration > 0 {
-		return c.SessionExpiry.Duration
-	}
-	return 10 * time.Hour
-}
-
 // Abstract methods of oidc.Provider that our code uses into an interface. That
 // will allow us to implement a stub for unit testing.  If you start using more
 // oidc.Provider methods in this file, add them here and provide a stub

test/e2e/fixtures/e2e_suite.go

@@ -63,8 +63,9 @@ func (s *E2ESuite) SetupSuite() {
 	configController := config.NewController(Namespace, common.ConfigMapName, s.KubeClient)
 
 	ctx := context.Background()
-	err = configController.Get(ctx, &s.Config)
+	c, err := configController.Get(ctx)
 	s.CheckError(err)
+	s.Config = c
 	s.wfClient = versioned.NewForConfigOrDie(s.RestConfig).ArgoprojV1alpha1().Workflows(Namespace)
 	s.wfebClient = versioned.NewForConfigOrDie(s.RestConfig).ArgoprojV1alpha1().WorkflowEventBindings(Namespace)
 	s.wfTemplateClient = versioned.NewForConfigOrDie(s.RestConfig).ArgoprojV1alpha1().WorkflowTemplates(Namespace)

test/e2e/manifests/mixins/workflow-controller-configmap.yaml

@@ -23,4 +23,3 @@ data:
     completed: 10
     failed: 2
     errored: 2
-  kubeletInsecure: "true"

workflow/controller/controller.go

@@ -421,10 +421,11 @@ func (wfc *WorkflowController) createClusterWorkflowTemplateInformer(ctx context
 }
 
 func (wfc *WorkflowController) UpdateConfig(ctx context.Context) {
-	err := wfc.configController.Get(ctx, &wfc.Config)
+	c, err := wfc.configController.Get(ctx)
 	if err != nil {
 		log.Fatalf("Failed to register watch for controller config map: %v", err)
 	}
+	wfc.Config = *c
 	err = wfc.updateConfig()
 	if err != nil {
 		log.Fatalf("Failed to update config: %v", err)