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

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

Merged
merged 4 commits into from
Sep 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -168,6 +176,7 @@ static String getFullResourceName(String plural, String group) {
}

@JsonIgnore
@SuppressWarnings("unused")
default String getFullResourceName() {
return getFullResourceName(getClass());
}
Expand All @@ -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();
}

Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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) {
Expand All @@ -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();
}

/**
Expand All @@ -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());
Expand All @@ -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
*/
Expand All @@ -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()));
}
}

Expand Down