Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add security field to Dogu-CR and generate SecurityContext #222

Open
wants to merge 25 commits into
base: develop
Choose a base branch
from

Conversation

jelemux
Copy link
Contributor

@jelemux jelemux commented Dec 13, 2024

Resolves #221

@jelemux
Copy link
Contributor Author

jelemux commented Dec 13, 2024

Still WIP, do not merge

@@ -42,6 +47,16 @@ const (
DoguLabelVersion = "dogu.version"
)

// AllCapabilities are all possible values for capabilities.
var AllCapabilities = func() []Capability {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This var would fit better in the cesapp-lib

// AppArmorProfile defines a pod or container's AppArmor settings.
// +union
type AppArmorProfile struct {
// type indicates which kind of AppArmor profile will be applied.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// type indicates which kind of AppArmor profile will be applied.
// Type indicates which kind of AppArmor profile will be applied.

// +unionDiscriminator
Type AppArmorProfileType `json:"type"`

// localhostProfile indicates a profile loaded on the node that should be used.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// localhostProfile indicates a profile loaded on the node that should be used.
// LocalhostProfile indicates a profile loaded on the node that should be used.

}{
{"valid empty", args{&Dogu{}}, assert.NoError},
{"valid add filled", args{&Dogu{Spec: DoguSpec{Security: Security{Capabilities: Capabilities{Add: []Capability{core.AuditControl}}}}}}, assert.NoError},
{"valid add filled", args{&Dogu{Spec: DoguSpec{Security: Security{Capabilities: Capabilities{Drop: []Capability{core.AuditControl}}}}}}, assert.NoError},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{"valid add filled", args{&Dogu{Spec: DoguSpec{Security: Security{Capabilities: Capabilities{Drop: []Capability{core.AuditControl}}}}}}, assert.NoError},
{"valid drop filled", args{&Dogu{Spec: DoguSpec{Security: Security{Capabilities: Capabilities{Drop: []Capability{core.AuditControl}}}}}}, assert.NoError},

seccompProfile := seccompProfile(doguResource.Spec.Security.SeccompProfile)

readOnlyRootFS := isReadOnlyRootFS(dogu, doguResource)
// We never want those to be true and don't respect the dogu descriptor's privileged flag which is deprecated anyway.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍 This flag was only used by the registrator dogu which is no longer needed.

SecurityContext: &corev1.SecurityContext{
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
Add: []corev1.Capability{"CHOWN"},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The init container also needs DAC_OVERRIDE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add (Pod)SecurityContext to Dogus-Deployments
3 participants