Skip to content

Commit

Permalink
Fix eclipse-jkube#511: Namespace as resource fragment results in Null…
Browse files Browse the repository at this point in the history 
…PointerException

+ DefaultNamespaceEnricher should handle case when status is not
  provided in resource spec.
+ .metadata.namespace should be omitted from Project/Namespace resources
  since they are Cluster scoped
+ UndeployService should consider Project/Namespace resource while
  deletion phase
  • Loading branch information
@rohanKanojia
rohanKanojia committed Dec 23, 2020
1 parent bc81297 commit b232901
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Usage:
* Fix #387: Update Fabric8 Kubernetes Client to v4.13.0 to support `networking.k8s.io/v1` `Ingress`
* Fix #473: Debug goals work with QuarkusGenerator generated container images
* Fix #484: cacheFrom configuration parameter is missing
* Fix #511: Namespace as resource fragment results in NullPointerException

### 1.0.2 (2020-10-30)
* Fix #429: Added quickstart for Micronaut framework
Expand Down
17 changes: 17 additions & 0 deletions .../main/java/org/eclipse/jkube/kit/config/service/kubernetes/KubernetesUndeployService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public void undeploy(File resourceDir, ResourceConfig resourceConfig, File... ma
final String currentNamespace = currentNamespace(entities);
undeployResources(currentNamespace, entities);
undeployCustomResources(currentNamespace, resourceDir, resourceConfig);
undeployNamespace(entities);
}

private void undeployResources(String namespace, List<HasMetadata> entities) {
Expand All @@ -78,6 +79,13 @@ private void undeployResources(String namespace, List<HasMetadata> entities) {
.forEach(resourceDeleter);
}

private void undeployNamespace(List<HasMetadata> entities) {
final Consumer<HasMetadata> resourceDeleter = clusterScopedResourceDeleter();
entities.stream()
.filter(e -> (e instanceof Namespace || e instanceof Project))
.forEach(resourceDeleter);
}

protected Consumer<HasMetadata> resourceDeleter(String namespace) {
return resource -> {
logger.info("Deleting resource %s %s/%s", KubernetesHelper.getKind(resource), namespace, KubernetesHelper.getName(resource));
Expand All @@ -88,6 +96,15 @@ protected Consumer<HasMetadata> resourceDeleter(String namespace) {
};
}

protected Consumer<HasMetadata> clusterScopedResourceDeleter() {
return resource -> {
logger.info("Deleting resource %s %s", KubernetesHelper.getKind(resource), KubernetesHelper.getName(resource));
jKubeServiceHub.getClient().resource(resource)
.withPropagationPolicy(DeletionPropagation.BACKGROUND)
.delete();
};
}

private void undeployCustomResources(String namespace, File resourceDir, ResourceConfig resourceConfig) throws IOException {
if (resourceConfig == null || resourceConfig.getCustomResourceDefinitions() == null || resourceConfig.getCustomResourceDefinitions().isEmpty()) {
return;
Expand Down
5 changes: 4 additions & 1 deletion ...t/java/org/eclipse/jkube/kit/config/service/kubernetes/KubernetesUndeployServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,16 @@ public void undeployWithManifestShouldDeleteApplicableEntities(@Mocked File file
// Then
// @formatter:off
new Verifications() {{
kubernetesHelper.getKind((HasMetadata)any); times = 2;
kubernetesHelper.getKind((HasMetadata)any); times = 3;
jKubeServiceHub.getClient().resource(pod).inNamespace("default")
.withPropagationPolicy(DeletionPropagation.BACKGROUND).delete();
times = 1;
jKubeServiceHub.getClient().resource(service).inNamespace("default")
.withPropagationPolicy(DeletionPropagation.BACKGROUND).delete();
times = 1;
jKubeServiceHub.getClient().resource(namespace)
.withPropagationPolicy(DeletionPropagation.BACKGROUND).delete();
times = 1;
}};
// @formatter:on
}
Expand Down
14 changes: 12 additions & 2 deletions ...er/generic/src/main/java/org/eclipse/jkube/enricher/generic/DefaultNamespaceEnricher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
import io.fabric8.kubernetes.api.model.Namespace;
import io.fabric8.kubernetes.api.model.NamespaceBuilder;
import io.fabric8.kubernetes.api.model.NamespaceStatus;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.fabric8.openshift.api.model.Project;
import io.fabric8.openshift.api.model.ProjectBuilder;
import io.fabric8.openshift.api.model.ProjectStatus;
import lombok.AllArgsConstructor;
import lombok.Getter;
import org.eclipse.jkube.kit.common.Configs;
Expand Down Expand Up @@ -134,18 +136,26 @@ public void visit(ObjectMetaBuilder metaBuilder) {
builder.accept(new TypedVisitor<NamespaceBuilder>() {
@Override
public void visit(NamespaceBuilder builder) {
if (builder.buildStatus().getPhase().equals("active")) {
// Set status as empty
NamespaceStatus status = builder.buildStatus();
if (status != null && status.getPhase().equals("active")) {
builder.editOrNewStatus().endStatus().build();
}
// Set metadata.namespace as null
builder.editOrNewMetadata().withNamespace(null).endMetadata();
}
});

builder.accept(new TypedVisitor<ProjectBuilder>() {
@Override
public void visit(ProjectBuilder builder) {
if (builder.buildStatus().getPhase().equals("active")) {
// Set status as empty
ProjectStatus status = builder.buildStatus();
if (status != null && status.getPhase().equals("active")) {
builder.editOrNewStatus().endStatus().build();
}
// Set metadata.namespace as null
builder.editOrNewMetadata().withNamespace(null).endMetadata();
}
});
}
Expand Down
68 changes: 62 additions & 6 deletions ...eneric/src/test/java/org/eclipse/jkube/enricher/generic/DefaultNamespaceEnricherTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@
*/
package org.eclipse.jkube.enricher.generic;

import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
import io.fabric8.kubernetes.api.model.Namespace;
import io.fabric8.kubernetes.api.model.NamespaceBuilder;
import io.fabric8.kubernetes.api.model.apps.Deployment;
import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
import io.fabric8.openshift.api.model.Project;
import io.fabric8.openshift.api.model.ProjectBuilder;
import mockit.Expectations;
import mockit.Mocked;
import org.eclipse.jkube.kit.config.resource.PlatformMode;
Expand All @@ -30,6 +32,7 @@
import java.util.Properties;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertNull;

public class DefaultNamespaceEnricherTest {

Expand Down Expand Up @@ -108,18 +111,71 @@ public void createWithPropertiesAndConfigInKubernetesShouldAddConfigNamespace()
}

@Test
public void enrichWithPropertiesInKubernetesShouldAddNamespace() {
public void enrichWithPropertiesInKubernetesShouldAddNamespaceWithStatus() {
// Given
properties.put("jkube.enricher.jkube-namespace.namespace", "example");
final KubernetesListBuilder klb = new KubernetesListBuilder();
klb.addToItems(new NamespaceBuilder()
.editOrNewMetadata().withName("name").withNamespace("to-be-overwritten").endMetadata()
.editOrNewStatus().withPhase("active").endStatus().build());
Namespace namespace = new NamespaceBuilder()
.editOrNewMetadata().withName("name").withNamespace("to-be-overwritten").endMetadata()
.editOrNewStatus().withPhase("active").endStatus().build();
Deployment deployment = new DeploymentBuilder().withNewMetadata().withName("d1").endMetadata().build();
klb.addToItems(namespace, deployment);
// When
new DefaultNamespaceEnricher(context).enrich(PlatformMode.kubernetes, klb);
// Then
assertThat(klb.build().getItems()).hasSize(2);
assertThat(klb.build().getItems().get(1))
.hasFieldOrPropertyWithValue("metadata.namespace", "example");
}

@Test
public void enrichWithPropertiesInKubernetesShouldAddProjectWithStatus() {
// Given
final KubernetesListBuilder klb = new KubernetesListBuilder();
klb.addToItems(new ProjectBuilder()
.withNewMetadata().withName("name").endMetadata()
.withNewStatus().withPhase("active").endStatus().build());
// When
new DefaultNamespaceEnricher(context).enrich(PlatformMode.openshift, klb);
// Then
assertThat(klb.build().getItems()).hasSize(1);
assertThat(klb.build().getItems().iterator().next())
.hasFieldOrPropertyWithValue("metadata.namespace", "example");
.hasFieldOrPropertyWithValue("metadata.namespace", null);
}

@Test
public void enrichWithNamespaceFragmentWithNoStatus() {
// Given
final KubernetesListBuilder kubernetesListBuilder = new KubernetesListBuilder();
kubernetesListBuilder.addToItems(new NamespaceBuilder()
.withNewMetadata().withName("test-jkube").endMetadata()
.build());

// When
new DefaultNamespaceEnricher(context).enrich(PlatformMode.kubernetes, kubernetesListBuilder);

// Then
assertThat(kubernetesListBuilder.build().getItems()).hasSize(1);
assertThat(kubernetesListBuilder.build().getItems().iterator().next())
.hasFieldOrPropertyWithValue("metadata.name", "test-jkube");
assertNull(kubernetesListBuilder.build().getItems().get(0).getMetadata().getNamespace());
}

@Test
public void enrichWithOpenShiftProjectFragmentWithNoStatus() {
// Given
final KubernetesListBuilder kubernetesListBuilder = new KubernetesListBuilder();
kubernetesListBuilder.addToItems(new ProjectBuilder()
.withNewMetadata().withName("test-jkube").endMetadata()
.build());

// When
new DefaultNamespaceEnricher(context).enrich(PlatformMode.openshift, kubernetesListBuilder);

// Then
assertThat(kubernetesListBuilder.build().getItems()).hasSize(1);
assertThat(kubernetesListBuilder.build().getItems().iterator().next())
.hasFieldOrPropertyWithValue("metadata.name", "test-jkube");
assertNull(kubernetesListBuilder.build().getItems().get(0).getMetadata().getNamespace());
}
}

0 comments on commit b232901

Please sign in to comment.