Handle validation when value for runAsGroup and runAsUser is empty

pkg/apis/config/default.go

@@ -17,9 +17,7 @@ limitations under the License.
 package config
 
 import (
-	"fmt"
 	"os"
-	"strconv"
 
 	corev1 "k8s.io/api/core/v1"
 )
@@ -29,16 +27,16 @@ const (
 	defaultRunAsUserKey        = "default-run-as-user"
 	defaultRunAsGroupKey       = "default-run-as-group"
 	DefaultServiceAccountValue = "default"
-	defaultRunAsUserValue      = 65532
-	defaultRunAsGroupValue     = 65532
+	defaultRunAsUserValue      = "65532"
+	defaultRunAsGroupValue     = "65532"
 )
 
 // Defaults holds the default configurations
 // +k8s:deepcopy-gen=true
 type Defaults struct {
 	DefaultServiceAccount string
-	DefaultRunAsUser      int64
-	DefaultRunAsGroup     int64
+	DefaultRunAsUser      string
+	DefaultRunAsGroup     string
 }
 
 // GetDefaultsConfigName returns the name of the configmap containing all
@@ -78,27 +76,11 @@ func NewDefaultsFromMap(cfgMap map[string]string) (*Defaults, error) {
 	}
 
 	if defaultRunAsUser, ok := cfgMap[defaultRunAsUserKey]; ok {
-		if defaultRunAsUser == "" {
-			tc.DefaultRunAsUser = 0
-		} else {
-			runAsUser, err := strconv.ParseInt(defaultRunAsUser, 10, 0)
-			if err != nil {
-				return nil, fmt.Errorf("failed parsing runAsUser config %q", defaultRunAsUser)
-			}
-			tc.DefaultRunAsUser = runAsUser
-		}
+		tc.DefaultRunAsUser = defaultRunAsUser
 	}
 
 	if defaultRunAsGroup, ok := cfgMap[defaultRunAsGroupKey]; ok {
-		if defaultRunAsGroup == "" {
-			tc.DefaultRunAsGroup = 0
-		} else {
-			runAsGroup, err := strconv.ParseInt(defaultRunAsGroup, 10, 0)
-			if err != nil {
-				return nil, fmt.Errorf("failed parsing runAsUser config %q", defaultRunAsGroup)
-			}
-			tc.DefaultRunAsGroup = runAsGroup
-		}
+		tc.DefaultRunAsGroup = defaultRunAsGroup
 	}
 
 	return &tc, nil

pkg/apis/config/default_test.go

@@ -36,8 +36,8 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
 		{
 			expectedConfig: &config.Defaults{
 				DefaultServiceAccount: "default",
-				DefaultRunAsUser:      65532,
-				DefaultRunAsGroup:     65532,
+				DefaultRunAsUser:      "65532",
+				DefaultRunAsGroup:     "65532",
 			},
 			fileName: config.GetDefaultsConfigName(),
 		},
@@ -56,8 +56,8 @@ func TestNewDefaultsFromEmptyConfigMap(t *testing.T) {
 	DefaultsConfigEmptyName := "config-defaults-empty"
 	expectedConfig := &config.Defaults{
 		DefaultServiceAccount: "default",
-		DefaultRunAsUser:      65532,
-		DefaultRunAsGroup:     65532,
+		DefaultRunAsUser:      "65532",
+		DefaultRunAsGroup:     "65532",
 	}
 	verifyConfigFileWithExpectedConfig(t, DefaultsConfigEmptyName, expectedConfig)
 }
@@ -66,8 +66,8 @@ func TestNewDefaultsFromConfigMapWithEmptyVal(t *testing.T) {
 	DefaultsConfigEmptyVal := "config-defaults-triggers-empty-val"
 	expectedConfig := &config.Defaults{
 		DefaultServiceAccount: "default",
-		DefaultRunAsUser:      0,
-		DefaultRunAsGroup:     0,
+		DefaultRunAsUser:      "",
+		DefaultRunAsGroup:     "",
 	}
 	verifyConfigFileWithExpectedConfig(t, DefaultsConfigEmptyVal, expectedConfig)
 }

pkg/reconciler/eventlistener/resources/container.go

