feat: run testcontainer #128
Conversation
WalkthroughThis pull request updates several configurations and tests. In the GitHub Actions workflow, the Gradle build command now runs tests by removing the skip option. In enterprise tests, the Testcontainers configuration for the idP container has been modified and API endpoint URLs have been updated. The Docker Compose file renames the service and container. Multiple changes in the Keycloak test container/provider area include an updated LABEL constant, modifications to the Dockerfile (with JSON and JAR copy actions plus a base image update), introduction of a Podman manifest build script, and an enhanced JSON configuration with new roles and users. One redundant Dockerfile has been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions
participant Gradle as Gradle Build
CI->>Gradle: Execute "./gradlew build --scan"
Gradle->>Gradle: Run build + execute tests
Gradle-->>CI: Return build results
sequenceDiagram
participant Dev as Developer
participant BS as Build Script
participant Pod as Podman
Dev->>BS: Trigger Keycloak image build
BS->>Pod: podman manifest rm (remove existing manifest)
BS->>Pod: podman manifest create (create new manifest)
BS->>Pod: podman build (build image for linux/amd64 & linux/arm64)
BS->>Pod: podman manifest add (attach built images)
BS->>Pod: podman manifest push (publish manifest)
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
sep490-infrastructure/keycloak-testcontainer/greenbuildings.json (1)
47-82: New Roles Configuration Added:
A new"roles"section has been introduced that defines the realm roles for greenbuildings, including entries for"uma_authorization","default-roles-greenbuildings", and"offline_access". The composite role ("default-roles-greenbuildings") correctly aggregates roles via its"composites"field.It is recommended that you verify that all role IDs, names, and descriptions (using placeholders like
${role_uma_authorization}and${role_default-roles}) are consistent with downstream configurations and documentation.sep490-infrastructure/keycloak-testcontainer/build.sh (1)
1-10: Consider adding logging and error messages.The script would benefit from progress logging and error messages.
Here's an improved version:
#!/bin/bash set -euo pipefail +echo "Removing existing manifest..." podman manifest rm thongdh3401/keycloak:24.0.5 || true +echo "Initializing manifest..." # First, initialise the manifest podman manifest create thongdh3401/keycloak:24.0.5 +echo "Building multi-arch image..." # Build the image attaching them to the manifest podman build --platform linux/amd64,linux/arm64 --manifest thongdh3401/keycloak:24.0.5 . +echo "Publishing manifest..." # Finally publish the manifest podman manifest push thongdh3401/keycloak:24.0.5 + +echo "Build completed successfully!"🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/pull-request.yml(1 hunks)sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java(0 hunks)sep490-enterprise/src/test/java/enterprise/rest/BuildingControllerTest.java(5 hunks)sep490-infrastructure/docker-compose.yml(1 hunks)sep490-infrastructure/keycloak-testcontainer-provider/src/main/java/keycloak/provider/Sep490IdentityProviderTokenMapper.java(2 hunks)sep490-infrastructure/keycloak-testcontainer/Dockerfile(1 hunks)sep490-infrastructure/keycloak-testcontainer/build.sh(1 hunks)sep490-infrastructure/keycloak-testcontainer/greenbuildings.json(8 hunks)
💤 Files with no reviewable changes (1)
- sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java
✅ Files skipped from review due to trivial changes (2)
- sep490-infrastructure/docker-compose.yml
- sep490-infrastructure/keycloak-testcontainer-provider/src/main/java/keycloak/provider/Sep490IdentityProviderTokenMapper.java
🧰 Additional context used
🪛 Shellcheck (0.10.0)
sep490-infrastructure/keycloak-testcontainer/build.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Java Modules (sep490-enterprise)
- GitHub Check: Build Java Modules (sep490-idp)
🔇 Additional comments (8)
sep490-infrastructure/keycloak-testcontainer/greenbuildings.json (5)
381-465: New Users Configuration Added:
A new"users"section is added, defining multiple user objects along with their credentials. The structure is clear and detailed.
[security_advice]
Please double-check that the credential data (including secrets and hashing parameters) is appropriate for a test/integration environment, and ensure that sensitive credentials are not inadvertently exposed in production environments.
1475-1483: Review Allowed Protocol Mapper Types (Authenticated Block):
Within the components section, the configuration for"allowed-protocol-mapper-types"has been updated. Notice the changes indicated by the tildes:
- Line 1477 shows
"saml-role-list-mapper"added.- Line 1480 now includes
"oidc-full-name-mapper".- Line 1482 shows
"saml-user-property-mapper"with a change marker.Ensure these modifications correspond with the intended security and compliance policies and that the ordering and inclusion of these mapper types meet your system’s requirements.
1509-1516: Review Allowed Protocol Mapper Types (Anonymous Block):
In a separate anonymous configuration block for allowed protocol mapper types, changes are also noted:
- Line 1509 shows
"saml-role-list-mapper"marked as changed.- Line 1515 and 1516 indicate
"saml-user-attribute-mapper"and"oidc-full-name-mapper"with change markers.Please confirm that the set and ordering of protocol mapper types here remain consistent with those used in other contexts, ensuring their uniform application across client configurations.
1553-1558: HMAC Key Provider Configuration for HS512:
The configuration for the"hmac-generated-hs512"provider includes a key identifier and secret for HS512:
- Lines 1553–1558 show the changes to
"kid"and"secret".Double-check that these credentials comply with your security guidelines and that their exposure aligns with a test scenario rather than production use.
1573-1579: RSA-Generated Key Provider Configuration:
A new RSA key provider section for"rsa-generated"has been added, with details for private key and certificate:
- Lines 1573–1579 display these new values.
[security_advice]
Please verify that the RSA key and certificate are correct and meet your cryptographic and security standards. Also, confirm that these values are managed safely, particularly if they may be deployed beyond test environments.sep490-enterprise/src/test/java/enterprise/rest/BuildingControllerTest.java (1)
33-33: LGTM! Consistent API endpoint updates.The endpoint paths have been consistently updated to use the
/apiprefix across all test methods, maintaining a uniform API structure.Also applies to: 45-45, 62-62, 74-74, 86-86
sep490-infrastructure/keycloak-testcontainer/Dockerfile (1)
4-7:✅ Verification successful
Verify Keycloak import command.
The import command looks correct, but please verify:
- The import command's syntax for Keycloak 24.0.5
- That the JSON file is properly formatted for Keycloak import
🌐 Web query:
What is the correct syntax for importing realm configuration in Keycloak 24.0.5?💡 Result:
To import realm configurations in Keycloak 24.0.5, there are multiple methods depending on your deployment setup:
CLI-Based Import
For standalone servers/containers:
bin/kc.[sh|bat] import --file <realm-export.json> # Single file import bin/kc.[sh|bat] import --dir <export-directory> # Directory-based importKey options:
--override true/false(default=true) - controls overwriting existing realms[1][3]--realm <name>- specify single realm to import[1][3]Requirements:
- All Keycloak nodes must be stopped first[3]
- Database connection parameters must be configured[1]
Startup Auto-Import (Containers)
For containerized deployments:
# Place JSON files in container's import directory /opt/keycloak/data/import/Then start with:
bin/kc.[sh|bat] start --import-realmNote: Skips existing realms by default[1][6]. Use environment variables in JSON files:
{ "realm": "${ENV_VAR_NAME}", "enabled": true }Kubernetes Operator Method
For Red Hat Build of Keycloak Operator:
- Export realm to JSON[2]
- Convert to YAML CRD:
apiVersion: k8s.keycloak.org/v2alpha1 kind: KeycloakRealmImport metadata: name: my-realm-import spec: keycloakCRName: keycloak-deployment realm: id: my-realm realm: my-realm enabled: true
- Apply with:
oc apply -f realm-import.yamlAdmin Console Import (Partial)
- Realm Settings → Action Menu → Partial Import
- Select JSON file[1][4]
Limitations:
- No user data import[1][4]
- Not suitable for production backups[4]
For large deployments (>500 users), use directory-based exports instead of single files for better performance[1][3]. Always test imports in non-production environments first when using override functionality[1][3].
Citations:
- 1: https://docs.redhat.com/en/documentation/red_hat_build_of_keycloak/24.0/html/server_guide/importexport-
- 2: https://docs.redhat.com/en/documentation/red_hat_build_of_keycloak/24.0/html/operator_guide/realm-import-
- 3: https://www.keycloak.org/server/importExport
- 4: https://www.keycloak.org/docs/latest/server_admin/index.html
- 5: https://www.keycloak.org/2024/03/keycloak-2400-released
- 6: Import Realm in Keycloak 18.0.0 - failed keycloak/keycloak#11664
- 7: https://www.keycloak.org/docs-api/24.0.5/rest-api/index.html
- 8: Keycloak Operator fails to install realm authentication flow because "flow is null" keycloak/keycloak#23980
- 9: https://docs.redhat.com/en-us/documentation/red_hat_build_of_keycloak/24.0/pdf/server_guide/Red_Hat_build_of_Keycloak-24.0-Server_Guide-en-US.pdf
- 10: https://www.keycloak.org/docs/latest/upgrading/index.html
Update: Verified Keycloak import command syntax for CLI-based realm import
The command
/opt/keycloak/bin/kc.sh import --file /tmp/greenbuildings.jsonmatches the documented CLI syntax for Keycloak 24.0.5. This approach is valid for standalone/container deployments where a JSON file is used to import realm configuration.
- Confirm that your
greenbuildings.jsonadheres to Keycloak’s realm export schema.- Consider whether you need additional flags (e.g.,
--override) based on your usage..github/workflows/pull-request.yml (1)
41-41: LGTM! Tests now included in Gradle build.The removal of
-x testflag ensures that tests are executed during the build, which aligns with the PR objective to run testcontainer.
sep490-infrastructure/keycloak-testcontainer/greenbuildings.json
Outdated
Show resolved
Hide resolved
sep490-infrastructure/keycloak-testcontainer/greenbuildings.json
Outdated
Show resolved
Hide resolved
thongdanghoang
force-pushed
the
77-userstory-enterprise
branch
from
c76b0f0 to
7da95bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
sep490-infrastructure/keycloak-testcontainer-provider/greenbuildings.json (2)
1536-1545:⚠️ Potential issueAES-Generated Key Update: Check Cryptographic Material Exposure.
The updated kid and secret values for the AES-generated key have been introduced. These values are highly sensitive. Ensure that they are stored securely and that exposing them in this configuration file does not violate any security policies. It might be worthwhile to consider externalizing secrets if they are not meant to be public.
1590-1604:⚠️ Potential issueRSA-Enc-Generated Key Update: Confirm Cryptographic Integrity and Format.
The RSA-enc-generated configuration now has updated private key and certificate entries. Please verify that the new keys and their accompanying certificates are correctly formatted, match your infrastructure requirements, and are securely managed to prevent unintentional exposure.
🧹 Nitpick comments (2)
sep490-infrastructure/keycloak-testcontainer-provider/Dockerfile (1)
4-7: Verify file paths and optimize build.The build process could be optimized and made more reliable.
Consider these improvements:
-COPY ./greenbuildings.json /tmp -COPY ./target/greenbuildings-testcontainer-plugin.jar /opt/keycloak/providers -RUN /opt/keycloak/bin/kc.sh build --health-enabled=true -RUN /opt/keycloak/bin/kc.sh import --file /tmp/greenbuildings.json +COPY ./greenbuildings.json /tmp/ +COPY ./target/greenbuildings-testcontainer-plugin.jar /opt/keycloak/providers/ +RUN /opt/keycloak/bin/kc.sh build --health-enabled=true && \ + /opt/keycloak/bin/kc.sh import --file /tmp/greenbuildings.json && \ + rm -f /tmp/greenbuildings.jsonChanges:
- Added trailing slashes to directories for clarity
- Combined RUN commands to reduce layers
- Cleaned up temporary files
sep490-infrastructure/keycloak-testcontainer-provider/build.sh (1)
1-1: Improve Error Handling for Manifest Removal
The removal command (podman manifest rm thongdh3401/keycloak:24.0.5) might fail if the manifest does not exist, causing an unexpected script termination. Consider appending|| trueto allow the build to continue even if there’s no existing manifest.Suggested diff:
-podman manifest rm thongdh3401/keycloak:24.0.5 +podman manifest rm thongdh3401/keycloak:24.0.5 || true🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/pull-request.yml(1 hunks)sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java(1 hunks)sep490-enterprise/src/test/java/enterprise/rest/BuildingControllerTest.java(5 hunks)sep490-infrastructure/docker-compose.yml(1 hunks)sep490-infrastructure/keycloak-testcontainer-provider/.gitignore(1 hunks)sep490-infrastructure/keycloak-testcontainer-provider/Dockerfile(1 hunks)sep490-infrastructure/keycloak-testcontainer-provider/build.sh(1 hunks)sep490-infrastructure/keycloak-testcontainer-provider/greenbuildings.json(8 hunks)sep490-infrastructure/keycloak-testcontainer-provider/src/main/java/keycloak/provider/Sep490IdentityProviderTokenMapper.java(2 hunks)sep490-infrastructure/keycloak-testcontainer/Dockerfile(0 hunks)
💤 Files with no reviewable changes (1)
- sep490-infrastructure/keycloak-testcontainer/Dockerfile
✅ Files skipped from review due to trivial changes (1)
- sep490-infrastructure/keycloak-testcontainer-provider/.gitignore
🚧 Files skipped from review as they are similar to previous changes (4)
- sep490-infrastructure/keycloak-testcontainer-provider/src/main/java/keycloak/provider/Sep490IdentityProviderTokenMapper.java
- sep490-enterprise/src/test/java/enterprise/rest/BuildingControllerTest.java
- sep490-infrastructure/docker-compose.yml
- .github/workflows/pull-request.yml
🧰 Additional context used
🪛 Shellcheck (0.10.0)
sep490-infrastructure/keycloak-testcontainer-provider/build.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Java Modules (sep490-enterprise)
🔇 Additional comments (7)
sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java (1)
26-29: Verify container startup reliability.The removal of timeout configurations could lead to indefinite waits on slower systems or when the container fails to start properly.
Consider adding reasonable timeouts:
idP.withExposedPorts(8180) .withCommand("start-dev --http-port 8180") + .withStartupTimeout(Duration.ofMinutes(5)) .waitingFor(Wait.forHttp("/realms/greenbuildings/.well-known/openid-configuration")) .start();sep490-infrastructure/keycloak-testcontainer-provider/Dockerfile (1)
3-3: Consider security implications of running as root.Running as root user in containers is generally discouraged for security reasons.
Consider switching back to the non-root user after the required operations:
USER 0 COPY ./greenbuildings.json /tmp COPY ./target/greenbuildings-testcontainer-plugin.jar /opt/keycloak/providers RUN /opt/keycloak/bin/kc.sh build --health-enabled=true RUN /opt/keycloak/bin/kc.sh import --file /tmp/greenbuildings.json +USER 1000sep490-infrastructure/keycloak-testcontainer-provider/build.sh (1)
3-10: Verify Multi-Architecture Build and Manifest Push Commands
The subsequent commands (creating the manifest, building the image for multiple platforms, and pushing the manifest) are structured correctly for multi-arch support. Just ensure that the specified image tag (thongdh3401/keycloak:24.0.5) matches your intended version and that your environment is fully set up for these Podman operations.sep490-infrastructure/keycloak-testcontainer-provider/greenbuildings.json (4)
47-295: New Roles Definition Added: Verify Role Attributes and References.
The new “roles” block defines several realm and client roles with clearly set attributes and composite configurations. Please ensure that the placeholder expressions (e.g.${role_uma_authorization},${role_default-roles}) correctly resolve in your deployment environment and that their values match the intended role descriptions and composite mappings.
381-466: New Users Configuration Added: Review Credentials and Timestamps.
The added “users” array introduces new user definitions along with their credentials and role associations. Verify that the sensitive fields (such as secretData and credentialData) are handled according to security best practices and that the createdTimestamp values are appropriate. It is also worth checking that the realmRoles properly reference the newly defined default roles.
1477-1483: Protocol Mapper Types Update (Authenticated): Confirm Added Mappers.
Within the "Allowed Protocol Mapper Types" configuration for authenticated users, new entries like"saml-role-list-mapper","oidc-full-name-mapper", and"saml-user-property-mapper"have been added. Please verify that these mapper types are supported by your Keycloak version and that their use is consistent with your overall authentication strategy.
1509-1518: Protocol Mapper Types Update (Anonymous): Validate Mapper Consistency.
The anonymous configuration now includes entries such as"oidc-sha256-pairwise-sub-mapper","saml-user-attribute-mapper", and"oidc-full-name-mapper". Confirm that these mapper entries do not conflict with those defined for authenticated contexts and that they meet your policy requirements for anonymous flows.
sep490-infrastructure/keycloak-testcontainer-provider/greenbuildings.json
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
Chores