Add hmac_compute

[Ajnbro]

What does this PR do?

Creates a util and a module called hmac_compute for computing a HMAC SHA256 hexdigest using a string and secret.

What issues does this PR fix or reference?

N/A

Tests written?

Yes

Commits signed with GPG?

Yes

[waynew]

Thanks for the PR! This looks good - and the lint issues aren't actually with your code, just existing code.

Would you mind fixing those? It's not possible, apparently, to tell lint that we only care about changed lines on PRs 🙃

[nicholasmhughes]

@waynew , I think the right way to go here is to mark base64_encodestring and base64_decodestring for deprecation. Any attempts to fix the lint issues will end up duplicating the functionality already present in base64_b64encode and base64_b64decode. What do you think?

[waynew]

Actually, the lint issue is due to the fact that in Python3 it's base64.encodebytes instead of encodestring, now that I'm looking at the docs.

Since this is going into develop, and we're dropping Python2 support, and encodebytes was introduced in Python 3.1, I'd say that it's perfectly safe to change base64.encodestring to base64.encodebytes

[nicholasmhughes]

yeah, you could go that way. I'm just saying that "fixing" those lint issues would duplicate the functionality already found in base64_b64encode and base64_b64decode. you'd end up getting the same output. if you really want to keep the legacy functionality, you could use base64.encodebytes like you said or just alias hashutils.base64_b64encode and add a newline. I just wonder if we really need to preserve the legacy functionality.

[waynew]

I don't understand what you mean.

base64_b64* are wrappers around base64.b64(encode/decode)

base64.encodebytes does not produce the same output:

>>> import base64
>>> string = b'salt'*100
>>> print(base64.b64encode(string).decode().count('\n'))
0
>>> print(base64.encodebytes(string).decode().count('\n'))
8

What am I missing here?

[nicholasmhughes]

haha! my bad. I wasn't using a long enough test. *100 ftw.

[Ajnbro]

@waynew I apologize for the delay, I haven't been able to get to this until now. I will be fixing that issue later tonight most likely. Correct me if I am wrong here, but Python2 only has support for encodestring/decodestring and not for encodebytes/decodebyes. While Python3 has support for both (with encodestring/decodestring being depreciated). Is Python2 support anticipated to be dropped by the next version of Salt? If it isn't then should I just use both encodebytes/decodebytes and encodestring/decodestring in the module and utils files and ensure that the correct function is being used based on the Python version Salt supports at that moment? If it is going to be dropped by the next version then I can just set it up with only encodebytes/decodebytes for when it gets added to the develop branch.

Edit: Also I accidentally requested that review, sorry about that.

[waynew]

@waynew I apologize for the delay, I haven't been able to get to this until now.

No problem. Life gets busy for all of us :)

Python2 only has support for encodestring/decodestring and not for encodebytes/decodebyes. While Python3 has support for both (with encodestring/decodestring being depreciated). Is Python2 support anticipated to be dropped by the next version of Salt? If it isn't then should I just use both encodebytes/decodebytes

Right - with the Sodium release we are dropping Python2 support. That means that you can safely use encodebytes/decodebytes instead.

Edit: Also I accidentally requested that review, sorry about that.

Heh, no problem - Sometimes it's not entirely clear what does what in GitHub 😂

---

Summary: Yes, use encodebytes/decodebytes instead of ...string.

[Ajnbro]

@waynew Finished fixing that all up. Sorry about the multiple commit mess-ups, this is my first time ever submitting a PR :P

[waynew]

No problems 😀 We've all been there, I'm sure 🙃

Only one little tweak I think, and this should be good to go!

[waynew]

@Ajnbro looks great, thanks! We're working through some internal changes to exactly how we merge, so the actual merge will be coming soon(ish). Just translate that merge ready label to merged mentally if you need to 😃

[dwoz]

@Ajnbro We are no longer accepting PRs to the develop branch. This needs to be re-opened against the master branch and reference this PR so we have the conversation history. Sorry for the confusion.