Allow abra edit or similar to edit files within a container #213

New Issue

2021-10-26T20:34:54Z

cas commented

2021-10-26 20:34:54 +00:00

Describe the problem to be solved

It is a multi-step process to edit files within containers, which either involves abra cp to pull the file and edit it locally before pushing it back (which involves having to fix the permissions), or installing an editor in the container and editing it there.

Describe the solution you would like

abra edit would pull the file locally to a temporary file, launch $VISUAL or similar to edit the file, and if the file was changed, would push the file back to the container and ensure the permissions match the original state.

## Describe the problem to be solved It is a multi-step process to edit files within containers, which either involves `abra cp` to pull the file and edit it locally before pushing it back (which involves having to fix the permissions), or installing an editor in the container and editing it there. ## Describe the solution you would like `abra edit` would pull the file locally to a temporary file, launch $VISUAL or similar to edit the file, and if the file was changed, would push the file back to the container and ensure the permissions match the original state.

❤️ 2

cas added the enhancement abra labels 2021-10-26 20:34:54 +00:00

decentral1se commented

2021-10-27 07:01:11 +00:00

Nice one @cas! Could you not abra app run foo.com app vi myfile.txt? I guess the focus in this ticket is to get it back to your local for your own editor configs and such or? Can certainly see how this might be made more convenient.

Nice one @cas! Could you not `abra app run foo.com app vi myfile.txt`? I guess the focus in this ticket is to get it back to your local for your own editor configs and such or? Can certainly see how this might be made more convenient.

3wordchant commented

2021-10-27 09:27:02 +00:00

@decentral1se most containers don't include a text editor, so I guess we'd need

abra app run foo.com app sh -c "apt update && apt install vim-tiny && vi myfile.txt

Given we need this for Nextcloud and Federated Wiki SSO set-up (hand-editing config files YAY!) it seems worth having a convenience command to me 🤔

@decentral1se most containers don't include a text editor, so I guess we'd need ``` abra app run foo.com app sh -c "apt update && apt install vim-tiny && vi myfile.txt ``` Given we need this for Nextcloud and Federated Wiki SSO set-up (hand-editing config files YAY!) it seems worth having a convenience command to me 🤔

decentral1se commented

2021-10-27 09:40:50 +00:00

Right yep. How we can teach abra to understand the differences with permissions and package managers tho? E.g. on a keycloak, you can't even run an update/install with the container user permissions. In order to do that you have run app run --user root app mykeycloak app sh -c "microdnf update && microdnf install vim && vi myfile.txt" 🙈

Another approach, we do some sort of sshfs local mount of the file for local editing? Don't really wanna take on an additional dependency for just this but it might be doable without. (EDIT: did see https://github.com/nxsre/sshfs-go tho)

Right yep. How we can teach `abra` to understand the differences with permissions and package managers tho? E.g. on a keycloak, you can't even run an update/install with the container user permissions. In order to do that you have run `app run --user root app mykeycloak app sh -c "microdnf update && microdnf install vim && vi myfile.txt"` 🙈 Another approach, we do some sort of sshfs local mount of the file for local editing? Don't really wanna take on an additional dependency for just this but it might be doable without. (EDIT: did see https://github.com/nxsre/sshfs-go tho)

3wordchant commented

2021-10-27 11:01:44 +00:00

I think the original suggestion was abra app cp / local $VISUAL / abra app cp, to avoid that kind of mess. abra app edit-file foo.com app:/var/www/config/config.php would be an alias for

abra app cp foo.com app:/var/www/config/config.php /tmp/
$VISUAL /tmp/config.php (maybe using the same "what if you don't have an editor set" menu as abra app config?)
abra app cp foo.com /tmp/config.php app:/var/www/config/

And for incredible bonus points, saving and restoring the file attributes to avoid needing to chmod after.

SSHFS would be great usability but it would mean needing an SSH per service which seems slightly nightmarish, or implementing #10 which I still have no good ideas on.

I think the original suggestion was `abra app cp` / local `$VISUAL` / `abra app cp`, to avoid that kind of mess. `abra app edit-file foo.com app:/var/www/config/config.php` would be an alias for 1. `abra app cp foo.com app:/var/www/config/config.php /tmp/` 2. `$VISUAL /tmp/config.php` (maybe using the same "what if you don't have an editor set" menu as `abra app config`?) 3. `abra app cp foo.com /tmp/config.php app:/var/www/config/` And for incredible bonus points, saving and restoring the file attributes to avoid needing to chmod after. SSHFS would be great usability but it would mean needing an SSH per service which seems slightly nightmarish, or implementing #10 which I still have no good ideas on.

decentral1se commented

2021-10-27 11:51:11 +00:00

Ah riighhhttt, gotcha! 1. 2. 3. sounds v nice then, thanks for bringing me along 💙

I'll try to pick this one off shortly unless someone else wants to pick it up!

Ah riighhhttt, gotcha! 1. 2. 3. sounds v nice then, thanks for bringing me along 💙 I'll try to pick this one off shortly unless someone else wants to pick it up!

decentral1se added this to the (deleted) milestone 2021-10-27 11:51:39 +00:00

decentral1se added this to the Beta release (software) project 2021-10-27 11:51:43 +00:00

knoflook commented

2021-11-08 11:23:17 +00:00

I can take care of that if you haven't yet!

❤️ 1

decentral1se commented

2021-11-08 11:25:17 +00:00

Yes please 🙏

knoflook self-assigned this 2021-11-08 11:26:27 +00:00

knoflook added the due date 2021-11-14

2021-11-09 09:14:19 +00:00

knoflook commented

2021-11-11 16:05:43 +00:00

So far all abra data is stored in ~/.abra and i'd like to keep it that way. So I'd rather put the downloaded file in ~/.abra/.appname.filename or something simillar to avoid edge cases such as lack of /tmp directory or permission issues. What do you think about that?

3wordchant commented

2021-11-11 16:40:30 +00:00

So I'd rather put the downloaded file in ~/.abra/.appname.filename ... What do you think about that?

I think this sounds great!

> So I'd rather put the downloaded file in ~/.abra/.appname.filename ... What do you think about that? I think this sounds great!

❤️ 2

decentral1se commented

2021-11-11 17:06:24 +00:00

yeh nice! also ~/.abra/tmp might be a thing if other stuff needs it but idk that will happen :)

yeh nice! also `~/.abra/tmp` might be a thing if other stuff needs it but idk that will happen :)

knoflook commented

2021-11-15 14:26:06 +00:00

there is a permission problem. To reproduce:
deploy traefik
pull /entrypoint.sh: abra app cp -d traefik_your_domain app:/entrypoint.sh .

FATA[0001] lchown entrypoint.sh: operation not permitted

so the file gets copied but loses its permissions and date of creation/access etc:
inside docker container:

/ # stat /entrypoint.sh 
  File: /entrypoint.sh
  Size: 419       	Blocks: 8          IO Block: 4096   regular file
Device: 34h/52d	Inode: 153064      Links: 1
Access: (0775/-rwxrwxr-x)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2021-09-02 17:28:35.000000000 +0000
Modify: 2021-09-02 17:28:35.000000000 +0000
Change: 2021-11-13 17:28:58.111149343 +0000

local version:

user@host abra> stat entrypoint.sh 
  File: entrypoint.sh
  Size: 419       	Blocks: 8          IO Block: 4096   regular file
Device: fe03h/65027d	Inode: 24167334    Links: 1
Access: (0755/-rwxr-xr-x)  Uid: ( 1000/    user)   Gid: ( 1000/    user)
Access: 2021-11-15 15:10:52.044595896 +0100
Modify: 2021-11-15 15:10:52.044595896 +0100
Change: 2021-11-15 15:10:52.044595896 +0100
 Birth: 2021-11-15 15:10:52.044595896 +0100

Additionally the ACL's might also get lost if they exist. I don't have any image that supports them though.
possible solutions:

~~require root to pull files~~ jk
run stat -c "%u" "pulledfile"(for UID) and stat -c "%a" "pulledfile" (for access rights) on the file, save this information to ~/.abra/.filename.ownership and then run chmod and chown inside the docker container once it's there. Can do the same for dates and then use touch inside the image to change the dates.
pull the file without caring about permissions and then copy it into the container and run cat newfile > oldfile (sounds dodgy)

what do you think is best?

there is a permission problem. To reproduce: deploy traefik pull /entrypoint.sh: `abra app cp -d traefik_your_domain app:/entrypoint.sh .` ``` FATA[0001] lchown entrypoint.sh: operation not permitted ``` so the file gets copied but loses its permissions and date of creation/access etc: inside docker container: ``` / # stat /entrypoint.sh File: /entrypoint.sh Size: 419 Blocks: 8 IO Block: 4096 regular file Device: 34h/52d Inode: 153064 Links: 1 Access: (0775/-rwxrwxr-x) Uid: ( 0/ root) Gid: ( 0/ root) Access: 2021-09-02 17:28:35.000000000 +0000 Modify: 2021-09-02 17:28:35.000000000 +0000 Change: 2021-11-13 17:28:58.111149343 +0000 ``` local version: ``` user@host abra> stat entrypoint.sh File: entrypoint.sh Size: 419 Blocks: 8 IO Block: 4096 regular file Device: fe03h/65027d Inode: 24167334 Links: 1 Access: (0755/-rwxr-xr-x) Uid: ( 1000/ user) Gid: ( 1000/ user) Access: 2021-11-15 15:10:52.044595896 +0100 Modify: 2021-11-15 15:10:52.044595896 +0100 Change: 2021-11-15 15:10:52.044595896 +0100 Birth: 2021-11-15 15:10:52.044595896 +0100 ``` Additionally the ACL's might also get lost if they exist. I don't have any image that supports them though. possible solutions: - ~~require root to pull files~~ jk - run `stat -c "%u" "pulledfile"`(for UID) and `stat -c "%a" "pulledfile"` (for access rights) on the file, save this information to ~/.abra/.filename.ownership and then run chmod and chown inside the docker container once it's there. Can do the same for dates and then use `touch` inside the image to change the dates. - pull the file without caring about permissions and then copy it into the container and run `cat newfile > oldfile` (sounds dodgy) what do you think is best?

❤️ 1

3wordchant commented

2021-11-15 15:32:44 +00:00

Ah yes, well spotted and explained, thank you 🙏

My favourite option is the ~/.abra/.filename.ownership idea, I think.

Ah yes, well spotted and explained, thank you :pray: My favourite option is the `~/.abra/.filename.ownership` idea, I think.

🚀 1

knoflook referenced this issue from toolshed/abra

2021-11-15 15:49:32 +00:00

refactor: move file pulling/pushing logic to internal #133

knoflook commented

2021-11-15 19:35:16 +00:00

Some thoughts:
when the operation succeeds (the file is successfully pushed to the container), local version gets deleted and we're golden

BUT

when the operation fails (oopsie you tripped over your power cable while editing the file 👀)
~/.abra/.filename is not a good place to put those files because it will leave orphaned files in there. ~/.abra/tmp/edits would be better. Ideally we create two files every time you edit a file:

~/.abra/tmp/edits/appname/servicename/filename.file
~/.abra/tmp/edits/appname/servicename/filename.metadata

note that if the operation succeeds the whole edits directory gets removed (with an equivalent of cd ~/.abra/tmp; rmdir -p edits/appname/servicename) which means that if it exists, a previous operation was interrupted

filename.file contains the contents of the file and filename.metadata contains the following info:

full path
ownership
access rights
container ID: in case you pull a file for editing, something happens halfway trough, then you forget about it, redeploy the app and try editing again it will know that the app was redeployed. Is it an edge case? yes. We don't have to implement it now, but it's probably 5 lines (excluding the glorious if err != nil { logrus.Fatal(err)} of course)
ACL (if we ever have the need for that)

this approach makes it an atomic operation i think? as even when it's interrupted because of a failure it will be in a known state.
and then add --continue (default) and --redownload (maybe phrase it better) flags controlling the resolution of situation where the file already exists. Any improvement points?

Some thoughts: when the operation succeeds (the file is successfully pushed to the container), local version gets deleted and we're golden BUT when the operation fails (oopsie you tripped over your power cable while editing the file 👀) `~/.abra/.filename` is not a good place to put those files because it will leave orphaned files in there. `~/.abra/tmp/edits` would be better. Ideally we create two files every time you edit a file: - `~/.abra/tmp/edits/appname/servicename/filename.file` - `~/.abra/tmp/edits/appname/servicename/filename.metadata` note that if the operation succeeds the whole `edits` directory gets removed (with an equivalent of `cd ~/.abra/tmp; rmdir -p edits/appname/servicename`) which means that if it exists, a previous operation was interrupted `filename.file` contains the contents of the file and `filename.metadata` contains the following info: - full path - ownership - access rights - container ID: in case you pull a file for editing, something happens halfway trough, then you forget about it, redeploy the app and try editing again it will know that the app was redeployed. Is it an edge case? yes. We don't have to implement it now, but it's probably 5 lines (excluding the glorious `if err != nil { logrus.Fatal(err)}` of course) - ACL (if we ever have the need for that) this approach makes it an atomic operation i think? as even when it's interrupted because of a failure it will be in a known state. and then add `--continue` (default) and `--redownload` (maybe phrase it better) flags controlling the resolution of situation where the file already exists. Any improvement points?

knoflook modified the due date from 2021-11-14 to 2021-11-21

2021-11-15 19:42:32 +00:00

decentral1se commented

2021-11-18 18:39:27 +00:00

Wowza yeh, sick analysis! I'd be inclined to say that if such a convenience command requires such an inconvenient implementation (which will most likely result in bugs because yanno) then it is worth reconsidering if we wanna do this? Seems like getting into potential cases of data-loss would be no-bueno...

knoflook commented

2021-11-19 12:29:31 +00:00

there is no added data loss cases - after all you'll still interrupt the manual process by unplugging your pc halfway trough editing a file. The question is: how many hours do we want to throw at it?

👀 1

knoflook referenced this issue from a commit

2021-11-24 11:07:37 +00:00

WIP: not working!

knoflook modified the project from Beta release (software) to Spicing abra up

2021-11-24 11:07:59 +00:00

knoflook commented

2021-11-24 11:09:29 +00:00

This will require more work than expected because of how docker SDK for Go works. https://stackoverflow.com/questions/52774830/docker-exec-command-from-golang-api this is needed for preserving the correct access rights of file (running stat in the container). We'll take a look at this after beta release!

This will require more work than expected because of how docker SDK for Go works. https://stackoverflow.com/questions/52774830/docker-exec-command-from-golang-api this is needed for preserving the correct access rights of file (running `stat` in the container). We'll take a look at this after beta release!

knoflook removed the due date 2021-11-21

2021-11-24 11:09:36 +00:00

Sign in to join this conversation.

4 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: toolshed/organising#213

Allow abra edit or similar to edit files within a container #213

Describe the problem to be solved

Describe the solution you would like

Allow `abra edit` or similar to edit files within a container #213