feat: add KMS signer support for Fulcio (SECURESIGN-4462) - #2184
Draft
kdacosta0 wants to merge 6 commits into
Draft
feat: add KMS signer support for Fulcio (SECURESIGN-4462)#2184kdacosta0 wants to merge 6 commits into
kdacosta0 wants to merge 6 commits into
Conversation
…ervedGeneration
Two bugs in the Trillian logsigner controller actions:
1. logsigner/service.go and logsigner/monitoring.go use
actions.ServerCondition ("LogServerAvailable") instead of
actions.SignerCondition ("LogSignerAvailable"). When the logsigner
service is created, it overwrites the logserver's condition to
"Creating", causing LogServerAvailable to oscillate between
Creating and Ready on every reconcile cycle.
2. All four deployment/service actions in logserver and logsigner
set conditions without ObservedGeneration, which is inconsistent
with the rollout check actions that do set it.
The combined effect prevents Trillian from reliably reaching Ready
state, which blocks the entire SecureSign dependency chain
(CTLog → TUF → SecureSign).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add signer-agnostic pod customization fields to FulcioSpec and CTlogSpec, enabling users to inject init containers, volumes, and volume mounts regardless of which signer backend is active. API changes: - InitContainerSpec in common.go — curated corev1.Container subset - InitContainers, Volumes, VolumeMounts on FulcioSpec and CTlogSpec Controller changes: - User-defined resources applied in shared deployment path before signer type branching - CTLog operator-managed "keys" volume set after user volumes so operator always wins on reserved names - Shared helpers: HasVolume, EnsureVolumeDefaultMode, ReconcileInitContainers - Fulcio ensureCommonDeployment extracted for shared scaffolding Housekeeping: - Remove dead HasMountPath function - v1alpha1 conversion preserves new fields via MarshalData annotations - Roundtrip fuzzer coverage for new fields Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tion Move user pod-resource reconciliation (InitContainers, Volumes, VolumeMounts) from inside ensureCommonDeployment/ensureDeployment into a standalone deployment.PodResources() ensure function in the CreateOrUpdate chain. This makes the user pod-resource logic signer-agnostic and composable — the same PodResources ensure function can be reused by Rekor, TSA, and TUF without reimplementation per component. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Sachin Sampras M <sampras343@gmail.com>
Add Auth field on FulcioSigner and CTlogSigner for injecting authentication credentials (env vars and secret mounts) into the main server container, regardless of signer type. - Auth *Auth on FulcioSigner and CTlogSigner - ContainerAuth called unconditionally — cleans up stale auth volumes/mounts when auth is removed - Auth volume renamed to "signer-auth" with legacy "auth" volume cleanup on every reconcile for seamless upgrade - EnsureVolumeDefaultMode prevents infinite reconciliation loop - SecureSign.SetDefaults() calls Signer.SetDefaults() for explicit type defaulting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add Auth field on FulcioSigner and CTlogSigner for injecting authentication credentials (env vars and secret mounts) into the main server container, regardless of signer type. - Auth *Auth on FulcioSigner and CTlogSigner - ContainerAuth called unconditionally — cleans up stale auth volumes/mounts when auth is removed - Auth volume renamed to "signer-auth" with legacy "auth" volume cleanup on every reconcile for seamless upgrade - EnsureVolumeDefaultMode prevents infinite reconciliation loop - SecureSign.SetDefaults() calls Signer.SetDefaults() for explicit type defaulting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR Summary by QodoAdd KMS-backed Fulcio signer support (plus pod customization + auth fixes)
AI Description
Diagram
High-Level Assessment
Files changed (35)
|
v1 API types (KMS, Auth structs), v1alpha1 conversion with mutual exclusion guard, generate_signer skip for KMS, resolve_kms_signer action, deployment args for kmsca, FIPS validation for KMS. Cluster-tested: conversion roundtrip verified on OpenShift. Not yet PR-ready — needs cleanup and mutual exclusion cluster test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kdacosta0
force-pushed
the
fulcio-kms-conversion-wip
branch
from
August 5, 2026 13:59
0d4f8a6 to
fc9e208
Compare
Code Review by Qodo
1. Fulcio-cert source not reset
|
![qodo-for-securesign[bot]](https://avatars.githubusercontent.com/in/3596402?s=60&v=4){kind=small}
Comment on lines
+240
to
+243
| cert := kubernetes.FindVolumeByNameOrCreate(&template.Spec, "fulcio-cert") | ||
| if cert.Projected == nil { | ||
| cert.Projected = &core.ProjectedVolumeSource{} | ||
| } |
There was a problem hiding this comment.
3. Fulcio-cert source not reset 🐞 Bug ≡ Correctness
Fulcio deployment reconciliation applies user-defined Volumes first, then mutates only cert.Projected for the operator-managed "fulcio-cert" volume without resetting VolumeSource. If a user supplies a "fulcio-cert" volume with another source (e.g., EmptyDir), this can produce an invalid volume spec and block Deployment creation.
Agent Prompt
### Issue description
`fulcio-cert` is operator-managed but user volumes are reconciled first; later code only initializes/edits `cert.Projected` and `cert.Projected.Sources` without clearing any pre-existing `VolumeSource` fields.
### Issue Context
`ReconcileUserPodResources` overwrites `VolumeSource` for user-supplied volumes by name, so a user can unintentionally (or intentionally) provide a conflicting source under the reserved name `fulcio-cert`.
### Fix Focus Areas
- internal/controller/fulcio/actions/deployment.go[146-149]
- internal/controller/fulcio/actions/deployment.go[230-260]
- internal/controller/fulcio/actions/deployment.go[284-321]
- internal/utils/kubernetes/ensure/pod_spec.go[107-115]
### Suggested fix
- In both KMS and file signer branches, replace:
- `if cert.Projected == nil { cert.Projected = ... }` and direct `cert.Projected.Sources = ...`
- With:
- `cert.VolumeSource = core.VolumeSource{Projected: &core.ProjectedVolumeSource{Sources: ...}}`
- Then call `ensure.EnsureVolumeDefaultMode(cert)`.
- Add/extend a test similar to CTlog’s operator precedence test to cover `fulcio-cert` collisions.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
kdacosta0
marked this pull request as draft
August 5, 2026 14:24
Member
Author
|
Waiting for #2181 to merge |
This was referenced Aug 6, 2026
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
2 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.
What changed
Adds KMS signer support for Fulcio — operators can now configure Fulcio to use a remote KMS key (AWS, GCP, Azure, HashiCorp Vault) instead of a local file-based ECDSA key.
API
FulcioSigner.Kmsfield (type: kms+kms.keyResourceURI)fileandkmscertificateChainRefrequired for KMS (Fulcio server hard-requires--kms-cert-chain-path)Controller
resolve_kms_signeraction — resolves cert chain secret, setsCertConditiongenerate_signerskipped for KMS (IsEnabledguard:type == file || type == "")--ca=kmsca+--kms-resource+--kms-cert-chain-pathargs, cert volume with CA only (no private key)Conversion (v1 ↔ v1alpha1)
File, file branch nilsKmsAuthrestored unconditionally (orthogonal to signer type)Why
SECURESIGN-4462 — customers need remote key management for Fulcio CA signing keys.
Tested
eu-north-1) on OpenShift —tlog entry created with index: 0Stack
Depends on: #2181 (Auth support)
Parallel with: #2182 (PKCS#11) — no code dependency, either can merge first.