lgtm

This is supposed to be user-configurable, so this also needs to be placed in conf/taskotron.yaml.example, with a short description.

I'm confused. This only exposes it in our runner.yml. But how does a task know which file to load the secrets from? The variable needs to get forwarded to the task, right? In that case it needs to be added to FORWARDED_VARS (and also it needs to get documented in writingtasks.rst, see docs). Or what am I missing?

Why do you close the file descriptor and then open it again?

You can use file_utils._get_session() to get a session that automatically retries failed requests for 5xx errors and connection timeouts. Would it make sense to use it here as well? I've spent some time lately trying to get to code more robust against connection issues, and this is another connection we'll be making for each task.

Long story short - python 3. The file descriptor provided by mkstemp only accepts bytes, json.dumps returns str. I'm not willing to go into encode hell here, when "regular opened file" can work just fine.
I'm closing the 'rogue' fd just because I think it's polite, would you rather have it hanging in there?

If there's a connection problem, we'll not know about it, just the task will fail with missing credentials or something. Is there a reason for at least not logging the error?

mea culpa, for some reason I though that all the vars_ get forwarded

I actually added it, but forgot to git add the file thx for the catch.

I see no "decryption" happening, it's just storing retrieved data, right? I have to add I haven't read through vault code yet, so I might be missing something.

cool, why not

I don't really understand the point of the question..

This took me a minute to read :) Could you perhaps move this line into the if r and r.ok block, so that's it's together with the rest of the code parsing the vault data, and then add a short and simple example of how the input data looks? It would make understanding the code much easier. You could also show a short example of what the transformed output (written into a file) looks like - totally optional! Thanks!

This will not be executed if the break command a few lines above is hit. Is that expected?

If I understood the code correctly, the secrets file is always created, and therefore is should always be possible to delete it. Right? In that case, let's put a warning here, because something has clearly gone wrong if we can't delete that file (and add the reason, so include the OSError instance in the error message). Thanks.

Learn regexps, I guess? There are far more complicated regexps written by you in the code, without "explanations" :)

Moving it in to the next block makes sense, why not.

you have some good points!

Yes, this honestly only is a best-effort. I'm open to better solutions, though.

np

OK, a comment might prevent future editors from "optimizing" this the wrong way :)

Well, if I see what an input should look like, then it's much easier to read it and verify it's actually correct. I looked at vault project and there's no documentation how the return data should look like. Finally I found a sample in the unit tests. Since it's about 3 lines, I believe it would be of great help if it got included in a comment here as well.

Simply put this block above the if self.task_vm is not None block and we should be good.

On the other hand, the regexp can be simplified, so I give you that. Have a look at the new version, if that's still hard to understand, we can have a discussion about it :)

The code looks fine, ack by me. I haven't tested the new functionality, but running just a generic task-rpmlint works.

THX. I tested it with a task that uses the secrets, and it works as expected. Will merge shortly

Merged via https://pagure.io/taskotron/libtaskotron/c/4f3d0d0b3be6f065cb5a578070220fa5f4a212f5?branch=develop