Skip to content

Commit 87547b5

Browse files
ozzywalshozzywalsh
committed
fix(collector): default fsGroup from namespace range on OpenShift
On OpenShift, the restricted SCC injects fsGroup from the namespace range, but permissive SCCs (e.g. anyuid) do not. When a user grants privileges to the collector ServiceAccount, the effective SCC changes and PVC volumes become inaccessible due to missing group ownership. The controller now defaults podSecurityContext.fsGroup from the namespace's supplemental-groups annotation (falling back to uid-range) when running on OpenShift and no explicit fsGroup is configured. This follows the same pattern used by migtools/crane and istio/istio. An explicitly set fsGroup is never overwritten. Closes open-telemetry#5106
1 parent 48486ed commit 87547b5

3 files changed

Lines changed: 208 additions & 0 deletions

File tree

‎.chloggen/fix_fsgroup-openshift-vct.yaml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2+
change_type: bug_fix
3+
4+
# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
5+
component: collector
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: Collectors with persistent storage no longer fail with "permission denied" on OpenShift when running under a permissive SCC such as anyuid.
9+
10+
# One or more tracking issues related to the change
11+
issues: [5106]
12+
13+
# (Optional) One or more lines of additional information to render under the primary note.
14+
# These lines will be padded with 2 spaces and then inserted directly into the document.
15+
# Use pipe (|) for multiline entries.
16+
subtext: |
17+
On OpenShift, the restricted SCC injects fsGroup from the namespace range, but
18+
permissive SCCs (e.g. anyuid) do not. The controller now defaults
19+
podSecurityContext.fsGroup from the namespace's supplemental-groups or UID range
20+
annotation when no explicit fsGroup is configured. An explicitly set fsGroup is
21+
never overwritten.

