Skip to content
Draft
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
apiVersion: v1
kind: Pod
metadata:
name: kms-preflight-installer
namespace: {{.Namespace}}
labels:
app: openshift-kms-preflight-installer
spec:
automountServiceAccountToken: false
nodeSelector:
node-role.kubernetes.io/master: ""
tolerations:
- key: "node-role.kubernetes.io/master"
operator: "Exists"
effect: "NoSchedule"
- key: "node-role.kubernetes.io/master"
operator: "Exists"
effect: "NoExecute"
initContainers:
- name: cleanup
image: {{.InstallerImage}}
imagePullPolicy: IfNotPresent
terminationMessagePolicy: FallbackToLogsOnError
command:
- /bin/sh
- -c
- |
#!/bin/sh
set -euo pipefail

rm -f /etc/kubernetes/manifests/kms-preflight-pod.yaml
rm -rf /etc/kubernetes/manifests/secrets/kms-preflight-encryption-config
securityContext:
privileged: true
runAsUser: 0
Comment on lines +33 to +35

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files | rg 'pkg/operator/encryption/kms/preflight/assets/kms-preflight-installer-pod.yaml|pkg/operator/encryption/kms/preflight|kms-preflight'
printf '\n--- manifest ---\n'
cat -n pkg/operator/encryption/kms/preflight/assets/kms-preflight-installer-pod.yaml | sed -n '1,220p'
printf '\n--- references ---\n'
rg -n "kms-preflight-installer-pod|privileged: true|runAsUser: 0|allowPrivilegeEscalation|readOnlyRootFilesystem|capabilities:" pkg/operator/encryption/kms/preflight -S
printf '\n--- surrounding code ---\n'
fd -a . pkg/operator/encryption/kms/preflight

Repository: openshift/library-go

Length of output: 5957


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n--- pod_template.go ---\n'
cat -n pkg/operator/encryption/kms/preflight/pod_template.go | sed -n '1,240p'

printf '\n--- deployer.go ---\n'
cat -n pkg/operator/encryption/kms/preflight/deployer.go | sed -n '1,240p'

printf '\n--- deployer_test.go ---\n'
cat -n pkg/operator/encryption/kms/preflight/deployer_test.go | sed -n '1,220p'

printf '\n--- installer pod template tests ---\n'
cat -n pkg/operator/encryption/kms/preflight/pod_template_test.go | sed -n '1,260p'

Repository: openshift/library-go

Length of output: 27824


Installer pod still runs as root with full privilege

These host-write containers likely need elevated access, but the manifest still leaves them as privileged: true and runAsUser: 0 without any clear justification or extra hardening. If that access is required, document why it cannot be reduced; otherwise scope it down and add the missing securityContext settings where compatible.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@pkg/operator/encryption/kms/preflight/assets/kms-preflight-installer-pod.yaml`
around lines 33 - 35, The installer pod securityContext is still fully
privileged and runs as root without any additional hardening. Review the
installer pod manifest and the relevant securityContext block to either justify
the required elevated access or reduce it where possible; if privileged/root is
unavoidable, add the missing hardening settings in the same installer pod
definition (for example by tightening capabilities and related security options)
so the elevated privileges are explicitly scoped and documented.

Sources: Coding guidelines, Path instructions

resources:
requests:
memory: 10Mi
cpu: 10m
Comment on lines +37 to +39

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Missing resource limits on both containers.

Only requests are defined; no limits are set for either the cleanup init container or the installer container.

As per path instructions, "Resource limits (cpu, memory) on every container."

🔧 Proposed fix
       resources:
         requests:
           memory: 10Mi
           cpu: 10m
+        limits:
+          memory: 10Mi
+          cpu: 10m

Also applies to: 69-71

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@pkg/operator/encryption/kms/preflight/assets/kms-preflight-installer-pod.yaml`
around lines 37 - 39, The pod spec is missing resource limits for both
containers, so update the container definitions for cleanup and installer in
kms-preflight-installer-pod.yaml to include cpu and memory limits alongside the
existing requests. Ensure the limit settings are added consistently to each
container block so both the init container and main container satisfy the
resource policy.

