Skip to content

Commit

Permalink
Refactor to handle conversion at config level
Browse files Browse the repository at this point in the history
  • Loading branch information
@savitaashture @tekton-robot
savitaashture authored and tekton-robot committed Jul 8, 2024
1 parent 7797f41 commit 87e123e
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 77 deletions.
36 changes: 29 additions & 7 deletions pkg/apis/config/default.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,22 @@ const (
defaultRunAsGroupKey = "default-run-as-group"
defaultRunAsNonRootKey = "default-run-as-non-root"
DefaultServiceAccountValue = "default"
defaultRunAsUserValue = "65532"
defaultRunAsGroupValue = "65532"
defaultRunAsUserValue = 65532
defaultRunAsGroupValue = 65532
defaultRunAsNonRootValue = true
)

// Defaults holds the default configurations
// +k8s:deepcopy-gen=true
type Defaults struct {
DefaultServiceAccount string
DefaultRunAsUser string
DefaultRunAsGroup string
DefaultRunAsUser int64
DefaultRunAsGroup int64
DefaultRunAsNonRoot bool
// These two fields are used to decide whether to configure
// runAsUser and runAsGroup within a Security Context Constraint (SCC).
IsDefaultRunAsUserEmpty bool
IsDefaultRunAsGroupEmpty bool
}

// GetDefaultsConfigName returns the name of the configmap containing all
Expand Down Expand Up @@ -83,18 +87,36 @@ func NewDefaultsFromMap(cfgMap map[string]string) (*Defaults, error) {
}

if defaultRunAsUser, ok := cfgMap[defaultRunAsUserKey]; ok {
tc.DefaultRunAsUser = defaultRunAsUser
if defaultRunAsUser != "" {
runAsUser, err := strconv.ParseInt(defaultRunAsUser, 10, 0)
if err != nil {
return nil, fmt.Errorf("failed parsing runAsUser config %q", defaultRunAsUser)
}
tc.DefaultRunAsUser = runAsUser
} else {
// if runAsUser is "" don't set runAsUser in SCC
tc.IsDefaultRunAsUserEmpty = true
}
}

if defaultRunAsGroup, ok := cfgMap[defaultRunAsGroupKey]; ok {
tc.DefaultRunAsGroup = defaultRunAsGroup
if defaultRunAsGroup != "" {
runAsGroup, err := strconv.ParseInt(defaultRunAsGroup, 10, 0)
if err != nil {
return nil, fmt.Errorf("failed parsing runAsGroup config %q", defaultRunAsGroup)
}
tc.DefaultRunAsGroup = runAsGroup
} else {
// if runAsGroup is "" don't set runAsGroup in SCC
tc.IsDefaultRunAsGroupEmpty = true
}
}

if defaultRunAsNonRoot, ok := cfgMap[defaultRunAsNonRootKey]; ok {
if defaultRunAsNonRoot != "" {
runAsNonRoot, err := strconv.ParseBool(defaultRunAsNonRoot)
if err != nil {
return nil, fmt.Errorf("failed parsing runAsGroup config %v", defaultRunAsNonRoot)
return nil, fmt.Errorf("failed parsing runAsNonRoot config %q", defaultRunAsNonRoot)
}
tc.DefaultRunAsNonRoot = runAsNonRoot
} else {
Expand Down
18 changes: 10 additions & 8 deletions pkg/apis/config/default_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
{
expectedConfig: &config.Defaults{
DefaultServiceAccount: "default",
DefaultRunAsUser: "65532",
DefaultRunAsGroup: "65532",
DefaultRunAsUser: 65532,
DefaultRunAsGroup: 65532,
DefaultRunAsNonRoot: true,
},
fileName: config.GetDefaultsConfigName(),
Expand All @@ -57,8 +57,8 @@ func TestNewDefaultsFromEmptyConfigMap(t *testing.T) {
DefaultsConfigEmptyName := "config-defaults-empty"
expectedConfig := &config.Defaults{
DefaultServiceAccount: "default",
DefaultRunAsUser: "65532",
DefaultRunAsGroup: "65532",
DefaultRunAsUser: 65532,
DefaultRunAsGroup: 65532,
DefaultRunAsNonRoot: true,
}
verifyConfigFileWithExpectedConfig(t, DefaultsConfigEmptyName, expectedConfig)
Expand All @@ -67,10 +67,12 @@ func TestNewDefaultsFromEmptyConfigMap(t *testing.T) {
func TestNewDefaultsFromConfigMapWithEmptyVal(t *testing.T) {
DefaultsConfigEmptyVal := "config-defaults-triggers-empty-val"
expectedConfig := &config.Defaults{
DefaultServiceAccount: "default",
DefaultRunAsUser: "",
DefaultRunAsGroup: "",
DefaultRunAsNonRoot: true, // when empty value set from configmap we set back to default value for runAsNonRoot
DefaultServiceAccount: "default",
DefaultRunAsUser: 65532,
DefaultRunAsGroup: 65532,
DefaultRunAsNonRoot: true, // when empty value set from configmap we set back to default value for runAsNonRoot
IsDefaultRunAsUserEmpty: true,
IsDefaultRunAsGroupEmpty: true,
}
verifyConfigFileWithExpectedConfig(t, DefaultsConfigEmptyVal, expectedConfig)
}
Expand Down
21 changes: 6 additions & 15 deletions pkg/reconciler/eventlistener/resources/container.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package resources

import (
"errors"
"strconv"

"github.com/tektoncd/triggers/pkg/apis/config"
Expand All @@ -30,7 +29,7 @@ import (

type ContainerOption func(*corev1.Container)

func MakeContainer(el *v1beta1.EventListener, configAcc reconcilersource.ConfigAccessor, c Config, cfg *config.Config, opts ...ContainerOption) (corev1.Container, error) {
func MakeContainer(el *v1beta1.EventListener, configAcc reconcilersource.ConfigAccessor, c Config, cfg *config.Config, opts ...ContainerOption) corev1.Container {
isMultiNS := false
if len(el.Spec.NamespaceSelector.MatchNames) != 0 {
isMultiNS = true
Expand Down Expand Up @@ -65,19 +64,11 @@ func MakeContainer(el *v1beta1.EventListener, configAcc reconcilersource.ConfigA
}
}

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.IsDefaultRunAsUserEmpty {
containerSecurityContext.RunAsUser = ptr.Int64(cfg.Defaults.DefaultRunAsUser)
}
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)
if !cfg.Defaults.IsDefaultRunAsGroupEmpty {
containerSecurityContext.RunAsGroup = ptr.Int64(cfg.Defaults.DefaultRunAsGroup)
}

container := corev1.Container{
Expand Down Expand Up @@ -124,5 +115,5 @@ func MakeContainer(el *v1beta1.EventListener, configAcc reconcilersource.ConfigA
opt(&container)
}

return container, nil
return container
}
5 changes: 1 addition & 4 deletions pkg/reconciler/eventlistener/resources/container_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,10 +483,7 @@ func TestContainer(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := MakeContainer(tt.el, &reconcilersource.EmptyVarsGenerator{}, config, cfg.FromContextOrDefaults(context.Background()), tt.opts...)
if err != nil {
t.Error(err)
}
got := MakeContainer(tt.el, &reconcilersource.EmptyVarsGenerator{}, config, cfg.FromContextOrDefaults(context.Background()), tt.opts...)
if diff := cmp.Diff(tt.want, got); diff != "" {
t.Errorf("MakeContainer() did not return expected. -want, +got: %s", diff)
}
Expand Down
5 changes: 1 addition & 4 deletions pkg/reconciler/eventlistener/resources/custom.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func MakeCustomObject(ctx context.Context, el *v1beta1.EventListener, configAcc
namespace = el.GetNamespace()
}

container, err := MakeContainer(el, configAcc, c, cfg, func(c *corev1.Container) {
container := 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...)
Expand Down Expand Up @@ -77,9 +77,6 @@ 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))

Expand Down
5 changes: 1 addition & 4 deletions pkg/reconciler/eventlistener/resources/deployment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,7 @@ func MakeDeployment(ctx context.Context, el *v1beta1.EventListener, configAcc re
if err != nil {
return nil, err
}
container, err := MakeContainer(el, configAcc, c, cfg, opt, addCertsForSecureConnection(c))
if err != nil {
return nil, err
}
container := MakeContainer(el, configAcc, c, cfg, opt, addCertsForSecureConnection(c))

filteredLabels := FilterLabels(ctx, el.Labels)

Expand Down
81 changes: 46 additions & 35 deletions pkg/reconciler/eventlistener/resources/deployment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,27 +43,6 @@ 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
Expand All @@ -88,8 +67,12 @@ func TestDeployment(t *testing.T) {
},
Spec: corev1.PodSpec{
ServiceAccountName: "sa",
Containers: []corev1.Container{cData},
SecurityContext: &strongerSecurityPolicy,
Containers: []corev1.Container{
MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
addCertsForSecureConnection(config)),
},
SecurityContext: &strongerSecurityPolicy,
},
},
},
Expand Down Expand Up @@ -119,8 +102,12 @@ func TestDeployment(t *testing.T) {
},
Spec: corev1.PodSpec{
ServiceAccountName: "sa",
Containers: []corev1.Container{cData},
SecurityContext: &strongerSecurityPolicy,
Containers: []corev1.Container{
MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
addCertsForSecureConnection(config)),
},
SecurityContext: &strongerSecurityPolicy,
},
},
},
Expand Down Expand Up @@ -158,8 +145,12 @@ func TestDeployment(t *testing.T) {
},
Spec: corev1.PodSpec{
ServiceAccountName: "sa",
Containers: []corev1.Container{cData},
SecurityContext: &strongerSecurityPolicy,
Containers: []corev1.Container{
MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
addCertsForSecureConnection(config)),
},
SecurityContext: &strongerSecurityPolicy,
Tolerations: []corev1.Toleration{{
Key: "foo",
Value: "bar",
Expand Down Expand Up @@ -200,8 +191,12 @@ func TestDeployment(t *testing.T) {
},
Spec: corev1.PodSpec{
ServiceAccountName: "sa",
Containers: []corev1.Container{cData},
SecurityContext: &strongerSecurityPolicy,
Containers: []corev1.Container{
MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
addCertsForSecureConnection(config)),
},
SecurityContext: &strongerSecurityPolicy,
NodeSelector: map[string]string{
"foo": "bar",
},
Expand Down Expand Up @@ -239,8 +234,12 @@ func TestDeployment(t *testing.T) {
},
Spec: corev1.PodSpec{
ServiceAccountName: "bob",
Containers: []corev1.Container{cData},
SecurityContext: &strongerSecurityPolicy,
Containers: []corev1.Container{
MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
addCertsForSecureConnection(config)),
},
SecurityContext: &strongerSecurityPolicy,
},
},
},
Expand All @@ -265,7 +264,11 @@ func TestDeployment(t *testing.T) {
},
Spec: corev1.PodSpec{
ServiceAccountName: "sa",
Containers: []corev1.Container{cDataWithTLS},
Containers: []corev1.Container{
MakeContainer(makeEL(withTLSEnvFrom("Bill")), &reconcilersource.EmptyVarsGenerator{}, config,
cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(withTLSEnvFrom("Bill")), config),
addCertsForSecureConnection(config)),
},
Volumes: []corev1.Volume{{
Name: "https-connection",
VolumeSource: corev1.VolumeSource{
Expand Down Expand Up @@ -315,7 +318,11 @@ func TestDeployment(t *testing.T) {
TopologySpreadConstraints: []corev1.TopologySpreadConstraint{{
MaxSkew: 1,
}},
Containers: []corev1.Container{cData},
Containers: []corev1.Container{
MakeContainer(makeEL(), &reconcilersource.EmptyVarsGenerator{}, config,
cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(), config),
addCertsForSecureConnection(config)),
},
SecurityContext: &strongerSecurityPolicy,
},
},
Expand All @@ -341,8 +348,12 @@ func TestDeployment(t *testing.T) {
},
Spec: corev1.PodSpec{
ServiceAccountName: "sa",
Containers: []corev1.Container{cDataWithProbes},
SecurityContext: &strongerSecurityPolicy,
Containers: []corev1.Container{
MakeContainer(makeEL(setProbes()), &reconcilersource.EmptyVarsGenerator{}, config,
cfg.FromContextOrDefaults(context.Background()), mustAddDeployBits(t, makeEL(setProbes()), config),
addCertsForSecureConnection(config)),
},
SecurityContext: &strongerSecurityPolicy,
},
},
},
Expand Down

0 comments on commit 87e123e

Please sign in to comment.