Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jira: set user-agent for http calls #192

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions tasks/jira/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@
<artifactId>wiremock</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,6 @@ private Constants() {
}

static final String BOUNDARY = UUID.randomUUID().toString();
public static final String PARAMS_KEY = "jiraParams";

}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ default long writeTimeout() {
return 30L;
}

default String userAgent() {
return "Concord-Jira-Plugin";
}

default HttpVersion httpProtocolVersion() {
return HttpVersion.DEFAULT;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,19 @@

import javax.inject.Inject;
import javax.inject.Named;
import java.nio.file.Path;
import java.util.Map;
import java.util.UUID;

import static com.walmartlabs.concord.plugins.jira.Constants.PARAMS_KEY;
import static com.walmartlabs.concord.plugins.jira.JiraTaskCommon.JIRA_ISSUE_STATUS_KEY;
import static com.walmartlabs.concord.sdk.Constants.Context.CONTEXT_KEY;

@Named("jira")
@SuppressWarnings("unused")
public class JiraTask implements Task {

private final SecretService secretService;

@InjectVariable("jiraParams")
@InjectVariable(PARAMS_KEY)
private Map<String, Object> defaults;

@Inject
Expand All @@ -52,17 +52,17 @@ public JiraTask(SecretService secretService) {

@Override
public void execute(Context ctx) {
JiraSecretService jiraSecretService = getSecretService(ctx);
Map<String, Object> result = delegate(jiraSecretService)
.execute(TaskParams.of(new ContextVariables(ctx), defaults));
var jiraSecretService = getSecretService(ctx);

result.forEach(ctx::setVariable);
delegate(jiraSecretService)
.execute(TaskParams.of(new ContextVariables(ctx), defaults))
.forEach(ctx::setVariable);
}

public String getStatus(@InjectVariable("context") Context ctx, String issueKey) {
Variables vars = TaskParams.merge(new ContextVariables(ctx), defaults);
JiraSecretService jiraSecretService = getSecretService(ctx);
Map<String, Object> result = delegate(jiraSecretService)
public String getStatus(@InjectVariable(CONTEXT_KEY) Context ctx, String issueKey) {
var vars = TaskParams.merge(new ContextVariables(ctx), defaults);
var jiraSecretService = getSecretService(ctx);
var result = delegate(jiraSecretService)
.execute(new TaskParams.CurrentStatusParams(vars) {
@Override
public String issueKey() {
Expand Down Expand Up @@ -93,11 +93,11 @@ public V1SecretService(SecretService secretService, Context ctx) {

@Override
public JiraCredentials exportCredentials(String orgName, String secretName, String password) throws Exception {
UUID txId = ContextUtils.getTxId(ctx);
Path workDir = ContextUtils.getWorkDir(ctx);
var txId = ContextUtils.getTxId(ctx).toString();
var workDir = ContextUtils.getWorkDir(ctx).toString();

Map<String, String> result1 = secretService.exportCredentials(ctx, txId.toString(), workDir.toString(), orgName, secretName, password);
return new JiraCredentials(result1.get("username"), result1.get("password"));
var creds = secretService.exportCredentials(ctx, txId, workDir, orgName, secretName, password);
return new JiraCredentials(creds.get("username"), creds.get("password"));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,59 +65,53 @@ public Map<String, Object> execute(TaskParams in) {
log.info("Using JIRA URL: {}", jiraUrl);

Action action = in.action();

switch (action) {
case CREATEISSUE: {
case CREATEISSUE -> {
log.info("Starting 'CreateIssue' Action");
return createIssue((CreateIssueParams)in);
return createIssue((CreateIssueParams) in);
}
case ADDCOMMENT: {
case ADDCOMMENT -> {
log.info("Starting 'AddComment' Action");
addComment((AddCommentParams)in);
break;
addComment((AddCommentParams) in);
}
case ADDATTACHMENT: {
case ADDATTACHMENT -> {
log.info("Starting 'AddAttachment' Action");
addAttachment((AddAttachmentParams)in);
break;
addAttachment((AddAttachmentParams) in);
}
case CREATECOMPONENT: {
case CREATECOMPONENT -> {
log.info("Starting 'CreateComponent' Action");
return createComponent((CreateComponentParams)in);
return createComponent((CreateComponentParams) in);
}
case DELETECOMPONENT: {
case DELETECOMPONENT -> {
log.info("Starting 'DeleteComponent' Action");
deleteComponent((DeleteComponentParams)in);
break;
deleteComponent((DeleteComponentParams) in);
}
case TRANSITION: {
case TRANSITION -> {
log.info("Starting 'Transition' Action");
transition((TransitionParams)in);
break;
transition((TransitionParams) in);
}
case DELETEISSUE: {
case DELETEISSUE -> {
log.info("Starting 'DeleteIssue' Action");
deleteIssue((DeleteIssueParams)in);
break;
deleteIssue((DeleteIssueParams) in);
}
case UPDATEISSUE: {
case UPDATEISSUE -> {
log.info("Starting 'UpdateIssue' Action");
updateIssue((UpdateIssueParams)in);
break;
updateIssue((UpdateIssueParams) in);
}
case CREATESUBTASK: {
case CREATESUBTASK -> {
log.info("Starting 'CreateSubTask' Action");
return createSubTask((CreateSubTaskParams)in);
return createSubTask((CreateSubTaskParams) in);
}
case CURRENTSTATUS: {
case CURRENTSTATUS -> {
log.info("Starting 'CurrentStatus' Action");
return currentStatus((CurrentStatusParams)in);
return currentStatus((CurrentStatusParams) in);
}
case GETISSUES: {
case GETISSUES -> {
log.info("Starting 'GetIssues' Action");
return getIssues((GetIssuesParams)in);
return getIssues((GetIssuesParams) in);
}
default:
throw new IllegalArgumentException("Unsupported action type: " + action);
default -> throw new IllegalArgumentException("Unsupported action type: " + action);
}
return Collections.emptyMap();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ private HttpRequest.Builder requestBuilder(String auth) {
return HttpRequest.newBuilder()
.timeout(Duration.ofSeconds(cfg.readTimeout()))
.header("Authorization", auth)
.header("User-Agent", cfg.userAgent())
.header("Accept", "application/json");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,50 +25,34 @@

import javax.annotation.Nonnull;
import java.util.*;
import java.util.stream.Stream;

public class TaskParams implements JiraClientCfg {

public static TaskParams of(Variables input, Map<String, Object> defaults) {
Variables variables = merge(input, defaults);
return of(input.toMap(), Map.of(), defaults);
}

public static TaskParams of(Map<String, Object> input,
Map<String, Object> globalDefaults,
Map<String, Object> policyDefaults) {

Variables variables = merge(input, globalDefaults, policyDefaults);

Action action = new TaskParams(variables).action();
switch (action) {
case ADDCOMMENT: {
return new AddCommentParams(variables);
}
case CREATECOMPONENT: {
return new CreateComponentParams(variables);
}
case CREATEISSUE: {
return new CreateIssueParams(variables);
}
case DELETECOMPONENT: {
return new DeleteComponentParams(variables);
}
case DELETEISSUE: {
return new DeleteIssueParams(variables);
}
case TRANSITION: {
return new TransitionParams(variables);
}
case UPDATEISSUE: {
return new UpdateIssueParams(variables);
}
case CREATESUBTASK: {
return new CreateSubTaskParams(variables);
}
case CURRENTSTATUS: {
return new CurrentStatusParams(variables);
}
case ADDATTACHMENT: {
return new AddAttachmentParams(variables);
}
case GETISSUES: {
return new GetIssuesParams(variables);
}
default:
throw new IllegalArgumentException("Unsupported action type: " + action);
}
return switch (action) {
case ADDCOMMENT -> new AddCommentParams(variables);
case CREATECOMPONENT -> new CreateComponentParams(variables);
case CREATEISSUE -> new CreateIssueParams(variables);
case DELETECOMPONENT -> new DeleteComponentParams(variables);
case DELETEISSUE -> new DeleteIssueParams(variables);
case TRANSITION -> new TransitionParams(variables);
case UPDATEISSUE -> new UpdateIssueParams(variables);
case CREATESUBTASK -> new CreateSubTaskParams(variables);
case CURRENTSTATUS -> new CurrentStatusParams(variables);
case ADDATTACHMENT -> new AddAttachmentParams(variables);
case GETISSUES -> new GetIssuesParams(variables);
};
}

private static final String ACTION_KEY = "action";
Expand All @@ -77,9 +61,11 @@ public static TaskParams of(Variables input, Map<String, Object> defaults) {
private static final String JIRA_USER_ID_KEY = "userId";
private static final String JIRA_PASSWORD_KEY = "password";
private static final String JIRA_HTTP_CLIENT_PROTOCOL_VERSION_KEY = "httpClientProtocolVersion";
private static final String CLIENT_CONNECTTIMEOUT = "connectTimeout";
private static final String CLIENT_READTIMEOUT = "readTimeout";
private static final String CLIENT_WRITETIMEOUT = "writeTimeout";
private static final String CLIENT_CONNECT_TIMEOUT_KEY = "connectTimeout";
private static final String CLIENT_READ_TIMEOUT_KEY = "readTimeout";
private static final String CLIENT_WRITE_TIMEOUT_KEY = "writeTimeout";
private static final String USER_AGENT_KEY = "userAgent";
private static final String TX_ID_KEY = "txId";

protected final Variables variables;

Expand All @@ -102,17 +88,27 @@ public String jiraUrl() {

@Override
public long connectTimeout() {
return variables.getLong(CLIENT_CONNECTTIMEOUT, JiraClientCfg.super.connectTimeout());
return variables.getLong(CLIENT_CONNECT_TIMEOUT_KEY, JiraClientCfg.super.connectTimeout());
}

@Override
public long readTimeout() {
return variables.getLong(CLIENT_READTIMEOUT, JiraClientCfg.super.readTimeout());
return variables.getLong(CLIENT_READ_TIMEOUT_KEY, JiraClientCfg.super.readTimeout());
}

@Override
public long writeTimeout() {
return variables.getLong(CLIENT_WRITETIMEOUT, JiraClientCfg.super.writeTimeout());
return variables.getLong(CLIENT_WRITE_TIMEOUT_KEY, JiraClientCfg.super.writeTimeout());
}

@Override
public String userAgent() {
return Optional.ofNullable(variables.getString(USER_AGENT_KEY))
.orElseGet(() -> "Concord-Jira-Plugin: " + txId());
}

public String txId() {
return variables.assertVariable(TX_ID_KEY, Object.class).toString();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

context.processInstanceId()

or is this not a process identifier?

Copy link
Collaborator Author

@benbroadaway benbroadaway Feb 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand the question/suggestion here. The overall task behavior is to use the process id (txId process variable in both runtimes). However, in runtime-v2 Context, that value is explicitly available through context.processInstanceId().

So in the task params merging (or just before), the value has to be copied into one of the input variable Maps or TaskParams would ned to be refactored to have another field for the process identifier--which seems a little unnecessary with the current big-bucket(map)-of-vars pattern.

It is decoupled enough from most of the runtime bits so that TaskParams and JiraTaskCommon could possibly used on their own.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sry. my bad. missed this part:

policyDefaults.put("txId", context.processInstanceId());

It seemed that txId needed to be explicitly passed as an input parameter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's in the growing pains of a super simple task requiring more system variables/services over time. We could further refactor JiraTaskCommon to accept another config object that covers that sort of stuff, but not coupled with the runtime (e.g. container/supplier(s) for equivalent of workDir, txId, etc) so they don't appear to be expected in: params.

}

public Map<String, Object> auth() {
Expand Down Expand Up @@ -159,6 +155,7 @@ public String projectKey() {
public String summary() {
return variables.assertString(JIRA_SUMMARY_KEY);
}

public String description() {
return variables.assertString(JIRA_DESCRIPTION_KEY);
}
Expand Down Expand Up @@ -405,6 +402,22 @@ public static Variables merge(Variables variables, Map<String, Object> defaults)
return new MapBackedVariables(variablesMap);
}

/**
* Merge task inputs from multiple locations
* @param input Direct task input parameters (e.g. from <code>in:</code> block in runtime-v2)
* @param globalDefaults Flow global defaults (e.g. from <code>configuration.arguments.jiraParams</code>
* @param policyDefaults System policy-provided defaults
* @return Merged variables
*/
static Variables merge(Map<String, Object> input, Map<String, Object> globalDefaults, Map<String, Object> policyDefaults) {
var merged = new HashMap<String, Object>();
Stream.of(policyDefaults, globalDefaults, input)
.filter(Objects::nonNull)
.forEach(merged::putAll);

return new MapBackedVariables(merged);
}

public enum Action {
ADDCOMMENT,
CREATECOMPONENT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,24 +28,33 @@

import javax.inject.Inject;
import javax.inject.Named;
import java.util.HashMap;
import java.util.Map;

import static com.walmartlabs.concord.plugins.jira.Constants.PARAMS_KEY;

@Named("jira")
@DryRunReady
public class JiraTaskV2 implements Task {

private final Context context;
private final JiraTaskCommon delegate;
private final Map<String, Object> globalDefaults;

@Inject
public JiraTaskV2(Context context) {
this.context = context;
this.globalDefaults = context.variables().getMap(PARAMS_KEY, Map.of());
this.delegate = new JiraTaskCommon(new V2SecretService(context.secretService()), context.processConfiguration().dryRun());
}

@Override
public TaskResult execute(Variables input) {
Map<String, Object> result = getDelegate().execute(TaskParams.of(input, context.defaultVariables().toMap()));
var policyDefaults = new HashMap<>(context.defaultVariables().toMap());
policyDefaults.put("txId", context.processInstanceId());

var result = getDelegate().execute(TaskParams.of(input.toMap(), globalDefaults, policyDefaults));

return TaskResult.success()
.values(result);
}
Expand All @@ -63,8 +72,8 @@ public V2SecretService(SecretService secretService) {

@Override
public JiraCredentials exportCredentials(String orgName, String secretName, String password) throws Exception {
SecretService.UsernamePassword up = secretService.exportCredentials(orgName, secretName, password);
return new JiraCredentials(up.username(), up.password());
var creds = secretService.exportCredentials(orgName, secretName, password);
return new JiraCredentials(creds.username(), creds.password());
}
}

Expand Down
Loading