Source: Path instructions

volumeMounts:
- mountPath: /etc/kubernetes/manifests
name: manifests-dir
containers:
- name: installer
image: {{.InstallerImage}}
imagePullPolicy: IfNotPresent
terminationMessagePolicy: FallbackToLogsOnError
command:
- /bin/sh
- -c
- |
#!/bin/sh
set -euo pipefail

target_secret_dir="/etc/kubernetes/manifests/secrets/kms-preflight-encryption-config"
mkdir -p "${target_secret_dir}"
cp -f /install/secret/* "${target_secret_dir}/"
cp -f /install/pod/lb-int.kubeconfig "${target_secret_dir}/lb-int.kubeconfig"

manifest_path="/etc/kubernetes/manifests/kms-preflight-pod.yaml"
mkdir -p /etc/kubernetes/manifests
cp -f /install/pod/pod.yaml "${manifest_path}"
chmod 0600 "${manifest_path}"
securityContext:
privileged: true
runAsUser: 0
resources:
requests:
memory: 10Mi
cpu: 10m
volumeMounts:
- mountPath: /etc/kubernetes/manifests
name: manifests-dir
- mountPath: /install/pod
name: pod-manifest
- mountPath: /install/secret
name: encryption-config
readOnly: true
restartPolicy: Never
priorityClassName: system-node-critical
securityContext:
runAsUser: 0
volumes:
- hostPath:
path: /etc/kubernetes/manifests
name: manifests-dir
- name: pod-manifest
configMap:
name: kms-preflight-pod
- name: encryption-config
secret:
secretName: kms-preflight-encryption-config
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ spec:
# This is a one-shot preflight check, not a long-running pod.
# The operator creates it, waits for completion, and inspects the result.
restartPolicy: Never
{{- if not .StaticPod}}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

serviceAccountName: kms-preflight
{{- end}}
priorityClassName: system-cluster-critical
nodeSelector:
node-role.kubernetes.io/master: ""
Expand All @@ -24,6 +26,13 @@ spec:
- key: "node-role.kubernetes.io/master"
operator: "Exists"
effect: "NoExecute"
{{- if .StaticPod}}
hostNetwork: true
volumes:
- name: resource-dir
hostPath:
path: /etc/kubernetes/manifests
{{- end}}
Comment thread
tjungblu marked this conversation as resolved.
containers:
- name: kms-preflight-check
image: {{.OperatorImage}}
Expand All @@ -32,8 +41,10 @@ spec:
- --kms-call-timeout={{.KMSCallTimeout}}
- --config-hash=$(CONFIG_HASH)
- --pod-name=$(POD_NAME)
- --pod-namespace=$(POD_NAMESPACE){{if .Kubeconfig}}
- --kubeconfig={{.Kubeconfig}}{{end}}
- --pod-namespace=$(POD_NAMESPACE)
{{- if .StaticPod}}
- --kubeconfig=/etc/kubernetes/static-pod-resources/secrets/kms-preflight-encryption-config/lb-int.kubeconfig
{{- end}}
env:
- name: POD_NAME
valueFrom: { fieldRef: { fieldPath: metadata.name } }
Expand All @@ -46,3 +57,11 @@ spec:
requests:
memory: 50Mi
cpu: 5m
{{- if .StaticPod}}
securityContext:
runAsUser: 0
volumeMounts:
- name: resource-dir
mountPath: /etc/kubernetes/static-pod-resources
readOnly: true
{{- end}}
Comment thread
tjungblu marked this conversation as resolved.
1 change: 0 additions & 1 deletion pkg/operator/encryption/kms/preflight/deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ func (d *PodPreflightDeployer) Deploy(ctx context.Context, configHash string, en
d.operatorImage,
d.operatorCommand,
d.kmsCallTimeout,
"",
)
if err != nil {
return fmt.Errorf("failed to generate preflight pod template: %w", err)
Expand Down
87 changes: 75 additions & 12 deletions pkg/operator/encryption/kms/preflight/pod_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,45 +12,108 @@ import (
"github.com/openshift/library-go/pkg/operator/resource/resourceread"
)

const (
preflightPodConfigMapPrefix = "kms-preflight-pod"
preflightInstallerPodName = "kms-preflight-installer"
)

type kmsPreflightTemplate struct {
PodName string
Namespace string
ConfigHash string
OperatorImage string
Command string
KMSCallTimeout string
Kubeconfig string
StaticPod bool
}

// generatePodTemplate renders the KMS preflight pod YAML template.
// The pod runs the preflight checker as a one-shot command and shares an emptyDir
// volume with the KMS plugin sidecar at /var/run/kmsplugin.
func generatePodTemplate(
func newKmsPreflightTemplate(
podName string,
namespace string,
configHash string,
operatorImage string,
operatorCommand []string,
kmsCallTimeout time.Duration,
kubeconfig string,
) (*corev1.Pod, error) {
rawManifest := mustAsset("assets/kms-preflight-pod.yaml")

staticPod bool,
) kmsPreflightTemplate {
operatorCommandQuoted := make([]string, len(operatorCommand))
for i, cmd := range operatorCommand {
operatorCommandQuoted[i] = fmt.Sprintf("%q", cmd)
}

tmplVal := kmsPreflightTemplate{
return kmsPreflightTemplate{
PodName: podName,
Namespace: namespace,
ConfigHash: configHash,
OperatorImage: operatorImage,
Command: strings.Join(operatorCommandQuoted, ","),
KMSCallTimeout: kmsCallTimeout.String(),
Kubeconfig: kubeconfig,
StaticPod: staticPod,
}
}

// generatePodTemplate renders the KMS preflight pod YAML template.
// The pod runs the preflight checker as a one-shot command and shares an emptyDir
// volume with the KMS plugin sidecar at /var/run/kmsplugin.
func generatePodTemplate(
podName string,
namespace string,
configHash string,
operatorImage string,
operatorCommand []string,
kmsCallTimeout time.Duration,
) (*corev1.Pod, error) {
return renderPreflightPodTemplate(podName, namespace, configHash, operatorImage, operatorCommand, kmsCallTimeout, false)
}

// generateStaticPodTemplate renders the KMS preflight static pod YAML template.
func generateStaticPodTemplate(
podName string,
namespace string,
configHash string,
operatorImage string,
operatorCommand []string,
kmsCallTimeout time.Duration,
) (*corev1.Pod, error) {
return renderPreflightPodTemplate(podName, namespace, configHash, operatorImage, operatorCommand, kmsCallTimeout, true)
}

func renderPreflightPodTemplate(
podName string,
namespace string,
configHash string,
operatorImage string,
operatorCommand []string,
kmsCallTimeout time.Duration,
staticPod bool,
) (*corev1.Pod, error) {
rawManifest := mustAsset("assets/kms-preflight-pod.yaml")
tmplVal := newKmsPreflightTemplate(
podName, namespace, configHash, operatorImage, operatorCommand, kmsCallTimeout, staticPod,
)
return renderPodTemplate("kms-preflight", string(rawManifest), tmplVal)
}

type kmsPreflightInstallerTemplate struct {
Namespace string
InstallerImage string
}

// generateInstallerPodTemplate renders the pod that installs the KMS preflight static pod
// manifest and encryption-config secret onto a control plane node.
func generateInstallerPodTemplate(namespace, operatorImage string) (*corev1.Pod, error) {
rawManifest := mustAsset("assets/kms-preflight-installer-pod.yaml")

tmplVal := kmsPreflightInstallerTemplate{
Namespace: namespace,
InstallerImage: operatorImage,
}
tmpl, err := template.New("kms-preflight").Parse(string(rawManifest))

return renderPodTemplate("kms-preflight-installer", string(rawManifest), tmplVal)
}

func renderPodTemplate(name, rawManifest string, tmplVal any) (*corev1.Pod, error) {
tmpl, err := template.New(name).Parse(rawManifest)
if err != nil {
return nil, err
}
Expand Down
Loading