Skip to content

Commit

Permalink
Fix eclipse-jkube#570: Change Behavior of jkube.namespace
Browse files Browse the repository at this point in the history 
Now `jkube.namespace` would only append namespace in Kubernetes
resources metadata. Earlier we were creating a new namespace too when
this property was configured.

For creating a new namespace this property should be used:
`jkube.enricher.jkube-namespace.namespace`
  • Loading branch information
@rohanKanojia
rohanKanojia committed Feb 11, 2021
1 parent a70b19b commit f230216
Show file tree
Hide file tree
Showing 10 changed files with 232 additions and 71 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Usage:
* Fix #546: Use `java.util.function.Function` instead of Guava's `com.google.common.base.Function`
* Fix #545: Replace Boolean.valueOf with Boolean.parseBoolean for string to boolean conversion
* Fix #515: Properties now get resolved in CustomResource fragments
* Fix #570: Disable namespace creation during k8s:resource with `jkube.namespace` flag

### 1.1.0 (2021-01-28)
* Fix #455: Use OpenShiftServer with JUnit rule instead of directly instantiating the OpenShiftMockServer
Expand Down
35 changes: 28 additions & 7 deletions jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/util/KubernetesHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
import io.fabric8.kubernetes.api.model.LabelSelectorBuilder;
import io.fabric8.kubernetes.api.model.LabelSelectorRequirement;
import io.fabric8.kubernetes.api.model.NamedContext;
import io.fabric8.kubernetes.api.model.Namespace;
import io.fabric8.kubernetes.api.model.Namespaced;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.Pod;
import io.fabric8.kubernetes.api.model.PodCondition;
Expand Down Expand Up @@ -92,7 +92,6 @@
import io.fabric8.openshift.api.model.Build;
import io.fabric8.openshift.api.model.DeploymentConfig;
import io.fabric8.openshift.api.model.DeploymentConfigSpec;
import io.fabric8.openshift.api.model.Project;
import io.fabric8.openshift.api.model.Template;
import org.apache.commons.lang3.ArrayUtils;
import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -925,15 +924,37 @@ public static String getAnnotationValue(HasMetadata item, String annotationKey)
return null;
}

