Skip to content

Update KRA tests - #5417

Merged
edewata merged 1 commit into
dogtagpki:masterfrom
edewata:kra
Aug 12, 2026
Merged

Update KRA tests#5417
edewata merged 1 commit into
dogtagpki:masterfrom
edewata:kra

Conversation

@edewata

@edewata edewata commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

The test for KRA with RSA-OAEP has been updated to execute the commands directly instead of calling test-cert-key-archival.sh to allow more detailed testing.

Other KRA tests have also been modified to check various config params including keywrap, transport unit, storage unit, PKCS #12 encryption as well as CA and KRA info returned by the REST API.

Summary by CodeRabbit

  • Tests
    • Expanded automated validation for KRA transport, storage, key-wrapping, encryption, and PKCS #12 settings.
    • Added REST API checks for CA and KRA archival, recovery, and encryption capabilities.
    • Added end-to-end RSA-OAEP key archival and recovery coverage, including connectivity and diagnostics.
    • Replaced the previous OAEP test workflow with the new RSA-OAEP workflow.
    • Removed the obsolete standalone OAEP workflow and streamlined ML-KEM validation.

The test for KRA with RSA-OAEP has been updated to execute the
commands directly instead of calling test-cert-key-archival.sh
to allow more detailed testing.

Other KRA tests have also been modified to check various
config params including keywrap, transport unit, storage unit,
PKCS #12 encryption as well as CA and KRA info returned by the
REST API.
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR expands KRA workflow validation for transport, storage, wrapping, encryption, archival, recovery, and REST metadata. It replaces the OAEP test job with a reusable RSA-OAEP workflow that performs end-to-end provisioning, key archival, recovery, cleanup, and diagnostics.

Changes

KRA configuration and REST validation

Layer / File(s) Summary
Configuration and REST assertions
.github/workflows/kra-basic-test.yml, .github/workflows/kra-ecc-test.yml, .github/workflows/kra-pqc-test.yml
The workflows validate transport-unit, storage-unit, PKCS #12, wrapping, encryption, archival, recovery, and CA/KRA /v2/info settings for basic, ECC, and PQC configurations.
RSA-OAEP end-to-end test
.github/workflows/kra-rsa-oaep-test.yml, .github/workflows/kra-tests.yml
The test matrix invokes the RSA-OAEP workflow. The workflow provisions CA, KRA, and Directory Server containers, validates configuration and connectivity, archives and recovers a certificate key, and collects diagnostics.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GitHub_Actions
  participant CA_KRA_Containers
  participant LDAP
  GitHub_Actions->>CA_KRA_Containers: Install CA and KRA with RSA-OAEP wrapping
  GitHub_Actions->>CA_KRA_Containers: Enroll OAEP-protected certificate request
  CA_KRA_Containers->>LDAP: Store archived key metadata
  GitHub_Actions->>CA_KRA_Containers: Request key recovery
  CA_KRA_Containers->>LDAP: Read archived key data
  LDAP-->>CA_KRA_Containers: Return key record
  CA_KRA_Containers-->>GitHub_Actions: Return recovered private key
Loading

Possibly related PRs

Suggested reviewers: fmarco76

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change, which updates and expands the KRA test workflows.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/workflows/kra-rsa-oaep-test.yml (1)

16-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the stale in-container dumpasn1 install comment.

This workflow installs dumpasn1 on the runner with apt-get, so the commented docker exec pki dnf install -y jq dumpasn1 at line 54 leaves two competing install approaches. Keep the runner install, and add the container install only if dumpasn1 is actually required inside the PKI container.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/kra-rsa-oaep-test.yml around lines 16 - 19, Remove the
stale commented in-container install command near the PKI container setup, while
retaining the runner-level dumpasn1 installation in the “Install dependencies”
step. Add a container installation only if a workflow step actually invokes
dumpasn1 inside that container.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/kra-rsa-oaep-test.yml:
- Around line 278-320: Replace the ca-cert-issue invocation in the “Enroll cert
with key archival” step with the existing enrollment command that supports the
-oaep key-wrap option and accepts the transport certificate settings. Preserve
the generated CSR, subject, profile, and output certificate behavior while
ensuring the request is submitted through the supported enrollment path rather
than the CRMFPopClient CA path.

---

Nitpick comments:
In @.github/workflows/kra-rsa-oaep-test.yml:
- Around line 16-19: Remove the stale commented in-container install command
near the PKI container setup, while retaining the runner-level dumpasn1
installation in the “Install dependencies” step. Add a container installation
only if a workflow step actually invokes dumpasn1 inside that container.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b9b48315-fa5d-48c3-8440-e29225421591

📥 Commits

Reviewing files that changed from the base of the PR and between be8895d and d442ab2.

📒 Files selected for processing (6)
  • .github/workflows/kra-basic-test.yml
  • .github/workflows/kra-ecc-test.yml
  • .github/workflows/kra-oaep-test.yml
  • .github/workflows/kra-pqc-test.yml
  • .github/workflows/kra-rsa-oaep-test.yml
  • .github/workflows/kra-tests.yml
💤 Files with no reviewable changes (1)
  • .github/workflows/kra-oaep-test.yml

Comment thread .github/workflows/kra-rsa-oaep-test.yml

@fmarco76 fmarco76 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@edewata

edewata commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

@fmarco76 Thanks! I'm still trying to understand CodeRabbit's comment. Hopefully it's nothing major.

@edewata

edewata commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

OK, it's not an issue. Merging now.

@edewata
edewata merged commit ed1f3da into dogtagpki:master Aug 12, 2026
227 of 232 checks 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.

2 participants