‎internal/controllers/fsgroup_test.go‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package controllers
5+
6+
import (
7+
"context"
8+
"testing"
9+
10+
"github.com/go-logr/logr"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
corev1 "k8s.io/api/core/v1"
14+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15+
"k8s.io/utils/ptr"
16+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
17+
18+
"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
19+
"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift"
20+
"github.com/open-telemetry/opentelemetry-operator/internal/config"
21+
)
22+
23+
func TestDefaultFSGroupOnOpenShift(t *testing.T) {
24+
tests := []struct {
25+
name string
26+
isOpenShift bool
27+
nsAnnotations map[string]string
28+
existingFSGroup *int64
29+
wantFSGroup *int64
30+
}{
31+
{
32+
name: "OpenShift with UID range annotation",
33+
isOpenShift: true,
34+
nsAnnotations: map[string]string{"openshift.io/sa.scc.uid-range": "1000850000/10000"},
35+
wantFSGroup: ptr.To(int64(1000850000)),
36+
},
37+
{
38+
name: "OpenShift with explicit fsGroup preserves it",
39+
isOpenShift: true,
40+
nsAnnotations: map[string]string{"openshift.io/sa.scc.uid-range": "1000850000/10000"},
41+
existingFSGroup: ptr.To(int64(65532)),
42+
wantFSGroup: ptr.To(int64(65532)),
43+
},
44+
{
45+
name: "non-OpenShift does not set fsGroup",
46+
isOpenShift: false,
47+
nsAnnotations: map[string]string{"openshift.io/sa.scc.uid-range": "1000850000/10000"},
48+
wantFSGroup: nil,
49+
},
50+
{
51+
name: "OpenShift without annotation does not set fsGroup",
52+
isOpenShift: true,
53+
nsAnnotations: map[string]string{},
54+
wantFSGroup: nil,
55+
},
56+
{
57+
name: "OpenShift with malformed annotation does not set fsGroup",
58+
isOpenShift: true,
59+
nsAnnotations: map[string]string{"openshift.io/sa.scc.uid-range": "notanumber"},
60+
wantFSGroup: nil,
61+
},
62+
{
63+
name: "OpenShift with supplemental-groups annotation takes precedence",
64+
isOpenShift: true,
65+
nsAnnotations: map[string]string{
66+
"openshift.io/sa.scc.supplemental-groups": "1000900000/10000",
67+
"openshift.io/sa.scc.uid-range": "1000850000/10000",
68+
},
69+
wantFSGroup: ptr.To(int64(1000900000)),
70+
},
71+
{
72+
name: "OpenShift falls back to uid-range when supplemental-groups absent",
73+
isOpenShift: true,
74+
nsAnnotations: map[string]string{"openshift.io/sa.scc.uid-range": "1000850000/10000"},
75+
wantFSGroup: ptr.To(int64(1000850000)),
76+
},
77+
{
78+
name: "OpenShift with dash-format annotation",
79+
isOpenShift: true,
80+
nsAnnotations: map[string]string{"openshift.io/sa.scc.uid-range": "1000850000-1000860000"},
81+
wantFSGroup: ptr.To(int64(1000850000)),
82+
},
83+
}
84+
85+
for _, tt := range tests {
86+
t.Run(tt.name, func(t *testing.T) {
87+
ns := &corev1.Namespace{
88+
ObjectMeta: metav1.ObjectMeta{
89+
Name: "test-ns",
90+
Annotations: tt.nsAnnotations,
91+
},
92+
}
93+
94+
cl := fake.NewClientBuilder().
95+
WithScheme(reconcilerTestScheme).
96+
WithObjects(ns).
97+
Build()
98+
99+
routesAvailability := openshift.RoutesNotAvailable
100+
if tt.isOpenShift {
101+
routesAvailability = openshift.RoutesAvailable
102+
}
103+
104+
r := &OpenTelemetryCollectorReconciler{
105+
Client: cl,
106+
log: logr.Discard(),
107+
config: config.Config{
108+
OpenShiftRoutesAvailability: routesAvailability,
109+
},
110+
}
111+
112+
instance := v1beta1.OpenTelemetryCollector{
113+
ObjectMeta: metav1.ObjectMeta{
114+
Name: "test-collector",
115+
Namespace: "test-ns",
116+
},
117+
}
118+
if tt.existingFSGroup != nil {
119+
instance.Spec.PodSecurityContext = &corev1.PodSecurityContext{
120+
FSGroup: tt.existingFSGroup,
121+
}
122+
}
123+
124+
ctx := context.Background()
125+
params, err := r.GetParams(ctx, instance)
126+
require.NoError(t, err)
127+
128+
if tt.wantFSGroup == nil {
129+
if params.OtelCol.Spec.PodSecurityContext != nil {
130+
assert.Nil(t, params.OtelCol.Spec.PodSecurityContext.FSGroup)
131+
}
132+
} else {
133+
require.NotNil(t, params.OtelCol.Spec.PodSecurityContext)
134+
require.NotNil(t, params.OtelCol.Spec.PodSecurityContext.FSGroup)
135+
assert.Equal(t, *tt.wantFSGroup, *params.OtelCol.Spec.PodSecurityContext.FSGroup)
136+
}
137+
})
138+
}
139+
}

‎internal/controllers/opentelemetrycollector_controller.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212

1313
"github.com/go-logr/logr"
1414
routev1 "github.com/openshift/api/route/v1"
15+
securityv1 "github.com/openshift/api/security/v1"
16+
"github.com/openshift/library-go/pkg/security/uid"
1517
monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
1618
appsv1 "k8s.io/api/apps/v1"
1719
autoscalingv2 "k8s.io/api/autoscaling/v2"
@@ -156,6 +158,10 @@ func getCollectorConfigMapsToKeep(configVersionsToKeep int, configMaps []*corev1
156158
}
157159

