Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add possibility to pass secrets via files #31

Merged
merged 10 commits into from
Oct 25, 2024

Conversation

britter
Copy link
Contributor

@britter britter commented Oct 4, 2024

Resolves #30

@britter
Copy link
Contributor Author

britter commented Oct 4, 2024

Tested this in my homelab and it looks like it's working.

Copy link
Owner

@cromefire cromefire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the docker samples in the readme should be updated accordingly to make use of secrets.

main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
@britter
Copy link
Contributor Author

britter commented Oct 4, 2024

@cromefire I addressed the two code comments.

Also the docker samples in the readme should be updated accordingly to make use of secrets.

I don't use docker that much so I don't exactly know how to pass secrets via files. Would you mind adding this yourself on top of my changes?

@cromefire
Copy link
Owner

Yeah was planning to, but I probably won't get to it today, I'll try and add it in the coming days, when I test it locally anyways.

@britter
Copy link
Contributor Author

britter commented Oct 11, 2024

Hey @cromefire did you have a chance to look into this yet? Would love to have a release with this included.

@cromefire
Copy link
Owner

No I didn't have any time to look at it or test it yet. Time's currently very scarce for me, I hope I do get to it in the next few days, but thanks for the reminder.

@britter
Copy link
Contributor Author

britter commented Oct 24, 2024

Hey @cromefire I've given it a shot and added an example for how to use the new feature with docker compose secrets. Let me know what you think!

Copy link
Owner

@cromefire cromefire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works in general, but I'd have 2 smaller requests in addition to the 2 comments (but those I'll simply review and don't need to retest, so I promise it'll be merged soon after and I'll trigger a release then!):

  • Please make a note on the 2 secret variables (in the variable list in the readme) that it's recommended to use files/secrets if possible
  • Please remove the secret variables from the docker files (or add the _FILE suffix)

Also if you fancy (I leave that up to you) maybe an info level log in the readSecret function in the case that the env is set, recommending to use files instead (maybe with a link to the readme secrets section: https://github.com/cromefire/fritzbox-cloudflare-dyndns?tab=readme-ov-file#passing-secrets)?

You can also put yourself into the CONTRIBUTORS.md if you want.

README.md Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
@cromefire cromefire merged commit 103005b into cromefire:main Oct 25, 2024
10 checks passed
@britter britter deleted the secrets-via-files branch October 25, 2024 12:16
@cromefire
Copy link
Owner

v1.3.0 has been created, binaries are attached and containers are built

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

Successfully merging this pull request may close these issues.

Feature request: Read secrets from files
2 participants