Skip to content

Commit

Permalink
Fix privileged steps in kubernetes (woodpecker-ci#3711)
Browse files Browse the repository at this point in the history
  • Loading branch information
anbraten authored and 6543 committed Sep 5, 2024
1 parent ae9cee2 commit 723a7b3
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 15 deletions.
12 changes: 12 additions & 0 deletions pipeline/backend/kubernetes/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,19 @@ func containerSecurityContext(sc *SecurityContext, stepPrivileged bool) *v1.Secu
return nil
}

privileged := false

// if security context privileged is set explicitly
if sc != nil && sc.Privileged != nil && *sc.Privileged {
privileged = true
}

// if security context privileged is not set explicitly, but step is privileged
if (sc == nil || sc.Privileged == nil) && stepPrivileged {
privileged = true
}

if privileged {
securityContext := &v1.SecurityContext{
Privileged: newBool(true),
}
Expand Down
10 changes: 8 additions & 2 deletions pipeline/backend/kubernetes/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -406,15 +406,21 @@ func TestPodPrivilege(t *testing.T) {
}
pod, err = createTestPod(true, false, secCtx)
assert.NoError(t, err)
assert.Equal(t, true, *pod.Spec.Containers[0].SecurityContext.Privileged)
assert.True(t, *pod.Spec.Containers[0].SecurityContext.Privileged)

// step is privileged and no security context is provided
secCtx = SecurityContext{}
pod, err = createTestPod(true, false, secCtx)
assert.NoError(t, err)
assert.True(t, *pod.Spec.Containers[0].SecurityContext.Privileged)

// global runAsNonRoot is true and override is requested value by security context
secCtx = SecurityContext{
RunAsNonRoot: newBool(false),
}
pod, err = createTestPod(false, true, secCtx)
assert.NoError(t, err)
assert.Equal(t, true, *pod.Spec.SecurityContext.RunAsNonRoot)
assert.True(t, *pod.Spec.SecurityContext.RunAsNonRoot)
}

func TestScratchPod(t *testing.T) {
Expand Down
37 changes: 37 additions & 0 deletions pipeline/frontend/yaml/compiler/compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,3 +342,40 @@ func TestSecretMatch(t *testing.T) {
})
}
}

func TestCompilerCompilePrivileged(t *testing.T) {
compiler := New(
WithEscalated("test/image"),
)

fronConf := &yaml_types.Workflow{
SkipClone: true,
Steps: yaml_types.ContainerList{
ContainerList: []*yaml_types.Container{
{
Name: "privileged-plugin",
Image: "test/image",
DependsOn: []string{}, // no dependencies => enable dag mode & all steps are executed in parallel
},
{
Name: "no-plugin",
Image: "test/image",
Commands: []string{"echo 'i am not a plugin anymore'"},
},
{
Name: "not-privileged-image",
Image: "some/other-image",
},
},
},
}

backConf, err := compiler.Compile(fronConf)
assert.NoError(t, err)

assert.Len(t, backConf.Stages, 1)
assert.Len(t, backConf.Stages[0].Steps, 3)
assert.True(t, backConf.Stages[0].Steps[0].Privileged)
assert.False(t, backConf.Stages[0].Steps[1].Privileged)
assert.False(t, backConf.Stages[0].Steps[2].Privileged)
}
32 changes: 19 additions & 13 deletions pipeline/frontend/yaml/linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,40 +148,46 @@ func (l *Linter) lintCommands(config *WorkflowConfig, c *types.Container, field

func (l *Linter) lintTrusted(config *WorkflowConfig, c *types.Container, area string) error {
yamlPath := fmt.Sprintf("%s.%s", area, c.Name)
err := ""
errors := []string{}
if c.Privileged {
err = "Insufficient privileges to use privileged mode"
errors = append(errors, "Insufficient privileges to use privileged mode")
}
if c.ShmSize != 0 {
err = "Insufficient privileges to override shm_size"
errors = append(errors, "Insufficient privileges to override shm_size")
}
if len(c.DNS) != 0 {
err = "Insufficient privileges to use custom dns"
errors = append(errors, "Insufficient privileges to use custom dns")
}
if len(c.DNSSearch) != 0 {
err = "Insufficient privileges to use dns_search"
errors = append(errors, "Insufficient privileges to use dns_search")
}
if len(c.Devices) != 0 {
err = "Insufficient privileges to use devices"
errors = append(errors, "Insufficient privileges to use devices")
}
if len(c.ExtraHosts) != 0 {
err = "Insufficient privileges to use extra_hosts"
errors = append(errors, "Insufficient privileges to use extra_hosts")
}
if len(c.NetworkMode) != 0 {
err = "Insufficient privileges to use network_mode"
errors = append(errors, "Insufficient privileges to use network_mode")
}
if c.Networks.Networks != nil && len(c.Networks.Networks) != 0 {
err = "Insufficient privileges to use networks"
errors = append(errors, "Insufficient privileges to use networks")
}
if c.Volumes.Volumes != nil && len(c.Volumes.Volumes) != 0 {
err = "Insufficient privileges to use volumes"
errors = append(errors, "Insufficient privileges to use volumes")
}
if len(c.Tmpfs) != 0 {
err = "Insufficient privileges to use tmpfs"
errors = append(errors, "Insufficient privileges to use tmpfs")
}

if len(err) != 0 {
return newLinterError(err, config.File, yamlPath, false)
if len(errors) > 0 {
var err error

for _, e := range errors {
err = multierr.Append(err, newLinterError(e, config.File, yamlPath, false))
}

return err
}

return nil
Expand Down

0 comments on commit 723a7b3

Please sign in to comment.