@@ -17,6 +17,7 @@ limitations under the License.
 package resources
 
 import (
+	"errors"
 	"strconv"
 
 	"github.com/tektoncd/triggers/pkg/apis/config"
@@ -29,7 +30,7 @@ import (
 
 type ContainerOption func(*corev1.Container)
 
-func MakeContainer(el *v1beta1.EventListener, configAcc reconcilersource.ConfigAccessor, c Config, cfg *config.Config, opts ...ContainerOption) corev1.Container {
+func MakeContainer(el *v1beta1.EventListener, configAcc reconcilersource.ConfigAccessor, c Config, cfg *config.Config, opts ...ContainerOption) (corev1.Container, error) {
 	isMultiNS := false
 	if len(el.Spec.NamespaceSelector.MatchNames) != 0 {
 		isMultiNS = true
@@ -64,8 +65,20 @@ func MakeContainer(el *v1beta1.EventListener, configAcc reconcilersource.ConfigA
 		}
 	}
 
-	containerSecurityContext.RunAsUser = ptr.Int64(cfg.Defaults.DefaultRunAsUser)
-	containerSecurityContext.RunAsGroup = ptr.Int64(cfg.Defaults.DefaultRunAsGroup)
+	if cfg.Defaults.DefaultRunAsUser != "" {
+		runAsUser, err := strconv.ParseInt(cfg.Defaults.DefaultRunAsUser, 10, 0)
+		if err != nil {
+			return corev1.Container{}, errors.New("failed parsing runAsUser config default-run-as-user")
+		}
+		containerSecurityContext.RunAsUser = ptr.Int64(runAsUser)
+	}
+	if cfg.Defaults.DefaultRunAsGroup != "" {
+		runAsGroup, err := strconv.ParseInt(cfg.Defaults.DefaultRunAsGroup, 10, 0)
+		if err != nil {
+			return corev1.Container{}, errors.New("failed parsing runAsGroup config default-run-as-group")
+		}
+		containerSecurityContext.RunAsGroup = ptr.Int64(runAsGroup)
+	}
 
 	container := corev1.Container{
 		Name:  "event-listener",
@@ -111,5 +124,5 @@ func MakeContainer(el *v1beta1.EventListener, configAcc reconcilersource.ConfigA
 		opt(&container)
 	}
 
-	return container
+	return container, nil
 }

pkg/reconciler/eventlistener/resources/container_test.go

@@ -483,7 +483,10 @@ func TestContainer(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := MakeContainer(tt.el, &reconcilersource.EmptyVarsGenerator{}, config, cfg.FromContextOrDefaults(context.Background()), tt.opts...)
+			got, err := MakeContainer(tt.el, &reconcilersource.EmptyVarsGenerator{}, config, cfg.FromContextOrDefaults(context.Background()), tt.opts...)
+			if err != nil {
+				t.Error(err)
+			}
 			if diff := cmp.Diff(tt.want, got); diff != "" {
 				t.Errorf("MakeContainer() did not return expected. -want, +got: %s", diff)
 			}

pkg/reconciler/eventlistener/resources/custom.go

@@ -48,7 +48,7 @@ func MakeCustomObject(ctx context.Context, el *v1beta1.EventListener, configAcc
 		namespace = el.GetNamespace()
 	}
 
-	container := MakeContainer(el, configAcc, c, cfg, func(c *corev1.Container) {
+	container, err := MakeContainer(el, configAcc, c, cfg, func(c *corev1.Container) {
 		// handle env and resources for custom object
 		if len(original.Spec.Template.Spec.Containers) == 1 {
 			c.Env = append(c.Env, original.Spec.Template.Spec.Containers[0].Env...)
@@ -77,6 +77,9 @@ func MakeCustomObject(ctx context.Context, el *v1beta1.EventListener, configAcc
 			SuccessThreshold: 1,
 		}
 	})
+	if err != nil {
+		return nil, err
+	}
 
 	podlabels := kmeta.UnionMaps(FilterLabels(ctx, el.Labels), GenerateLabels(el.Name, c.StaticResourceLabels))
 

pkg/reconciler/eventlistener/resources/deployment.go

@@ -47,7 +47,10 @@ func MakeDeployment(ctx context.Context, el *v1beta1.EventListener, configAcc re
 	if err != nil {
 		return nil, err
 	}
-	container := MakeContainer(el, configAcc, c, cfg, opt, addCertsForSecureConnection(c))
+	container, err := MakeContainer(el, configAcc, c, cfg, opt, addCertsForSecureConnection(c))
+	if err != nil {
+		return nil, err
+	}
 
 	filteredLabels := FilterLabels(ctx, el.Labels)
 

pkg/reconciler/eventlistener/resources/deployment_test.go

@@ -43,6 +43,27 @@ func TestDeployment(t *testing.T) {
 		"eventlistener":                eventListenerName,
 	}
 
+	cData, err := MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
+		cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
+		addCertsForSecureConnection(config))
+	if err != nil {
+		t.Error(err)
+	}
+
+	cDataWithTLS, err := MakeContainer(makeEL(withTLSEnvFrom("Bill")), &reconcilersource.EmptyVarsGenerator{}, config,
+		cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(withTLSEnvFrom("Bill")), config),
+		addCertsForSecureConnection(config))
+	if err != nil {
+		t.Error(err)
+	}
+
+	cDataWithProbes, err := MakeContainer(makeEL(setProbes()), &reconcilersource.EmptyVarsGenerator{}, config,
+		cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(setProbes()), config),
+		addCertsForSecureConnection(config))
+	if err != nil {
+		t.Error(err)
+	}
+
 	tests := []struct {
 		name string
 		el   *v1beta1.EventListener
@@ -67,12 +88,8 @@ func TestDeployment(t *testing.T) {
 					},
 					Spec: corev1.PodSpec{
 						ServiceAccountName: "sa",
-						Containers: []corev1.Container{
-							MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
-								cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
-								addCertsForSecureConnection(config)),
-						},
-						SecurityContext: &strongerSecurityPolicy,
+						Containers:         []corev1.Container{cData},
+						SecurityContext:    &strongerSecurityPolicy,
 					},
 				},
 			},
@@ -102,12 +119,8 @@ func TestDeployment(t *testing.T) {
 					},
 					Spec: corev1.PodSpec{
 						ServiceAccountName: "sa",
-						Containers: []corev1.Container{
-							MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
-								cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
-								addCertsForSecureConnection(config)),
-						},
-						SecurityContext: &strongerSecurityPolicy,
+						Containers:         []corev1.Container{cData},
+						SecurityContext:    &strongerSecurityPolicy,
 					},
 				},
 			},
@@ -145,12 +158,8 @@ func TestDeployment(t *testing.T) {
 					},
 					Spec: corev1.PodSpec{
 						ServiceAccountName: "sa",
-						Containers: []corev1.Container{
-							MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
-								cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
-								addCertsForSecureConnection(config)),
-						},
-						SecurityContext: &strongerSecurityPolicy,
+						Containers:         []corev1.Container{cData},
+						SecurityContext:    &strongerSecurityPolicy,
 						Tolerations: []corev1.Toleration{{
 							Key:   "foo",
 							Value: "bar",
@@ -191,12 +200,8 @@ func TestDeployment(t *testing.T) {
 					},
 					Spec: corev1.PodSpec{
 						ServiceAccountName: "sa",
-						Containers: []corev1.Container{
-							MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
-								cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
-								addCertsForSecureConnection(config)),
-						},
-						SecurityContext: &strongerSecurityPolicy,
+						Containers:         []corev1.Container{cData},
+						SecurityContext:    &strongerSecurityPolicy,
 						NodeSelector: map[string]string{
 							"foo": "bar",
 						},
@@ -234,12 +239,8 @@ func TestDeployment(t *testing.T) {
 					},
 					Spec: corev1.PodSpec{
 						ServiceAccountName: "bob",
-						Containers: []corev1.Container{
-							MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
-								cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
-								addCertsForSecureConnection(config)),
-						},
-						SecurityContext: &strongerSecurityPolicy,
+						Containers:         []corev1.Container{cData},
+						SecurityContext:    &strongerSecurityPolicy,
 					},
 				},
 			},
