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

Add test for keystone-openidc #925

Merged
merged 5 commits into from
Oct 5, 2022
Merged

Add test for keystone-openidc #925

merged 5 commits into from
Oct 5, 2022

Conversation

freyes
Copy link
Member

@freyes freyes commented Sep 22, 2022 •
edited

This patchset introduces testing for the keystone-openidc charm, the code does
the following:

  • Configure the oidc-client-id, oidc-client-secret and
    oidc-provider-metadata-url, this information is tightly related to the
    Identity Provider configured, which for testing purposes this is the
    openidc-test-fixture charm, the setup function
    zaza.openstack.charm_tests.openidc.setup.configure_keystone_openidc takes
    care of setting these values once the fixture charm is ready for service.
  • Create the OpenStack objects to correctly configure the federation, this is
    made by the setup function
    zaza.openstack.charm_tests.openidc.setup.keystone_federation_setup_site1
    which will create and configure the following resources:
    • Create a domain named 'federated_domain'.
    • Create a group named 'federated_users'.
    • Grant the 'Member' role to users in the 'federated_users' group.
    • Create an identity provider named 'openid'.
    • Create a mapping named 'openid_mapping'.
    • Create a federation protocol named 'openid' that relates the mapping
      and the identity provider.
  • Issue a token using a user backed by OpenID Connect
  • Create the networking objects in project defined by the mapping rules.
  • Launch an instance within the user's project.

Changes to infrastructure code:

  • Add support for v3.OidcPassword class in get_keystone_session()
  • Add argument keystone_session to launch_guest()

@freyes
Copy link
Member Author

freyes commented Sep 22, 2022 •
edited

do not merge until #898 gets merged and this patchset rebased on top of master. this PR introduces the changes that were on the other PR to keep things simple and easier to review

@freyes freyes force-pushed the test-keystone-openidc branch 7 times, most recently from d15f582 to 9e8cd9e Compare September 27, 2022 00:41
@freyes freyes mentioned this pull request Sep 27, 2022
Closed
@freyes
Copy link
Member Author

freyes commented Sep 27, 2022 

   File "/home/runner/work/zaza-openstack-tests/zaza-openstack-tests/.tox/py3.8/lib/python3.8/site-packages/OpenSSL/crypto.py", line 3224, in <module>
    utils.deprecated(
TypeError: deprecated() got an unexpected keyword argument 'name'

^ this is the error when running the tests

the deprecated decorator is provided by cryptography library, the name argument is available since 37.0.0 - pyca/cryptography@7274228

@freyes
Copy link
Member Author

freyes commented Sep 27, 2022

This commit fixes the CI failure - #930

@freyes freyes marked this pull request as ready for review September 27, 2022 01:18
@freyes
Copy link
Member Author

freyes commented Sep 27, 2022

This pull request is being exercised by this other patchset https://review.opendev.org/c/openstack/charm-keystone-openidc/+/858844

ajkavanagh
ajkavanagh reviewed Sep 28, 2022
View reviewed changes
Copy link
Contributor

@ajkavanagh ajkavanagh left a comment

Choose a reason for hiding this comment

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

Just a few minor nitpicks; otherwise looks great.

zaza/openstack/charm_tests/keystone_federation/utils.py
domain=domain,
enabled=True)

