fix: provide more context when group/plural are missing (#4397)

* fix: provide more context when group/plural are missing Fixes #4396 * fix: format * fix: plural should never be null * fix: remove NAME constants
fabric8io · Sep 8, 2022 · e7f6fa3 · e7f6fa3
1 parent f88bd2a
commit e7f6fa3
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 54 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 #### Improvements
 * Fix #4348: Introduce specific annotations for the generators
 * Fix #4365: The Watch retry logic will handle more cases, as well as perform an exceptional close for events that are not properly handled.  Informers can directly provide those exceptional outcomes via the SharedIndexInformer.stopped CompletableFuture.
+* Fix #4396: Provide more error context when @Group/@Version annotations are missing
 
 #### Dependency Upgrade
 * Fix #4243: Update Tekton pipeline model to v0.39.0
@@ -37,7 +38,6 @@ fix #4373: NO_PROXY should allow URIs with hyphens ("circleci-internal-outer-bui
 * Fix #4320: corrected leader transitions field on leader election leases
 * Fix #4360: JUnit dependencies aren't leaked in child modules
 
-
 #### Improvements
 * Fix #887: added KubernetesClient.visitResources to search and perform other operations across all resources.
 * Fix #3960: adding a KubernetesMockServer.expectCustomResource helper method and additional mock crd support

diff --git a/...r/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/annotation/Group.java b/...r/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/annotation/Group.java
@@ -27,7 +27,7 @@
  * determine the `apiVersion` field associated with the annotated resource.
  * See https://kubernetes.io/docs/reference/using-api/#api-groups for more details.
  */
-@Target({TYPE})
+@Target({ TYPE })
 @Retention(RUNTIME)
 public @interface Group {
 

diff --git a/...kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/annotation/Version.java b/...kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/annotation/Version.java
@@ -15,36 +15,36 @@
  */
 package io.fabric8.kubernetes.model.annotation;
 
-import static java.lang.annotation.ElementType.TYPE;
-
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
+import static java.lang.annotation.ElementType.TYPE;
+
 /**
  * Allows to specify which version of the API the annotated class is defined under. Together with {@link Group}, this allows to
  * determine the `apiVersion` field associated with the annotated resource.
  * See https://kubernetes.io/docs/reference/using-api/#api-versioning for more details.
  */
-@Target({TYPE})
+@Target({ TYPE })
 @Retention(RetentionPolicy.RUNTIME)
 public @interface Version {
-  
+
   /**
    * The name of this version.
    *
    * @return the name of this version
    */
   String value();
-  
+
   /**
    * Whether or not this version corresponds to the persisted version for the associated CRD. Note that only one version can set
    * {@code storage} to {@code true} for a given CRD.
    *
    * @return {@code true} if this version corresponds to the persisted version for the associated CRD, {@code false} otherwise
    */
   boolean storage() default true;
-  
+
   /**
    * Whether this version is served (i.e. enabled for consumption from the REST API) or not.
    *

diff --git a/...ator/kubernetes-model-core/src/main/java/io/fabric8/kubernetes/api/model/HasMetadata.java b/...ator/kubernetes-model-core/src/main/java/io/fabric8/kubernetes/api/model/HasMetadata.java
@@ -22,6 +22,7 @@
 import io.fabric8.kubernetes.model.annotation.Plural;
 import io.fabric8.kubernetes.model.annotation.Singular;
 import io.fabric8.kubernetes.model.annotation.Version;
+
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -45,8 +46,8 @@ public interface HasMetadata extends KubernetesResource {
    * Pattern that checks the format of finalizer identifiers, which should be in {@code <domain name>/<finalizer name>} format.
    */
   Pattern FINALIZER_NAME_MATCHER = Pattern.compile(
-    "^((" + DNS_LABEL_REGEXP + "\\.)+" + DNS_LABEL_START + 2 + DNS_LABEL_END + ")/"
-      + DNS_LABEL_REGEXP);
+      "^((" + DNS_LABEL_REGEXP + "\\.)+" + DNS_LABEL_START + 2 + DNS_LABEL_END + ")/"
+          + DNS_LABEL_REGEXP);
 
   ObjectMeta getMetadata();
 
@@ -84,8 +85,8 @@ static String getApiVersion(Class<?> clazz) {
     }
     if (group != null || version != null) {
       throw new IllegalArgumentException(
-        "You need to specify both @" + Group.class.getSimpleName() + " and @"
-          + Version.class.getSimpleName() + " annotations if you specify either");
+          "You need to specify both @" + Group.class.getSimpleName() + " and @" + Version.class.getSimpleName()
+              + " annotations if you specify either");
     }
     return null;
   }
