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

PR - Ticket 49471 - Rename dscreate options #2864

Closed
389-ds-bot opened this issue Sep 13, 2020 · 13 comments
Closed

PR - Ticket 49471 - Rename dscreate options #2864

389-ds-bot opened this issue Sep 13, 2020 · 13 comments
Labels
merged Migration flag - PR pr Migration flag - PR

Comments

@389-ds-bot
Copy link

389-ds-bot commented Sep 13, 2020

Cloned from Pagure Pull-Request: https://pagure.io/389-ds-base/pull-request/49805


Description: Changed the names of the two positional arguemnts from
"fromfile" --> "install", and "example" --> "create-template"

          Added option for specifying template file instead
          of dumping the template to STDOUT

          Finally added autocomplete arg parsing to the cli tools,
          and used a unique file name in UI when creating template.

Resolves: #2850

Reviewed by: ?

@389-ds-bot 389-ds-bot added merged Migration flag - PR pr Migration flag - PR labels Sep 13, 2020
@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2018-06-25 21:35:57

I think setup_file should also have strict permissions like 600, since files in /tmp are world readable by default. Even though we delete a file later, there is a time interval when this file can be read.

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-25 22:02:09

rebased onto 664a06520e368ac83ea4910674268e0e0b55434e

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-25 22:03:40

@vashirov - changes made!

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2018-06-25 22:27:10

I still can read this file using while loop under regular user:

while : ; do ls -la /tmp/*inf 2> /dev/null; cat /tmp/*inf 2> /dev/null; done

If I run this loop on a terminal, and create an instance in webui, I can see contents of the file for some time.

We should create an empty file with strict permissions fist and only then write data there.

Also, it seems that the password is literally set to 'False' (line 855:

        setup_inf = setup_inf.replace('ROOTPW', 'False');

After I create an instance through webui, my password doesn't work, but 'False' works.

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-25 22:44:08

rebased onto bf6570d4dd5932f28adc54a26d88d63b16c1bb27

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-25 22:44:27

@vashirov - fixed!

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2018-06-25 23:20:54

Perhaps we can move chmod before writing any data here as well. The rest LGTM, ack.

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-25 23:37:09

But this just the example template. Nothing is sensitive in it, but I'll change it just in case.

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-25 23:40:12

rebased onto 749b9f3

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-25 23:41:08

Pull-Request has been merged by mreynolds389

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2018-06-30 03:31:17

The problem with this is that it may affect container builds. So the dockerfile needs an update.

As well, people may not want to write to a "specific" location, that's why I put it to stdotu, to let people choose what they wanted to do with it, pipe, edit, etc.

So I'm not really for this change, but I can't stop it either.

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-06-30 16:02:34

The problem with this is that it may affect container builds. So the dockerfile needs an update.

Doing that now...

As well, people may not want to write to a "specific" location, that's why I put it to stdotu, to let people choose what they wanted to do with it, pipe, edit, etc.

That still works, the output file is only an option

@389-ds-bot
Copy link
Author

Patch
49805.patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged Migration flag - PR pr Migration flag - PR
Projects
None yet
Development

No branches or pull requests

1 participant