turbot / steampipe-plugin-jira

Use SQL to instantly query Jira. Open source CLI. No DB required.
https://hub.steampipe.io/plugins/turbot/jira
Apache License 2.0
22 stars 14 forks source link

Add support for on-premise Jira instances #86

Closed juandspy closed 1 year ago

juandspy commented 1 year ago

Adding support for on-premise Jira instances. I made use of the Bearer PAT from the parent Jira package.

On self-hosted Jira instances the username is not needed for API calls.

lorcanmcdonald commented 1 year ago

Just FYI, this change would also apply to Steampipe+Confluence and Steampipe+BitBucket too

juandspy commented 1 year ago

Just FYI, this change would also apply to Steampipe+Confluence and Steampipe+BitBucket too

Do you mean I should also create PRs there? I don't have any self-hosted instance of bitbucket or confluence to test :grimacing:

misraved commented 1 year ago

Welcome to Steampipe @juandspy and great to see the PR πŸ‘!!

Few questions on the changes:

juandspy commented 1 year ago

Welcome to Steampipe @juandspy and great to see the PR +1!!

Thanks! I'm loving Steampipe so far :)

Few questions on the changes:

  • Do we need to pass the Bearer PAT in the token config argument? Or should we create a new config argument? IMO the latter sounds like a more feasible idea.

I think this way is friendlier. What do you propose? :)

  • When using the PAT, are there any tables that would still require the username?

In the code, the username is just used for the connection. However, there are some tables like jira_user that cannot be queried (at least in my self-hosted instance):

> select * from jira_user;

Error: request failed. Please analyze the request body for more details. Status code: 404 (SQLSTATE HV000)

+--------------+------------+---------------+--------------+--------+------+-------------+-------------+-------+------+
| display_name | account_id | email_address | account_type | active | self | avatar_urls | group_names | title | _ctx |
+--------------+------------+---------------+--------------+--------+------+-------------+-------------+-------+------+
+--------------+------------+---------------+--------------+--------+------+-------------+-------------+-------+------+

I'm not sure if this is due to my instance or my permissions or if this would happen for all cases. Maybe we can just append to the error message a note about PAT tokens not supported for all tables (?)

lorcanmcdonald commented 1 year ago

Just FYI, this change would also apply to Steampipe+Confluence and Steampipe+BitBucket too

Do you mean I should also create PRs there? I don't have any self-hosted instance of bitbucket or confluence to test 😬

I just meant that the same problem is present in the other plugins too.

(I’m not associated with the steampipe project, by the way! Just someone who found the project over the weekend and was amazed to find a PR already open for an issue I was having πŸ˜„)

Happy work to port, and/or test, the fix to the other plugins once this one is merged πŸ‘

juandspy commented 1 year ago

(I’m not associated with the steampipe project, by the way! Just someone who found the project over the weekend and was amazed to find a PR already open for an issue I was having smile)

awesome! glad it helped :)

cbruno10 commented 1 year ago

@juandspy Do you have an example/mock of what a bearer PAT looks like? When we add plugins to Turbot Pipes, we add validation to configuration arguments, so if the PATs have different regex patterns than the API tokens the plugin takes in today, that may be a good reason to have a separate config arg (and also more clear what users should pass in when creating connections).

Also, for the error message in:

> select * from jira_user;

Error: request failed. Please analyze the request body for more details. Status code: 404 (SQLSTATE HV000)

Does Jira give more details? Is the request body actually available and we can return that along with the error? This would be difficult for users to understand and if we add a note around PATs in this error message, that may not always be applicable since Jira is just throwing a 404 error here, which they may throw for other cases too.

juandspy commented 1 year ago

@juandspy Do you have an example/mock of what a bearer PAT looks like? When we add plugins to Turbot Pipes, we add validation to configuration arguments, so if the PATs have different regex patterns than the API tokens the plugin takes in today, that may be a good reason to have a separate config arg (and also more clear what users should pass in when creating connections).

I can only say that I generated a couple of tokens and they have length of 44 characters, being numbers, letters and symbols. F.e "MDU0MDMx7cE25TQ3OujDfy/vkv/eeSXXoh/zXY1ex9cp" (this is not my token btw :laughing: ).

Also, for the error message in:

> select * from jira_user;

Error: request failed. Please analyze the request body for more details. Status code: 404 (SQLSTATE HV000)

Does Jira give more details? Is the request body actually available and we can return that along with the error? This would be difficult for users to understand and if we add a note around PATs in this error message, that may not always be applicable since Jira is just throwing a 404 error here, which they may throw for other cases too.

The request body is empty actually... You can see it's set to nil here.

cbruno10 commented 1 year ago

@juandspy I just checked my API token and it had a format like wkNO4jUlNktmg4s1AJL1FAKE (though this may be an older token with fewer characters, I think the newer tokens are 100+ chars though, see https://jira.atlassian.com/browse/ID-8131), so they do appear to be different. Given this, I think I'd recommend creating another config arg called personal_access_token with a link to https://confluence.atlassian.com/enterprise/using-personal-access-tokens-1026032365.html as a comment.

If both the token and personal_access_token config args are set in a connection, the plugin should return an error with the message 'token' and 'personal_access_token' are both set, please only use one.

Can you please also update the comment line for token to be API token for Jira Cloud. See https://support.atlassian.com/atlassian-account/docs/manage-api-tokens-for-your-atlassian-account/.

In retrospect, I'd probably would have named token api_token instead if we knew about PATs before, but we shouldn't change it now since that'd be a breaking change (but we may add api_token later and deprecate token).

For the username config arg, given that Jira returns a vague error, maybe it's better to leave username mandatory? This will ensure the jira_user table will work, as long as a valid username is passed in.

lorcanmcdonald commented 1 year ago

For the username config arg, given that Jira returns a vague error, maybe it's better to leave username mandatory? This will ensure the jira_user table will work, as long as a valid username is passed in.

Yep, username should be required, if only for consistency.

A Personal Access Token is essentially an extra password on the user’s account. At least some of the other Atlassian APIs use it via Basic Auth (Confluence does, if I remember correctly)

juandspy commented 1 year ago

OK, I'll do the changes. However, as of now, if the username is not defined, there is no error message in the plugin:

❯ steampipe query "select key, epic_key, created, updated, creator_display_name from jira_issue limit 10;"

Error: rpc error: code = Internal desc = hydrate function listIssues failed with panic runtime error: invalid memory address or nil pointer dereference (SQLSTATE HV000)

+-----+----------+---------+---------+----------------------+
| key | epic_key | created | updated | creator_display_name |
+-----+----------+---------+---------+----------------------+
+-----+----------+---------+---------+----------------------+

however if you check the plugin logs:

❯ cat ~/.steampipe/logs/plugin-2023-08-10.log | grep username
2023-08-10 05:50:18.253 UTC [ERROR] steampipe-plugin-jira.plugin: [ERROR] 1691646618420: jira_issue.listIssues.searchWithContext: connection_error="'username' must be set in the connection configuration. Edit your connection configuration file and then restart Steampipe"

Is this the expected behavior?

juandspy commented 1 year ago

If both the token and personal_access_token config args are set in a connection, the plugin should return an error with the message 'token' and 'personal_access_token' are both set, please only use one.

@cbruno10 alright, updated :)

cbruno10 commented 1 year ago

@juandspy No that's not expected, the plugin should be showing the error instead of a nil pointer dereference. We could be trying to grab the error from a missing property or element.

juandspy commented 1 year ago

@cbruno10 can we merge this one already?

lorcanmcdonald commented 1 year ago
> select * from jira_project

Error: invalid character '<' looking for beginning of value (SQLSTATE HV000)

I'm getting errors on some of the Jira tables when testing this branch, jira_project and jira_epic for example.

Tables which appear to work correctly are: jira_board and jira_issue

(We're on the Long Term Release version)

cbruno10 commented 1 year ago

Thanks for testing @lorcanmcdonald !

@juandspy Were you able to query the tables @lorcanmcdonald had mentioned successfully? If so, are you on a different version?

juandspy commented 1 year ago

Sorry @cbruno10 @lorcanmcdonald , I've been busy lately.

> select * from jira_project

Error: invalid character '<' looking for beginning of value (SQLSTATE HV000)

I've been investigating this issue. It looks like the jira_project table gets the API response in HTML format, that's why we see the invalid character '<' looking for beginning of value error. If you look at the issues table, you may realize that a JQL query is used, not a raw query to the REST API. This is because the JQL is used just for getting issues, not projects. I'm afraid we will need to change the way we get the projects.

juandspy commented 1 year ago

For me just these tables work:

table works
jira_advanced_setting ❌
jira_backlog_issue βœ…
jira_board βœ…
jira_component ❌
jira_dashboard ❌
jira_epic ❌
jira_global_setting ❌
jira_group ❌
jira_issue βœ…
jira_issue_type ❌
jira_priority ❌
jira_project ❌
jira_project_role ❌
jira_sprint βœ…
jira_user ❌
jira_workflow ❌

Maybe we can put a disclaimer that for self-hosted Jira instances just these tables work and create a set of issues so that we fix the rest in the future.

BTW the code I've used to generate this:

#!/bin/bash

folder="docs/tables"

echo "table,works"

for file in "$folder"/*; do
    if [ -f "$file" ]; then
        table=$(basename "$file" ".md")

        cmd="steampipe query \"select * from $table LIMIT 5;\""
        eval "$cmd" &> /dev/null
        ret=$?
        if [ $ret -ne 0 ]; then
                echo "$table,❌"
        else
                echo "$table,βœ…"
        fi
    fi
done

I guess this can be reused in any plugin. So I'm sharing just for saving future work :laughing:

juandspy commented 1 year ago

@lorcanmcdonald Also, the lazy approach for listing the Jira projects would be

SELECT DISTINCT project_key FROM jira_issue;

:stuck_out_tongue:

misraved commented 1 year ago

@juandspy apologies for the delayed response and thanks for sharing the table query results πŸ‘!!

Could you please resolve the merge conflicts so that we can merge in the PR and make it available on the hub πŸ‘!!

juandspy commented 1 year ago

@juandspy apologies for the delayed response and thanks for sharing the table query results πŸ‘!!

Could you please resolve the merge conflicts so that we can merge in the PR and make it available on the hub πŸ‘!!

shall I add some notes about the tables that are working in Jira server vs Jira cloud? @misraved

misraved commented 1 year ago

Thanks @juandspy for pushing the changes πŸ‘ !!

In the docs file, we could add a section that mentions the tables that work with PAT πŸ‘. So yes, notes around Jira server vs Jira cloud would be great πŸ‘.

We could merge and release the feature today πŸš€ !!

juandspy commented 1 year ago

Thanks @juandspy for pushing the changes πŸ‘ !!

In the docs file, we could add a section that mentions the tables that work with PAT πŸ‘. So yes, notes around Jira server vs Jira cloud would be great πŸ‘.

We could merge and release the feature today πŸš€ !!

Updated. Once you ship a new version of the plugin, I will create an issue to track this bug.

juandspy commented 1 year ago

Hey @misraved , I cannot see https://github.com/turbot/steampipe-plugin-jira/pull/86/commits/6121a8a5faa17c186c58d78c2086a1a3de25da3c in docs/index.md. Any idea?