Skip to content
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

feat(policy-summary): add spec to structured view #3579

Merged
merged 3 commits into from
Feb 24, 2025

Conversation

schogges
Copy link
Contributor

@schogges schogges commented Feb 20, 2025

This is the first part and in a follow-up I'll add more view types for the config. Here I added the spec in a code block to the structured view, so the user can still get all information in this view and doesn't need to switch to YAML.


I added a prop to hide the Copy as K8s button from the code block. I think the spec is agnostic to universal and k8s.


image

Part of #3489

Signed-off-by: schogges <moritz.fleck@konghq.com>
@schogges schogges requested a review from a team as a code owner February 20, 2025 17:27
@schogges schogges requested review from johncowen and removed request for a team February 20, 2025 17:27
Copy link

netlify bot commented Feb 20, 2025

Deploy Preview for kuma-gui ready!

Name Link
🔨 Latest commit d05c9b5
🔍 Latest deploy log https://app.netlify.com/sites/kuma-gui/deploys/67bc327a07a71200080ceab0
😎 Deploy Preview https://deploy-preview-3579--kuma-gui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@johncowen johncowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still looking at this but just spotted something.

Totally not related to this PR, but this looks like it could be a pre-existing bug to me? wdyt?

Screenshot 2025-02-21 at 11 20 12

I'm not totally sure what logic it uses to make the targetRef say [Mesh] instead of [MeshService]?

Copy link
Contributor

@johncowen johncowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I had one tiny suggestion, and a non-important curiosity question. Otherwise I reckon this is good to go! lmk on those

@johncowen
Copy link
Contributor

Totally not related to this PR, but this looks like it could be a pre-existing bug to me? wdyt?

Oh wait, I think this depends on a topLevel targetRef, so I think the screengrab I posted here is correct?

@schogges
Copy link
Contributor Author

Totally not related to this PR, but this looks like it could be a pre-existing bug to me? wdyt?

Oh wait, I think this depends on a topLevel targetRef, so I think the screengrab I posted here is correct?

We fall back to Mesh in case spec.targetRef is not set 🤔
https://github.com/kumahq/kuma-gui/blob/master/packages/kuma-gui/src/app/policies/components/PolicySummary.vue#L20-L30

Signed-off-by: schogges <moritz.fleck@konghq.com>
@schogges schogges merged commit e614095 into kumahq:master Feb 24, 2025
19 checks passed
@schogges schogges deleted the feat/summaries/add_more_info branch February 24, 2025 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants