Skip to content

refactor: Major refactor#75

Open
spbsoluble wants to merge 37 commits intorelease-2.0from
break/major_refactor
Open

refactor: Major refactor#75
spbsoluble wants to merge 37 commits intorelease-2.0from
break/major_refactor

Conversation

@spbsoluble
Copy link
Copy Markdown
Contributor

No description provided.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 8, 2026

Dependency Review

The following issues were found:
  • ✅ 0 vulnerable package(s)
  • ✅ 0 package(s) with incompatible licenses
  • ✅ 0 package(s) with invalid SPDX license definitions
  • ⚠️ 2 package(s) with unknown licenses.
See the Details below.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 538affb.
Ensure 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/keyfactor-starter-workflow.yml

PackageVersionLicenseIssue Type
Keyfactor/actions/.github/workflows/starter.yml6.*.*NullUnknown License

.github/workflows/test-doctool.yml

PackageVersionLicenseIssue Type
Keyfactor/actions/.github/workflows/generate-readme.ymlfeature/dotnet-doctoolNullUnknown License

OpenSSF Scorecard

PackageVersionScoreDetails
actions/Keyfactor/actions/.github/workflows/starter.yml 6.*.* UnknownUnknown
actions/Keyfactor/actions/.github/workflows/generate-readme.yml feature/dotnet-doctool UnknownUnknown
nuget/Microsoft.NET.Test.Sdk 17.12.0 🟢 4.5
Details
CheckScoreReason
Code-Review🟢 8Found 18/22 approved changesets -- score normalized to 8
Maintained🟢 1030 commit(s) and 28 issue activity found in the last 90 days -- score normalized to 10
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Packaging⚠️ -1packaging workflow not detected
Security-Policy🟢 10security policy file detected
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
License🟢 10license file detected
Binary-Artifacts⚠️ 0binaries present in source code
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ 2branch protection is not maximal on development and all release branches
Fuzzing⚠️ 0project is not fuzzed
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
nuget/Moq 4.20.72 UnknownUnknown
nuget/coverlet.collector 6.0.4 🟢 5.1
Details
CheckScoreReason
Code-Review⚠️ 0Found 0/25 approved changesets -- score normalized to 0
Maintained🟢 1030 commit(s) and 21 issue activity found in the last 90 days -- score normalized to 10
Packaging⚠️ -1packaging workflow not detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Token-Permissions🟢 9detected GitHub workflow tokens with excessive permissions
Binary-Artifacts🟢 7binaries present in source code
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
Signed-Releases⚠️ 0Project has not signed or included provenance with any releases.
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
Security-Policy⚠️ 0security policy file not detected
SAST🟢 10SAST tool is run on all commits
nuget/xunit 2.9.3 🟢 4.3
Details
CheckScoreReason
Code-Review⚠️ 1Found 3/30 approved changesets -- score normalized to 1
Maintained🟢 1030 commit(s) and 25 issue activity found in the last 90 days -- score normalized to 10
Packaging⚠️ -1packaging workflow not detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Binary-Artifacts🟢 10no binaries found in the repo
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
License🟢 9license file detected
Fuzzing⚠️ 0project is not fuzzed
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
Security-Policy⚠️ 0security policy file not detected
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
nuget/xunit.runner.visualstudio 3.0.2 UnknownUnknown
nuget/System.Drawing.Common 8.0.0 🟢 6.8
Details
CheckScoreReason
Maintained🟢 1030 commit(s) and 19 issue activity found in the last 90 days -- score normalized to 10
Packaging⚠️ -1packaging workflow not detected
Security-Policy🟢 10security policy file detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Code-Review🟢 10all changesets reviewed
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
License🟢 10license file detected
Binary-Artifacts🟢 10no binaries found in the repo
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
Pinned-Dependencies🟢 8dependency not pinned by hash detected -- score normalized to 8
Fuzzing⚠️ 0project is not fuzzed
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0

Scanned Files

  • .github/workflows/keyfactor-starter-workflow.yml
  • .github/workflows/test-doctool.yml
  • TestConsole/TestConsole.csproj
  • kubernetes-orchestrator-extension.Tests/Keyfactor.Orchestrators.K8S.Tests.csproj
  • kubernetes-orchestrator-extension/Keyfactor.Orchestrators.K8S.csproj

@github-actions github-actions bot added documentation Improvements or additions to documentation dependencies Pull requests that update a dependency file ci/cd needs-review tests labels Mar 8, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 8, 2026

Integration Test Results (K8s v1.29.0)

215 tests   215 ✅  2m 58s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 4f309a8.

♻️ This comment has been updated with latest results.

@spbsoluble spbsoluble changed the title break: Major refactor refactor: Major refactor Mar 8, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 8, 2026

PR Quality Signoff Summary

Security Checks

Check Status Details
Secrets Scan No secrets detected (TruffleHog)
Dependency Review No high/critical vulnerabilities in dependencies
Vulnerability Scan No vulnerable packages detected
License Compliance License check skipped (unknown project type)
Commit PII Check No PII detected in commit history

Code Quality

Check Status Details
Code Quality 8 warnings, 0 errors
Code Formatting Code formatting is correct
Prohibited Keywords Warning: 10 TODO/FIXME markers found

PR Standards

Check Status Details
PR Title PR title follows Conventional Commits format
PR Size Large PR: 56374 lines changed; Many files: 228 files changed; PR exceeds 3000 lines - consider splitting; PR exceeds 50 files - consider splitting
CHANGELOG Updated CHANGELOG.md was updated
Manifest Validation integration-manifest.json is valid
Breaking Changes No breaking changes detected

Self-Review Checklist

Before requesting review, please confirm:

  • I have reviewed my own code
  • No secrets are being logged
  • CHANGELOG.md has been updated (if applicable)
  • Commit history contains no PII or customer names
  • Documentation has been updated (if applicable)
  • Tests have been added/updated for new functionality

DevOps Workflow Reminders

  • DevOps epic created (pattern: <integration_name> vX.Y.Z)
  • Bug fixes and features attached to epic
  • Target Version field filled out
  • PR link attached to epic
  • Description filled out (changelog entry is sufficient)
  • "Waiting SignOff" tag set (required for signoff)

🎉 All automated quality checks passed!

Generated by Keyfactor Actions v6 PR Quality Checks

- Development.md: note net10.0 SDK support, update test counts to ~1337
  unit tests, add inspect-jks/inspect-pkcs12 keystore inspection targets
- ARCHITECTURE.md: replace removed KeystoreManager with KeystoreOperations
  in data flow diagrams; add JobCertificateParser to Services layer
- Development.md: note net10.0 SDK support, update test counts to ~1337
  unit tests, add inspect-jks/inspect-pkcs12 keystore inspection targets
- ARCHITECTURE.md: replace removed KeystoreManager with KeystoreOperations
  in data flow diagrams; add JobCertificateParser to Services layer
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 8, 2026

PR Quality Signoff Summary

Security Checks

Check Status Details
Secrets Scan No secrets detected (TruffleHog)
Dependency Review No high/critical vulnerabilities in dependencies
Vulnerability Scan No vulnerable packages detected
License Compliance License check skipped (unknown project type)
Commit PII Check No PII detected in commit history

Code Quality

Check Status Details
Code Quality 8 warnings, 0 errors
Code Formatting Code formatting is correct
Prohibited Keywords Warning: 10 TODO/FIXME markers found

PR Standards

Check Status Details
PR Title PR title follows Conventional Commits format
PR Size Large PR: 55766 lines changed; Many files: 226 files changed; PR exceeds 3000 lines - consider splitting; PR exceeds 50 files - consider splitting
CHANGELOG Updated CHANGELOG.md was updated
Manifest Validation integration-manifest.json is valid
Breaking Changes No breaking changes detected

Self-Review Checklist

Before requesting review, please confirm:

  • I have reviewed my own code
  • No secrets are being logged
  • CHANGELOG.md has been updated (if applicable)
  • Commit history contains no PII or customer names
  • Documentation has been updated (if applicable)
  • Tests have been added/updated for new functionality

DevOps Workflow Reminders

  • DevOps epic created (pattern: <integration_name> vX.Y.Z)
  • Bug fixes and features attached to epic
  • Target Version field filled out
  • PR link attached to epic
  • Description filled out (changelog entry is sufficient)
  • "Waiting SignOff" tag set (required for signoff)

🎉 All automated quality checks passed!

Generated by Keyfactor Actions v6 PR Quality Checks

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 8, 2026

PR Quality Signoff Summary

Security Checks

Check Status Details
Secrets Scan No secrets detected (TruffleHog)
Dependency Review No high/critical vulnerabilities in dependencies
Vulnerability Scan No vulnerable packages detected
License Compliance License check skipped (unknown project type)
Commit PII Check No PII detected in commit history

Code Quality

Check Status Details
Code Quality 8 warnings, 0 errors
Code Formatting Code formatting is correct
Prohibited Keywords Warning: 10 TODO/FIXME markers found

PR Standards

