From bf0fb2aef1390bf4ffbc819a6d962eb8d5b85dbd Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Wed, 11 Jun 2025 14:38:09 +0200 Subject: [PATCH] fix: have service selectors match templates, too (#882) Signed-off-by: Andre Dietisheim --- .../editor/util/KubernetesTypeInfoUtils.kt | 5 + .../editor/util/ResourcePsiElementUtils.kt | 23 +++++ .../intellij/kubernetes/usage/LabelsFilter.kt | 60 ++++++------ .../editor/mocks/ResourcePsiElementMocks.kt | 42 ++++++--- .../util/ResourcePsiElementUtilsTest.kt | 85 ++++++++++++++++- .../kubernetes/usage/LabelsFilterTest.kt | 93 ++++++++++++++++++- 6 files changed, 253 insertions(+), 55 deletions(-) diff --git a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/editor/util/KubernetesTypeInfoUtils.kt b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/editor/util/KubernetesTypeInfoUtils.kt index e9b8e0b28..7e251faf7 100644 --- a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/editor/util/KubernetesTypeInfoUtils.kt +++ b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/editor/util/KubernetesTypeInfoUtils.kt @@ -21,6 +21,7 @@ private const val KIND_PERSISTENT_VOLUME = "PersistentVolume" private const val KIND_PERSISTENT_VOLUME_CLAIM = "PersistentVolumeClaim" private const val KIND_POD = "Pod" private const val KIND_POD_DISRUPTION_BUDGET = "PodDisruptionBudget" +private const val KIND_REPLICATION_CONTROLLER = "ReplicationController" private const val KIND_REPLICA_SET = "ReplicaSet" private const val KIND_SERVICE = "Service" private const val KIND_STATEFUL_SET = "StatefulSet" @@ -61,6 +62,10 @@ fun KubernetesTypeInfo.isPodDisruptionBudget(): Boolean { return this.kind == KIND_POD_DISRUPTION_BUDGET } +fun KubernetesTypeInfo.isReplicationController(): Boolean { + return this.kind == KIND_REPLICATION_CONTROLLER +} + fun KubernetesTypeInfo.isReplicaSet(): Boolean { return this.kind == KIND_REPLICA_SET } diff --git a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/editor/util/ResourcePsiElementUtils.kt b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/editor/util/ResourcePsiElementUtils.kt index e27b7dd94..3c87327e3 100644 --- a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/editor/util/ResourcePsiElementUtils.kt +++ b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/editor/util/ResourcePsiElementUtils.kt @@ -29,6 +29,7 @@ private const val KEY_LABELS = "labels" private const val KEY_SPEC = "spec" private const val KEY_SELECTOR = "selector" private const val KEY_TEMPLATE = "template" +private const val KEY_JOB_TEMPLATE = "jobTemplate" private const val KEY_BINARY_DATA = "binaryData" private const val KEY_DATA = "data" @@ -210,6 +211,7 @@ fun JsonObject.getTemplate(): JsonObject? { return (this.findProperty(KEY_SPEC)?.value as? JsonObject?) ?.findProperty(KEY_TEMPLATE)?.value as? JsonObject? } + fun PsiElement.hasTemplateLabels(): Boolean { return this.getTemplateLabels() != null } @@ -223,12 +225,33 @@ fun PsiElement.getTemplateLabels(): PsiElement? { } } +fun PsiElement.getJobTemplate(): PsiElement? { + return when(this) { + is YAMLMapping -> this.getJobTemplate() + is JsonObject -> this.getJobTemplate() + else -> + null + } +} + +fun YAMLMapping.getJobTemplate(): YAMLMapping? { + return (this.getKeyValueByKey(KEY_SPEC)?.value as? YAMLMapping) + ?.getKeyValueByKey(KEY_JOB_TEMPLATE)?.value as? YAMLMapping +} + +fun JsonObject.getJobTemplate(): JsonObject? { + return (this.findProperty(KEY_SPEC)?.value as? JsonObject?) + ?.findProperty(KEY_JOB_TEMPLATE)?.value as? JsonObject? +} + fun YAMLMapping.getTemplateLabels(): YAMLMapping? { return this.getTemplate()?.getLabels() + ?: getJobTemplate()?.getTemplate()?.getLabels() } fun JsonObject.getTemplateLabels(): JsonObject? { return this.getTemplate()?.getLabels() + ?: getJobTemplate()?.getTemplate()?.getLabels() } /** diff --git a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/usage/LabelsFilter.kt b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/usage/LabelsFilter.kt index 6b2b99796..c5bd42680 100644 --- a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/usage/LabelsFilter.kt +++ b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/usage/LabelsFilter.kt @@ -14,6 +14,7 @@ import com.intellij.psi.PsiElement import com.redhat.devtools.intellij.common.validation.KubernetesTypeInfo import com.redhat.devtools.intellij.kubernetes.editor.util.areMatchingMatchExpressions import com.redhat.devtools.intellij.kubernetes.editor.util.areMatchingMatchLabels +import com.redhat.devtools.intellij.kubernetes.editor.util.getJobTemplate import com.redhat.devtools.intellij.kubernetes.editor.util.getKubernetesTypeInfo import com.redhat.devtools.intellij.kubernetes.editor.util.getLabels import com.redhat.devtools.intellij.kubernetes.editor.util.getResource @@ -21,6 +22,7 @@ import com.redhat.devtools.intellij.kubernetes.editor.util.getTemplateLabels import com.redhat.devtools.intellij.kubernetes.editor.util.hasMatchExpressions import com.redhat.devtools.intellij.kubernetes.editor.util.hasMatchLabels import com.redhat.devtools.intellij.kubernetes.editor.util.hasSelector +import com.redhat.devtools.intellij.kubernetes.editor.util.hasTemplateLabels import com.redhat.devtools.intellij.kubernetes.editor.util.isCronJob import com.redhat.devtools.intellij.kubernetes.editor.util.isDaemonSet import com.redhat.devtools.intellij.kubernetes.editor.util.isDeployment @@ -31,6 +33,7 @@ import com.redhat.devtools.intellij.kubernetes.editor.util.isPersistentVolumeCla import com.redhat.devtools.intellij.kubernetes.editor.util.isPod import com.redhat.devtools.intellij.kubernetes.editor.util.isPodDisruptionBudget import com.redhat.devtools.intellij.kubernetes.editor.util.isReplicaSet +import com.redhat.devtools.intellij.kubernetes.editor.util.isReplicationController import com.redhat.devtools.intellij.kubernetes.editor.util.isService import com.redhat.devtools.intellij.kubernetes.editor.util.isStatefulSet @@ -90,51 +93,42 @@ class LabelsFilter(selector: PsiElement): PsiElementMappingsFilter { private fun canSelect(type: KubernetesTypeInfo): Boolean { val selectorType = selectorResourceType ?: return false return when { - selectorType.isDeployment() -> - type.isPod() - || type.isDeployment() // can select deployment template - - selectorType.isCronJob() -> - type.isPod() - || type.isCronJob() // template - - selectorType.isDaemonSet() -> - type.isPod() - || type.isDaemonSet() // template - - selectorType.isJob() -> - type.isPod() - || type.isJob() // template - - selectorType.isReplicaSet() -> - type.isPod() - || type.isReplicaSet() // template - - selectorType.isStatefulSet() -> - type.isPod() - || type.isStatefulSet() // template - - selectorType.isNetworkPolicy() + selectorType.isDaemonSet() + || selectorType.isDeployment() + || selectorType.isJob() + || selectorType.isNetworkPolicy() || selectorType.isPodDisruptionBudget() - || selectorType.isService() -> + || selectorType.isReplicaSet() + || selectorType.isReplicationController() + || selectorType.isService() + || selectorType.isStatefulSet() -> type.isPod() + || hasPodTemplate(type) selectorType.isPersistentVolumeClaim() -> type.isPersistentVolume() - || type.isPersistentVolumeClaim() else -> false } } + private fun hasPodTemplate(type: KubernetesTypeInfo): Boolean { + return type.isCronJob() + || type.isDaemonSet() + || type.isDeployment() + || type.isJob() + || type.isReplicaSet() + || type.isStatefulSet() + } + override fun getMatchingElement(element: PsiElement): PsiElement? { val labeledType = element.getKubernetesTypeInfo() ?: return null return getLabels(labeledType, element, selectorResourceType) } private fun getLabels( - labeledType: KubernetesTypeInfo, + labeledResourceType: KubernetesTypeInfo, labeledResource: PsiElement, selectorResourceType: KubernetesTypeInfo? ): PsiElement? { @@ -142,12 +136,10 @@ class LabelsFilter(selector: PsiElement): PsiElementMappingsFilter { selectorResourceType == null -> null - (selectorResourceType.isCronJob() && labeledType.isCronJob()) - || (selectorResourceType.isDaemonSet() && labeledType.isDaemonSet()) - || (selectorResourceType.isDeployment() && labeledType.isDeployment()) - || (selectorResourceType.isJob() && labeledType.isJob()) - || (selectorResourceType.isReplicaSet() && labeledType.isReplicaSet()) - || (selectorResourceType.isStatefulSet() && labeledType.isStatefulSet()) -> + labeledResourceType.isCronJob() -> + labeledResource.getJobTemplate()?.getTemplateLabels() + + labeledResource.hasTemplateLabels() -> labeledResource.getTemplateLabels() else -> diff --git a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/editor/mocks/ResourcePsiElementMocks.kt b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/editor/mocks/ResourcePsiElementMocks.kt index e34c94def..f3ebacd85 100644 --- a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/editor/mocks/ResourcePsiElementMocks.kt +++ b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/editor/mocks/ResourcePsiElementMocks.kt @@ -35,6 +35,7 @@ private const val KEY_METADATA = "metadata" private const val KEY_SELECTOR = "selector" private const val KEY_SPEC = "spec" private const val KEY_TEMPLATE = "template" +private const val KEY_JOB_TEMPLATE = "jobTemplate" fun YAMLMapping.createLabels(labelsChildren: YAMLMapping): YAMLKeyValue { val metadataChildren = mock() @@ -54,23 +55,25 @@ fun JsonObject.createLabels(labelsChildren: JsonObject): JsonProperty { } fun YAMLMapping.createTemplate(templateChildren: YAMLMapping): YAMLKeyValue { - var spec = getKeyValueByKey(KEY_SPEC) - if (spec == null) { - val specMapping = mock() - spec = createYAMLKeyValue(KEY_SPEC, specMapping, this) - } + val spec = getOrCreateSpec() return createYAMLKeyValue(KEY_TEMPLATE, templateChildren, spec.value as YAMLMapping) } fun JsonObject.createTemplate(templateChildren: JsonObject): JsonProperty { - var spec = findProperty(KEY_SPEC) - if (spec == null) { - val specMapping = mock() - spec = createJsonProperty(KEY_SPEC, specMapping, this) - } + val spec = getOrCreateSpec() return createJsonProperty(KEY_TEMPLATE, templateChildren, spec.value as JsonObject) } +fun YAMLMapping.createJobTemplate(templateChildren: YAMLMapping): YAMLKeyValue { + val spec = getOrCreateSpec() + return createYAMLKeyValue(KEY_JOB_TEMPLATE, templateChildren, spec.value as YAMLMapping) +} + +fun JsonObject.createJobTemplate(templateChildren: JsonObject): JsonProperty { + val spec = getOrCreateSpec() + return createJsonProperty(KEY_JOB_TEMPLATE, templateChildren, spec.value as JsonObject) +} + fun YAMLMapping.createMatchLabels(matchLabels: YAMLMapping): YAMLKeyValue { var selectorChildren = getSelector() if (selectorChildren == null) { @@ -119,21 +122,32 @@ fun createYAMLSequenceItem(key: String, operator: String, values: List): } fun YAMLMapping.createSelector(selectorChildren: YAMLMapping = mock()): YAMLKeyValue { + val spec = getOrCreateSpec() + return createYAMLKeyValue(KEY_SELECTOR, selectorChildren, spec.value as YAMLMapping) +} + +private fun YAMLMapping.getOrCreateSpec(): YAMLKeyValue { var spec = getKeyValueByKey(KEY_SPEC) if (spec == null) { val specChildren = mock() spec = createYAMLKeyValue(KEY_SPEC, specChildren, this) } - return createYAMLKeyValue(KEY_SELECTOR, selectorChildren, spec.value as YAMLMapping) + return spec } + fun JsonObject.createSelector(selectorChildren: JsonObject = mock()): JsonProperty { + val spec = getOrCreateSpec() + return createJsonProperty(KEY_SELECTOR, selectorChildren, spec.value as JsonObject) +} + +private fun JsonObject.getOrCreateSpec(): JsonProperty { var spec = findProperty(KEY_SPEC) if (spec == null) { - val specChildren = mock() - spec = createJsonProperty(KEY_SPEC, specChildren, this) + val specChild = mock() + spec = createJsonProperty(KEY_SPEC, specChild, this) } - return createJsonProperty(KEY_SELECTOR, selectorChildren, spec.value as JsonObject) + return spec } fun YAMLMapping.createMetadata(): YAMLMapping { diff --git a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/editor/util/ResourcePsiElementUtilsTest.kt b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/editor/util/ResourcePsiElementUtilsTest.kt index 9ece43712..516dd1915 100644 --- a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/editor/util/ResourcePsiElementUtilsTest.kt +++ b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/editor/util/ResourcePsiElementUtilsTest.kt @@ -19,6 +19,7 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.whenever import com.redhat.devtools.intellij.common.validation.KubernetesTypeInfo +import com.redhat.devtools.intellij.kubernetes.editor.mocks.createJobTemplate import com.redhat.devtools.intellij.kubernetes.editor.mocks.createJsonObject import com.redhat.devtools.intellij.kubernetes.editor.mocks.createJsonProperty import com.redhat.devtools.intellij.kubernetes.editor.mocks.createLabels @@ -35,6 +36,7 @@ import org.jetbrains.yaml.psi.YAMLMapping import org.junit.Before import org.junit.Test import org.mockito.MockedStatic +import kotlin.collections.listOf class ResourcePsiElementUtilsTest { @@ -294,6 +296,19 @@ class ResourcePsiElementUtilsTest { assertThat(result).isSameAs(templateMapping) } + @Test + fun `#getJobTemplate for YAML returns jobTemplate mapping`() { + // given + val specMapping = mock() + createYAMLKeyValue("spec", specMapping, yamlElement) + val templateMapping = mock() + createYAMLKeyValue("jobTemplate", templateMapping, specMapping) + // when + val result = yamlElement.getJobTemplate() + // then + assertThat(result).isSameAs(templateMapping) + } + @Test fun `#getTemplateLabel for YAML returns template labels`() { // given @@ -319,10 +334,43 @@ class ResourcePsiElementUtilsTest { assertThat(found).isSameAs(templateLabels) } + @Test + fun `#getJobTemplateLabel for YAML returns template labels`() { + // given + val jobTemplateLabels = createYAMLMapping(listOf( + createYAMLKeyValue("jedi", "skywalker") + )) + yamlElement.createJobTemplate( + createYAMLMapping(listOf( + createYAMLKeyValue("spec", + createYAMLMapping(listOf( + createYAMLKeyValue("template", + createYAMLMapping(listOf( + createYAMLKeyValue( + "metadata", + createYAMLMapping(listOf( + createYAMLKeyValue( + "labels", + jobTemplateLabels + ) + )) + ) + )) + ) + )) + ) + )) + ) + // when + val found = yamlElement.getJobTemplate()?.getTemplateLabels() + // then + assertThat(found).isSameAs(jobTemplateLabels) + } + @Test fun `#getTemplateLabel for Json returns template labels`() { // given - val templateLabels = createJsonObject(listOf( + val jobTemplateLabels = createJsonObject(listOf( createJsonProperty("jedi", "skywalker") )) jsonElement.createTemplate( @@ -332,7 +380,7 @@ class ResourcePsiElementUtilsTest { createJsonObject(listOf( createJsonProperty( "labels", - templateLabels + jobTemplateLabels ) )) ) @@ -341,6 +389,39 @@ class ResourcePsiElementUtilsTest { // when val found = jsonElement.getTemplateLabels() // then + assertThat(found).isSameAs(jobTemplateLabels) + } + + @Test + fun `#getJobTemplateLabel for Json returns template labels`() { + // given + val templateLabels = createJsonObject(listOf( + createJsonProperty("jedi", "skywalker") + )) + jsonElement.createJobTemplate( + createJsonObject(listOf( + createJsonProperty("spec", + createJsonObject(listOf( + createJsonProperty("template", + createJsonObject(listOf( + createJsonProperty( + "metadata", + createJsonObject(listOf( + createJsonProperty( + "labels", + templateLabels + ) + )) + ) + )) + ) + )) + ) + )) + ) + // when + val found = jsonElement.getJobTemplate()?.getTemplateLabels() + // then assertThat(found).isSameAs(templateLabels) } diff --git a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/usage/LabelsFilterTest.kt b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/usage/LabelsFilterTest.kt index d377511e3..ce6cda340 100644 --- a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/usage/LabelsFilterTest.kt +++ b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/usage/LabelsFilterTest.kt @@ -188,12 +188,12 @@ class LabelsFilterTest { } @Test - fun `#isAccepted returns true if deployment is selector and filtered element is deployment with matching template labels`() { + fun `#isAccepted returns true if deployment has selector that matches labels in it's template`() { // given - val matchingTemplateLabels = createYAMLMapping(listOf( + val hasMatchingTemplateLabels = createYAMLMapping(listOf( createYAMLKeyValue("kind", "Deployment") )) - matchingTemplateLabels.createTemplate( + hasMatchingTemplateLabels.createTemplate( createYAMLMapping(listOf( createYAMLKeyValue( "metadata", @@ -201,7 +201,7 @@ class LabelsFilterTest { createYAMLKeyValue( "labels", createYAMLMapping(listOf( - createYAMLKeyValue(LABEL_KEY, LABEL_VALUE) // matching labels in spec>template + createYAMLKeyValue(LABEL_KEY, LABEL_VALUE) // matching labels are in spec>template )) ) )) @@ -209,11 +209,94 @@ class LabelsFilterTest { )) ) // when - val isAccepted = filter.isAccepted(matchingTemplateLabels) + val isAccepted = filter.isAccepted(hasMatchingTemplateLabels) // filter created with selector using LABEL_KEY, LABEL_VALUE // then assertThat(isAccepted).isTrue() } + @Test + fun `#isAccepted returns true if service has selector that is matching labels in a template of a cron job`() { + // given + val serviceWithSelector = createYAMLMapping(listOf( + createYAMLKeyValue("kind", "Service"), + createYAMLKeyValue("spec", + createYAMLMapping(listOf( + createYAMLKeyValue("selector", + createYAMLMapping(listOf( + createYAMLKeyValue(LABEL_KEY, LABEL_VALUE) + )) + ) + )) + ) + )) + val cronJob = createCronJob(LABEL_KEY, LABEL_VALUE) + val filter = LabelsFilter(serviceWithSelector) + // when + val isAccepted = filter.isAccepted(cronJob) + // then + assertThat(isAccepted).isTrue() + } + + @Test + fun `#isAccepted returns false if service has selector that is NOT matching labels in a template of a cron job`() { + // given + val serviceWithSelector = createYAMLMapping(listOf( + createYAMLKeyValue("kind", "Service"), + createYAMLKeyValue("spec", + createYAMLMapping(listOf( + createYAMLKeyValue("selector", + createYAMLMapping(listOf( + createYAMLKeyValue(LABEL_KEY, LABEL_VALUE) + )) + ) + )) + ) + )) + val cronJob = createCronJob("NOT_LABEL_KEY", "NOT_LABEL_KEY") + val filter = LabelsFilter(serviceWithSelector) + // when + val isAccepted = filter.isAccepted(cronJob) + // then + assertThat(isAccepted).isFalse() + } + + private fun createCronJob(key: String, value: String): YAMLMapping = createYAMLMapping( + listOf( + createYAMLKeyValue("kind", "CronJob"), + createYAMLKeyValue( + "spec", + createYAMLMapping(listOf( + createYAMLKeyValue( + "jobTemplate", + createYAMLMapping(listOf( + createYAMLKeyValue( + "spec", + createYAMLMapping(listOf( + createYAMLKeyValue( + "template", + createYAMLMapping(listOf( + createYAMLKeyValue( + "metadata", + createYAMLMapping(listOf( + createYAMLKeyValue( + "labels", + createYAMLMapping(listOf( + createYAMLKeyValue(key, value) + )) + ) + )) + ) + )) + ) + )) + ) + )) + ) + )) + ) + ) + ) + @Test fun `#getMatchingElement returns pod labels that match given deployment selector`() { // given