role = keystone_client.roles.find(name=role_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably should check that a role was returned and fail if not found?

Copy link
Member Author

Choose a reason for hiding this comment

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

ack, makes sense.

zaza/openstack/charm_tests/openidc/tests.py Outdated
self.assertIsNotNone(token)
logging.info('OK')

def test_10_network_configuration(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a set-up function? it "looks" like a test, but seems to just configure things?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, this doesn't look right. I will refactor this code to have a another class that interacts with the cloud, add this "bootstrap" code in the setupClass() of that new class.

zaza/openstack/charm_tests/test_utils.py Outdated
Comment on lines 776 to 777
:param keystone_session: Keystone session to use.
:type keystone_session: keystoneauth1.session.Session
Copy link
Contributor

Choose a reason for hiding this comment

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

The :type ..: is Optional[keystoneauth1.session.Session] isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

correct, will fix that.

zaza/openstack/configure/guest.py Outdated
@@ -120,10 +121,14 @@ def launch_instance(instance_key, use_boot_volume=False, vm_name=None,
:param attach_to_external_network: Attach instance directly to external
network.
:type attach_to_external_network: bool
:param keystone_session: Keystone session to use.
:type keystone_session: keystoneauth1.session.Session
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, isn't it Optional[...]?

Copy link
Member Author

Choose a reason for hiding this comment

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

correct.

@freyes
Add keystone-openidc setup code.
1feee16 
The keystone-openidc charm requires 2 configuration steps:

1) Configure the oidc-client-id, oidc-client-secret and
   oidc-provider-metadata-url, this information is tightly related to
   the Identity Provider configured, which for testing purposes this is
   the openidc-test-fixture charm, the setup function
   zaza.openstack.charm_tests.openidc.setup.configure_keystone_openidc
   takes care of setting these values once the fixture charm is ready
   for service.
2) Create the OpenStack objects to correctly configure the federation,
   this is made by the setup function
   zaza.openstack.charm_tests.openidc.setup.keystone_federation_setup_site1
   which will create and configure the following resources:
   - Create a domain named 'federated_domain'.
   - Create a group named 'federated_users'.
   - Grant the 'Member' role to users in the 'federated_users' group.
   - Create an identity provider named 'openid'.
   - Create a mapping named 'openid_mapping'.
   - Create a federation protocol named 'openid' that relates the mapping
     and the identity provider.
@freyes
Add support for v3oidcpassword auth plugin.
91edb71 
get_keystone_session() uses the v3.OidcPassword class when the
OS_AUTH_TYPE is set to v3oidcpassword, this class expects the following
extra configuration options:

- OS_IDENTITY_PROVIDER
- OS_PROTOCOL
- OS_CLIENT_ID
- OS_CLIENT_SECRET
- OS_ACCESS_TOKEN_ENDPOINT (optional)
- OS_DISCOVERY_ENDPOINT (optional)
@freyes
Add test for keystone-openidc
0911b43 
This patch introduces a new testing class named CharmKeystoneOpenIDCTest
which interacts with keystone using users provided by
openidc-test-fixture via OpenID Connect.
@freyes
Add keystone_session argument to launch instances.
9362a90 
Adding the option to pass a keystone session allows callers to use
credentials different from the ones provided by
get_overcloud_keystone_session(), this is helpful when testing non
default keystone configurations (e.g. Federation).
@freyes freyes force-pushed the test-keystone-openidc branch 3 times, most recently from fe7f676 to 8655aa1 Compare October 4, 2022 13:14
@freyes
Add zaza.openstack.charm_tests.openidc.tests.TestLaunchInstance
18909b9 
This testing class configures a private network in the user's project defined by the mapping
rules during the setUpClass stage. Specifically this test performs the following steps:

- Create keypair named 'zaza' in the user's project
- Create a router for the project
- Attach the router to the external network
- Create a network
- Create a subnet attached to the previously create network
- Connect the subnet to the project's router

The testing method launches an instance using a keystone session
associated with a user backed by OpenID Connect.
@freyes freyes requested a review from ajkavanagh October 5, 2022 12:08
ajkavanagh
ajkavanagh approved these changes Oct 5, 2022
View reviewed changes
Copy link
Contributor

@ajkavanagh ajkavanagh left a comment

Choose a reason for hiding this comment

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

LGTM

@ajkavanagh ajkavanagh merged commit a55f320 into openstack-charmers:master Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajkavanagh ajkavanagh ajkavanagh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@freyes @ajkavanagh