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

Create admin user #85

Merged
merged 1 commit into from
Apr 3, 2020
Merged

Create admin user #85

merged 1 commit into from
Apr 3, 2020

Conversation

sjha4
Copy link
Contributor

@sjha4 sjha4 commented Apr 2, 2020

We need to create the default admin user since pulpcore.app.authentication.PulpNoCreateRemoteUserBackend authentication backend does not create it automatically for us.

manifests/database.pp Outdated Show resolved Hide resolved
@sjha4 sjha4 force-pushed the admin_user branch 2 times, most recently from 70a3304 to 1501def Compare April 2, 2020 22:09
Copy link
Collaborator

@wbclark wbclark left a comment

Choose a reason for hiding this comment

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

If you run the tests locally rather than in the CI env (where selinux is disabled), the idempotence test will fail, but it's an unrelated issue. When I apply the fix to that other issue locally in my test env, then all tests are passing.

I'm also trying to validate that the admin user/pass was created, but when I try:

[vagrant@centos8-64-1 ~]$ DJANGO_SETTINGS_MODULE=pulpcore.app.settings python3-django-admin dumpdata | grep "auth.user"
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/pulpcore/app/settings.py", line 242, in <module>
    CONTENT_ORIGIN
NameError: name 'CONTENT_ORIGIN' is not defined

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/bin/python3-django-admin", line 11, in <module>
    load_entry_point('Django==2.2.11', 'console_scripts', 'django-admin')()
  File "/usr/lib/python3.6/site-packages/django/core/management/__init__.py", line 381, in execute_from_command_line
    utility.execute()
  File "/usr/lib/python3.6/site-packages/django/core/management/__init__.py", line 357, in execute
    django.setup()
  File "/usr/lib/python3.6/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/usr/lib/python3.6/site-packages/django/apps/registry.py", line 122, in populate
    app_config.ready()
  File "/usr/lib/python3.6/site-packages/pulpcore/app/apps.py", line 75, in ready
    self.import_viewsets()
  File "/usr/lib/python3.6/site-packages/pulpcore/app/apps.py", line 102, in import_viewsets
    from pulpcore.app.viewsets import NamedModelViewSet
  File "/usr/lib/python3.6/site-packages/pulpcore/app/viewsets/__init__.py", line 1, in <module>
    from .base import (  # noqa
  File "/usr/lib/python3.6/site-packages/pulpcore/app/viewsets/base.py", line 15, in <module>
    from pulpcore.app import tasks
  File "/usr/lib/python3.6/site-packages/pulpcore/app/tasks/__init__.py", line 1, in <module>
    from pulpcore.app.tasks import base, repository, upload  # noqa
  File "/usr/lib/python3.6/site-packages/pulpcore/app/tasks/repository.py", line 6, in <module>
    from pulpcore.app import models, serializers
  File "/usr/lib/python3.6/site-packages/pulpcore/app/serializers/__init__.py", line 42, in <module>
    from .repository import (  # noqa
  File "/usr/lib/python3.6/site-packages/pulpcore/app/serializers/repository.py", line 8, in <module>
    from pulpcore.app import models, settings
  File "/usr/lib/python3.6/site-packages/pulpcore/app/settings.py", line 244, in <module>
    raise ImproperlyConfigured(_('You must specify the CONTENT_ORIGIN setting.'))
django.core.exceptions.ImproperlyConfigured: You must specify the CONTENT_ORIGIN setting.

Copy link
Collaborator

@wbclark wbclark left a comment

Choose a reason for hiding this comment

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

@sjha4 could you add a test for this as well?

@sjha4
Copy link
Contributor Author

sjha4 commented Apr 3, 2020

@wbclark : Can you try this:
sudo -u pulp DJANGO_SETTINGS_MODULE=pulpcore.app.settings python3-django-admin dumpdata | grep "auth.user"

Added a test. Let me know if that is logical. I repeated the migrate --no-input test for this.

@wbclark
Copy link
Collaborator

wbclark commented Apr 3, 2020

@sjha4 same output when I try sudo as pulp user.

I checked settings.py:

[vagrant@centos8-64-1 ~]$ sudo fgrep CONTENT_ORIGIN /etc/pulp/settings.py 
CONTENT_ORIGIN = "http://centos8-64-1"

From the documentation[1] added in the same commit that introduced CONTENT_ORIGIN setting into Pulpcore, it looks like we may need the port defined as well.

[1] pulp/pulpcore@e027cd3#diff-572a386e0821364b3217fc84b059de24R212-R220

But I tried adding the port here on a separate branch and still faced the same issue. I also tried other python3-django-admin commands and saw the same stack trace, so I don't think this PR caused it but I'm not confident that unless => 'python3-django-admin dumpdata | grep "auth.user"' will work properly until we fix it.

@@ -50,6 +50,29 @@
.with_unless('/usr/bin/false')
end
end

context 'default parameters' do
Copy link
Collaborator

Choose a reason for hiding this comment

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

spec/defines/admin_spec.rb is used to test that the pulpcore::admin class compiles properly under various params.

In all the examples here, we're testing pulpcore::admin { 'help' } as you can see when we defined the title for the class here: https://github.com/theforeman/puppet-pulpcore/blob/master/spec/defines/admin_spec.rb#L7 , basically using help as a dummy admin command we might to run, and then testing that it behaves properly with various combinations of unless, refreshonly, etc.

So probably this isn't the right test to use for our purposes. I'll suggest alternatives in another comment.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In spec/acceptance/basic_spec.rb I think we could test that the admin user got created after running puppet-apply, using the same logic to test as in your unless condition, adding after L55:

describe command("DJANGO_SETTINGS_MODULE=pulpcore.app.settings python3-django-admin dumpdata | grep "auth.user"") do
  its(:exit_status) { is_expected.to eq 0 }

We have to fix the issue NameError: name 'CONTENT_ORIGIN' is not defined that we saw previously before this will work.

Copy link
Member

Choose a reason for hiding this comment

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

As @wbclark said, this probably belongs in spec/classes/pulpcore_spec.rb

@sjha4
Copy link
Contributor Author

sjha4 commented Apr 3, 2020

same output when I try sudo as pulp user.

sudo -u pulp DJANGO_SETTINGS_MODULE=pulpcore.app.settings PULP_SETTINGS=/etc/pulp/settings.py python3-django-admin dumpdata | grep "auth.user"

@sjha4 sjha4 requested a review from wbclark April 3, 2020 14:55
@wbclark
Copy link
Collaborator

wbclark commented Apr 3, 2020

same output when I try sudo as pulp user.

sudo -u pulp DJANGO_SETTINGS_MODULE=pulpcore.app.settings PULP_SETTINGS=/etc/pulp/settings.py python3-django-admin dumpdata | grep "auth.user"

Great, that worked for me! Thanks @sjha4

One more request, could you add a unit test in spec/classes/pulpcore_spec.rb, adding after L18:

is_expected.to contain_pulpcore__admin('reset-admin-password --random')

I think at that point we are good to ACK and merge this :)

Copy link
Collaborator

@wbclark wbclark left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @sjha4 !

manifests/database.pp Outdated Show resolved Hide resolved
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Overall 👍. Small comment on the test and I'll let you have a look at whether you want to address that.

spec/acceptance/basic_spec.rb Outdated Show resolved Hide resolved
@wbclark wbclark merged commit 934ab67 into theforeman:master Apr 3, 2020
@wbclark wbclark added the Enhancement New feature or request label May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants