You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2022/10/21 13:57:53 UTC
[brooklyn-server] 01/11: address code review comments, improve https support
This is an automated email from the ASF dual-hosted git repository.
heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit a50296510d3cb950bc158fd69c8875be020f643a
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Wed Oct 19 09:29:32 2022 +0100
address code review comments, improve https support
---
.../core/workflow/WorkflowStepDefinition.java | 8 ++--
.../core/workflow/steps/HttpWorkflowStep.java | 6 +++
.../core/workflow/steps/SshWorkflowStep.java | 6 +--
.../core/workflow/WorkflowBeefyStepTest.java | 53 +++++++++++++++++-----
.../tasks/kubectl/ContainerWorkflowStep.java | 9 +---
.../brooklyn/location/winrm/WinrmWorkflowStep.java | 4 +-
.../brooklyn/util/http/executor/HttpConfig.java | 5 ++
.../brooklyn/util/http/executor/HttpRequest.java | 2 +-
.../executor/apacheclient/HttpExecutorImpl.java | 3 +-
9 files changed, 65 insertions(+), 31 deletions(-)
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
index 5d32073a34..92532a0bc9 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
@@ -47,15 +47,12 @@ public abstract class WorkflowStepDefinition {
private static final Logger log = LoggerFactory.getLogger(WorkflowStepDefinition.class);
- // name: a name to display in the UI; if omitted it is constructed from the step ID and step type
- @JsonInclude(JsonInclude.Include.NON_EMPTY)
- protected Map<String,Object> input = MutableMap.of();
-
protected String id;
public String getId() {
return id;
}
+ // name: a name to display in the UI; if omitted it is constructed from the step ID and step type
protected String name;
public String getName() {
return name;
@@ -63,6 +60,9 @@ public abstract class WorkflowStepDefinition {
protected String userSuppliedShorthand;
+ @JsonInclude(JsonInclude.Include.NON_EMPTY)
+ protected Map<String,Object> input = MutableMap.of();
+
// next: the next step to go to, assuming the step runs and succeeds; if omitted, or if the condition does not apply, it goes to the next step per the ordering (described below)
@JsonProperty("next") //use this field for access, not the getter/setter
protected String next;
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/HttpWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/HttpWorkflowStep.java
index 35501196e0..8a4988029b 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/HttpWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/HttpWorkflowStep.java
@@ -34,6 +34,7 @@ import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.http.HttpTool;
import org.apache.brooklyn.util.http.auth.UsernamePassword;
+import org.apache.brooklyn.util.http.executor.HttpConfig;
import org.apache.brooklyn.util.http.executor.HttpExecutor;
import org.apache.brooklyn.util.http.executor.HttpRequest;
import org.apache.brooklyn.util.http.executor.HttpResponse;
@@ -62,6 +63,9 @@ public class HttpWorkflowStep extends WorkflowStepDefinition {
public static final ConfigKey<Map<String, String>> HEADERS = new MapConfigKey<>(String.class, "headers");
public static final ConfigKey<String> METHOD = ConfigKeys.newStringConfigKey("method");
+ /** directly customizable here, otherwise based on entity and brooklyn.properties per {@link BrooklynHttpConfig} */
+ public static final ConfigKey<HttpConfig> HTTPS_CONFIG = ConfigKeys.newConfigKey(HttpConfig.class, "config");
+
public static final ConfigKey<String> USERNAME = ConfigKeys.newStringConfigKey("username", "Username for HTTP request, if required");
public static final ConfigKey<String> PASSWORD = ConfigKeys.newStringConfigKey("password", "Password for HTTP request, if required");
@@ -116,6 +120,8 @@ public class HttpWorkflowStep extends WorkflowStepDefinition {
Map<String, String> headers = context.getInput(HEADERS);
if (headers!=null) httpb.headers(headers);
+ httpb.config(context.getInput(HTTPS_CONFIG));
+
String method = context.getInput(METHOD);
if (Strings.isBlank(method)) method = "get";
httpb.method(method);
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
index 0d14f70345..5b084c8bd9 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
@@ -19,7 +19,6 @@
package org.apache.brooklyn.core.workflow.steps;
import com.google.common.reflect.TypeToken;
-import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.config.ConfigKey;
import org.apache.brooklyn.core.config.ConfigKeys;
import org.apache.brooklyn.core.config.MapConfigKey;
@@ -34,7 +33,6 @@ import org.apache.brooklyn.util.core.task.DynamicTasks;
import org.apache.brooklyn.util.core.task.ssh.SshTasks;
import org.apache.brooklyn.util.core.task.system.ProcessTaskFactory;
import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
-import org.apache.brooklyn.util.core.units.Range;
import org.apache.brooklyn.util.text.Strings;
import java.util.Map;
@@ -45,7 +43,7 @@ public class SshWorkflowStep extends WorkflowStepDefinition {
public static final ConfigKey<String> ENDPOINT = ConfigKeys.newStringConfigKey("endpoint");
public static final ConfigKey<String> COMMAND = ConfigKeys.newStringConfigKey("command");
- public static final ConfigKey<Map<String,Object>> ENv = new MapConfigKey.Builder(Object.class, "env").build();
+ public static final ConfigKey<Map<String,Object>> ENV = new MapConfigKey.Builder(Object.class, "env").build();
public static final ConfigKey<DslPredicates.DslPredicate<Integer>> EXIT_CODE = ConfigKeys.newConfigKey(new TypeToken<DslPredicates.DslPredicate<Integer>>() {}, "exit-code");
@Override
@@ -74,7 +72,7 @@ public class SshWorkflowStep extends WorkflowStepDefinition {
public static <U, T extends ProcessTaskFactory<U>> ProcessTaskFactory<Map<?,?>> customizeProcessTaskFactory(WorkflowStepInstanceExecutionContext context, T tf) {
DslPredicates.DslPredicate<Integer> exitcode = context.getInput(EXIT_CODE);
if (exitcode!=null) tf.allowingNonZeroExitCode();
- Map<String, Object> env = context.getInput(ENv);
+ Map<String, Object> env = context.getInput(ENV);
if (env!=null) tf.environmentVariables(new ShellEnvironmentSerializer(context.getWorkflowExectionContext().getManagementContext()).serialize(env));
return tf.returning(ptw -> {
checkExitCode(ptw, exitcode);
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBeefyStepTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBeefyStepTest.java
index e382b4cfe4..996bacf3f1 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBeefyStepTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBeefyStepTest.java
@@ -24,35 +24,25 @@ import org.apache.brooklyn.api.entity.EntitySpec;
import org.apache.brooklyn.api.location.LocationSpec;
import org.apache.brooklyn.api.location.NoMachinesAvailableException;
import org.apache.brooklyn.api.mgmt.Task;
-import org.apache.brooklyn.api.typereg.RegisteredType;
import org.apache.brooklyn.core.entity.EntityAsserts;
import org.apache.brooklyn.core.entity.EntityInternal;
-import org.apache.brooklyn.core.resolve.jackson.BeanWithTypePlanTransformer;
import org.apache.brooklyn.core.sensor.Sensors;
import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport;
-import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry;
-import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
-import org.apache.brooklyn.core.typereg.RegisteredTypes;
-import org.apache.brooklyn.core.workflow.steps.LogWorkflowStep;
import org.apache.brooklyn.entity.stock.BasicApplication;
import org.apache.brooklyn.location.localhost.LocalhostMachineProvisioningLocation;
import org.apache.brooklyn.location.ssh.SshMachineLocation;
-import org.apache.brooklyn.location.ssh.SshMachineLocationReuseIntegrationTest;
import org.apache.brooklyn.test.Asserts;
-import org.apache.brooklyn.test.ClassLogWatcher;
import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.core.config.ConfigBag;
import org.apache.brooklyn.util.core.http.BetterMockWebServer;
import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool;
+import org.apache.brooklyn.util.http.executor.HttpConfig;
import org.apache.brooklyn.util.net.Networking;
-import org.apache.brooklyn.util.text.Strings;
import org.apache.brooklyn.util.time.Duration;
-import org.testng.Assert;
import org.testng.annotations.Test;
import java.io.IOException;
-import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;
@@ -69,6 +59,9 @@ public class WorkflowBeefyStepTest extends BrooklynMgmtUnitTestSupport {
return runSteps(MutableList.<Object>of(step), appFunction);
}
Object runSteps(List<Object> steps, Consumer<BasicApplication> appFunction) {
+ return runSteps(steps, appFunction, null);
+ }
+ Object runSteps(List<Object> steps, Consumer<BasicApplication> appFunction, ConfigBag defaultConfig) {
loadTypes();
BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
this.lastApp = app;
@@ -76,6 +69,7 @@ public class WorkflowBeefyStepTest extends BrooklynMgmtUnitTestSupport {
.configure(WorkflowEffector.EFFECTOR_NAME, "myWorkflow")
.configure(WorkflowEffector.EFFECTOR_PARAMETER_DEFS, MutableMap.of("p1", MutableMap.of("defaultValue", "p1v")))
.configure(WorkflowEffector.STEPS, steps)
+ .putAll(defaultConfig)
);
if (appFunction!=null) appFunction.accept(app);
eff.apply((EntityLocal)app);
@@ -85,7 +79,7 @@ public class WorkflowBeefyStepTest extends BrooklynMgmtUnitTestSupport {
}
@Test
- public void testEffector() throws IOException {
+ public void testEffector() {
Object result = runSteps(MutableList.of(
"let x = ${entity.sensor.x} + 1 ?? 0",
"set-sensor x = ${x}",
@@ -130,6 +124,41 @@ public class WorkflowBeefyStepTest extends BrooklynMgmtUnitTestSupport {
Asserts.assertThat(result.get("duration"), x -> Duration.nanos(1).isShorterThan(Duration.of(x)));
}
+ @Test(groups="Integration") //requires internet
+ public void testHttps() throws IOException {
+ doTestHttpsGoogle("https://www.google.com", null, true);
+ doTestHttpsGoogle("www.google.com", null, true);
+ // IP of google won't work unless we trust it
+ doTestHttpsGoogle("172.217.169.68", null, false);
+ doTestHttpsGoogle("172.217.169.68", MutableMap.of("config", HttpConfig.builder().trustAll(true).build()), true);
+ doTestHttpsGoogle("172.217.169.68", MutableMap.of("config", MutableMap.of("trustAll", true)), true);
+ doTestHttpsGoogle("172.217.169.68", MutableMap.of("config", MutableMap.of("trustAll", false)), false);
+ }
+
+ public Map doTestHttpsGoogle(String url, Map<String, Object> extraConfig, Boolean shouldWork) {
+ Map result = null;
+ try {
+ result = (Map) runStep(MutableMap.<String, Object>of("s", "http " + url).add(extraConfig), null);
+ if (shouldWork == null) {
+ // no op, just return result
+ } else if (shouldWork) {
+ Asserts.assertEquals(result.get("status_code"), 200);
+ MutableList.of("" + result.get("content"), "" + new String((byte[]) result.get("content_bytes"))).forEach(s ->
+ Asserts.assertStringContains(s, "<html", "google.timers.load"));
+ } else {
+ Asserts.shouldHaveFailedPreviously("Instead got: " + result);
+ }
+ } catch (Exception e) {
+ if (Boolean.FALSE.equals(shouldWork)) {
+ // expected, just make sure it isn't the "should have failed" exception
+ Asserts.expectedFailure(e);
+ } else {
+ Asserts.fail(e);
+ }
+ }
+ return result;
+ }
+
// container, winrm defined in downstream projects and tested in those projects and/or workflow yaml
/*
diff --git a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java
index 9bd0c57d3c..0a41d5dbb8 100644
--- a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java
+++ b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java
@@ -19,7 +19,6 @@
package org.apache.brooklyn.tasks.kubectl;
import com.google.common.reflect.TypeToken;
-import org.apache.brooklyn.api.mgmt.Task;
import org.apache.brooklyn.config.ConfigKey;
import org.apache.brooklyn.core.config.ConfigKeys;
import org.apache.brooklyn.core.config.MapConfigKey;
@@ -30,11 +29,7 @@ import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.core.json.ShellEnvironmentSerializer;
import org.apache.brooklyn.util.core.predicates.DslPredicates;
import org.apache.brooklyn.util.core.task.DynamicTasks;
-import org.apache.brooklyn.util.core.task.ssh.SshTasks;
-import org.apache.brooklyn.util.core.task.system.ProcessTaskFactory;
-import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
import org.apache.brooklyn.util.text.Strings;
-import org.apache.brooklyn.util.time.Duration;
import java.util.Arrays;
import java.util.List;
@@ -49,7 +44,7 @@ public class ContainerWorkflowStep extends WorkflowStepDefinition {
public static final ConfigKey<List<String>> COMMANDS = ConfigKeys.newConfigKey(new TypeToken<List<String>>() {}, "commands");
public static final ConfigKey<List<String>> RAW_COMMAND = ConfigKeys.newConfigKey(new TypeToken<List<String>>() {}, "raw-command");
public static final ConfigKey<PullPolicy> PULL_POLICY = ConfigKeys.newConfigKey(PullPolicy.class, "pull-policy", ContainerCommons.CONTAINER_IMAGE_PULL_POLICY.getDescription(), ContainerCommons.CONTAINER_IMAGE_PULL_POLICY.getDefaultValue());
- public static final ConfigKey<Map<String,Object>> ENv = new MapConfigKey.Builder(Object.class, "env").build();
+ public static final ConfigKey<Map<String,Object>> ENV = new MapConfigKey.Builder(Object.class, "env").build();
public static final ConfigKey<DslPredicates.DslPredicate<Integer>> EXIT_CODE = ConfigKeys.newConfigKey(new TypeToken<DslPredicates.DslPredicate<Integer>>() {}, "exit-code");
@Override
@@ -92,7 +87,7 @@ public class ContainerWorkflowStep extends WorkflowStepDefinition {
throw new IllegalStateException("Incompatible command specification, max 1, received: "+commandTypesSet);
}
- Map<String, Object> env = context.getInput(ENv);
+ Map<String, Object> env = context.getInput(ENV);
if (env != null)
tf.environmentVariables(new ShellEnvironmentSerializer(context.getWorkflowExectionContext().getManagementContext()).serialize(env));
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java
index 60241c4db5..bbb8ab4e65 100644
--- a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java
+++ b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java
@@ -41,7 +41,7 @@ public class WinrmWorkflowStep extends WorkflowStepDefinition {
public static final ConfigKey<String> ENDPOINT = ConfigKeys.newStringConfigKey("endpoint");
public static final ConfigKey<String> COMMAND = ConfigKeys.newStringConfigKey("command");
- public static final ConfigKey<Map<String,Object>> ENv = new MapConfigKey.Builder(Object.class, "env").build();
+ public static final ConfigKey<Map<String,Object>> ENV = new MapConfigKey.Builder(Object.class, "env").build();
public static final ConfigKey<DslPredicates.DslPredicate<Integer>> EXIT_CODE = ConfigKeys.newConfigKey(new TypeToken<DslPredicates.DslPredicate<Integer>>() {}, "exit-code");
@Override
@@ -67,7 +67,7 @@ public class WinrmWorkflowStep extends WorkflowStepDefinition {
DslPredicates.DslPredicate<Integer> exitcode = context.getInput(EXIT_CODE);
ProcessTaskFactory<?> tf = WinRmTasks.newWinrmExecTaskFactory(machine, command);
if (exitcode!=null) tf.allowingNonZeroExitCode();
- Map<String, Object> env = context.getInput(ENv);
+ Map<String, Object> env = context.getInput(ENV);
if (env!=null) tf.environmentVariables(new ShellEnvironmentSerializer(context.getWorkflowExectionContext().getManagementContext()).serialize(env));
tf.returning(ptw -> {
checkExitCode(ptw, exitcode);
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java
index e9ac8eb67a..39a1294ec4 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpConfig.java
@@ -57,6 +57,11 @@ public class HttpConfig {
private final boolean trustAll;
private final boolean trustSelfSigned;
+ /** jackson provider */
+ protected HttpConfig() {
+ this(builder());
+ }
+
protected HttpConfig(Builder builder) {
laxRedirect = builder.laxRedirect;
trustAll = builder.trustAll;
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java
index 9db28392db..5cf36057d5 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/HttpRequest.java
@@ -125,7 +125,7 @@ public interface HttpRequest {
Credentials credentials();
/**
- * Additional optional configuration to customize how the call is done.
+ * Redirect and trust/cert settings. If supplied, overrides the executor's. If blank, takes from the exector.
*/
@Nullable
HttpConfig config();
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java
index 79cd722366..50c248bc82 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/http/executor/apacheclient/HttpExecutorImpl.java
@@ -61,6 +61,7 @@ public class HttpExecutorImpl implements HttpExecutor {
HttpConfig config = DEFAULT_CONFIG;
+ /** config to use if none is specified on the request */
public HttpExecutorImpl withConfig(HttpConfig config) {
this.config = config;
return this;
@@ -68,7 +69,7 @@ public class HttpExecutorImpl implements HttpExecutor {
@Override
public HttpResponse execute(HttpRequest request) throws IOException {
- HttpConfig config = (request.config() != null) ? request.config() : DEFAULT_CONFIG;
+ HttpConfig config = (request.config() != null) ? request.config() : this.config!=null ? this.config : DEFAULT_CONFIG;
Credentials creds = (request.credentials() != null) ? new UsernamePasswordCredentials(request.credentials().getUser(), request.credentials().getPassword()) : null;
HttpClient httpClient = HttpTool.httpClientBuilder()
.uri(request.uri())