-
Notifications
You must be signed in to change notification settings - Fork 78
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 ansible config for dev desktops #79
Conversation
It probably makes sense to just inline all the files into simpleinfra rather than checking out a separate repository -- we'd want to move that into rust-lang at least, and it doesn't seem like there's much benefit to keeping it separate rather than integrated into this repository. |
Hmm, is there a reason why the tool to create the users is a full Rust program depending on reqwest? A simple bash script with |
It started out as a tool that was run on each login and that took the username as an argument. The first few bash iterations of this got "hacked" by ppl I showed it to, just by looking at it. So I made it a Rust program where I was able to reason about things. Even reading articles about making bash scripts secure, I was not certain I could create one. Later when the script became a cron job, this was much less important, the only insecure input is the json dump from github, and if that is taken over we have lost anyway. So... in essence, yes we could go back to a bash script using jq, it's just not straight forward to understand what is going on in it (to me, even though I'm the author). |
Pushed up a few commits with fixes/improvements while experimenting with the sample machine. One likely blocker I noted is removing the global Rust installation, which conflicts with usage of rustup (or at least rustup complains about it). |
12b5fbc
to
571521a
Compare
fdb9346
to
ca3710d
Compare
It looks like we're not finding the rustup install -- maybe an explicit PATH is needed or something, not sure. |
That did not seem to help; I'm still seeing the same error after pulling the latest changes. |
Looks like this needs curl installed externally. |
Couple notes:
|
Yea I was worried about that. The default sshd config disables motd and I'm not sure if reenabling it actually works in this setup. We could change the default sshd config, but that would change it for all servers.
Huh? I thought linux can overwrite binaries that are running and that there are no locks on them? I'll look into that. Maybe we just spawn a script that keeps trying to copy the new binary until that succeeds.
Launch the cron script once manually right after deploy? |
I do see some message of the day, I think, but seems like something to fix. I don't think the particular fix is too important -- if we need to enable messages of the day across all servers, that seems OK, for example.
I think we probably want to not run the drop groups task for this server, and potentially that means it needs a different common base (or parts of the common base need factoring out); it's our first server with a non-empty set of additional users that aren't just the infra admins and aren't created by Ansible. (An alternative approach could be to move the user creation out of the server cron job and into Ansible, and then add logic to e.g. rust-lang/sync-team to run Ansible to deploy changes across servers after team repo updates. But that is a larger change, for sure.) |
check if it works now. According to the update-motd docs there is a cronjob updating it every 10 minutes |
|
OK, that seemed to work in terms of deploying. I'm not yet seeing the message of the day this is trying to configure though:
|
No obvious effect. FWIW, it seems like a good idea to replace the default MOTD text with the one we are trying to provide, both to make it more prominent and since I think the default isn't particularly useful. |
No more motd showing at all. FWIW, you should have ssh access, so you can probably try to make it work on the machine directly and then reproduce whatever steps into Ansible. |
could you remove the IP address lockdown? That would make it easier and we'll soon have to do it anyway |
I don't think it's currently in place already. |
OK, that seems to have worked:
I think this is nearly ready to go then, probably we just need to button up the policy and confirm with Joel that this will work as the only/primary prompt users get. @pietroalbini might also want to take a look at the Terraform and Ansible configurations, but I figure in the alpha/beta period with just a few testers the current machine should likely be OK. |
I'll take a look in the coming days. |
We are getting an alert from our monitoring infra:
Can we disable that systemd service or otherwise make it not actually fail to run? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments.
In general, I'd still prefer for the sync script not to be in Rust but to be in either bash or even better python: it'd remove the need to manage the rustup installation on the root user, the problem with replacing binaries, and it'd make on the fly changes easier.
If you don't have the time to do it I can push a commit changing the script over to Python.
Also, how do we expect users to authenticate with GitHub to push code? I might've missed that discussion. |
Co-authored-by: Joel Marcey <joelmarcey@rustfoundation.org>
I didn't know why I made them that way after the weekend
Otherwise it clashes with the admin ssh group, which is reset on redeploy. Thus users would not have access for 5 mins after redeploy, as only the next cronjob run would give the perms back.
Merging this, we can iterate with other PRs. |
Discussion in https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/cloud.20dev.20desktops