Skip to content

Commit

Permalink
fix fabric8io#4184: going back to a resource method
Browse files Browse the repository at this point in the history
now that we're considering allowing for the modification of the current
item, and not just create, this makes the api more consistent with the
rest of the dsl
  • Loading branch information
shawkins committed Jan 26, 2023
1 parent 1bb8fb6 commit 99af146
Show file tree
Hide file tree
Showing 15 changed files with 120 additions and 322 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@
import io.fabric8.kubernetes.client.dsl.AutoscalingAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.BatchAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.CertificatesAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.ConfigMapResource;
import io.fabric8.kubernetes.client.dsl.DiscoveryAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.EventingAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.ExtensionsAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.FlowControlAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.FromFileCreatableOperation;
import io.fabric8.kubernetes.client.dsl.InOutCreateable;
import io.fabric8.kubernetes.client.dsl.MetricAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.MixedOperation;
Expand Down Expand Up @@ -449,9 +449,9 @@ NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable<HasMetadata> resourc
/**
* API entrypoint for ConfigMap related operations. ConfigMap (core/v1)
*
* @return FromFileCreatableOperation object for ConfigMap related operations.
* @return MixedOperation object for ConfigMap related operations.
*/
FromFileCreatableOperation<ConfigMap, ConfigMapList, Resource<ConfigMap>> configMaps();
MixedOperation<ConfigMap, ConfigMapList, ConfigMapResource> configMaps();

/**
* API entrypoint for LimitRange related operations. LimitRange (core/v1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@
import io.fabric8.kubernetes.client.dsl.AutoscalingAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.BatchAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.CertificatesAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.ConfigMapResource;
import io.fabric8.kubernetes.client.dsl.DiscoveryAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.EventingAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.ExtensionsAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.FlowControlAPIGroupDSL;
import io.fabric8.kubernetes.client.dsl.FromFileCreatableOperation;
import io.fabric8.kubernetes.client.dsl.FunctionCallable;
import io.fabric8.kubernetes.client.dsl.InOutCreateable;
import io.fabric8.kubernetes.client.dsl.MetricAPIGroupDSL;
Expand Down Expand Up @@ -355,7 +355,7 @@ public NonNamespaceOperation<APIService, APIServiceList, Resource<APIService>> a
}

@Override
public FromFileCreatableOperation<ConfigMap, ConfigMapList, Resource<ConfigMap>> configMaps() {
public MixedOperation<ConfigMap, ConfigMapList, ConfigMapResource> configMaps() {
return getClient().configMaps();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package io.fabric8.kubernetes.client.dsl;

public interface FromFileCreatableResource<T> extends
NameableNamespaceableResource<T>, FromFileCreatable<FromFileCreatableResource<T>> {
import io.fabric8.kubernetes.api.model.ConfigMap;

public interface ConfigMapResource extends Resource<ConfigMap>,
FromFileCreatable<ConfigMapResource> {
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

public interface FromFileCreatable<T> {
/**
* Create new resource from a directory or file contents.
* Create or update an entry from a directory or file contents.
*
* @param key key for Kubernetes resource's .data field
* @param dirOrFilePath a file or directory path
Expand All @@ -28,7 +28,7 @@ public interface FromFileCreatable<T> {
T fromFile(String key, Path dirOrFilePath);

/**
* Create new resource with data populated with entry with key matching file name
* Create or update an entry with data populated with entry with key matching file name
* and value matching file contents. If it's a directory, key value pairs would be names
* of files present in directory as keys and their contents as value respectively
*
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import java.util.Map;
import java.util.Optional;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public class KubernetesResourceUtil {
Expand Down Expand Up @@ -397,7 +396,7 @@ public static List<EnvVar> convertMapToEnvVarList(Map<String, String> envVarMap)
* Check whether a Kubernetes resource is Ready or not. Applicable only to
* Deployment, ReplicaSet, Pod, ReplicationController, Endpoints, Node and
* StatefulSet
*
*
* @param item item which needs to be checked
* @return boolean value indicating it's status
*/
Expand All @@ -407,7 +406,7 @@ public static boolean isResourceReady(HasMetadata item) {

/**
* Calculates age of a kubernetes resource
*
*
* @param kubernetesResource
* @return a positive duration indicating age of the kubernetes resource
*/
Expand Down Expand Up @@ -507,44 +506,6 @@ public static ConfigMap createNewConfigMapWithEntry(final String key, final Path
return configMapBuilder.build();
}

/**
* Merge ConfigMap data of two ConfigMaps
*
* @param cm1 first ConfigMap
* @param cm2 cm2 Configmap which would be modified
* @return ConfigMap containing data of both ConfigMaps
*/
public static ConfigMap mergeConfigMapData(final ConfigMap cm1, final ConfigMap cm2) {
ConfigMapBuilder resultConfigMapBuilder = new ConfigMapBuilder();
if (cm1 != null || cm2 != null) {
if (cm1 == null) {
return cm2;
} else if (cm2 == null) {
return cm1;
} else {
Map<String, String> mergedData = mergeMaps(cm1.getData(), cm2.getData());
Map<String, String> mergedBinaryData = mergeMaps(cm1.getBinaryData(), cm2.getBinaryData());
resultConfigMapBuilder.withData(mergedData);
resultConfigMapBuilder.withBinaryData(mergedBinaryData);
}
}
return resultConfigMapBuilder.build();
}

private static Map<String, String> mergeMaps(Map<String, String> m1, Map<String, String> m2) {
if (m1 == null && m2 == null) {
return Collections.emptyMap();
} else if (m1 == null) {
return m2;
} else if (m2 == null) {
return m1;
} else {
return Stream.of(m1, m2)
.flatMap(m -> m.entrySet().stream())
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
}

private static Map.Entry<String, String> createConfigMapEntry(final String key, final Path file) throws IOException {
final byte[] bytes = Files.readAllBytes(file);
if (isFileWithBinaryContent(file)) {
Expand Down Expand Up @@ -598,7 +559,7 @@ private static void addEntryToConfigMap(ConfigMapBuilder configMapBuilder, Map.E
}
}

private static void addEntriesFromDirOrFileToConfigMap(ConfigMapBuilder configMapBuilder, final String key,
public static void addEntriesFromDirOrFileToConfigMap(ConfigMapBuilder configMapBuilder, final String key,
final Path dirOrFilePath) throws IOException {
if (Files.isDirectory(dirOrFilePath, LinkOption.NOFOLLOW_LINKS)) {
addEntriesFromDirectoryToConfigMap(configMapBuilder, dirOrFilePath);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,14 @@
import io.fabric8.kubernetes.api.model.ConfigMapList;
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.dsl.FromFileCreatableOperation;
import io.fabric8.kubernetes.client.dsl.ConfigMapResource;
import io.fabric8.kubernetes.client.dsl.MixedOperation;
import io.fabric8.kubernetes.client.dsl.ReplaceDeletable;
import io.fabric8.kubernetes.client.dsl.Resource;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.function.Executable;
import org.mockito.Answers;
import org.mockito.ArgumentMatchers;
import org.mockito.Mockito;

import java.time.Duration;
Expand All @@ -52,16 +51,19 @@
class ConfigMapLockTest {

private KubernetesClient kc;
private FromFileCreatableOperation<ConfigMap, ConfigMapList, Resource<ConfigMap>> configMaps;
private MixedOperation<ConfigMap, ConfigMapList, ConfigMapResource> configMaps;
private ConfigMapResource configMapResource;
private ConfigMapBuilder configMapBuilder;
private ConfigMapBuilder.MetadataNested<ConfigMapBuilder> metadata;

@BeforeEach
void setUp() {
kc = mock(KubernetesClient.class, RETURNS_DEEP_STUBS);
configMaps = mock(FromFileCreatableOperation.class, RETURNS_DEEP_STUBS);
configMaps = mock(MixedOperation.class);
configMapResource = mock(ConfigMapResource.class);
configMapBuilder = Mockito.mock(ConfigMapBuilder.class, RETURNS_DEEP_STUBS);
metadata = mock(ConfigMapBuilder.MetadataNested.class, RETURNS_DEEP_STUBS);
when(configMaps.withName("name")).thenReturn(configMapResource);
when(kc.configMaps().inNamespace(anyString())).thenReturn(configMaps);
when(configMapBuilder.editOrNewMetadata()).thenReturn(metadata);
}
Expand Down Expand Up @@ -102,7 +104,7 @@ void missingIdentityShouldThrowException() {
void getWithExistingConfigMapShouldReturnLeaderElectionRecord() {
// Given
final ConfigMap cm = new ConfigMap();
when(configMaps.withName(ArgumentMatchers.eq("name")).get()).thenReturn(cm);
when(configMapResource.get()).thenReturn(cm);
cm.setMetadata(new ObjectMetaBuilder()
.withAnnotations(
Collections.singletonMap("control-plane.alpha.kubernetes.io/leader",
Expand All @@ -125,6 +127,7 @@ void createWithValidLeaderElectionRecordShouldSendPostRequest() throws Exception
final LeaderElectionRecord record = new LeaderElectionRecord(
"1", Duration.ofSeconds(1), ZonedDateTime.now(), ZonedDateTime.now(), 0);
final ConfigMapLock lock = new ConfigMapLock("namespace", "name", "1337");
when(configMapResource.get()).thenReturn(new ConfigMap());
// When
lock.create(kc, record);
// Then
Expand All @@ -134,7 +137,6 @@ void createWithValidLeaderElectionRecordShouldSendPostRequest() throws Exception
@Test
void updateWithValidLeaderElectionRecordShouldSendPutRequest() throws Exception {
// Given
final Resource<ConfigMap> configMapResource = configMaps.withName("name");
final ReplaceDeletable<ConfigMap> replaceable = mock(ReplaceDeletable.class, Answers.RETURNS_DEEP_STUBS);
when(configMapResource.lockResourceVersion(any())).thenReturn(replaceable);
final ConfigMap configMapInTheCluster = new ConfigMap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
import java.util.List;
import java.util.Map;

import static io.fabric8.kubernetes.client.utils.KubernetesResourceUtil.mergeConfigMapData;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.entry;
Expand Down Expand Up @@ -328,84 +327,6 @@ void createNewConfigMapFromDirOrFiles_whenBinaryFileProvided_shouldCreateConfigM
.contains(entry("test.bin", "wA=="));
}

@Test
void mergeConfigMapData_whenOneConfigMapNull_thenReturnNonNullConfigMap() {
// Given
ConfigMap cm = new ConfigMapBuilder()
.addToData("one", "1")
.build();

// When
ConfigMap result1 = mergeConfigMapData(cm, null);
ConfigMap result2 = mergeConfigMapData(null, cm);

// Then
assertThat(result1).isEqualTo(cm);
assertThat(result2).isEqualTo(cm);
}

@Test
void mergeConfigMapData_whenOneConfigMapNullData_thenReturnNonNullConfigMap() {
// Given
ConfigMap cm1 = new ConfigMapBuilder()
.addToData("one", "1")
.build();
ConfigMap cm2 = new ConfigMapBuilder().withData(null).build();

// When
ConfigMap result1 = mergeConfigMapData(cm1, cm2);
ConfigMap result2 = mergeConfigMapData(cm2, cm1);

// Then
assertThat(result1)
.hasFieldOrPropertyWithValue("data.one", "1")
.isEqualTo(result2);
}

@Test
void mergeConfigMapData_whenBothConfigMapNullData_thenReturnConfigMapWithEmptyData() {
// Given
ConfigMap cm1 = new ConfigMapBuilder().withData(null).build();
ConfigMap cm2 = new ConfigMapBuilder().withData(null).build();

// When
ConfigMap result = mergeConfigMapData(cm1, cm2);

// Then
assertThat(result)
.extracting(ConfigMap::getData)
.isNotNull();
}

@Test
void mergeConfigMapData_whenBothConfigMapsNonNullData_thenMergeConfigMaps() {
// Given
ConfigMap cm1 = new ConfigMapBuilder()
.addToData("e1", "v1")
.addToData("e2", "v2")
.addToData("e3", "v3")
.build();
ConfigMap cm2 = new ConfigMapBuilder()
.addToData("color.good", "blue")
.addToData("color.bad", "yellow")
.addToBinaryData("bin1", "fu+/vWIK")
.build();

// When
ConfigMap result = mergeConfigMapData(cm1, cm2);

// Then
assertThat(result)
.satisfies(r -> assertThat(r.getData())
.containsEntry("e1", "v1")
.containsEntry("e2", "v2")
.containsEntry("e3", "v3")
.containsEntry("color.good", "blue")
.containsEntry("color.bad", "yellow"))
.satisfies(r -> assertThat(r.getBinaryData())
.containsEntry("bin1", "fu+/vWIK"));
}

@SafeVarargs
private final void assertConfigMapContainsData(ConfigMap configMap, MapEntry<String, String>... mapEntries) {
assertThat(configMap)
Expand Down
Loading

0 comments on commit 99af146

Please sign in to comment.