feat(dpp): adds rules API data to policies tab #461
Conversation
✅ Deploy Preview for kuma-gui ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
kleinfreund
changed the title
There was a problem hiding this comment.
So questions and nits but this is already looking very cool and such an improvement over the old UI.
I think we should be able to improve a bit the from entry it wasn't clear to me that backend and cluster-1.backend-02 were services in this view:
I'll try to think some more what could be done but if you have ideas don't hesitate :)
@lahabana Does something like the following make sense?
Screenshot
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return props.tags.map((tag) => { | ||
| const { label, value } = tag | ||
| const route = getRoute(tag) | ||
| const isKumaIoLabel = label.toLowerCase().includes('kuma.io/') |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization
|
I like the fact that it still looks nice even with long names
|
|
Something that's probably not related is that refreshing a page causes a 404 and you have to go back to the root of the page because none of the links "in the middle" work as well (so opening a new tab and linking to a specific resource does not work). refresh_404.mov |
|
The IPs have multiple ports, it's probably a bug in the mocked response but just making sure
|
@slonka This mock data for |
|
What's the difference between
I thought it meant "all" but then I saw this:
so I thought why are there two ways of presenting "all". ? |
@slonka There are technically three scenarios that are relevant here:
This (specifically the third point) might be confusing either if we do it or if we don’t (summoning @jakubdyszkiewicz and @lahabana who both had light opinions about this) so I’d greatly appreciate some input on whether this difference of "this DPP" versus "all services" is significant. In the case of the second point, I think this should definitely show the wildcard service tag because that’s what’s coming from the API. |
|
Sometimes I'm not really sure if a "thing" I'm noticing is just the way mocked data is set-up or there might be something in the GUI code. Maybe we could think about re-doing the mocked data to better reflect a real scenario. |
I'd have an explicit "This DPP" instead of —.
Just to make sure I understand "this DPP" - it means that it was selected by some selector but not via "all services" wildcard ("*") - right? EDIT: |
@slonka Happy to run a poll for this. I initially had exactly that, but through feedback and iteration, dropped it again.
@slonka Not sure I get you right, but really "this DPP" just means "the DPP you’re looking at right now for which you went to the Policies tab". I don’t know if the wildcard service tag implies semantically the same source or if there is a distinction. |
|
Also, have we thought about explicitly stating that the top ones are "source & destination" policies and the bottom ones instead of "Rules" - "targetRef policies"? WDYT @lahabana ? |
@slonka I thought about that, but I couldn’t come up with a handy name that makes sense to users. Happy to hear suggestions because I would also prefer to have a heading for both. In the long run (but perhaps not now), we would not split these structures up I think so then it would be fine to not have a heading at all (the tab would be the quasi heading). |
|
I don't think It means "not applicable" and because of that it feels like MeshAccessLog can't have a "from" section in general |
I think there are some subtle differences here overall and so I’d probably prefer to go with the non-significant em dash ("—"). |
There was a problem hiding this comment.
I had a quick look through the code and had a few questions. Apart from that my first thought was, do we want to resize that imagery down to the size we are using? I'm guessing we can't convert them all to svgs, which is a shame - there's no way of easily removing the white background from that big jpg is there? Then at least they are all transparent pngs.
@johncowen I haven’t touched these images at all and left them exactly as I found them in the Kuma docs project. I’d like to get a complete set of images, and resize and compress them appropriately, but I’d like to do this once only.
In theory, these icons could be turned into SVGs, but that’s quite a lot of work for a rather specific use case that I’m not willing to do at this point. I also don’t see a strong advantage in having those be SVGs either. Bitmap graphics for this use case are fine.
Why should these have transparent backgrounds? They aren’t designed to be background color-agnostic and some of them would in fact look horrible on anything that’s not close to white. |
Adds rules from the inspect API to the policies tab. Reworks layout for policies tab content to be grouped by policy type. Signed-off-by: Philipp Rudloff <philipp.rudloff@konghq.com>
Changes
Adds rules from the inspect API to the policies tab.
Reworks layout for policies tab content to be grouped by policy type.
Resolves #387.
Signed-off-by: Philipp Rudloff philipp.rudloff@konghq.com
Screenshots
Before:
After: