feat: v2.0.0 — service layer, handler pattern, per-store-type jobs, X509Certificate2 removal#85
Open
spbsoluble wants to merge 9 commits intorelease-2.0from
Open
feat: v2.0.0 — service layer, handler pattern, per-store-type jobs, X509Certificate2 removal#85spbsoluble wants to merge 9 commits intorelease-2.0from
spbsoluble wants to merge 9 commits intorelease-2.0from
Conversation
* feat: `x509certificate2` removal (#71) * Update generated docs * chore(lint): Fix PR review lint. * Update generated docs * test: unit tests for SeparateChain/IncludeCertChain conflict resolution in JobBase Adds StorePropertiesParsingTests covering the four flag combinations so that the override logic (SeparateChain forced to false when IncludeCertChain=false) is caught at the unit level, not only by integration tests. * Update generated docs --------- Co-authored-by: spb <1661003+spbsoluble@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io>
Break domain logic out of JobBase into focused, testable services: - StoreConfigurationParser: parses CertificateStoreDetails.Properties JSON into a typed StoreConfiguration, eliminating dynamic dispatch - StorePathResolver: resolves StorePath strings (namespace/secret-name) into structured PathResolutionResult for all store type patterns - JobCertificateParser: extracts certificate/key/chain from ManagementJobConfiguration with explicit format detection - PasswordResolver: resolves passwords from inline values or K8S secret references, centralising the "buddy password" pattern - CertificateChainExtractor: parses PEM chains into leaf + intermediates, handling both bundled and pre-separated chain formats - KeystoreOperations: JKS/PKCS12 read/write operations moved out of handlers into a standalone service None of these services require a Kubernetes client, making them fully unit-testable without network access.
Replace inline switch/if chains in JobBase with a proper Strategy pattern: - ISecretHandler: contract for Inventory, Management, Discovery, and Reenrollment operations on a specific secret/store type - SecretHandlerBase: shared infrastructure (client, logging, result helpers) - SecretHandlerFactory: creates the correct handler from SecretType enum - Per-type handlers: TlsSecretHandler, OpaqueSecretHandler, JksSecretHandler, Pkcs12SecretHandler, ClusterSecretHandler, NamespaceSecretHandler, CertificateSecretHandler (read-only) Supporting additions: - SecretTypes enum: typed representation of Kubernetes secret types with normalisation and IsTlsType/IsOpaqueType helpers - K8SJobCertificate model: replaces ad-hoc certificate data passing - Exceptions: StoreNotFoundException, InvalidK8SSecretException, JkSisPkcs12Exception — typed errors replace bare Exception throws - ICertificateStoreSerializer + JKS/PKCS12 serializer implementations moved from StoreTypes/ to Serializers/ (interface renamed for clarity)
KubeClient.cs was a 3000+ line file mixing authentication, kubeconfig parsing, secret CRUD, and CSR operations. Split into: - KubeconfigParser: parses kubeconfig JSON into typed configuration, validates required fields, provides clear error messages - SecretOperations: Kubernetes secret CRUD (create, read, update, delete, list) with retry logic and structured logging - CertificateOperations: CSR-specific operations (list, read, approve, inject certificate status) - KubeClient (KubeCertificateManagerClient): now a thin coordinator that initialises the authenticated client and delegates to the above Also removes unreachable code branches, converts string interpolation log calls to structured logging throughout, and adds retry logic with configurable backoff.
Job structure (flat → per-store-type):
- Remove Jobs/Inventory.cs, Management.cs, Discovery.cs, Reenrollment.cs
(monolithic files with large switch statements on store type)
- Add Jobs/Base/: K8SJobBase, InventoryBase, ManagementBase, DiscoveryBase,
ReenrollmentBase — shared logic each job type delegates to its handler
- Add Jobs/StoreTypes/<Type>/: one class per operation per store type
(7 store types × up to 4 operations = 26 concrete job classes)
- manifest.json updated to route each capability to its dedicated class
X509Certificate2 removal:
- Replace X509Certificate2 usage throughout with BouncyCastle types
- K8SCertificateContext replaces X509Certificate2-based SerializedStoreInfo
- LoggingUtilities updated: GetCertificateSummary now accepts BouncyCastle
X509Certificate; RedactPassword no longer leaks password length
Version logging:
- JobBase reads AssemblyInformationalVersionAttribute at startup and logs
"K8S Orchestrator Extension version: {Version}" on every job execution
(baked in at build time by GitHub Actions via -p:Version=<tag>)
Also removes TestConsole (superseded by integration test suite) and
store_types.json (superseded by integration-manifest.json).
Test infrastructure: - CachedCertificateProvider: thread-safe cache for generated certificates; eliminates redundant RSA key generation across test collections (RSA 8192 takes 30+ seconds per key — this alone cut full-suite runtime by ~60%) - IntegrationTestFixture: shared kubeconfig loading, K8S client creation, namespace setup/teardown for all integration test collections - SkipUnless attribute: skips integration tests when RUN_INTEGRATION_TESTS is not set, keeping unit test runs fast New unit tests (zero network access): - Services: StoreConfigurationParser, StorePathResolver, PasswordResolver, CertificateChainExtractor, JobCertificateParser, KeystoreOperations - Handlers: SecretHandlerBase, SecretHandlerFactory, all handler types (no-network paths), alias routing regression - Clients: KubeconfigParser, SecretOperations, CertificateOperations, KubeCertificateManagerClient - Jobs: ManagementBase, DiscoveryBase, PAMUtilities, exception paths, K8SJobCertificate, K8SCertificateContext - Utilities: LoggingUtilities (60 cases including DoesNotRevealLength), CertificateUtilities, LoggingSafetyTests - Enums: SecretTypes Updated integration tests: migrated all 7 store-type integration test files to use IntegrationTestFixture and new job class namespaces. Also adds scripts/analyze-coverage.py for coverage gap analysis.
…2.0.0 - CHANGELOG.md: document v2.0.0 breaking changes — new store type routing via per-store-type job classes, removed X509Certificate2 dependency, updated job configuration model - docs/ARCHITECTURE.md: new file documenting the service/handler/job architecture, authentication flow, and extension points - Development.md: updated testing guide with CachedCertificateProvider guidance, integration test setup, coverage targets - README.md: regenerated from docsource/ with updated store type dialogs - docsource/: updated content and added SVG store type dialog images for all 7 store types - .github/workflows: add test-doctool workflow, update starter workflow - scripts/store_types/: updated kfutil helper scripts - terraform/: add Terraform module examples for all store types
github-actions
bot
added
documentation
Dependency ReviewThe following issues were found:
Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. License Issues.github/workflows/dotnet-security-scan.yml
.github/workflows/sbom-generation.yml
.github/workflows/unit-tests.yml
OpenSSF ScorecardScorecard details
Scanned Files
|
Unit Test Results (.NET 10.0.x)0 tests 0 ✅ 0s ⏱️ Results for commit 19bb081. |
Unit Test Results (.NET 8.0.x)0 tests 0 ✅ 0s ⏱️ Results for commit 19bb081. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Integration Test Results (K8s v1.29.0)0 tests 0 ✅ 0s ⏱️ Results for commit 19bb081. |
Reenrollment is not a supported operation. Remove it from the overview sentence, fix the store type operations table (K8SJKS and K8SPKCS12 were incorrectly listed as 'All + Reenrollment'), and remove ReenrollmentBase.cs from the base class directory listing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Major architectural refactor for v2.0.0. This is a breaking change to the extension's internal structure — the external behaviour (supported store types, job operations, store properties) is unchanged.
What changed
StoreConfigurationParser,StorePathResolver,JobCertificateParser,PasswordResolver,CertificateChainExtractor,KeystoreOperationsare now standalone, fully unit-testable services with no Kubernetes dependencySecretHandlerFactoryselects the right handler fromSecretTypeenumKubeconfigParser,SecretOperations,CertificateOperationsextracted from the monolithic 3000-lineKubeClient.csJobs/Inventory.cs,Management.csetc. replaced byJobs/Base/shared logic +Jobs/StoreTypes/<Type>/concrete classes;manifest.jsonroutes each capability directly to its classK8SCertificateContextreplaces X509Certificate2-based store info; BouncyCastle and Keyfactor.PKI used throughoutAssemblyInformationalVersionAttribute) logged at every job startCachedCertificateProvidereliminates redundant RSA key generation, cutting full-suite runtime by ~60%Security hardening (embedded in refactor commits)
KubeClient(commitab3fe8aa): preserveresourceVersionon TLS secret replace for Kubernetes optimistic concurrencyStorePathResolver(commit294a225c): validate namespace/secret-name components against DNS subdomain rules; log warning and pass through for backwards compatibilityK8SCertStoreIntegrationTests(commit6c8f244c): pass--contexttokubectl patchin CSR injection helper so it targets the correct clusterCommits
294a225ccfc42330ab3fe8aa1ee4a8326c8f244c19bb081eTest plan
kf-integrationscluster