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

feat: read or execute perms for files from configmap mounting #99

Merged
merged 3 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions apis/v1alpha1/topologyspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ type FileFromConfigMap struct {
// be mounted without a sub-path.
// +optional
ConfigMapPath string `json:"configMapPath"`
// Mode sets the file permissions when mounting the configmap. Since the configmap will be read
// only filesystem anyway, we basically just want to expose if the file should be mounted as
// executable or not. So, default permissions would be 0o444 (read) and execute would be 0o555.
// +kubebuilder:validation:Enum=read;execute
// +kubebuilder:default=read
Mode string `json:"mode"`
}

// FileFromURL represents a file that you would like to mount from a URL in the launcher pod for
Expand Down
27 changes: 21 additions & 6 deletions assets/crd/clabernetes.containerlab.dev_topologies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,20 @@ spec:
filePath:
description: FilePath is the path to mount the file.
type: string
mode:
default: read
description: |-
Mode sets the file permissions when mounting the configmap. Since the configmap will be read
only filesystem anyway, we basically just want to expose if the file should be mounted as
executable or not. So, default permissions would be 0o444 (read) and execute would be 0o555.
enum:
- read
- execute
type: string
required:
- configMapName
- filePath
- mode
type: object
type: array
description: |-
Expand Down Expand Up @@ -195,12 +206,16 @@ spec:
type: object
privilegedLauncher:
description: |-
PrivilegedLauncher, when true, sets the launcher containers to privileged. By default, we do
our best to *not* need this/set this, and instead set only the capabilities we need, however
its possible that some containers launched by the launcher may need/want more capabilities,
so this flag exists for users to bypass the default settings and enable fully privileged
launcher pods. If this value is unset, the global config value (default of "false") will be
used.
PrivilegedLauncher, when true, sets the launcher containers to privileged. Historically we
tried very hard to *not* need to set privileged mode on pods, however the reality is it is
much, much easier to get various network operating system images booting with this enabled,
so, the default mode is to set the privileged flag on pods. Disabling this option causes
clabernetes to try to run the pods for this topology in the "not so privileged" mode -- this
basically means we mount all capabilities we think should be available, set apparmor to
"unconfined", and mount paths like /dev/kvm and dev/net/tun. With this "not so privileged"
mode, Nokia SRL devices and Arista cEOS devices have been able to boot on some clusters, but
your mileage may vary. In short: if you don't care about having some privileged pods, just
leave this alone.
type: boolean
resources:
additionalProperties:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,20 @@ spec:
filePath:
description: FilePath is the path to mount the file.
type: string
mode:
default: read
description: |-
Mode sets the file permissions when mounting the configmap. Since the configmap will be read
only filesystem anyway, we basically just want to expose if the file should be mounted as
executable or not. So, default permissions would be 0o444 (read) and execute would be 0o555.
enum:
- read
- execute
type: string
required:
- configMapName
- filePath
- mode
type: object
type: array
description: |-
Expand Down Expand Up @@ -195,12 +206,16 @@ spec:
type: object
privilegedLauncher:
description: |-
PrivilegedLauncher, when true, sets the launcher containers to privileged. By default, we do
our best to *not* need this/set this, and instead set only the capabilities we need, however
its possible that some containers launched by the launcher may need/want more capabilities,
so this flag exists for users to bypass the default settings and enable fully privileged
launcher pods. If this value is unset, the global config value (default of "false") will be
used.
PrivilegedLauncher, when true, sets the launcher containers to privileged. Historically we
tried very hard to *not* need to set privileged mode on pods, however the reality is it is
much, much easier to get various network operating system images booting with this enabled,
so, the default mode is to set the privileged flag on pods. Disabling this option causes
clabernetes to try to run the pods for this topology in the "not so privileged" mode -- this
basically means we mount all capabilities we think should be available, set apparmor to
"unconfined", and mount paths like /dev/kvm and dev/net/tun. With this "not so privileged"
mode, Nokia SRL devices and Arista cEOS devices have been able to boot on some clusters, but
your mileage may vary. In short: if you don't care about having some privileged pods, just
leave this alone.
type: boolean
resources:
additionalProperties:
Expand Down
2 changes: 1 addition & 1 deletion charts/clicker/tests/default_vaules/default_values_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestDefaultValues(t *testing.T) {
actualRootDir := fmt.Sprintf("test-fixtures/%s-actual", testName)
actualDir := fmt.Sprintf("%s/clicker/templates", actualRootDir)

err := os.MkdirAll(actualDir, clabernetesconstants.PermissionsEveryoneRead)
err := os.MkdirAll(actualDir, clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute)
if err != nil {
t.Fatalf(
"failed creating actual output directory %q, error: %s", actualDir, err,
Expand Down
1 change: 1 addition & 0 deletions clabverter/assets/topology.yaml.template
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ spec:
- filePath: {{ $nodeFile.FilePath }}
configMapName: {{ $nodeFile.ConfigMapName }}
configMapPath: {{ $nodeFile.FileName }}
mode: read
{{- end }}
{{- end }}
{{- end }}
Expand Down
7 changes: 5 additions & 2 deletions clabverter/clabverter.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,10 @@ func (c *Clabverter) ensureOutputDirectory() error {
return err
}

err = os.MkdirAll(c.outputDirectory, clabernetesconstants.PermissionsEveryoneRead)
err = os.MkdirAll(
c.outputDirectory,
clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute,
)
if err != nil {
c.logger.Criticalf("failed ensuring output directory exists, error: %s", err)

Expand Down Expand Up @@ -407,7 +410,7 @@ func (c *Clabverter) output() error {
err := os.WriteFile(
rendered.fileName,
rendered.content,
clabernetesconstants.PermissionsEveryoneRead,
clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute,
)
if err != nil {
c.logger.Criticalf(
Expand Down
5 changes: 4 additions & 1 deletion clabverter/clabverter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ func TestClabvert(t *testing.T) {

actualDir := fmt.Sprintf("test-fixtures/%s-actual", testCase.name)

err := os.MkdirAll(actualDir, clabernetesconstants.PermissionsEveryoneRead)
err := os.MkdirAll(
actualDir,
clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute,
)
if err != nil {
t.Fatalf(
"failed creating actual output directory %q, error: %s", actualDir, err,
Expand Down
6 changes: 6 additions & 0 deletions clabverter/test-fixtures/golden/srl02.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,28 @@ spec:
- configMapName: srl02-srl1-startup-config
configMapPath: REPLACED
filePath: srl1.cfg
mode: read
- configMapName: srl02-srl1-files
configMapPath: REPLACED
filePath: taco/srl1.license
mode: read
srl2:
- configMapName: srl02-srl2-files
configMapPath: REPLACED
filePath: /some/dir/clabernetes/clabverter/test-fixtures/clabversiontest/potato.txt
mode: read
- configMapName: srl02-srl2-files
configMapPath: REPLACED
filePath: /some/dir/clabernetes/clabverter/test-fixtures/clabversiontest/srl2/potato.txt
mode: read
- configMapName: srl02-srl2-startup-config
configMapPath: REPLACED
filePath: srl2.cfg
mode: read
- configMapName: srl02-srl2-files
configMapPath: REPLACED
filePath: srl2.license
mode: read
filesFromURL: null
persistence:
enabled: false
Expand Down
7 changes: 7 additions & 0 deletions constants/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,11 @@ const (

// UDP is... UDP.
UDP = "UDP"

// Read is "read". Used for configmap mount permissions in the TopologySpec/FilesFromConfigMap.
Read = "read"

// Execute is "execute". Used for configmap mount permissions in the
// TopologySpec/FilesFromConfigMap.
Execute = "execute"
)
14 changes: 11 additions & 3 deletions constants/os.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,19 @@ const (
// read, write, and execute permissions.
PermissionsEveryoneAllPermissions = 0o777

// PermissionsEveryoneRead is 0755 permissions for files/directories -- everyone can read, and
// execute, and owner can write.
PermissionsEveryoneRead = 0o755
// PermissionsEveryoneReadWriteOwnerExecute is 0755 permissions for files/directories --
// everyone can read, and execute, and owner can write.
PermissionsEveryoneReadWriteOwnerExecute = 0o755

// PermissionsEveryoneReadWrite is 0666 permissions for files/directories -- everyone has read
// and write permissions.
PermissionsEveryoneReadWrite = 0o666

// PermissionsEveryoneReadExecute is 0555 permissions for files/directories -- everyone has read
// and execute permissions.
PermissionsEveryoneReadExecute = 0o555

// PermissionsEveryoneRead is 0444 permissions for files/directories -- everyone has read
// permissions.
PermissionsEveryoneRead = 0o444
)
48 changes: 32 additions & 16 deletions controllers/topology/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func (r *DeploymentReconciler) renderDeploymentVolumes(
Name: owningTopologyName,
},
DefaultMode: clabernetesutil.ToPointer(
int32(clabernetesconstants.PermissionsEveryoneRead),
int32(clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute),
),
},
},
Expand Down Expand Up @@ -243,7 +243,7 @@ func (r *DeploymentReconciler) renderDeploymentVolumes(
Secret: &k8scorev1.SecretVolumeSource{
SecretName: dockerDaemonConfigSecret,
DefaultMode: clabernetesutil.ToPointer(
int32(clabernetesconstants.PermissionsEveryoneRead),
int32(clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute),
),
},
},
Expand All @@ -268,27 +268,43 @@ func (r *DeploymentReconciler) renderDeploymentVolumes(
)

for _, podVolume := range volumesFromConfigMaps {
if !clabernetesutilkubernetes.VolumeAlreadyMounted(
volumeName := clabernetesutilkubernetes.SafeConcatNameKubernetes(
podVolume.ConfigMapName,
podVolume.ConfigMapPath,
)

var mode *int32

switch podVolume.Mode {
case clabernetesconstants.Read:
mode = clabernetesutil.ToPointer(
int32(clabernetesconstants.PermissionsEveryoneRead),
)
case clabernetesconstants.Execute:
mode = clabernetesutil.ToPointer(
int32(clabernetesconstants.PermissionsEveryoneReadExecute),
)
default:
mode = nil
}

volumes = append(
volumes,
) {
volumes = append(
volumes,
k8scorev1.Volume{
Name: podVolume.ConfigMapName,
VolumeSource: k8scorev1.VolumeSource{
ConfigMap: &k8scorev1.ConfigMapVolumeSource{
LocalObjectReference: k8scorev1.LocalObjectReference{
Name: podVolume.ConfigMapName,
},
k8scorev1.Volume{
Name: volumeName,
VolumeSource: k8scorev1.VolumeSource{
ConfigMap: &k8scorev1.ConfigMapVolumeSource{
LocalObjectReference: k8scorev1.LocalObjectReference{
Name: podVolume.ConfigMapName,
},
DefaultMode: mode,
},
},
)
}
},
)

volumeMount := k8scorev1.VolumeMount{
Name: podVolume.ConfigMapName,
Name: volumeName,
ReadOnly: false,
MountPath: fmt.Sprintf("/clabernetes/%s", podVolume.FilePath),
SubPath: podVolume.ConfigMapPath,
Expand Down
12 changes: 10 additions & 2 deletions generated/openapi/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion launcher/connectivity/slurpeeth.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func (m *slurpeethManager) renderSlurpeethConfig(
err = os.WriteFile(
slurpeethConfigPath,
slurpeethConfigYAML,
clabernetesconstants.PermissionsEveryoneRead,
clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute,
)
if err != nil {
m.logger.Fatalf(
Expand Down
2 changes: 1 addition & 1 deletion launcher/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (c *clabernetes) handleInsecureRegistries() error {
err = os.WriteFile(
dockerDaemonConfig,
rendered.Bytes(),
clabernetesconstants.PermissionsEveryoneRead,
clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute,
)
if err != nil {
return err
Expand Down
11 changes: 7 additions & 4 deletions manager/perparecertificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
func prepareCertificates(c clabernetesmanagertypes.Clabernetes) error {
clabernetesutil.MustCreateDirectory(
clabernetesconstants.CertificateDirectory,
clabernetesconstants.PermissionsEveryoneRead,
clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute,
)

caDirectory := ensureCaDirectory()
Expand Down Expand Up @@ -117,7 +117,10 @@ func ensureCaDirectory() string {
clabernetesconstants.CertificateAuthoritySubDir,
)

clabernetesutil.MustCreateDirectory(caDirectory, clabernetesconstants.PermissionsEveryoneRead)
clabernetesutil.MustCreateDirectory(
caDirectory,
clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute,
)

return caDirectory
}
Expand All @@ -131,7 +134,7 @@ func ensureClientCertDirectory() string {

clabernetesutil.MustCreateDirectory(
clientCertDirectory,
clabernetesconstants.PermissionsEveryoneRead,
clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute,
)

return clientCertDirectory
Expand All @@ -146,7 +149,7 @@ func ensureWebhookCertDirectory() string {

clabernetesutil.MustCreateDirectory(
webhookCertDirectory,
clabernetesconstants.PermissionsEveryoneRead,
clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute,
)

return webhookCertDirectory
Expand Down
2 changes: 1 addition & 1 deletion testhelper/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func WriteTestFixtureFile(t *testing.T, f string, b []byte) { //nolint:thelper
func WriteTestFile(t *testing.T, f string, b []byte) {
t.Helper()

err := os.WriteFile(f, b, clabernetesconstants.PermissionsEveryoneRead)
err := os.WriteFile(f, b, clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute)
if err != nil {
t.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion testhelper/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func HelmTest(t *testing.T, testName, namespace, valuesFileName string) {
}
}

err := os.MkdirAll(actualDir, clabernetesconstants.PermissionsEveryoneRead)
err := os.MkdirAll(actualDir, clabernetesconstants.PermissionsEveryoneReadWriteOwnerExecute)
if err != nil {
t.Fatalf(
"failed creating actual output directory %q, error: %s", actualDir, err,
Expand Down
Loading
Loading