Skip to content
Open
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
13 changes: 13 additions & 0 deletions manifests/03-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,17 @@ rules:
verbs:
- get
- list
- apiGroups:
- "apps"
resources:
- daemonsets
verbs:
- get
- list
- watch
- create
- delete
- update
---
Comment on lines +129 to 140
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Scope DaemonSet write permissions to namespace-level Role (least privilege).

Granting create/delete/update on apps/daemonsets in a ClusterRole allows mutating DaemonSets cluster-wide. The runtime-extractor controller manages a namespaced DaemonSet (openshift-insights), so this should be moved to the existing namespaced Role in openshift-insights.

🔧 Suggested RBAC tightening
-  - apiGroups:
-      - "apps"
-    resources:
-      - daemonsets
-    verbs:
-      - get
-      - list
-      - watch
-      - create
-      - delete
-      - update
 # In Role `insights-operator` (namespace: openshift-insights), add:
+  - apiGroups:
+      - apps
+    resources:
+      - daemonsets
+    verbs:
+      - get
+      - list
+      - watch
+      - create
+      - delete
+      - update
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@manifests/03-clusterrole.yaml` around lines 129 - 140, The ClusterRole
currently grants create/delete/update on apiGroups "apps" resources "daemonsets"
which is overbroad; remove the mutating verbs (create, delete, update) for
apps/daemonsets from the ClusterRole and leave only read verbs (get, list,
watch) there, then add those mutating verbs to the existing namespaced Role that
targets the openshift-insights namespace (the namespaced Role that the
runtime-extractor controller uses), and update/confirm the RoleBinding so the
controller's ServiceAccount is bound to that namespaced Role (ensuring the
controller retains get/list/watch at cluster or namespace scope as intended and
only gains create/delete/update within openshift-insights).

apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
Expand Down Expand Up @@ -526,6 +537,8 @@ rules:
- events
verbs:
- create
- patch
- update
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
Expand Down
144 changes: 75 additions & 69 deletions manifests/06-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,75 +33,81 @@ spec:
kubernetes.io/os: linux
node-role.kubernetes.io/master: ""
tolerations:
- effect: NoSchedule
key: node-role.kubernetes.io/master
operator: Exists
- effect: NoExecute
key: node.kubernetes.io/unreachable
operator: Exists
tolerationSeconds: 900
- effect: NoExecute
key: node.kubernetes.io/not-ready
operator: Exists
tolerationSeconds: 900
- effect: NoSchedule
key: node-role.kubernetes.io/master
operator: Exists
- effect: NoExecute
key: node.kubernetes.io/unreachable
operator: Exists
tolerationSeconds: 900
- effect: NoExecute
key: node.kubernetes.io/not-ready
operator: Exists
tolerationSeconds: 900
volumes:
- emptyDir: {}
name: tmp
- name: snapshots
emptyDir: {}
#sizeLimit: 1Gi # bug https://bugzilla.redhat.com/show_bug.cgi?id=1713207
- name: trusted-ca-bundle
configMap:
name: trusted-ca-bundle
optional: true
- name: service-ca-bundle
configMap:
name: service-ca-bundle
optional: true
- name: serving-cert
secret:
secretName: openshift-insights-serving-cert
optional: true
containers:
- name: insights-operator
securityContext:
readOnlyRootFilesystem: true
allowPrivilegeEscalation: false
capabilities:
drop: ["ALL"]
image: quay.io/openshift/origin-insights-operator:latest
terminationMessagePolicy: FallbackToLogsOnError
volumeMounts:
- mountPath: /tmp
- emptyDir: {}
name: tmp
- name: snapshots
mountPath: /var/lib/insights-operator
- mountPath: /var/run/configmaps/trusted-ca-bundle
name: trusted-ca-bundle
readOnly: true
- mountPath: /var/run/configmaps/service-ca-bundle
name: service-ca-bundle
readOnly: true
- mountPath: /var/run/secrets/serving-cert
name: serving-cert
ports:
- containerPort: 8443
name: metrics
resources:
requests:
cpu: 10m
memory: 54Mi
env:
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
- name: POD_NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
- name: RELEASE_VERSION
value: "0.0.1-snapshot"
args:
- start
- --config=/etc/insights-operator/server.yaml
emptyDir: {}
#sizeLimit: 1Gi # bug https://bugzilla.redhat.com/show_bug.cgi?id=1713207
- name: trusted-ca-bundle
configMap:
name: trusted-ca-bundle
optional: true
- name: service-ca-bundle
configMap:
name: service-ca-bundle
optional: true
- name: serving-cert
secret:
secretName: openshift-insights-serving-cert
optional: true
containers:
- name: insights-operator
securityContext:
readOnlyRootFilesystem: true
allowPrivilegeEscalation: false
capabilities:
drop: ["ALL"]
image: quay.io/openshift/origin-insights-operator:latest
terminationMessagePolicy: FallbackToLogsOnError
volumeMounts:
- mountPath: /tmp
name: tmp
- name: snapshots
mountPath: /var/lib/insights-operator
- mountPath: /var/run/configmaps/trusted-ca-bundle
name: trusted-ca-bundle
readOnly: true
- mountPath: /var/run/configmaps/service-ca-bundle
name: service-ca-bundle
readOnly: true
- mountPath: /var/run/secrets/serving-cert
name: serving-cert
ports:
- containerPort: 8443
name: metrics
resources:
requests:
cpu: 10m
memory: 54Mi
env:
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
- name: POD_NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
- name: RELEASE_VERSION
value: "0.0.1-snapshot"
- name: RELATED_IMAGE_INSIGHTS_RUNTIME_EXTRACTOR
value: quay.io/openshift/origin-insights-runtime-extractor:latest
- name: RELATED_IMAGE_INSIGHTS_RUNTIME_EXPORTER
value: quay.io/openshift/origin-insights-runtime-exporter:latest
- name: RELATED_IMAGE_KUBE_RBAC_PROXY
value: quay.io/openshift/origin-kube-rbac-proxy:latest
args:
- start
- --config=/etc/insights-operator/server.yaml
75 changes: 58 additions & 17 deletions pkg/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/openshift/insights-operator/pkg/config"
"github.com/openshift/insights-operator/pkg/config/configobserver"
"github.com/openshift/insights-operator/pkg/controller/periodic"
"github.com/openshift/insights-operator/pkg/controller/runtimeextractor"
"github.com/openshift/insights-operator/pkg/controller/status"
"github.com/openshift/insights-operator/pkg/gather"
"github.com/openshift/insights-operator/pkg/insights"
Expand Down Expand Up @@ -126,6 +127,54 @@ func (s *Operator) Run(ctx context.Context, controller *controllercmd.Controller
desiredVersion = envVersion
}

kubeInf := v1helpers.NewKubeInformersForNamespaces(kubeClient, "openshift-insights")
configMapObserver, err := configobserver.NewConfigMapObserver(ctx, gatherKubeConfig, controller.EventRecorder, kubeInf)
if err != nil {
return err
}
go kubeInf.Start(ctx.Done())
go configMapObserver.Run(ctx, 1)

// secretConfigObserver synthesizes all config into the status reporter controller
secretConfigObserver := configobserver.New(s.Controller, kubeClient)
go secretConfigObserver.Start(ctx)

configAggregator := configobserver.NewConfigAggregator(secretConfigObserver, configMapObserver)
go configAggregator.Listen(ctx)

// updateCh is used to signal a version update to the runtimeextractor controller
updateCh := make(chan struct{}, 1)

// Create informer factory for runtime-extractor resources in openshift-insights namespace
runtimeExtractorInformerFactory := clientInformers.NewSharedInformerFactoryWithOptions(
kubeClient,
informerTimeout,
clientInformers.WithNamespace(insightsNamespace),
)

// Create resource informer to watch for external modifications to runtime-extractor resources
runtimeExtractorResourceInformer, err := runtimeextractor.NewResourceInformer(
controller.EventRecorder,
runtimeExtractorInformerFactory,
)
if err != nil {
return fmt.Errorf("failed to create runtime extractor resource informer: %w", err)
}

// Start the informer factory and resource informer controller
go runtimeExtractorInformerFactory.Start(ctx.Done())
go runtimeExtractorResourceInformer.Run(ctx, 1)

// Start runtimeExtractor controller
runtimeExtractorCtrl := runtimeextractor.NewRuntimeExtractorController(
configAggregator,
updateCh,
kubeClient,
controller.EventRecorder,
runtimeExtractorResourceInformer,
)
go runtimeExtractorCtrl.Run(ctx)

// By default, this will exit(0) the process if the featuregates ever change to a different set of values.
featureGateAccessor := featuregates.NewFeatureGateAccess(
desiredVersion, missingVersion,
Expand Down Expand Up @@ -178,21 +227,6 @@ func (s *Operator) Run(ctx context.Context, controller *controllercmd.Controller
}
}

kubeInf := v1helpers.NewKubeInformersForNamespaces(kubeClient, "openshift-insights")
configMapObserver, err := configobserver.NewConfigMapObserver(ctx, gatherKubeConfig, controller.EventRecorder, kubeInf)
if err != nil {
return err
}
go kubeInf.Start(ctx.Done())
go configMapObserver.Run(ctx, 1)

// secretConfigObserver synthesizes all config into the status reporter controller
secretConfigObserver := configobserver.New(s.Controller, kubeClient)
go secretConfigObserver.Start(ctx)

configAggregator := configobserver.NewConfigAggregator(secretConfigObserver, configMapObserver)
go configAggregator.Listen(ctx)

// additional configurations may exist besides the default one
if customPath := getCustomStoragePath(configAggregator, nil); customPath != "" {
isValid, err := pathIsAvailable(customPath)
Expand All @@ -212,8 +246,15 @@ func (s *Operator) Run(ctx context.Context, controller *controllercmd.Controller

// the status controller initializes the cluster operator object and retrieves
// the last sync time, if any was set
statusReporter := status.NewController(configClient.ConfigV1(), configAggregator,
insightsDataGatherObserver, os.Getenv("POD_NAMESPACE"), insightsConfigEnabled, controller.EventRecorder)
statusReporter := status.NewController(
configClient.ConfigV1(),
configAggregator,
insightsDataGatherObserver,
os.Getenv("POD_NAMESPACE"),
insightsConfigEnabled,
controller.EventRecorder,
updateCh,
)

var anonymizer *anonymization.Anonymizer
var recdriver *diskrecorder.DiskRecorder
Expand Down
Loading