Skip to content

Commit 4f73a9e

Browse files
martinohansenmartinohansen
committed
Scope barman Secret RBAC
Avoid granting namespace-wide Secret access when ObjectStores do not need credential Secrets. Ref: #892
1 parent e550b48 commit 4f73a9e

5 files changed

Lines changed: 76 additions & 18 deletions

File tree

‎internal/cnpgi/operator/specs/role.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func BuildRoleRules(barmanObjects []barmancloudv1.ObjectStore) []rbacv1.PolicyRu
6060
}
6161
}
6262

63-
return []rbacv1.PolicyRule{
63+
rules := []rbacv1.PolicyRule{
6464
{
6565
APIGroups: []string{
6666
barmancloudv1.GroupVersion.Group,
@@ -87,7 +87,11 @@ func BuildRoleRules(barmanObjects []barmancloudv1.ObjectStore) []rbacv1.PolicyRu
8787
},
8888
ResourceNames: barmanObjectsSet.ToSortedList(),
8989
},
90-
{
90+
}
91+
92+
secrets := secretsSet.ToSortedList()
93+
if len(secrets) > 0 {
94+
rules = append(rules, rbacv1.PolicyRule{
9195
APIGroups: []string{
9296
"",
9397
},
@@ -99,9 +103,11 @@ func BuildRoleRules(barmanObjects []barmancloudv1.ObjectStore) []rbacv1.PolicyRu
99103
"watch",
100104
"list",
101105
},
102-
ResourceNames: secretsSet.ToSortedList(),
103-
},
106+
ResourceNames: secrets,
107+
})
104108
}
109+
110+
return rules
105111
}
106112

107113
// ObjectStoreNamesFromRole extracts the ObjectStore names referenced

‎internal/cnpgi/operator/specs/role_test.go‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,12 @@ var _ = Describe("BuildRoleRules", func() {
7878
Expect(rules[2].ResourceNames).To(ConsistOf("secret-a", "secret-b"))
7979
})
8080

81-
It("should produce rules with empty ResourceNames for empty input", func() {
81+
It("should not produce a secrets rule for empty input", func() {
8282
rules := BuildRoleRules(nil)
83-
Expect(rules).To(HaveLen(3))
83+
Expect(rules).To(HaveLen(2))
8484
Expect(rules[0].ResourceNames).To(BeEmpty())
8585
Expect(rules[0].ResourceNames).NotTo(BeNil())
8686
Expect(rules[1].ResourceNames).To(BeEmpty())
87-
Expect(rules[2].ResourceNames).To(BeEmpty())
8887
})
8988

9089
It("should deduplicate secret names across ObjectStores", func() {
@@ -95,6 +94,31 @@ var _ = Describe("BuildRoleRules", func() {
9594
rules := BuildRoleRules(objects)
9695
Expect(rules[2].ResourceNames).To(Equal([]string{"shared-secret"}))
9796
})
97+
98+
It("should not produce a secrets rule when ObjectStores use IAM role inheritance", func() {
99+
objects := []barmancloudv1.ObjectStore{
100+
{
101+
ObjectMeta: metav1.ObjectMeta{
102+
Name: "store-a",
103+
Namespace: "default",
104+
},
105+
Spec: barmancloudv1.ObjectStoreSpec{
106+
Configuration: barmanapi.BarmanObjectStoreConfiguration{
107+
DestinationPath: "s3://bucket/path",
108+
BarmanCredentials: barmanapi.BarmanCredentials{
109+
AWS: &barmanapi.S3Credentials{
110+
InheritFromIAMRole: true,
111+
},
112+
},
113+
},
114+
},
115+
},
116+
}
117+
rules := BuildRoleRules(objects)
118+
Expect(rules).To(HaveLen(2))
119+
Expect(rules[0].ResourceNames).To(Equal([]string{"store-a"}))
120+
Expect(rules[1].ResourceNames).To(Equal([]string{"store-a"}))
121+
})
98122
})
99123

100124
var _ = Describe("BuildRole", func() {

‎internal/cnpgi/operator/specs/secrets.go‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,17 @@ import (
2828
func CollectSecretNamesFromCredentials(barmanCredentials *barmanapi.BarmanCredentials) []string {
2929
var references []*machineryapi.SecretKeySelector
3030
if barmanCredentials.AWS != nil {
31-
references = append(
32-
references,
33-
barmanCredentials.AWS.AccessKeyIDReference,
34-
barmanCredentials.AWS.SecretAccessKeyReference,
35-
barmanCredentials.AWS.RegionReference,
36-
barmanCredentials.AWS.SessionToken,
37-
)
31+
// When using IAM role inheritance, barman-cloud uses the pod
32+
// environment credential chain and does not read credential Secrets.
33+
if !barmanCredentials.AWS.InheritFromIAMRole {
34+
references = append(
35+
references,
36+
barmanCredentials.AWS.AccessKeyIDReference,
37+
barmanCredentials.AWS.SecretAccessKeyReference,
38+
barmanCredentials.AWS.RegionReference,
39+
barmanCredentials.AWS.SessionToken,
40+
)
41+
}
3842
}
3943
if barmanCredentials.Azure != nil {
4044
// When using default Azure credentials or managed identity, no secrets are required

‎internal/cnpgi/operator/specs/secrets_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,29 @@ var _ = Describe("CollectSecretNamesFromCredentials", func() {
5757
secrets := CollectSecretNamesFromCredentials(credentials)
5858
Expect(secrets).To(BeEmpty())
5959
})
60+
61+
It("should return empty list when using InheritFromIAMRole", func() {
62+
credentials := &barmanapi.BarmanCredentials{
63+
AWS: &barmanapi.S3Credentials{
64+
InheritFromIAMRole: true,
65+
AccessKeyIDReference: &machineryapi.SecretKeySelector{
66+
LocalObjectReference: machineryapi.LocalObjectReference{
67+
Name: "aws-secret",
68+
},
69+
Key: "access-key-id",
70+
},
71+
RegionReference: &machineryapi.SecretKeySelector{
72+
LocalObjectReference: machineryapi.LocalObjectReference{
73+
Name: "aws-region",
74+
},
75+
Key: "region",
76+
},
77+
},
78+
}
79+
80+
secrets := CollectSecretNamesFromCredentials(credentials)
81+
Expect(secrets).To(BeEmpty())
82+
})
6083
})
6184

6285
Context("when collecting secrets from Azure credentials", func() {

‎internal/controller/objectstore_controller_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ import (
3131
apierrs "k8s.io/apimachinery/pkg/api/errors"
3232
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3333
"k8s.io/apimachinery/pkg/runtime"
34-
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
3534
"k8s.io/apimachinery/pkg/types"
35+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
3636
"sigs.k8s.io/controller-runtime/pkg/client"
3737
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3838
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
@@ -261,7 +261,7 @@ var _ = Describe("ObjectStoreReconciler", func() {
261261
Expect(result).To(Equal(reconcile.Result{}))
262262
})
263263

264-
It("should produce empty ResourceNames when all ObjectStores are deleted", func() {
264+
It("should omit the secrets rule when all ObjectStores are deleted", func() {
265265
store := newTestObjectStore("my-store", "default", "aws-creds")
266266
role := newLabeledRole("my-cluster", "default", []barmancloudv1.ObjectStore{*store})
267267

@@ -291,10 +291,11 @@ var _ = Describe("ObjectStoreReconciler", func() {
291291
Name: "my-cluster-barman-cloud",
292292
}, &updatedRole)).To(Succeed())
293293

294-
// All rules should have empty ResourceNames
295294
Expect(updatedRole.Rules[0].ResourceNames).To(BeEmpty())
296295
Expect(updatedRole.Rules[1].ResourceNames).To(BeEmpty())
297-
Expect(updatedRole.Rules[2].ResourceNames).To(BeEmpty())
296+
for _, rule := range updatedRole.Rules {
297+
Expect(rule.Resources).NotTo(Equal([]string{"secrets"}))
298+
}
298299
})
299300

300301
It("should return an error when listing Roles fails", func() {

0 commit comments

Comments
 (0)