search

koenrh / dnscontrol-action

Deploy your DNS configuration using GitHub Actions using DNSControl.
ISC License
79 stars 37 forks source link

Added support for specifying a working directory. #37

Closed markdorison closed 3 years ago

markdorison commented 3 years ago
koenrh commented 3 years ago

@szczot3k Well spotted! I still like having the ability to explicitly specify paths to the 'config' and 'credentials' file.

Since the require functions specify paths relative to the 'config' file, we could figure out the 'config' file directory and change the working directory to that one. That way the require functions keep working and you could still specify the paths to the 'config' and 'credentials' file.

WORKING_DIR="${INPUT_DNSCONTROL_CONFIG_FILE%/*}"
cd "$WORKING_DIR"
markdorison commented 3 years ago

That way the require functions keep working and you could still specify the paths to the 'config' and 'credentials' file.

I pushed up a change along these lines but it is not yet working as expected. For example, a user specifies the full path to both the config file and the creds file and we then take the directory path to the config file and cd into it. We also need to pass the filename of the config file in case it is different from the default (the user probably expects this since they specified it). We are OK to this point (I think), but what if they have also specified a custom path to the creds file? We would have to determine if it is at the same path as the config file since if it is, we have already changed the directory to that path, and if it is at a different path we are in a different relative location then from where the user-specified. Am I over-thinking this?

szczot3k commented 3 years ago

Well, tbh I think that you're over-thinking it.

koenrh commented 3 years ago

My apologies for the delay!

These changes look good to me, but they didn't work for existing configurations (where the config and credentials arguments where not specified). As I couldn't figure out how to push to this branch, I created a new one: https://github.com/koenrh/dnscontrol-action/pull/41 (I cherry-picked your commits, @markdorison), including a few minor changes. I think this should do. Let me know what you think.

koenrh commented 3 years ago

Thank you for your help, @markdorison! (and also @szczot3k) I have merged https://github.com/koenrh/dnscontrol-action/pull/41, which include your changes.