Check Status Details
PR Title PR title follows Conventional Commits format
PR Size Large PR: 55737 lines changed; Many files: 226 files changed; PR exceeds 3000 lines - consider splitting; PR exceeds 50 files - consider splitting
CHANGELOG Updated CHANGELOG.md was updated
Manifest Validation integration-manifest.json is valid
Breaking Changes No breaking changes detected

Self-Review Checklist

Before requesting review, please confirm:

  • I have reviewed my own code
  • No secrets are being logged
  • CHANGELOG.md has been updated (if applicable)
  • Commit history contains no PII or customer names
  • Documentation has been updated (if applicable)
  • Tests have been added/updated for new functionality

DevOps Workflow Reminders

  • DevOps epic created (pattern: <integration_name> vX.Y.Z)
  • Bug fixes and features attached to epic
  • Target Version field filled out
  • PR link attached to epic
  • Description filled out (changelog entry is sufficient)
  • "Waiting SignOff" tag set (required for signoff)

🎉 All automated quality checks passed!

Generated by Keyfactor Actions v6 PR Quality Checks

spbsoluble and others added 14 commits March 8, 2026 14:58
…st.json

- Add generate_scripts.py: reads integration-manifest.json and regenerates
  all four store type scripts (kfutil + curl bash, kfutil + REST PowerShell)
- All scripts now cover all 7 store types (was 3: K8SCert, K8SSecret, K8STLSSecr)
- Remove KubeSvcCreds; set ServerRequired=true everywhere
- Add OAuth support to curl and REST scripts: KEYFACTOR_AUTH_ACCESS_TOKEN,
  KEYFACTOR_AUTH_CLIENT_ID/SECRET/TOKEN_URL (client credentials), and
  Basic auth fallback (KEYFACTOR_USERNAME/PASSWORD/DOMAIN)
- Add scripts/store_types/README.md documenting auth methods and regeneration
- Roll update_store_types.sh into Makefile as store-types-gen-scripts,
  store-types-create, store-types-update, and store-types-split targets
- store-types-gen-scripts prefers doctool if installed, falls back to python3

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The generator logic lives in doctool/manifest/storetype_scripts.py.
The standalone script was a duplicate that would drift.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ctor.PKI directly

Remove GetThumbprint, GetSubjectCN, GetSerialNumber, and ConvertToPem from
CertificateUtilities — all four were single-line delegates to Keyfactor.PKI
APIs (BouncyCastleX509Extensions and PemUtilities) with no added logic.
All call sites now use the canonical Shared-PKI extension methods and
PemUtilities.DERToPEM directly.

Tests for the deleted methods are removed; remaining tests that used these
methods as helpers are updated in-place. 1335/1335 tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace dynamic parameters with concrete types across StoreConfigurationParser,
JobCertificateParser, and JobBase; replace switch expressions in SecretHandlerFactory
with dictionary lookup tables. All dynamic CallSite branches that inflated cyclomatic
complexity counts are gone.

Methods affected:
- StoreConfigurationParser: Parse, ApplyKeystoreDefaults, GetPropertyOrDefault<T>,
  ParseBoolProperty — dynamic → IDictionary<string,object>
- JobCertificateParser.Parse + helpers — dynamic config → ManagementJobConfiguration
- JobBase: InitializeProperties, InitializeStoreCore, ApplyKeystoreDefaultsFromParser,
  InitJobCertificate — dynamic → typed
- SecretHandlerFactory: Create, HasHandler, GetHandlerTypeName — switch → Dictionary

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…GetKubeClient

GetCertificateContext had 0% branch coverage (CRAP 156). New tests cover all
branches: null CertificateEntry, null/empty chain array, chain with no explicit
ChainPem (auto-computed), chain with explicit ChainPem, PEM/key field copy.

KubeCertificateManagerClient.GetKubeClient (CRAP 35) is exercised through the
constructor: token-auth kubeconfig, useSSL=false, base64-encoded kubeconfig,
invalid CA cert data (triggers fallback branch), null/empty/non-JSON inputs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
KubeconfigParser.Parse() always throws on error — it never returns null.
The else-if ("should never happen") and else (BuildConfigFromConfigFile)
branches were dead code guarded by the k8sConfiguration != null check.

Removing them drops CC from 14 → 6 and CRAP from 137 → 26.8, clearing
the last CRAP > 30 hotspot in the codebase. Also removes the assembly-
path retrieval lines that only existed to support the dead file-path branch.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…andler no-network paths

ExceptionTests (9 tests):
- All 3 constructors for JkSisPkcs12Exception, InvalidK8SSecretException,
  and StoreNotFoundException — brings each class from 0/33% to 100% line coverage

