Folder permissions reconciler #821
Conversation
|
@Iridias I found the issue with the e2e test. It was a simple copy-paste issue containing the rbac settings. |
There was a problem hiding this comment.
Nit, else I think it looks good.
If you can add some docs about the new feature in https://github.com/grafana-operator/grafana-operator/blob/master/documentation/README.md that would be great.
Maybe create a new file in https://github.com/grafana-operator/grafana-operator/blob/master/documentation/ called folder.md or something like that and write a few rows about the feature.
Thanks allot for your contribution @Iridias!
| // Run the actions to reach the desired state | ||
| actionRunner := common.NewClusterActionRunner(ctx, r.Client, r.Scheme, cr) | ||
| err = actionRunner.RunAll(desiredState) | ||
| if err != nil { | ||
| return r.manageError(cr, err, request) | ||
| } | ||
|
|
||
| log.V(1).Info("Now processing configMaps...") |
There was a problem hiding this comment.
The reconciler loop is already rather chatty. Is it worth writing this log for every run?
|
|
||
| var logger = logf.Log.WithName("folder-grafana-client") | ||
|
|
||
| // FIXME: client is not controller-specific - refactor and avoid duplications |
There was a problem hiding this comment.
This will be solved in version 5. You can remove the comment.
| FolderName string `json:"title"` | ||
| FolderPermissions []GrafanaPermissionItem `json:"permissions,omitempty"` |
There was a problem hiding this comment.
It is kind of self explanatory but it would be nice with some extra comments explaining what the different CRD values does.
Maybe copy the description from the grafana API docs.
| log.Log.Info("dashboard found but selectors do not match", "namespace", folder.Namespace, "name", folder.Name) | ||
| continue | ||
| } | ||
| // FIXME: implementation |
| return NewGrafanaClient(url, username, password, r.transport, duration), nil | ||
| } | ||
|
|
||
| // Handle success case: update dashboard metadata (id, uid) and update the list |
There was a problem hiding this comment.
Please update this comment.
There was a problem hiding this comment.
LGTM! @Iridias thanks for implementing this.
I verified this locally on minikube, everything seems to be working as intended.
Upgrade path also works seamlessly between unmanaged folders on the main branc, to your branch, and fully transition to your logic without issues. Great job!
Description
Sure
Relevant issues/tickets
#809