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

feat: allow reading secrets from files #20

Merged
merged 3 commits into from
Jan 18, 2025
Merged

Conversation

Gregofi
Copy link
Contributor

@Gregofi Gregofi commented Jan 17, 2025

Allows reading password from file whose path is in environment variable.

Comment on lines 21 to 24
type Secret struct {
value string
Value string `env:""`
FromFile string `env:"_FILE,file"`
}
Copy link
Contributor Author

@Gregofi Gregofi Jan 17, 2025

Choose a reason for hiding this comment

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

I tried to make this struct as reusable as possible. If another secret was added in the future, it can be reused without any hassle.

The reason for env:"" (and not env:"PASSWORD") is that it doesn't lock into naming (for example if authentication token was ever added, it can be named LOGIN_TOKEN, not something ending with PASSWORD).

The drawback of this solution is that the fields are now public. They were accessible before via the getter, but now one must be even more careful to not print them (for example by explicitly implementing serializers, as below).

@Gregofi Gregofi changed the title Dev feat: allow reading secrets from files Jan 17, 2025
@davidmasek davidmasek self-requested a review January 18, 2025 09:46
@davidmasek
Copy link
Owner

davidmasek commented Jan 18, 2025

hi, thanks for PR.

It is not passing the "Docker Image CI" test. Not sure if the problem is with the changes or the CI pipeline, I'll take a proper look later.

When logging the parsed output the "email" section is empty -
https://github.com/davidmasek/beacon/actions/runs/12837955985/job/35814332202?pr=20#step:3:340

email:
    smtp_server: ""
    smtp_port: 0
    smtp_username: ""
    smtp_password: '*****'
    send_to: ""
    sender: ""
    prefix: '[staging]'
    enabled: ""

@davidmasek davidmasek merged commit 3ad21d0 into davidmasek:main Jan 18, 2025
4 of 6 checks passed
@davidmasek
Copy link
Owner

The testing is currently broken when the PR originates from a fork. I will track the bug here - #22 .

Thanks for help, I'm merging this manually

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.

2 participants