Skip to content

Commit c02ace3

Browse files
robobariorobobario
authored
Merge pull request kroxylicious#1994 from robobario/namespace-in-error
Include namespace in ResolveRefs message
2 parents ab79555 + 6a285df commit c02ace3

File tree

12 files changed

+84
-53
lines changed

12 files changed

+84
-53
lines changed

‎kroxylicious-operator/src/main/java/io/kroxylicious/kubernetes/operator/ClusterCondition.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,16 @@
88

99
import java.util.Set;
1010

11+
import io.fabric8.kubernetes.api.model.HasMetadata;
12+
1113
import io.kroxylicious.kubernetes.api.common.Condition;
1214
import io.kroxylicious.kubernetes.api.common.LocalRef;
1315
import io.kroxylicious.kubernetes.operator.model.ingress.IngressConflictException;
1416

1517
import edu.umd.cs.findbugs.annotations.NonNull;
1618
import edu.umd.cs.findbugs.annotations.Nullable;
1719

20+
import static io.kroxylicious.kubernetes.operator.ResourcesUtil.name;
1821
import static java.util.stream.Collectors.joining;
1922

2023
public record ClusterCondition(@NonNull String cluster,
@@ -29,14 +32,9 @@ static ClusterCondition accepted(String cluster) {
2932
return new ClusterCondition(cluster, Condition.Type.Accepted, Condition.Status.TRUE, null, null);
3033
}
3134

32-
static ClusterCondition filterInvalid(String cluster, String filterName, String detail) {
33-
return new ClusterCondition(cluster, Condition.Type.Accepted, Condition.Status.FALSE, INVALID,
34-
String.format("Filter \"%s\" is invalid: %s", filterName, detail));
35-
}
36-
37-
public static ClusterCondition refNotFound(String cluster, LocalRef<?> ref) {
38-
return new ClusterCondition(cluster, Condition.Type.ResolvedRefs, Condition.Status.FALSE, INVALID,
39-
String.format("Resource of kind \"%s\" in group \"%s\" named \"%s\" does not exist.", ref.getKind(), ref.getGroup(), ref.getName()));
35+
public static ClusterCondition refNotFound(HasMetadata referrer, LocalRef<?> ref) {
36+
return new ClusterCondition(name(referrer), Condition.Type.ResolvedRefs, Condition.Status.FALSE, INVALID,
37+
String.format("Resource %s was not found.", ResourcesUtil.namespacedSlug(ref, referrer)));
4038
}
4139

4240
public static ClusterCondition ingressConflict(String cluster, Set<IngressConflictException> ingressConflictExceptions) {

‎kroxylicious-operator/src/main/java/io/kroxylicious/kubernetes/operator/ResourcesUtil.java‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.Collection;
1414
import java.util.Comparator;
1515
import java.util.List;
16+
import java.util.Locale;
1617
import java.util.Map;
1718
import java.util.Objects;
1819
import java.util.Optional;
@@ -31,8 +32,6 @@
3132
import io.fabric8.kubernetes.api.model.OwnerReference;
3233
import io.fabric8.kubernetes.api.model.OwnerReferenceBuilder;
3334
import io.fabric8.kubernetes.client.CustomResource;
34-
import io.fabric8.kubernetes.model.annotation.Group;
35-
import io.fabric8.kubernetes.model.annotation.Singular;
3635
import io.javaoperatorsdk.operator.api.reconciler.Context;
3736
import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext;
3837
import io.javaoperatorsdk.operator.processing.event.ResourceID;
@@ -54,6 +53,7 @@
5453
import edu.umd.cs.findbugs.annotations.NonNull;
5554

5655
public class ResourcesUtil {
56+
5757
private ResourcesUtil() {
5858
}
5959

@@ -176,6 +176,10 @@ public static <T extends HasMetadata> LocalRef<T> toLocalRef(T ref) {
176176
}
177177

178178
public static String group(HasMetadata resource) {
179+
// core CustomResource classes like Secret, Deployment etc. have a group of empty string and their apiVersion is the String 'v1'
180+
if (!resource.getApiVersion().contains("/")) {
181+
return "";
182+
}
179183
return resource.getApiVersion().substring(0, resource.getApiVersion().indexOf("/"));
180184
}
181185

@@ -452,15 +456,15 @@ static Condition resolvedRefsUnknown(Clock clock,
452456
return newUnknownCondition(clock, observedGenerationSource, Condition.Type.ResolvedRefs, e);
453457
}
454458

455-
static String slug(String singular, String group, String name) {
456-
return singular + "." + group + "/" + name;
459+
public static @NonNull String namespacedSlug(LocalRef<?> ref, HasMetadata resource) {
460+
return slug(ref) + " in namespace '" + namespace(resource) + "'";
457461
}
458462

459-
static String slug(Class<? extends CustomResource<?, ?>> annotatedCrdClass, String crName) {
460-
return slug(
461-
annotatedCrdClass.getAnnotation(Singular.class).value(),
462-
annotatedCrdClass.getAnnotation(Group.class).value(),
463-
crName);
463+
private static @NonNull String slug(LocalRef<?> ref) {
464+
String group = ref.getGroup();
465+
String name = ref.getName();
466+
String groupString = group.isEmpty() ? "" : "." + group;
467+
return ref.getKind().toLowerCase(Locale.ROOT) + groupString + "/" + name;
464468
}
465469

466470
}

‎kroxylicious-operator/src/main/java/io/kroxylicious/kubernetes/operator/VirtualKafkaClusterReconciler.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -130,20 +130,20 @@ public UpdateControl<VirtualKafkaCluster> reconcile(VirtualKafkaCluster cluster,
130130
condition = ResourcesUtil.newResolvedRefsTrue(clock, cluster);
131131
}
132132
else {
133-
Stream<String> serviceMsg = refsMessage("spec.targetKafkaServiceRef references ", KafkaService.class, unresolvedServices);
134-
Stream<String> ingressMsg = refsMessage("spec.ingressRefs references ", KafkaProxyIngress.class, unresolvedIngresses);
135-
Stream<String> filterMsg = refsMessage("spec.filterRefs references ", KafkaProtocolFilter.class, unresolvedFilters);
133+
Stream<String> serviceMsg = refsMessage("spec.targetKafkaServiceRef references ", cluster, unresolvedServices);
134+
Stream<String> ingressMsg = refsMessage("spec.ingressRefs references ", cluster, unresolvedIngresses);
135+
Stream<String> filterMsg = refsMessage("spec.filterRefs references ", cluster, unresolvedFilters);
136136
condition = ResourcesUtil.newResolvedRefsFalse(clock,
137137
cluster,
138138
TRANSITIVELY_REFERENCED_RESOURCES_NOT_FOUND,
139139
joiningMessages(serviceMsg, ingressMsg, filterMsg));
140140
}
141141
}
142142
else {
143-
Stream<String> proxyMsg = refsMessage("spec.proxyRef references ", KafkaProxy.class, missingProxies);
144-
Stream<String> serviceMsg = refsMessage("spec.targetKafkaServiceRef references ", KafkaService.class, missingServices);
145-
Stream<String> ingressMsg = refsMessage("spec.ingressRefs references ", KafkaProxyIngress.class, missingIngresses);
146-
Stream<String> filterMsg = refsMessage("spec.filterRefs references ", KafkaProtocolFilter.class, missingFilters);
143+
Stream<String> proxyMsg = refsMessage("spec.proxyRef references ", cluster, missingProxies);
144+
Stream<String> serviceMsg = refsMessage("spec.targetKafkaServiceRef references ", cluster, missingServices);
145+
Stream<String> ingressMsg = refsMessage("spec.ingressRefs references ", cluster, missingIngresses);
146+
Stream<String> filterMsg = refsMessage("spec.filterRefs references ", cluster, missingFilters);
147147

148148
condition = ResourcesUtil.newResolvedRefsFalse(clock,
149149
cluster,
@@ -173,12 +173,12 @@ private static boolean hasAnyResolvedRefsFalse(List<Condition> conditions) {
173173
@NonNull
174174
private static <R extends CustomResource<?, ?>> Stream<String> refsMessage(
175175
String prefix,
176-
Class<R> crdClass,
176+
VirtualKafkaCluster cluster,
177177
TreeSet<? extends LocalRef<R>> refs) {
178178
return refs.isEmpty() ? Stream.of()
179179
: Stream.of(
180180
prefix + refs.stream()
181-
.map(ref -> ResourcesUtil.slug(crdClass, ref.getName()))
181+
.map(ref -> ResourcesUtil.namespacedSlug(ref, cluster))
182182
.collect(Collectors.joining(", ")));
183183
}
184184

‎kroxylicious-operator/src/main/java/io/kroxylicious/kubernetes/operator/resolver/ClusterConditionUnresolvedDependencyReporter.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
import io.kroxylicious.kubernetes.operator.ClusterCondition;
1818
import io.kroxylicious.kubernetes.operator.SharedKafkaProxyContext;
1919

20-
import static io.kroxylicious.kubernetes.operator.ResourcesUtil.name;
21-
2220
public record ClusterConditionUnresolvedDependencyReporter(Context<KafkaProxy> context) implements UnresolvedDependencyReporter {
2321

2422
@Override
@@ -33,6 +31,6 @@ public void reportUnresolvedDependencies(VirtualKafkaCluster cluster, Set<LocalR
3331
.sorted(comparator).findFirst()
3432
.orElseThrow();
3533
SharedKafkaProxyContext.addClusterCondition(context, cluster,
36-
ClusterCondition.refNotFound(name(cluster), firstUnresolvedDependency));
34+
ClusterCondition.refNotFound(cluster, firstUnresolvedDependency));
3735
}
3836
}

‎kroxylicious-operator/src/test/java/io/kroxylicious/kubernetes/operator/KafkaProxyReconcilerTest.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,11 @@ void transitionToReadyFalseShouldChangeTransitionTime2() {
329329
.build();
330330
// @formatter:on
331331
doReturn(mdrc).when(context).managedWorkflowAndDependentResourceContext();
332-
doReturn(Set.of(new VirtualKafkaClusterBuilder().withNewMetadata().withName("my-cluster").withNamespace("my-ns").endMetadata().withNewSpec().withNewProxyRef()
333-
.withName("my-proxy").endProxyRef().endSpec().build())).when(context).getSecondaryResources(VirtualKafkaCluster.class);
334-
doReturn(Optional.of(Map.of("my-cluster", ClusterCondition.refNotFound("my-cluster", new FilterRefBuilder().withName("MissingFilter").build())))).when(mdrc).get(
332+
VirtualKafkaCluster cluster = new VirtualKafkaClusterBuilder().withNewMetadata().withName("my-cluster").withNamespace("my-ns").endMetadata().withNewSpec()
333+
.withNewProxyRef()
334+
.withName("my-proxy").endProxyRef().endSpec().build();
335+
doReturn(Set.of(cluster)).when(context).getSecondaryResources(VirtualKafkaCluster.class);
336+
doReturn(Optional.of(Map.of("my-cluster", ClusterCondition.refNotFound(cluster, new FilterRefBuilder().withName("MissingFilter").build())))).when(mdrc).get(
335337
SharedKafkaProxyContext.CLUSTER_CONDITIONS_KEY,
336338
Map.class);
337339

@@ -351,7 +353,7 @@ void transitionToReadyFalseShouldChangeTransitionTime2() {
351353
statusAssert.singleCluster()
352354
.nameIsEqualTo("my-cluster")
353355
.singleCondition()
354-
.isResolvedRefsFalse("Invalid", "Resource of kind \"KafkaProtocolFilter\" in group \"filter.kroxylicious.io\" named \"MissingFilter\" does not exist.")
356+
.isResolvedRefsFalse("Invalid", "Resource kafkaprotocolfilter.filter.kroxylicious.io/MissingFilter in namespace 'my-ns' was not found.")
355357
.hasObservedGeneration(generation);
356358
// TODO .lastTransitionTimeIsEqualTo(time);
357359

‎kroxylicious-operator/src/test/java/io/kroxylicious/kubernetes/operator/ResourcesUtilTest.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@
2323
import io.fabric8.kubernetes.api.model.Secret;
2424
import io.fabric8.kubernetes.api.model.SecretBuilder;
2525

26+
import io.kroxylicious.kubernetes.api.common.AnyLocalRefBuilder;
2627
import io.kroxylicious.kubernetes.api.common.Condition;
2728
import io.kroxylicious.kubernetes.api.common.ConditionBuilder;
2829
import io.kroxylicious.kubernetes.api.common.FilterRefBuilder;
2930
import io.kroxylicious.kubernetes.api.common.IngressRefBuilder;
3031
import io.kroxylicious.kubernetes.api.common.KafkaServiceRefBuilder;
3132
import io.kroxylicious.kubernetes.api.common.ProxyRefBuilder;
33+
import io.kroxylicious.kubernetes.api.v1alpha1.KafkaProxy;
3234
import io.kroxylicious.kubernetes.api.v1alpha1.KafkaProxyBuilder;
3335
import io.kroxylicious.kubernetes.api.v1alpha1.KafkaProxyIngressBuilder;
3436
import io.kroxylicious.kubernetes.api.v1alpha1.KafkaServiceBuilder;
@@ -161,6 +163,30 @@ void generation() {
161163
assertThat(ResourcesUtil.generation(secret)).isEqualTo(generation);
162164
}
163165

166+
@Test
167+
void toLocalRefNoGroup() {
168+
assertThat(ResourcesUtil.toLocalRef(new SecretBuilder().withNewMetadata().withName("name").endMetadata().build()))
169+
.isEqualTo(new AnyLocalRefBuilder().withName("name").withGroup("").withKind("Secret").build());
170+
}
171+
172+
@Test
173+
void toLocalRefWithGroup() {
174+
assertThat(ResourcesUtil.toLocalRef(new KafkaProxyBuilder().withNewMetadata().withName("name").endMetadata().build()))
175+
.isEqualTo(new AnyLocalRefBuilder().withName("name").withGroup("kroxylicious.io").withKind("KafkaProxy").build());
176+
}
177+
178+
@Test
179+
void slugWithNamespaceEmptyGroup() {
180+
Secret secret = new SecretBuilder().withNewMetadata().withNamespace("my-ns").withName("secreto").endMetadata().build();
181+
assertThat(ResourcesUtil.namespacedSlug(ResourcesUtil.toLocalRef(secret), secret)).isEqualTo("secret/secreto in namespace 'my-ns'");
182+
}
183+
184+
@Test
185+
void slugWithNamespaceNonEmptyGroup() {
186+
KafkaProxy secret = new KafkaProxyBuilder().withNewMetadata().withNamespace("my-ns").withName("secreto").endMetadata().build();
187+
assertThat(ResourcesUtil.namespacedSlug(ResourcesUtil.toLocalRef(secret), secret)).isEqualTo("kafkaproxy.kroxylicious.io/secreto in namespace 'my-ns'");
188+
}
189+
164190
@Test
165191
void toLocalRef() {
166192
assertThat(ResourcesUtil.toLocalRef(new KafkaProxyBuilder().withNewMetadata().withName("foo").endMetadata().build()))

‎kroxylicious-operator/src/test/java/io/kroxylicious/kubernetes/operator/VirtualKafkaClusterReconcilerTest.java‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class VirtualKafkaClusterReconcilerTest {
4646
public static final VirtualKafkaCluster CLUSTER_NO_FILTERS = new VirtualKafkaClusterBuilder()
4747
.withNewMetadata()
4848
.withName("foo")
49+
.withNamespace("my-namespace")
4950
.withGeneration(42L)
5051
.endMetadata()
5152
.withNewSpec()
@@ -129,7 +130,7 @@ static List<Arguments> shouldSetResolvedRefsToTrueOrFalse() {
129130
Set.of(),
130131
(Consumer<ConditionAssert>) ca -> ca.isResolvedRefsFalse(
131132
VirtualKafkaClusterReconciler.REFERENCED_RESOURCES_NOT_FOUND,
132-
"spec.proxyRef references kafkaproxy.kroxylicious.io/my-proxy")),
133+
"spec.proxyRef references kafkaproxy.kroxylicious.io/my-proxy in namespace 'my-namespace'")),
133134
Arguments.argumentSet("service not found",
134135
CLUSTER_NO_FILTERS,
135136
Optional.of(PROXY),
@@ -138,7 +139,7 @@ static List<Arguments> shouldSetResolvedRefsToTrueOrFalse() {
138139
Set.of(),
139140
(Consumer<ConditionAssert>) ca -> ca.isResolvedRefsFalse(
140141
VirtualKafkaClusterReconciler.REFERENCED_RESOURCES_NOT_FOUND,
141-
"spec.targetKafkaServiceRef references kafkaservice.kroxylicious.io/my-kafka")),
142+
"spec.targetKafkaServiceRef references kafkaservice.kroxylicious.io/my-kafka in namespace 'my-namespace'")),
142143
Arguments.argumentSet("service has unresolved refs",
143144
CLUSTER_NO_FILTERS,
144145
Optional.of(PROXY),
@@ -148,7 +149,7 @@ static List<Arguments> shouldSetResolvedRefsToTrueOrFalse() {
148149
Set.of(),
149150
(Consumer<ConditionAssert>) ca -> ca.isResolvedRefsFalse(
150151
VirtualKafkaClusterReconciler.TRANSITIVELY_REFERENCED_RESOURCES_NOT_FOUND,
151-
"spec.targetKafkaServiceRef references kafkaservice.kroxylicious.io/my-kafka")),
152+
"spec.targetKafkaServiceRef references kafkaservice.kroxylicious.io/my-kafka in namespace 'my-namespace'")),
152153
Arguments.argumentSet("ingress not found",
153154
CLUSTER_NO_FILTERS,
154155
Optional.of(PROXY),
@@ -157,7 +158,7 @@ static List<Arguments> shouldSetResolvedRefsToTrueOrFalse() {
157158
Set.of(),
158159
(Consumer<ConditionAssert>) ca -> ca.isResolvedRefsFalse(
159160
VirtualKafkaClusterReconciler.REFERENCED_RESOURCES_NOT_FOUND,
160-
"spec.ingressRefs references kafkaproxyingress.kroxylicious.io/my-ingress")),
161+
"spec.ingressRefs references kafkaproxyingress.kroxylicious.io/my-ingress in namespace 'my-namespace'")),
161162
Arguments.argumentSet("ingress has unresolved refs",
162163
CLUSTER_NO_FILTERS,
163164
Optional.of(PROXY),
@@ -167,7 +168,7 @@ static List<Arguments> shouldSetResolvedRefsToTrueOrFalse() {
167168
Set.of(),
168169
(Consumer<ConditionAssert>) ca -> ca.isResolvedRefsFalse(
169170
VirtualKafkaClusterReconciler.TRANSITIVELY_REFERENCED_RESOURCES_NOT_FOUND,
170-
"spec.ingressRefs references kafkaproxyingress.kroxylicious.io/my-ingress")),
171+
"spec.ingressRefs references kafkaproxyingress.kroxylicious.io/my-ingress in namespace 'my-namespace'")),
171172
Arguments.argumentSet("filter not found",
172173
CLUSTER_ONE_FILTER,
173174
Optional.of(PROXY),
@@ -176,7 +177,7 @@ static List<Arguments> shouldSetResolvedRefsToTrueOrFalse() {
176177
Set.of(),
177178
(Consumer<ConditionAssert>) ca -> ca.isResolvedRefsFalse(
178179
VirtualKafkaClusterReconciler.REFERENCED_RESOURCES_NOT_FOUND,
179-
"spec.filterRefs references kafkaprotocolfilter.filter.kroxylicious.io/my-filter")),
180+
"spec.filterRefs references kafkaprotocolfilter.filter.kroxylicious.io/my-filter in namespace 'my-namespace'")),
180181
Arguments.argumentSet("filter has unresolved refs",
181182
CLUSTER_ONE_FILTER,
182183
Optional.of(PROXY),
@@ -186,7 +187,7 @@ static List<Arguments> shouldSetResolvedRefsToTrueOrFalse() {
186187
.withStatus(Condition.Status.FALSE).endCondition().endStatus().build()),
187188
(Consumer<ConditionAssert>) ca -> ca.isResolvedRefsFalse(
188189
VirtualKafkaClusterReconciler.TRANSITIVELY_REFERENCED_RESOURCES_NOT_FOUND,
189-
"spec.filterRefs references kafkaprotocolfilter.filter.kroxylicious.io/my-filter")));
190+
"spec.filterRefs references kafkaprotocolfilter.filter.kroxylicious.io/my-filter in namespace 'my-namespace'")));
190191
}
191192

192193
@ParameterizedTest

0 commit comments

Comments
 (0)