add support for jira authentication via Personal Access Token

[buzzdeee]

Description

For Jira instances, that have basic-auth disabled, a Personal Access Token can be used.
This change allows to make use of such PAT.

Checklist

• I have reviewed the contributing guidelines.
• I have included unit tests for my changes or additions.
• I have successfully run make test-docker with my changes.
• I have manually tested all relevant modes of the change in this PR.
• I have updated the documentation.
• I have updated the changelog.

Questions or Comments

The reading of the jira_account.yaml file seems to be shared with the smtp alerter, but the change shouldn't affect it. I'm not good with writing python tests, therefore no tests added.

[jertel]

Thanks for your contribution. The existing unit tests are failing due to these changes so I cannot merge this PR as-is.

[nsano-rururu]

I need to change the test code

[buzzdeee]

@jertel @nsano-rururu

thanks for chiming in. Unfortunately, my knowledge about python mock tests is next to zero. I don't even know if my change broke existing functionality, or only the tests would need to be adapted.
Looking at the test output, I see where it's failing, and that's where I did the change in alerts.py, but as I don't really know how these tests work, I've unfortunately no clue how to address that (best).
@nsano-rururu if you know how to address the test, would be awesome if you could do so.

thanks,
Sebastian

[nsano-rururu]

I'm worried that I'm getting an error even though I haven't changed the jira test.
If you specify only user and password and do not specify api_key, it is assumed that no error will occur.
Does that mean that the implementation is not good?

[nsano-rururu]

Do not allow the merge.

[nsano-rururu]

@buzzdeee

I think that it is not the problem that the test of api_key is not added, but the problem that the test without api_key is failed.
If the implementation is correct, the test should not fail.
If the implementation is correct, it should have just pointed out that you should add the test by specifying only api_key.

[ferozsalam]

Hey @buzzdeee - you have two problems with your change which are making the unit tests fail. The first is that the logic of your conditional is incorrect.

Let's assume I defined my Jira account file as {'user': 'jirauser', 'password': 'jirapassword'}.

Looking at your conditional in this case, we have:

if 'user' not in account_conf and 'password' not in account_conf or 'apikey' not in account_conf:
    raise EAException('Account file must have user and password fields, or apikey field')

'user' not in account_conf here evaluates to false
'password' not in account_conf evaluates to false
'apikey' not in account_conf: to true

so now we have false and false or true, which evaluates to true - which will raise an exception and fail the unit tests.

I'll leave it to you to work out what the correct conditional format should be. There's another error after that: self.apikey - this call errors if there is no apikey set, so you either need to use hasattr to check for the property existing first, or wrap the call in a try/except block. I think the second way is more Pythonic.

Overall, regarding your PR I think you should reimplement the get_account functionality within jira.py rather than adding more functionality in alerts.py that is specific to the Jira alerter.

[buzzdeee]

Hi @ferozsalam

thanks for the explanations of my logic error. I only could test the api_key version, as that's the type of Jira server I've around, and that case just worked for me.

Also thanks for the suggestion regarding overriding get_account in jira.py, I'll look into that.

[ferozsalam]

Nicely done on getting the tests to work 👍

Are you able to add a unit test to check that if an API key alone is defined, things work as expected? This would be very similar to the existing Jira tests.

[buzzdeee]

Hi,

updated version now with tests passing:

• reverted changes to alerts.py
• override get_account in alerters/jira.py
• changed the logic in the if statement a bit, all these NOTs cobined with AND and OR were too confusing to me, that's why I didn't catched it initially. At least for me, this is now much more easy to follow.
• had to update the tests, a bit of google foo and reading pointed out, mock patch has to patch where function is used, not where it's declared.

Additionally, I tested with a config file:

• only with user given -> exception
• only with password given -> exception
• with user/password -> works
• with apikey -> works

As I understand my code, the apikey takes precedence. So if apikey is combined with user or password or both in the authentication file, it will try to use the apikey, without falling back to user/password in case that doesn't work. hope that's fine/fair enough ;)

[buzzdeee]

@ferozsalam

now there should be a test at the end of jira_test.py.
I hope I got it all right, at least it's passing now after a few attempts ;)

[nsano-rururu]

If user and password are specified, the value of self.apiKey will not be set, so if you pass "if self.apiKey:" in that state, an error will occur. I think that will happen. I thought it would be better to do "self.apikey = None" before calling the overridden get_account.