Skip to content

Commit

Permalink
fix: Use CSS for decoration, fixes CodeQL error with TagList (#3261)
Browse files Browse the repository at this point in the history
Also see
#1803 (review)
(you have to open this thing:)

<img width="855" alt="Screenshot 2024-12-04 at 10 35 28"
src="https://github.com/user-attachments/assets/69eaa2e8-4bb0-4d55-8968-0f8f5748b359">

---

This should fix an over-zealous CodeQL linting security issue/error by
using CSS (a super powerful, declarative and therefore pretty much bug
free, "from The Outside" language) instead of Javascript.

I think this should probably be an almost global type of rule (I would
like to delete `TagList` at some point), and I think it would be a good
candidate for something similar to `XLayout`, probably called `XTheme`
which we would primarily use at the root of our application.

Finally we won't know for sure if this fixes the issue without merging
to master, but I think its a better approach even if it doesn't fix the
issue.

<img width="1478" alt="Screenshot 2024-12-04 at 10 28 40"
src="https://github.com/user-attachments/assets/2dfb6460-7999-4ea8-8f11-7eb349ea85bc">

Signed-off-by: John Cowen <[email protected]>
  • Loading branch information
johncowen authored Dec 4, 2024
1 parent 90baf72 commit 6a814ae
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 7 deletions.
19 changes: 13 additions & 6 deletions packages/kuma-gui/src/app/common/TagList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
<XBadge
v-for="(tag, index) in tagList"
:key="index"
class="tag"
:appearance="tag.isKuma ? 'info' : 'neutral'"
class="tag kv"
:data-kv-key="tag.label"
:data-kv-owner="tag.label.split('/')[0]"
>
<component
:is="tag.route ? 'XAction' : 'span'"
Expand All @@ -31,7 +32,6 @@ import type { RouteLocationNamedRaw } from 'vue-router'
interface LabelValueWithRoute extends LabelValue {
route: RouteLocationNamedRaw | undefined
isKuma: boolean
}
const props = withDefaults(defineProps<{
Expand All @@ -49,9 +49,8 @@ const tagList = computed<LabelValueWithRoute[]>(() => {
return labels.map((tag) => {
const { label, value } = tag
const route = getRoute(tag)
const isKuma = label.includes('.kuma.io/') || label.startsWith('kuma.io/')
return { label, value, route, isKuma }
return { label, value, route }
})
})
const shouldTruncate = computed(() => props.shouldTruncate || Object.keys(tagList.value).length > 10)
Expand Down Expand Up @@ -93,8 +92,16 @@ function getRoute(tag: LabelValue): RouteLocationNamedRaw | undefined {
}
}
</script>

<style lang="scss" scoped>
.kv:not(:where(
[data-kv-owner$='.kuma.io'],
[data-kv-owner^='kuma.io']
)) {
background-color: $kui-color-background-neutral-weaker !important;
color: $kui-color-text-neutral-strong !important;
}
.tag-list {
display: inline-flex;
flex-wrap: wrap;
Expand Down
15 changes: 14 additions & 1 deletion packages/kuma-gui/src/test-support/FakeKuma.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,20 @@ gbXR5RnEs0hDxugaIknJMKk1b0g=
}

tags<T extends Record<string, string | undefined>>({ protocol, service, zone }: T): Tags<T> {
const additionalTags = Object.fromEntries(this.faker.helpers.multiple(() => [this.faker.hacker.noun(), this.faker.hacker.noun()], { count: this.faker.number.int({ min: 1, max: 3 }) }))
const additionalTags = Object.fromEntries(
this.faker.helpers.multiple(
() => [
this.faker.helpers.arrayElement([
this.faker.hacker.noun(),
'k8s.kuma.io/service-name',
'not-kuma.io/service',
'not/kuma.io/service',
]),
this.faker.hacker.noun(),
],
{ count: this.faker.number.int({ min: 1, max: 3 }) },
),
)

// @ts-ignore TS isn’t happy when the service tag is not always provided, but I don’t know how to type this out better.
return {
Expand Down

0 comments on commit 6a814ae

Please sign in to comment.