Skip to content

Commit 255c1b5

Browse files
pmarkiewkapmarkiewka
authored
fix(rbac): ensure Prometheus runs with correct permissions in ArgoCD-managed namespaces (#291)
- Add conditional node access rule for non-OpenShift environments - Pass config to Role for templating RBAC based on setup context - Replace hardcoded namespace list with config.application.activeNamespaces - Move validation logic into Role, RoleBinding, and ServiceAccountRef constructors - Update templates and tests to reflect RBAC and config handling changes This fixes permission issues that prevented Prometheus from working in a namespace-isolated setup via ArgoCD.
1 parent 34009c9 commit 255c1b5

File tree

10 files changed

+216
-37
lines changed

10 files changed

+216
-37
lines changed

‎.dockerignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
!scripts/utils.sh
1414
!scripts/apply-ng.sh
1515
!scripts/downloadHelmCharts.sh
16+
!templates/kubernetes/rbac/
1617
!.curlrc
1718
!LICENSE
1819

‎argocd/cluster-resources/misc/namespaces.ftl.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<#if config.application.openshift == false>
1+
<#if !config.application.openshift && !config.application.namespaceIsolation>
22
apiVersion: v1
33
kind: Namespace
44
metadata:

‎src/main/groovy/com/cloudogu/gitops/features/argocd/ArgoCD.groovy‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ class ArgoCD extends Feature {
326326
namespace,
327327
["argocd-argocd-server", "argocd-argocd-application-controller", "argocd-applicationset-controller"]
328328
)
329+
.withConfig(config)
329330
.withRepo(argocdRepoInitializationAction.repo)
330331
.withSubfolder(OPERATOR_RBAC_PATH)
331332
.generate()

‎src/main/groovy/com/cloudogu/gitops/kubernetes/rbac/RbacDefinition.groovy‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.cloudogu.gitops.kubernetes.rbac
22

3+
import com.cloudogu.gitops.config.Config
34
import com.cloudogu.gitops.scmm.ScmmRepo
45
import com.cloudogu.gitops.utils.TemplatingEngine
56
import groovy.util.logging.Slf4j
@@ -15,6 +16,7 @@ class RbacDefinition {
1516
private List<ServiceAccountRef> serviceAccounts = []
1617
private String subfolder = "rbac"
1718
private ScmmRepo repo
19+
private Config config
1820

1921
private final TemplatingEngine templater = new TemplatingEngine()
2022

@@ -51,17 +53,24 @@ class RbacDefinition {
5153
return this
5254
}
5355

56+
RbacDefinition withConfig(Config config) {
57+
this.config = config
58+
return this
59+
}
60+
5461
void generate() {
55-
validateInputs()
62+
if (!repo) {
63+
throw new IllegalStateException("SCMM repo must be set using withRepo() before calling generate()")
64+
}
65+
66+
def role = new Role(name, namespace, variant, config)
67+
def binding = new RoleBinding(name, namespace, name, serviceAccounts)
5668

57-
log.debug("Generating RBAC for name='${name}', namespace='${namespace}', subfolder='${subfolder}'")
69+
log.trace("Generating RBAC for name='${name}', namespace='${namespace}', subfolder='${subfolder}'")
5870

5971
def outputDir = Path.of(repo.absoluteLocalRepoTmpDir, subfolder).toFile()
6072
outputDir.mkdirs()
6173

62-
def role = new Role(name, namespace, variant)
63-
def binding = new RoleBinding(name, namespace, name, serviceAccounts)
64-
6574
templater.template(
6675
role.getTemplateFile(),
6776
role.getOutputFile(outputDir),
@@ -75,18 +84,4 @@ class RbacDefinition {
7584
)
7685
}
7786

78-
private void validateInputs() {
79-
if (!repo) {
80-
throw new IllegalStateException("SCMM repo must be set using withRepo() before calling generate()")
81-
}
82-
if (!name?.trim()) {
83-
throw new IllegalStateException("RBAC definition requires a non-empty name")
84-
}
85-
if (!namespace?.trim()) {
86-
throw new IllegalStateException("RBAC definition requires a non-empty namespace")
87-
}
88-
if (!serviceAccounts || serviceAccounts.isEmpty()) {
89-
throw new IllegalStateException("RBAC definition requires at least one service account")
90-
}
91-
}
9287
}

‎src/main/groovy/com/cloudogu/gitops/kubernetes/rbac/Role.groovy‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,23 @@
11
package com.cloudogu.gitops.kubernetes.rbac
22

3+
import com.cloudogu.gitops.config.Config
4+
35
class Role {
46
String name
57
String namespace
68
Variant variant
9+
Config config
10+
11+
Role(String name, String namespace, Variant variant, Config config) {
12+
if (!name?.trim()) throw new IllegalArgumentException("Role name must not be blank")
13+
if (!namespace?.trim()) throw new IllegalArgumentException("Role namespace must not be blank")
14+
if (!variant) throw new IllegalArgumentException("Role variant must not be null")
15+
if (!config) throw new IllegalArgumentException("Config must not be null")
716

8-
Role(String name, String namespace, Variant variant) {
917
this.name = name
1018
this.namespace = namespace
1119
this.variant = variant
20+
this.config = config
1221
}
1322

1423
enum Variant {
@@ -24,7 +33,8 @@ class Role {
2433
Map<String, Object> toTemplateParams() {
2534
return [
2635
name : name,
27-
namespace: namespace
36+
namespace: namespace,
37+
config : config
2838
]
2939
}
3040

‎src/main/groovy/com/cloudogu/gitops/kubernetes/rbac/RoleBinding.groovy‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ class RoleBinding {
77
List<ServiceAccountRef> serviceAccounts
88

99
RoleBinding(String name, String namespace, String roleName, List<ServiceAccountRef> serviceAccounts) {
10+
if (!name?.trim()) throw new IllegalArgumentException("RoleBinding name must not be blank")
11+
if (!namespace?.trim()) throw new IllegalArgumentException("RoleBinding namespace must not be blank")
12+
if (!roleName?.trim()) throw new IllegalArgumentException("Role name must not be blank")
13+
if (!serviceAccounts || serviceAccounts.isEmpty()) throw new IllegalArgumentException("At least one service account is required")
14+
1015
this.name = name
1116
this.namespace = namespace
1217
this.roleName = roleName

‎src/main/groovy/com/cloudogu/gitops/kubernetes/rbac/ServiceAccountRef.groovy‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,29 @@
11
package com.cloudogu.gitops.kubernetes.rbac
22

33
class ServiceAccountRef {
4-
final String name
5-
final String namespace
4+
String name
5+
String namespace
66

77
ServiceAccountRef(String name, String namespace) {
8+
if (!name?.trim()) {
9+
throw new IllegalArgumentException("ServiceAccount name must not be blank")
10+
}
11+
if (!namespace?.trim()) {
12+
throw new IllegalArgumentException("ServiceAccount namespace must not be blank")
13+
}
814
this.name = name
915
this.namespace = namespace
1016
}
1117

1218
static List<ServiceAccountRef> fromNames(String namespace, List<String> names) {
13-
return names.collect { new ServiceAccountRef(it, namespace) }
19+
if (!namespace?.trim()) {
20+
throw new IllegalArgumentException("Namespace must not be blank for service accounts")
21+
}
22+
23+
return names
24+
.findAll { it?.trim() }
25+
.unique()
26+
.collect { new ServiceAccountRef(it, namespace) }
1427
}
1528

1629
Map<String, String> toMap() {

‎src/test/groovy/com/cloudogu/gitops/features/argocd/ArgoCDTest.groovy‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1179,24 +1179,22 @@ class ArgoCDTest {
11791179

11801180
@Test
11811181
void 'RBACs with operator using RbacDefinition outputs'() {
1182-
config.application.namePrefix = "testPrefix-"
1183-
def argoCD = setupOperatorTest(openshift: false)
1184-
11851182
List<String> expectedNamespaces = [
11861183
"testPrefix-monitoring",
11871184
"testPrefix-secrets",
11881185
"testPrefix-ingress-nginx",
11891186
"testPrefix-example-apps-staging",
11901187
"testPrefix-example-apps-production"
11911188
]
1192-
// have to prepare activeNamespaces for unit-test, Application.groovy is setting this in integration way
1193-
config.application.activeNamespaces = expectedNamespaces
1189+
config.application.namePrefix = "testPrefix-"
1190+
1191+
def argoCD = setupOperatorTest(openshift: false)
1192+
11941193

11951194
argoCD.install()
11961195

11971196
File rbacPath = Path.of(argocdRepo.getAbsoluteLocalRepoTmpDir(), ArgoCD.OPERATOR_RBAC_PATH).toFile()
11981197

1199-
12001198
expectedNamespaces.each { String ns ->
12011199
File roleFile = new File(rbacPath, "role-argocd-${ns}.yaml")
12021200
File bindingFile = new File(rbacPath, "rolebinding-argocd-${ns}.yaml")
@@ -1478,6 +1476,46 @@ class ArgoCDTest {
14781476
.doesNotExist()
14791477
}
14801478

1479+
@Test
1480+
void 'Operator RBAC includes node access rules when not on OpenShift'() {
1481+
config.application.namePrefix = "testprefix-"
1482+
1483+
def argoCD = setupOperatorTest(openshift: false)
1484+
argoCD.install()
1485+
1486+
print config.toMap()
1487+
1488+
File rbacDir = Path.of(argocdRepo.getAbsoluteLocalRepoTmpDir(), ArgoCD.OPERATOR_RBAC_PATH).toFile()
1489+
File roleFile = new File(rbacDir, "role-argocd-testprefix-monitoring.yaml")
1490+
1491+
Map yaml = new YamlSlurper().parse(roleFile) as Map
1492+
List<Map<String, Object>> rules = yaml["rules"] as List<Map<String, Object>>
1493+
1494+
assertThat(rules).anyMatch { rule ->
1495+
List<String> resources = rule["resources"] as List<String>
1496+
resources.contains("nodes") && resources.contains("nodes/metrics")
1497+
}
1498+
}
1499+
1500+
@Test
1501+
void 'Operator RBAC does not include node access rules when on OpenShift'() {
1502+
config.application.namePrefix = "testprefix-"
1503+
1504+
def argoCD = setupOperatorTest(openshift: true)
1505+
argoCD.install()
1506+
1507+
File rbacDir = Path.of(argocdRepo.getAbsoluteLocalRepoTmpDir(), ArgoCD.OPERATOR_RBAC_PATH).toFile()
1508+
File roleFile = new File(rbacDir, "role-argocd-testprefix-monitoring.yaml")
1509+
println roleFile
1510+
1511+
Map yaml = new YamlSlurper().parse(roleFile) as Map
1512+
List<Map<String, Object>> rules = yaml["rules"] as List<Map<String, Object>>
1513+
1514+
assertThat(rules).noneMatch { rule ->
1515+
List<String> resources = rule["resources"] as List<String>
1516+
resources.contains("nodes") && resources.contains("nodes/metrics")
1517+
}
1518+
}
14811519

14821520
private ArgoCD setupOperatorTest(Map options = [:]) {
14831521
config.features.argocd.operator = true
@@ -1517,10 +1555,20 @@ class ArgoCDTest {
15171555
)
15181556
}
15191557

1558+
private static mockPrefixActiveNamespaces(Config config) {
1559+
def namespaces = config.application.activeNamespaces
1560+
def prefix = config.application.namePrefix ?: ""
1561+
1562+
config.application.activeNamespaces = namespaces.collect { ns ->
1563+
"${prefix}${ns}".toString()
1564+
} as List<String>
1565+
}
1566+
15201567
class ArgoCDForTest extends ArgoCD {
15211568
ArgoCDForTest(Config config, CommandExecutorForTest k8sCommands, CommandExecutorForTest helmCommands) {
15221569
super(config, new K8sClientForTest(config, k8sCommands), new HelmClient(helmCommands), new FileSystemUtils(),
15231570
new TestScmmRepoProvider(config, new FileSystemUtils()))
1571+
mockPrefixActiveNamespaces(config)
15241572
}
15251573

15261574
@Override

0 commit comments

Comments
 (0)