CertificateChainExtractorTests (10 tests):
- Null/whitespace string inputs (lines 48-50)
- DER fallback path when PEM chain fails (lines 68-81)
- Null/empty byte[] inputs (lines 93-95)
- ExtractAndAppendUnique null/empty bytes (lines 141-142)
- ExtractFromSecretData with null secretData (lines 167-169)
- ca.crt chain append with addedCount > 0 log (line 191)

HandlerNoNetworkTests (26 tests):
- CertificateSecretHandler: AllowedKeys, SecretTypeName, SupportsManagement,
  HasPrivateKey, HandleAdd/HandleRemove/CreateEmptyStore throw NotSupportedException
- ClusterSecretHandler: HasPrivateKey, CreateEmptyStore, short-alias ArgumentException,
  unsupported inner type NotSupportedException
- NamespaceSecretHandler: same pattern as Cluster

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…vements

Document GetKubeClient dead code removal (CC 14→6, CRAP 137→26.8) and
the three new unit test files (exceptions, CertificateChainExtractor,
handler no-network paths). Update test count to ~1397.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…h flow

Document JkSisPkcs12Exception and InvalidK8SSecretException in the Error
Handling section (were previously omitted). Note that GetKubeClient delegates
exclusively to KubeconfigParser with no file-path fallback. Clarify the
Exceptions/ directory/namespace split.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Correct test counts: 457/603 → 1397 unit + ~200 integration
- Update test structure tree to reflect actual Unit/ subdirectory layout
- Fix unit test template to use CachedCertificateProvider (not direct generation)
- Fix CertificateTestHelper section to distinguish cache vs low-level helpers
- Correct unit test runtime estimate: 3-5 min → ~17 min
- Remove stale UNIT_TEST_COMPLETION_SUMMARY.md reference
- Fix TESTING_QUICKSTART.md: MAKEFILE_TEST_TARGETS.md → MAKEFILE_GUIDE.md
- Remove hardcoded local path from quickstart
- Replace raw dotnet commands with make targets throughout
- Update CI duration estimates and coverage numbers (90.5% line / 81.6% branch)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…DME.md

Add detailed test tables for five sections missing from the catalog:
- Unit/Handlers/HandlerNoNetworkTests.cs (26 tests — handler properties,
  NotSupportedException, and alias-parsing ArgumentException paths)
- Unit/Services/CertificateChainExtractorTests.cs (11 tests — null/empty,
  DER fallback, byte arrays, ExtractAndAppendUnique, ExtractFromSecretData)
- Unit/Services/JobCertificateParserTests.cs (stub entry)
- Unit/Jobs/K8SJobCertificateTests.cs (8 tests — GetCertificateContext chain
  handling and PEM copy)
- Unit/Jobs/ExceptionTests.cs (9 tests — all 3 constructors of each custom
  exception class)

Update test counts: 1,156 unit / 1,371 total → 1,397 unit / ~1,600 total,
with coverage numbers (90.5% line / 81.6% branch).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Standalone workflow_dispatch-only job that calls
generate-readme.yml@feature/dotnet-doctool for isolated testing of
the new .NET doctool action before it lands in the main pipeline.

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {}

Copilot Autofix

AI 1 day ago

To fix the problem, explicitly define permissions for the workflow so the GITHUB_TOKEN is limited to the least privilege required. Since this file only orchestrates a call to a reusable workflow and does not itself perform repository‑modifying operations, a safe, minimal default is typically contents: read. If the called reusable workflow needs broader permissions, it should declare them in its own permissions block; this file should not assume write permissions unless strictly necessary.

The single best change with minimal impact is to add a root‑level permissions block (applies to all jobs without their own permissions) directly after the name declaration and before the on: section. Concretely, in .github/workflows/keyfactor-starter-workflow.yml, insert:

permissions:
  contents: read

on new lines 2–3, shifting the rest of the file down. No imports or additional methods are required because this is a GitHub Actions workflow YAML file, and permissions is a standard top‑level key.

Suggested changeset 1
.github/workflows/keyfactor-starter-workflow.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/keyfactor-starter-workflow.yml b/.github/workflows/keyfactor-starter-workflow.yml
--- a/.github/workflows/keyfactor-starter-workflow.yml
+++ b/.github/workflows/keyfactor-starter-workflow.yml
@@ -1,4 +1,6 @@
 name: Keyfactor Bootstrap Workflow
+permissions:
+  contents: read
 
 on:
   workflow_dispatch:
EOF
@@ -1,4 +1,6 @@
name: Keyfactor Bootstrap Workflow
permissions:
contents: read

on:
workflow_dispatch:
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation needs-review tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant