feat(dotfiles): add ability to apply dotfiles as any user #133

phorcys420 · 2024-01-26T14:28:42Z

In my template (basic-env), dotfiles are also applied to root.
I like doing this because whenever switching to a root shell for more than one command (e.g installing packages, etc), my shell is consistent with my user.

I have added an optional user parameter that applies the dotfiles as a specific user.

I have added an optional dotfiles_uri parameter that, if passed, hides the coder_parameter that asks the end user for their dotfiles URI, this way you can "daisy chain" modules together.

Running dotfiles as root only

module "dotfiles" {
  source = "git::https://github.com/phorcys420/coder-modules.git//dotfiles?ref=dotfiles-root"
  agent_id = coder_agent.dev.id
  user = "root"
}

"Daisy chaining" to run as the default user (usually "coder") and as root without prompting the user twice.

module "dotfiles" {
  source = "git::https://github.com/phorcys420/coder-modules.git//dotfiles?ref=dotfiles-root"
  agent_id = coder_agent.dev.id
}

module "dotfiles-root" {
  source       = "git::https://github.com/phorcys420/coder-modules.git//dotfiles?ref=dotfiles-root"
  agent_id     = coder_agent.dev.id
  user         = "root"
  dotfiles_uri = module.dotfiles.dotfiles_uri
}

matifali

Hmm. Can you run bun fmt and bun lint to fix the formatting issues?

dotfiles/run.sh

phorcys420 · 2024-01-28T08:40:17Z

Hmm. Can you run bun fmt and bun lint to fix the formatting issues?

Yes, sorry, I'm not used to formatting every commit yet.

dotfiles/README.md

phorcys420 · 2024-01-30T12:02:35Z

I didn't mean to close, i'm starting off of a clean slate, which is probably what triggered it to close.

…edundancy

phorcys420 · 2024-01-30T12:40:53Z

hey @mafredri, while writing this PR, I realized that the script running the dotfiles is named "personalize", do you want me to rename it to "dotfiles" in this PR even though it's not really it's scope, or should I open another PR ? to fix it

resource "coder_script" "personalize" {
  agent_id = var.agent_id
  script = templatefile("${path.module}/run.sh", {
    DOTFILES_URI : local.dotfiles_uri,
  })
  display_name = "Dotfiles"
  icon         = "/icon/dotfiles.svg"
  run_on_start = true
}

mafredri · 2024-01-30T12:42:21Z

hey @mafredri, while writing this PR, I realized that the script running the dotfiles is named "personalize", do you want me to rename it to "dotfiles" in this PR even though it's not really it's scope, or should I open another PR ? to fix it
resource "coder_script" "personalize" {
  agent_id = var.agent_id
  script = templatefile("${path.module}/run.sh", {
    DOTFILES_URI : local.dotfiles_uri,
  })
  display_name = "Dotfiles"
  icon         = "/icon/dotfiles.svg"
  run_on_start = true
}

@phorcys420 feel free to do it here, no need for another PR IMO. Thanks for spotting.

dotfiles/main.tf

phorcys420 · 2024-01-30T13:37:34Z

sorry for all the noise, but given that terraform modules can only be loaded via an URL, I have to push debug commits and then force-push to clean my branch.

here is an example of what you can do with this PR :

module "dotfiles" {
  source = "git::https://github.com/phorcys420/coder-modules.git//dotfiles?ref=dotfiles-root"
  agent_id = coder_agent.dev.id
  #dotfiles_uri = "<defined by template admin>"
}

module "dotfiles-root" {
  source = "git::https://github.com/phorcys420/coder-modules.git//dotfiles?ref=dotfiles-root"
  agent_id = coder_agent.dev.id
  user = "root"
  dotfiles_uri = module.dotfiles.dotfiles_uri # "<defined by template admin>" if uncommented, or the value the end user provided in template settings
}

I have tested all the following scenarios and they are working :