158160
func (r *OpenTelemetryCollectorReconciler) GetParams(ctx context.Context, instance v1beta1.OpenTelemetryCollector) (manifests.Params, error) {
161+
if r.config.OpenShiftRoutesAvailability == openshift.RoutesAvailable {
162+
r.defaultFSGroupOnOpenShift(ctx, &instance)
163+
}
164+
159165
p := manifests.Params{
160166
Config: r.config,
161167
Client: r.Client,
@@ -175,6 +181,47 @@ func (r *OpenTelemetryCollectorReconciler) GetParams(ctx context.Context, instan
175181
return p, nil
176182
}
177183

184+
// defaultFSGroupOnOpenShift sets podSecurityContext.fsGroup from the namespace's
185+
// supplemental-groups or UID range annotation when running on OpenShift and no
186+
// explicit fsGroup is configured.
187+
//
188+
// On OpenShift, the restricted SCC normally injects fsGroup from the namespace range,
189+
// but more permissive SCCs (e.g. anyuid) do not. Explicitly setting fsGroup ensures
190+
// PVC volumes are group-writable regardless of which SCC is selected.
191+
//
192+
// See: https://github.com/migtools/crane/blob/440432b/cmd/transfer-pvc/transfer-pvc.go#L554
193+
func (r *OpenTelemetryCollectorReconciler) defaultFSGroupOnOpenShift(ctx context.Context, instance *v1beta1.OpenTelemetryCollector) {
194+
if instance.Spec.PodSecurityContext != nil && instance.Spec.PodSecurityContext.FSGroup != nil {
195+
return
196+
}
197+
198+
ns := &corev1.Namespace{}
199+
if err := r.Get(ctx, types.NamespacedName{Name: instance.Namespace}, ns); err != nil {
200+
r.log.V(4).Info("unable to fetch namespace for fsGroup defaulting", "namespace", instance.Namespace, "error", err)
201+
return
202+
}
203+
204+
rangeAnnotation := ns.Annotations[securityv1.SupplementalGroupsAnnotation]
205+
if rangeAnnotation == "" {
206+
rangeAnnotation = ns.Annotations[securityv1.UIDRangeAnnotation]
207+
}
208+
if rangeAnnotation == "" {
209+
return
210+
}
211+
212+
block, err := uid.ParseBlock(rangeAnnotation)
213+
if err != nil {
214+
r.log.V(4).Info("unable to parse group range annotation", "annotation", rangeAnnotation, "error", err)
215+
return
216+
}
217+
218+
fsGroup := int64(block.Start)
219+
if instance.Spec.PodSecurityContext == nil {
220+
instance.Spec.PodSecurityContext = &corev1.PodSecurityContext{}
221+
}
222+
instance.Spec.PodSecurityContext.FSGroup = &fsGroup
223+
}
224+
178225
func (r *OpenTelemetryCollectorReconciler) getTargetAllocator(ctx context.Context, params manifests.Params) (*v1alpha1.TargetAllocator, error) {
179226
if taName, ok := params.OtelCol.GetLabels()[constants.LabelTargetAllocator]; ok {
180227
targetAllocator := &v1alpha1.TargetAllocator{}
@@ -223,6 +270,7 @@ func NewReconciler(p Params) *OpenTelemetryCollectorReconciler {
223270
// +kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=get;list;watch;create;update;patch;delete
224271
// +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=httproutes,verbs=get;list;watch;create;update;patch;delete
225272
// +kubebuilder:rbac:groups=apps,resources=deployments/finalizers,verbs=get;watch;update;patch
273+
// +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch
226274
// +kubebuilder:rbac:groups=route.openshift.io,resources=routes;routes/custom-host,verbs=get;list;watch;create;update;patch;delete
227275
// +kubebuilder:rbac:groups=config.openshift.io,resources=infrastructures;infrastructures/status;apiservers,verbs=get;list;watch
228276
// +kubebuilder:rbac:groups=opentelemetry.io,resources=opentelemetrycollectors,verbs=get;list;watch;update;patch

0 commit comments

Comments
 (0)