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.

Sign up for GitHub

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

Jump to bottom

RHBK support #17338

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

RHBK support #17338

wants to merge 3 commits into from

Conversation

lhellebr
Copy link
Contributor

@lhellebr lhellebr commented Jan 15, 2025 •
edited
Loading

Make changes so RHSSO tests become parametrized and work for both RHSSO and RHBK, limit TemplateSync tests

Problem Statement

Solution

Related Issues

@lhellebr lhellebr added CherryPick PR needs CherryPick to previous branches 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 6.16.z Introduced in or relating directly to Satellite 6.16 labels Jan 15, 2025
@lhellebr
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_login_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_timeout_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_two_factor_login_using_hammer

@lhellebr
Copy link
Contributor Author

PRT running test_rhsso_login_using_hammer that is newly parametrized, RHSSO and RHBK version. Also running two unparametrized tests to make sure the fixtures are backwards compatible.

@lhellebr
RHBK support
4a7da48 
Make changes so RHSSO tests become parametrized and work for both RHSSO and RHBK
@lhellebr
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_login_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_timeout_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_two_factor_login_using_hammer

@lhellebr lhellebr marked this pull request as ready for review January 16, 2025 11:32
@lhellebr lhellebr requested review from a team as code owners January 16, 2025 11:32
@lhellebr
Copy link
Contributor Author

PRT waiting for config merge

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 9899
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_login_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_timeout_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_two_factor_login_using_hammer --external-logging
Test Result : ============ 1 passed, 466 warnings, 4 errors in 5207.71s (1:26:47) ============

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Jan 16, 2025
@ogajduse
Copy link
Member

trigger: test-robottelo
pytest: tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_login_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_timeout_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_two_factor_login_using_hammer

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 9953
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_login_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_timeout_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_two_factor_login_using_hammer --external-logging
Test Result : ================= 4 passed, 511 warnings in 4386.01s (1:13:06) =================

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Jan 22, 2025
ogajduse
ogajduse reviewed Jan 22, 2025
View reviewed changes
conf/rhbk.yaml.template Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add validators for this config?

pytest_fixtures/component/satellite_auth.py Show resolved Hide resolved
robottelo/hosts.py
@@ -2422,24 +2423,44 @@ def run_orphan_cleanup(self, smart_proxy_id=None):
class SSOHost(Host):
"""Class for RHSSO functions and setup"""

def __init__(self, sat_obj, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

Not well-thought-through, but would splitting SSOHost to RHSSOHost and RHBKHost make sense here?

graph TD;
    RHBKHost-->SSOHost;
    RHSSOHost-->SSOHost;
    SSOHost-->Host;
Loading

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Jan 28, 2025
@lhellebr lhellebr force-pushed the rhsso_to_rhbk branch 3 times, most recently from 8d9eb02 to bb9a399 Compare January 28, 2025 14:59
@lhellebr lhellebr requested a review from ogajduse January 28, 2025 15:40
@lhellebr
Copy link
Contributor Author

@ogajduse comments addressed

@ogajduse ogajduse requested a review from Copilot January 28, 2025 17:39
ogajduse
ogajduse reviewed Jan 28, 2025
View reviewed changes
Copy link
Member

@ogajduse ogajduse left a comment

Choose a reason for hiding this comment

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

Thank you for addressing my above review comments. I have left one minor comment.

robottelo/hosts.py Outdated
"""Class for RHBK functions and setup"""

def __init__(self, sat_obj, **kwargs):
self.rhbk = True
Copy link
Member

Choose a reason for hiding this comment

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

I am still not sure if this is a nitpick or not ... anyway...

I can not see the rhbk attribute being used anywhere. Is it useful in any way? If we would like to perform any sort of checks whther given SSO instance is RHBK or RHSSO, we could simply

my_sat = Satellite()
the_sso_host = RHBKHost(my_sat)
# or
the_sso_host = RHSSOHost(my_sat)

# check
is_rhbk = type(the_sso_host) == RHBKHost  # or we could use isinstance() ...

@lhellebr
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_login_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_timeout_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_two_factor_login_using_hammer tests/foreman/api/test_templatesync.py::TestTemplateSyncTestCase::test_positive_export_all_templates_to_repo tests/foreman/cli/test_templatesync.py::TestTemplateSyncTestCase::test_positive_import_dir_filtered tests/foreman/ui/test_templatesync.py::test_positive_import_templates

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 10012
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_login_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_timeout_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_two_factor_login_using_hammer tests/foreman/api/test_templatesync.py::TestTemplateSyncTestCase::test_positive_export_all_templates_to_repo tests/foreman/cli/test_templatesync.py::TestTemplateSyncTestCase::test_positive_import_dir_filtered tests/foreman/ui/test_templatesync.py::test_positive_import_templates --external-logging
Test Result : ================ 18 passed, 636 warnings in 5069.02s (1:24:29) =================

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Jan 29, 2025
@lhellebr
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_login_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_timeout_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_two_factor_login_using_hammer tests/foreman/api/test_templatesync.py::TestTemplateSyncTestCase::test_positive_export_all_templates_to_repo tests/foreman/cli/test_templatesync.py::TestTemplateSyncTestCase::test_positive_import_dir_filtered tests/foreman/ui/test_templatesync.py::test_positive_import_templates

@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Jan 31, 2025
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 10028
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_login_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_timeout_using_hammer tests/foreman/destructive/test_ldapauthsource.py::test_rhsso_two_factor_login_using_hammer tests/foreman/api/test_templatesync.py::TestTemplateSyncTestCase::test_positive_export_all_templates_to_repo tests/foreman/cli/test_templatesync.py::TestTemplateSyncTestCase::test_positive_import_dir_filtered tests/foreman/ui/test_templatesync.py::test_positive_import_templates --external-logging
Test Result : ======= 2 failed, 8 passed, 672 warnings, 8 errors in 5723.00s (1:35:22) =======

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogajduse ogajduse ogajduse left review comments

Copilot code review Copilot Awaiting requested review from Copilot

Assignees

@lhellebr lhellebr

Labels
6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 6.16.z Introduced in or relating directly to Satellite 6.16 CherryPick PR needs CherryPick to previous branches PRT-Failed Indicates that latest PRT run is failed for the PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lhellebr @Satellite-QE @ogajduse