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

Add option to generate the demo users #2495

Merged
merged 4 commits into from
Sep 16, 2021
Merged

Add option to generate the demo users #2495

merged 4 commits into from
Sep 16, 2021

Conversation

jvillafanez
Copy link
Member

@jvillafanez jvillafanez commented Sep 14, 2021

Description

Include an option to generate the demo users and groups. Only a minimal set of users and groups will be generated automatically unless the new option is enabled.

So far the minimal set includes:

  • for users -> the reva IOP, the Kopano IDP, and the admin user. (passwords remain unchanged)
  • for groups -> the sysusers group, and the users group.

Related Issue

#2484

Motivation and Context

How Has This Been Tested?

  • Start a fresh installation with the new option activated -> the demo users will be generated as it has been happening until now.
  • Start a fresh installation without the new option -> only a minimal set of users and groups will be generated.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@update-docs
Copy link

update-docs bot commented Sep 14, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@wkloucek wkloucek requested review from refs and C0rby and removed request for lookacat, kulmann and pascalwengerter September 14, 2021 07:59
@wkloucek
Copy link
Contributor

@jvillafanez I'm not sure if we should stick with the default to create demo users as long as we are in the tech preview stage.
We also should mention this new config option in the docs here https://owncloud.dev/ocis/deployment/#secure-an-ocis-instance

@jvillafanez
Copy link
Member Author

I agree. We can merge this (it seems there are tests to adjust) once we get close to the actual product release

@wkloucek
Copy link
Contributor

I agree. We can merge this (it seems there are tests to adjust) once we get close to the actual product release

No, I would love to have this right now. I just wanted to have the demo users as a default as of now ;-) So basically an opt-out instead of an opt-in to demo users...

@jvillafanez
Copy link
Member Author

From a product perspective, I think this option is better. Usually, you don't want to create demo users that you need to delete afterwards. This is why we don't want to create the demo users by default.
The demo users are a convenience now during the tech preview, but later they'll be in the way.

I'd vote for keeping this behavior.

@C0rby
Copy link
Contributor

C0rby commented Sep 15, 2021

From a product perspective, I think this option is better. Usually, you don't want to create demo users that you need to delete afterwards.

That is true but currently we are still heavily in dev mode and are using the demo users during development. I think it would make developing a little bit more annoying if we have to explicitly generate the accounts every time we delete the ocis storage.

So I'm agreeing with @wkloucek here. I'd like to have an opt-out option for now and later on when we have more "production" deployments we can always (and should) switch to opt-in.

We could then also move this code to a command in the accounts service like ./ocis accounts create-demo-users or something like that.

@jvillafanez
Copy link
Member Author

Turns out it's easier than initially thought... The default value is set to true, and we only need to change it to false (or remove it) when we no longer want to create the demo users by default. In any case, the environment variable will be respected if present.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell B 4 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@jvillafanez
Copy link
Member Author

I think this is ready to review and merge.

Copy link
Contributor

@C0rby C0rby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@C0rby C0rby merged commit e2306ad into master Sep 16, 2021
@delete-merged-branch delete-merged-branch bot deleted the skip_demo_users branch September 16, 2021 11:46
ownclouders pushed a commit that referenced this pull request Sep 16, 2021
Merge: 6fcae4d a680fc1
Author: David Christofas <[email protected]>
Date:   Thu Sep 16 07:46:47 2021 -0400

    Merge pull request #2495 from owncloud/skip_demo_users

    Add option to generate the demo users
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants