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(modules/home-manager): add support for configuring environment variables #615

Conversation

marksisson
Copy link
Contributor

Add support for configuring environment variables before calling sops-install-secrets. Introduce a new environment option which allows specifying environment variables. Modify systemd service and launchd agent to use the specified environment variables.

Environment variables to set before calling sops-install-secrets.

The values are placed in single quotes and not escaped any further to
allow usage of command substitutions for more flexibility. To properly quote
Copy link
Owner

Choose a reason for hiding this comment

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

Is this actually true? I don't think systemd will do command substitution.

Copy link
Owner

Choose a reason for hiding this comment

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

I don't now about launchd, but it also looks to me that it should be the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That may not be true. I copied the description for the environment option from the NixOS module at modules/sops/default.nix.

I also have only tested the launchd code path. It does work on macOS. I do not have access to a NixOS linux system. Hopefully someone can test the systemd code path.

@marksisson marksisson force-pushed the patches/add-environment-option-to-home-manager-module branch from 31b34ca to c8c892a Compare September 7, 2024 16:00
@Mic92
Copy link
Owner

Mic92 commented Sep 27, 2024

@SebTM could you test linux?

@SebTM
Copy link
Contributor

SebTM commented Oct 21, 2024

Hey, sorry for the delay - I can test it but would appreciate a test-case/instructions?

@Mic92
Copy link
Owner

Mic92 commented Oct 22, 2024

@SebTM for now it's enough if you just make sure it doesn't break your existing usage.

Copy link
Contributor

@SebTM SebTM left a comment

Choose a reason for hiding this comment

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

I've used this rebased on master and rebuild my workstation with it and can't spot any issues 🙏🏻

@Mic92
Copy link
Owner

Mic92 commented Oct 23, 2024

@mergify queue

Copy link
Contributor

mergify bot commented Oct 23, 2024

queue

🛑 The pull request has been removed from the queue default

The pull request can't be updated.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@Mic92
Copy link
Owner

Mic92 commented Oct 23, 2024

@mergify rebase

Added support for configuring environment variables before calling
`sops-install-secrets`. Introduced a new `environment` option which
allows specifying environment variables. Modified systemd service
and launchd agent to use the specified environment variables.
Copy link
Contributor

mergify bot commented Oct 23, 2024

rebase

✅ Branch has been successfully rebased

@Mic92 Mic92 force-pushed the patches/add-environment-option-to-home-manager-module branch from c8c892a to 7a8b208 Compare October 23, 2024 13:34
@Mic92
Copy link
Owner

Mic92 commented Oct 23, 2024

@mergify queue

Copy link
Contributor

mergify bot commented Oct 23, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at d089e74

@mergify mergify bot merged commit d089e74 into Mic92:master Oct 23, 2024
6 checks passed
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.

3 participants