Skip to content

Commit

Permalink
feat: use fsGroup (#188)
Browse files Browse the repository at this point in the history
* feat: use fsGroup

Signed-off-by: Smuu <[email protected]>

* feat: improve readability

Co-authored-by: Nguyen Nhu Viet <[email protected]>

* feat: dont handle error, as it cant fail

Signed-off-by: Smuu <[email protected]>

* doc: explain code

Signed-off-by: Smuu <[email protected]>

---------

Signed-off-by: Smuu <[email protected]>
Co-authored-by: Nguyen Nhu Viet <[email protected]>
  • Loading branch information
smuu and Bidon15 authored Sep 19, 2023
1 parent 4b61e27 commit 287c841
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 7 deletions.
7 changes: 7 additions & 0 deletions pkg/k8s/k8s_pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ type PodConfig struct {
Name string // Name to assign to the Pod
Labels map[string]string // Labels to apply to the Pod
ServiceAccountName string // ServiceAccount to assign to Pod
FsGroup int64 // FSGroup to apply to the Pod
ContainerConfig ContainerConfig // ContainerConfig for the Pod
SidecarConfigs []ContainerConfig // SideCarConfigs for the Pod
}
Expand Down Expand Up @@ -494,6 +495,11 @@ func preparePodVolumes(config ContainerConfig) ([]v1.Volume, error) {
func preparePodSpec(spec PodConfig, init bool) (v1.PodSpec, error) {
var err error

// Prepare security context
securityContext := v1.PodSecurityContext{
FSGroup: &spec.FsGroup,
}

// Prepare main container
mainContainer, err := prepareContainer(spec.ContainerConfig)
if err != nil {
Expand All @@ -514,6 +520,7 @@ func preparePodSpec(spec PodConfig, init bool) (v1.PodSpec, error) {

podSpec := v1.PodSpec{
ServiceAccountName: spec.ServiceAccountName,
SecurityContext: &securityContext,
InitContainers: initContainers,
Containers: []v1.Container{mainContainer},
Volumes: podVolumes,
Expand Down
52 changes: 45 additions & 7 deletions pkg/knuu/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"io"
"os"
"path/filepath"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -43,6 +44,7 @@ type Instance struct {
isSidecar bool
parentInstance *Instance
sidecars []*Instance
fsGroup int64
}

// NewInstance creates a new instance of the Instance struct
Expand Down Expand Up @@ -162,7 +164,10 @@ func (i *Instance) AddPortTCP(port int) error {
if !i.IsInState(Preparing, Committed) {
return fmt.Errorf("adding port is only allowed in state 'Preparing' or 'Committed'. Current state is '%s'", i.state.String())
}
validatePort(port)
err := validatePort(port)
if err != nil {
return err
}
if i.isTCPPortRegistered(port) {
return fmt.Errorf("TCP port '%d' is already in registered", port)
}
Expand All @@ -177,7 +182,10 @@ func (i *Instance) PortForwardTCP(port int) (int, error) {
if !i.IsInState(Started) {
return -1, fmt.Errorf("random port forwarding is only allowed in state 'Started'. Current state is '%s'", i.state.String())
}
validatePort(port)
err := validatePort(port)
if err != nil {
return 0, err
}
if !i.isTCPPortRegistered(port) {
return -1, fmt.Errorf("TCP port '%d' is not registered", port)
}
Expand Down Expand Up @@ -214,7 +222,10 @@ func (i *Instance) AddPortUDP(port int) error {
if !i.IsInState(Preparing, Committed) {
return fmt.Errorf("adding port is only allowed in state 'Preparing' or 'Committed'. Current state is '%s'", i.state.String())
}
validatePort(port)
err := validatePort(port)
if err != nil {
return err
}
if i.isUDPPortRegistered(port) {
return fmt.Errorf("UDP port '%d' is already in registered", port)
}
Expand Down Expand Up @@ -280,7 +291,10 @@ func (i *Instance) AddFile(src string, dest string, chown string) error {
return err
}

i.validateFileArgs(src, dest, chown)
err := i.validateFileArgs(src, dest, chown)
if err != nil {
return err
}

// check if src exists (either as file or as folder)
if _, err := os.Stat(src); os.IsNotExist(err) {
Expand All @@ -291,7 +305,7 @@ func (i *Instance) AddFile(src string, dest string, chown string) error {
dstPath := filepath.Join(i.getBuildDir(), dest)

// make sure dir exists
err := os.MkdirAll(filepath.Dir(dstPath), os.ModePerm)
err = os.MkdirAll(filepath.Dir(dstPath), os.ModePerm)
if err != nil {
return fmt.Errorf("error creating directory: %w", err)
}
Expand All @@ -317,7 +331,10 @@ func (i *Instance) AddFile(src string, dest string, chown string) error {

switch i.state {
case Preparing:
i.addFileToBuilder(src, dest, chown)
err := i.addFileToBuilder(src, dest, chown)
if err != nil {
return err
}
case Committed:
// only allow files, not folders
srcInfo, err := os.Stat(src)
Expand All @@ -326,6 +343,24 @@ func (i *Instance) AddFile(src string, dest string, chown string) error {
}
file := k8s.NewFile(dstPath, dest)

// the user provided a chown string (e.g. "10001:10001") and we only need the group (second part)
parts := strings.Split(chown, ":")
if len(parts) != 2 {
return fmt.Errorf("invalid format")
}

// second part of array, base of number is 10, and we want a 64-bit integer
group, err := strconv.ParseInt(parts[1], 10, 64)
if err != nil {
return fmt.Errorf("failed to convert to int64: %s", err)
}

if i.fsGroup != 0 && i.fsGroup != group {
return fmt.Errorf("all files must have the same group")
} else {
i.fsGroup = group
}

i.files = append(i.files, file)
}

Expand Down Expand Up @@ -498,7 +533,10 @@ func (i *Instance) SetEnvironmentVariable(key, value string) error {
return fmt.Errorf("setting environment variable is only allowed in state 'Preparing' or 'Committed'. Current state is '%s'", i.state.String())
}
if i.state == Preparing {
i.builderFactory.SetEnvVar(key, value)
err := i.builderFactory.SetEnvVar(key, value)
if err != nil {
return err
}
} else if i.state == Committed {
i.env[key] = value
}
Expand Down
1 change: 1 addition & 0 deletions pkg/knuu/instance_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ func (i *Instance) prepareStatefulSetConfig() k8s.StatefulSetConfig {
Name: i.k8sName,
Labels: i.getLabels(),
ServiceAccountName: i.k8sName,
FsGroup: i.fsGroup,
ContainerConfig: containerConfig,
SidecarConfigs: sidecarConfigs,
}
Expand Down

0 comments on commit 287c841

Please sign in to comment.