fix(rbac): do not mix apigroups.

buehler · buehler · commit 0b8cbab6057e · 2024-01-17T12:52:51.000+01:00
This fixes [bug]: RBAC roles get mixed between apigroups #583.
diff --git a/.editorconfig b/.editorconfig
@@ -27,6 +27,7 @@ insert_final_newline = true
 dotnet_diagnostic.S3604.severity = none
 
 dotnet_diagnostic.SA0001.severity = none
+dotnet_diagnostic.SA1010.severity = none
 dotnet_diagnostic.SA1600.severity = none
 dotnet_diagnostic.CS1591.severity = none
 dotnet_diagnostic.SA1008.severity = suggestion
diff --git a/src/KubeOps.Generator/SyntaxReceiver/EntityControllerSyntaxReceiver.cs b/src/KubeOps.Generator/SyntaxReceiver/EntityControllerSyntaxReceiver.cs
@@ -5,7 +5,7 @@ namespace KubeOps.Generator.SyntaxReceiver;
 
 internal class EntityControllerSyntaxReceiver : ISyntaxContextReceiver
 {
-    public List<(ClassDeclarationSyntax Controller, string EntityName)> Controllers { get; } = new();
+    public List<(ClassDeclarationSyntax Controller, string EntityName)> Controllers { get; } = [];
 
     public void OnVisitSyntaxNode(GeneratorSyntaxContext context)
     {
diff --git a/src/KubeOps.Generator/SyntaxReceiver/EntityFinalizerSyntaxReceiver.cs b/src/KubeOps.Generator/SyntaxReceiver/EntityFinalizerSyntaxReceiver.cs
@@ -5,7 +5,7 @@ namespace KubeOps.Generator.SyntaxReceiver;
 
 internal class EntityFinalizerSyntaxReceiver : ISyntaxContextReceiver
 {
-    public List<(ClassDeclarationSyntax Finalizer, string EntityName)> Finalizer { get; } = new();
+    public List<(ClassDeclarationSyntax Finalizer, string EntityName)> Finalizer { get; } = [];
 
     public void OnVisitSyntaxNode(GeneratorSyntaxContext context)
     {
diff --git a/src/KubeOps.Generator/SyntaxReceiver/KubernetesEntitySyntaxReceiver.cs b/src/KubeOps.Generator/SyntaxReceiver/KubernetesEntitySyntaxReceiver.cs
@@ -11,7 +11,7 @@ internal class KubernetesEntitySyntaxReceiver : ISyntaxContextReceiver
     private const string VersionName = "ApiVersion";
     private const string DefaultVersion = "v1";
 
-    public List<AttributedEntity> Entities { get; } = new();
+    public List<AttributedEntity> Entities { get; } = [];
 
     public void OnVisitSyntaxNode(GeneratorSyntaxContext context)
     {
diff --git a/src/KubeOps.Transpiler/Crds.cs b/src/KubeOps.Transpiler/Crds.cs
@@ -31,7 +31,7 @@ public static class Crds
     private const string Decimal = "decimal";
     private const string DateTime = "date-time";
 
-    private static readonly string[] IgnoredToplevelProperties = { "metadata", "apiversion", "kind" };
+    private static readonly string[] IgnoredToplevelProperties = ["metadata", "apiversion", "kind"];
 
     /// <summary>
     /// Transpile a single type to a CRD.
diff --git a/src/KubeOps.Transpiler/Rbac.cs b/src/KubeOps.Transpiler/Rbac.cs
@@ -48,17 +48,13 @@ public static IEnumerable<V1PolicyRule> Transpile(
                 group => (
                     Crd: context.ToEntityMetadata(group.Key),
                     Verbs: group.Aggregate(RbacVerb.None, (accumulator, element) => accumulator | element.Verbs)))
-            .GroupBy(group => group.Verbs)
-            .Select(
-                group => (
-                    Verbs: group.Key,
-                    Crds: group.Select(element => element.Crd).ToList()))
+            .GroupBy(group => (group.Crd.Metadata.Group, group.Verbs))
             .Select(
                 group => new V1PolicyRule
                 {
-                    ApiGroups = group.Crds.Select(crd => crd.Metadata.Group).Distinct().ToList(),
-                    Resources = group.Crds.Select(crd => crd.Metadata.PluralName).Distinct().ToList(),
-                    Verbs = ConvertToStrings(group.Verbs),
+                    ApiGroups = [group.Key.Group],
+                    Resources = group.Select(crd => crd.Crd.Metadata.PluralName).Distinct().ToList(),
+                    Verbs = ConvertToStrings(group.Key.Verbs),
                 });
 
         var entityStatus = list
@@ -75,8 +71,8 @@ public static IEnumerable<V1PolicyRule> Transpile(
             .Select(
                 crd => new V1PolicyRule
                 {
-                    ApiGroups = new[] { crd.Metadata.Group },
-                    Resources = new[] { $"{crd.Metadata.PluralName}/status" },
+                    ApiGroups = [crd.Metadata.Group],
+                    Resources = [$"{crd.Metadata.PluralName}/status"],
                     Verbs = ConvertToStrings(RbacVerb.Get | RbacVerb.Patch | RbacVerb.Update),
                 });
 
@@ -86,7 +82,7 @@ public static IEnumerable<V1PolicyRule> Transpile(
     private static string[] ConvertToStrings(RbacVerb verbs) => verbs switch
     {
         RbacVerb.None => Array.Empty<string>(),
-        _ when verbs.HasFlag(RbacVerb.All) => new[] { "*" },
+        _ when verbs.HasFlag(RbacVerb.All) => ["*"],
         _ =>
             Enum.GetValues<RbacVerb>()
                 .Where(v => verbs.HasFlag(v) && v != RbacVerb.All && v != RbacVerb.None)
diff --git a/src/KubeOps.Transpiler/Utilities.cs b/src/KubeOps.Transpiler/Utilities.cs
@@ -92,7 +92,7 @@ public static IList<T> GetCustomAttributeNamedArrayArg<T>(this CustomAttributeDa
         attr.NamedArguments.FirstOrDefault(a => a.MemberName == name).TypedValue.Value is
             ReadOnlyCollection<CustomAttributeTypedArgument> value
             ? value.Select(v => (T)v.Value!).ToList()
-            : new List<T>();
+            : [];
 
     /// <summary>
     /// Load a specific constructor argument from a custom attribute.
@@ -145,7 +145,7 @@ public static IList<T> GetCustomAttributeCtorArrayArg<T>(
         attr.ConstructorArguments[index].Value is
             ReadOnlyCollection<CustomAttributeTypedArgument> value
             ? value.Select(v => (T)v.Value!).ToList()
-            : new List<T>();
+            : [];
 
     /// <summary>
     /// Load a type from a metadata load context.
diff --git a/test/KubeOps.Operator.Test/IntegrationTestCollection.cs b/test/KubeOps.Operator.Test/IntegrationTestCollection.cs
@@ -72,7 +72,7 @@ public async Task DisposeAsync()
 
 public sealed class CrdInstaller : IAsyncLifetime
 {
-    private List<V1CustomResourceDefinition> _crds = new();
+    private List<V1CustomResourceDefinition> _crds = [];
 
     public async Task InitializeAsync()
     {
diff --git a/test/KubeOps.Operator.Test/InvocationCounter.cs b/test/KubeOps.Operator.Test/InvocationCounter.cs
@@ -9,7 +9,7 @@ public class InvocationCounter<TEntity>
     where TEntity : IKubernetesObject<V1ObjectMeta>
 {
     private TaskCompletionSource _task = new();
-    public readonly List<(string Method, TEntity Entity)> Invocations = new();
+    public readonly List<(string Method, TEntity Entity)> Invocations = [];
 
     public Task WaitForInvocations => _task.Task;
 
diff --git a/test/KubeOps.Operator.Web.Test/IntegrationTestCollection.cs b/test/KubeOps.Operator.Web.Test/IntegrationTestCollection.cs
@@ -30,7 +30,7 @@ public abstract class IntegrationTestBase;
 
 public sealed class CrdInstaller : IAsyncLifetime
 {
-    private List<V1CustomResourceDefinition> _crds = new();
+    private List<V1CustomResourceDefinition> _crds = [];
 
     public async Task InitializeAsync()
     {
diff --git a/test/KubeOps.Transpiler.Test/Rbac.Mlc.Test.cs b/test/KubeOps.Transpiler.Test/Rbac.Mlc.Test.cs
@@ -75,6 +75,14 @@ public void Should_Group_Types_With_Same_Verbs_Together()
         roles.Should().HaveCount(2);
     }
 
+    [Fact]
+    public void Should_Not_Mix_ApiGroups()
+    {
+        var roles = _mlc
+            .Transpile(_mlc.GetContextType<RbacTest5>().GetCustomAttributesData<EntityRbacAttribute>()).ToList();
+        roles.Should().HaveCount(5);
+    }
+
     [KubernetesEntity(Group = "test", ApiVersion = "v1")]
     [EntityRbac(typeof(RbacTest1), Verbs = RbacVerb.Get)]
     [EntityRbac(typeof(RbacTest1), Verbs = RbacVerb.Update)]
@@ -102,7 +110,13 @@ public class RbacTest3 : CustomKubernetesEntity;
     public class RbacTest4 : CustomKubernetesEntity;
 
     [KubernetesEntity(Group = "test", ApiVersion = "v1")]
-    [GenericRbac(Urls = new[] { "url", "foobar" }, Resources = new[] { "configmaps" }, Groups = new[] { "group" },
+    [EntityRbac(typeof(V1Deployment), Verbs = RbacVerb.All)]
+    [EntityRbac(typeof(V1Service), Verbs = RbacVerb.All)]
+    [EntityRbac(typeof(V1Lease), Verbs = RbacVerb.All)]
+    public class RbacTest5 : CustomKubernetesEntity;
+
+    [KubernetesEntity(Group = "test", ApiVersion = "v1")]
+    [GenericRbac(Urls = ["url", "foobar"], Resources = ["configmaps"], Groups = ["group"],
         Verbs = RbacVerb.Delete | RbacVerb.Get)]
     public class GenericRbacTest : CustomKubernetesEntity;
 }

Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@ namespace KubeOps.Generator.SyntaxReceiver;`
`5`	`5`
`6`	`6`	`internal class EntityControllerSyntaxReceiver : ISyntaxContextReceiver`
`7`	`7`	`{`
`8`		`- public List<(ClassDeclarationSyntax Controller, string EntityName)> Controllers { get; } = new();`
	`8`	`+ public List<(ClassDeclarationSyntax Controller, string EntityName)> Controllers { get; } = [];`
`9`	`9`
`10`	`10`	`public void OnVisitSyntaxNode(GeneratorSyntaxContext context)`
`11`	`11`	`{`
Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@ namespace KubeOps.Generator.SyntaxReceiver;`
`5`	`5`
`6`	`6`	`internal class EntityFinalizerSyntaxReceiver : ISyntaxContextReceiver`
`7`	`7`	`{`
`8`		`- public List<(ClassDeclarationSyntax Finalizer, string EntityName)> Finalizer { get; } = new();`
	`8`	`+ public List<(ClassDeclarationSyntax Finalizer, string EntityName)> Finalizer { get; } = [];`
`9`	`9`
`10`	`10`	`public void OnVisitSyntaxNode(GeneratorSyntaxContext context)`
`11`	`11`	`{`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ internal class KubernetesEntitySyntaxReceiver : ISyntaxContextReceiver`
`11`	`11`	`private const string VersionName = "ApiVersion";`
`12`	`12`	`private const string DefaultVersion = "v1";`
`13`	`13`
`14`		`- public List<AttributedEntity> Entities { get; } = new();`
	`14`	`+ public List<AttributedEntity> Entities { get; } = [];`
`15`	`15`
`16`	`16`	`public void OnVisitSyntaxNode(GeneratorSyntaxContext context)`
`17`	`17`	`{`
Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,7 @@ public async Task DisposeAsync()`
`72`	`72`
`73`	`73`	`public sealed class CrdInstaller : IAsyncLifetime`
`74`	`74`	`{`
`75`		`- private List<V1CustomResourceDefinition> _crds = new();`
	`75`	`+ private List<V1CustomResourceDefinition> _crds = [];`
`76`	`76`
`77`	`77`	`public async Task InitializeAsync()`
`78`	`78`	`{`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ public class InvocationCounter<TEntity>`
`9`	`9`	`where TEntity : IKubernetesObject<V1ObjectMeta>`
`10`	`10`	`{`
`11`	`11`	`private TaskCompletionSource _task = new();`
`12`		`- public readonly List<(string Method, TEntity Entity)> Invocations = new();`
	`12`	`+ public readonly List<(string Method, TEntity Entity)> Invocations = [];`
`13`	`13`
`14`	`14`	`public Task WaitForInvocations => _task.Task;`
`15`	`15`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ public abstract class IntegrationTestBase;`
`30`	`30`
`31`	`31`	`public sealed class CrdInstaller : IAsyncLifetime`
`32`	`32`	`{`
`33`		`- private List<V1CustomResourceDefinition> _crds = new();`
	`33`	`+ private List<V1CustomResourceDefinition> _crds = [];`
`34`	`34`
`35`	`35`	`public async Task InitializeAsync()`
`36`	`36`	`{`