@@ -264,11 +265,7 @@ func TestDeployment(t *testing.T) {
 					},
 					Spec: corev1.PodSpec{
 						ServiceAccountName: "sa",
-						Containers: []corev1.Container{
-							MakeContainer(makeEL(withTLSEnvFrom("Bill")), &reconcilersource.EmptyVarsGenerator{}, config,
-								cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(withTLSEnvFrom("Bill")), config),
-								addCertsForSecureConnection(config)),
-						},
+						Containers:         []corev1.Container{cDataWithTLS},
 						Volumes: []corev1.Volume{{
 							Name: "https-connection",
 							VolumeSource: corev1.VolumeSource{
@@ -318,11 +315,7 @@ func TestDeployment(t *testing.T) {
 						TopologySpreadConstraints: []corev1.TopologySpreadConstraint{{
 							MaxSkew: 1,
 						}},
-						Containers: []corev1.Container{
-							MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
-								cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
-								addCertsForSecureConnection(config)),
-						},
+						Containers:      []corev1.Container{cData},
 						SecurityContext: &strongerSecurityPolicy,
 					},
 				},
@@ -348,12 +341,8 @@ func TestDeployment(t *testing.T) {
 					},
 					Spec: corev1.PodSpec{
 						ServiceAccountName: "sa",
-						Containers: []corev1.Container{
-							MakeContainer(makeEL(setProbes()), &reconcilersource.EmptyVarsGenerator{}, config,
-								cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(setProbes()), config),
-								addCertsForSecureConnection(config)),
-						},
-						SecurityContext: &strongerSecurityPolicy,
+						Containers:         []corev1.Container{cDataWithProbes},
+						SecurityContext:    &strongerSecurityPolicy,
 					},
 				},
 			},