single module without dotfiles_uri defined
- asks the user on template creation
- applies dotfiles to the default user (coder)
single module with dotfiles_uri defined
- doesn't ask the user on template creation
- applies dotfiles to the default user (coder)
single module without dotfiles_uri defined with user = "root"
- asks the user on template creation
- applies dotfiles to the root user
single module with dotfiles_uri defined with `user = "root"
- doesn't ask the user on template creation
- applies dotfiles to the root user
module without dotfiles_uri defined + module with dotfiles_uri = module.dotfiles.dotfiles_uri
- asks the user on template creation once and uses the same URI when applying both times
- applies dotfiles to the default user (coder)
module with dotfiles_uri = "<defined by template admin>" + module with dotfiles_uri = module.dotfiles.dotfiles_uri
- doesn't ask the user at all
- applies dotfiles to the default user (coder)

phorcys420 · 2024-01-30T13:42:27Z

@mafredri I would like your input on these things.

main.tf

variable "dotfiles_uri" {
  type        = string
  description = "The URL to a dotfiles repository. (optional)"

  default = null
}

variable "user" {
  type        = string
  description = "The name of the user to apply the dotfiles to. (optional, applies to the current user by default)"

  default = null
}

locals {
  dotfiles_uri = var.dotfiles_uri != null ? var.dotfiles_uri : data.coder_parameter.dotfiles_uri[0].value
  user         = var.user != null ? var.user : ""
}

Currently, var.user's default is null and I use a ternary operator to send an empty string ("") to templatefile() if it's null because templatefile() doesn't take null arguments.
I think this is ugly, but while I could set var.user's default to an empty string (""), I think it doesn't make sense that the default is not null if nothing was passed.

run.sh

if [ -n "$${DOTFILES_URI// }" ]; then
  if [ -z "$DOTFILES_USER" ]; then
    DOTFILES_USER="$USER"
  fi
fi

In this section, I use [ -z "$DOTFILES_USER" ] when I could theoretically use [ "$${DOTFILES_USER// }" ] to be more coherent, but I only got it working in my shell and never in the script and ran out of nerve, is it fine if it stays like this ?
Furthemore, I think that the following can look quite confusing :

if [ -n "$${DOTFILES_URI// }" ]; then
  if [ "$${DOTFILES_USER// }" ]; then
    DOTFILES_USER="$USER"
  fi
fi

dotfiles/run.sh

phorcys420 · 2024-04-14T12:43:59Z

hey!
sorry for letting this PR become stale, I went ahead and applied the changes you suggested.

matifali · 2024-04-14T14:07:26Z

@phorcys420 can you rebase it?

matifali

@mafredri, I approve it, but wait for you to merge in case you want a final review.

phorcys420 · 2024-04-14T14:20:08Z

hey @matifali, just resolved the merge conflict, there was an error in the docs where the variable name was default_dotfiles_repo instead of default_dotfiles_uri so I corrected that.

Also, does it make sense that there is a dotfiles_default_uri output? I don't see how this could be useful.

output "dotfiles_default_uri" {
  description = "Dotfiles Default URI"
  value       = var.default_dotfiles_uri
}

phorcys420 · 2024-04-14T14:20:55Z

Another thing, could you review the README.md to see if it makes sense to you?
I think the section about default dotfiles URI might be better somewhere else, but I don't really know.

matifali · 2024-04-14T14:22:09Z

@phorcys420 having an output doesn't do any harm but could be helpful for some users, so I did not remove it. I agree that to doesn't add much value as its not anything computed but a direct output from an input.

Let's remove that.

phorcys420 · 2024-04-14T14:22:51Z

@matifali do you want me to remove it in this PR?

dotfiles/main.tf

dotfiles/README.md

matifali

done

matifali · 2024-04-14T14:26:32Z

@phorcys420 We can do it in a separate PR and keep the changes related in this PR.

The new PR can fix the typo and remove output. I will do that

matifali · 2024-04-14T14:30:31Z

#225 does that but we need to wait for someone to approve that :D

phorcys420 · 2024-04-14T14:31:50Z

oh okay, but I already fixed the typo in this branch @matifali, do you want me to unfix it to keep the changes related to the PR? 😄

matifali · 2024-04-14T14:34:10Z

@phorcys420 Yes, its better to do ina separate PR.

phorcys420 · 2024-04-14T14:53:59Z

I have tested the module again after all the changes and I can confirm that it still works as expected.

matifali · 2024-04-17T08:04:11Z

@phorcys420 Could you rebase it again. :D

phorcys420 · 2024-04-17T08:07:50Z

@phorcys420 Could you rebase it again. :D

will do!

matifali · 2024-04-22T21:24:44Z

dotfiles/main.tf

 variable "default_dotfiles_uri" {
  type        = string
-  description = "The default dotfiles URI if the workspace user does not provide one."
+  description = "The default dotfiles URI if the workspace user does not provide one"
  default     = ""
 }

+variable "dotfiles_uri" {
+  type        = string
+  description = "The URL to a dotfiles repository. (optional, when set, the user isn't prompted for their dotfiles)"
+
+  default = null
+}


I think we should only have one of these

matifali reviewed Jan 27, 2024

View reviewed changes

dotfiles/run.sh Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

phorcys420 force-pushed the dotfiles-root branch from d720a16 to 72fcae9 Compare January 28, 2024 08:58

This comment was marked as outdated.

Sign in to view

phorcys420 force-pushed the dotfiles-root branch 2 times, most recently from a059f11 to ede72ca Compare January 28, 2024 09:22

phorcys420 changed the title ~~feat(dotfiles): add ability to run apply dotfiles as root~~ feat(dotfiles): add ability to apply dotfiles as root Jan 28, 2024

This comment was marked as outdated.

Sign in to view

matifali requested a review from mafredri January 28, 2024 20:09

mafredri reviewed Jan 29, 2024

View reviewed changes

dotfiles/README.md Outdated Show resolved Hide resolved

phorcys420 closed this Jan 30, 2024

phorcys420 force-pushed the dotfiles-root branch from ede72ca to 7d31865 Compare January 30, 2024 11:56

feat(dotfiles): move script to run.sh

d6b338c

phorcys420 reopened this Jan 30, 2024

phorcys420 added 2 commits January 30, 2024 12:23

feat(dotfiles): add ability to provide an arbitrary dotfiles_uri

873f057

fix(dotfiles): remove non-existant ROOT variable from run.sh

b8db92e

phorcys420 force-pushed the dotfiles-root branch from 5349aea to b8db92e Compare January 30, 2024 12:24

phorcys420 changed the title ~~feat(dotfiles): add ability to apply dotfiles as root~~ feat(dotfiles): add ability to apply dotfiles as any user Jan 30, 2024

refactor(dotfiles): move dotfiles_uri to local variables to avoid r…

8444934

…edundancy

matifali reviewed Jan 30, 2024

View reviewed changes

dotfiles/main.tf Outdated Show resolved Hide resolved

phorcys420 force-pushed the dotfiles-root branch from e4770c0 to 037b089 Compare January 30, 2024 13:17

feat(dotfiles): add ability to provide an arbitrary user

e653503

phorcys420 force-pushed the dotfiles-root branch from 31d8b0e to e653503 Compare January 30, 2024 13:30

fix(dotfiles): rename coder_script to "dotfiles" (from "personalize")

ae23a5f

mafredri reviewed Mar 18, 2024

View reviewed changes

dotfiles/run.sh Outdated Show resolved Hide resolved

chore(dotfiles): clarify the script for other users

6ffc79f

phorcys420 requested a review from mafredri April 14, 2024 12:42

phorcys420 added 2 commits April 14, 2024 14:44

Merge branch 'coder:main' into dotfiles-root

b9bec0f

fix(dotfiles): fix formatting

0b8fe33

Merge branch 'main' into dotfiles-root

f94d0b7

matifali approved these changes Apr 14, 2024

View reviewed changes

fix(dotfiles): format & lint

808ee2b

matifali reviewed Apr 14, 2024

View reviewed changes

dotfiles/main.tf Outdated Show resolved Hide resolved

matifali reviewed Apr 14, 2024

View reviewed changes

dotfiles/README.md Outdated Show resolved Hide resolved

matifali reviewed Apr 14, 2024

View reviewed changes

fix(dotfiles): fix typos

437ea98

This comment was marked as outdated.

Sign in to view

chore(dotfiles): unfix typo in README to stay in the scope of the PR

f92a612

Merge branch 'main' into dotfiles-root

9239a4c

matifali reviewed Apr 22, 2024

View reviewed changes

matifali merged commit 443485a into coder:main Apr 26, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dotfiles): add ability to apply dotfiles as any user #133

feat(dotfiles): add ability to apply dotfiles as any user #133

phorcys420 commented Jan 26, 2024 •

edited

Loading

matifali left a comment

phorcys420 commented Jan 28, 2024

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

phorcys420 commented Jan 30, 2024

phorcys420 commented Jan 30, 2024

mafredri commented Jan 30, 2024

phorcys420 commented Jan 30, 2024

phorcys420 commented Jan 30, 2024 •

edited

Loading

phorcys420 commented Apr 14, 2024

matifali commented Apr 14, 2024

matifali left a comment •

edited

Loading

phorcys420 commented Apr 14, 2024

phorcys420 commented Apr 14, 2024

matifali commented Apr 14, 2024

phorcys420 commented Apr 14, 2024

matifali left a comment

matifali commented Apr 14, 2024 •

edited

Loading

This comment was marked as outdated.

matifali commented Apr 14, 2024

phorcys420 commented Apr 14, 2024

matifali commented Apr 14, 2024

phorcys420 commented Apr 14, 2024

matifali commented Apr 17, 2024

phorcys420 commented Apr 17, 2024

matifali Apr 22, 2024

feat(dotfiles): add ability to apply dotfiles as any user #133

feat(dotfiles): add ability to apply dotfiles as any user #133

Conversation

phorcys420 commented Jan 26, 2024 • edited Loading

Running dotfiles as root only

"Daisy chaining" to run as the default user (usually "coder") and as root without prompting the user twice.

matifali left a comment

Choose a reason for hiding this comment

phorcys420 commented Jan 28, 2024

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

phorcys420 commented Jan 30, 2024

phorcys420 commented Jan 30, 2024

mafredri commented Jan 30, 2024

phorcys420 commented Jan 30, 2024

phorcys420 commented Jan 30, 2024 • edited Loading

phorcys420 commented Apr 14, 2024

matifali commented Apr 14, 2024

matifali left a comment • edited Loading

Choose a reason for hiding this comment

phorcys420 commented Apr 14, 2024

phorcys420 commented Apr 14, 2024

matifali commented Apr 14, 2024

phorcys420 commented Apr 14, 2024

matifali left a comment

Choose a reason for hiding this comment

matifali commented Apr 14, 2024 • edited Loading

This comment was marked as outdated.

matifali commented Apr 14, 2024

phorcys420 commented Apr 14, 2024

matifali commented Apr 14, 2024

phorcys420 commented Apr 14, 2024

matifali commented Apr 17, 2024

phorcys420 commented Apr 17, 2024

matifali Apr 22, 2024

Choose a reason for hiding this comment

phorcys420 commented Jan 26, 2024 •

edited

Loading

phorcys420 commented Jan 30, 2024 •

edited

Loading

matifali left a comment •

edited

Loading

matifali commented Apr 14, 2024 •

edited

Loading