Skip to content
This repository has been archived by the owner on Aug 24, 2022. It is now read-only.

getZoneId is always using the ZONE environment variable, even when ZONE_FILE is set. #90

Open
devantler opened this issue Jan 6, 2022 · 4 comments

Comments

@devantler
Copy link

devantler commented Jan 6, 2022

I am currently setting up a repo for easily setting up a fully managed cluster with Portainer and Docker Swarm. Right now I am using this service to update DDNS on Cloudflare as it is very helpful in avoiding dependency on scripts or router-based DDNS updates.

Because my repo is planned to be public I want to ensure all sensitive information is added with Docker Secrets. Right now the ZONE_FILE and SUBDOMAIN_FILE are not working with Docker Secrets which requires me to write the values in the compose file using ZONE and SUBDOMAIN environment variables.

The API_KEY_FILE works just fine, so i think it is an easy fix :-)

@devantler
Copy link
Author

The fix should enable Docker Secret users to only have to add the ***_KEY_FILE variables as they overwrite the non KEY_FILE variables, and as such the non KEY_FILE variables are not required.

@devantler devantler changed the title ZONE_KEY and SUBDOMAIN_KEY not working ZONE_FILE and SUBDOMAIN_FILE not working Jan 7, 2022
@devantler devantler changed the title ZONE_FILE and SUBDOMAIN_FILE not working getZoneId is always using the ZONE environment variable, even when ZONE_FILE is set. Jan 7, 2022
@devantler
Copy link
Author

To fix this I believe getZoneId should have a parameter so that the correctly populated ZONE value in 30-cloudflare-setup.sh can be passed along, when ZONE is set from ZONE_FILE.

@djkenne
Copy link

djkenne commented Jan 31, 2022

Yes I found the same thing. API_KEY_FILE works just fine with a Docker secret via CLI or Compose while ZONE_FILE and SUBDOMAIN_FILE do not work. The README suggests that the latter two are supported while the docs on Docker Hub do not mention them.

Thanks for this project by the way - very handy!

@djkenne
Copy link

djkenne commented Jan 31, 2022

Looks like this was fixed and merged already: a96b637

Maybe just need to cut a release?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants