Add support for managing HumioGroup and roles assignment to groups in HumioOperator #964

bowens-stripe · 2025-04-23T20:16:46Z

Overview

as title, this PR adds support for basic CRUD of HumioGroups in HumioOperator to manage user groups in LogScale, as well as the functionalities to assign and unassign (existing) roles to / from these groups that are associated with (existing) views.

I am using this PR which added HumioView heavily as a reference, as well as existing code for HumioRepository

Test

make test passes locally. Please feel free to advice on additional testing strategies as needed!

wxu-stripe

Some comments, nothing is hard blocking

wxu-stripe · 2025-04-24T15:18:28Z

internal/api/humiographql/graphql/groups.graphql

+    }
+}
+
+mutation CreateGroup(


Maybe keep the name consistent as AddGroup?

Create* is consistent with the function naming for other resources though, so we either match with the graphQL API name or follow the same convention as repos and views for example. I think Create* is cleaner overall and makes the golang code easier to find at least

wxu-stripe · 2025-04-24T15:29:22Z

api/v1alpha1/humiogroup_types.go

+
+const (
+	// HumioGroupStateUnknown is the Unknown state of the group
+	HumioGroupStateUnknown = "Unknown"


Any idea how we get this state?

line 116 in the controller code sets this state, looks like its from generic API errors

wxu-stripe · 2025-04-24T15:38:00Z

charts/humio-operator/crds/core.humio.com_humiogroups.yaml

+              externalClusterName:
+                description: |-
+                  ExternalClusterName refers to an object of type HumioExternalCluster where the Humio resources should be created.
+                  This conflicts with ManagedClusterName.


Do we want validation to make sure they're not both provided?

the validation happens here https://github.com/bowens-stripe/humio-operator/blob/487b7742b31f1d92254fe6357195379cb83605ec/internal/helpers/clusterinterface.go#L52-L58

wxu-stripe · 2025-04-24T15:40:22Z

internal/api/humiographql/graphql/groups.graphql

+mutation DeleteGroup(
+    $GroupId: String!
+) {
+    removeGroup(


Similar here, tho maybe not as bad, since your go function has the same naming.

wxu-stripe · 2025-04-24T15:48:35Z

internal/controller/humiogroup_controller.go

+	defer func(ctx context.Context, hg *humiov1alpha1.HumioGroup) {
+		_, err := r.HumioClient.GetGroup(ctx, humioHttpClient, req, hg)
+		if errors.As(err, &humioapi.EntityNotFound{}) {
+			_ = r.setState(ctx, humiov1alpha1.HumioGroupStateNotFound, hg)


We're ignoring the errors here? Maybe at least log it?

I believe the "finalizer" acts on this error state "eventually". for this part I am mostly copying the existing logic from other controllers, would love if someone from CrowdStrike can confirm this is correct though

SaaldjorMike · 2025-05-13T10:42:28Z

@bowens-stripe Thank you for this contribution. Much appreciated.

I do have a few changes I'd like to see before this is ready to get merged in. Would you mind if I take your branch and make some adjustments to it?

Can you tell me if you strictly need support for assigning existing roles to the group (as in, roles that are not managed by the operator), or if it would be OK to only allow role assignments to roles that the operator manages?

For context, LogScale supports 3 different "types/classes" of permissions, and the current state of this PR only deals with 1 of the 3 types, namely "view permissions". We intend to add support for all of these 3 types of role permissions and mechanisms for assigning them to groups. If you're able to answer my two questions above, then I'd like to pick this up and continue the work based on your contribution.

bowens-stripe · 2025-05-13T19:05:15Z

@SaaldjorMike I just added you as a collaborator on my fork, feel free to add anything to the branch!

Can you tell me if you strictly need support for assigning existing roles to the group (as in, roles that are not managed by the operator), or if it would be OK to only allow role assignments to roles that the operator manages?

I dont really "need" that, but I was just planning to use the Admin/Member/Reader role created automatically for us with these groups. If that's not the recommended way I can also make fresh roles to go along with the groups. (which needs a separate PR to create roles in humio-operator probably)

bowens-stripe · 2025-05-13T19:08:31Z

@SaaldjorMike
separately, do you think this PR with only "view permissions" as-is is merge-able? Or you would rather get all 3 covered and merged them together? (and if the latter, do you have a rough time estimate for that?)

asking because it affects planning on our side to decide what to pick up next

SaaldjorMike · 2025-05-14T06:19:55Z

@SaaldjorMike I just added you as a collaborator on my fork, feel free to add anything to the branch!

Can you tell me if you strictly need support for assigning existing roles to the group (as in, roles that are not managed by the operator), or if it would be OK to only allow role assignments to roles that the operator manages?

I dont really "need" that, but I was just planning to use the Admin/Member/Reader role created automatically for us with these groups. If that's not the recommended way I can also make fresh roles to go along with the groups. (which needs a separate PR to create roles in humio-operator probably)

Awesome. I'll probably end up just pushing a branch directly to the upstream repo based on your branch, and continue working on it there. This way all the github actions workflows that runs test and so on will work to validate the work.

@SaaldjorMike separately, do you think this PR with only "view permissions" as-is is merge-able? Or you would rather get all 3 covered and merged them together? (and if the latter, do you have a rough time estimate for that?)

asking because it affects planning on our side to decide what to pick up next

I do not see this is mergeable just yet, but I should be able to pick this one up alongside support for all the types of roles (which I'm already working on), including ways of assigning the roles to groups. The primary reason is that I'd like to move the group assignment part into the new CRD's for the roles for each of the 3 types of permissions. The primary reasoning is to keep the "who can assign this system permission role" as part of the CRUD of the "system permission role" itself, rather than on the "generic" group. I believe this creates a slightly better situation for the user that'll be setting up k8s RBAC rules for who can manage which types of roles and which groups they are assigned to.

My overall plan is like this:

Merge support for CRUD of the 3 types of roles. I already have Introduce HumioSystemPermissionRole CRD #974 and Introduce HumioOrganizationPermissionRole CRD #975 that'll soon be ready. I just need the last one, which is the one for "view permissions", which is what you're asking for as part of this PR.
Once the 3 new CRD's have been introduced, I'll be continuing the work started here. And I intend to push the comments to the upstream repo, then first remove the "assignments" part of HumioGroup. Get that updated variant of this code for HumioGroup merged in. Perhaps there's a few more changes, I'll figure that out.
At this stage there'll be 4 new CRD's. 1 CRD for HumioGroup. 3 CRD's for roles, one per type of permission. I'll then be updating the 3 CRD's for roles to include which groups the roles are assigned to. So I intend to move the "which groups is this role assigned to" onto the role itself, rather than part of the HumioGroup CRD.

To answer your question on an estimate for when: I intend to get all the 4 new CRD's wrapped up this month.

SaaldjorMike · 2025-05-14T06:25:46Z

I've opened #976 which uses the upstream branch that is based on your work. I'll circle back to working on the HumioViewPermissionRole CRD shortly, and after that I'll be doing the updates outlined above in the branch for #976

bowens-stripe · 2025-05-15T04:57:52Z

@SaaldjorMike sounds good, thank you! I just subscribed myself to #976 , please keep us posted :)

SaaldjorMike · 2025-05-19T10:01:14Z

@bowens-stripe Quick update. I've now merged the CRD's for roles and groups today. This means I now have all the building blocks I need to extend the role CRD's with mechanisms for assigning the roles to groups.

bowens-stripe · 2025-05-19T15:53:29Z

@SaaldjorMike thank you for the update! are there plans to cut a release with the new CRDs now or that's coming after the role assignments piece?

SaaldjorMike · 2025-05-19T16:49:38Z

@SaaldjorMike thank you for the update! are there plans to cut a release with the new CRDs now or that's coming after the role assignments piece?

Currently I do not intend to push out a release until the role assignment work is done, as I didn't feel like it was of as much use without the role assignment mechanisms. I expect I'll be able to get the role assignment work done within days rather than weeks so I thought I might as well just wait until that lands before cutting a new release.

We can adjust the plans if needed, so if you think it is useful for us to prioritize cutting the new release before the role assignment work is done, let me know and I'll see what I can do.

bowens-stripe · 2025-05-19T17:12:25Z

I expect I'll be able to get the role assignment work done within days rather than weeks so I thought I might as well just wait until that lands before cutting a new release.

that's awesome to know! waiting until after that sounds good to me, thank you! @SaaldjorMike

SaaldjorMike · 2025-05-21T11:07:20Z

@bowens-stripe Quick update. I just opened up #984 to add some functionality for assigning roles to groups. It may still need a bit more work, but I've shared this around internally for some review/feedback.

SaaldjorMike · 2025-05-22T10:40:47Z

@bowens-stripe I'm going to close this PR now. The addition of the new CRD's have already been merged, and as soon as #984 gets merged, I'll see what I can do about getting a new release out with these features.

Thank you again for your contributions here. Much appreciated.

bowens-stripe requested a review from a team as a code owner April 23, 2025 20:16

bowens-stripe force-pushed the bowens-add-humio-group branch from d8f6242 to c9f0a01 Compare April 23, 2025 20:42

bowens-stripe added 3 commits April 23, 2025 13:46

add HumioGroup CRUD to operator

375434e

handle role assignments for HumioGroups

dc602b4

bump version number

95e0b91

bowens-stripe force-pushed the bowens-add-humio-group branch from 4571097 to 95e0b91 Compare April 23, 2025 20:46

wxu-stripe reviewed Apr 24, 2025

View reviewed changes

bowens-stripe and others added 2 commits April 24, 2025 09:14

fix a state usage

af8b477

Merge branch 'humio:master' into bowens-add-humio-group

df649ac

SaaldjorMike mentioned this pull request May 15, 2025

Introduce CRD's HumioSystemPermissionRole, HumioOrganizationPermissionRole, HumioViewPermissionRole #978

Merged

SaaldjorMike closed this May 22, 2025

Add support for managing HumioGroup and roles assignment to groups in HumioOperator #964

Add support for managing HumioGroup and roles assignment to groups in HumioOperator #964

Uh oh!

Conversation

bowens-stripe commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Test

Uh oh!

wxu-stripe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SaaldjorMike commented May 13, 2025

Uh oh!

bowens-stripe commented May 13, 2025

Uh oh!

bowens-stripe commented May 13, 2025

Uh oh!

SaaldjorMike commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SaaldjorMike commented May 14, 2025

Uh oh!

bowens-stripe commented May 15, 2025

Uh oh!

SaaldjorMike commented May 19, 2025

Uh oh!

bowens-stripe commented May 19, 2025

Uh oh!

SaaldjorMike commented May 19, 2025

Uh oh!

bowens-stripe commented May 19, 2025

Uh oh!

SaaldjorMike commented May 21, 2025

Uh oh!

SaaldjorMike commented May 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bowens-stripe commented Apr 23, 2025 •

edited

Loading

SaaldjorMike commented May 14, 2025 •

edited

Loading