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 <[email protected]> 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())
