Repository: coryodaniel/bonny
Pull request #213 (2023-05-31 07:00:01)

fix rbac rules overwriting one another
If two controllers define RBAC rules for the same resource, the last definition curently overwrites the previous one. This would be the result:

```yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: my-operator
rules:
  - apiGroups: [""]
    resources: ["secrets"]
    verbs: ["*"]
  - apiGroups: [""]
    resources: ["secrets"]
    verbs: ["get", "list"]
```
 
This PR fixes this by merging verbs of the same resource.



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b363d85..a06288b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,7 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Fixed
 
-- Compile time dependency at runtime: [#212](https://github.com/coryodaniel/bonny/pull/212)
+- `mix bonny.gen.manifest` - Merge RBAC rules for the same resources - []()
+- Compile time dependency at runtime - [#212](https://github.com/coryodaniel/bonny/pull/212)
 
 ## Changed
 
diff --git a/lib/bonny/api/crd.ex b/lib/bonny/api/crd.ex
index 868b770..083add0 100644
--- a/lib/bonny/api/crd.ex
+++ b/lib/bonny/api/crd.ex
@@ -131,4 +131,9 @@ defmodule Bonny.API.CRD do
       shortNames: short_names
     }
   end
+
+  # Implement Access behaviour
+  defdelegate fetch(term, key), to: Map
+  defdelegate get(term, key, default), to: Map
+  defdelegate get_and_update(term, key, fun), to: Map
 end
diff --git a/lib/mix/operator.ex b/lib/mix/operator.ex
index cb54c30..b364142 100644
--- a/lib/mix/operator.ex
+++ b/lib/mix/operator.ex
@@ -18,7 +18,25 @@ defmodule Bonny.Mix.Operator do
   end
 
   def rbac_rules(operators) do
-    Enum.uniq(base_rules() ++ legacy_rules() ++ operator_rules(operators))
+    rules =
+      for rule <- base_rules() ++ legacy_rules() ++ operator_rules(operators),
+          api_group <- rule.apiGroups,
+          resource <- rule.resources,
+          verb <- rule.verbs,
+          reduce: %{} do
+        acc ->
+          if verb == "*" or Map.get(acc, {api_group, resource}) == ["*"] do
+            Map.put(acc, {api_group, resource}, ["*"])
+          else
+            Map.update(acc, {api_group, resource}, [verb], &[verb | &1])
+          end
+      end
+
+    rules
+    |> Enum.map(fn {{api_group, resource}, verbs} ->
+      %{apiGroups: [api_group], resources: [resource], verbs: verbs |> Enum.uniq() |> Enum.sort()}
+    end)
+    |> Enum.sort_by(&{&1.apiGroups, &1.resources})
   end
 
   defp base_rules() do


Test patch:
diff --git a/test/mix/operator_test.exs b/test/mix/operator_test.exs
index f779dc2..56dd891 100644
--- a/test/mix/operator_test.exs
+++ b/test/mix/operator_test.exs
@@ -11,21 +11,22 @@ defmodule Bonny.Mix.OperatorTest do
       kind: "ClusterRole",
       metadata: %{labels: %{"k8s-app" => "bonny"}, name: "bonny"},
       rules: [
+        %{apiGroups: [""], resources: ["configmaps"], verbs: ["create", "read"]},
         %{
           apiGroups: ["apiextensions.k8s.io"],
           resources: ["customresourcedefinitions"],
           verbs: ["*"]
         },
-        %{apiGroups: ["events.k8s.io"], resources: ["events"], verbs: ["*"]},
+        %{apiGroups: ["apps"], resources: ["deployments"], verbs: ["*"]},
+        %{apiGroups: ["apps"], resources: ["services"], verbs: ["*"]},
         %{apiGroups: ["coordination.k8s.io"], resources: ["leases"], verbs: ["*"]},
-        %{apiGroups: ["example.com"], resources: ["widgets"], verbs: ["*"]},
+        %{apiGroups: ["events.k8s.io"], resources: ["events"], verbs: ["*"]},
         %{apiGroups: ["example.com"], resources: ["cogs"], verbs: ["*"]},
-        %{apiGroups: ["example.com"], resources: ["whizbangs"], verbs: ["*"]},
         %{apiGroups: ["example.com"], resources: ["testresources"], verbs: ["*"]},
-        %{apiGroups: ["apps"], resources: ["deployments", "services"], verbs: ["*"]},
-        %{apiGroups: [""], resources: ["configmaps"], verbs: ["create", "read"]},
         %{apiGroups: ["example.com"], resources: ["testresourcev2s"], verbs: ["*"]},
-        %{apiGroups: ["example.com"], resources: ["testresourcev2s/status"], verbs: ["*"]}
+        %{apiGroups: ["example.com"], resources: ["testresourcev2s/status"], verbs: ["*"]},
+        %{apiGroups: ["example.com"], resources: ["whizbangs"], verbs: ["*"]},
+        %{apiGroups: ["example.com"], resources: ["widgets"], verbs: ["*"]}
       ]
     }
 
diff --git a/test/support/controllers/test_resource_v2_controller.ex b/test/support/controllers/test_resource_v2_controller.ex
index 5e580ba..a8abb34 100644
--- a/test/support/controllers/test_resource_v2_controller.ex
+++ b/test/support/controllers/test_resource_v2_controller.ex
@@ -46,6 +46,7 @@ defmodule TestResourceV2Controller do
 
   def rbac_rules() do
     [to_rbac_rule({"example.com", "testresourcev2s/status", "*"})]
+    [to_rbac_rule({"example.com", "whizbangs", "get"})]
   end
 
   def cleanup(axn) do