Skip to content

Commit de0aa5c

Browse files
chrissetoRafalKorepta
chrisseto
authored and
RafalKorepta
committed
operator: correct rack awareness permissions
Prior to this commit the operator's chart incorrect referenced a non-existent `Role` for the permissions of rack awareness. As helm loves to swallow errors, this went unnoticed until reported by an end user. This commit corrects the mistake and adds a regression acceptance test.
1 parent 59f6af7 commit de0aa5c

File tree

8 files changed

+305
-6
lines changed

8 files changed

+305
-6
lines changed

‎.changes/unreleased/operator-Fixed-20250609-152934.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
project: operator
2+
kind: Fixed
3+
body: |-
4+
`get` permissions on `Node` resources is now correctly configured by default.
5+
6+
`--set rbac.createAdditionalControllerCRs=true` is no longer required for rackawareness to work.
7+
time: 2025-06-09T15:29:34.084852-04:00

‎acceptance/features/cluster.feature‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,32 @@ Feature: Basic cluster tests
4444
Then cluster "upgrade" is stable with 1 nodes
4545
And service "upgrade-external" should have named port "admin-default" with value 9640
4646
And rpk is configured correctly in "upgrade" cluster
47+
48+
49+
@skip:gke @skip:aks @skip:eks
50+
Scenario: Rack Awareness
51+
Given I apply Kubernetes manifest:
52+
# NB: You wouldn't actually use kubernetes.io/os for the value of rack,
53+
# it's just a value that we know is both present and deterministic for the
54+
# purpose of testing.
55+
"""
56+
---
57+
apiVersion: cluster.redpanda.com/v1alpha2
58+
kind: Redpanda
59+
metadata:
60+
name: rack-awareness
61+
spec:
62+
clusterSpec:
63+
console:
64+
enabled: false
65+
statefulset:
66+
replicas: 1
67+
rackAwareness:
68+
enabled: true
69+
nodeAnnotation: 'kubernetes.io/os'
70+
"""
71+
And cluster "rack-awareness" is stable with 1 nodes
72+
Then running `cat /etc/redpanda/redpanda.yaml | grep -o 'rack: .*$'` will output:
73+
"""
74+
rack: linux
75+
"""

‎acceptance/steps/register.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ func init() {
6767
framework.RegisterStep(`^service "([^"]*)" has named port "([^"]*)" with value (\d+)$`, checkServiceWithPort)
6868
framework.RegisterStep(`^service "([^"]*)" should have named port "([^"]*)" with value (\d+)$`, checkServiceWithPort)
6969
framework.RegisterStep(`^rpk is configured correctly in "([^"]*)" cluster$`, checkRPKCommands)
70+
framework.RegisterStep("running `(.*)` will output:$", runScriptInClusterCheckOutput)
7071

7172
// Decommissioning scenario steps
7273
framework.RegisterStep(`^cluster "([^"]*)" is unhealthy$`, checkClusterUnhealthy)

‎acceptance/steps/rpk.go‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,74 @@ package steps
33
import (
44
"bytes"
55
"context"
6+
"strings"
67

8+
"github.com/cucumber/godog"
79
"github.com/stretchr/testify/require"
810
appsv1 "k8s.io/api/apps/v1"
911
corev1 "k8s.io/api/core/v1"
1012
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1113
"sigs.k8s.io/controller-runtime/pkg/client"
1214

1315
framework "github.com/redpanda-data/redpanda-operator/harpoon"
16+
redpandav1alpha2 "github.com/redpanda-data/redpanda-operator/operator/api/redpanda/v1alpha2"
1417
"github.com/redpanda-data/redpanda-operator/pkg/kube"
1518
)
1619

17-
func checkRPKCommands(ctx context.Context, t framework.TestingT, clusterName string) {
20+
func runScriptInClusterCheckOutput(ctx context.Context, t framework.TestingT, command string, output *godog.DocString) {
21+
var redpandas redpandav1alpha2.RedpandaList
22+
require.NoError(t, t.List(ctx, &redpandas))
23+
24+
if len(redpandas.Items) != 1 {
25+
require.FailNow(t, "expected to find 1 %T but found %d", (*redpandav1alpha2.Redpanda)(nil), len(redpandas.Items))
26+
}
27+
28+
redpanda := redpandas.Items[0]
29+
30+
var sts appsv1.StatefulSet
31+
require.NoError(t, t.Get(ctx, t.ResourceKey(redpanda.Name), &sts))
32+
33+
selector, err := metav1.LabelSelectorAsSelector(sts.Spec.Selector)
34+
require.NoError(t, err)
35+
36+
var pods corev1.PodList
37+
require.NoError(t, t.List(ctx, &pods, client.MatchingLabelsSelector{
38+
Selector: selector,
39+
}))
40+
41+
if len(pods.Items) < 1 {
42+
require.FailNow(t, "expected to find at least 1 Pod but found none")
43+
}
44+
45+
pod := pods.Items[0]
46+
1847
ctl, err := kube.FromRESTConfig(t.RestConfig())
1948
require.NoError(t, err)
2049

21-
var clusterSet appsv1.StatefulSet
50+
t.Logf("executing %q in Pod %q", command, pod.Name)
51+
52+
var stdout bytes.Buffer
53+
require.NoError(t, ctl.Exec(ctx, &pod, kube.ExecOptions{
54+
Container: "redpanda",
55+
Command: []string{"/bin/bash", "-c", command},
56+
Stdout: &stdout,
57+
}))
58+
59+
// Correct for extra whitespace from either the command itself or from
60+
// godog's parsing.
61+
expected := strings.Trim(output.Content, "\n ")
62+
actual := strings.Trim(stdout.String(), "\n ")
63+
64+
require.Equal(t, expected, actual)
65+
}
66+
67+
func checkRPKCommands(ctx context.Context, t framework.TestingT, clusterName string) {
68+
ctl, err := kube.FromRESTConfig(t.RestConfig())
69+
require.NoError(t, err)
2270

2371
key := t.ResourceKey(clusterName)
2472

73+
var clusterSet appsv1.StatefulSet
2574
require.NoError(t, t.Get(ctx, key, &clusterSet))
2675

2776
selector, err := metav1.LabelSelectorAsSelector(clusterSet.Spec.Selector)

‎operator/CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@ This is required to ensure that a pre-existing sts can roll over to new configur
9595
* Setting `serviceAccount.create` to `false` no longer prevents the Kubernetes ServiceAccountToken volume from being mounted to the operator Pod.
9696
* updated operator v1 to ignore "cluster.redpanda.com/node-pool-spec" annotation for pod rolls. previously, under certain conditions, the operator started rolling pods if this annotation changed - but there is no need to do so.
9797
* Added the missing `https` port to the operator Pod that was referenced by the [`ServiceMonitor`](https://github.com/redpanda-data/redpanda-operator/blob/4e34c5ea79b00fa0caeda64955e3291666194274/operator/chart/servicemonitor.go#L42)
98+
* `get` permissions on `Node` resources is now correctly configured by default.
99+
100+
`--set rbac.createAdditionalControllerCRs=true` is no longer required for rackawareness to work.
98101

99102
## [v25.1.1-beta3](https://github.com/redpanda-data/redpanda-operator/releases/tag/operator%2Fv25.1.1-beta3) - 2025-05-07
100103
### Added

‎operator/chart/rbac.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ func ClusterRoles(dot *helmette.Dot) []rbacv1.ClusterRole {
4545
RuleFiles: []string{
4646
"files/rbac/leader-election.ClusterRole.yaml",
4747
"files/rbac/pvcunbinder.ClusterRole.yaml",
48+
"files/rbac/rack-awareness.ClusterRole.yaml", // Rack awareness is a toggle on the CR, so we always need RBAC for it.
4849
"files/rbac/v1-manager.ClusterRole.yaml",
4950
},
5051
},
@@ -140,8 +141,7 @@ func Roles(dot *helmette.Dot) []rbacv1.Role {
140141
Name: Fullname(dot),
141142
Enabled: values.Scope == Namespace,
142143
RuleFiles: []string{
143-
"files/rbac/rack-awareness.Role.yaml", // Rack awareness is a toggle on the CR, so we always need RBAC for it.
144-
"files/rbac/sidecar.Role.yaml", // Sidecar is a toggle on the CR, so we always need RBAC for it.
144+
"files/rbac/sidecar.Role.yaml", // Sidecar is a toggle on the CR, so we always need RBAC for it.
145145
"files/rbac/v2-manager.Role.yaml",
146146
},
147147
},

‎operator/chart/templates/_rbac.go.tpl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
{{- if (or $values.crds.enabled $values.crds.experimental) -}}
1515
{{- $managerFiles = (concat (default (list) $managerFiles) (list "files/rbac/crd-installation.ClusterRole.yaml")) -}}
1616
{{- end -}}
17-
{{- $bundles := (list (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "Enabled" (eq $values.scope "Cluster") "RuleFiles" (list "files/rbac/leader-election.ClusterRole.yaml" "files/rbac/pvcunbinder.ClusterRole.yaml" "files/rbac/v1-manager.ClusterRole.yaml"))) (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "Enabled" (eq $values.scope "Namespace") "RuleFiles" $managerFiles)) (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.cleanForK8sWithSuffix" (dict "a" (list (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "additional-controllers")))) "r") "Enabled" (and (eq $values.scope "Namespace") $values.rbac.createAdditionalControllerCRs) "RuleFiles" (list "files/rbac/decommission.ClusterRole.yaml" "files/rbac/node-watcher.ClusterRole.yaml" "files/rbac/old-decommission.ClusterRole.yaml" "files/rbac/pvcunbinder.ClusterRole.yaml")))) -}}
17+
{{- $bundles := (list (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "Enabled" (eq $values.scope "Cluster") "RuleFiles" (list "files/rbac/leader-election.ClusterRole.yaml" "files/rbac/pvcunbinder.ClusterRole.yaml" "files/rbac/rack-awareness.ClusterRole.yaml" "files/rbac/v1-manager.ClusterRole.yaml"))) (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "Enabled" (eq $values.scope "Namespace") "RuleFiles" $managerFiles)) (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.cleanForK8sWithSuffix" (dict "a" (list (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "additional-controllers")))) "r") "Enabled" (and (eq $values.scope "Namespace") $values.rbac.createAdditionalControllerCRs) "RuleFiles" (list "files/rbac/decommission.ClusterRole.yaml" "files/rbac/node-watcher.ClusterRole.yaml" "files/rbac/old-decommission.ClusterRole.yaml" "files/rbac/pvcunbinder.ClusterRole.yaml")))) -}}
1818
{{- $clusterRoles := (list (mustMergeOverwrite (dict "metadata" (dict "creationTimestamp" (coalesce nil)) "rules" (coalesce nil)) (mustMergeOverwrite (dict) (dict "apiVersion" "rbac.authorization.k8s.io/v1" "kind" "ClusterRole")) (dict "metadata" (mustMergeOverwrite (dict "creationTimestamp" (coalesce nil)) (dict "name" (get (fromJson (include "operator.cleanForK8sWithSuffix" (dict "a" (list (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "metrics-reader")))) "r") "labels" (get (fromJson (include "operator.Labels" (dict "a" (list $dot)))) "r") "annotations" $values.annotations)) "rules" (list (mustMergeOverwrite (dict "verbs" (coalesce nil)) (dict "verbs" (list "get") "nonResourceURLs" (list "/metrics"))))))) -}}
1919
{{- range $_, $bundle := $bundles -}}
2020
{{- if (not $bundle.Enabled) -}}
@@ -49,7 +49,7 @@
4949
{{- (dict "r" (coalesce nil)) | toJson -}}
5050
{{- break -}}
5151
{{- end -}}
52-
{{- $bundles := (list (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.cleanForK8sWithSuffix" (dict "a" (list (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "election-role")))) "r") "Enabled" true "RuleFiles" (list "files/rbac/leader-election.Role.yaml"))) (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "Enabled" (eq $values.scope "Cluster") "RuleFiles" (list "files/rbac/pvcunbinder.Role.yaml"))) (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "Enabled" (eq $values.scope "Namespace") "RuleFiles" (list "files/rbac/rack-awareness.Role.yaml" "files/rbac/sidecar.Role.yaml" "files/rbac/v2-manager.Role.yaml"))) (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (printf "%s%s" (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "-additional-controllers") "Enabled" (and (eq $values.scope "Namespace") $values.rbac.createAdditionalControllerCRs) "RuleFiles" (list "files/rbac/decommission.Role.yaml" "files/rbac/node-watcher.Role.yaml" "files/rbac/old-decommission.Role.yaml" "files/rbac/pvcunbinder.Role.yaml"))) (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.cleanForK8sWithSuffix" (dict "a" (list (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "rpk-bundle")))) "r") "Enabled" $values.rbac.createRPKBundleCRs "RuleFiles" (list "files/rbac/rpk-debug-bundle.Role.yaml")))) -}}
52+
{{- $bundles := (list (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.cleanForK8sWithSuffix" (dict "a" (list (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "election-role")))) "r") "Enabled" true "RuleFiles" (list "files/rbac/leader-election.Role.yaml"))) (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "Enabled" (eq $values.scope "Cluster") "RuleFiles" (list "files/rbac/pvcunbinder.Role.yaml"))) (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "Enabled" (eq $values.scope "Namespace") "RuleFiles" (list "files/rbac/sidecar.Role.yaml" "files/rbac/v2-manager.Role.yaml"))) (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (printf "%s%s" (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "-additional-controllers") "Enabled" (and (eq $values.scope "Namespace") $values.rbac.createAdditionalControllerCRs) "RuleFiles" (list "files/rbac/decommission.Role.yaml" "files/rbac/node-watcher.Role.yaml" "files/rbac/old-decommission.Role.yaml" "files/rbac/pvcunbinder.Role.yaml"))) (mustMergeOverwrite (dict "Enabled" false "RuleFiles" (coalesce nil) "Name" "") (dict "Name" (get (fromJson (include "operator.cleanForK8sWithSuffix" (dict "a" (list (get (fromJson (include "operator.Fullname" (dict "a" (list $dot)))) "r") "rpk-bundle")))) "r") "Enabled" $values.rbac.createRPKBundleCRs "RuleFiles" (list "files/rbac/rpk-debug-bundle.Role.yaml")))) -}}
5353
{{- $roles := (coalesce nil) -}}
5454
{{- range $_, $bundle := $bundles -}}
5555
{{- if (not $bundle.Enabled) -}}

0 commit comments

Comments
 (0)