Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
eebfecf
apply chmod after mkdir for directories in --data-dir
nvanthao Jan 22, 2025
c5e7097
address code reviews
nvanthao Jan 23, 2025
d115dba
add more dir
nvanthao Jan 23, 2025
1d31f87
update os.MkdirAll in all places
nvanthao Feb 5, 2025
841e54a
Merge remote-tracking branch 'origin/main' into gerard/sc-118372/chmod
laverya Feb 5, 2025
67f728a
potentially fix airgap upgrade job
laverya Feb 5, 2025
1bb7a9f
attempt setting umask 0077 in CI
laverya Feb 5, 2025
2ddbf46
use bash to run umask
laverya Feb 5, 2025
c9fe8c3
remove e2e test in favor of future dryrun test
laverya Feb 5, 2025
13ebcf3
set restrictive umask before running a dryrun test
laverya Feb 6, 2025
6563885
f
laverya Feb 6, 2025
59bfc2d
test that this does not pass without new code
laverya Feb 6, 2025
98efb3c
properly set umask
laverya Feb 6, 2025
a06c81b
return to using new code
laverya Feb 6, 2025
80af595
octal prints, no new code
laverya Feb 6, 2025
646db33
reenable new code
laverya Feb 6, 2025
67ba616
check all folders even after failure
laverya Feb 6, 2025
e5d2a27
update test to match behavior
laverya Feb 6, 2025
acec990
Merge remote-tracking branch 'origin/main' into gerard/sc-118372/chmod
laverya Feb 10, 2025
fbc4e04
set umask in root command
laverya Feb 10, 2025
79db89b
return to os.MkdirAll
laverya Feb 10, 2025
048b806
printf debugging
laverya Feb 11, 2025
c5b087f
test without setting umask to 077 first
laverya Feb 11, 2025
1d7ba6a
check if the setting actually stuck
laverya Feb 11, 2025
83b1132
call umask in each prerun function
laverya Feb 11, 2025
ce907b8
remove debug logs
laverya Feb 11, 2025
fc169e7
check that the kubectl-preflight binary has appropriate permissions
laverya Feb 12, 2025
15d4856
set umask in main function, improve comments
laverya Feb 17, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cmd/installer/cli/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"runtime"
"sort"
"strings"
"syscall"
"time"

"github.com/gosimple/slug"
Expand Down Expand Up @@ -171,6 +172,10 @@ func preRunInstall(cmd *cobra.Command, flags *InstallCmdFlags) error {
return fmt.Errorf("install command must be run as root")
}

// set the umask to 022 so that we can create files/directories with 755 permissions
// this does not return an error - it returns the previous umask
_ = syscall.Umask(0o022)
Copy link
Member

Choose a reason for hiding this comment

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

can you at the very least logrus.Debug this error?

Copy link
Member

Choose a reason for hiding this comment

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

it doesn't return an error, it returns the previous umask value


