Skip to content

Commit

Permalink
reflect review
Browse files Browse the repository at this point in the history
  • Loading branch information
@hnarimiya
hnarimiya committed Jul 6, 2022
1 parent b67eab8 commit 0b6b32c
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 31 deletions.
52 changes: 23 additions & 29 deletions ...tandards/src/main/java/io/digdag/standards/command/kubernetes/KubernetesClientConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@
import com.google.common.base.Optional;
import io.digdag.client.config.Config;
import io.digdag.client.config.ConfigException;
import io.digdag.client.config.ConfigFactory;
import io.digdag.core.storage.StorageManager;

import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

import static io.digdag.client.DigdagClient.objectMapper;

public class KubernetesClientConfig
{
private static final String KUBERNETES_CLIENT_PARAMS_PREFIX = "agent.command_executor.kubernetes.";
Expand All @@ -19,47 +22,38 @@ public static KubernetesClientConfig create(final Optional<String> name,
final Config systemConfig,
final Config requestConfig)
{

String clusterName = null;
if (name.isPresent()) {
clusterName = name.get();
if (!systemConfig.get("agent.command_executor.type", String.class, "").equals("kubernetes")) {
throw new ConfigException("agent.command_executor.type: is not 'kubernetes'");
}

Config extractedSystemConfig = null;
if (systemConfig != null && systemConfig.get("agent.command_executor.type", String.class, "").equals("kubernetes")) {
if (clusterName == null) {
clusterName = systemConfig.get(KUBERNETES_CLIENT_PARAMS_PREFIX + "name", String.class); // ConfigException
}
final String keyPrefix = KUBERNETES_CLIENT_PARAMS_PREFIX + clusterName + ".";
extractedSystemConfig = StorageManager.extractKeyPrefix(systemConfig, keyPrefix);
String clusterName;
if (!name.isPresent()) {
clusterName = systemConfig.get(KUBERNETES_CLIENT_PARAMS_PREFIX + "name", String.class); // ConfigException
} else {
clusterName = name.get();
}

Config extractedRequestConfig = null;
if (requestConfig != null && requestConfig.has("kubernetes")) {
final String keyPrefix = KUBERNETES_CLIENT_PARAMS_PREFIX + clusterName + ".";
Config extractedSystemConfig = StorageManager.extractKeyPrefix(systemConfig, keyPrefix);;
Config extractedRequestConfig;
if (systemConfig.get(KUBERNETES_CLIENT_PARAMS_PREFIX + "allow_configure_workflow_definition", Boolean.class, false)
&& requestConfig != null
&& requestConfig.has("kubernetes")) {
if (clusterName == null) {
clusterName = requestConfig.get("name", String.class); // ConfigException
}
extractedRequestConfig = requestConfig.getNested("kubernetes");
}

// Create a config that merges RequestConfig with SystemConfig
final Config config;
if (extractedSystemConfig != null && extractedRequestConfig != null) {
config = extractedSystemConfig.merge(extractedRequestConfig);

} else if (extractedRequestConfig != null) {
config = extractedRequestConfig;

} else if (extractedSystemConfig != null) {
config = extractedSystemConfig;

} else {
throw new ConfigException("systemConfig and requestConfig does not exist");
extractedRequestConfig = newEmptyConfig();
}

// Create a config that merges RequestConfig with SystemConfig
final Config config = extractedSystemConfig.merge(extractedRequestConfig);
return KubernetesClientConfig.createKubeConfig(clusterName, config);
}

private static Config newEmptyConfig() {
return new ConfigFactory(objectMapper()).create();
}

private static KubernetesClientConfig createKubeConfig(final String clusterName, final Config config){
if (config.has("kube_config_path")) {
String kubeConfigPath = config.get("kube_config_path", String.class);
Expand Down
40 changes: 38 additions & 2 deletions ...ards/src/test/java/io/digdag/standards/command/kubernetes/KubernetesClientConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ public void testCreateFromSystemConfigWithKubeConfig()
public void testCreateFromRequestConfig()
throws Exception
{
final Config systemConfig = cf.create()
.set("agent.command_executor.type", "kubernetes")
.set("agent.command_executor.kubernetes.allow_configure_workflow_definition", true);

final Config kubernetesConfig = cf.create()
.set("master", "https://127.0.0.1")
.set("certs_ca_data", "test=")
Expand All @@ -113,7 +117,7 @@ public void testCreateFromRequestConfig()
final Config requestConfig = cf.create()
.setNested("kubernetes", kubernetesConfig);

KubernetesClientConfig kubernetesClientConfig = KubernetesClientConfig.create(clusterName, null, requestConfig);
KubernetesClientConfig kubernetesClientConfig = KubernetesClientConfig.create(clusterName, systemConfig, requestConfig);

String masterUrl = "https://127.0.0.1";
String namespace = "default";
Expand All @@ -135,7 +139,8 @@ public void testCreateFromRequestConfigAndSystemConfigMerge()
.set(KUBERNETES_CLIENT_PARAMS_PREFIX+"test.master", "https://127.0.0.1")
.set(KUBERNETES_CLIENT_PARAMS_PREFIX+"test.certs_ca_data", "test=")
.set(KUBERNETES_CLIENT_PARAMS_PREFIX+"test.oauth_token", "test=")
.set(KUBERNETES_CLIENT_PARAMS_PREFIX+"test.namespace", "default");
.set(KUBERNETES_CLIENT_PARAMS_PREFIX+"test.namespace", "default")
.set("agent.command_executor.kubernetes.allow_configure_workflow_definition", true);

final Config kubernetesConfig = cf.create()
.set("master", "https://localhost")
Expand All @@ -158,4 +163,35 @@ public void testCreateFromRequestConfigAndSystemConfigMerge()
assertThat(namespace, is(kubernetesClientConfig.getNamespace()));
}

@Test
public void testCreateFromRequestConfigAndSystemConfigNotMerge()
throws Exception
{

final Config systemConfig = cf.create()
.set("agent.command_executor.type", "kubernetes")
.set(KUBERNETES_CLIENT_PARAMS_PREFIX+"test.master", "https://127.0.0.1")
.set(KUBERNETES_CLIENT_PARAMS_PREFIX+"test.certs_ca_data", "test=")
.set(KUBERNETES_CLIENT_PARAMS_PREFIX+"test.oauth_token", "test=")
.set(KUBERNETES_CLIENT_PARAMS_PREFIX+"test.namespace", "default");

final Config kubernetesConfig = cf.create()
.set("master", "https://localhost")
.set("namespace", "request");

final Config requestConfig = cf.create()
.setNested("kubernetes", kubernetesConfig);

KubernetesClientConfig kubernetesClientConfig = KubernetesClientConfig.create(clusterName, systemConfig, requestConfig);

String masterUrl = "https://127.0.0.1";
String namespace = "default";
String caCertData = "test=";
String oauthToken = "test=";
assertThat(masterUrl, is(kubernetesClientConfig.getMaster()));
assertThat(caCertData, is(kubernetesClientConfig.getCertsCaData()));
assertThat(oauthToken, is(kubernetesClientConfig.getOauthToken()));
assertThat(namespace, is(kubernetesClientConfig.getNamespace()));
}

}

0 comments on commit 0b6b32c

Please sign in to comment.