Merge 2.0.1 to main - #98
Merged
Merged
Conversation
* feat: `x509certificate2` removal (#73) (#79) * 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> * refactor: extract service layer from monolithic JobBase 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. * refactor: introduce handler strategy pattern for secret operations 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) * refactor: split monolithic KubeClient into focused client components 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. * refactor: restructure job classes by store type, remove X509Certificate2 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). * feat: add CachedCertificateProvider and comprehensive test suite 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. * docs: update CHANGELOG, ARCHITECTURE.md, Development.md, README for v2.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 * docs(architecture): remove incorrect reenrollment references 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. * docs: auto-generate README and documentation [skip ci] * docs: update compatibility to include Command 24.x and 25.x Update the compatibility statement and UO version matrix to explicitly call out support for Keyfactor Command platform versions 24.x and 25.x, and add a net10.0 row for Command 25.x and newer. * docs: auto-generate README and documentation [skip ci] * docs: call out .NET 8 and .NET 10 compatibility in README Add explicit mention of net8.0/net10.0 dual-targeting to the Compatibility section so users know which build to download without having to dig into the installation table. * docs: auto-generate README and documentation [skip ci] * docs(changelog): add v2.0.0 entry * docs(changelog): merge pre-rebase content into v2.0.0 and 1.3.0 entries Add missing breaking changes (JobBase dead property removal, KeystoreManager removal), terraform feature, and richer 1.3.0 bug fixes (create-if-missing, buddy-secret password, alias routing) and refactor/test chores from the break/major_refactor branch changelog. * fix: add missing Serializers directory to fix build The Serializers/ directory containing JKS and PKCS12 store serializers was never committed, causing build failures when handler files attempted to reference the Keyfactor.Extensions.Orchestrator.K8S.Serializers namespace. * docs(auth): add client certificate auth as alternative to SA token - Fix fragile grep/awk token lookup in get_service_account_creds.sh and create_service_account.sh — now uses direct jsonpath lookup with a clear error message if the token Secret is missing (k8s v1.22+) - Add generate_client_cert_creds.sh: end-to-end script that applies RBAC, generates an RSA key, submits and approves a k8s CSR, and builds a client-cert kubeconfig in one step - Add kubernetes_svc_account_cert_auth.yaml: ClusterRole + ClusterRoleBinding for cert-based auth (kind: User subject, no ServiceAccount required) - Add example_kubeconfig_cert.json showing client-certificate-data layout - Rewrite scripts/kubernetes/README.md to present both auth options equally with comparison table, quickstart, config reference, and manual steps - Update docsource/content.md Requirements section to document both methods * docs: auto-generate README and documentation [skip ci] * feat(auth): add in-cluster pod identity as third authentication option Plugin changes: - KubeClient.GetKubeClient(): detect KUBERNETES_SERVICE_HOST and call InClusterConfig() when no kubeconfig is provided, using the projected service account token mounted by kubelet (auto-rotated every hour) - JobBase.InitializeProperties(): allow empty KubeSvcCreds when running in-cluster instead of throwing ConfigurationException Scripts/docs: - Add keyfactor-orchestrator-deployment.yaml: Deployment manifest that runs the UO as a pod using the keyfactor-orchestrator-sa ServiceAccount - Update scripts/kubernetes/README.md: add Option 3 to comparison table and full setup section (apply SA YAML, deploy, leave Server Password blank) - Update docsource/content.md: document all three auth options equally * docs: auto-generate README and documentation [skip ci] * docs(auth): clarify in-cluster requires "No value" for Server Password in Command UI * docs: auto-generate README and documentation [skip ci] * fix(security): SOX/SOC2 compliance remediations and UseSSL bug fix Compliance remediations (all findings were pre-existing on branch): - Redact certificate bytes in UpdateOpaqueSecret log traces (CRIT-1) - Log CSR certificate length only, not content preview (CRIT-2) - Add structured AUDIT log entries (store_access, secret_read/write/delete) to ManagementBase, InventoryBase, DiscoveryBase, SecretOperations (CRIT-3) - ValidateK8SName throws ArgumentException instead of warning; 5+ segment paths return Success=false and fail the job (CRIT-4) - Zero KubeSvcCreds and ServerPassword after KubeClient construction (HIGH-1) - RedactKubeconfig validates JSON structure before applying label; non-JSON returns POSSIBLY_MALFORMED_CREDENTIAL (HIGH-2) - Silent catch blocks in JKS/PKCS12 serializers now log exception type (HIGH-3) - PAM resolution outcome promoted from LogTrace to LogInformation (HIGH-4) - TLS skip override promoted from LogWarning to LogError with structured SECURITY_CONFIG_OVERRIDE field (HIGH-5) - ReadBuddyPass: make passwordSecretName discard explicit with _ (HIGH-6) - HandleRemove returns Warning (not Success) when store not found so job history distinguishes no-op from actual removal (HIGH-7) - Remove KubeSvcCreds from storeProperties dict after client construction (MED-1) - StorePathResolver rejects 5+ segment paths (MED-4) - Handler NotFound catch blocks use HttpOperationException status code comparison instead of ex.Message string matching (MED-5) - Discovery InitializeStore wrapped in try/catch matching Inventory/Management pattern (MED-6) Bug fix: - UseSSL value from job config (config.UseSSL) was never forwarded to KubeCertificateManagerClient — TLS verification was always defaulting to true regardless of the store's Use SSL checkbox. Now captured in each InitializeStore overload and passed through InitializeKubeClient. * security: remove GetPasswordCorrelationId and update changelog Removes SHA-256 password correlation ID (MED-2) — low-entropy passwords are reversible via dictionary attack and RedactPassword is already present at all call sites. Updates CHANGELOG.md with all v2.0.0 changes from this session including client cert auth, in-cluster auth, UseSSL fix, audit logging, and compliance remediations. * docs: remove duplicate content sections from generated README Regenerated with fixed doctooldotnet (Keyfactor/doctooldotnet#9). Named content.md sections were being emitted twice due to title mutation before the custom-sections filter ran. NOTE: Actions will revert this until doctooldotnet PR #9 is merged. * docs: auto-generate README and documentation [skip ci] * chore(ci): revert to old doctool * fix(k8scert): ignore storepath for csr mode and add regression coverage * fix(inventory): sanitize URL cluster names in discovery location strings (#88) --------- Co-authored-by: Morgan Gangwere <470584+indrora@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…es (#92) * feat: Major refactor for v2.0.0 (#85) (#90) * feat: `x509certificate2` removal (#73) (#79) * 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 --------- * refactor: extract service layer from monolithic JobBase 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. * refactor: introduce handler strategy pattern for secret operations 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) * refactor: split monolithic KubeClient into focused client components 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. * refactor: restructure job classes by store type, remove X509Certificate2 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). * feat: add CachedCertificateProvider and comprehensive test suite 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. * docs: update CHANGELOG, ARCHITECTURE.md, Development.md, README for v2.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 * docs(architecture): remove incorrect reenrollment references 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. * docs: auto-generate README and documentation [skip ci] * docs: update compatibility to include Command 24.x and 25.x Update the compatibility statement and UO version matrix to explicitly call out support for Keyfactor Command platform versions 24.x and 25.x, and add a net10.0 row for Command 25.x and newer. * docs: auto-generate README and documentation [skip ci] * docs: call out .NET 8 and .NET 10 compatibility in README Add explicit mention of net8.0/net10.0 dual-targeting to the Compatibility section so users know which build to download without having to dig into the installation table. * docs: auto-generate README and documentation [skip ci] * docs(changelog): add v2.0.0 entry * docs(changelog): merge pre-rebase content into v2.0.0 and 1.3.0 entries Add missing breaking changes (JobBase dead property removal, KeystoreManager removal), terraform feature, and richer 1.3.0 bug fixes (create-if-missing, buddy-secret password, alias routing) and refactor/test chores from the break/major_refactor branch changelog. * fix: add missing Serializers directory to fix build The Serializers/ directory containing JKS and PKCS12 store serializers was never committed, causing build failures when handler files attempted to reference the Keyfactor.Extensions.Orchestrator.K8S.Serializers namespace. * docs(auth): add client certificate auth as alternative to SA token - Fix fragile grep/awk token lookup in get_service_account_creds.sh and create_service_account.sh — now uses direct jsonpath lookup with a clear error message if the token Secret is missing (k8s v1.22+) - Add generate_client_cert_creds.sh: end-to-end script that applies RBAC, generates an RSA key, submits and approves a k8s CSR, and builds a client-cert kubeconfig in one step - Add kubernetes_svc_account_cert_auth.yaml: ClusterRole + ClusterRoleBinding for cert-based auth (kind: User subject, no ServiceAccount required) - Add example_kubeconfig_cert.json showing client-certificate-data layout - Rewrite scripts/kubernetes/README.md to present both auth options equally with comparison table, quickstart, config reference, and manual steps - Update docsource/content.md Requirements section to document both methods * docs: auto-generate README and documentation [skip ci] * feat(auth): add in-cluster pod identity as third authentication option Plugin changes: - KubeClient.GetKubeClient(): detect KUBERNETES_SERVICE_HOST and call InClusterConfig() when no kubeconfig is provided, using the projected service account token mounted by kubelet (auto-rotated every hour) - JobBase.InitializeProperties(): allow empty KubeSvcCreds when running in-cluster instead of throwing ConfigurationException Scripts/docs: - Add keyfactor-orchestrator-deployment.yaml: Deployment manifest that runs the UO as a pod using the keyfactor-orchestrator-sa ServiceAccount - Update scripts/kubernetes/README.md: add Option 3 to comparison table and full setup section (apply SA YAML, deploy, leave Server Password blank) - Update docsource/content.md: document all three auth options equally * docs: auto-generate README and documentation [skip ci] * docs(auth): clarify in-cluster requires "No value" for Server Password in Command UI * docs: auto-generate README and documentation [skip ci] * fix(security): SOX/SOC2 compliance remediations and UseSSL bug fix Compliance remediations (all findings were pre-existing on branch): - Redact certificate bytes in UpdateOpaqueSecret log traces (CRIT-1) - Log CSR certificate length only, not content preview (CRIT-2) - Add structured AUDIT log entries (store_access, secret_read/write/delete) to ManagementBase, InventoryBase, DiscoveryBase, SecretOperations (CRIT-3) - ValidateK8SName throws ArgumentException instead of warning; 5+ segment paths return Success=false and fail the job (CRIT-4) - Zero KubeSvcCreds and ServerPassword after KubeClient construction (HIGH-1) - RedactKubeconfig validates JSON structure before applying label; non-JSON returns POSSIBLY_MALFORMED_CREDENTIAL (HIGH-2) - Silent catch blocks in JKS/PKCS12 serializers now log exception type (HIGH-3) - PAM resolution outcome promoted from LogTrace to LogInformation (HIGH-4) - TLS skip override promoted from LogWarning to LogError with structured SECURITY_CONFIG_OVERRIDE field (HIGH-5) - ReadBuddyPass: make passwordSecretName discard explicit with _ (HIGH-6) - HandleRemove returns Warning (not Success) when store not found so job history distinguishes no-op from actual removal (HIGH-7) - Remove KubeSvcCreds from storeProperties dict after client construction (MED-1) - StorePathResolver rejects 5+ segment paths (MED-4) - Handler NotFound catch blocks use HttpOperationException status code comparison instead of ex.Message string matching (MED-5) - Discovery InitializeStore wrapped in try/catch matching Inventory/Management pattern (MED-6) Bug fix: - UseSSL value from job config (config.UseSSL) was never forwarded to KubeCertificateManagerClient — TLS verification was always defaulting to true regardless of the store's Use SSL checkbox. Now captured in each InitializeStore overload and passed through InitializeKubeClient. * security: remove GetPasswordCorrelationId and update changelog Removes SHA-256 password correlation ID (MED-2) — low-entropy passwords are reversible via dictionary attack and RedactPassword is already present at all call sites. Updates CHANGELOG.md with all v2.0.0 changes from this session including client cert auth, in-cluster auth, UseSSL fix, audit logging, and compliance remediations. * docs: remove duplicate content sections from generated README Regenerated with fixed doctooldotnet (Keyfactor/doctooldotnet#9). Named content.md sections were being emitted twice due to title mutation before the custom-sections filter ran. NOTE: Actions will revert this until doctooldotnet PR #9 is merged. * docs: auto-generate README and documentation [skip ci] * chore(ci): revert to old doctool * fix(k8scert): ignore storepath for csr mode and add regression coverage * fix(inventory): sanitize URL cluster names in discovery location strings (#88) --------- Co-authored-by: spb <1661003+spbsoluble@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix(management): stop reporting Success on silent secret write failures ManagementBase.HandleAdd discarded the handler's returned V1Secret and unconditionally returned Success, so a K8S write that failed without throwing (e.g. a pre-existing secret update that silently no-opped) was still reported to Command as a successful deployment. HandleAdd now checks the returned V1Secret and fails the job with an actionable message when it is null. KubeCertificateManagerClient.CreateOrUpdateCertificateStoreSecret used a blind create-then-catch flow, routing to the update path only on a free-text e.Message.Contains("Conflict") match; any other HttpOperationException (e.g. 403 from RBAC scoping) fell through and returned null silently. It now reads the secret first (existence check via SecretOperations.GetSecret, which only swallows a typed 404) and branches to update when it already exists, matching the JKS/PKCS12 path. A typed HttpStatusCode.Conflict on the create call still falls back to update for a genuine create race; every other HttpOperationException now propagates instead of being swallowed. Adds regression coverage for both fixes: ManagementBaseTests covers the null-handler-result -> Failure path, and the new KubeClientCreateOrUpdateSecretTests covers create/update routing plus 403 propagation on both the existence check and the create call. Fixes #91 * fix(make): pin test-cluster-cleanup to the integration test kube context test-cluster-cleanup used the current kubectl context, so with a different active context (e.g. docker-desktop) it silently cleaned the wrong cluster and stale test namespaces accumulated on kf-integrations for months — making cluster-wide inventory integration tests pathologically slow. Introduce TEST_KUBE_CONTEXT (default kf-integrations, overridable) and pass --context to all kubectl calls in the cleanup target. * test(k8scert): skip cluster-wide CSR inventory test on unpopulated clusters Inventory_ClusterWideMode_InventoriesAllIssuedCsrs_InCurrentCluster hard- asserted >=30 issued CSRs, which only holds on the populated lab cluster — it can never pass on the ephemeral cluster CI provisions, and this PR's CI run was the first to execute it there. Soft-skip when the cluster has no issued CSRs; behavior against the lab cluster is unchanged (verified via make test-store-cert, 10 passed on both TFMs). * chore(deps): bump GitHub Actions versions Incorporates the dependabot version bumps from PRs #80-84: - actions/checkout v4 -> v6 (#80) - actions/setup-dotnet v4 -> v5 (#81) - softprops/action-gh-release v1 -> v2 (#82) - actions/github-script v7 -> v8 (#83) - actions/upload-artifact v4 -> v7 (#84) * style(tests): use ternary in SetupRead per code-quality bot feedback on PR #92 Both branches assign to the same seq variable — a ternary expresses that more directly than if/else. --------- Co-authored-by: Morgan Gangwere <470584+indrora@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| try | ||
| { | ||
| await _k8sClient.CoreV1.DeleteNamespacedSecretAsync(secretName, ns); | ||
| await _k8sClient.CoreV1.DeleteCollectionNamespacedSecretAsync( |
| } | ||
|
|
||
| return result; | ||
| return result!; |
| var expectedThumbprint = BouncyCastleX509Extensions.Thumbprint(certInfo.Certificate); | ||
| var inventoriedCertPem = inventoriedCerts[0].Certificates.First(); | ||
| using var reader = new System.IO.StringReader(inventoriedCertPem); | ||
| var pemReader = new Org.BouncyCastle.OpenSsl.PemReader(reader); |
| var expectedThumbprint = BouncyCastleX509Extensions.Thumbprint(certInfo.Certificate); | ||
| var inventoriedCertPem = inventoriedCerts[0].Certificates.First(); | ||
| using var reader = new System.IO.StringReader(inventoriedCertPem); | ||
| var pemReader = new Org.BouncyCastle.OpenSsl.PemReader(reader); |
| // Arrange | ||
| var pkcs12Bytes = CachedCertificateProvider.GetOrCreatePkcs12(KeyType.Rsa2048, "password"); | ||
| var store = new Pkcs12StoreBuilder().Build(); | ||
| store.Load(new MemoryStream(pkcs12Bytes), "password".ToCharArray()); |
| // Arrange | ||
| var pkcs12Bytes = CachedCertificateProvider.GetOrCreatePkcs12(KeyType.Rsa2048, "password"); | ||
| var store = new Pkcs12StoreBuilder().Build(); | ||
| store.Load(new MemoryStream(pkcs12Bytes), "password".ToCharArray()); |
| // Arrange | ||
| var pkcs12Bytes = CachedCertificateProvider.GetOrCreatePkcs12(keyType, "password"); | ||
| var store = new Pkcs12StoreBuilder().Build(); | ||
| store.Load(new MemoryStream(pkcs12Bytes), "password".ToCharArray()); |
| private static HttpOperationException MakeHttpException(HttpStatusCode status) => | ||
| new($"Operation returned an invalid status code '{status}'") | ||
| { | ||
| Response = new HttpResponseMessageWrapper(new HttpResponseMessage(status), string.Empty) |
| }; | ||
|
|
||
| private static Task<HttpOperationResponse<V1Secret>> Response(V1Secret secret) => | ||
| Task.FromResult(new HttpOperationResponse<V1Secret> { Body = secret }); |
| { | ||
| // Arrange | ||
| var certInfo = CachedCertificateProvider.GetOrCreate(KeyType.Rsa2048, "Test Summary X509"); | ||
| var x509Cert = new System.Security.Cryptography.X509Certificates.X509Certificate2(certInfo.Certificate.GetEncoded()); |
Contributor
There was a problem hiding this comment.
Pull request overview
Automated merge from release-2.0 into main, bringing in a broad set of additions and refactors around Terraform-based store provisioning, Kubernetes auth/setup scripts, and a re-organization of orchestrator job/handler code (plus related docs/tests and CI workflow updates).
Changes:
- Added reusable Terraform modules (and examples) for all supported Kubernetes store types.
- Refactored/expanded orchestrator internals (new job base classes, handlers, utilities, and tests) and updated logging/redaction behavior.
- Updated scripts and documentation for Kubernetes authentication and store type creation; refreshed multiple GitHub Actions workflows.
Reviewed changes
Copilot reviewed 66 out of 211 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| update_store_types.sh | Removed legacy store type update helper script. |
| TestConsole/yaml2json.sh | Removed YAML→JSON helper script for TestConsole. |
| TestConsole/tests.yml | TestConsole asset (content change not shown in diff). |
| TestConsole/tests.json | TestConsole asset (content change not shown in diff). |
| TestConsole/TestConsole.csproj | Removed TestConsole project file. |
| terraform/README.md | Added top-level Terraform documentation for modules/examples. |
| terraform/modules/k8s-tls/variables.tf | Added TLS module inputs. |
| terraform/modules/k8s-tls/README.md | Added TLS module usage documentation. |
| terraform/modules/k8s-tls/outputs.tf | Added TLS module outputs. |
| terraform/modules/k8s-tls/main.tf | Implemented TLS store + deployments resources. |
| terraform/modules/k8s-secret/variables.tf | Added Opaque PEM secret module inputs. |
| terraform/modules/k8s-secret/README.md | Added Opaque PEM secret module documentation. |
| terraform/modules/k8s-secret/outputs.tf | Added Opaque PEM secret module outputs. |
| terraform/modules/k8s-secret/main.tf | Implemented Opaque PEM secret store + deployments. |
| terraform/modules/k8s-pkcs12/variables.tf | Added PKCS12 module inputs (incl. buddy password options). |
| terraform/modules/k8s-pkcs12/README.md | Added PKCS12 module documentation and examples. |
| terraform/modules/k8s-pkcs12/outputs.tf | Added PKCS12 module outputs (incl. password mode flag). |
| terraform/modules/k8s-pkcs12/main.tf | Implemented PKCS12 store + deployments resources. |
| terraform/modules/k8s-ns/variables.tf | Added namespace-level module inputs. |
| terraform/modules/k8s-ns/README.md | Added namespace-level module documentation. |
| terraform/modules/k8s-ns/outputs.tf | Added namespace-level module outputs. |
| terraform/modules/k8s-ns/main.tf | Implemented namespace-level store + deployments resources. |
| terraform/modules/k8s-jks/variables.tf | Added JKS module inputs (incl. buddy password options). |
| terraform/modules/k8s-jks/README.md | Added JKS module documentation and examples. |
| terraform/modules/k8s-jks/outputs.tf | Added JKS module outputs (incl. password mode flag). |
| terraform/modules/k8s-jks/main.tf | Implemented JKS store + deployments resources. |
| terraform/modules/k8s-cluster/variables.tf | Added cluster-wide module inputs. |
| terraform/modules/k8s-cluster/README.md | Added cluster-wide module documentation. |
| terraform/modules/k8s-cluster/outputs.tf | Added cluster-wide module outputs. |
| terraform/modules/k8s-cluster/main.tf | Implemented cluster-wide store + deployments resources. |
| terraform/modules/k8s-cert/variables.tf | Added CSR inventory module inputs. |
| terraform/modules/k8s-cert/README.md | Added CSR inventory module documentation. |
| terraform/modules/k8s-cert/outputs.tf | Added CSR inventory module outputs. |
| terraform/modules/k8s-cert/main.tf | Implemented CSR inventory store resource. |
| terraform/examples/k8s-tls-basic/main.tf | Added example for TLS secret store + deployment. |
| terraform/examples/k8s-jks-buddy-password/main.tf | Added example for JKS store using buddy password secret. |
| scripts/store_types/README.md | Added/updated docs for store type script generation and usage. |
| scripts/store_types/powershell/kfutil_create_store_types.ps1 | Updated generated PowerShell store type creation script. |
| scripts/store_types/bash/kfutil_create_store_types.sh | Updated generated Bash store type creation script. |
| scripts/kubernetes/kubernetes_svc_account_cert_auth.yaml | Added RBAC manifest for client-cert auth scenario. |
| scripts/kubernetes/keyfactor-orchestrator-deployment.yaml | Added in-cluster UO deployment manifest and credential secret example. |
| scripts/kubernetes/get_service_account_creds.sh | Updated token secret lookup logic + added explicit error handling. |
| scripts/kubernetes/example_kubeconfig_cert.json | Added example kubeconfig JSON for client certificate auth. |
| scripts/kubernetes/create_service_account.sh | Updated token secret lookup logic for service account setup. |
| Makefile | Added explicit kube context for cleanup; updated cleanup target to use it. |
| kubernetes-orchestrator-extension/Utilities/LoggingUtilities.cs | Adjusted password redaction and added kubeconfig JSON validation. |
| kubernetes-orchestrator-extension/Utilities/CertificateUtilities.cs | Removed password correlation logging call. |
| kubernetes-orchestrator-extension/Services/PasswordResolver.cs | Added centralized password resolution for keystore operations. |
| kubernetes-orchestrator-extension/Services/KeystoreOperations.cs | Added shared keystore parsing/helpers to reduce duplication. |
| kubernetes-orchestrator-extension/Serializers/ICertificateStoreSerializer.cs | Moved serializer interface namespace to Serializers. |
| kubernetes-orchestrator-extension/Models/K8SJobCertificate.cs | Added comprehensive job certificate model. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8STLSSecr/Reenrollment.cs | Added store-type-specific reenrollment wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8STLSSecr/Management.cs | Added store-type-specific management wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8STLSSecr/Inventory.cs | Added store-type-specific inventory wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8STLSSecr/Discovery.cs | Added store-type-specific discovery wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SSecret/Reenrollment.cs | Added store-type-specific reenrollment wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SSecret/Management.cs | Added store-type-specific management wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SSecret/Inventory.cs | Added store-type-specific inventory wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SSecret/Discovery.cs | Added store-type-specific discovery wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SPKCS12/Reenrollment.cs | Added store-type-specific reenrollment wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SPKCS12/Management.cs | Added store-type-specific management wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SPKCS12/Inventory.cs | Added store-type-specific inventory wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SPKCS12/Discovery.cs | Added store-type-specific discovery wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SNS/Reenrollment.cs | Added store-type-specific reenrollment wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SNS/Management.cs | Added store-type-specific management wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SNS/Inventory.cs | Added store-type-specific inventory wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SNS/Discovery.cs | Added store-type-specific discovery wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SJKS/Reenrollment.cs | Added store-type-specific reenrollment wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SJKS/Management.cs | Added store-type-specific management wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SJKS/Inventory.cs | Added store-type-specific inventory wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SJKS/Discovery.cs | Added store-type-specific discovery wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SCluster/Reenrollment.cs | Added store-type-specific reenrollment wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SCluster/Management.cs | Added store-type-specific management wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SCluster/Inventory.cs | Added store-type-specific inventory wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SCluster/Discovery.cs | Added store-type-specific discovery wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SCert/Inventory.cs | Added CSR store-type inventory wrapper. |
| kubernetes-orchestrator-extension/Jobs/StoreTypes/K8SCert/Discovery.cs | Added CSR store-type discovery wrapper. |
| kubernetes-orchestrator-extension/Jobs/Reenrollment.cs | Removed legacy global reenrollment job implementation. |
| kubernetes-orchestrator-extension/Jobs/PAMUtilities.cs | Added additional PAM resolution outcome logging. |
| kubernetes-orchestrator-extension/Jobs/Base/ReenrollmentBase.cs | Added reenrollment base class for store-type-specific reenrollment jobs. |
| kubernetes-orchestrator-extension/Jobs/Base/K8SJobBase.cs | Added consolidated base job scaffolding + handler initialization helpers. |
| kubernetes-orchestrator-extension/Jobs/Base/DiscoveryBase.cs | Added discovery base implementation delegating to handlers. |
| kubernetes-orchestrator-extension/Handlers/SecretHandlerFactory.cs | Added factory mapping normalized secret types to handlers. |
| kubernetes-orchestrator-extension/Handlers/ISecretHandler.cs | Added unified handler interface + operation context contracts. |
| kubernetes-orchestrator-extension/Exceptions/StoreNotFoundException.cs | Added custom exception type. |
| kubernetes-orchestrator-extension/Exceptions/JkSisPkcs12Exception.cs | Added custom exception type for format mismatch. |
| kubernetes-orchestrator-extension/Exceptions/InvalidK8SSecretException.cs | Added custom exception type for invalid/missing fields. |
| kubernetes-orchestrator-extension/Clients/CertificateOperations.cs | Added wrapper client for certificate parsing/format operations. |
| kubernetes-orchestrator-extension.Tests/Unit/ReenrollmentTests.cs | Added unit tests validating reenrollment behavior for store types. |
| kubernetes-orchestrator-extension.Tests/Unit/Jobs/ExceptionTests.cs | Added unit tests for new exception classes. |
| kubernetes-orchestrator-extension.Tests/Unit/Clients/KubeCertificateManagerClientTests.cs | Added constructor/behavior tests for kube client parsing paths. |
| kubernetes-orchestrator-extension.Tests/Services/KeystoreOperationsTests.cs | Added tests for keystore helper operations. |
| kubernetes-orchestrator-extension.Tests/LoggingSafetyTests.cs | Updated logging safety tests for removed correlation id + stronger redaction assertion. |
| kubernetes-orchestrator-extension.Tests/Jobs/StorePropertiesParsingTests.cs | Updated imports for store properties parsing tests. |
| kubernetes-orchestrator-extension.Tests/Integration/K8SNSStoreIntegrationTests.cs | Updated integration tests to reuse cached cert generation and updated using. |
| kubernetes-orchestrator-extension.Tests/Integration/Collections/KubeClientCollection.cs | Added xUnit collection definition for integration fixture sharing. |
| Keyfactor.Orchestrators.K8S.sln | Removed TestConsole project reference. |
| docsource/k8spkcs12.md | Added Terraform module reference and example snippet. |
| docsource/k8sns.md | Added Terraform module reference and example snippet. |
| docsource/k8sjks.md | Added Terraform module reference and example snippet. |
| docsource/k8scluster.md | Added Terraform module reference and example snippet. |
| docsource/images/K8STLSSecr-advanced-store-type-dialog.svg | Added/updated generated documentation image asset. |
| docsource/images/K8SSecret-advanced-store-type-dialog.svg | Added/updated generated documentation image asset. |
| docsource/images/K8SPKCS12-advanced-store-type-dialog.svg | Added/updated generated documentation image asset. |
| docsource/images/K8SNS-advanced-store-type-dialog.svg | Added/updated generated documentation image asset. |
| docsource/images/K8SJKS-advanced-store-type-dialog.svg | Added/updated generated documentation image asset. |
| docsource/images/K8SCluster-advanced-store-type-dialog.svg | Added/updated generated documentation image asset. |
| docsource/images/K8SCert-custom-fields-store-type-dialog.svg | Added/updated generated documentation image asset. |
| docsource/images/K8SCert-advanced-store-type-dialog.svg | Added/updated generated documentation image asset. |
| docsource/content.md | Expanded Kubernetes API access/auth documentation + added Terraform section. |
| .github/workflows/unit-tests.yml | Updated action versions used in unit test workflow. |
| .github/workflows/test-doctool.yml | Added (commented-out) doctool test workflow scaffold. |
| .github/workflows/secret-scanning.yml | Updated action versions used in secret scanning workflow. |
| .github/workflows/sbom-generation.yml | Updated action versions and release action in SBOM workflow. |
| .github/workflows/pr-quality-gate.yml | Updated action versions used in PR quality gate workflow. |
| .github/workflows/license-compliance.yml | Updated action versions used in license compliance workflow. |
| .github/workflows/keyfactor-starter-workflow.yml | Expanded triggers/branches and annotated required vs optional secrets/vars. |
| .github/workflows/integration-tests.yml | Updated action versions used in integration test workflow. |
| .github/workflows/dotnet-security-scan.yml | Updated action versions used in security scan workflow. |
| .github/workflows/dependency-submission.yml | Updated action versions used in dependency submission workflow. |
| .github/workflows/dependency-review.yml | Updated checkout action version in dependency review workflow. |
| .github/workflows/code-quality.yml | Updated action versions used in code quality workflow. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+116
to
+125
| var passwordPath = jobCertificate.StorePasswordPath.Split("/"); | ||
| if (passwordPath.Length < 2) | ||
| { | ||
| throw new InvalidOperationException( | ||
| $"Invalid StorePasswordPath format: '{jobCertificate.StorePasswordPath}'. Expected format: 'namespace/secretname' or 'secretname/namespace'"); | ||
| } | ||
|
|
||
| var passwordNamespace = passwordPath.Length > 1 ? passwordPath[0] : "default"; | ||
| var passwordSecretName = passwordPath.Length > 1 ? passwordPath[1] : passwordPath[0]; | ||
|
|
Comment on lines
+34
to
+39
| variable "store_password" { | ||
| description = "The password for the JKS keystore. Required unless store_password_k8s_secret_path is set." | ||
| type = string | ||
| default = null | ||
| sensitive = true | ||
| } |
Comment on lines
+34
to
+39
| variable "store_password" { | ||
| description = "The password for the PKCS12 keystore. Required unless store_password_k8s_secret_path is set." | ||
| type = string | ||
| default = null | ||
| sensitive = true | ||
| } |
Comment on lines
+1
to
+6
| #!/bin/bash | ||
| # Store Type creation script using kfutil | ||
| # Generated by Doctool | ||
|
|
||
| set -e | ||
|
|
Comment on lines
+1
to
+3
| # Store Type creation script using kfutil | ||
| # Generated by Doctool | ||
|
|
github-actions
Bot
added
documentation
Contributor
Dependency ReviewThe following issues were found:
License Issues.github/workflows/integration-tests.yml
.github/workflows/unit-tests.yml
OpenSSF Scorecard
Scanned Files
|
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
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.
Merge release-2.0 to main - Automated PR