@@ -129,7 +130,7 @@ default String getApiVersion() {
   static String getPlural(Class<?> clazz) {
     final Plural fromAnnotation = clazz.getAnnotation(Plural.class);
     return (fromAnnotation != null ? fromAnnotation.value().toLowerCase(Locale.ROOT)
-      : Pluralize.toPlural(getSingular(clazz)));
+        : Pluralize.toPlural(getSingular(clazz)));
   }
 
   @JsonIgnore
@@ -144,12 +145,12 @@ default String getPlural() {
    *
    * @param clazz the class whose singular form we want to retrieve
    * @return the singular form defined by the {@link Singular} annotation or a computed default
-   * value
+   *         value
    */
   static String getSingular(Class<?> clazz) {
     final Singular fromAnnotation = clazz.getAnnotation(Singular.class);
     return (fromAnnotation != null ? fromAnnotation.value() : HasMetadata.getKind(clazz))
-      .toLowerCase(Locale.ROOT);
+        .toLowerCase(Locale.ROOT);
   }
 
   @JsonIgnore
@@ -158,7 +159,14 @@ default String getSingular() {
   }
 
   static String getFullResourceName(Class<?> clazz) {
-    return getFullResourceName(getPlural(clazz), getGroup(clazz));
+    final String plural = getPlural(clazz);
+    final String group = getGroup(clazz);
+    if (group == null) {
+      throw new IllegalArgumentException(
+          "Should provide non-null group. Is " + clazz.getName() + " properly annotated with @"
+              + Group.class.getSimpleName() + " and/or @" + Version.class.getSimpleName() + "?");
+    }
+    return getFullResourceName(plural, group);
   }
 
   static String getFullResourceName(String plural, String group) {
@@ -168,6 +176,7 @@ static String getFullResourceName(String plural, String group) {
   }
 
   @JsonIgnore
+  @SuppressWarnings("unused")
   default String getFullResourceName() {
     return getFullResourceName(getClass());
   }
@@ -180,7 +189,7 @@ default String getFullResourceName() {
   @JsonIgnore
   default boolean isMarkedForDeletion() {
     final String deletionTimestamp = optionalMetadata().map(ObjectMeta::getDeletionTimestamp)
-      .orElse(null);
+        .orElse(null);
     return deletionTimestamp != null && !deletionTimestamp.isEmpty();
   }
 
@@ -202,8 +211,10 @@ default List<String> getFinalizers() {
   /**
    * Adds the specified finalizer to this {@code HasMetadata} if it's valid. See {@link #isFinalizerValid(String)}.
    *
-   * @param finalizer the identifier of the finalizer to add to this {@code HasMetadata} in {@code <domain name>/<finalizer name>} format.
-   * @return {@code true} if the finalizer was successfully added, {@code false} otherwise (in particular, if the object is marked for deletion)
+   * @param finalizer the identifier of the finalizer to add to this {@code HasMetadata} in
+   *        {@code <domain name>/<finalizer name>} format.
+   * @return {@code true} if the finalizer was successfully added, {@code false} otherwise (in particular, if the object is
+   *         marked for deletion)
    * @throws IllegalArgumentException if the specified finalizer identifier is null or is invalid
    */
   default boolean addFinalizer(String finalizer) {
@@ -222,13 +233,14 @@ default boolean addFinalizer(String finalizer) {
       return metadata.getFinalizers().add(finalizer);
     } else {
       throw new IllegalArgumentException("Invalid finalizer name: '" + finalizer
-        + "'. Must consist of a domain name, a forward slash and the valid kubernetes name.");
+          + "'. Must consist of a domain name, a forward slash and the valid kubernetes name.");
     }
   }
 
   /**
    * Determines whether the specified finalizer is valid according to the
-   * <a href='https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#finalizers'>finalizer
+   * <a href=
+   * 'https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#finalizers'>finalizer
    * specification</a>.
    *
    * @param finalizer the identifier of the finalizer which validity we want to check
@@ -246,14 +258,15 @@ static boolean validateFinalizer(String finalizer) {
       return false;
     }
   }
+
   /**
    * @see HasMetadata#validateFinalizer(String)
    *
    * @param finalizer the identifier of the finalizer which validity we want to check
    * @return {@code true} if the identifier is valid, {@code false} otherwise
    */
   default boolean isFinalizerValid(String finalizer) {
-     return HasMetadata.validateFinalizer(finalizer);
+    return HasMetadata.validateFinalizer(finalizer);
   }
 
   /**
@@ -291,7 +304,8 @@ default boolean hasOwnerReferenceFor(String ownerUid) {
    * Retrieves the {@link OwnerReference} associated with the specified owner if it's part of this {@code HasMetadata}'s owners.
    *
    * @param owner the potential owner of which we want to retrieve the associated {@link OwnerReference}
-   * @return an {@link Optional} containing the {@link OwnerReference} associated with the specified owner if it exists or {@link Optional#empty()} otherwise.
+   * @return an {@link Optional} containing the {@link OwnerReference} associated with the specified owner if it exists or
+   *         {@link Optional#empty()} otherwise.
    */
   default Optional<OwnerReference> getOwnerReferenceFor(HasMetadata owner) {
     if (owner == null) {
@@ -303,24 +317,25 @@ default Optional<OwnerReference> getOwnerReferenceFor(HasMetadata owner) {
   }
 
   /**
-   * Retrieves the {@link OwnerReference} associated with the owner identified by the specified UID if it's part of this{@code HasMetadata}'s owners.
+   * Retrieves the {@link OwnerReference} associated with the owner identified by the specified UID if it's part of
+   * this{@code HasMetadata}'s owners.
    *
    * @param ownerUid the UID of the potential owner of which we want to retrieve the associated {@link
-   *              OwnerReference}
+   *        OwnerReference}
    * @return an {@link Optional} containing the {@link OwnerReference} associated with the
-   * owner identified by the specified UID if it exists or {@link Optional#empty()} otherwise.
+   *         owner identified by the specified UID if it exists or {@link Optional#empty()} otherwise.
    */
   default Optional<OwnerReference> getOwnerReferenceFor(String ownerUid) {
     if (ownerUid == null || ownerUid.isEmpty()) {
       return Optional.empty();
     }
 
     return optionalMetadata()
-      .map(m -> Optional.ofNullable(m.getOwnerReferences()).orElse(Collections.emptyList()))
-      .orElse(Collections.emptyList())
-      .stream()
-      .filter(o -> ownerUid.equals(o.getUid()))
-      .findFirst();
+        .map(m -> Optional.ofNullable(m.getOwnerReferences()).orElse(Collections.emptyList()))
+        .orElse(Collections.emptyList())
+        .stream()
+        .filter(o -> ownerUid.equals(o.getUid()))
+        .findFirst();
   }
 
   /**
@@ -332,36 +347,38 @@ default Optional<OwnerReference> getOwnerReferenceFor(String ownerUid) {
   default OwnerReference addOwnerReference(HasMetadata owner) {
     if (owner == null) {
       throw new IllegalArgumentException("Cannot add a reference to a null owner to "
-        + optionalMetadata().map(m -> "'" + m.getName() + "' ").orElse("unnamed ")
-        + getKind());
+          + optionalMetadata().map(m -> "'" + m.getName() + "' ").orElse("unnamed ")
+          + getKind());
     }
 
     final ObjectMeta metadata = owner.getMetadata();
     if (metadata == null) {
       throw new IllegalArgumentException("Cannot add a reference to an owner without metadata to "
-        + optionalMetadata().map(m -> "'" + m.getName() + "' ").orElse("unnamed ")
-        + getKind());
+          + optionalMetadata().map(m -> "'" + m.getName() + "' ").orElse("unnamed ")
+          + getKind());
     }
 
     final OwnerReference ownerReference = new OwnerReferenceBuilder()
-      .withUid(metadata.getUid())
-      .withApiVersion(owner.getApiVersion())
-      .withName(metadata.getName())
-      .withKind(owner.getKind())
-      .build();
+        .withUid(metadata.getUid())
+        .withApiVersion(owner.getApiVersion())
+        .withName(metadata.getName())
+        .withKind(owner.getKind())
+        .build();
     return addOwnerReference(sanitizeAndValidate(ownerReference));
   }
 
   /**
-   * Adds the specified, supposed valid (for example because validated by calling {@link #sanitizeAndValidate(OwnerReference)} beforehand), {@link OwnerReference} to this {@code HasMetadata}
+   * Adds the specified, supposed valid (for example because validated by calling {@link #sanitizeAndValidate(OwnerReference)}
+   * beforehand), {@link OwnerReference} to this {@code HasMetadata}
+   *
    * @param ownerReference the {@link OwnerReference} to add to this {@code HasMetadata}'s owner references
    * @return the newly added {@link OwnerReference}
    */
   default OwnerReference addOwnerReference(OwnerReference ownerReference) {
     if (ownerReference == null) {
       throw new IllegalArgumentException("Cannot add a null reference to "
-        + optionalMetadata().map(m -> "'" + m.getName() + "' ").orElse("unnamed ")
-        + getKind());
+          + optionalMetadata().map(m -> "'" + m.getName() + "' ").orElse("unnamed ")
+          + getKind());
     }
 
     final Optional<OwnerReference> existing = getOwnerReferenceFor(ownerReference.getUid());
@@ -385,6 +402,7 @@ default OwnerReference addOwnerReference(OwnerReference ownerReference) {
 
   /**
    * Sanitizes and validates the specified {@link OwnerReference}, presumably to add it
+   *
    * @param ownerReference the {@link OwnerReference} to sanitize and validate
    * @return the sanitized and validated {@link OwnerReference} which should be used instead of the original one
    */
@@ -410,34 +428,36 @@ static OwnerReference sanitizeAndValidate(OwnerReference ownerReference) {
       }
     };
     final Supplier<IllegalArgumentException> exceptionSupplier = () -> new IllegalArgumentException(
-      error.toString());
+        error.toString());
 
     final Optional<String> uid = trimmedFieldIfValid.apply("uid", ownerReference.getUid());
     final Optional<String> apiVersion = trimmedFieldIfValid.apply("apiVersion",
-      ownerReference.getApiVersion());
+        ownerReference.getApiVersion());
     final Optional<String> name = trimmedFieldIfValid.apply("name", ownerReference.getName());
     final Optional<String> kind = trimmedFieldIfValid.apply("kind", ownerReference.getKind());
 
     // check that required values are present
     ownerReference = new OwnerReferenceBuilder(ownerReference)
-      .withUid(uid.orElseThrow(exceptionSupplier))
-      .withApiVersion(apiVersion.orElseThrow(exceptionSupplier))
-      .withName(name.orElseThrow(exceptionSupplier))
-      .withKind(kind.orElseThrow(exceptionSupplier))
-      .build();
+        .withUid(uid.orElseThrow(exceptionSupplier))
+        .withApiVersion(apiVersion.orElseThrow(exceptionSupplier))
+        .withName(name.orElseThrow(exceptionSupplier))
+        .withKind(kind.orElseThrow(exceptionSupplier))
+        .build();
     return ownerReference;
   }
 
   /**
-   * Removes the {@link OwnerReference} identified by the specified UID if it's part of this {@code HasMetadata}'s owner references
+   * Removes the {@link OwnerReference} identified by the specified UID if it's part of this {@code HasMetadata}'s owner
+   * references
+   *
    * @param ownerUid the UID of the {@link OwnerReference} to remove
    */
   default void removeOwnerReference(String ownerUid) {
     if (ownerUid != null && !ownerUid.isEmpty()) {
       optionalMetadata()
-        .map(m -> Optional.ofNullable(m.getOwnerReferences()).orElse(Collections.emptyList()))
-        .orElse(Collections.emptyList())
-        .removeIf(o -> ownerUid.equals(o.getUid()));
+          .map(m -> Optional.ofNullable(m.getOwnerReferences()).orElse(Collections.emptyList()))
+          .orElse(Collections.emptyList())
+          .removeIf(o -> ownerUid.equals(o.getUid()));
     }
   }