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

[issue 105] - Adding the RHSSO Template provisioner #109

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

fabiobrz
Copy link
Member

@fabiobrz fabiobrz commented Nov 9, 2023

Description

Fix #105 - ported to support testing of RH-SSO via templates, which is still being used by internal pipelines.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • Pull Request contains a description of the changes
  • Pull Request does not include fixes for multiple issues/topics
  • Code is self-descriptive and/or documented
  • I have implemented unit tests to cover my changes
  • I tested my code in OpenShift

Copy link

openshift-ci bot commented Nov 9, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@fabiobrz
Copy link
Member Author

fabiobrz commented Nov 9, 2023

/test all

@fabiobrz fabiobrz changed the title [issue 105] - Addint the RHSSO Template provisioner [issue 105] - Adding the RHSSO Template provisioner Nov 9, 2023
@fabiobrz fabiobrz force-pushed the add-rhsso-template-prov branch from 721e4da to 8ce8913 Compare November 9, 2023 12:25
@fabiobrz
Copy link
Member Author

fabiobrz commented Nov 9, 2023

/test all

@fabiobrz fabiobrz force-pushed the add-rhsso-template-prov branch from 8ce8913 to fbfb0b6 Compare November 9, 2023 13:11
@fabiobrz
Copy link
Member Author

fabiobrz commented Nov 9, 2023

/test all

@fabiobrz fabiobrz marked this pull request as ready for review November 10, 2023 08:24
@fabiobrz fabiobrz closed this Nov 10, 2023
@fabiobrz fabiobrz reopened this Nov 10, 2023
@fabiobrz
Copy link
Member Author

/test prod-intersmash-e2e-prod

* @return Instance of {@link Path} representing a YAML definition for the desired realm configuration
*/
default Path getRealmConfigurationFilePath() {
return null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@fabiobrz importing a realm isn't needed, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

It depends, there are internal tests relying on this specific provisioner, hence I am going to check and get beck here, thanks for now!

Copy link
Member Author

Choose a reason for hiding this comment

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

Confirmed, it is used by existing EAP7 + RHSSO tests.

@@ -62,6 +62,7 @@ public KeycloakAdminClient(final String url, final String realm, final String us
*/
public void importRealmConfiguration(InputStream is) throws IOException {
PartialImportRepresentation piRep = JsonSerialization.readValue(is, PartialImportRepresentation.class);
// TODO - fix, the realm name should be passed in as a parameter
keycloak.realm("wildfly-realm").partialImport(piRep);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@fabiobrz isn't the realm name specified in the JSON file to import?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now that you've mentioned it, I see I have been maybe a bit too quick in here.
There's actually some controversial issue around the Keycloak/RH-SSO admin client (also about the compatible versions), and I am going to check it, thanks for bringing it up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Back with some updates. So the answer is "no", meaning that the Keycloak Admin API contracts require for the realm name, to identify the realm where resources will be created.
Here's an example from an existing test:

{
  "roles": {
    "realm" : [{
      "name": "user"
    }]
  },

  "users": [{
    "username" : "user",
    "enabled": true,
    "credentials" : [{
      "type" : "password",
      "value" : "password"
    }],
    "realmRoles": ["user"],
    "clientRoles": {
      "account": ["view-profile", "manage-account"]
    }
  }]
}

Anyway, I fixed the method as well, according to the TODO. Thanks again for bringing this up.

@tommaso-borgato
Copy link
Collaborator

@fabiobrz LGTM, I just left couple of questions...

@fabiobrz
Copy link
Member Author

@fabiobrz LGTM, I just left couple of questions...

Thanks @tommaso-borgato - I addressed your concerns, feel free to let me know in case you think something else is needed here.

@fabiobrz fabiobrz force-pushed the add-rhsso-template-prov branch from 6ab5971 to 93ede93 Compare November 16, 2023 15:57
@tommaso-borgato
Copy link
Collaborator

@fabiobrz please resolve the conflicts and then we can merge!

@fabiobrz fabiobrz force-pushed the add-rhsso-template-prov branch from 93ede93 to e88d246 Compare November 21, 2023 11:29
@fabiobrz
Copy link
Member Author

/retest

@tommaso-borgato tommaso-borgato merged commit 9dddd26 into Intersmash:main Nov 21, 2023
1 check passed
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.

[issue 105] - Add RHSSO 7.6.z Template based provisioning tooling
2 participants