public static String getNamespaceFromKubernetesList(Collection<HasMetadata> entities) {
for (HasMetadata h : entities) {
if (h instanceof Namespace || h instanceof Project) {
return h.getMetadata().getName();
}
public static String getNamespaceSetInKubernetesList(Collection<HasMetadata> entities) {
String namespaceSetInMetadata = getFirstNotNullNamespaceSetInMetadata(entities);
if (namespaceSetInMetadata != null) {
return getNamespaceIfEntitiesHaveNamespacedSetTo(entities, namespaceSetInMetadata);
}
return null;
}

private static String getFirstNotNullNamespaceSetInMetadata(Collection<HasMetadata> entities) {
return entities.stream()
.filter(h -> Namespaced.class.isAssignableFrom(h.getClass()))
.filter(h -> h.getMetadata().getNamespace() != null)
.findFirst()
.map(HasMetadata::getMetadata)
.map(ObjectMeta::getNamespace)
.orElse(null);
}

private static String getNamespaceIfEntitiesHaveNamespacedSetTo(Collection<HasMetadata> entities, String namespaceSetInMetadata) {
boolean isNamespaceSetInAllEntities = isNamespaceSetInAllEntities(entities, namespaceSetInMetadata);
return isNamespaceSetInAllEntities ? namespaceSetInMetadata : null;
}

private static boolean isNamespaceSetInAllEntities(Collection<HasMetadata> entities, String namespaceSetInMetadata) {
return entities.stream()
.filter(Objects::nonNull)
.filter(h -> Namespaced.class.isAssignableFrom(h.getClass()))
.filter(h -> h.getMetadata().getNamespace() != null)
.allMatch(h -> h.getMetadata().getNamespace().equals(namespaceSetInMetadata));
}

public static boolean containsPort(List<ContainerPort> ports, String portValue) {
for (ContainerPort port : ports) {
Integer containerPort = port.getContainerPort();
Expand Down
21 changes: 16 additions & 5 deletions jkube-kit/common/src/test/java/org/eclipse/jkube/kit/common/util/KubernetesHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import io.fabric8.kubernetes.api.model.EnvVarBuilder;
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.Namespace;
import io.fabric8.kubernetes.api.model.NamespaceBuilder;
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.fabric8.kubernetes.api.model.Quantity;
import io.fabric8.kubernetes.api.model.ReplicationControllerBuilder;
Expand Down Expand Up @@ -414,20 +413,32 @@ public void testIsControllerResource() {
}

@Test
public void testGetNamespaceFromKubernetesList() {
public void testGetNamespaceFromKubernetesListReturnsValidNamespace() {
// Given
List<HasMetadata> entities = new ArrayList<>();
entities.add(new NamespaceBuilder().withNewMetadata().withName("ns1").endMetadata().build());
entities.add(new DeploymentBuilder().withNewMetadata().withName("d1").endMetadata().build());
entities.add(new DeploymentBuilder().withNewMetadata().withName("d1").withNamespace("ns1").endMetadata().build());

// When
String namespace = KubernetesHelper.getNamespaceFromKubernetesList(entities);
String namespace = KubernetesHelper.getNamespaceSetInKubernetesList(entities);

// Then
assertNotNull(namespace);
assertEquals("ns1", namespace);
}

@Test
public void testGetNamespaceFromKubernetesListReturnsNull() {
// Given
List<HasMetadata> entities = new ArrayList<>();
entities.add(new DeploymentBuilder().withNewMetadata().withName("d1").endMetadata().build());

// When
String namespace = KubernetesHelper.getNamespaceSetInKubernetesList(entities);

// Then
assertNull(namespace);
}

@Test
public void loadResourcesWithNestedTemplateAndDuplicateResources() throws IOException {
// Given
Expand Down
15 changes: 9 additions & 6 deletions ...e-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/ApplyService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
import java.util.stream.Collectors;

import static org.eclipse.jkube.kit.common.util.KubernetesHelper.getCrdContext;
import static org.eclipse.jkube.kit.common.util.KubernetesHelper.getFullyQualifiedApiGroupWithKind;
import static org.eclipse.jkube.kit.common.util.KubernetesHelper.getKind;
import static org.eclipse.jkube.kit.common.util.KubernetesHelper.getName;
import static org.eclipse.jkube.kit.common.util.KubernetesHelper.getOrCreateLabels;
Expand Down Expand Up @@ -199,6 +198,10 @@ private void applyEntity(Object dto, String sourceName) {
applyJob((Job) dto, sourceName);
} else if (dto instanceof GenericCustomResource) {
applyGenericCustomResource((GenericCustomResource) dto, sourceName);
} else if (dto instanceof Namespace) {
applyNamespace((Namespace) dto);
} else if (dto instanceof Project) {
applyProject((Project) dto);
} else if (dto instanceof HasMetadata) {
HasMetadata entity = (HasMetadata) dto;
try {
Expand Down Expand Up @@ -1051,17 +1054,17 @@ public void applyNamespace(String namespaceName, Map<String,String> labels) {
*/
public boolean applyNamespace(Namespace entity) {
String currentNamespace = getOrCreateMetadata(entity).getName();
log.info("Using currentNamespace: " + currentNamespace);
log.info("Creating currentNamespace: " + currentNamespace);
String name = getName(entity);
Objects.requireNonNull(name, "No name for " + entity );
Namespace old = kubernetesClient.namespaces().withName(name).get();
if (!isRunning(old)) {
try {
Object answer = kubernetesClient.namespaces().create(entity);
logGeneratedEntity("Created currentNamespace: ", currentNamespace, entity, answer);
logGeneratedEntity("Created Namespace: ", currentNamespace, entity, answer);
return true;
} catch (Exception e) {
onApplyError("Failed to create currentNamespace: " + name + " due " + e.getMessage(), e);
onApplyError("Failed to create Namespace: " + name + " due " + e.getMessage(), e);
}
}
return false;
Expand All @@ -1087,7 +1090,7 @@ public boolean applyProjectRequest(ProjectRequest entity) {
return false;
}
String currentNamespace = getOrCreateMetadata(entity).getName();
log.info("Using project: " + currentNamespace);
log.info("Creating project: " + currentNamespace);
String name = getName(entity);
Objects.requireNonNull(name, "No name for " + entity);
OpenShiftClient openshiftClient = getOpenShiftClient();
Expand Down Expand Up @@ -1443,7 +1446,7 @@ private void applyStandardEntities(String fileName, List<HasMetadata> entities)
* @return namespace for ApplyService
*/
public static String getNamespaceForApplyService(Set<HasMetadata> entities, ClusterAccess clusterAccess) {
String namespaceInProvidedKubernetesList = KubernetesHelper.getNamespaceFromKubernetesList(entities);
String namespaceInProvidedKubernetesList = KubernetesHelper.getNamespaceSetInKubernetesList(entities);
if (namespaceInProvidedKubernetesList == null) {
return clusterAccess.getNamespace();
}
Expand Down
7 changes: 3 additions & 4 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 @@ -104,10 +104,9 @@ protected Consumer<HasMetadata> customResourceDeleter(String namespace) {

// Visible for testing
String currentNamespace(List<HasMetadata> entities) {
for (HasMetadata entity : entities) {
if (entity instanceof Namespace || entity instanceof Project) {
return entity.getMetadata().getName();
}
String namespaceSetInEntities = KubernetesHelper.getNamespaceSetInKubernetesList(entities);
if (namespaceSetInEntities != null) {
return namespaceSetInEntities;
}
return jKubeServiceHub.getClusterAccess().getNamespace();
}
Expand Down
15 changes: 14 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 @@ -15,6 +15,7 @@

import java.io.File;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;

import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
Expand Down Expand Up @@ -91,6 +92,8 @@ public void undeployWithManifestShouldDeleteAllEntities(@Mocked File file) throw
file.isFile(); result = true;
kubernetesHelper.loadResources(file);
result = new HashSet<>(Arrays.asList(namespace, pod, service));
kubernetesHelper.getNamespaceSetInKubernetesList((Collection<HasMetadata>)any);
result = "default";
}};
// @formatter:on
// When
Expand Down Expand Up @@ -152,7 +155,12 @@ public void undeployWithManifestAndCustomResourcesShouldDeleteAllEntities(
@Test
public void currentNamespaceWithNamespaceEntityShouldReturnFromNamespace() {
// Given
final Pod other = new PodBuilder().withNewMetadata().withName("MrPoddington").endMetadata().build();
// @formatter:off
new Expectations() {{
kubernetesHelper.getNamespaceSetInKubernetesList((Collection<HasMetadata>)any); result = "FromNamespace";
}};
// @formatter:on
final Pod other = new PodBuilder().withNewMetadata().withName("MrPoddington").withNamespace("FromNamespace").endMetadata().build();
final Namespace namespace = new NamespaceBuilder().withNewMetadata().withName("FromNamespace").endMetadata().build();
final Project project = new ProjectBuilder().withNewMetadata().withName("FromProject").endMetadata().build();
// When
Expand All @@ -164,6 +172,11 @@ public void currentNamespaceWithNamespaceEntityShouldReturnFromNamespace() {
@Test
public void currentNamespaceWithNoNamespaceEntityShouldReturnFromProject() {
// Given
// @formatter:off
new Expectations() {{
kubernetesHelper.getNamespaceSetInKubernetesList((Collection<HasMetadata>)any); result = "FromProject";
}};
// @formatter:on
final Pod other = new PodBuilder().withNewMetadata().withName("MrPoddington").endMetadata().build();
final Project project = new ProjectBuilder().withNewMetadata().withName("FromProject").endMetadata().build();
// When
Expand Down
74 changes: 41 additions & 33 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 @@ -16,16 +16,14 @@
import io.fabric8.kubernetes.api.builder.TypedVisitor;
import io.fabric8.kubernetes.api.model.HasMetadata;
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.apache.commons.lang3.StringUtils;
import org.eclipse.jkube.kit.common.Configs;
import org.eclipse.jkube.kit.config.resource.PlatformMode;
import org.eclipse.jkube.kit.config.resource.ResourceConfig;
Expand Down Expand Up @@ -75,27 +73,13 @@ public DefaultNamespaceEnricher(JKubeEnricherContext buildContext) {
*/
@Override
public void create(PlatformMode platformMode, KubernetesListBuilder builder) {
String newNamespaceToCreate = getConfig(Config.NAMESPACE, null);

final String ns = getNamespace(config, getConfig(Config.NAMESPACE));

if (ns == null || ns.isEmpty()) {
if (StringUtils.isEmpty(newNamespaceToCreate)) {
return;
}

if (!KubernetesResourceUtil.checkForKind(builder, NAMESPACE_KINDS)) {
String type = getConfig(Config.TYPE);
if ("project".equalsIgnoreCase(type) || NAMESPACE.equalsIgnoreCase(type)) {
if (platformMode == PlatformMode.kubernetes) {
log.info("Adding a default Namespace: %s", ns);
Namespace namespace = handlerHub.getNamespaceHandler().getNamespace(ns);
builder.addToNamespaceItems(namespace);
} else {
log.info("Adding a default Project %s", ns);
Project project = handlerHub.getProjectHandler().getProject(ns);
builder.addToItems(project);
}
}
}
addNewNamespaceToBuilderIfProvided(platformMode, newNamespaceToCreate, builder);
}

/**
Expand All @@ -108,26 +92,20 @@ public void create(PlatformMode platformMode, KubernetesListBuilder builder) {
public void enrich(PlatformMode platformMode, KubernetesListBuilder builder) {
builder.accept(new TypedVisitor<ObjectMetaBuilder>() {
private String getNamespaceName() {
final String defaultValue = builder.buildItems().stream()
.filter(item -> NAMESPACE_KINDS_LIST.contains(item.getKind()))
.map(HasMetadata::getMetadata)
.map(ObjectMeta::getName)
.findFirst().orElse(null);
return getNamespace(config, getConfig(Config.NAMESPACE, defaultValue));
return getNamespace(config, null);
}

private boolean shouldConfigureNamespaceInMetadata() {
return StringUtils.isNotEmpty(getNamespaceName());
}

@Override
public void visit(ObjectMetaBuilder metaBuilder) {
if (!KubernetesResourceUtil.checkForKind(builder, NAMESPACE_KINDS)) {
return;
}

String name = getNamespaceName();
if (name == null || name.isEmpty()) {
if (!shouldConfigureNamespaceInMetadata()) {
return;
}

metaBuilder.withNamespace(name).build();
metaBuilder.withNamespace(getNamespaceName()).build();
}
});

Expand Down Expand Up @@ -159,4 +137,34 @@ public void visit(ProjectBuilder builder) {
}
});
}

private void addNewNamespaceToBuilderIfProvided(PlatformMode platformMode, String newNamespaceToCreate, KubernetesListBuilder builder) {
if (!KubernetesResourceUtil.checkForKind(builder, NAMESPACE_KINDS)) {
String type = getConfig(Config.TYPE);
if (StringUtils.isNotEmpty(newNamespaceToCreate)) {
addNamespaceToBuilder(platformMode, newNamespaceToCreate, builder, type);
}
}
}

private void addNamespaceToBuilder(PlatformMode platformMode, String newNamespaceToCreate, KubernetesListBuilder builder, String type) {
HasMetadata namespaceOrProject = getNamespaceOrProject(platformMode, type, newNamespaceToCreate);
if (namespaceOrProject != null) {
builder.addToItems(namespaceOrProject);
}
}

private HasMetadata getNamespaceOrProject(PlatformMode platformMode, String type, String ns) {
if ("project".equalsIgnoreCase(type) || NAMESPACE.equalsIgnoreCase(type)) {
if (platformMode == PlatformMode.kubernetes) {
log.info("Adding a default Namespace: %s", ns);
return handlerHub.getNamespaceHandler().getNamespace(ns);
} else {
log.info("Adding a default Project %s", ns);
return handlerHub.getProjectHandler().getProject(ns);
}
}
return null;
}

}
Loading

0 comments on commit f230216

Please sign in to comment.