p, err := parseProxyFlags(cmd)
if err != nil {
return err
Expand Down
5 changes: 5 additions & 0 deletions cmd/installer/cli/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"os"
"strings"
"syscall"

ecv1beta1 "github.com/replicatedhq/embedded-cluster/kinds/apis/v1beta1"
"github.com/replicatedhq/embedded-cluster/pkg/addons"
Expand Down Expand Up @@ -96,6 +97,10 @@ func preRunJoin(flags *JoinCmdFlags) error {

flags.isAirgap = flags.airgapBundle != ""

// set the umask to 022 so that we can create files/directories with 755 permissions
// this does not return an error - it returns the previous umask
_ = syscall.Umask(0o022)
Copy link
Member

Choose a reason for hiding this comment

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

logrus.Debug the error


return nil
}

Expand Down
5 changes: 5 additions & 0 deletions cmd/installer/cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"os"
"syscall"

"github.com/replicatedhq/embedded-cluster/pkg/dryrun"
"github.com/replicatedhq/embedded-cluster/pkg/metrics"
Expand Down Expand Up @@ -68,6 +69,10 @@ func RootCmd(ctx context.Context, name string) *cobra.Command {
metrics.DisableMetrics()
}

// set the umask to 022 so that we can create files/directories with 755 permissions
// this does not return an error - it returns the previous umask
_ = syscall.Umask(0o022)
Copy link
Member

Choose a reason for hiding this comment

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

i assume having this in root isn't sufficient, can you please add a comment to the other ones explaining why?

also, are there no other commands that generate dirs and files other than install, join, and restore? what about materialize? isn't it used by LAM? should we set umask for materialize and LAM cli commands as well? operator cli?

Copy link
Member

Choose a reason for hiding this comment

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

I'll add it to the various 'main' functions, and try to come up with a better comment for it

Copy link
Member

Choose a reason for hiding this comment

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

logrus.Debug the error

Copy link
Member

Choose a reason for hiding this comment

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

The return value is not an error. Its the previous umask.


return nil
},
PersistentPostRunE: func(cmd *cobra.Command, args []string) error {
Expand Down
6 changes: 6 additions & 0 deletions cmd/installer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"os"
"path"
"syscall"

"github.com/mattn/go-isatty"
"github.com/replicatedhq/embedded-cluster/cmd/installer/cli"
Expand All @@ -19,5 +20,10 @@ func main() {

name := path.Base(os.Args[0])

// set the umask to 022 so that we can create files/directories with 755 permissions
// this does not return an error - it returns the previous umask
// we do this before calling cli.InitAndExecute so that it is set before the process forks
_ = syscall.Umask(0o022)

cli.InitAndExecute(ctx, name)
}
5 changes: 5 additions & 0 deletions cmd/local-artifact-mirror/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ func main() {

name := path.Base(os.Args[0])

// set the umask to 022 so that we can create files/directories with 755 permissions
// this does not return an error - it returns the previous umask
// we do this before calling cli.InitAndExecute so that it is set before the process forks
_ = syscall.Umask(0o022)

InitAndExecute(ctx, name)
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/replicatedhq/embedded-cluster

go 1.23.2
go 1.23.4

require (
github.com/AlecAivazis/survey/v2 v2.3.7
Expand Down
2 changes: 1 addition & 1 deletion operator/pkg/cli/upgrade_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func UpgradeJobCmd() *cobra.Command {

airgapChartsPath := ""
if in.Spec.AirGap {
airgapChartsPath = runtimeconfig.EmbeddedClusterChartsSubDir()
airgapChartsPath = runtimeconfig.EmbeddedClusterChartsSubDirNoCreate()
}

hcli, err := helm.NewClient(helm.HelmOptions{
Expand Down
2 changes: 1 addition & 1 deletion tests/dryrun/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM golang:1.23-alpine AS build
FROM golang:1.23.4-alpine AS build

RUN apk add --no-cache ca-certificates curl git make bash

Expand Down
42 changes: 40 additions & 2 deletions tests/dryrun/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,26 @@ import (
"os"
"path/filepath"
"regexp"
"syscall"
"testing"
"time"

"github.com/replicatedhq/embedded-cluster/pkg/dryrun"
"github.com/replicatedhq/embedded-cluster/pkg/helm"
"github.com/replicatedhq/embedded-cluster/pkg/kubeutils"
"github.com/replicatedhq/embedded-cluster/pkg/runtimeconfig"
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

func TestDefaultInstallation(t *testing.T) {
testDefaultInstallationImpl(t)
t.Logf("%s: test complete", time.Now().Format(time.RFC3339))
}

func testDefaultInstallationImpl(t *testing.T) {
hcli := &helm.MockClient{}

mock.InOrder(
Expand Down Expand Up @@ -152,8 +159,6 @@ func TestDefaultInstallation(t *testing.T) {
assert.Equal(t, "10.244.0.0/17", k0sConfig.Spec.Network.PodCIDR)
assert.Equal(t, "10.244.128.0/17", k0sConfig.Spec.Network.ServiceCIDR)
assert.Contains(t, k0sConfig.Spec.API.SANs, "kubernetes.default.svc.cluster.local")

t.Logf("%s: test complete", time.Now().Format(time.RFC3339))
}

func TestCustomDataDir(t *testing.T) {
Expand Down Expand Up @@ -391,3 +396,36 @@ func TestConfigValuesInstallation(t *testing.T) {

t.Logf("%s: test complete", time.Now().Format(time.RFC3339))
}

func TestRestrictiveUmask(t *testing.T) {
oldUmask := syscall.Umask(0o077)
defer syscall.Umask(oldUmask)

testDefaultInstallationImpl(t)

// check that folders created in this test have the right permissions
folderList := []string{
runtimeconfig.EmbeddedClusterHomeDirectory(),
runtimeconfig.EmbeddedClusterBinsSubDir(),
runtimeconfig.EmbeddedClusterChartsSubDir(),
runtimeconfig.PathToEmbeddedClusterBinary("kubectl-preflight"),
}
gotFailure := false
for _, folder := range folderList {
stat, err := os.Stat(folder)
if err != nil {
t.Logf("failed to stat %s: %v", folder, err)
gotFailure = true
continue
}
if stat.Mode().Perm() != 0755 {
t.Logf("expected folder %s to have mode 0755, got %O", folder, stat.Mode().Perm())
gotFailure = true
}
}
if gotFailure {
t.Fatalf("at least one folder had incorrect permissions")
}

t.Logf("%s: test complete", time.Now().Format(time.RFC3339))
}
Loading