From 2eee229a8f93cf26526f5c85edf2479ce2e9eaa9 Mon Sep 17 00:00:00 2001 From: Prashant Jaikumar Date: Fri, 8 Dec 2023 00:15:26 -0800 Subject: [PATCH] Make mount permissions of secrets configurable When default.mode.secret is set in CDAP config, the corresponding value (interpreted as decimal) will be used as the default mode for mounting CDAP secrets. --- controllers/constants.go | 12 ++++++---- controllers/deployment.go | 30 ++++++++++++++++++++++-- controllers/spec.go | 16 +++++++++++++ controllers/testdata/appfabric.json | 2 +- controllers/testdata/artifactcache.json | 2 ++ controllers/testdata/authentication.json | 1 + controllers/testdata/router.json | 3 ++- controllers/testdata/supportbundle.json | 2 ++ controllers/testdata/tetheringagent.json | 2 ++ templates/cdap-deployment.yaml | 2 ++ templates/cdap-sts.yaml | 2 ++ webhooks/cdap_webhook.go | 4 ++-- 12 files changed, 67 insertions(+), 11 deletions(-) diff --git a/controllers/constants.go b/controllers/constants.go index 2e8cab85..ee2876a4 100644 --- a/controllers/constants.go +++ b/controllers/constants.go @@ -64,13 +64,15 @@ const ( confTwillSecurityWorkerSecretDiskName = "twill.security.worker.secret.disk.name" confTwillSecurityWorkerSecretDiskPath = "twill.security.worker.secret.disk.path" confJMXServerPort = "jmx.metrics.collector.server.port" + confSecretMountDefaultMode = "secret.mount.default.mode" // default values - defaultImage = "gcr.io/cdapio/cdap:latest" - defaultRouterPort = 11015 - defaultUserInterfacePort = 11011 - defaultStorageSize = "200Gi" - defaultSecuritySecretPath = "/etc/cdap/security" + defaultImage = "gcr.io/cdapio/cdap:latest" + defaultRouterPort = 11015 + defaultUserInterfacePort = 11011 + defaultStorageSize = "200Gi" + defaultSecuritySecretPath = "/etc/cdap/security" + defaultSecretMountDefaultMode = 420 // kubernetes labels labelInstanceKey = "cdap.instance" diff --git a/controllers/deployment.go b/controllers/deployment.go index d6292c82..b8310b4a 100644 --- a/controllers/deployment.go +++ b/controllers/deployment.go @@ -4,6 +4,7 @@ import ( "fmt" "reflect" "sort" + "strconv" "strings" "cdap.io/cdap-operator/api/v1alpha1" @@ -172,6 +173,11 @@ func buildStatefulSets(master *v1alpha1.CDAPMaster, name string, services Servic return nil, err } + defaultMode, err := getSecretMountDefaultMode(master) + if err != nil { + return nil, err + } + spec := newStatefulSpec(master, objName, labels, cconf, hconf, sysappconf). setServiceAccountName(serviceAccount). setNodeSelector(nodeSelector). @@ -179,7 +185,8 @@ func buildStatefulSets(master *v1alpha1.CDAPMaster, name string, services Servic setPriorityClassName(priorityClass). setSecurityContext(securityContext). setReplicas(replicas). - setAffinity(affinity) + setAffinity(affinity). + setSecretMountDefaultMode(defaultMode) // Add init container spec = spec.withInitContainer( @@ -317,6 +324,11 @@ func buildDeployment(master *v1alpha1.CDAPMaster, name string, services ServiceG return nil, err } + defaultMode, err := getSecretMountDefaultMode(master) + if err != nil { + return nil, err + } + spec := newDeploymentSpec(master, objName, labels, cconf, hconf, sysappconf). setServiceAccountName(serviceAccount). setNodeSelector(nodeSelector). @@ -324,7 +336,8 @@ func buildDeployment(master *v1alpha1.CDAPMaster, name string, services ServiceG setPriorityClassName(priorityClass). setReplicas(replicas). setSecurityContext(securityContext). - setAffinity(affinity) + setAffinity(affinity). + setSecretMountDefaultMode(defaultMode) // Add each service as a container for _, s := range services { @@ -703,6 +716,19 @@ func getAffinity(master *v1alpha1.CDAPMaster, services ServiceGroup) (*corev1.Af return nil, fmt.Errorf("unable to cast value of type %T into Affinity", val) } +// Return the default secret mount permissions. +func getSecretMountDefaultMode(master *v1alpha1.CDAPMaster) (int32, error) { + val, ok := master.Spec.Config[confSecretMountDefaultMode] + if !ok { + return defaultSecretMountDefaultMode, nil + } + i, err := strconv.ParseInt(val, 10, 32) + if err != nil { + return 0, err + } + return int32(i), nil +} + // getReplicas returns the Replicas if all supplied services have the same setting, otherwise return an error func getReplicas(master *v1alpha1.CDAPMaster, services ServiceGroup) (int32, error) { replicas := int32(0) diff --git a/controllers/spec.go b/controllers/spec.go index c3efbd04..49150582 100644 --- a/controllers/spec.go +++ b/controllers/spec.go @@ -147,6 +147,7 @@ type BaseSpec struct { AdditionalVolumeMounts []corev1.VolumeMount `json:"additionalVolumeMounts,omitempty"` SecurityContext *v1alpha1.SecurityContext `json:"securityContext,omitempty"` Affinity *corev1.Affinity `json:"affinity,omitemtpy"` + SecretMountDefaultMode int32 `json:"secretMountDefaultSecret,omitemtpy"` } func newBaseSpec(master *v1alpha1.CDAPMaster, name string, labels map[string]string, cconf, hconf, sysappconf string) *BaseSpec { @@ -180,6 +181,11 @@ func (s *BaseSpec) setAffinity(affinity *corev1.Affinity) *BaseSpec { return s } +func (s *BaseSpec) setSecretMountDefaultMode(mode int32) *BaseSpec { + s.SecretMountDefaultMode = mode + return s +} + func (s *BaseSpec) setReplicas(replicas int32) *BaseSpec { s.Replicas = replicas return s @@ -369,6 +375,11 @@ func (s *DeploymentSpec) setAffinity(affinity *corev1.Affinity) *DeploymentSpec return s } +func (s *DeploymentSpec) setSecretMountDefaultMode(mode int32) *DeploymentSpec { + s.Base.setSecretMountDefaultMode(mode) + return s +} + // For VolumnClaimTemplate in Statefulset type StorageSpec struct { StorageClassName string `json:"storageClassName,omitempty"` @@ -411,6 +422,11 @@ func (s *StatefulSpec) setAffinity(affinity *corev1.Affinity) *StatefulSpec { return s } +func (s *StatefulSpec) setSecretMountDefaultMode(mode int32) *StatefulSpec { + s.Base.setSecretMountDefaultMode(mode) + return s +} + func (s *StatefulSpec) setReplicas(replicas int32) *StatefulSpec { s.Base.setReplicas(replicas) return s diff --git a/controllers/testdata/appfabric.json b/controllers/testdata/appfabric.json index 71c85a09..234d842d 100644 --- a/controllers/testdata/appfabric.json +++ b/controllers/testdata/appfabric.json @@ -407,4 +407,4 @@ "replicas": 0, "updatedReplicas": 1 } -} \ No newline at end of file +} diff --git a/controllers/testdata/artifactcache.json b/controllers/testdata/artifactcache.json index 54a31877..d0c849a0 100644 --- a/controllers/testdata/artifactcache.json +++ b/controllers/testdata/artifactcache.json @@ -102,6 +102,7 @@ { "name":"cdap-security", "secret":{ + "defaultMode": 420, "secretName":"cdap-secret" } }, @@ -120,6 +121,7 @@ { "name":"cdap-se-vol-my-secret-1", "secret":{ + "defaultMode": 420, "secretName":"my-secret-1" } }, diff --git a/controllers/testdata/authentication.json b/controllers/testdata/authentication.json index 53f5b581..bfc45f8c 100644 --- a/controllers/testdata/authentication.json +++ b/controllers/testdata/authentication.json @@ -240,6 +240,7 @@ { "name": "cdap-se-vol-secret-key", "secret": { + "defaultMode": 420, "secretName": "secret-key" } }, diff --git a/controllers/testdata/router.json b/controllers/testdata/router.json index ef6248fa..7085ebf0 100644 --- a/controllers/testdata/router.json +++ b/controllers/testdata/router.json @@ -260,6 +260,7 @@ { "name": "cdap-se-vol-secret-key", "secret": { + "defaultMode": 420, "secretName": "secret-key" } }, @@ -319,4 +320,4 @@ "replicas": 2, "updatedReplicas": 2 } -} \ No newline at end of file +} diff --git a/controllers/testdata/supportbundle.json b/controllers/testdata/supportbundle.json index f0a032f6..29fd4824 100644 --- a/controllers/testdata/supportbundle.json +++ b/controllers/testdata/supportbundle.json @@ -102,6 +102,7 @@ { "name":"cdap-security", "secret":{ + "defaultMode": 420, "secretName":"cdap-secret" } }, @@ -120,6 +121,7 @@ { "name":"cdap-se-vol-my-secret-1", "secret":{ + "defaultMode": 420, "secretName":"my-secret-1" } }, diff --git a/controllers/testdata/tetheringagent.json b/controllers/testdata/tetheringagent.json index e928172f..6a983fe9 100644 --- a/controllers/testdata/tetheringagent.json +++ b/controllers/testdata/tetheringagent.json @@ -102,6 +102,7 @@ { "name":"cdap-security", "secret":{ + "defaultMode": 420, "secretName":"cdap-secret" } }, @@ -120,6 +121,7 @@ { "name":"cdap-se-vol-my-secret-1", "secret":{ + "defaultMode": 420, "secretName":"my-secret-1" } }, diff --git a/templates/cdap-deployment.yaml b/templates/cdap-deployment.yaml index a4ed238a..b202b77b 100644 --- a/templates/cdap-deployment.yaml +++ b/templates/cdap-deployment.yaml @@ -164,6 +164,7 @@ spec: {{if .Base.SecuritySecret}} - name: cdap-security secret: + defaultMode: {{$.Base.SecretMountDefaultMode}} secretName: {{.Base.SecuritySecret}} {{end}} {{range $k,$v := $.Base.ConfigMapVolumes}} @@ -174,5 +175,6 @@ spec: {{range $k,$v := $.Base.SecretVolumes}} - name: cdap-se-vol-{{$k}} secret: + defaultMode: {{$.Base.SecretMountDefaultMode}} secretName: {{$k}} {{end}} diff --git a/templates/cdap-sts.yaml b/templates/cdap-sts.yaml index 9e62abc1..6c77d101 100644 --- a/templates/cdap-sts.yaml +++ b/templates/cdap-sts.yaml @@ -204,6 +204,7 @@ spec: {{if $.Base.SecuritySecret}} - name: cdap-security secret: + defaultMode: {{$.Base.SecretMountDefaultMode}} secretName: {{$.Base.SecuritySecret}} {{end}} {{range $k,$v := $.Base.ConfigMapVolumes}} @@ -214,6 +215,7 @@ spec: {{range $k,$v := $.Base.SecretVolumes}} - name: cdap-se-vol-{{$k}} secret: + defaultMode: {{$.Base.SecretMountDefaultMode}} secretName: {{$k}} {{end}} volumeClaimTemplates: diff --git a/webhooks/cdap_webhook.go b/webhooks/cdap_webhook.go index 6005186d..2a2229d1 100644 --- a/webhooks/cdap_webhook.go +++ b/webhooks/cdap_webhook.go @@ -17,9 +17,9 @@ import ( ) const ( - labelInstanceKey = "cdap.instance" + labelInstanceKey = "cdap.instance" // cdapMasterNamespaceKey is the label added by CDAP on pods launched by CDAP. - cdapMasterNamespaceKey = "cdap.k8s.namespace" + cdapMasterNamespaceKey = "cdap.k8s.namespace" // customResourceNamespaceKey is the label added by controller-reconciler on resources // managed by the CDAP operator. customResourceNamespaceKey = "custom-resource-namespace"