-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the daemon config UI #909
Conversation
@jw3 I'm trying to implement the |
✔️ There is also something going on that causes the unsaved rule changes dialog to always pop up, even when no rules have been edited. |
@egbicker do you have tests coming for the new stuff? |
@jw3 Just got them pushed |
|
||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whats going on here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's trying to load the changeset data and now we have 2 Changeset
that return as string and the RuleChangeset
has additional exceptions in the rust layer (I think its something about having 0 rules)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering about the nesting of the second try. Was that intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was but not like that. Good catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking pretty good. A few things
- Noted the nested exception handling above for clarification
- I still saw the unsaved rule notification, but that looks like a problem with the rules view state management Text editor state issues #912
- Can a summary of config changes be added to this view? What bindings do we need added for this?
Added #912 describing some related concerns. Doesnt have to be fixed for this PR, but could be if simple enough. Another thought with this is that the unsaved rule edit check would apply to config as well. |
@jw3 I believe we'd need a binding like |
#913 is on the way |
Add a binding function for diffing the configuration between two system configurations. The initial implementation here is for API and basic functionality to support #909. There are improvements that will follow to eliminate false positives.
Adds the user interface components for fapolicyd configuration editing and deployment.
Closes #830