Release v0.13.0 (minor) - #586
Open
github-actions[bot] wants to merge 50 commits into
Open
github-actions[bot] wants to merge 50 commits into
github-actions[bot] wants to merge 50 commits into
Conversation
The autosign policy only checked certname patterns, exact CSR-attribute values, and DNS SANs. It ignored privileged authorization extensions and non-DNS SANs, and "any: true" short-circuited before any of these checks. A node whose CSR matched a policy (or any CSR when an any:true policy existed) could embed pp_cli_auth=true and, since the CA auth.conf grants admin to any cert carrying that extension, obtain a CA-admin certificate. IP/URI SANs could likewise impersonate services because only DNS SANs were validated. Introduce a fail-closed guard plane on SigningPolicy, enforced for every policy including any:true: - extensions: privileged authorization extensions (the 1.3.6.1.4.1.34380.1.3 arc: pp_cli_auth, pp_authorization, pp_auth_token) are denied unless explicitly allow-listed. The gate is OID-prefix based, so unknown authorization-arc OIDs cannot be allow-listed and are always denied. Trusted-fact extensions (...1.1.* arc) are not gated. - ipAltNames / uriAltNames / emailAltNames: fail-closed allowlists for the remaining SAN types, mirroring dnsAltNames. IPs use CIDR containment; URIs/emails use a wildcard matcher whose '*' spans '/' and '@'. The guard plane is evaluated before the any:true short-circuit so no policy can implicitly waive escalation protection. The renderer emits the guard fields for every policy (including any:true), and the webhook validates CIDR entries and known extension names. CA-level allow-* flags are unchanged; enforcement now lives in the autosign binary and policy. Refs #506
The certname-matching field was the only one that did not name what it matches (unlike dnsAltNames/ipAltNames/extensions/csrAttributes), so a reader could not tell 'pattern.allow' matched the certname. Rename it to 'certnames' for a self-describing, consistent spec. Breaking change on the v1alpha1 API, batched with the SigningPolicy rework in this branch. Also render the new guard fields (ipAltNames/uriAltNames/emailAltNames/ extensions) in the openvox-stack Helm SigningPolicy template so they are usable via the chart. Refs #506
…y-san-extension-allowlists
The guard plane closed the pp_cli_auth path but not the one the CA
auth.conf opens beside it. builtinAuthRules grants CA admin to two things:
the extension, and the operator-signing certname. The certname is derived
as {ca}-operator and is therefore predictable, and the guard never looks at
it - it is checked only in the match plane, which any: true skips.
The operator now renders that name into the policy as reservedCertnames,
and the binary refuses it before any policy is consulted, so no policy can
hand it out. The name is derived from one helper used by the auth.conf
rendering, the operator-signing certificate and the reservation alike; a
drift between them would reserve one name while granting admin to another.
Second, the subject is now bound to the requested certname. puppetserver
takes the certname from the request path and passes it as the argument,
while the CN lives in the CSR subject, and nothing compared the two. The
policy would then judge one name while the certificate is issued carrying
another. Whether puppetserver rejects that first is an upstream question
this no longer depends on.
Scope, honestly: with the operator's own certificate in place the name is
taken, so puppetserver would refuse a second CSR for it anyway. The
reservation covers the window before that certificate exists and the state
after it is removed. That is also why the e2e step asserts the rendered
policy rather than the CA's signed directory - the latter would pass
without the reservation and prove nothing.
…d the guards The rename from pattern to certnames had reached the reference page but not the prose elsewhere: the overview table, two places in the architecture concept and the feature snippet still named a field that no longer exists, and binaries.md described the old evaluation model. Also documents what the reference did not cover yet: the two checks that precede every policy. Reserved certnames, because the CA auth.conf grants admin rights to the operator-signing certname, and subject binding, because puppetserver passes the certname as an argument while the CN lives in the CSR subject. Both apply to any: true, which is the part worth stating explicitly - the field reads like an escape hatch and is not one. The chart values gain an example of the fail-closed allowlists, since the existing ones only showed any and csrAttributes. helm-docs produces no change from it. Checked: no occurrence of pattern as a field name remains outside the word operator pattern, and the cross-reference anchor resolves.
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Three classes of error that make a copied example fail rather than mislead
slowly.
spec.code is a list of sources, not a single one. Ten examples showed it as
an object; the API server answers 'unknown field spec.code.image' and
refuses the manifest. Verified against the CRD with kubectl apply
--dry-run=server: the list form is accepted, the object form is not.
The image tags 8.12.1 and 8.13.0 appeared at ten places and cannot exist.
images/openvox-versions.yaml states that the image name encodes the OpenVox
major while the tag carries the operator release version; the published tags
are 0.12.0, develop and latest.
Three label selectors in the troubleshooting and CA import guides do not
exist in labels.go: app.kubernetes.io/instance is never set,
app.kubernetes.io/name is openvox rather than openvox-server, and the CA
label is certificateauthority without a hyphen. A selector that matches
nothing is worse than no command at all, because it reads like an answer.
Also corrects two resource names: the Config ConfigMap is {name}-config,
and the CA setup Job is {ca}-ca-setup. The remaining names in the reference
tables were checked against the code and are correct.
The Error phase is documented for Config, Server and Database as 'reconciliation failed'. It exists in the CRD enum but no code path assigns it: a failing reconcile leaves the phase where it was and reports the reason in a condition. Someone waiting for phase Error waits forever, which is the worst kind of wrong - it looks like a working diagnosis. Replaced with the condition to watch, and the jsonpath was checked against a live object. The Gateway API concept showed serverRef on a Pool. That field exists neither on the CRD nor in the chart; a Server names its pools through poolRefs and the Pool selects those pods. Removed, and the direction of the relationship spelled out, since getting it backwards is the natural guess. Added what the code does but nothing described: renewal reuses the existing private key deliberately, because the CA renews for the same public key. So renewal extends validity without rotating the key, and replacing a key needs a new Certificate. Checked and left alone: the Renewing phase description is already accurate, and no page claims renewal generates a new key. The review report asserted both; neither held up.
The operator writes autosign = /usr/local/bin/openvox-autosign as soon as a CertificateAuthority exists, regardless of whether any SigningPolicy does, and evaluatePolicies returns false for an empty list. An empty policy list is therefore deny-all rather than off, which is the state every fresh install starts in. Nothing surfaces it: the servers run, the Config reports Running, and every agent sits in --waitforcert until it gives up. Documented in the quickstart and in the agent section of the troubleshooting guide, with the manual puppetserver ca sign path as the immediate way out. Also separates two cases the troubleshooting guide conflated. A Certificate resource stuck in Pending is almost never a policy problem: for an internal CA the operator signs its own Certificates over the CA API with the operator signing certificate, so autosign is not involved. That entry now lists the causes that do apply - CA not ready, signing certificate not yet available, certname already claimed, external CA - each with a command that reads the condition rather than the phase. Policies govern agents, and that is where the note now lives.
Compared every documented field against the generated CRDs, in both directions. observedGeneration exists on all nine status types since #549 but appeared on two reference pages. Added to the remaining seven, with the sentence that makes it useful: a value below metadata.generation means the rest of the status has not caught up yet. Certificate gained signedSpecHash and effectiveDNSAltNames in the status table. Both drive behaviour a reader needs to predict - the first decides when a certificate is re-signed, the second says which alt names it is actually issued for once Pools contribute. Server gained readOnlyRootFilesystem, the per-Server override added in #575. The ImageSpec table still listed defaults that #549 and #576 removed: repository ghcr.io/slauger/openvox-server-8, tag latest, pullPolicy IfNotPresent. It contradicted the prose directly beneath it. Corrected, and the reason spelled out - a nested default is applied whether or not the parent object was given, so a defaulted field can never mean inherit - plus the pullSecrets rule that a Server list replaces rather than extends. Checked and found correct: every other documented default matches its CRD, and no reference page names a field that does not exist.
The user documentation never showed an agent connecting. puppet agent, --waitforcert, ca_server and puppetserver ca sign appeared nowhere, so the moment the product exists for was the one step a reader had to work out alone. The guide covers what an agent needs, where to point it, running one inside the cluster and reaching one from outside, signing by hand when no policy matches, revoking, and how to tell from the server side that it worked. Two things it states that are easy to get wrong. ca_server is only needed once the CA and the compilers are in separate pools - with the default poolRefs [ca, server] one address serves both, which is why the setting appears nowhere in this repository. And puppet agent --test returns 2 when it applied changes, so a Job that treats non-zero as failure reports a successful run as broken. It also names what the agent image is: openvox-agent-<major> is built by CI and not by the release workflow, so it carries only the develop tag - no latest, no version. Documented as the test artifact it is, rather than implying it is a supported way to run agents. Every claim was checked: the Service default is ClusterIP, the Deployment carries the Server name, crlRefreshInterval defaults to 5m, and the image tags are what ghcr actually holds.
Extracted all 42 manifests from the documentation and ran them against a real API server with kubectl apply --dry-run=server. One was rejected for a reason a reader could not guess: the NodeClassifier page abbreviated the image block as 'image: ...', and the resulting error talks about a type mismatch rather than the ellipsis. Replaced with the real block; all 42 are accepted now. The Database concept claimed the Config controller waits when the Database is not Running. It does not: renderPuppetDBConf returns a puppetdb.conf without server_urls and the servers start regardless. Together with soft_write_failure = true, which the operator sets in every path, a server in that state compiles catalogs normally while reports and exported resources go nowhere and nothing is raised. That resolves itself during bring-up and only bites when the Database never becomes ready, so the symptom is an empty PuppetDB rather than an error. Monitoring now states that CertificateAuthority and Certificate write the same expiry metric with the same labels and nothing distinguishing them, so the .*-ca matcher in the CA alert is a naming convention and not a guarantee. Added an absence rule for the CRL metric: a staleness alert cannot fire on a series that was never created, which is exactly the case when the operator never got far enough to refresh one.
README and the feature list promise 'Heap size calculated from memory limits (90%) - no manual -Xmx tuning needed'. The controller does contain that calculation, but it is unreachable: ServerSpec.JavaArgs carries a CRD default, so the field is never empty and the first branch of resolveJavaArgs always wins. Every Server runs with -Xms512m -Xmx1024m no matter how much memory it is given. Replaced the claim with what actually applies, and noted it on the field in the Server reference with a link to #592, where the fix is tracked. Once the default is removed the derivation works and the wording can go back. Third instance of the same pattern after #550 and #576: a CRD default takes away the empty state a fallback depends on.
docs: correct and complete the documentation against the code
…n-allowlists feat: per-policy SAN and authorization-extension allowlists for autosign
resolveJavaArgs sizes the JVM heap at 90 percent of the pod's memory limit when javaArgs is unset. The CRD default made that unreachable: the field was never empty, the explicit branch always won, and every Server ran with -Xms512m -Xmx1024m no matter how much memory it was given. A Server with 8Gi used 1Gi of it. Third instance of one mistake after #550 and #576. A nested or plain default removes the empty state a fallback depends on, so the default belongs in the chart where it is visible, not in the CRD where it disables code. The existing unit tests did not catch this and could not: they call resolveJavaArgs directly and therefore pass whether or not the field can ever be empty in a real cluster. Added the round-trip through the API server instead, which fails against the old CRD with the materialised value in the message. Restores the auto-tuning statement in the README and the feature list, which I had corrected to match the broken behaviour rather than the intended one. Closes #592
Every SigningPolicy edit rolled the CA pod. The policy Secret was mounted with SubPath, which the kubelet never refreshes, so the operator compensated with a hash annotation on the pod template. Because the CA Deployment uses the Recreate strategy, that meant a short outage - no signing, no CRL - for a change that alters no running state. On a VM-based Puppet, editing autosign configuration has no serving impact at all. Nothing downstream required the restart: openvox-autosign is executed per CSR and reads the file each time. Only the mount did. Mount the Secret as a directory instead, the way the CRL Secret already is for the same reason, and pass the file to the binary with --config. The hash annotation is gone, so a policy edit no longer touches the pod template. The trade-off is timing: a change now takes effect within the kubelet sync period, up to about a minute, rather than immediately after a restart. For a rule deciding which CSRs get signed, that is the better end of the trade. The former test asserted the annotation exists; it now asserts the opposite and that the mount carries no SubPath, since a SubPath would silently reintroduce the whole problem. The rendered puppet.conf is checked for the config path too - a wrong one denies every CSR and would otherwise only surface in an end-to-end run. Closes #588
github-actions
Bot
changed the title
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
The metrics endpoint served plaintext HTTP to anything that could reach the pod. It carries no key material, but it lists every certificate the operator manages and when each expires, which is a ready-made inventory for picking a moment. metrics.secure turns on secure serving plus the controller-runtime filter, which verifies each scrape against the API server with a TokenReview and a SubjectAccessReview. The chart adds a metrics-reader ClusterRole granting get on /metrics; nothing is bound to it by default, so a scraper needs an explicit grant. The default stays off. Turning it on switches the endpoint to HTTPS and makes a token mandatory, and Prometheus reports the target as down without saying why. That belongs in a release note rather than in a patch. Two details worth stating. The auth-delegation rules live in their own ClusterRole. TokenReview and SubjectAccessReview are cluster-scoped, so the namespace-scoped Role the chart otherwise renders cannot grant them - the filter would reject every scrape in namespace mode. The issuer chain moved out of the webhook template into its own file. It is now rendered when either the webhook or the metrics endpoint asks for a certificate, so secure metrics no longer require webhooks to be enabled. For the certificate itself: cert-manager by default, a supplied Secret via metrics.tls.certSecret, or a self-signed one generated at startup. The protection comes from the filter rather than the certificate, which is why the self-signed path is usable at all - the bundled ServiceMonitor verifies against the CA when cert-manager issues it and skips verification otherwise. Refs #510
The documentation had never been checked against the code until the review on 2026-09-03. It found manifests the API server rejects, three label selectors that match nothing, a phase no controller sets, image tags that cannot exist and a defaults table contradicting the prose beneath it. Every one of them looked plausible on the page, which is why none had been noticed. AGENT.md described docs/ as design docs and architecture diagrams. It is 35 pages of user-facing reference covering every field, phase, condition and metric - closer to the API than to design notes, and it drifts the same way an API would. Adds the rule that a change altering observable behaviour updates the documentation in the same commit, a table of what to touch for each kind of change, and the checks that would have caught the drift: manifests through kubectl apply --dry-run=server, selectors against labels.go, documented fields against the generated CRDs. The commands are runnable rather than sketched. Also asks for the honest version when a claim cannot be verified - saying so in the text beats leaving it to look confirmed.
feat(metrics): authenticated HTTPS serving behind an opt-in (#510)
docs: make updating the documentation part of the change
The build stage patched openvoxserver-ca with sed to survive rootless containers: it commented out the symlink_to_old_cadir call in setup.rb and every FileUtils.chown in file_system.rb, because both raised Errno::EPERM without CAP_CHOWN. Upstream fixed this properly in openvoxserver-ca 3.2.0: PR #33 routes all ownership changes through an ensure_ownership helper that is a no-op when not running as root, and forcibly_symlink uses the same helper, so the cadir compatibility symlink now works rootless as well (it lands in the ssl emptyDir and is harmless). The openvox-server 8.15.2 tarball ships openvoxserver-ca 3.2.1, which includes the fix. The patch had also been degrading silently: since 3.2.0 the chown sed was editing the inside of ensure_ownership, and both find|sed commands suppressed all errors, so a non-matching pattern would never have failed the build. OpenVoxProject/openvoxserver-ca#33
fix(image): drop obsolete openvoxserver-ca rootless patches
golangci-lint ran with the bare v2 standard preset so far. The new config keeps the standard linters and adds errorlint, gosec, misspell, revive and unconvert, plus gofmt and goimports as enforced formatters. Generated files are excluded via the lax mode, gosec is disabled for test files and the revive exported-comment rule is off to avoid noise from kubebuilder scaffolding.
Fix the issues the new golangci-lint config surfaced: - read config files via filepath.Clean to satisfy gosec G304 - tighten ENC cache permissions to 0750/0600 (G301/G306) - use errors.As instead of a type assertion in isNotFound (errorlint) - give the mock server a ReadHeaderTimeout (G114) and annotate its debug log lines, which print CLI flag values and request data by design (G706) - clamp intstrInt before the int32 conversion (G115) - drop unnecessary conversions around gwapiv1.PortNumber (unconvert)
The golangci-lint defaults cap repeated findings (max-same-issues: 3), so the first CI run hid additional G304 sites that only surfaced after the first batch was fixed. Disable both caps so runs are exhaustive, and wrap the remaining variable file reads in filepath.Clean.
Aligns with what cluster-api, cert-manager, CloudNativePG, external-secrets and the kubebuilder scaffolding enable: bodyclose, copyloopvar, dogsled, durationcheck, gocritic, intrange, loggercheck, modernize, nakedret, nilerr, nolintlint, prealloc and unparam, plus asciicheck/bidichk to back the unicode-lint CI check on the Go level. importas enforces the usual k8s import aliases (no-unaliased), which in particular settles the split between plain and apierrors imports of k8s.io/apimachinery/pkg/api/errors.
Mostly mechanical, largely via golangci-lint run --fix: - unify k8s.io/apimachinery/pkg/api/errors imports on apierrors (importas); the unaliased form shadowed the stdlib errors name - modernize rewrites: new(expr) instead of local ptr helpers (the int64Ptr/boolPtr/int32Ptr/fsGroupChangePolicyPtr helpers and their tests are gone), strings.SplitSeq, and friends - rewrite if-else chains as switches, close fire-and-forget response bodies in the mock tests, preallocate slices where the capacity is known (gocritic, bodyclose, prealloc) - drop test-helper parameters that only ever received one value and make the two no-panic tests assert through t (unparam) - keep buildPodSecurityContext's uid parameter with an explained nolint: all workloads currently run as uid 1001, but the parameter stays symmetric with group/fsGroup
ci: expand golangci-lint configuration to the common operator linter set
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
chore(deps): update dependency puppetlabs-stdlib to v10.1.0
fix(deps): update container base images
fix(deps): update module golang.org/x/vuln to v1.8.0
fix(deps): update openvox 8 versions
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
ubi9/ubi 9.8-1789348643 -> 9.8-1789552280 ubi9/ubi-minimal 9.8-1789349365 -> 9.8-1789546276 Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
GO-2026-6348 (heap memory exhaustion via HTTP/2 DATA frame fragmentation) is reachable from our code through the controller-runtime event recorder, so govulncheck reports it as called and the go/vulncheck job fails. The advisory landed after develop last ran CI, which is why develop still shows green while every open PR fails the job. grpc is an indirect dependency, so Renovate does not bump it on its own. Pinning it to the fixed release clears the finding; go mod tidy pulls cel.dev/expr v0.25.2 along with it. govulncheck now reports no called vulnerabilities.
chore(deps): combined dependency updates and grpc vulnerability fix
…9.8-1789646010 (#612) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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
1 participant
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.
Features
Fixes
Refactoring
Documentation
Build and CI
Chores
Files changed
Release prediction
Predicted release:
v0.12.0->v0.13.0(minor)2 feature(s) 15 fix(es)
Source:
develop| Target:main| Trigger: Successful CI on develop