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

Explicitly set REALM when querying workgroup #2671 #2672

Conversation

FroggyFlox
Copy link
Member

Fixes #2671
@phillxnet, @Hooverdan96, ready for review.

During Active Directory activation, we query the Workgroup information from the AD server using net ads workgroup. This normally gets its parameters from smb.conf but we need here to explicitly give the required server information at the command call given we usually work with an empty smb.conf file at this stage.

This Pull request switches from using the -S|--server flag to the --realm flag as that has proven more robust.

Functional testing

Rockstor built from this branch on Leap 15.5 can successfully activate the Active Directory service configured to connect to a Samba 4-backed AD domain controller found at samdom.example.com.

Unit testing

buildvm155:/opt/rockstor # cd src/rockstor/ && poetry run django-admin test ; cd -
Creating test database for alias 'default'...
Creating test database for alias 'smart_manager'...
System check identified no issues (0 silenced).
......................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 262 tests in 13.149s

OK
Destroying test database for alias 'default'...
Destroying test database for alias 'smart_manager'...

During Active Directory activation, we query the Workgroup information
from the AD server using `net ads workgroup`. This normally gets its
parameters from `smb.conf` but we need here to explicitly give the
required server information at the command call given we usually work
with an empty `smb.conf` file at this stage.

This commit switches from using the `-S|--server` flag to the `--realm`
flag as that has proven more robust.
Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

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

@FroggyFlox This is a nice find/fix: much appreciated.

This is more of a cursory review I'm afraid as I don't have a setup to test this. But merging into relatively early testing to gain more 'proper' field tests.

Thanks for the continuing work re moving us over to using Type hints & non single character variable names, & our on-going test instructions/fstring updates: again much appreciated.

@phillxnet
Copy link
Member

We have a successful Tumbleweed build so tests look to be good there also:
pr2672-build-ok

@phillxnet phillxnet removed the needs review Ideally by prior rockstor-core contributor label Sep 15, 2023
@phillxnet phillxnet merged commit e822e65 into rockstor:testing Sep 15, 2023
@FroggyFlox FroggyFlox deleted the 2671-Failure-to-automatically-update-configure-Samba-workgroup-during-AD-service-activation branch October 20, 2023 20:30
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.

Failure to automatically update/configure Samba workgroup during AD service activation
2 participants