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

Make warning about authorizedKeys+NixOps more nuanced #123642

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jleeuwes
Copy link

@jleeuwes jleeuwes commented May 19, 2021

This is a documentation-only change.

Motivation for this change

The warning about authorizedKeys in combination with NixOps is a bit too alarming. I'm using NixOps and provision my own keys, for which using authorizedKeys is just fine. Also, for non-root users there's also no problem.

Things done

Built the manual and visually checked that the change looked okay.

@stale
Copy link

stale bot commented Nov 16, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 16, 2021
@jleeuwes
Copy link
Author

What can I do to move this PR forward?

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 16, 2021
@roberth
Copy link
Member

roberth commented Nov 22, 2021

I don't see any obvious evidence that NixOps would even behave this way. I've checked 1.7 and master, but neither seem to use something like lib.mkDefault in their keys definition that would cause this problem.
When the priorities are the same (no mix of mkDefault and regular defs for example) the lists will be combined.

Of course, me reading code doesn't prove much. This needs testing.

@jleeuwes
Copy link
Author

@roberth would some manual testing suffice, or are you thinking of an automated test (maybe as part of this PR)? If the latter, could you maybe point me to some documentation about how testing is done in this project?

@roberth
Copy link
Member

roberth commented Feb 19, 2022

@jleeuwes sorry for the late response. NixOps testing is split across the nixops repo and its providers; and it could use some improvement. Manual testing is sufficient for this change.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 21, 2022
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 12, 2023
@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants