diff --git a/.github/workflows/_conforma-validate.yaml b/.github/workflows/_conforma-validate.yaml index e2ee867c..cbd3257a 100644 --- a/.github/workflows/_conforma-validate.yaml +++ b/.github/workflows/_conforma-validate.yaml @@ -27,7 +27,7 @@ jobs: packages: read env: # renovate: datasource=github-releases depName=conforma/cli - EC_VERSION: "0.10.2" + EC_VERSION: "0.10.17" steps: - name: Checkout uses: actions/checkout@v7 diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..bc68a23d --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,80 @@ +version: "2" + +run: + timeout: 5m + +linters: + default: standard + enable: + - asciicheck + - bidichk + - bodyclose + - copyloopvar + - dogsled + - durationcheck + - errorlint + - gocritic + - gosec + - importas + - intrange + - loggercheck + - misspell + - modernize + - nakedret + - nilerr + - nolintlint + - prealloc + - revive + - unconvert + - unparam + settings: + importas: + no-unaliased: true + alias: + - pkg: k8s.io/api/core/v1 + alias: corev1 + - pkg: k8s.io/api/apps/v1 + alias: appsv1 + - pkg: k8s.io/api/autoscaling/v2 + alias: autoscalingv2 + - pkg: k8s.io/api/batch/v1 + alias: batchv1 + - pkg: k8s.io/api/discovery/v1 + alias: discoveryv1 + - pkg: k8s.io/api/networking/v1 + alias: networkingv1 + - pkg: k8s.io/api/policy/v1 + alias: policyv1 + - pkg: k8s.io/api/rbac/v1 + alias: rbacv1 + - pkg: k8s.io/apimachinery/pkg/api/errors + alias: apierrors + - pkg: k8s.io/apimachinery/pkg/apis/meta/v1 + alias: metav1 + - pkg: sigs.k8s.io/gateway-api/apis/v1 + alias: gwapiv1 + nolintlint: + require-specific: true + revive: + rules: + - name: exported + disabled: true + exclusions: + generated: lax + rules: + # Test helpers construct fixtures with hardcoded values and do not need + # the same hardening as production code. + - path: _test\.go + linters: + - gosec + +formatters: + enable: + - gofmt + - goimports + +issues: + # Report everything instead of capping repeated findings (the defaults hide + # issues beyond 3 of the same kind, which makes CI runs non-exhaustive). + max-issues-per-linter: 0 + max-same-issues: 0 diff --git a/AGENT.md b/AGENT.md index 71c44e31..282f1698 100644 --- a/AGENT.md +++ b/AGENT.md @@ -96,11 +96,58 @@ docs/ Documentation ## Documentation -- `README.md` is the primary user-facing doc - update when adding user-visible features +`docs/` is user-facing reference, not design notes: 35 pages covering every CRD +field, phase, condition and metric. Treat it as part of the API. + +- `README.md` is the entry point - update when adding user-visible features - `CONTRIBUTING.md` covers developer setup and workflow -- `docs/` contains design docs and architecture diagrams - No CHANGELOG - release notes are auto-generated from commit messages by semantic-release +### Documentation is part of the change, not a follow-up + +**Every change that alters observable behaviour updates the documentation in +the same commit.** Not in a later pass: drift found afterwards is drift that +shipped, and a reader cannot tell a stale sentence from a correct one. + +What to check, by kind of change: + +| Change | Update | +|---|---| +| CRD field added, removed or renamed | the field table in `docs/reference/.md` | +| A `+kubebuilder:default` added or removed | the table **and** the prose around it - these have contradicted each other | +| New or changed condition, phase, reason | the conditions table; do not document a phase the controller never sets | +| New or removed metric | `docs/guides/monitoring.md`, including an alert rule where one makes sense | +| Behaviour a reader would predict wrongly | the relevant `docs/concepts/` page | +| New chart value | `values.yaml` comments, then regenerate `README.md` and `values.schema.json` | +| Resource name, label or selector | anything in `docs/` that names it - troubleshooting commands go stale silently | + +### Verify rather than assume + +The documentation had never been checked against the code before 2026-09-03, +and the review that followed found examples the API server rejects, three label +selectors that match nothing, a phase no controller sets and image tags that +cannot exist. Each was plausible on the page. + +Cheap checks worth running when touching docs: + +```bash +# Manifests in the docs must be accepted by a real API server +kubectl apply --dry-run=server -f .yaml + +# Selectors must exist in internal/controller/labels.go +grep -rn "kubectl.*-l " docs/ + +# Fields documented for a kind must exist in its CRD, and vice versa +ruby -ryaml -e 'd=YAML.load_file(ARGV[0]); + acc=[]; w=lambda{|n| next unless n.is_a?(Hash); + (n["properties"]||{}).each{|k,v| acc< 0 { - if policy.DNSAltNames == nil { + if policy.DNSAltNames == nil || !matchDNSAltNames(policy.DNSAltNames, csr.DNSNames) { + return false + } + } + if len(csr.IPAddresses) > 0 { + if policy.IPAltNames == nil || !allIPInCIDRs(policy.IPAltNames.Allow, csr.IPAddresses) { return false } - if !matchDNSAltNames(policy.DNSAltNames, csr.DNSNames) { + } + if len(csr.URIs) > 0 { + if policy.URIAltNames == nil || !allWildcardMatch(policy.URIAltNames.Allow, uriStrings(csr.URIs)) { + return false + } + } + if len(csr.EmailAddresses) > 0 { + if policy.EmailAltNames == nil || !allWildcardMatch(policy.EmailAltNames.Allow, csr.EmailAddresses) { return false } } + return true +} - // A policy with no conditions matches nothing - return hasCondition +// allIPInCIDRs reports whether every IP is contained in at least one allowed +// CIDR range. Invalid CIDR entries never match (fail-closed on the allow side). +func allIPInCIDRs(cidrs []string, ips []net.IP) bool { + nets := make([]*net.IPNet, 0, len(cidrs)) + for _, c := range cidrs { + if _, n, err := net.ParseCIDR(c); err == nil { + nets = append(nets, n) + } + } + for _, ip := range ips { + matched := false + for _, n := range nets { + if n.Contains(ip) { + matched = true + break + } + } + if !matched { + return false + } + } + return true +} + +// allWildcardMatch reports whether every name matches at least one wildcard +// pattern, where '*' spans any characters (including '/' and '@'). +func allWildcardMatch(patterns, names []string) bool { + for _, name := range names { + matched := false + for _, p := range patterns { + if wildcardMatch(p, name) { + matched = true + break + } + } + if !matched { + return false + } + } + return true +} + +// wildcardMatch matches name against a pattern where '*' matches any sequence of +// characters; all other characters match literally. Unlike filepath.Match, '*' +// is not stopped by '/' or '@', which suits URIs and email addresses. +func wildcardMatch(pattern, name string) bool { + parts := strings.Split(pattern, "*") + if len(parts) == 1 { + return pattern == name + } + if !strings.HasPrefix(name, parts[0]) { + return false + } + name = name[len(parts[0]):] + for _, seg := range parts[1 : len(parts)-1] { + idx := strings.Index(name, seg) + if idx < 0 { + return false + } + name = name[idx+len(seg):] + } + return strings.HasSuffix(name, parts[len(parts)-1]) +} + +// uriStrings renders CSR URI SANs to their string form. +func uriStrings(uris []*url.URL) []string { + out := make([]string, len(uris)) + for i, u := range uris { + out[i] = u.String() + } + return out } // matchPattern checks if certname matches any of the allow patterns. diff --git a/cmd/autosign/policy_test.go b/cmd/autosign/policy_test.go index e768f312..85dd31e0 100644 --- a/cmd/autosign/policy_test.go +++ b/cmd/autosign/policy_test.go @@ -8,6 +8,8 @@ import ( "crypto/x509/pkix" "encoding/asn1" "encoding/pem" + "net" + "net/url" "os" "path/filepath" "testing" @@ -15,6 +17,40 @@ import ( "github.com/slauger/openvox-operator/internal/puppet" ) +// generateCSRWithSANs creates a test CSR carrying the given SAN sets. +func generateCSRWithSANs(t *testing.T, cn string, dns []string, ips []net.IP, uris []*url.URL, emails []string) *x509.CertificateRequest { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generating key: %v", err) + } + template := &x509.CertificateRequest{ + Subject: pkix.Name{CommonName: cn}, + DNSNames: dns, + IPAddresses: ips, + URIs: uris, + EmailAddresses: emails, + } + csrDER, err := x509.CreateCertificateRequest(rand.Reader, template, key) + if err != nil { + t.Fatalf("creating CSR: %v", err) + } + csr, err := x509.ParseCertificateRequest(csrDER) + if err != nil { + t.Fatalf("parsing CSR: %v", err) + } + return csr +} + +func mustURL(t *testing.T, raw string) *url.URL { + t.Helper() + u, err := url.Parse(raw) + if err != nil { + t.Fatalf("parsing URL %q: %v", raw, err) + } + return u +} + // generateCSR creates a test CSR with optional SANs and extensions. func generateCSR(t *testing.T, cn string, dnsNames []string, extensions []pkix.Extension) *x509.CertificateRequest { t.Helper() @@ -73,7 +109,7 @@ func TestLoadPolicyConfig(t *testing.T) { - name: allow-all any: true - name: pattern-match - pattern: + certnames: allow: - "*.example.com" ` @@ -91,7 +127,7 @@ func TestLoadPolicyConfig(t *testing.T) { if !cfg.Policies[0].Any { t.Error("expected first policy to have any=true") } - if cfg.Policies[1].Pattern == nil { + if cfg.Policies[1].Certnames == nil { t.Error("expected second policy to have pattern") } } @@ -164,8 +200,8 @@ func TestEvaluatePolicies_NoPolicies(t *testing.T) { func TestEvaluatePolicy_PatternMatch(t *testing.T) { policy := Policy{ - Name: "pattern", - Pattern: &PatternConf{Allow: []string{"*.example.com"}}, + Name: "pattern", + Certnames: &PatternConf{Allow: []string{"*.example.com"}}, } csr := generateCSR(t, "node1.example.com", nil, nil) @@ -180,8 +216,8 @@ func TestEvaluatePolicy_PatternMatch(t *testing.T) { func TestEvaluatePolicy_PatternMultiple(t *testing.T) { policy := Policy{ - Name: "multi-pattern", - Pattern: &PatternConf{Allow: []string{"*.prod.com", "*.staging.com"}}, + Name: "multi-pattern", + Certnames: &PatternConf{Allow: []string{"*.prod.com", "*.staging.com"}}, } csr := generateCSR(t, "node1.staging.com", nil, nil) @@ -205,8 +241,8 @@ func TestEvaluatePolicy_CSRAttributes(t *testing.T) { csr := generateCSR(t, "node1", nil, []pkix.Extension{ext}) policy := Policy{ - Name: "env-check", - Pattern: &PatternConf{Allow: []string{"*"}}, + Name: "env-check", + Certnames: &PatternConf{Allow: []string{"*"}}, CSRAttributes: []CSRAttributeConf{ {Name: "pp_environment", Value: "production"}, }, @@ -226,8 +262,8 @@ func TestEvaluatePolicy_CSRAttributeNotPresent(t *testing.T) { csr := generateCSR(t, "node1", nil, nil) policy := Policy{ - Name: "env-check", - Pattern: &PatternConf{Allow: []string{"*"}}, + Name: "env-check", + Certnames: &PatternConf{Allow: []string{"*"}}, CSRAttributes: []CSRAttributeConf{ {Name: "pp_environment", Value: "production"}, }, @@ -244,7 +280,7 @@ func TestEvaluatePolicy_DNSAltNames(t *testing.T) { // Policy allows the SANs policy := Policy{ Name: "with-sans", - Pattern: &PatternConf{Allow: []string{"puppet"}}, + Certnames: &PatternConf{Allow: []string{"puppet"}}, DNSAltNames: &PatternConf{Allow: []string{"*.example.com", "*.local"}}, } if !evaluatePolicy(policy, "puppet", csr) { @@ -263,8 +299,8 @@ func TestEvaluatePolicy_DNSAltNamesNotAllowed(t *testing.T) { // Policy has no dnsAltNames field but CSR has SANs -> deny policy := Policy{ - Name: "no-sans", - Pattern: &PatternConf{Allow: []string{"*"}}, + Name: "no-sans", + Certnames: &PatternConf{Allow: []string{"*"}}, } if evaluatePolicy(policy, "node1", csr) { t.Error("expected CSR with SANs but no SAN policy to deny") @@ -277,8 +313,8 @@ func TestEvaluatePolicy_ANDLogic(t *testing.T) { csr := generateCSR(t, "web1.prod.com", nil, []pkix.Extension{ext}) policy := Policy{ - Name: "and-logic", - Pattern: &PatternConf{Allow: []string{"*.prod.com"}}, + Name: "and-logic", + Certnames: &PatternConf{Allow: []string{"*.prod.com"}}, CSRAttributes: []CSRAttributeConf{ {Name: "pp_role", Value: "webserver"}, }, @@ -301,8 +337,8 @@ func TestEvaluatePolicies_ORLogic(t *testing.T) { cfg := &PolicyConfig{ Policies: []Policy{ - {Name: "prod-only", Pattern: &PatternConf{Allow: []string{"*.prod.com"}}}, - {Name: "staging-only", Pattern: &PatternConf{Allow: []string{"*.staging.com"}}}, + {Name: "prod-only", Certnames: &PatternConf{Allow: []string{"*.prod.com"}}}, + {Name: "staging-only", Certnames: &PatternConf{Allow: []string{"*.staging.com"}}}, }, } @@ -440,3 +476,203 @@ func TestGlobMatch_InvalidPattern(t *testing.T) { t.Error("expected false for invalid glob pattern") } } + +func ppCliAuthExt(t *testing.T) []pkix.Extension { + t.Helper() + oid, ok := puppet.OIDByName("pp_cli_auth") + if !ok { + t.Fatal("pp_cli_auth OID not found") + } + return []pkix.Extension{makeExtension(t, oid, "true")} +} + +// A privileged authorization extension must be denied even by an any:true policy +// unless the policy explicitly allows it. +func TestGuardExtensions_AnyTrueStillGated(t *testing.T) { + csr := generateCSR(t, "node.example.com", nil, ppCliAuthExt(t)) + + deny := Policy{Name: "bootstrap", Any: true} + if evaluatePolicy(deny, "node.example.com", csr) { + t.Error("any:true must not sign a CSR carrying pp_cli_auth without extensions.allow") + } + + allow := Policy{Name: "bootstrap", Any: true, Extensions: &PatternConf{Allow: []string{"pp_cli_auth"}}} + if !evaluatePolicy(allow, "node.example.com", csr) { + t.Error("any:true with extensions.allow [pp_cli_auth] should sign") + } +} + +// A certname-matching policy without extensions.allow must deny a pp_cli_auth CSR. +func TestGuardExtensions_PatternPolicyDenies(t *testing.T) { + csr := generateCSR(t, "worker-1", nil, ppCliAuthExt(t)) + p := Policy{Name: "workers", Certnames: &PatternConf{Allow: []string{"worker-*"}}} + if evaluatePolicy(p, "worker-1", csr) { + t.Error("pattern policy without extensions.allow must deny pp_cli_auth CSR") + } +} + +// Trusted-fact extensions (...1.1.* arc) are not gated. +func TestGuardExtensions_TrustedFactNotGated(t *testing.T) { + oid, _ := puppet.OIDByName("pp_role") + csr := generateCSR(t, "worker-1", nil, []pkix.Extension{makeExtension(t, oid, "web")}) + p := Policy{Name: "workers", Certnames: &PatternConf{Allow: []string{"worker-*"}}} + if !evaluatePolicy(p, "worker-1", csr) { + t.Error("trusted-fact extension pp_role must not be gated") + } +} + +// An authorization-arc OID with no known name cannot be allow-listed and is denied. +func TestGuardExtensions_UnknownAuthzOIDDenied(t *testing.T) { + unknown := asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 34380, 1, 3, 999} + csr := generateCSR(t, "node", nil, []pkix.Extension{makeExtension(t, unknown, "x")}) + p := Policy{Name: "any", Any: true, Extensions: &PatternConf{Allow: []string{"pp_cli_auth"}}} + if evaluatePolicy(p, "node", csr) { + t.Error("unknown authorization-arc OID must be denied, not allow-listable") + } +} + +func TestGuardSANs_IPCIDR(t *testing.T) { + csr := generateCSRWithSANs(t, "svc", nil, []net.IP{net.ParseIP("10.0.5.4")}, nil, nil) + + allow := Policy{Name: "svc", Any: true, IPAltNames: &PatternConf{Allow: []string{"10.0.0.0/16"}}} + if !evaluatePolicy(allow, "svc", csr) { + t.Error("IP within allowed CIDR should sign") + } + + outside := Policy{Name: "svc", Any: true, IPAltNames: &PatternConf{Allow: []string{"192.168.0.0/16"}}} + if evaluatePolicy(outside, "svc", csr) { + t.Error("IP outside allowed CIDR must be denied") + } + + // IP SAN present but no ipAltNames → deny, even for any:true. + none := Policy{Name: "svc", Any: true} + if evaluatePolicy(none, "svc", csr) { + t.Error("IP SAN without ipAltNames must be denied") + } +} + +func TestGuardSANs_URIWildcard(t *testing.T) { + csr := generateCSRWithSANs(t, "svc", nil, nil, + []*url.URL{mustURL(t, "spiffe://example.com/workload/db")}, nil) + + allow := Policy{Name: "svc", Any: true, URIAltNames: &PatternConf{Allow: []string{"spiffe://example.com/workload/*"}}} + if !evaluatePolicy(allow, "svc", csr) { + t.Error("URI matching wildcard (across '/') should sign") + } + + deny := Policy{Name: "svc", Any: true, URIAltNames: &PatternConf{Allow: []string{"spiffe://other.com/*"}}} + if evaluatePolicy(deny, "svc", csr) { + t.Error("URI not matching allow must be denied") + } +} + +func TestGuardSANs_EmailWildcard(t *testing.T) { + csr := generateCSRWithSANs(t, "svc", nil, nil, nil, []string{"ops@example.com"}) + allow := Policy{Name: "svc", Any: true, EmailAltNames: &PatternConf{Allow: []string{"*@example.com"}}} + if !evaluatePolicy(allow, "svc", csr) { + t.Error("email matching *@example.com should sign") + } + deny := Policy{Name: "svc", Any: true, EmailAltNames: &PatternConf{Allow: []string{"*@other.com"}}} + if evaluatePolicy(deny, "svc", csr) { + t.Error("email not matching allow must be denied") + } +} + +func TestWildcardMatch(t *testing.T) { + cases := []struct { + pattern, name string + want bool + }{ + {"spiffe://example.com/workload/*", "spiffe://example.com/workload/db", true}, + {"spiffe://example.com/*", "spiffe://example.com/a/b/c", true}, + {"*@example.com", "ops@example.com", true}, + {"*@example.com", "ops@other.com", false}, + {"exact", "exact", true}, + {"exact", "other", false}, + {"*", "anything/at@all", true}, + {"a*b*c", "axxbyyc", true}, + {"a*b*c", "axxc", false}, + } + for _, c := range cases { + if got := wildcardMatch(c.pattern, c.name); got != c.want { + t.Errorf("wildcardMatch(%q, %q) = %v, want %v", c.pattern, c.name, got, c.want) + } + } +} + +// --- reserved certnames and subject binding --- + +// TestReservedCertname_DeniedEvenByAnyPolicy is the point of the reservation. +// The CA auth.conf grants admin rights by certname as well as by extension, so +// a policy must never be able to hand that name to an agent - and any: true +// would otherwise approve unconditionally once the guards pass, and the guards +// inspect the CSR, not the name. +func TestReservedCertname_DeniedEvenByAnyPolicy(t *testing.T) { + cfg := &PolicyConfig{ + ReservedCertnames: []string{"production-ca-operator"}, + Policies: []Policy{{Name: "open", Any: true}}, + } + csr := generateCSR(t, "production-ca-operator", nil, nil) + + if evaluatePolicies(cfg, "production-ca-operator", csr) { + t.Error("a reserved certname must never be signed, not even under any: true") + } +} + +// TestReservedCertname_CaseInsensitive closes the obvious way around it, since +// Puppet lowercases certnames. +func TestReservedCertname_CaseInsensitive(t *testing.T) { + cfg := &PolicyConfig{ + ReservedCertnames: []string{"production-ca-operator"}, + Policies: []Policy{{Name: "open", Any: true}}, + } + csr := generateCSR(t, "Production-CA-Operator", nil, nil) + + if evaluatePolicies(cfg, "Production-CA-Operator", csr) { + t.Error("the reservation must not depend on capitalisation") + } +} + +// TestReservedCertname_LeavesOtherNamesAlone bounds the rule. +func TestReservedCertname_LeavesOtherNamesAlone(t *testing.T) { + cfg := &PolicyConfig{ + ReservedCertnames: []string{"production-ca-operator"}, + Policies: []Policy{{Name: "open", Any: true}}, + } + csr := generateCSR(t, "web01.example.com", nil, nil) + + if !evaluatePolicies(cfg, "web01.example.com", csr) { + t.Error("an ordinary agent must still be signed") + } +} + +// TestSubjectMustMatchCertname covers the second half of the same escalation. +// puppetserver takes the certname from the request path and passes it as the +// argument; the CN lives in the CSR subject. Judging the argument while signing +// the document means the policy can approve a name the certificate will not +// carry - here a harmless one, while the CSR asks for the reserved name. +func TestSubjectMustMatchCertname(t *testing.T) { + cfg := &PolicyConfig{ + ReservedCertnames: []string{"production-ca-operator"}, + Policies: []Policy{{Name: "open", Any: true}}, + } + csr := generateCSR(t, "production-ca-operator", nil, nil) + + if evaluatePolicies(cfg, "harmless-node", csr) { + t.Error("a CSR whose subject differs from the requested certname must be refused") + } +} + +// TestSubjectMatchesCertname_AcceptsTheNormalCase makes sure the check does not +// reject everyday enrolment. +func TestSubjectMatchesCertname_AcceptsTheNormalCase(t *testing.T) { + cfg := &PolicyConfig{Policies: []Policy{{Name: "open", Any: true}}} + csr := generateCSR(t, "web01.example.com", nil, nil) + + if !evaluatePolicies(cfg, "web01.example.com", csr) { + t.Error("matching subject and certname must pass") + } + if !evaluatePolicies(cfg, "WEB01.example.com", csr) { + t.Error("the comparison must ignore capitalisation") + } +} diff --git a/cmd/enc/classifier.go b/cmd/enc/classifier.go index 9b497a2b..93a0b65d 100644 --- a/cmd/enc/classifier.go +++ b/cmd/enc/classifier.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "crypto/x509" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -66,9 +67,9 @@ type SSLConfig struct { // ENCResult represents a Puppet ENC response. type ENCResult struct { - Classes interface{} `yaml:"classes" json:"classes"` - Parameters map[string]interface{} `yaml:"parameters,omitempty" json:"parameters,omitempty"` - Environment string `yaml:"environment,omitempty" json:"environment,omitempty"` + Classes any `yaml:"classes" json:"classes"` + Parameters map[string]any `yaml:"parameters,omitempty" json:"parameters,omitempty"` + Environment string `yaml:"environment,omitempty" json:"environment,omitempty"` } // notFoundError indicates a 404 response. @@ -77,13 +78,13 @@ type notFoundError struct{ msg string } func (e *notFoundError) Error() string { return e.msg } func isNotFound(err error) bool { - _, ok := err.(*notFoundError) - return ok + var nfe *notFoundError + return errors.As(err, &nfe) } // loadENCConfig reads and parses the ENC config YAML file. func loadENCConfig(path string) (*ENCConfig, error) { - data, err := os.ReadFile(path) + data, err := os.ReadFile(filepath.Clean(path)) if err != nil { return nil, fmt.Errorf("reading ENC config: %w", err) } @@ -174,7 +175,7 @@ func buildRequestBody(bodyType, certname string) (string, error) { return string(data), err case "facts": facts := loadFacts(certname) - data, err := json.Marshal(map[string]interface{}{ + data, err := json.Marshal(map[string]any{ "certname": certname, "facts": facts, }) @@ -185,22 +186,22 @@ func buildRequestBody(bodyType, certname string) (string, error) { } // loadFacts reads Puppet facts for a certname from the YAML facts cache. -func loadFacts(certname string) map[string]interface{} { +func loadFacts(certname string) map[string]any { safeName := filepath.Base(certname) factsPath := filepath.Join("/opt/puppetlabs/server/data/puppetserver/yaml/facts", safeName+".yaml") - data, err := os.ReadFile(factsPath) + data, err := os.ReadFile(filepath.Clean(factsPath)) if err != nil { - return map[string]interface{}{} + return map[string]any{} } var factsFile struct { - Values map[string]interface{} `yaml:"values"` + Values map[string]any `yaml:"values"` } if err := yaml.Unmarshal(data, &factsFile); err != nil { - return map[string]interface{}{} + return map[string]any{} } if factsFile.Values == nil { - return map[string]interface{}{} + return map[string]any{} } return factsFile.Values } @@ -224,7 +225,7 @@ func normalizeResponse(body []byte, format string) (string, error) { // Ensure classes is never nil if result.Classes == nil { - result.Classes = map[string]interface{}{} + result.Classes = map[string]any{} } out, err := yaml.Marshal(&result) @@ -240,7 +241,7 @@ func buildHTTPClient(cfg *ENCConfig) (*http.Client, error) { // Load CA certificate for server verification if cfg.SSL.CAFile != "" { - caCert, err := os.ReadFile(cfg.SSL.CAFile) + caCert, err := os.ReadFile(filepath.Clean(cfg.SSL.CAFile)) if err != nil { return nil, fmt.Errorf("reading CA cert: %w", err) } @@ -270,19 +271,19 @@ func buildHTTPClient(cfg *ENCConfig) (*http.Client, error) { // saveCache writes the classification result to a cache file. func saveCache(dir, certname, data string) error { - if err := os.MkdirAll(dir, 0755); err != nil { + if err := os.MkdirAll(dir, 0750); err != nil { return err } safeName := filepath.Base(certname) path := filepath.Join(dir, safeName+".yaml") - return os.WriteFile(path, []byte(data), 0644) + return os.WriteFile(path, []byte(data), 0600) } // readCache reads a cached classification result. func readCache(dir, certname string) (string, error) { safeName := filepath.Base(certname) path := filepath.Join(dir, safeName+".yaml") - data, err := os.ReadFile(path) + data, err := os.ReadFile(filepath.Clean(path)) if err != nil { return "", err } diff --git a/cmd/enc/classifier_test.go b/cmd/enc/classifier_test.go index 080624ed..1f8092e4 100644 --- a/cmd/enc/classifier_test.go +++ b/cmd/enc/classifier_test.go @@ -151,8 +151,8 @@ func TestClassify_GET_JSON(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(ENCResult{ - Classes: map[string]interface{}{"nginx": nil}, - Parameters: map[string]interface{}{"role": "proxy"}, + Classes: map[string]any{"nginx": nil}, + Parameters: map[string]any{"role": "proxy"}, Environment: "staging", }) })) @@ -733,7 +733,7 @@ func TestBuildRequestBody_Facts(t *testing.T) { t.Fatalf("buildRequestBody: %v", err) } - var data map[string]interface{} + var data map[string]any if err := json.Unmarshal([]byte(body), &data); err != nil { t.Fatalf("parsing body: %v", err) } @@ -760,9 +760,10 @@ func TestNormalizeResponse_InvalidJSON(t *testing.T) { } func TestNormalizeResponse_InvalidYAML(t *testing.T) { + // The YAML parser is lenient, so this may or may not error; the test + // ensures normalizeResponse does not panic on garbage input. _, err := normalizeResponse([]byte(":\n :\n - :\n invalid"), "yaml") - // YAML parser is lenient, so this may or may not error; just ensure no panic - _ = err + t.Logf("normalizeResponse on invalid YAML returned err=%v", err) } func TestLoadENCConfig_InvalidYAML(t *testing.T) { diff --git a/cmd/main.go b/cmd/main.go index 839f5a5e..2fd828ae 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/metrics/filters" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" ctrlwebhook "sigs.k8s.io/controller-runtime/pkg/webhook" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -35,6 +36,8 @@ func init() { func main() { var metricsAddr string + var secureMetrics bool + var metricsCertDir string var probeAddr string var enableLeaderElection bool var enableWebhooks bool @@ -42,6 +45,14 @@ func main() { var watchNamespace string flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") + flag.BoolVar(&secureMetrics, "metrics-secure", false, + "Serve metrics over HTTPS and require an authenticated, authorized client. "+ + "Off by default so existing scrape configurations keep working; this is "+ + "expected to become the default in a later release.") + flag.StringVar(&metricsCertDir, "metrics-cert-dir", "", + "Directory holding tls.crt and tls.key for the metrics endpoint. "+ + "Empty means controller-runtime generates a self-signed certificate at startup. "+ + "Only used with --metrics-secure.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ @@ -59,7 +70,7 @@ func main() { mgrOptions := ctrl.Options{ Scheme: scheme, - Metrics: metricsserver.Options{BindAddress: metricsAddr}, + Metrics: metricsOptions(metricsAddr, secureMetrics, metricsCertDir), HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "openvox-operator.voxpupuli.org", @@ -193,3 +204,39 @@ func main() { os.Exit(1) } } + +// metricsOptions builds the metrics server configuration. +// +// The endpoint used to be plaintext HTTP reachable by anything in the cluster. +// It carries no key material, but it does list every certificate the operator +// manages and when each expires - a ready-made inventory for picking a moment. +// +// With secure serving the filter authenticates each request against the API +// server (TokenReview) and checks authorization (SubjectAccessReview), so a +// scraper needs an explicit RBAC grant on /metrics. +// +// The certificate is self-signed and regenerated on restart. That is +// deliberate: the protection comes from the filter, not from the certificate, +// and requiring cert-manager for an in-cluster endpoint that exposes no +// secrets would buy little for the dependency. Scrapers verify it with +// insecureSkipVerify, or point at a cert-manager issued one. +func metricsOptions(addr string, secure bool, certDir string) metricsserver.Options { + if !secure { + return metricsserver.Options{BindAddress: addr} + } + opts := metricsserver.Options{ + BindAddress: addr, + SecureServing: true, + FilterProvider: filters.WithAuthenticationAndAuthorization, + } + // Without a directory controller-runtime generates a self-signed + // certificate at startup and regenerates it on every restart, which forces + // scrapers to skip verification. A directory - from cert-manager or + // supplied by hand - gives them something to verify against. + if certDir != "" { + opts.CertDir = certDir + opts.CertName = "tls.crt" + opts.KeyName = "tls.key" + } + return opts +} diff --git a/cmd/main_test.go b/cmd/main_test.go new file mode 100644 index 00000000..99af2774 --- /dev/null +++ b/cmd/main_test.go @@ -0,0 +1,54 @@ +package main + +import "testing" + +// TestMetricsOptions_InsecureKeepsTheOldShape pins the default: plaintext, no +// filter. Changing it breaks every existing scrape configuration silently, so +// it is a deliberate decision rather than a side effect. +func TestMetricsOptions_InsecureKeepsTheOldShape(t *testing.T) { + opts := metricsOptions(":8080", false, "") + + if opts.SecureServing { + t.Error("the default must stay plaintext until the switch is announced") + } + if opts.FilterProvider != nil { + t.Error("no filter without secure serving, it would reject every scrape") + } + if opts.BindAddress != ":8080" { + t.Errorf("bind address = %q, want :8080", opts.BindAddress) + } +} + +// TestMetricsOptions_SecureInstallsTheFilter covers the point of the option. +// Secure serving without the filter would encrypt the transport and still +// serve the metrics to anyone who asks. +func TestMetricsOptions_SecureInstallsTheFilter(t *testing.T) { + opts := metricsOptions(":8080", true, "") + + if !opts.SecureServing { + t.Error("expected secure serving") + } + if opts.FilterProvider == nil { + t.Fatal("secure serving without the auth filter still serves to anyone") + } + if opts.CertDir != "" { + t.Errorf("without a cert dir controller-runtime self-signs, got %q", opts.CertDir) + } +} + +// TestMetricsOptions_CertDirIsUsedOnlyWhenSecure keeps a supplied certificate +// from creating the impression of protection while the endpoint is plaintext. +func TestMetricsOptions_CertDirIsUsedOnlyWhenSecure(t *testing.T) { + secure := metricsOptions(":8080", true, "/tmp/certs") + if secure.CertDir != "/tmp/certs" { + t.Errorf("cert dir = %q, want /tmp/certs", secure.CertDir) + } + if secure.CertName != "tls.crt" || secure.KeyName != "tls.key" { + t.Errorf("expected the Secret's key names, got %q/%q", secure.CertName, secure.KeyName) + } + + insecure := metricsOptions(":8080", false, "/tmp/certs") + if insecure.CertDir != "" { + t.Error("a cert dir without secure serving would suggest a protection that is not there") + } +} diff --git a/cmd/mock/main.go b/cmd/mock/main.go index d931ee8b..81730141 100644 --- a/cmd/mock/main.go +++ b/cmd/mock/main.go @@ -8,6 +8,7 @@ import ( "log" "net/http" "os" + "path/filepath" "strings" "sync" "time" @@ -99,7 +100,7 @@ func main() { } if encClasses != "" { - for _, c := range strings.Split(encClasses, ",") { + for c := range strings.SplitSeq(encClasses, ",") { c = strings.TrimSpace(c) if c != "" { s.encClasses = append(s.encClasses, c) @@ -112,15 +113,20 @@ func main() { if err := s.loadClassificationsFile(); err != nil { log.Printf("WARNING: failed to load classifications file: %v", err) } else { - log.Printf("Loaded classifications from %s", s.classificationsFile) + log.Printf("Loaded classifications from %s", s.classificationsFile) // #nosec G706 -- path comes from a CLI flag set by the test harness } go s.watchClassificationsFile() } mux := newServeMux(s) - log.Printf("openvox-mock listening on %s", listen) - log.Fatal(http.ListenAndServe(listen, mux)) + log.Printf("openvox-mock listening on %s", listen) // #nosec G706 -- listen address comes from a CLI flag set by the test harness + srv := &http.Server{ + Addr: listen, + Handler: mux, + ReadHeaderTimeout: 10 * time.Second, + } + log.Fatal(srv.ListenAndServe()) } func newServeMux(s *server) *http.ServeMux { @@ -176,7 +182,7 @@ func (s *server) loadClassificationsFile() error { return err } - data, err := os.ReadFile(s.classificationsFile) + data, err := os.ReadFile(filepath.Clean(s.classificationsFile)) if err != nil { return err } @@ -229,7 +235,7 @@ func (s *server) handleENC(w http.ResponseWriter, r *http.Request) { } certname := r.PathValue("certname") - log.Printf("ENC request for certname=%s", certname) + log.Printf("ENC request for certname=%s", certname) // #nosec G706 -- mock server for E2E tests, logs are debug output only s.mu.Lock() s.classifications = append(s.classifications, storedClassification{ diff --git a/cmd/mock/main_test.go b/cmd/mock/main_test.go index 9d50b4c9..dd869f9c 100644 --- a/cmd/mock/main_test.go +++ b/cmd/mock/main_test.go @@ -599,16 +599,23 @@ func TestHECEvent_InvalidJSON(t *testing.T) { } } +// discardResponse closes the body of a fire-and-forget request. +func discardResponse(resp *http.Response, _ error) { + if resp != nil { + _ = resp.Body.Close() + } +} + func TestAPIReset(t *testing.T) { ts, _ := newTestServer() defer ts.Close() // Store some data - _, _ = http.Post(ts.URL+"/reports", "application/json", strings.NewReader(`{"host":"test"}`)) - _, _ = http.Post(ts.URL+"/services/collector/event", "application/json", strings.NewReader(`{"event":"test"}`)) + discardResponse(http.Post(ts.URL+"/reports", "application/json", strings.NewReader(`{"host":"test"}`))) + discardResponse(http.Post(ts.URL+"/services/collector/event", "application/json", strings.NewReader(`{"event":"test"}`))) cmd := `{"command":"replace facts","version":5,"payload":{"certname":"node1"}}` - _, _ = http.Post(ts.URL+"/pdb/cmd/v1", "application/json", strings.NewReader(cmd)) - _, _ = http.Get(ts.URL + "/node/test-node") + discardResponse(http.Post(ts.URL+"/pdb/cmd/v1", "application/json", strings.NewReader(cmd))) + discardResponse(http.Get(ts.URL + "/node/test-node")) // Reset req, _ := http.NewRequest("DELETE", ts.URL+"/api/reset", nil) @@ -677,8 +684,11 @@ func TestReloadClassificationsIfChanged(t *testing.T) { func TestReloadClassificationsIfChanged_MissingFile(t *testing.T) { s := &server{classificationsFile: "/nonexistent/file.yaml"} - // Should not panic, just return + // Should not panic, just return without loading anything s.reloadClassificationsIfChanged() + if s.classificationsData != nil { + t.Error("expected no classifications to be loaded from a missing file") + } } func TestLoadClassificationsFile_StatError(t *testing.T) { diff --git a/cmd/report/processor.go b/cmd/report/processor.go index a3a2c1d1..0f6c6daf 100644 --- a/cmd/report/processor.go +++ b/cmd/report/processor.go @@ -9,6 +9,7 @@ import ( "io" "net/http" "os" + "path/filepath" "strings" "time" @@ -55,7 +56,7 @@ type HeaderConfig struct { // loadReportConfig reads and parses the report config YAML file. func loadReportConfig(path string) (*ReportConfig, error) { - data, err := os.ReadFile(path) + data, err := os.ReadFile(filepath.Clean(path)) if err != nil { return nil, fmt.Errorf("reading report config: %w", err) } @@ -144,7 +145,7 @@ func buildHTTPClient(endpoint EndpointConfig) (*http.Client, error) { // Load CA certificate for server verification if endpoint.SSL.CAFile != "" { - caCert, err := os.ReadFile(endpoint.SSL.CAFile) + caCert, err := os.ReadFile(filepath.Clean(endpoint.SSL.CAFile)) if err != nil { return nil, fmt.Errorf("reading CA cert: %w", err) } diff --git a/config/crd/bases/openvox.voxpupuli.org_servers.yaml b/config/crd/bases/openvox.voxpupuli.org_servers.yaml index 9712cfb0..7de5904f 100644 --- a/config/crd/bases/openvox.voxpupuli.org_servers.yaml +++ b/config/crd/bases/openvox.voxpupuli.org_servers.yaml @@ -3428,8 +3428,13 @@ spec: type: string type: object javaArgs: - default: -Xms512m -Xmx1024m - description: JavaArgs defines the JVM arguments. + description: |- + JavaArgs defines the JVM arguments. + + There is deliberately no default. A defaulted field is never empty, and + the controller uses emptiness to decide whether to derive the heap from + the pod's memory limit. With a default in place that derivation is dead + code and every Server runs on the same heap regardless of its limit. type: string maxActiveInstances: default: 1 diff --git a/config/crd/bases/openvox.voxpupuli.org_signingpolicies.yaml b/config/crd/bases/openvox.voxpupuli.org_signingpolicies.yaml index 75d54ca6..69fb2547 100644 --- a/config/crd/bases/openvox.voxpupuli.org_signingpolicies.yaml +++ b/config/crd/bases/openvox.voxpupuli.org_signingpolicies.yaml @@ -62,6 +62,21 @@ spec: description: CertificateAuthorityRef references the CertificateAuthority this policy applies to. type: string + certnames: + description: |- + Certnames defines allowed certname glob patterns. The certname must match + at least one. + properties: + allow: + description: Allow is a list of glob patterns. The certname must + match at least one. + items: + type: string + type: array + x-kubernetes-list-type: set + required: + - allow + type: object csrAttributes: description: |- CSRAttributes defines CSR extension attributes that must all match (AND logic). @@ -110,8 +125,8 @@ spec: x-kubernetes-list-type: map dnsAltNames: description: |- - DNSAltNames defines allowed DNS subject alternative name patterns. - If not set and Any is false, CSRs with SANs are denied by the autosign binary. + DNSAltNames defines allowed DNS subject alternative name patterns (glob). + If a CSR carries DNS SANs and this is not set, the CSR is denied. properties: allow: description: Allow is a list of glob patterns. The certname must @@ -123,8 +138,61 @@ spec: required: - allow type: object - pattern: - description: Pattern defines certname glob matching rules. + emailAltNames: + description: |- + EmailAltNames defines allowed email subject alternative name patterns. A "*" + wildcard matches any run of characters, including "@". If a CSR carries email + SANs and this is not set, the CSR is denied. + properties: + allow: + description: Allow is a list of glob patterns. The certname must + match at least one. + items: + type: string + type: array + x-kubernetes-list-type: set + required: + - allow + type: object + extensions: + description: |- + Extensions lists Puppet CSR extension names (e.g. pp_cli_auth) that a CSR + matched by this policy is permitted to carry. 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 listed here; this gate applies to every + policy, including one with any=true. Trusted-fact extensions are unaffected. + properties: + allow: + description: Allow is a list of glob patterns. The certname must + match at least one. + items: + type: string + type: array + x-kubernetes-list-type: set + required: + - allow + type: object + ipAltNames: + description: |- + IPAltNames defines allowed IP subject alternative names as CIDR ranges + (e.g. "10.0.0.0/16", "::1/128"). If a CSR carries IP SANs and this is not + set, the CSR is denied. + properties: + allow: + description: Allow is a list of glob patterns. The certname must + match at least one. + items: + type: string + type: array + x-kubernetes-list-type: set + required: + - allow + type: object + uriAltNames: + description: |- + URIAltNames defines allowed URI subject alternative name patterns. A "*" + wildcard matches any run of characters, including "/". If a CSR carries URI + SANs and this is not set, the CSR is denied. properties: allow: description: Allow is a list of glob patterns. The certname must diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 79ae37b2..c54229b5 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -57,3 +57,12 @@ rules: - apiGroups: ["coordination.k8s.io"] resources: ["leases"] verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] + # The metrics filter authenticates each scrape against the API server and + # checks authorization, which needs these two review APIs. Without them the + # secure metrics endpoint rejects every request. + - apiGroups: ["authentication.k8s.io"] + resources: ["tokenreviews"] + verbs: ["create"] + - apiGroups: ["authorization.k8s.io"] + resources: ["subjectaccessreviews"] + verbs: ["create"] diff --git a/config/samples/signingpolicy.yaml b/config/samples/signingpolicy.yaml index 924a82b1..f9e1241a 100644 --- a/config/samples/signingpolicy.yaml +++ b/config/samples/signingpolicy.yaml @@ -12,7 +12,7 @@ metadata: name: trusted-hosts spec: certificateAuthorityRef: production-ca - pattern: + certnames: allow: - "*.example.com" - "web-*" @@ -36,7 +36,7 @@ metadata: name: trusted-with-psk spec: certificateAuthorityRef: production-ca - pattern: + certnames: allow: - "*.example.com" csrAttributes: diff --git a/docs/_snippets/features.md b/docs/_snippets/features.md index 39bb1998..c7a61bba 100644 --- a/docs/_snippets/features.md +++ b/docs/_snippets/features.md @@ -1,12 +1,12 @@ - 🔐 **Automated CA Lifecycle** - CA initialization, certificate signing, distribution, and periodic CRL refresh - fully managed -- 📜 **Declarative Signing Policies** - CSR approval via patterns, DNS SANs, CSR attributes, or open signing - no autosign scripts +- 📜 **Declarative Signing Policies** - CSR approval via certname globs, SAN allowlists, CSR attributes, or open signing - no autosign scripts - 🏷ïļ **External Node Classification** - Declarative ENC support for custom HTTP classifiers - ðŸ“Ķ **One Image, Two Roles** - Same rootless image runs as CA or server, configured by the operator - ⚡ **Scalable Servers** - Scale catalog compilation horizontally - multiple server pools with HPA - 🔄 **Multi-Version Deployments** - Run different server versions side by side - canary deployments, rolling upgrades - 🔒 **Rootless & OpenShift Ready** - Random UID compatible, no root, no ezbake, no privilege escalation - ðŸŠķ **Minimal Image** - UBI9-based, no agent Ruby, no ezbake packaging - smaller footprint, fewer updates -- 🧠 **Auto-tuned JVM** - Heap size calculated from memory limits (90%) - no manual `-Xmx` tuning needed +- 🧠 **Auto-tuned JVM** - Heap derived from the pod memory limit (90%) unless `javaArgs` is set - ðŸ“Ķ **OCI Image Volumes** - Package Puppet code as OCI images, deploy immutably with automatic rollout (K8s 1.35+) - 🌐 **Gateway API** - SNI-based TLSRoute support - share a single LoadBalancer across environments via TLS passthrough - 🗄ïļ **Managed OpenVox DB** - Deploy OpenVox DB (PuppetDB) with external PostgreSQL - TLS, config, and credentials managed by the operator diff --git a/docs/concepts/architecture.md b/docs/concepts/architecture.md index 5b9201db..a19e1f6d 100644 --- a/docs/concepts/architecture.md +++ b/docs/concepts/architecture.md @@ -33,7 +33,7 @@ graph TD - A **Config** is the root resource. It generates ConfigMaps for puppet.conf/puppetdb.conf/webserver.conf and holds shared configuration. - A **CertificateAuthority** is a standalone resource managing the CA infrastructure: PVC, setup Job, and CA Secret. A Config references it via `authorityRef`. -- A **SigningPolicy** is an optional resource that references a CertificateAuthority and defines declarative CSR signing rules (any, pattern match, or CSR attribute match). The Config controller renders all SigningPolicies into an autosign policy file. If no SigningPolicy exists, autosigning is disabled. +- A **SigningPolicy** is an optional resource that references a CertificateAuthority and defines declarative CSR signing rules (any, certname match, or CSR attribute match) together with fail-closed allowlists for SANs and privileged extensions. The Config controller renders all SigningPolicies into an autosign policy file. If no SigningPolicy exists, autosigning is disabled. - A **NodeClassifier** is an optional standalone resource defining an External Node Classifier endpoint. A Config references it via `nodeClassifierRef`. The Config controller renders the classifier configuration into an ENC Secret, and puppet.conf gets `node_terminus = exec`. See [External Node Classification](external-node-classification.md). - A **ReportProcessor** is an optional standalone resource that defines an external endpoint for Puppet run reports. One or more ReportProcessors can reference the same Config via `configRef`. The Config controller collects all matching ReportProcessors, renders a `report-webhook.yaml` config, and sets `reports = webhook` in puppet.conf. A minimal Ruby shim (`webhook.rb`) pipes each report as JSON to the `openvox-report` binary, which forwards it to all configured endpoints. Supports built-in PuppetDB wire format v8 (`processor: puppetdb`) and generic HTTP webhooks with configurable auth (mTLS, Bearer, Basic, custom headers). See [Report Processing](report-processing.md). - A **Certificate** references a CertificateAuthority and manages the lifecycle of a single certificate: signing Job and TLS Secret. @@ -229,7 +229,7 @@ Existing containers decide at startup whether to run as CA or server based on en | **Privileges** | Requires root | Fully rootless, random UID compatible | | **CA Management** | `puppetserver ca` CLI (CRuby) | Custom JRuby wrapper via `clojure.main` | | **Certificates** | Each server has its own certificate | `Certificate` CRD manages the cert lifecycle - all replicas of a Server share one certificate | -| **CSR Signing** | `autosign.conf` or Ruby scripts | `SigningPolicy` CRD with declarative rules (any, pattern, CSR attributes, DNS SAN validation) | +| **CSR Signing** | `autosign.conf` or Ruby scripts | `SigningPolicy` CRD with declarative rules (any, certnames, CSR attributes) plus fail-closed SAN and extension allowlists | | **CRL** | File on disk, manual refresh | Split Secret (`{ca}-ca-crl`), operator-driven periodic refresh via CA HTTP API | | **Scaling** | Manual VM provisioning | Deployment replicas + HPA | | **Code Deployment** | r10k on the VM, cron/webhook | OCI image volumes or PVC - code packaged as immutable container images | diff --git a/docs/concepts/certificate-signing.md b/docs/concepts/certificate-signing.md index d2b21941..fd9d0ce6 100644 --- a/docs/concepts/certificate-signing.md +++ b/docs/concepts/certificate-signing.md @@ -29,7 +29,7 @@ sequenceDiagram Operator->>K8s: Create PVC ({ca}-data) Operator->>K8s: Create Service ({ca}-internal) Operator->>K8s: Create ServiceAccount + RBAC - Operator->>K8s: Create Job ({ca}-setup) + Operator->>K8s: Create Job ({ca}-ca-setup) Job->>PVC: Run puppetserver ca setup Job->>K8s: Create Secret {ca}-ca (public cert) Job->>K8s: Create Secret {ca}-ca-key (private key) @@ -67,6 +67,11 @@ From this point on, the Certificate controller uses this Secret for mTLS-authent External CAs do not get an operator-signing Certificate: they manage their own signing credentials externally. + +The certname of this certificate is reserved: the CA `auth.conf` grants admin +rights to it, so the autosign policy refuses to issue it to anyone else. See +[Checks that no policy can waive](../reference/signingpolicy.md#checks-that-no-policy-can-waive). + ## Certificate Signing Strategies The operator uses two strategies depending on when the Certificate is created relative to the CA: diff --git a/docs/concepts/code-deployment.md b/docs/concepts/code-deployment.md index 0ea0d947..3f5c5925 100644 --- a/docs/concepts/code-deployment.md +++ b/docs/concepts/code-deployment.md @@ -58,9 +58,9 @@ metadata: spec: image: repository: ghcr.io/slauger/openvox-server-8 - tag: "8.12.1" + tag: "latest" code: - image: ghcr.io/example/puppet-code:v1.0.0 + - image: ghcr.io/example/puppet-code:v1.0.0 ``` ### Pull Policy @@ -70,8 +70,8 @@ Control when the image is pulled via `imagePullPolicy`. Defaults to `IfNotPresen ```yaml spec: code: - image: ghcr.io/example/puppet-code:v1.0.0 - imagePullPolicy: Always + - image: ghcr.io/example/puppet-code:v1.0.0 + imagePullPolicy: Always ``` Supported values: `Always`, `IfNotPresent`, `Never`. @@ -83,7 +83,7 @@ For immutable, reproducible deployments you can reference images by digest inste ```yaml spec: code: - image: ghcr.io/example/puppet-code@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2 + - image: ghcr.io/example/puppet-code@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2 ``` A tag+digest combination also works: @@ -91,7 +91,7 @@ A tag+digest combination also works: ```yaml spec: code: - image: ghcr.io/example/puppet-code:v1.0.0@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2 + - image: ghcr.io/example/puppet-code:v1.0.0@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2 ``` ### Rolling Out Code Changes @@ -101,7 +101,7 @@ Update the image reference to deploy new code. The operator detects the change a ```yaml spec: code: - image: ghcr.io/example/puppet-code:v1.1.0 + - image: ghcr.io/example/puppet-code:v1.1.0 ``` ### Private Registries @@ -111,8 +111,8 @@ For private registries, create a pull secret and reference it: ```yaml spec: code: - image: registry.example.com/puppet-code:v1.0.0 - imagePullSecret: registry-credentials + - image: registry.example.com/puppet-code:v1.0.0 + imagePullSecret: registry-credentials ``` ### Rollout Visibility @@ -163,7 +163,7 @@ spec: configRef: production certificateRef: canary-cert code: - image: ghcr.io/example/puppet-code:v2.0.0-rc1 + - image: ghcr.io/example/puppet-code:v2.0.0-rc1 ``` ## PVC @@ -181,9 +181,9 @@ metadata: spec: image: repository: ghcr.io/slauger/openvox-server-8 - tag: "8.12.1" + tag: "latest" code: - claimName: puppet-code + - claimName: puppet-code ``` Like the image volume, the PVC is mounted at the configured `environmentPath` (default `/etc/puppetlabs/code/environments`), so its root must contain the environment directories directly (`production/`, `staging/`, ...). diff --git a/docs/concepts/config-rollout.md b/docs/concepts/config-rollout.md index 9f36c0f9..ec60e256 100644 --- a/docs/concepts/config-rollout.md +++ b/docs/concepts/config-rollout.md @@ -15,7 +15,6 @@ Tracked annotations: | `openvox.voxpupuli.org/ca-secret-hash` | CA Secret (`{ca}-ca`) | Yes | | `openvox.voxpupuli.org/enc-secret-hash` | ENC Secret (`{config}-enc`) | Yes | | `openvox.voxpupuli.org/report-webhook-secret-hash` | Report webhook Secret (`{config}-report-webhook`) | Yes | -| `openvox.voxpupuli.org/autosign-policy-secret-hash` | Autosign policy Secret (`{ca}-autosign-policy`, CA pods only) | Yes | | `openvox.voxpupuli.org/code-image` | Code OCI image reference | Yes | ## What Triggers a Restart @@ -88,8 +87,20 @@ Creating or updating a SigningPolicy: 1. Config controller is triggered via SigningPolicy watcher 2. Autosign policy Secret (`{ca}-autosign-policy`) is updated -3. Server controller detects the updated `autosign-policy-secret-hash` annotation -4. CA pod is recreated so `openvox-autosign` reads the new policy on the next CSR (no manual restart) +3. The kubelet syncs the updated Secret into the CA pod, which mounts it as a + directory rather than through `subPath` +4. `openvox-autosign` reads the file on the next CSR + +**No restart is involved.** The policy Secret is deliberately the one Secret +that does not roll its pod. A `subPath` mount is never refreshed by the +kubelet, which is why this used to need a restart - and because the CA +Deployment uses the `Recreate` strategy, that restart meant a short outage with +no signing and no CRL, for a change that alters no running state. + +The trade-off is timing: a policy edit takes effect within the kubelet sync +period, up to about a minute, instead of immediately after a restart. For a +rule that governs which CSRs get signed, waiting a minute is preferable to +dropping the CA. ### Changing ENC Configuration diff --git a/docs/concepts/database.md b/docs/concepts/database.md index 6f9604eb..51e4f9a8 100644 --- a/docs/concepts/database.md +++ b/docs/concepts/database.md @@ -71,10 +71,21 @@ spec: databaseRef: production-db # operator reads Database.status.url image: repository: ghcr.io/slauger/openvox-server-8 - tag: "8.12.1" + tag: "latest" ``` -The operator reads `Database.status.url` (e.g. `https://production-db.namespace.svc.cluster.local:8081`) and renders it into `puppetdb.conf`. When the Database is not yet `Running`, the Config controller waits. +The operator reads `Database.status.url` (e.g. `https://production-db.namespace.svc.cluster.local:8081`) and renders it into `puppetdb.conf`. + +When the Database has no URL yet, the Config controller does **not** wait. It +renders a `puppetdb.conf` without `server_urls` and carries on, so the servers +start regardless. Combined with `soft_write_failure = true`, which the operator +always sets, a server in that state compiles catalogs normally while reports +and exported resources go nowhere and no error is raised. + +The Config is re-reconciled when the Database status changes, so this resolves +by itself during bring-up. It becomes a problem only if the Database never +reaches `Running`: the symptom is an empty PuppetDB with healthy-looking +servers, not a failure. ### Via static `puppetdb.serverUrls` diff --git a/docs/concepts/external-node-classification.md b/docs/concepts/external-node-classification.md index 6f5d19fd..c12b0914 100644 --- a/docs/concepts/external-node-classification.md +++ b/docs/concepts/external-node-classification.md @@ -136,7 +136,7 @@ spec: nodeClassifierRef: pe-classifier image: repository: ghcr.io/slauger/openvox-server-8 - tag: "8.12.1" + tag: "latest" ``` This generates the following puppet.conf entries in the `[server]` section: diff --git a/docs/concepts/gateway-api.md b/docs/concepts/gateway-api.md index 3bbacb6b..515ec044 100644 --- a/docs/concepts/gateway-api.md +++ b/docs/concepts/gateway-api.md @@ -107,6 +107,10 @@ spec: The `openvox-stack` chart provides a `gateway` section for shared Gateway settings: +A Pool does not name its servers. The relationship runs the other way: each +entry under `servers` lists the pools it joins via `poolRefs`, and the Pool +selects those pods through its Service. + ```yaml gateway: name: puppet-gateway @@ -114,7 +118,6 @@ gateway: pools: - name: puppet - serverRef: ca service: type: ClusterIP port: 8140 diff --git a/docs/concepts/report-processing.md b/docs/concepts/report-processing.md index 8fa5858f..a77190bd 100644 --- a/docs/concepts/report-processing.md +++ b/docs/concepts/report-processing.md @@ -107,7 +107,7 @@ spec: authorityRef: production-ca image: repository: ghcr.io/slauger/openvox-server-8 - tag: "8.12.1" + tag: "latest" ``` ```yaml diff --git a/docs/examples/index.md b/docs/examples/index.md index f23a52a8..90419ac5 100644 --- a/docs/examples/index.md +++ b/docs/examples/index.md @@ -13,7 +13,7 @@ spec: authorityRef: lab-ca image: repository: ghcr.io/slauger/openvox-server-8 - tag: "8.12.1" + tag: "latest" --- apiVersion: openvox.voxpupuli.org/v1alpha1 kind: CertificateAuthority @@ -74,7 +74,7 @@ spec: databaseRef: production-db image: repository: ghcr.io/slauger/openvox-server-8 - tag: "8.12.1" + tag: "latest" puppet: environmentTimeout: unlimited storeconfigs: true @@ -232,7 +232,7 @@ spec: replicas: 3 maxActiveInstances: 2 code: - claimName: puppet-code + - claimName: puppet-code resources: requests: cpu: "1" @@ -250,10 +250,10 @@ spec: certificateRef: canary-cert poolRefs: [puppet] image: - tag: "8.13.0" + tag: "latest" replicas: 1 code: - claimName: puppet-code + - claimName: puppet-code resources: requests: cpu: "1" diff --git a/docs/getting-started/quickstart.md b/docs/getting-started/quickstart.md index 0b37421d..a22d5da6 100644 --- a/docs/getting-started/quickstart.md +++ b/docs/getting-started/quickstart.md @@ -75,7 +75,7 @@ This guide sets up an OpenVox Server deployment. Choose between the Helm chart ( authorityRef: lab-ca image: repository: ghcr.io/slauger/openvox-server-8 - tag: "8.12.1" + tag: "latest" --- apiVersion: openvox.voxpupuli.org/v1alpha1 kind: CertificateAuthority @@ -162,6 +162,18 @@ NAME TYPE ENDPOINTS AGE pool.openvox.voxpupuli.org/puppet ClusterIP 1 2m ``` +!!! warning "Without a SigningPolicy nothing gets signed" + + The operator points `autosign` at its own binary as soon as a + CertificateAuthority exists, and that binary denies every CSR it has no + matching policy for. An empty policy list therefore means deny-all, not + off. + + Nothing surfaces this. The servers come up, the Config reports `Running`, + and every agent sits in `puppet agent --waitforcert` until it gives up. + Check with `kubectl get signingpolicy -n `; if the list is + empty, see [SigningPolicy](../reference/signingpolicy.md). + ## Next Steps See the [Examples](../examples/index.md) section for production setups with separate CA, server pools, canary deployments, and code deployment via OCI image volumes. diff --git a/docs/guides/ca-import.md b/docs/guides/ca-import.md index f92e972f..a5d4eeb8 100644 --- a/docs/guides/ca-import.md +++ b/docs/guides/ca-import.md @@ -28,7 +28,7 @@ If you have an existing CA and want the operator to manage it going forward, you ```bash # Find the PVC - kubectl get pvc -l openvox.voxpupuli.org/certificate-authority=production-ca + kubectl get pvc -l openvox.voxpupuli.org/certificateauthority=production-ca # Create a temporary pod to copy data kubectl run ca-import --image=busybox --restart=Never \ diff --git a/docs/guides/connecting-agents.md b/docs/guides/connecting-agents.md new file mode 100644 index 00000000..6fbd0b18 --- /dev/null +++ b/docs/guides/connecting-agents.md @@ -0,0 +1,163 @@ +# Connecting Agents + +The operator manages the server side. Agents are ordinary Puppet agents: they +are not Kubernetes resources, and nothing in the operator creates or tracks +them. This page covers what they need from a stack deployed by the operator. + +## What an agent needs + +Three things, and they are the usual ones: + +| | | +|---|---| +| **A reachable server address** | the Service of a Pool the server joins, on port 8140 | +| **A certname** | the identity the certificate is issued for | +| **A signed certificate** | either autosigned by a [SigningPolicy](../reference/signingpolicy.md) or signed by hand | + +!!! warning "Without a SigningPolicy nothing is signed" + + The operator points `autosign` at its own binary as soon as a + CertificateAuthority exists, and that binary denies every CSR no policy + matches. An empty policy list is deny-all, not off. Agents then sit in + `--waitforcert` while the servers look perfectly healthy. + +## Where to point the agent + +Agents talk to the Service of a Pool. With the default layout the CA server +joins both pools (`poolRefs: [ca, server]`), so one address serves catalog +requests and the CA: + +```bash +puppet agent --test \ + --server -server \ + --certname web01.example.com \ + --waitforcert 30 +``` + +If you separate the roles - the CA in the `ca` pool only, compilers in +`server` - the agent needs both addresses, because catalog and CA no longer +share one: + +```ini +[main] +server = -server +ca_server = -ca +``` + +## From inside the cluster + +Any Puppet agent works. This project also builds `openvox-agent-`, but +be aware of what it is: a **test artifact**. It is built by CI and not by the +release workflow, so it carries only the `develop` tag - no `latest`, no +version. For anything but a throwaway check, use your own agent image and pin +it. + +The shape below is what the e2e suite runs: + +```yaml +apiVersion: batch/v1 +kind: Job +metadata: + name: puppet-agent +spec: + backoffLimit: 0 + template: + spec: + restartPolicy: Never + containers: + - name: puppet-agent + image: ghcr.io/slauger/openvox-agent-8:develop # test artifact, see above + command: ["sh", "-c"] + args: + - | + puppet agent --test \ + --server -server \ + --certname agent-01 \ + --waitforcert 30 + # 0 = no changes, 2 = changes applied; both are success + EXIT=$? + if [ $EXIT -eq 0 ] || [ $EXIT -eq 2 ]; then exit 0; else exit $EXIT; fi +``` + +The exit code check matters: `puppet agent --test` returns **2** when it +applied changes, which is a success and not a failure. + +## From outside the cluster + +A Pool Service defaults to `ClusterIP`, which is unreachable from outside. +Choose one: + +| Option | How | +|---|---| +| **LoadBalancer** | `pools[].service.type: LoadBalancer` | +| **NodePort** | `pools[].service.type: NodePort`, optionally `nodePort` | +| **Gateway API** | `pools[].route`, see [Gateway API](../concepts/gateway-api.md) | + +Whichever you pick, the name agents connect through must be in the server +certificate. The chart derives the Service names of every Pool a server joins +into `dnsAltNames` automatically; an external name such as a load balancer +address has to be added explicitly: + +```yaml +servers: + - name: ca + certificate: + certname: puppet + dnsAltNames: + - puppet.example.com +``` + +A missing name shows up as a TLS error on the agent, not as an operator +problem: + +``` +Server hostname 'puppet.example.com' did not match server certificate +``` + +## Signing by hand + +Without a matching policy the CSR waits. List and sign on the CA pod: + +```bash +kubectl exec -n deploy/ -- \ + puppetserver ca list --all + +kubectl exec -n deploy/ -- \ + puppetserver ca sign --certname web01.example.com +``` + +## Removing an agent + +Revoking is a CA operation, and the operator does not do it for you - it only +manages the certificates that belong to its own resources: + +```bash +kubectl exec -n deploy/ -- \ + puppetserver ca clean --certname web01.example.com +``` + +The revocation reaches agents with the next CRL refresh, which runs on +`spec.crlRefreshInterval` (default `5m`). Until then the revoked certificate is +still accepted. + +## Verifying it worked + +The agent reports success itself, but two things are worth checking on the +server side. + +**Did the catalog compile?** + +```bash +kubectl logs -n deploy/ | grep -i "Compiled catalog" +``` + +**Did the facts reach PuppetDB?** Only when a Database is wired up: + +```bash +kubectl exec -n deploy/ -- \ + curl -sf "http://127.0.0.1:8080/pdb/query/v4/nodes" +``` + +An empty result with a healthy agent run usually means `routes.yaml` is not +routing the facts terminus to PuppetDB - see +[Database](../reference/database.md). diff --git a/docs/guides/monitoring.md b/docs/guides/monitoring.md index 376a080a..8a1fe8c5 100644 --- a/docs/guides/monitoring.md +++ b/docs/guides/monitoring.md @@ -17,6 +17,48 @@ metrics: This passes `--metrics-bind-address=0` to the operator and removes the metrics port from the Deployment. +## Securing the endpoint + +By default the metrics endpoint serves plaintext HTTP to anything that can +reach the pod. It carries no key material, but it does list every certificate +the operator manages and when each expires - a ready-made inventory. Turn on +authentication with: + +```yaml +metrics: + secure: true +``` + +Each scrape then has to present a bearer token, which the operator verifies +against the API server with a TokenReview and a SubjectAccessReview. The chart +creates a `{release}-metrics-reader` ClusterRole granting `get` on `/metrics`; +bind your scraper's ServiceAccount to it. Nothing is bound by default: + +```bash +kubectl create clusterrolebinding prometheus-openvox-metrics \ + --clusterrole=-openvox-operator-metrics-reader \ + --serviceaccount=monitoring:prometheus +``` + +### The serving certificate + +| `metrics.certManager.enabled` | Result | +|---|---| +| `true` (default) | cert-manager issues the certificate from the chart's CA issuer, and the bundled ServiceMonitor verifies against it | +| `false` | the operator generates a self-signed certificate at startup and a new one on every restart, so scrapers must skip verification | +| `metrics.tls.certSecret` set | your own Secret with `tls.crt` and `tls.key` is mounted instead | + +The protection comes from the authentication filter, not from the +certificate - which is why the self-signed path is usable and why +`insecureSkipVerify` against an in-cluster endpoint that exposes no secrets is +not the problem it looks like. + +!!! warning "Turning this on breaks existing scrape configurations" + + The endpoint switches to HTTPS and starts requiring a token. Prometheus + reports the target as down without saying why. The bundled ServiceMonitor + is updated automatically; hand-written scrape configs are not. + ## Custom Metrics | Metric | Type | Labels | Description | @@ -114,6 +156,24 @@ Alert when a certificate expires within 30 days: description: "{{ $labels.name }} in {{ $labels.namespace }} expires in {{ $value | humanizeDuration }}" ``` +### Missing CRL series + +A stale CRL is alertable only while the series exists. If the operator never +refreshed the CRL - it crashed early, or the CA never became ready - there is +no series at all and the staleness rule below stays silent. Alert on the +absence separately: + +```yaml + - alert: OpenVoxCRLMetricMissing + expr: absent(openvox_crl_last_refresh_timestamp_seconds) + for: 15m + labels: + severity: warning + annotations: + summary: "No CRL refresh has been recorded" + description: "The operator has not refreshed any CRL since it started. Revocations are not reaching agents." +``` + ### Stale CRL A CRL that is no longer refreshed means revoked agents keep being accepted. @@ -132,6 +192,18 @@ Alert when the last successful refresh is more than a day old: ### CA Expiring +!!! note "CAs and certificates share one metric" + + `openvox_certificate_expiry_timestamp_seconds` is written by both the + Certificate and the CertificateAuthority controller, with the same + `name`/`namespace` labels and nothing that says which kind a series belongs + to. Telling them apart in a query means matching on the name. + + The rule below uses `.*-ca`, which is a convention rather than a guarantee: + it also catches a Certificate that happens to end in `-ca`, and it misses a + CertificateAuthority named otherwise. Replace the matcher with your actual + CA names if you rely on the distinction. + Alert when a CA certificate expires within 90 days: ```yaml diff --git a/docs/index.md b/docs/index.md index 7925a7d4..e259b034 100644 --- a/docs/index.md +++ b/docs/index.md @@ -14,7 +14,7 @@ The operator manages OpenVox Server infrastructure through a set of Custom Resou |---|---|---| | **Config** | Shared config (puppet.conf, auth.conf, etc.), OpenVox DB connection | ConfigMaps, Secrets, ServiceAccount | | **CertificateAuthority** | CA infrastructure: keys, signing, split Secrets (cert, key, CRL) | PVC, Job, ServiceAccount, Role, RoleBinding, 3 Secrets | -| **SigningPolicy** | Declarative CSR signing policy (any, pattern, CSR attributes) | *(rendered into Config's autosign Secret)* | +| **SigningPolicy** | Declarative CSR signing policy (any, certnames, CSR attributes, SAN and extension allowlists) | *(rendered into Config's autosign Secret)* | | **NodeClassifier** | External Node Classifier (ENC) endpoint (Foreman, PE, custom HTTP) | *(rendered into Config's ENC Secret)* | | **Certificate** | Lifecycle of a single certificate (request, sign) | TLS Secret | | **Server** | OpenVox Server instance pool (CA and/or server role), declares pool membership via `poolRefs` | Deployment, HPA, PDB, NetworkPolicy | diff --git a/docs/reference/binaries.md b/docs/reference/binaries.md index 53fb4959..c77961b1 100644 --- a/docs/reference/binaries.md +++ b/docs/reference/binaries.md @@ -29,10 +29,17 @@ The CSR is read from stdin as PEM-encoded data. - Multiple policies are evaluated with OR logic (any match signs) - Within a policy, all conditions use AND logic (all must match) -- `any: true` signs unconditionally -- `pattern` matches certnames using glob patterns (`*`, `?`) +- `any: true` signs unconditionally **once the guards below have passed** +- `certnames` matches certnames using glob patterns (`*`, `?`) - `csrAttributes` matches Puppet CSR extension OIDs (e.g. `pp_role`, `pp_environment`, `pp_preshared_key`) -- `dnsAltNames` validates DNS SANs in the CSR - if a CSR contains SANs, they must be explicitly allowed +- `dnsAltNames`, `ipAltNames`, `uriAltNames`, `emailAltNames` and `extensions` are fail-closed allowlists: if a CSR carries a SAN of a given type, or a privileged authorization extension, the policy must allow it explicitly + +**Checks that precede every policy:** + +Two conditions are refused before any policy is consulted, so no policy -- including `any: true` -- can waive them: + +- **Reserved certnames.** The operator renders its own signing certname (`{ca}-operator`) into the policy file as `reservedCertnames`. The CA `auth.conf` grants admin rights to that name, so an agent holding a certificate under it would be a CA admin. +- **Subject binding.** puppetserver passes the certname as an argument, taken from the request path, while the CN lives in the CSR subject. A CSR whose subject differs from the requested certname is refused, so the policy cannot judge one name while the certificate is issued carrying another. **Supported CSR attributes:** diff --git a/docs/reference/certificate.md b/docs/reference/certificate.md index 881383e2..90e6a10f 100644 --- a/docs/reference/certificate.md +++ b/docs/reference/certificate.md @@ -31,9 +31,12 @@ spec: | Field | Type | Description | |---|---|---| +| `observedGeneration` | int64 | The `.metadata.generation` the status was last derived from. A value below `.metadata.generation` means the rest of this status has not caught up with the current spec yet | | `phase` | string | Current lifecycle phase | | `secretName` | string | Name of the Secret containing `cert.pem` and `key.pem` | | `notAfter` | time | Expiry time of the signed certificate | +| `signedSpecHash` | string | Digest of what the current certificate was issued for: certname, effective alt names and CSR extensions. A mismatch triggers re-signing; empty means the hash was never recorded and is adopted rather than triggering one | +| `effectiveDNSAltNames` | []string | The alt names the certificate is actually issued for: `spec.dnsAltNames` plus the route hostname of every Pool with `injectDNSAltName` that a Server using this Certificate joins | | `conditions` | []Condition | `CertSigned` | ## Deletion @@ -89,6 +92,17 @@ Certificates issued before this field existed carry an empty hash. The controller adopts the current spec as the baseline for them rather than re-signing every certificate after an operator upgrade. +### Renewal reuses the private key + +A renewal submits a CSR for the key the certificate already has +(`certificate_signing.go`: the existing `key.pem` is read from the TLS Secret +and reused). The CA renews for the same public key, so the key material must +not change between the old and the new certificate. + +The consequence is worth stating: renewal extends validity, it does not rotate +the key. A key that must be replaced needs a new Certificate under a different +name, since `certname` is immutable and the CA keeps one entry per name. + ### CSR Poll Backoff When the CA does not immediately sign the CSR (e.g. autosigning is disabled), the controller enters `WaitingForSigning` after 10 unsuccessful poll attempts and retries with exponential backoff: diff --git a/docs/reference/certificateauthority.md b/docs/reference/certificateauthority.md index 51a69a7d..b1a8c831 100644 --- a/docs/reference/certificateauthority.md +++ b/docs/reference/certificateauthority.md @@ -81,6 +81,7 @@ spec: | Field | Type | Description | |---|---|---| +| `observedGeneration` | int64 | The `.metadata.generation` the status was last derived from. A value below `.metadata.generation` means the rest of this status has not caught up with the current spec yet | | `phase` | string | Current lifecycle phase | | `caSecretName` | string | Name of the Secret containing `ca_crt.pem` (public CA certificate) | | `serviceName` | string | Name of the internal ClusterIP Service for operator communication. Empty when `spec.external` is set. | @@ -182,7 +183,7 @@ The failed Job is left in place; its logs are the only record of what went wrong: ```bash -kubectl logs -n job/-setup +kubectl logs -n job/-ca-setup ``` The attempt counter lives in the `openvox.voxpupuli.org/setup-attempts` diff --git a/docs/reference/config.md b/docs/reference/config.md index 790e6083..afc3e97a 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -13,7 +13,7 @@ spec: authorityRef: production-ca image: repository: ghcr.io/slauger/openvox-server-8 - tag: "8.12.1" + tag: "latest" puppet: environmentTimeout: "0" storeconfigs: true @@ -199,6 +199,7 @@ Controls Puppet Server metrics.conf settings. | Field | Type | Description | |---|---|---| +| `observedGeneration` | int64 | The `.metadata.generation` the status was last derived from. A value below `.metadata.generation` means the rest of this status has not caught up with the current spec yet | | `phase` | string | Current lifecycle phase | | `conditions` | []Condition | `ConfigReady` | @@ -208,7 +209,17 @@ Controls Puppet Server metrics.conf settings. |---|---| | `Pending` | Config created, waiting for reconciliation | | `Running` | ConfigMap created, ready for use | -| `Error` | Reconciliation failed | +| `Error` | Defined in the API, but never set by the controller (see below) | + +`Error` is part of the API but the controller never assigns it. A failing +reconcile leaves the phase at its previous value, reports the reason in the +`ConfigReady` condition and emits a warning event. Watch the condition rather than +the phase: + +```bash +kubectl get config -o jsonpath='{range .status.conditions[*]}{.type}={.status} {.reason}: {.message}{"\n"}{end}' +``` + ### Image resolution @@ -223,6 +234,6 @@ credentials along. Secrets for code images are always added on top. | Resource | Name | Description | |---|---|---| -| ConfigMap | `{name}` | puppet.conf, puppetserver.conf, auth.conf, webserver.conf, `routes.yaml` (facts terminus, when PuppetDB is the active backend), etc. | +| ConfigMap | `{name}-config` | puppet.conf, puppetserver.conf, auth.conf, webserver.conf, `routes.yaml` (facts terminus, when PuppetDB is the active backend), etc. | | Secret | `{name}-enc` | ENC config for openvox-enc binary (only when `nodeClassifierRef` is set) | | ServiceAccount | `{name}-server` | Shared ServiceAccount for all Server pods (`automountServiceAccountToken: false`) | diff --git a/docs/reference/database.md b/docs/reference/database.md index 8d8cf676..d7f7e83e 100644 --- a/docs/reference/database.md +++ b/docs/reference/database.md @@ -86,6 +86,7 @@ When enabled, the default policy allows TCP/8081 only from pods with `app.kubern | Field | Type | Description | |---|---|---| +| `observedGeneration` | int64 | The `.metadata.generation` the status was last derived from. A value below `.metadata.generation` means the rest of this status has not caught up with the current spec yet | | `phase` | string | Current lifecycle phase | | `url` | string | HTTPS endpoint of the Database Service (e.g. `https://production-db:8081`) | | `ready` | int32 | Number of ready replicas | @@ -99,7 +100,17 @@ When enabled, the default policy allows TCP/8081 only from pods with `app.kubern | `Pending` | Database created, resolving references | | `WaitingForCert` | Certificate not yet `Signed` | | `Running` | Deployment created and running | -| `Error` | Reconciliation failed | +| `Error` | Defined in the API, but never set by the controller (see below) | + +`Error` is part of the API but the controller never assigns it. A failing +reconcile leaves the phase at its previous value, reports the reason in the +`DatabaseReady` condition and emits a warning event. Watch the condition rather than +the phase: + +```bash +kubectl get database -o jsonpath='{range .status.conditions[*]}{.type}={.status} {.reason}: {.message}{"\n"}{end}' +``` + ## Pod Anatomy diff --git a/docs/reference/index.md b/docs/reference/index.md index 517a0015..c4654701 100644 --- a/docs/reference/index.md +++ b/docs/reference/index.md @@ -52,17 +52,28 @@ These types are reused across multiple CRDs. | Field | Type | Default | Description | |---|---|---|---| -| `repository` | string | `ghcr.io/slauger/openvox-server-8` | Container image repository | -| `tag` | string | `latest` | Container image tag | -| `pullPolicy` | string | `IfNotPresent` | Image pull policy | +| `repository` | string | - | Container image repository | +| `tag` | string | - | Container image tag | +| `pullPolicy` | string | - | Image pull policy | | `pullSecrets` | []LocalObjectReference | - | Image pull secrets | -`repository` and `tag` carry no API-level default. They are required on `Config` -and `Database`; on `Server` both are optional and fall back to the referenced -`Config`, which is what lets one Config drive a whole set of Servers. +No field here carries an API-level default. That is deliberate: a nested +default is applied whether or not the parent object was given, so a defaulted +field can never express "inherit from the Config" - it is simply never empty. -The defaults live in the Helm charts (`config.image.*`, `database.image.*`), -where changing the registry is a values change rather than a CRD update. +`repository` and `tag` are required on `Config` and `Database`. On `Server` +both are optional and fall back to the referenced `Config`, which is what lets +one Config drive a whole set of Servers. + +`pullPolicy` follows the same rule, falling back to the Config and then to +`IfNotPresent`. `pullSecrets` behaves differently: a non-empty list on a +`Server` *replaces* the Config's rather than extending it, so a Server pulling +from another registry does not carry the Config's credentials along. Secrets +for code images are always added on top. + +The registry defaults live in the Helm charts (`config.image.*`, +`database.image.*`), where changing them is a values change rather than a CRD +update. ### StorageSpec diff --git a/docs/reference/nodeclassifier.md b/docs/reference/nodeclassifier.md index ff1c19d7..db7b1c84 100644 --- a/docs/reference/nodeclassifier.md +++ b/docs/reference/nodeclassifier.md @@ -80,8 +80,10 @@ kind: Config metadata: name: production spec: - image: ... authorityRef: production-ca + image: + repository: ghcr.io/slauger/openvox-server-8 + tag: "latest" nodeClassifierRef: foreman ``` @@ -166,6 +168,7 @@ At most one authentication method may be configured. | Field | Type | Description | |---|---|---| +| `observedGeneration` | int64 | The `.metadata.generation` the status was last derived from. A value below `.metadata.generation` means the rest of this status has not caught up with the current spec yet | | `phase` | string | Current lifecycle phase | | `conditions` | []Condition | `Ready` | diff --git a/docs/reference/reportprocessor.md b/docs/reference/reportprocessor.md index 628fb85f..e757d7d3 100644 --- a/docs/reference/reportprocessor.md +++ b/docs/reference/reportprocessor.md @@ -175,6 +175,7 @@ Either `value` or `valueFrom` may be set, not both. | Field | Type | Description | |---|---|---| +| `observedGeneration` | int64 | The `.metadata.generation` the status was last derived from. A value below `.metadata.generation` means the rest of this status has not caught up with the current spec yet | | `phase` | string | Current lifecycle phase | | `conditions` | []Condition | `Ready` | diff --git a/docs/reference/server.md b/docs/reference/server.md index 18cbb515..f58641ee 100644 --- a/docs/reference/server.md +++ b/docs/reference/server.md @@ -33,7 +33,7 @@ spec: | `replicas` | int32 | `1` | Number of pod replicas | | `autoscaling` | [AutoscalingSpec](#autoscalingspec) | - | HPA configuration | | `resources` | ResourceRequirements | - | CPU/memory requests and limits | -| `javaArgs` | string | `-Xms512m -Xmx1024m` | JVM arguments | +| `javaArgs` | string | *(derived)* | JVM arguments. Unset derives the heap from the pod's memory limit (90%), falling back to `-Xms512m -Xmx1024m` when no limit is set | | `maxActiveInstances` | int32 | `1` | Number of JRuby instances per pod | | `code` | [[]CodeSpec](index.md#codespec) | - | Override the Config's code sources (replace, not merge). A list; see [CodeSpec](index.md#codespec) | | `topologySpreadConstraints` | []TopologySpreadConstraint | - | Pod spread constraints across topology domains | @@ -45,6 +45,7 @@ spec: | `envFrom` | []EnvFromSource | - | ConfigMap/Secret sources to populate environment variables from | | `extraVolumes` | []Volume | - | Extra volumes added to the Server pods | | `extraVolumeMounts` | []VolumeMount | - | Extra volume mounts for the `openvox-server` container | +| `readOnlyRootFilesystem` | *bool | *(inherits from Config)* | Overrides the Config's setting for this Server. One Config backs several Servers with different roles, so the CA and the compilers can differ. Unset inherits | | `securityContext` | [PodSecurityContextSpec](index.md#podsecuritycontextspec) | - | Override pod-level security context (runAsUser/runAsGroup/fsGroup) | ### Extra Environment and Volumes @@ -131,7 +132,17 @@ When enabled, the default policy allows TCP/8140 from all sources (agents may co | `Pending` | Server created, resolving references | | `WaitingForCert` | Certificate not yet `Signed` | | `Running` | Deployment created and running | -| `Error` | Reconciliation failed | +| `Error` | Defined in the API, but never set by the controller (see below) | + +`Error` is part of the API but the controller never assigns it. A failing +reconcile leaves the phase at its previous value, reports the reason in the +`Ready` condition and emits a warning event. Watch the condition rather than +the phase: + +```bash +kubectl get server -o jsonpath='{range .status.conditions[*]}{.type}={.status} {.reason}: {.message}{"\n"}{end}' +``` + ## Deployment Strategy diff --git a/docs/reference/signingpolicy.md b/docs/reference/signingpolicy.md index 4b4c0690..002bc8cd 100644 --- a/docs/reference/signingpolicy.md +++ b/docs/reference/signingpolicy.md @@ -2,6 +2,11 @@ A SigningPolicy defines a policy for automatic CSR signing against a CertificateAuthority. Multiple policies can reference the same CA -- if **any** policy matches, the CSR is signed (OR logic between policies). Within a single policy, **all** set fields must match (AND logic). +Signing has two planes: + +- **Match plane** (`any`, `certnames`, `csrAttributes`) -- decides *whether a policy applies* to a CSR. +- **Guard plane** (`dnsAltNames`, `ipAltNames`, `uriAltNames`, `emailAltNames`, `extensions`) -- **fail-closed** constraints that decide *whether the CSR is safe to sign*. If a CSR carries a SAN type or a privileged authorization extension the policy does not explicitly allow, it is denied. The guard plane applies to **every** policy, including `any: true`, so no policy can implicitly grant a privileged extension. + ## Example ```yaml @@ -14,7 +19,7 @@ spec: any: true ``` -### Pattern Matching +### Certname Matching ```yaml apiVersion: openvox.voxpupuli.org/v1alpha1 @@ -23,7 +28,7 @@ metadata: name: trusted-hosts spec: certificateAuthorityRef: production-ca - pattern: + certnames: allow: - "*.example.com" - "web-*" @@ -38,7 +43,7 @@ metadata: name: allow-internal-sans spec: certificateAuthorityRef: production-ca - pattern: + certnames: allow: - "*.example.com" dnsAltNames: @@ -47,6 +52,76 @@ spec: - "*.svc.cluster.local" ``` +### IP / URI / Email SAN Validation + +Each SAN type has its own fail-closed allowlist. `ipAltNames` uses **CIDR** ranges; `uriAltNames` and `emailAltNames` use wildcard patterns where `*` spans any characters (including `/` and `@`). + +```yaml +apiVersion: openvox.voxpupuli.org/v1alpha1 +kind: SigningPolicy +metadata: + name: gateway-sans +spec: + certificateAuthorityRef: production-ca + certnames: + allow: + - "gateway-*" + ipAltNames: + allow: + - "10.0.0.0/16" + - "::1/128" + uriAltNames: + allow: + - "spiffe://example.com/gateway/*" + emailAltNames: + allow: + - "*@example.com" +``` + +A CSR carrying a SAN of a type whose allowlist is unset is denied. + +### Checks that no policy can waive + +Two conditions are evaluated before any policy is consulted. They apply to +every policy, `any: true` included. + +**Reserved certnames.** The operator issues itself a certificate under +`{ca}-operator` for mTLS against the CA API, and the CA `auth.conf` grants +admin rights to that name as well as to the `pp_cli_auth` extension. An agent +holding a certificate under it would therefore be a CA admin. The operator +renders the name into the generated policy file, and the autosign binary +refuses it regardless of what any policy allows. + +You do not configure this: the name is derived from the CertificateAuthority +and reserved automatically. + +**Subject binding.** puppetserver takes the certname from the request path and +passes it to the autosign binary as an argument, while the CN lives in the CSR +subject. A CSR whose subject differs from the requested certname is refused, +so a policy cannot approve one name while the certificate is issued carrying +another. + +### Authorization 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 by default**, even for `any: true` policies. A certificate carrying `pp_cli_auth=true` is granted CA-admin access by the built-in auth.conf rules, so a CSR requesting it must be explicitly allowed: + +```yaml +apiVersion: openvox.voxpupuli.org/v1alpha1 +kind: SigningPolicy +metadata: + name: ca-admin-bootstrap +spec: + certificateAuthorityRef: production-ca + certnames: + allow: + - "ca-admin.example.com" + extensions: + allow: + - pp_cli_auth +``` + +Authorization-arc OIDs with no known Puppet name cannot be allow-listed and are always denied. Trusted-fact extensions (the `1.3.6.1.4.1.34380.1.1` arc, e.g. `pp_role`, `pp_environment`) are not gated. + ### CSR Attribute Matching Match CSR extension attributes with inline values or Secret references: @@ -75,7 +150,7 @@ metadata: name: trusted-with-psk spec: certificateAuthorityRef: production-ca - pattern: + certnames: allow: - "*.example.com" csrAttributes: @@ -96,8 +171,12 @@ This policy requires a matching certname pattern **and** a valid PSK **and** the |---|---|---|---| | `certificateAuthorityRef` | string | **required** | Reference to the CertificateAuthority | | `any` | bool | `false` | Sign all CSRs unconditionally | -| `pattern` | [PatternSpec](#patternspec) | - | Certname glob matching | -| `dnsAltNames` | [PatternSpec](#patternspec) | - | Allowed DNS SAN patterns. If unset and `any` is false, CSRs with SANs are denied | +| `certnames` | [PatternSpec](#patternspec) | - | Allowed certname glob patterns; the certname must match at least one | +| `dnsAltNames` | [PatternSpec](#patternspec) | - | Allowed DNS SAN glob patterns. If a CSR carries DNS SANs and this is unset, it is denied | +| `ipAltNames` | [PatternSpec](#patternspec) | - | Allowed IP SAN **CIDR** ranges. If a CSR carries IP SANs and this is unset, it is denied | +| `uriAltNames` | [PatternSpec](#patternspec) | - | Allowed URI SAN wildcard patterns (`*` spans `/`). If a CSR carries URI SANs and this is unset, it is denied | +| `emailAltNames` | [PatternSpec](#patternspec) | - | Allowed email SAN wildcard patterns (`*` spans `@`). If a CSR carries email SANs and this is unset, it is denied | +| `extensions` | [PatternSpec](#patternspec) | - | Puppet extension names a CSR may carry. Authorization-arc extensions are denied unless listed here (applies to `any: true` too) | | `csrAttributes` | [][CSRAttributeMatch](#csrattributematch) | - | CSR extension attributes that must all match (AND) | ### PatternSpec @@ -127,6 +206,7 @@ Either `value` or `valueFrom` must be set. | Field | Type | Description | |---|---|---| +| `observedGeneration` | int64 | The `.metadata.generation` the status was last derived from. A value below `.metadata.generation` means the rest of this status has not caught up with the current spec yet | | `phase` | string | Current lifecycle phase | | `conditions` | []Condition | `Ready` | @@ -153,14 +233,17 @@ flowchart TD Any -->|No| Deny Any -->|Yes| Loop["Evaluate next policy"] - Loop --> CheckAny{"any: true?"} + Loop --> CheckExt{"authz extensions
allowed? (guard)"} + CheckExt -->|No| Next + CheckExt -->|Yes / none| CheckSAN{"all SAN types
allowed? (guard)"} + CheckSAN -->|No| Next + + CheckSAN -->|Yes / none| CheckAny{"any: true?"} CheckAny -->|Yes| Sign - CheckAny -->|No| CheckPattern{"pattern matches?"} + CheckAny -->|No| CheckPattern{"certname matches?"} CheckPattern -->|No| Next - CheckPattern -->|Yes / not set| CheckSAN{"dnsAltNames matches?"} - CheckSAN -->|No| Next - CheckSAN -->|Yes / not set| CheckCSR{"csrAttributes match?"} + CheckPattern -->|Yes / not set| CheckCSR{"csrAttributes match?"} CheckCSR -->|No| Next CheckCSR -->|Yes / not set| Sign @@ -171,10 +254,14 @@ flowchart TD Deny["exit 1 (deny)"] ``` +- **Guard plane first**: privileged authorization extensions and every SAN type are fail-closed and evaluated for **every** policy, including `any: true` - **Between policies**: OR -- any matching policy is sufficient -- **Within a policy**: AND -- all set fields must match +- **Within a policy**: AND -- all set match fields must match - **No policies** → deny all -- **`any: true`** → approve unconditionally +- **`any: true`** → approve unconditionally **after** the guard plane passes (it does not waive extension/SAN protection) + +!!! warning "OR composition" + Adding a restrictive policy never removes permission granted by another policy. A CSR is signed if **any** policy both matches it **and** permits its extensions/SANs. Avoid a broad `any: true` policy alongside restrictive ones unless you intend it. ## Supported CSR Attributes diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 062bd9f6..2cebf701 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -43,16 +43,45 @@ condition names it. **Symptoms:** Certificate never reaches `Signed` phase. +A SigningPolicy is usually *not* the cause here. For an internal CA the +operator signs its own Certificate resources over the CA API, authenticated +with the operator signing certificate, so autosign is not involved. Policies +govern agents, not Certificate resources. + **Possible causes:** -1. **CA not ready:** The CertificateAuthority must be in `Ready` phase. -2. **No matching SigningPolicy:** No policy exists that would sign this certificate. +1. **CA not ready.** The CertificateAuthority must report `CAReady`. + + ```bash + kubectl get certificateauthority -n \ + -o jsonpath='{range .status.conditions[*]}{.type}={.status} {.reason}{"\n"}{end}' + ``` + +2. **Operator signing certificate not available yet.** Without it the operator + cannot sign and falls back to polling, which only succeeds if something else + signs the CSR. + + ```bash + kubectl get certificateauthority -n \ + -o jsonpath='{.status.signingSecretName}{"\n"}' + ``` + + An empty value means the `{ca}-operator-signing` Certificate is not signed + yet. During bootstrap this resolves on its own. + +3. **Certname already claimed.** Two Certificates cannot share a certname + against the same CA. The condition names the holder: ```bash - kubectl get signingpolicy -n + kubectl get certificate -n \ + -o jsonpath='{range .status.conditions[*]}{.type}={.status} {.reason}: {.message}{"\n"}{end}' ``` -**Solution:** Check CA status and verify a SigningPolicy with matching criteria exists. +4. **External CA.** With `spec.external` the operator has no admin access and + cannot sign. The CSR must be signed on the external CA. + +**Agents** stuck in `--waitforcert` are the case where SigningPolicy matters - +see [Agents cannot connect to server](#agents-cannot-connect-to-server). ### Server pods not starting @@ -68,7 +97,7 @@ condition names it. ```bash kubectl describe deployment -n -kubectl describe pod -l app.kubernetes.io/instance= -n +kubectl describe pod -l openvox.voxpupuli.org/server= -n kubectl get events -n --sort-by='.lastTimestamp' ``` @@ -110,14 +139,36 @@ kubectl logs -n --previous ### Agents cannot connect to server -**Symptoms:** Puppet agents fail to connect to the server endpoint. +**Symptoms:** Puppet agents fail to connect to the server endpoint, or hang in +`puppet agent --waitforcert`. -**Debugging steps:** +An agent that reaches the server but hangs waiting for its certificate has a +signing problem, not a connectivity one. The operator points `autosign` at its +own binary as soon as a CertificateAuthority exists, and that binary denies +every CSR no policy matches - so **no SigningPolicy means deny-all, not off**. +The servers run normally and the Config reports `Running` either way. + +```bash +kubectl get signingpolicy -n +``` + +An empty list is the common cause on a fresh install. See +[SigningPolicy](reference/signingpolicy.md) for the available match rules, or +sign by hand: + +```bash +kubectl exec -n deploy/ -- \ + puppetserver ca list --all +kubectl exec -n deploy/ -- \ + puppetserver ca sign --certname +``` + +**Debugging steps for connectivity:** 1. Verify the Pool Service exists: ```bash - kubectl get svc -n -l app.kubernetes.io/name=openvox-server + kubectl get svc -n -l app.kubernetes.io/name=openvox ``` 2. Check endpoints are populated: diff --git a/go.mod b/go.mod index bf9bb007..e2e4ff2d 100644 --- a/go.mod +++ b/go.mod @@ -11,22 +11,28 @@ require ( k8s.io/apimachinery v0.37.0 k8s.io/client-go v0.37.0 k8s.io/utils v0.0.0-20260707023825-cf1189d6abe3 - sigs.k8s.io/controller-runtime v0.25.0 - sigs.k8s.io/gateway-api v1.6.1 + sigs.k8s.io/controller-runtime v0.25.1 + sigs.k8s.io/gateway-api v1.6.2 sigs.k8s.io/yaml v1.6.0 ) require ( + cel.dev/expr v0.25.2 // indirect + github.com/antlr4-go/antlr/v4 v4.13.1 // indirect github.com/beorn7/perks v1.0.1 // indirect + github.com/blang/semver/v4 v4.0.0 // indirect + github.com/cenkalti/backoff/v5 v5.0.3 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/emicklei/go-restful/v3 v3.13.0 // indirect github.com/evanphx/json-patch v5.9.11+incompatible // indirect github.com/evanphx/json-patch/v5 v5.9.11 // indirect github.com/fatih/color v1.19.0 // indirect + github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fsnotify/fsnotify v1.10.0 // indirect github.com/fxamacker/cbor/v2 v2.9.1 // indirect github.com/go-logr/logr v1.4.3 // indirect + github.com/go-logr/stdr v1.2.2 // indirect github.com/go-logr/zapr v1.3.0 // indirect github.com/go-openapi/jsonpointer v1.0.0 // indirect github.com/go-openapi/jsonreference v1.0.0 // indirect @@ -43,8 +49,10 @@ require ( github.com/go-openapi/swag/typeutils v0.27.1 // indirect github.com/go-openapi/swag/yamlutils v0.27.1 // indirect github.com/gobuffalo/flect v1.0.3 // indirect + github.com/google/cel-go v0.29.2 // indirect github.com/google/gnostic-models v0.7.1 // indirect github.com/google/uuid v1.6.0 // indirect + github.com/grpc-ecosystem/grpc-gateway/v2 v2.29.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/kylelemons/godebug v1.1.0 // indirect @@ -60,30 +68,47 @@ require ( github.com/spf13/cobra v1.10.2 // indirect github.com/spf13/pflag v1.0.10 // indirect github.com/x448/float16 v0.8.4 // indirect + go.opentelemetry.io/auto/sdk v1.2.1 // indirect + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.69.0 // indirect + go.opentelemetry.io/otel v1.44.0 // indirect + go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.44.0 // indirect + go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.44.0 // indirect + go.opentelemetry.io/otel/metric v1.44.0 // indirect + go.opentelemetry.io/otel/sdk v1.44.0 // indirect + go.opentelemetry.io/otel/trace v1.44.0 // indirect + go.opentelemetry.io/proto/otlp v1.10.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect - golang.org/x/mod v0.39.0 // indirect - golang.org/x/net v0.58.0 // indirect + golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f // indirect + golang.org/x/mod v0.41.0 // indirect + golang.org/x/net v0.59.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect - golang.org/x/sync v0.22.0 // indirect - golang.org/x/sys v0.47.0 // indirect - golang.org/x/telemetry v0.0.0-20260811182544-a038080d80e5 // indirect - golang.org/x/term v0.45.0 // indirect - golang.org/x/text v0.41.0 // indirect + golang.org/x/sync v0.23.0 // indirect + golang.org/x/sys v0.48.0 // indirect + golang.org/x/telemetry v0.0.0-20260908163034-4bcc4b2ee518 // indirect + golang.org/x/term v0.46.0 // indirect + golang.org/x/text v0.42.0 // indirect golang.org/x/time v0.15.0 // indirect - golang.org/x/tools v0.49.0 // indirect - golang.org/x/vuln v1.7.0 // indirect + golang.org/x/tools v0.50.0 // indirect + golang.org/x/vuln v1.8.0 // indirect gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa // indirect + google.golang.org/grpc v1.83.1 // indirect google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af // indirect gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect k8s.io/apiextensions-apiserver v0.37.0 // indirect + k8s.io/apiserver v0.37.0 // indirect k8s.io/code-generator v0.37.0 // indirect + k8s.io/component-base v0.37.0 // indirect k8s.io/gengo/v2 v2.0.0-20260408192533-25e2208e0dc3 // indirect k8s.io/klog/v2 v2.140.0 // indirect k8s.io/kube-openapi v0.0.0-20260721132016-d427ff9ee9ad // indirect + k8s.io/streaming v0.37.0 // indirect + sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.36.0 // indirect sigs.k8s.io/controller-tools v0.22.0 // indirect sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect sigs.k8s.io/randfill v1.0.0 // indirect diff --git a/go.sum b/go.sum index 7fa0f489..34155084 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -cel.dev/expr v0.25.1 h1:1KrZg61W6TWSxuNZ37Xy49ps13NUovb66QLprthtwi4= -cel.dev/expr v0.25.1/go.mod h1:hrXvqGP6G6gyx8UAHSHJ5RGk//1Oj5nXQ2NI02Nrsg4= +cel.dev/expr v0.25.2 h1:K6j46C81hXtZQfuX60cVWQFBJahKSE2gfRbNuvr5bFs= +cel.dev/expr v0.25.2/go.mod h1:hrXvqGP6G6gyx8UAHSHJ5RGk//1Oj5nXQ2NI02Nrsg4= github.com/Masterminds/semver/v3 v3.4.0 h1:Zog+i5UMtVoCU8oKka5P7i9q9HgrJeGzI9SA1Xbatp0= github.com/Masterminds/semver/v3 v3.4.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/antlr4-go/antlr/v4 v4.13.1 h1:SqQKkuVZ+zWkMMNkjy5FZe5mr5WURWnlpmOuzYWrPrQ= @@ -31,6 +31,7 @@ github.com/fsnotify/fsnotify v1.10.0 h1:Xx/5Ydg9CeBDX/wi4VJqStNtohYjitZhhlHt4h3S github.com/fsnotify/fsnotify v1.10.0/go.mod h1:TLheqan6HD6GBK6PrDWyDPBaEV8LspOxvPSjC+bVfgo= github.com/fxamacker/cbor/v2 v2.9.1 h1:2rWm8B193Ll4VdjsJY28jxs70IdDsHRWgQYAI80+rMQ= github.com/fxamacker/cbor/v2 v2.9.1/go.mod h1:vM4b+DJCtHn+zz7h3FFp/hDAI9WNWCsZj23V5ytsSxQ= +github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= @@ -75,6 +76,8 @@ github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1v github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= github.com/gobuffalo/flect v1.0.3 h1:xeWBM2nui+qnVvNM4S3foBhCAL2XgPU+a7FdpelbTq4= github.com/gobuffalo/flect v1.0.3/go.mod h1:A5msMlrHtLqh9umBSnvabjsMrCcCpAyzglnDvkbYKHs= +github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= +github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/cel-go v0.29.2 h1:ZtDxkeiMmz0mxbKDYiNkE5Lk7V5edMRcaaDf2jX002k= github.com/google/cel-go v0.29.2/go.mod h1:X0bD6iVNR8pkROSOoHVdgTkzmRcosof7WQqCD6wcMc8= github.com/google/gnostic-models v0.7.1 h1:SisTfuFKJSKM5CPZkffwi6coztzzeYUhc3v4yxLWH8c= @@ -168,6 +171,8 @@ go.opentelemetry.io/otel/metric v1.44.0 h1:1w0gILTcHdr3YI+ixLyjemwrVnsMURbTZFrSY go.opentelemetry.io/otel/metric v1.44.0/go.mod h1:8O7hanEPBNgEMmybD3s2VBKcgWOCsA6tzHBPODAiquo= go.opentelemetry.io/otel/sdk v1.44.0 h1:nHYwb9lK+fJPU/dnT6s7W7Z8itMWyqrnVfbheVYrZ58= go.opentelemetry.io/otel/sdk v1.44.0/go.mod h1:Osuydd3Se74nqjAKxid74N5eC+jfEqfTegHRnq58oK0= +go.opentelemetry.io/otel/sdk/metric v1.44.0 h1:3LlKgI+VjbVsjNRFZJZAJ30WjXC5VkNRks6si09iEfI= +go.opentelemetry.io/otel/sdk/metric v1.44.0/go.mod h1:5B5pMARnXxKhltooO4xUuCBorl65a4EpnTalObqOigA= go.opentelemetry.io/otel/trace v1.44.0 h1:jxF5CsGYCe74MCRx2X4g7WsY/VBKRqqpNvXlX/6gtIk= go.opentelemetry.io/otel/trace v1.44.0/go.mod h1:oLl1jrMQAVo6v3GAggN+1VH9VIz9iUSvW53sW1Q8PIE= go.opentelemetry.io/proto/otlp v1.10.0 h1:IQRWgT5srOCYfiWnpqUYz9CVmbO8bFmKcwYxpuCSL2g= @@ -185,41 +190,43 @@ go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f h1:W3F4c+6OLc6H2lb//N1q4WpJkhzJCK5J6kUi1NTVXfM= golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f/go.mod h1:J1xhfL/vlindoeF/aINzNzt2Bket5bjo9sdOYzOsU80= -golang.org/x/mod v0.39.0 h1:UF5zwQdCRRUpHfyPwr7d4UrGiVeldIsogtzWVnczL74= -golang.org/x/mod v0.39.0/go.mod h1:bvIbwjQ0HUFFf5AKukeeYQG4ZBUG9yxQbR9aEweIwYY= -golang.org/x/net v0.58.0 h1:ynWG7rqYi4ccpTEuPZ2QGWHktVEM9DMCj9yzDE0Q7To= -golang.org/x/net v0.58.0/go.mod h1:YwCddHnFlT7eLQqVprV19OnhLGtc5xOKgE0RyqgfWAU= +golang.org/x/mod v0.41.0 h1:qJmnOUb4YB+FsEuM3HcWucdZASCPGhsX6uljO6pog0c= +golang.org/x/mod v0.41.0/go.mod h1:Ek9pY8RKWXwsWvd3rQiHYtMqkjSUV+s1Rj7j4H5Ur6o= +golang.org/x/net v0.59.0 h1:5zfYln+w5XCxwrnMMJPufRgNoXEaGxl0wo5GqPXyues= +golang.org/x/net v0.59.0/go.mod h1:2DA/G1UfVbCpQPeWTmMPGY7Cs2PkBkwu743bVX5PIVg= golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= -golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= -golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk= +golang.org/x/sync v0.23.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= -golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/telemetry v0.0.0-20260811182544-a038080d80e5 h1:ZUSxONxc981v7AW7QUg+I9WwZzSTTJ019ENBYr5pV/Q= -golang.org/x/telemetry v0.0.0-20260811182544-a038080d80e5/go.mod h1:LVehoXe41cL5SCVQilsV7Gg6BNG+Js6P9PhSbYTIUkQ= -golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= -golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= -golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8= -golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M= +golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo= +golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og= +golang.org/x/telemetry v0.0.0-20260908163034-4bcc4b2ee518 h1:F5BWKvW126NXR74uxkxuc1jQHhm/rwm/J3rSiFyuRs4= +golang.org/x/telemetry v0.0.0-20260908163034-4bcc4b2ee518/go.mod h1:i+ivNqjDnTF3WTElsdk5g9V5DTSBYgdNo7xTU9SDwYA= +golang.org/x/term v0.46.0 h1:3+OXuTbaKDgwk8jTi3aSLHRlmWqHEUDUtxnbFigO4YE= +golang.org/x/term v0.46.0/go.mod h1:+K02xbkittuwc0Am4abfA3Fc+XRGXkvBXNO88NCXPoc= +golang.org/x/text v0.42.0 h1:JbOZXgfeCPU9gacVtYliJqOhD+zhrEqK4LfdpmlUZqI= +golang.org/x/text v0.42.0/go.mod h1:ojzP1Z+2QtioaF8DTtO8K5q7JWVVYwZKenzujK0Zd0E= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= -golang.org/x/tools v0.49.0 h1:3NI7VXzL9+1WZD52Dx2ttoPwD5DWrFGpl9mFZDlmisI= -golang.org/x/tools v0.49.0/go.mod h1:SJNXV9DBKT0UbdttsQjbfJlAE/q+y36++zo3uL3N0Oo= +golang.org/x/tools v0.50.0 h1:c2ifzfcuY7L90lZ2aKd8S4K2NpASF08SZx9ZuJkHmSU= +golang.org/x/tools v0.50.0/go.mod h1:7ulVMw3831Mwi5EZD6RomGyffr4VFjuNYXf2BbCEAV0= golang.org/x/tools/go/expect v0.1.1-deprecated h1:jpBZDwmgPhXsKZC6WhL20P4b/wmnpsEAGHaNy0n/rJM= golang.org/x/tools/go/expect v0.1.1-deprecated/go.mod h1:eihoPOH+FgIqa3FpoTwguz/bVUSGBlGQU67vpBeOrBY= golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated h1:1h2MnaIAIXISqTFKdENegdpAgUXz6NrPEsbIeWaBRvM= golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated/go.mod h1:RVAQXBGNv1ib0J382/DPCRS/BPnsGebyM1Gj5VSDpG8= -golang.org/x/vuln v1.7.0 h1:4MQBuhmXbz2uepNJrf3v+aaZLGDqw1JluwYboegA1qg= -golang.org/x/vuln v1.7.0/go.mod h1:Xw7zvU3e1bsCYYBXu+w4wcn2Kgn27f34WBCTw8LL5Us= +golang.org/x/vuln v1.8.0 h1:clG4qBU6zH5VKjti8n5j8BBuYzoSha392xXMkXS351U= +golang.org/x/vuln v1.8.0/go.mod h1:Fzm4XK3Hbl1ZvZ7JpNTEWb7CJWOZ7m2LX0GLu4Fsrwo= gomodules.xyz/jsonpatch/v2 v2.5.0 h1:JELs8RLM12qJGXU4u/TO3V25KW8GreMKl9pdkk14RM0= gomodules.xyz/jsonpatch/v2 v2.5.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY= +gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= +gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa h1:Kjn0N0tCrDgiAFW+lGO4JZ3ck44CehvJQMAwj9QF0G8= google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa/go.mod h1:q4lMZS6kskjT5HvCPrnnypcDPVJqT/f4nfxmkE7gryY= google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa h1:mZHHdPZl0dbGHCflZgAq/Q468DWVFcU2whhB2KAo8fk= google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= -google.golang.org/grpc v1.82.1 h1:NnAxzGRA0677vCa4BUkOAnO5+FfQqVl9iUXeD0IqcGE= -google.golang.org/grpc v1.82.1/go.mod h1:yzTZ1TB1Z3SG+LIYaI+WiE8D5+PZ3ArnrSp8zF3+/ZA= +google.golang.org/grpc v1.83.1 h1:HIO0+BEtBP6soyqvqC8sNUjZ7bTs+0hFQuFF+RAy++Y= +google.golang.org/grpc v1.83.1/go.mod h1:kDyl6SKsiHKt0uylY5gtn5cEjkrIOhQOGDgIc4JGwzQ= google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af h1:+5/Sw3GsDNlEmu7TfklWKPdQ0Ykja5VEmq2i817+jbI= google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= @@ -252,16 +259,18 @@ k8s.io/klog/v2 v2.140.0 h1:Tf+J3AH7xnUzZyVVXhTgGhEKnFqye14aadWv7bzXdzc= k8s.io/klog/v2 v2.140.0/go.mod h1:o+/RWfJ6PwpnFn7OyAG3QnO47BFsymfEfrz6XyYSSp0= k8s.io/kube-openapi v0.0.0-20260721132016-d427ff9ee9ad h1:oXImqH8mQNk7PmvzKhmN3ddJoY6OnyM225MXwGHPm0A= k8s.io/kube-openapi v0.0.0-20260721132016-d427ff9ee9ad/go.mod h1:0/mqHCVhlumdJ3BhCfnjSZQE037nAhNodh1/hK0T8/I= +k8s.io/streaming v0.37.0 h1:iPBUZLZiKt5bV+lxJurASMOV07VuBhNpiwJt2//AWrM= +k8s.io/streaming v0.37.0/go.mod h1:APlJR26ZWRcVy5bIEj0QRrKUXROtBHPcxl2NT7EAzPU= k8s.io/utils v0.0.0-20260707023825-cf1189d6abe3 h1:jVkFFVfXdXP74B/zbO3hM3hpSFD0xvhQ5U686DPurkE= k8s.io/utils v0.0.0-20260707023825-cf1189d6abe3/go.mod h1:M2s5JB1lIYP3jzZdorPLHXIPJzt9vv2muW5a6L9DtNM= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.36.0 h1:/YpDJ4vReG7ZmzSpBGxduXgywWkJU9zHubgJG03MT+Y= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.36.0/go.mod h1:tJo1aepTXyR+8Xs3sUsGBDk4Ub2AM5dPAPKJx0mpm5c= -sigs.k8s.io/controller-runtime v0.25.0 h1:44KgRUPew331KSJpNu8zJow3iTR5W0p/SfrHdw3lV40= -sigs.k8s.io/controller-runtime v0.25.0/go.mod h1:4QqLdT6z/L6Olj8JJCtvztid4/fnIiYsfaTFScegctc= +sigs.k8s.io/controller-runtime v0.25.1 h1:BKgU9OeE8xv8EbbM8cY0NVzTQs35rokkdq1jh12fMb4= +sigs.k8s.io/controller-runtime v0.25.1/go.mod h1:4QqLdT6z/L6Olj8JJCtvztid4/fnIiYsfaTFScegctc= sigs.k8s.io/controller-tools v0.22.0 h1:eG3FAVja/KnlXKIWg95udIFz1cMyAtMjP11cqBh3t+k= sigs.k8s.io/controller-tools v0.22.0/go.mod h1:VizwUStoZK7rReCj704czGGrB7mLxXTiJSJt7wN5ilI= -sigs.k8s.io/gateway-api v1.6.1 h1:mock6phZbI6rvZerwrVNk7hVNymQgHo+6sJ81Ia7ftY= -sigs.k8s.io/gateway-api v1.6.1/go.mod h1:FVfx3t389ybeXOqvDghLbdvJdSCfI/PReqCUI3lu3mY= +sigs.k8s.io/gateway-api v1.6.2 h1:vh5YzKlbdBivEaLX61+APKLGRq4tZ7Fj4XfGkv08xB4= +sigs.k8s.io/gateway-api v1.6.2/go.mod h1:FVfx3t389ybeXOqvDghLbdvJdSCfI/PReqCUI3lu3mY= sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 h1:IpInykpT6ceI+QxKBbEflcR5EXP7sU1kvOlxwZh5txg= sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730/go.mod h1:mdzfpAEoE6DHQEN0uh9ZbOCuHbLK5wOm7dK4ctXE9Tg= sigs.k8s.io/randfill v1.0.0 h1:JfjMILfT8A6RbawdsK2JXGBR5AQVfd+9TbzrlneTyrU= diff --git a/images/openvox-agent/Containerfile b/images/openvox-agent/Containerfile index 1d0eca01..4a9cc543 100644 --- a/images/openvox-agent/Containerfile +++ b/images/openvox-agent/Containerfile @@ -13,7 +13,7 @@ ARG OPENVOX_AGENT_VERSION=8.28.1 # OPENVOX_MAJOR selects the release repo (openvox${OPENVOX_MAJOR}-release-el-9). ARG OPENVOX_MAJOR=8 -FROM registry.access.redhat.com/ubi9/ubi:9.8-1788245065 +FROM registry.access.redhat.com/ubi9/ubi:9.8-1789646010 ARG OPENVOX_AGENT_VERSION ARG OPENVOX_MAJOR diff --git a/images/openvox-db/Containerfile b/images/openvox-db/Containerfile index 073ed00b..d494ae6a 100644 --- a/images/openvox-db/Containerfile +++ b/images/openvox-db/Containerfile @@ -16,7 +16,7 @@ ARG OPENVOXDB_VERSION=8.15.0 ################################################################################ # Stage: base — JRE + minimal runtime deps ################################################################################ -FROM registry.access.redhat.com/ubi9/ubi:9.8-1788245065 AS base +FROM registry.access.redhat.com/ubi9/ubi:9.8-1789646010 AS base ARG JDK_VERSION=21 diff --git a/images/openvox-e2e-code/Containerfile b/images/openvox-e2e-code/Containerfile index 79062e72..6f0cbf1d 100644 --- a/images/openvox-e2e-code/Containerfile +++ b/images/openvox-e2e-code/Containerfile @@ -8,7 +8,7 @@ # podman build -t openvox-e2e-code:latest -f images/openvox-e2e-code/Containerfile . # Stage 1: Install modules with r10k -FROM registry.access.redhat.com/ubi9/ubi:9.8-1788245065 AS builder +FROM registry.access.redhat.com/ubi9/ubi:9.8-1789646010 AS builder RUN dnf module enable ruby:3.3 -y \ && dnf install -y --setopt=install_weak_deps=False ruby ruby-devel rubygem-bundler git gcc make redhat-rpm-config libffi-devel \ diff --git a/images/openvox-e2e-code/environments/production/Puppetfile b/images/openvox-e2e-code/environments/production/Puppetfile index 816bcdba..feade239 100644 --- a/images/openvox-e2e-code/environments/production/Puppetfile +++ b/images/openvox-e2e-code/environments/production/Puppetfile @@ -1,3 +1,3 @@ mod 'puppetlabs-stdlib', :git => 'https://github.com/puppetlabs/puppetlabs-stdlib.git', - :tag => 'v10.0.2' + :tag => 'v10.1.0' diff --git a/images/openvox-mock/Containerfile b/images/openvox-mock/Containerfile index 1da9398f..dd72a7f0 100644 --- a/images/openvox-mock/Containerfile +++ b/images/openvox-mock/Containerfile @@ -11,7 +11,7 @@ RUN go mod download COPY cmd/mock/ cmd/mock/ RUN CGO_ENABLED=0 go build -o /openvox-mock ./cmd/mock/ -FROM registry.access.redhat.com/ubi9/ubi-minimal:9.8-1788166357 +FROM registry.access.redhat.com/ubi9/ubi-minimal:9.8-1789546276 LABEL org.opencontainers.image.title="OpenVox Mock" \ org.opencontainers.image.description="Mock ENC/Report/OpenVox DB receiver for E2E tests" \ diff --git a/images/openvox-operator/Containerfile b/images/openvox-operator/Containerfile index e6c1f76b..4a4739b5 100644 --- a/images/openvox-operator/Containerfile +++ b/images/openvox-operator/Containerfile @@ -9,7 +9,7 @@ COPY internal/ internal/ ARG TARGETARCH RUN CGO_ENABLED=0 GOOS=linux GOARCH=${TARGETARCH} go build -a -o manager ./cmd/main.go -FROM registry.access.redhat.com/ubi9/ubi-minimal:9.8-1788166357 +FROM registry.access.redhat.com/ubi9/ubi-minimal:9.8-1789546276 LABEL org.opencontainers.image.title="OpenVox Operator" \ org.opencontainers.image.description="OpenVox Operator for Kubernetes/OpenShift" \ diff --git a/images/openvox-server-reference/Containerfile b/images/openvox-server-reference/Containerfile index e18a9460..5658ad38 100644 --- a/images/openvox-server-reference/Containerfile +++ b/images/openvox-server-reference/Containerfile @@ -4,7 +4,7 @@ # Build: # podman build -t openvox-server-reference:latest images/openvox-server-reference/ -FROM registry.access.redhat.com/ubi9/ubi:9.8-1788245065 +FROM registry.access.redhat.com/ubi9/ubi:9.8-1789646010 RUN rpm -Uvh https://yum.voxpupuli.org/openvox8-release-el-9.noarch.rpm \ && dnf install -y --setopt=install_weak_deps=False openvox-server \ diff --git a/images/openvox-server/Containerfile b/images/openvox-server/Containerfile index 02f425f4..cb800bb8 100644 --- a/images/openvox-server/Containerfile +++ b/images/openvox-server/Containerfile @@ -17,7 +17,7 @@ ARG OPENVOX_VERSION=8.28.1 ################################################################################ # Stage: base — JRE + minimal runtime deps (no Ruby) ################################################################################ -FROM registry.access.redhat.com/ubi9/ubi:9.8-1788245065 AS base +FROM registry.access.redhat.com/ubi9/ubi:9.8-1789646010 AS base ARG JDK_VERSION=21 @@ -150,13 +150,6 @@ RUN printf '%s\n' \ > /etc/sysconfig/puppetserver \ && puppetserver gem install --no-document openvox -v "${OPENVOX_VERSION}" -# Patch openvoxserver-ca to skip cadir symlink and chown (fails rootless) -RUN find / -path '*/openvoxserver-ca-*/lib/puppetserver/ca/action/setup.rb' \ - -exec sed -i '/Puppetserver::Ca::Utils::Config\.symlink_to_old_cadir/ s/^/# /' {} + 2>/dev/null \ - ; find / -path '*/openvoxserver-ca-*/lib/puppetserver/ca/utils/file_system.rb' \ - -exec sed -i 's/FileUtils\.chown/# FileUtils.chown/' {} + 2>/dev/null \ - ; true - ################################################################################ # Stage: autosign — compile the openvox-autosign Go binary ################################################################################ diff --git a/images/openvox-versions.yaml b/images/openvox-versions.yaml index 20a388de..e4e55ab0 100644 --- a/images/openvox-versions.yaml +++ b/images/openvox-versions.yaml @@ -14,15 +14,15 @@ include: - major: "8" latest: true # renovate: datasource=github-releases depName=OpenVoxProject/openvox-server - server: 8.15.2 + server: 8.16.0 # renovate: datasource=github-releases depName=OpenVoxProject/openvoxdb - termini: 8.15.0 + termini: 8.16.0 # renovate: datasource=github-releases depName=OpenVoxProject/openvox - openvox: 8.28.1 + openvox: 8.29.0 # renovate: datasource=github-releases depName=OpenVoxProject/openvoxdb - db: 8.15.0 + db: 8.16.0 # renovate: datasource=github-releases depName=OpenVoxProject/openvox - agent: 8.28.1 + agent: 8.29.0 # Re-enable at OpenVox 9.0 GA (also fix server/db to install from RPM, not tarball): # - major: "9" # latest: false diff --git a/internal/controller/ca_config_uniqueness_test.go b/internal/controller/ca_config_uniqueness_test.go index 5f6aeff4..6e4d33eb 100644 --- a/internal/controller/ca_config_uniqueness_test.go +++ b/internal/controller/ca_config_uniqueness_test.go @@ -27,7 +27,7 @@ func TestFindConfigForCA_Deterministic(t *testing.T) { ) r := newCertificateAuthorityReconciler(c) - for i := 0; i < 5; i++ { + for i := range 5 { cfg, err := r.findConfigForCA(testCtx(), ca) if err != nil { t.Fatalf("lookup %d: %v", i, err) diff --git a/internal/controller/certificate_controller.go b/internal/controller/certificate_controller.go index 0cf9edd5..fc1f7583 100644 --- a/internal/controller/certificate_controller.go +++ b/internal/controller/certificate_controller.go @@ -9,7 +9,7 @@ import ( "time" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -83,7 +83,7 @@ func (r *CertificateReconciler) Reconcile(ctx context.Context, req ctrl.Request) cert := &openvoxv1alpha1.Certificate{} if err := r.Get(ctx, req.NamespacedName, cert); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { forgetCertificateMetrics(req.Name, req.Namespace) return ctrl.Result{}, nil } @@ -158,7 +158,7 @@ func (r *CertificateReconciler) Reconcile(ctx context.Context, req ctrl.Request) // Resolve CertificateAuthority ca := &openvoxv1alpha1.CertificateAuthority{} if err := r.Get(ctx, types.NamespacedName{Name: cert.Spec.AuthorityRef, Namespace: cert.Namespace}, ca); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("waiting for CertificateAuthority", "authorityRef", cert.Spec.AuthorityRef) return ctrl.Result{}, nil } @@ -401,7 +401,7 @@ func (r *CertificateReconciler) adoptTLSSecret(ctx context.Context, cert *openvo func (r *CertificateReconciler) extractNotAfter(ctx context.Context, secretName, namespace string) *metav1.Time { secret := &corev1.Secret{} if err := r.Get(ctx, types.NamespacedName{Name: secretName, Namespace: namespace}, secret); err != nil { - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { log.FromContext(ctx).Error(err, "failed to get TLS Secret", "name", secretName, "namespace", namespace) } return nil @@ -462,10 +462,7 @@ func (r *CertificateReconciler) scheduleRenewalCheck(ctx context.Context, cert * r.emitExpiryWarnings(ctx, cert, timeUntilExpiry) // Schedule next check: half the time until renewal, capped at 12h - requeueAfter := timeUntilRenewal / 2 - if requeueAfter > maxRenewalCheckInterval { - requeueAfter = maxRenewalCheckInterval - } + requeueAfter := min(timeUntilRenewal/2, maxRenewalCheckInterval) return ctrl.Result{RequeueAfter: requeueAfter}, nil } @@ -544,7 +541,7 @@ func (r *CertificateReconciler) handleCertificateCleanup(ctx context.Context, ce // Resolve CertificateAuthority ca := &openvoxv1alpha1.CertificateAuthority{} if err := r.Get(ctx, types.NamespacedName{Name: cert.Spec.AuthorityRef, Namespace: cert.Namespace}, ca); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("CertificateAuthority not found, skipping cleanup", "authorityRef", cert.Spec.AuthorityRef) return nil } diff --git a/internal/controller/certificate_controller_test.go b/internal/controller/certificate_controller_test.go index e4f8d652..9c307cb8 100644 --- a/internal/controller/certificate_controller_test.go +++ b/internal/controller/certificate_controller_test.go @@ -1,11 +1,12 @@ package controller import ( + "slices" "testing" "time" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" clocktesting "k8s.io/utils/clock/testing" @@ -444,13 +445,7 @@ func TestCertReconcile_FinalizerAdded(t *testing.T) { t.Fatalf("failed to get Certificate: %v", err) } - found := false - for _, f := range updated.Finalizers { - if f == certificateFinalizer { - found = true - break - } - } + found := slices.Contains(updated.Finalizers, certificateFinalizer) if !found { t.Error("expected finalizer to be added to Certificate") } @@ -488,7 +483,7 @@ func TestCertReconcile_DeletionCleansUp(t *testing.T) { // Object should be gone (fake client deletes once last finalizer is removed) updated := &openvoxv1alpha1.Certificate{} err = c.Get(testCtx(), types.NamespacedName{Name: "my-cert", Namespace: testNamespace}, updated) - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { t.Errorf("expected Certificate to be deleted, got err: %v", err) } } @@ -513,7 +508,7 @@ func TestCertReconcile_DeletionCANotFound(t *testing.T) { // Object should be gone (CA missing -> cleanup skipped -> finalizer removed) updated := &openvoxv1alpha1.Certificate{} err = c.Get(testCtx(), types.NamespacedName{Name: "my-cert", Namespace: testNamespace}, updated) - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { t.Errorf("expected Certificate to be deleted, got err: %v", err) } } diff --git a/internal/controller/certificate_derived_test.go b/internal/controller/certificate_derived_test.go index 6e25948d..91ef1929 100644 --- a/internal/controller/certificate_derived_test.go +++ b/internal/controller/certificate_derived_test.go @@ -20,7 +20,7 @@ func TestEffectiveDNSAltNames_AddsTheRouteHostname(t *testing.T) { server.Spec.CertificateRef = "web-cert" server.Spec.PoolRefs = []string{"puppet"} - pool := newPool("puppet", withRoute(true, "puppet.example.com", "gw")) + pool := newPool("puppet", withRoute("puppet.example.com", "gw")) pool.Spec.Route.InjectDNSAltName = true r := newCertificateReconciler(setupTestClient(cert, server, pool)) @@ -43,7 +43,7 @@ func TestEffectiveDNSAltNames_LeavesTheSpecAlone(t *testing.T) { server := newServer("web") server.Spec.CertificateRef = "web-cert" server.Spec.PoolRefs = []string{"puppet"} - pool := newPool("puppet", withRoute(true, "puppet.example.com", "gw")) + pool := newPool("puppet", withRoute("puppet.example.com", "gw")) pool.Spec.Route.InjectDNSAltName = true c := setupTestClient(cert, server, pool) @@ -69,7 +69,7 @@ func TestEffectiveDNSAltNames_IsIdempotent(t *testing.T) { server := newServer("web") server.Spec.CertificateRef = "web-cert" server.Spec.PoolRefs = []string{"puppet"} - pool := newPool("puppet", withRoute(true, "puppet.example.com", "gw")) + pool := newPool("puppet", withRoute("puppet.example.com", "gw")) pool.Spec.Route.InjectDNSAltName = true r := newCertificateReconciler(setupTestClient(cert, server, pool)) @@ -96,7 +96,7 @@ func TestEffectiveDNSAltNames_IgnoresPoolsWithoutInjection(t *testing.T) { server := newServer("web") server.Spec.CertificateRef = "web-cert" server.Spec.PoolRefs = []string{"puppet"} - pool := newPool("puppet", withRoute(true, "puppet.example.com", "gw")) // InjectDNSAltName stays false + pool := newPool("puppet", withRoute("puppet.example.com", "gw")) // InjectDNSAltName stays false r := newCertificateReconciler(setupTestClient(cert, server, pool)) names, err := r.effectiveDNSAltNames(testCtx(), cert) @@ -121,7 +121,7 @@ func TestEffectiveDNSAltNames_IgnoresUnrelatedServers(t *testing.T) { notJoined.Spec.CertificateRef = "web-cert" notJoined.Spec.PoolRefs = nil - pool := newPool("puppet", withRoute(true, "puppet.example.com", "gw")) + pool := newPool("puppet", withRoute("puppet.example.com", "gw")) pool.Spec.Route.InjectDNSAltName = true r := newCertificateReconciler(setupTestClient(cert, otherServer, notJoined, pool)) @@ -142,7 +142,7 @@ func TestEffectiveDNSAltNames_IgnoresTerminatingPool(t *testing.T) { server.Spec.CertificateRef = "web-cert" server.Spec.PoolRefs = []string{"puppet"} - pool := newPool("puppet", withRoute(true, "puppet.example.com", "gw")) + pool := newPool("puppet", withRoute("puppet.example.com", "gw")) pool.Spec.Route.InjectDNSAltName = true now := metav1.Now() pool.DeletionTimestamp = &now @@ -167,7 +167,7 @@ func TestEnqueueCertificatesForPool(t *testing.T) { unrelated := newServer("other") unrelated.Spec.CertificateRef = "other-cert" unrelated.Spec.PoolRefs = []string{"different-pool"} - pool := newPool("puppet", withRoute(true, "puppet.example.com", "gw")) + pool := newPool("puppet", withRoute("puppet.example.com", "gw")) c := setupTestClient(server, unrelated, pool) got := certificatesForPool(c)(testCtx(), pool) diff --git a/internal/controller/certificate_signing.go b/internal/controller/certificate_signing.go index 6669ac36..e72abee3 100644 --- a/internal/controller/certificate_signing.go +++ b/internal/controller/certificate_signing.go @@ -20,7 +20,7 @@ import ( "time" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -251,11 +251,12 @@ func (r *CertificateReconciler) submitCSR(ctx context.Context, cert *openvoxv1al if err != nil { logger.Error(err, "failed to read CSR response body") } - if resp.StatusCode == http.StatusOK { + switch { + case resp.StatusCode == http.StatusOK: logger.Info("CSR submitted successfully", "certname", certname) - } else if resp.StatusCode == http.StatusBadRequest && strings.Contains(string(body), "already has a requested certificate") { + case resp.StatusCode == http.StatusBadRequest && strings.Contains(string(body), "already has a requested certificate"): logger.Info("CSR already pending", "certname", certname) - } else { + default: return ctrl.Result{RequeueAfter: RequeueIntervalCRL}, fmt.Errorf("CA rejected CSR (HTTP %d): %s", resp.StatusCode, string(body)) } @@ -499,7 +500,7 @@ func (r *CertificateReconciler) signCertificate(ctx context.Context, cert *openv } // Clean up pending Secret - if err := r.Delete(ctx, pendingSecret); err != nil && !errors.IsNotFound(err) { + if err := r.Delete(ctx, pendingSecret); err != nil && !apierrors.IsNotFound(err) { logger.Info("failed to delete pending Secret", "error", err) } @@ -729,7 +730,7 @@ func (r *CertificateReconciler) ensurePendingKey(ctx context.Context, cert *open if err == nil && len(pendingSecret.Data["key.pem"]) > 0 { return pendingSecret.Data["key.pem"], nil } - if err != nil && !errors.IsNotFound(err) { + if err != nil && !apierrors.IsNotFound(err) { return nil, fmt.Errorf("checking pending Secret: %w", err) } @@ -755,7 +756,7 @@ func (r *CertificateReconciler) ensurePendingKey(ctx context.Context, cert *open if err := controllerutil.SetControllerReference(cert, secret, r.Scheme); err != nil { return nil, fmt.Errorf("setting owner reference on pending Secret %s: %w", pendingSecretName, err) } - if err := r.Create(ctx, secret); err != nil && !errors.IsAlreadyExists(err) { + if err := r.Create(ctx, secret); err != nil && !apierrors.IsAlreadyExists(err) { return nil, fmt.Errorf("creating pending Secret: %w", err) } return keyPEM, nil diff --git a/internal/controller/certificate_spec_drift_test.go b/internal/controller/certificate_spec_drift_test.go index f9fc6865..70b881e6 100644 --- a/internal/controller/certificate_spec_drift_test.go +++ b/internal/controller/certificate_spec_drift_test.go @@ -13,8 +13,8 @@ import ( // signedCert builds a Certificate that is already signed, with the spec hash // and expiry a freshly issued certificate would have. -func signedCert(name string, notAfter time.Time) *openvoxv1alpha1.Certificate { - cert := newCertificate(name, "production-ca", openvoxv1alpha1.CertificatePhaseSigned) +func signedCert(notAfter time.Time) *openvoxv1alpha1.Certificate { + cert := newCertificate("web", "production-ca", openvoxv1alpha1.CertificatePhaseSigned) cert.Spec.DNSAltNames = []string{"puppet.example.com"} cert.Status.SignedSpecHash = specHash(cert) t := metav1.NewTime(notAfter) @@ -23,7 +23,7 @@ func signedCert(name string, notAfter time.Time) *openvoxv1alpha1.Certificate { } func TestSigningSpecHash(t *testing.T) { - base := signedCert("web", time.Now().Add(365*24*time.Hour)) + base := signedCert(time.Now().Add(365 * 24 * time.Hour)) t.Run("stable across equal specs", func(t *testing.T) { if specHash(base.DeepCopy()) != specHash(base) { @@ -82,7 +82,7 @@ func TestRenewalDue_DerivedFromObservedState(t *testing.T) { r := &CertificateReconciler{Clock: testclock.NewFakePassiveClock(now)} t.Run("not due outside the window", func(t *testing.T) { - cert := signedCert("web", now.Add(200*24*time.Hour)) + cert := signedCert(now.Add(200 * 24 * time.Hour)) cert.Spec.RenewBefore = "60d" if r.renewalDue(cert) { t.Error("renewal should not be due 200 days before expiry with renewBefore 60d") @@ -90,7 +90,7 @@ func TestRenewalDue_DerivedFromObservedState(t *testing.T) { }) t.Run("due inside the window", func(t *testing.T) { - cert := signedCert("web", now.Add(30*24*time.Hour)) + cert := signedCert(now.Add(30 * 24 * time.Hour)) cert.Spec.RenewBefore = "60d" if !r.renewalDue(cert) { t.Error("renewal should be due 30 days before expiry with renewBefore 60d") @@ -98,7 +98,7 @@ func TestRenewalDue_DerivedFromObservedState(t *testing.T) { }) t.Run("suppressed by the cooldown annotation", func(t *testing.T) { - cert := signedCert("web", now.Add(30*24*time.Hour)) + cert := signedCert(now.Add(30 * 24 * time.Hour)) cert.Spec.RenewBefore = "60d" cert.Annotations = map[string]string{ AnnotationLastRenewalTime: now.Add(-1 * time.Minute).Format(time.RFC3339), @@ -109,7 +109,7 @@ func TestRenewalDue_DerivedFromObservedState(t *testing.T) { }) t.Run("phase is irrelevant", func(t *testing.T) { - cert := signedCert("web", now.Add(30*24*time.Hour)) + cert := signedCert(now.Add(30 * 24 * time.Hour)) cert.Spec.RenewBefore = "60d" cert.Status.Phase = "" if !r.renewalDue(cert) { @@ -124,7 +124,7 @@ func TestRenewalDue_DerivedFromObservedState(t *testing.T) { func TestReconcile_ResignsOnSpecDrift(t *testing.T) { ca := newCertificateAuthority("production-ca") ca.Status.Phase = openvoxv1alpha1.CertificateAuthorityPhaseReady - cert := signedCert("web", time.Now().Add(365*24*time.Hour)) + cert := signedCert(time.Now().Add(365 * 24 * time.Hour)) c := setupTestClient(ca, cert) r := newCertificateReconciler(c) @@ -167,7 +167,7 @@ func TestReconcile_ResignsOnSpecDrift(t *testing.T) { func TestReconcile_AdoptsMissingHashWithoutResigning(t *testing.T) { ca := newCertificateAuthority("production-ca") ca.Status.Phase = openvoxv1alpha1.CertificateAuthorityPhaseReady - cert := signedCert("web", time.Now().Add(365*24*time.Hour)) + cert := signedCert(time.Now().Add(365 * 24 * time.Hour)) cert.Status.SignedSpecHash = "" // pre-upgrade state c := setupTestClient(ca, cert) diff --git a/internal/controller/certificateauthority_controller.go b/internal/controller/certificateauthority_controller.go index c4465dd2..9c955a69 100644 --- a/internal/controller/certificateauthority_controller.go +++ b/internal/controller/certificateauthority_controller.go @@ -8,7 +8,7 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -66,7 +66,7 @@ func (r *CertificateAuthorityReconciler) Reconcile(ctx context.Context, req ctrl ca := &openvoxv1alpha1.CertificateAuthority{} if err := r.Get(ctx, req.NamespacedName, ca); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { forgetCertificateAuthorityMetrics(req.Name, req.Namespace) return ctrl.Result{}, nil } @@ -376,7 +376,7 @@ func (r *CertificateAuthorityReconciler) reconcileExternalCA(ctx context.Context caSecretName = ext.CASecretRef secret := &corev1.Secret{} if err := r.Get(ctx, types.NamespacedName{Name: ext.CASecretRef, Namespace: ca.Namespace}, secret); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("waiting for CA Secret referenced by external CA", "secret", ext.CASecretRef) return ctrl.Result{RequeueAfter: RequeueIntervalShort}, nil } @@ -433,7 +433,7 @@ func (r *CertificateAuthorityReconciler) reconcileExternalCA(ctx context.Context func (r *CertificateAuthorityReconciler) adoptSecret(ctx context.Context, ca *openvoxv1alpha1.CertificateAuthority, secretName string) error { secret := &corev1.Secret{} if err := r.Get(ctx, types.NamespacedName{Name: secretName, Namespace: ca.Namespace}, secret); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return nil } return fmt.Errorf("getting Secret %s: %w", secretName, err) @@ -455,7 +455,7 @@ func (r *CertificateAuthorityReconciler) adoptSecret(ctx context.Context, ca *op func (r *CertificateAuthorityReconciler) extractCANotAfter(ctx context.Context, secretName, namespace string) *metav1.Time { secret := &corev1.Secret{} if err := r.Get(ctx, types.NamespacedName{Name: secretName, Namespace: namespace}, secret); err != nil { - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { log.FromContext(ctx).Error(err, "failed to get CA Secret", "name", secretName, "namespace", namespace) } return nil diff --git a/internal/controller/certificateauthority_controller_test.go b/internal/controller/certificateauthority_controller_test.go index 31630c85..1f0f81f3 100644 --- a/internal/controller/certificateauthority_controller_test.go +++ b/internal/controller/certificateauthority_controller_test.go @@ -1,6 +1,7 @@ package controller import ( + "slices" "strings" "testing" "time" @@ -11,14 +12,13 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/events" - "k8s.io/utils/ptr" openvoxv1alpha1 "github.com/slauger/openvox-operator/api/v1alpha1" ) -// caPrereqs returns a Config with authorityRef pointing to the given CA name. -func caPrereqs(caName string) *openvoxv1alpha1.Config { - return newConfig("production", withAuthorityRef(caName)) +// caPrereqs returns a Config with authorityRef pointing to the test CA. +func caPrereqs() *openvoxv1alpha1.Config { + return newConfig("production", withAuthorityRef("test-ca")) } func TestCAReconcile_NotFound(t *testing.T) { @@ -80,7 +80,7 @@ func TestCAReconcile_NoConfig_EmitsEvent(t *testing.T) { func TestCAReconcile_PVCCreation(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() c := setupTestClient(ca, cfg) r := newCertificateAuthorityReconciler(c) @@ -106,8 +106,8 @@ func TestCAReconcile_PVCCreation(t *testing.T) { func TestCAReconcile_PVCCustomStorageClass(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - ca.Spec.Storage = &openvoxv1alpha1.StorageSpec{StorageClass: "fast-ssd", Size: ptr.To(resource.MustParse("10Gi"))} - cfg := caPrereqs("test-ca") + ca.Spec.Storage = &openvoxv1alpha1.StorageSpec{StorageClass: "fast-ssd", Size: new(resource.MustParse("10Gi"))} + cfg := caPrereqs() c := setupTestClient(ca, cfg) r := newCertificateAuthorityReconciler(c) @@ -133,7 +133,7 @@ func TestCAReconcile_PVCCustomStorageClass(t *testing.T) { func TestCAReconcile_RBACCreation(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() c := setupTestClient(ca, cfg) r := newCertificateAuthorityReconciler(c) @@ -166,7 +166,7 @@ func TestCAReconcile_RBACCreation(t *testing.T) { func TestCAReconcile_RBACResourceNames(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() cert := newCertificate("my-cert", "test-ca", openvoxv1alpha1.CertificatePhasePending) c := setupTestClient(ca, cfg, cert) r := newCertificateAuthorityReconciler(c) @@ -202,7 +202,7 @@ func TestCAReconcile_RBACResourceNames(t *testing.T) { func TestCAReconcile_JobCreation(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() c := setupTestClient(ca, cfg) r := newCertificateAuthorityReconciler(c) @@ -253,7 +253,7 @@ func TestCAReconcile_JobCreation(t *testing.T) { func TestCAReconcile_PhasePending(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" // reset - cfg := caPrereqs("test-ca") + cfg := caPrereqs() c := setupTestClient(ca, cfg) r := newCertificateAuthorityReconciler(c) @@ -274,7 +274,7 @@ func TestCAReconcile_PhasePending(t *testing.T) { func TestCAReconcile_PhaseReady(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() caSecret := newSecret("test-ca-ca", map[string][]byte{ "ca_crt.pem": []byte("ca-cert"), }) @@ -316,7 +316,7 @@ func TestCAReconcile_PhaseReady(t *testing.T) { func TestCAReconcile_NotAfterRequeue(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() // CA secret exists but with non-parseable cert data, so NotAfter will be nil caSecret := newSecret("test-ca-ca", map[string][]byte{ "ca_crt.pem": []byte("not-a-valid-cert"), @@ -489,7 +489,7 @@ func TestCAReconcile_ExternalCA_CASecretMissingKey(t *testing.T) { func TestCAReconcile_ServiceCreation(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() c := setupTestClient(ca, cfg) r := newCertificateAuthorityReconciler(c) @@ -540,7 +540,7 @@ func TestCAReconcile_ServiceCreation(t *testing.T) { func TestCAReconcile_JobIncludesServiceFQDN(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() server := newServer("ca-server", withCA(true), withServerRole(true)) server.Spec.ConfigRef = "production" server.Spec.CertificateRef = "ca-cert" @@ -593,7 +593,7 @@ func TestCAReconcile_JobIncludesServiceFQDN(t *testing.T) { func TestCAReconcile_StatusServiceName(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() caSecret := newSecret("test-ca-ca", map[string][]byte{ "ca_crt.pem": []byte("ca-cert"), }) @@ -657,7 +657,7 @@ func TestCAReconcile_ExternalCA_SkipsPVCAndJob(t *testing.T) { func TestCAReconcile_StatusSigningSecretName(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() caSecret := newSecret("test-ca-ca", map[string][]byte{ "ca_crt.pem": []byte("ca-cert"), }) @@ -686,7 +686,7 @@ func TestCAReconcile_StatusSigningSecretName(t *testing.T) { func TestCAReconcile_StatusSigningSecretName_NoCert(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() caSecret := newSecret("test-ca-ca", map[string][]byte{ "ca_crt.pem": []byte("ca-cert"), }) @@ -711,7 +711,7 @@ func TestCAReconcile_StatusSigningSecretName_NoCert(t *testing.T) { func TestCAReconcile_OperatorSigningCertCreated(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() certPEM, _ := generateTestCert(t) caSecret := newSecret("test-ca-ca", map[string][]byte{ "ca_crt.pem": certPEM, @@ -755,7 +755,7 @@ func TestCAReconcile_OperatorSigningCertActivated(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" ca.Status.SigningSecretName = "old-init-job-tls" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() certPEM, keyPEM := generateTestCert(t) caSecret := newSecret("test-ca-ca", map[string][]byte{ @@ -808,7 +808,7 @@ func TestCAReconcile_OperatorSigningDoesNotOverwriteInitJobCert(t *testing.T) { // When operator-signing cert is not yet active, the Init-Job cert should still be used ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" - cfg := caPrereqs("test-ca") + cfg := caPrereqs() caSecret := newSecret("test-ca-ca", map[string][]byte{ "ca_crt.pem": []byte("ca-cert"), }) @@ -883,7 +883,7 @@ func TestEnsureCARole_CreateAndUpdate(t *testing.T) { } // Update with additional resource names - updatedNames := append(resourceNames, "extra-cert-tls") + updatedNames := slices.Concat(resourceNames, []string{"extra-cert-tls"}) if err := r.ensureCARole(testCtx(), "test-ca-ca-setup", testNamespace, labels, updatedNames, ca); err != nil { t.Fatalf("ensureCARole update: %v", err) } diff --git a/internal/controller/certificateauthority_job.go b/internal/controller/certificateauthority_job.go index 936f5be3..eaf067c2 100644 --- a/internal/controller/certificateauthority_job.go +++ b/internal/controller/certificateauthority_job.go @@ -9,7 +9,7 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -195,8 +195,8 @@ func (r *CertificateAuthorityReconciler) buildCASetupJob(ctx context.Context, ca {Name: "tmp", MountPath: "/tmp"}, }, SecurityContext: &corev1.SecurityContext{ - AllowPrivilegeEscalation: boolPtr(false), - ReadOnlyRootFilesystem: boolPtr(true), + AllowPrivilegeEscalation: new(false), + ReadOnlyRootFilesystem: new(true), Capabilities: &corev1.Capabilities{ Drop: []corev1.Capability{"ALL"}, }, @@ -497,7 +497,7 @@ func (r *CertificateAuthorityReconciler) reconcileJob(ctx context.Context, ca *o existingJob := &batchv1.Job{} err := r.Get(ctx, types.NamespacedName{Name: jobName, Namespace: ca.Namespace}, existingJob) - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("creating CA setup job", "name", jobName) if err := controllerutil.SetControllerReference(ca, desiredJob, r.Scheme); err != nil { return ctrl.Result{}, err diff --git a/internal/controller/certificateauthority_job_test.go b/internal/controller/certificateauthority_job_test.go index 86aa5d6d..e602561e 100644 --- a/internal/controller/certificateauthority_job_test.go +++ b/internal/controller/certificateauthority_job_test.go @@ -249,7 +249,7 @@ func TestResolveCAJobResources(t *testing.T) { func TestReconcileJob_CreatesNew(t *testing.T) { ca := newCertificateAuthority("test-ca") - cfg := caPrereqs("test-ca") + cfg := caPrereqs() c := setupTestClient(ca, cfg) r := newCertificateAuthorityReconciler(c) @@ -284,7 +284,7 @@ func TestReconcileJob_CreatesNew(t *testing.T) { func TestReconcileJob_Succeeded(t *testing.T) { ca := newCertificateAuthority("test-ca") - cfg := caPrereqs("test-ca") + cfg := caPrereqs() // Pre-create the expected secret caSecret := newSecret("test-ca-ca", map[string][]byte{ @@ -324,7 +324,7 @@ func TestReconcileJob_Succeeded(t *testing.T) { func TestReconcileJob_Failed(t *testing.T) { ca := newCertificateAuthority("test-ca") - cfg := caPrereqs("test-ca") + cfg := caPrereqs() existingJob := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ @@ -361,7 +361,7 @@ func TestReconcileJob_Failed(t *testing.T) { func TestReconcileJob_Running(t *testing.T) { ca := newCertificateAuthority("test-ca") - cfg := caPrereqs("test-ca") + cfg := caPrereqs() existingJob := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ @@ -393,7 +393,7 @@ func TestReconcileJob_Running(t *testing.T) { func TestReconcileJob_ImageChanged(t *testing.T) { ca := newCertificateAuthority("test-ca") - cfg := caPrereqs("test-ca") + cfg := caPrereqs() existingJob := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controller/certificateauthority_pvc.go b/internal/controller/certificateauthority_pvc.go index 11873961..1ca7a2a4 100644 --- a/internal/controller/certificateauthority_pvc.go +++ b/internal/controller/certificateauthority_pvc.go @@ -5,7 +5,7 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -24,7 +24,7 @@ func (r *CertificateAuthorityReconciler) reconcileCAPVC(ctx context.Context, ca pvc := &corev1.PersistentVolumeClaim{} err := r.Get(ctx, types.NamespacedName{Name: pvcName, Namespace: ca.Namespace}, pvc) - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { storage := resolveCAStorage(ca) storageSize := defaultCAStorageQuantity if storage.Size != nil { diff --git a/internal/controller/certificateauthority_rbac.go b/internal/controller/certificateauthority_rbac.go index f865bfde..8d241313 100644 --- a/internal/controller/certificateauthority_rbac.go +++ b/internal/controller/certificateauthority_rbac.go @@ -6,7 +6,7 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -21,7 +21,8 @@ func (r *CertificateAuthorityReconciler) reconcileCASetupRBAC(ctx context.Contex caKeySecretName := fmt.Sprintf("%s-ca-key", ca.Name) caCRLSecretName := fmt.Sprintf("%s-ca-crl", ca.Name) - resourceNames := []string{caSecretName, caKeySecretName, caCRLSecretName} + resourceNames := make([]string, 0, 3+len(certs)) + resourceNames = append(resourceNames, caSecretName, caKeySecretName, caCRLSecretName) for _, cert := range certs { resourceNames = append(resourceNames, fmt.Sprintf("%s-tls", cert.Name)) } @@ -46,7 +47,7 @@ func (r *CertificateAuthorityReconciler) reconcileCASetupRBAC(ctx context.Contex func (r *CertificateAuthorityReconciler) ensureCAServiceAccount(ctx context.Context, name, namespace string, labels map[string]string, owner *openvoxv1alpha1.CertificateAuthority) error { sa := &corev1.ServiceAccount{} - if err := r.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, sa); errors.IsNotFound(err) { + if err := r.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, sa); apierrors.IsNotFound(err) { sa = &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -67,7 +68,7 @@ func (r *CertificateAuthorityReconciler) ensureCAServiceAccount(ctx context.Cont func (r *CertificateAuthorityReconciler) ensureCARole(ctx context.Context, name, namespace string, labels map[string]string, resourceNames []string, owner *openvoxv1alpha1.CertificateAuthority) error { role := &rbacv1.Role{} err := r.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, role) - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { role = &rbacv1.Role{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -115,7 +116,7 @@ func (r *CertificateAuthorityReconciler) ensureCARole(ctx context.Context, name, func (r *CertificateAuthorityReconciler) ensureCARoleBinding(ctx context.Context, name, namespace string, labels map[string]string, owner *openvoxv1alpha1.CertificateAuthority) error { rb := &rbacv1.RoleBinding{} err := r.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, rb) - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { rb = &rbacv1.RoleBinding{ ObjectMeta: metav1.ObjectMeta{ Name: name, diff --git a/internal/controller/certificateauthority_service.go b/internal/controller/certificateauthority_service.go index 8760eb5a..a6b22763 100644 --- a/internal/controller/certificateauthority_service.go +++ b/internal/controller/certificateauthority_service.go @@ -5,7 +5,7 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" @@ -28,7 +28,7 @@ func (r *CertificateAuthorityReconciler) reconcileCAService(ctx context.Context, svc := &corev1.Service{} err := r.Get(ctx, types.NamespacedName{Name: svcName, Namespace: ca.Namespace}, svc) - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("creating CA Service", "name", svcName) svc = &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controller/certificateauthority_setup_backoff_test.go b/internal/controller/certificateauthority_setup_backoff_test.go index a95cc9c6..d8c3c8b8 100644 --- a/internal/controller/certificateauthority_setup_backoff_test.go +++ b/internal/controller/certificateauthority_setup_backoff_test.go @@ -44,11 +44,11 @@ func failedSetupJob(message string) *batchv1.Job { } // reloadCA re-reads the CertificateAuthority, the way each reconcile does. -func reloadCA(t *testing.T, c client.Client, name string) *openvoxv1alpha1.CertificateAuthority { +func reloadCA(t *testing.T, c client.Client) *openvoxv1alpha1.CertificateAuthority { t.Helper() ca := &openvoxv1alpha1.CertificateAuthority{} - if err := c.Get(testCtx(), types.NamespacedName{Name: name, Namespace: testNamespace}, ca); err != nil { - t.Fatalf("reading CertificateAuthority %s: %v", name, err) + if err := c.Get(testCtx(), types.NamespacedName{Name: "test-ca", Namespace: testNamespace}, ca); err != nil { + t.Fatalf("reading CertificateAuthority: %v", err) } return ca } @@ -57,7 +57,7 @@ func reloadCA(t *testing.T, c client.Client, name string) *openvoxv1alpha1.Certi // a Job that cannot succeed used to be deleted and recreated roughly every 15 // seconds forever. func TestSetupJob_StopsRecreatingAfterRepeatedFailures(t *testing.T) { - c := setupTestClient(newCertificateAuthority("test-ca"), caPrereqs("test-ca")) + c := setupTestClient(newCertificateAuthority("test-ca"), caPrereqs()) r := newCertificateAuthorityReconciler(c) for attempt := 1; attempt <= maxSetupAttempts; attempt++ { @@ -67,7 +67,7 @@ func TestSetupJob_StopsRecreatingAfterRepeatedFailures(t *testing.T) { t.Fatalf("seeding the failed job for attempt %d: %v", attempt, err) } - ca := reloadCA(t, c, "test-ca") + ca := reloadCA(t, c) result, err := r.reconcileJob(testCtx(), ca, setupJobName, job.DeepCopy(), "test-ca-ca") if err != nil { t.Fatalf("reconcileJob attempt %d: %v", attempt, err) @@ -99,10 +99,10 @@ func TestSetupJob_StopsRecreatingAfterRepeatedFailures(t *testing.T) { // TestSetupJob_ReportsFailureInCondition covers the second half of the problem: // the CA sat in Initializing with no indication of why. func TestSetupJob_ReportsFailureInCondition(t *testing.T) { - c := setupTestClient(newCertificateAuthority("test-ca"), caPrereqs("test-ca")) + c := setupTestClient(newCertificateAuthority("test-ca"), caPrereqs()) r := newCertificateAuthorityReconciler(c) - ca := reloadCA(t, c, "test-ca") + ca := reloadCA(t, c) ca.Annotations = map[string]string{AnnotationSetupAttempts: "4"} if err := c.Update(testCtx(), ca); err != nil { t.Fatalf("seeding the attempt counter: %v", err) @@ -113,11 +113,11 @@ func TestSetupJob_ReportsFailureInCondition(t *testing.T) { t.Fatalf("seeding the failed job: %v", err) } - if _, err := r.reconcileJob(testCtx(), reloadCA(t, c, "test-ca"), setupJobName, job.DeepCopy(), "test-ca-ca"); err != nil { + if _, err := r.reconcileJob(testCtx(), reloadCA(t, c), setupJobName, job.DeepCopy(), "test-ca-ca"); err != nil { t.Fatalf("reconcileJob: %v", err) } - got := reloadCA(t, c, "test-ca") + got := reloadCA(t, c) if got.Status.Phase != openvoxv1alpha1.CertificateAuthorityPhaseError { t.Errorf("expected phase %q, got %q", openvoxv1alpha1.CertificateAuthorityPhaseError, got.Status.Phase) } @@ -137,12 +137,12 @@ func TestSetupJob_ReportsFailureInCondition(t *testing.T) { // TestSetupJob_CounterDoesNotGrowOnceTerminal keeps repeated reconciles from // inflating the counter and re-emitting the event. func TestSetupJob_CounterDoesNotGrowOnceTerminal(t *testing.T) { - c := setupTestClient(newCertificateAuthority("test-ca"), caPrereqs("test-ca")) + c := setupTestClient(newCertificateAuthority("test-ca"), caPrereqs()) r := newCertificateAuthorityReconciler(c) rec := events.NewFakeRecorder(100) r.Recorder = rec - ca := reloadCA(t, c, "test-ca") + ca := reloadCA(t, c) ca.Annotations = map[string]string{AnnotationSetupAttempts: "4"} if err := c.Update(testCtx(), ca); err != nil { t.Fatalf("seeding the attempt counter: %v", err) @@ -153,13 +153,13 @@ func TestSetupJob_CounterDoesNotGrowOnceTerminal(t *testing.T) { t.Fatalf("seeding the failed job: %v", err) } - for i := 0; i < 3; i++ { - if _, err := r.reconcileJob(testCtx(), reloadCA(t, c, "test-ca"), setupJobName, job.DeepCopy(), "test-ca-ca"); err != nil { + for i := range 3 { + if _, err := r.reconcileJob(testCtx(), reloadCA(t, c), setupJobName, job.DeepCopy(), "test-ca-ca"); err != nil { t.Fatalf("reconcileJob %d: %v", i, err) } } - if n := setupAttempts(reloadCA(t, c, "test-ca")); n != maxSetupAttempts { + if n := setupAttempts(reloadCA(t, c)); n != maxSetupAttempts { t.Errorf("the counter must stop at %d, got %d", maxSetupAttempts, n) } failures := 0 @@ -188,14 +188,14 @@ func TestSetupJob_SuccessResetsTheBudget(t *testing.T) { job := failedSetupJob("") job.Status = batchv1.JobStatus{Succeeded: 1} - c := setupTestClient(ca, caPrereqs("test-ca"), caSecret, job) + c := setupTestClient(ca, caPrereqs(), caSecret, job) r := newCertificateAuthorityReconciler(c) - if _, err := r.reconcileJob(testCtx(), reloadCA(t, c, "test-ca"), setupJobName, job.DeepCopy(), "test-ca-ca"); err != nil { + if _, err := r.reconcileJob(testCtx(), reloadCA(t, c), setupJobName, job.DeepCopy(), "test-ca-ca"); err != nil { t.Fatalf("reconcileJob: %v", err) } - if n := setupAttempts(reloadCA(t, c, "test-ca")); n != 0 { + if n := setupAttempts(reloadCA(t, c)); n != 0 { t.Errorf("a successful job must clear the counter, got %d", n) } } @@ -208,20 +208,20 @@ func TestSetupJob_ImageChangeResetsTheBudget(t *testing.T) { job := failedSetupJob("Job has reached the specified backoff limit") - c := setupTestClient(ca, caPrereqs("test-ca"), job) + c := setupTestClient(ca, caPrereqs(), job) r := newCertificateAuthorityReconciler(c) desired := job.DeepCopy() desired.Spec.Template.Spec.Containers[0].Image = "corrected:1.2.3" - res, err := r.reconcileJob(testCtx(), reloadCA(t, c, "test-ca"), setupJobName, desired, "test-ca-ca") + res, err := r.reconcileJob(testCtx(), reloadCA(t, c), setupJobName, desired, "test-ca-ca") if err != nil { t.Fatalf("reconcileJob: %v", err) } if res.RequeueAfter != RequeueIntervalMedium { t.Errorf("a corrected image must be retried, got RequeueAfter %v", res.RequeueAfter) } - if n := setupAttempts(reloadCA(t, c, "test-ca")); n != 0 { + if n := setupAttempts(reloadCA(t, c)); n != 0 { t.Errorf("a corrected image must clear the counter, got %d", n) } if err := c.Get(testCtx(), types.NamespacedName{Name: setupJobName, Namespace: testNamespace}, &batchv1.Job{}); !apierrors.IsNotFound(err) { diff --git a/internal/controller/certificateauthority_signing.go b/internal/controller/certificateauthority_signing.go index 2fe2c624..94e6675a 100644 --- a/internal/controller/certificateauthority_signing.go +++ b/internal/controller/certificateauthority_signing.go @@ -5,7 +5,7 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" @@ -27,7 +27,7 @@ func (r *CertificateAuthorityReconciler) reconcileOperatorSigningCert(ctx contex } certName := fmt.Sprintf("%s-operator-signing", ca.Name) - certname := fmt.Sprintf("%s-operator", ca.Name) + certname := operatorSigningCertname(ca.Name) // Look for existing operator-signing Certificate in the list var signingCert *openvoxv1alpha1.Certificate @@ -55,7 +55,7 @@ func (r *CertificateAuthorityReconciler) reconcileOperatorSigningCert(ctx contex } if err := r.Create(ctx, newCert); err != nil { - if errors.IsAlreadyExists(err) { + if apierrors.IsAlreadyExists(err) { // Already exists, requeue to pick it up next time return ctrl.Result{RequeueAfter: RequeueIntervalShort}, nil } diff --git a/internal/controller/config_autosign.go b/internal/controller/config_autosign.go index ee79e8e5..0daf9cf5 100644 --- a/internal/controller/config_autosign.go +++ b/internal/controller/config_autosign.go @@ -6,7 +6,7 @@ import ( "sort" "strings" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -20,6 +20,15 @@ import ( const autosignBinaryPath = "/usr/local/bin/openvox-autosign" +// autosignPolicyDir is where the rendered policy Secret is mounted. It is a +// directory so the kubelet keeps it in sync; see the mount in +// server_deployment.go. +const autosignPolicyDir = "/etc/puppetlabs/puppet/autosign-policy" + +// autosignPolicyPath is the file inside that directory, passed to the binary +// with --config. +const autosignPolicyPath = autosignPolicyDir + "/autosign-policy.yaml" + // findSigningPolicies returns all SigningPolicies referencing the given CA. // // A list error is returned rather than swallowed: an empty policy set renders @@ -51,7 +60,7 @@ func (r *ConfigReconciler) reconcileAutosignSecrets(ctx context.Context, cfg *op } ca := &openvoxv1alpha1.CertificateAuthority{} if err := r.Get(ctx, types.NamespacedName{Name: cfg.Spec.AuthorityRef, Namespace: cfg.Namespace}, ca); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return nil } return fmt.Errorf("getting CertificateAuthority %s: %w", cfg.Spec.AuthorityRef, err) @@ -76,7 +85,7 @@ func (r *ConfigReconciler) reconcileAutosignSecret(ctx context.Context, cfg *ope } // Render policy config YAML - policyYAML, renderErr := r.renderAutosignPolicyConfig(ctx, cfg.Namespace, policies) + policyYAML, renderErr := r.renderAutosignPolicyConfig(ctx, cfg.Namespace, ca, policies) if renderErr != nil { return fmt.Errorf("rendering autosign policy config: %w", renderErr) } @@ -94,8 +103,17 @@ func (r *ConfigReconciler) reconcileAutosignSecret(ctx context.Context, cfg *ope } // renderAutosignPolicyConfig renders the policy config YAML that openvox-autosign reads. -func (r *ConfigReconciler) renderAutosignPolicyConfig(ctx context.Context, namespace string, policies []openvoxv1alpha1.SigningPolicy) (string, error) { +func (r *ConfigReconciler) renderAutosignPolicyConfig(ctx context.Context, namespace string, + ca *openvoxv1alpha1.CertificateAuthority, policies []openvoxv1alpha1.SigningPolicy) (string, error) { var sb strings.Builder + + // The CA auth.conf grants admin rights to this certname as well as to the + // pp_cli_auth extension (see builtinAuthRules). An agent holding a + // certificate under it would be a CA admin, so the name is refused before + // any policy runs - including any: true, which no downstream guard can undo. + sb.WriteString("reservedCertnames:\n") + fmt.Fprintf(&sb, " - %q\n", operatorSigningCertname(ca.Name)) + sb.WriteString("policies:\n") // Sort policies by name for deterministic output @@ -108,23 +126,28 @@ func (r *ConfigReconciler) renderAutosignPolicyConfig(ctx context.Context, names if p.Spec.Any { sb.WriteString(" any: true\n") - continue } - if p.Spec.Pattern != nil { - sb.WriteString(" pattern:\n") - sb.WriteString(" allow:\n") - for _, a := range p.Spec.Pattern.Allow { - fmt.Fprintf(&sb, " - %q\n", a) - } + // Guard fields (SAN allowlists and extensions) are rendered for every + // policy, including any:true, so the autosign binary enforces them and no + // policy can implicitly waive escalation protection. + if p.Spec.Certnames != nil { + renderAllowList(&sb, "certnames", p.Spec.Certnames.Allow) } - if p.Spec.DNSAltNames != nil { - sb.WriteString(" dnsAltNames:\n") - sb.WriteString(" allow:\n") - for _, a := range p.Spec.DNSAltNames.Allow { - fmt.Fprintf(&sb, " - %q\n", a) - } + renderAllowList(&sb, "dnsAltNames", p.Spec.DNSAltNames.Allow) + } + if p.Spec.IPAltNames != nil { + renderAllowList(&sb, "ipAltNames", p.Spec.IPAltNames.Allow) + } + if p.Spec.URIAltNames != nil { + renderAllowList(&sb, "uriAltNames", p.Spec.URIAltNames.Allow) + } + if p.Spec.EmailAltNames != nil { + renderAllowList(&sb, "emailAltNames", p.Spec.EmailAltNames.Allow) + } + if p.Spec.Extensions != nil { + renderAllowList(&sb, "extensions", p.Spec.Extensions.Allow) } if len(p.Spec.CSRAttributes) > 0 { @@ -149,6 +172,15 @@ func (r *ConfigReconciler) renderAutosignPolicyConfig(ctx context.Context, names return sb.String(), nil } +// renderAllowList writes an "{field}: { allow: [...] }" block with quoted entries. +func renderAllowList(sb *strings.Builder, field string, allow []string) { + fmt.Fprintf(sb, " %s:\n", field) + sb.WriteString(" allow:\n") + for _, a := range allow { + fmt.Fprintf(sb, " - %q\n", a) + } +} + // updateSigningPolicyStatus sets the phase and condition on a SigningPolicy. func (r *ConfigReconciler) updateSigningPolicyStatus(ctx context.Context, sp *openvoxv1alpha1.SigningPolicy, err error) { var errMsg string diff --git a/internal/controller/config_autosign_test.go b/internal/controller/config_autosign_test.go index 21a87ad6..2e9f8533 100644 --- a/internal/controller/config_autosign_test.go +++ b/internal/controller/config_autosign_test.go @@ -120,14 +120,14 @@ func TestRenderAutosignPolicyConfig(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "pattern-policy", Namespace: testNamespace}, Spec: openvoxv1alpha1.SigningPolicySpec{ CertificateAuthorityRef: "ca", - Pattern: &openvoxv1alpha1.PatternSpec{ + Certnames: &openvoxv1alpha1.PatternSpec{ Allow: []string{"*.example.com", "web-*"}, }, }, }, }, contains: []string{ - "pattern:", + "certnames:", "allow:", `"*.example.com"`, `"web-*"`, @@ -175,6 +175,53 @@ func TestRenderAutosignPolicyConfig(t *testing.T) { "\n any: true", }, }, + { + name: "SAN and extension allowlists rendered", + policies: []openvoxv1alpha1.SigningPolicy{ + { + ObjectMeta: metav1.ObjectMeta{Name: "svc", Namespace: testNamespace}, + Spec: openvoxv1alpha1.SigningPolicySpec{ + CertificateAuthorityRef: "ca", + Certnames: &openvoxv1alpha1.PatternSpec{Allow: []string{"svc-*"}}, + IPAltNames: &openvoxv1alpha1.PatternSpec{Allow: []string{"10.0.0.0/16"}}, + URIAltNames: &openvoxv1alpha1.PatternSpec{Allow: []string{"spiffe://example.com/*"}}, + EmailAltNames: &openvoxv1alpha1.PatternSpec{Allow: []string{"*@example.com"}}, + Extensions: &openvoxv1alpha1.PatternSpec{Allow: []string{"pp_cli_auth"}}, + }, + }, + }, + contains: []string{ + "ipAltNames:", + `"10.0.0.0/16"`, + "uriAltNames:", + `"spiffe://example.com/*"`, + "emailAltNames:", + `"*@example.com"`, + "extensions:", + `"pp_cli_auth"`, + }, + }, + { + name: "any:true still renders guard fields", + policies: []openvoxv1alpha1.SigningPolicy{ + { + ObjectMeta: metav1.ObjectMeta{Name: "bootstrap", Namespace: testNamespace}, + Spec: openvoxv1alpha1.SigningPolicySpec{ + CertificateAuthorityRef: "ca", + Any: true, + Extensions: &openvoxv1alpha1.PatternSpec{Allow: []string{"pp_cli_auth"}}, + IPAltNames: &openvoxv1alpha1.PatternSpec{Allow: []string{"10.0.0.0/8"}}, + }, + }, + }, + contains: []string{ + "any: true", + "extensions:", + `"pp_cli_auth"`, + "ipAltNames:", + `"10.0.0.0/8"`, + }, + }, } for _, tt := range tests { @@ -182,7 +229,7 @@ func TestRenderAutosignPolicyConfig(t *testing.T) { c := setupTestClient(tt.objs...) r := newConfigReconciler(c) - out, err := r.renderAutosignPolicyConfig(testCtx(), testNamespace, tt.policies) + out, err := r.renderAutosignPolicyConfig(testCtx(), testNamespace, newCertificateAuthority("production-ca"), tt.policies) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -220,7 +267,7 @@ func TestRenderAutosignPolicyConfig_SortOrder(t *testing.T) { }, } - out, err := r.renderAutosignPolicyConfig(testCtx(), testNamespace, policies) + out, err := r.renderAutosignPolicyConfig(testCtx(), testNamespace, newCertificateAuthority("production-ca"), policies) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -269,3 +316,26 @@ func TestUpdateSigningPolicyStatus_Error(t *testing.T) { t.Errorf("expected phase %q, got %q", openvoxv1alpha1.SigningPolicyPhaseError, updated.Status.Phase) } } + +// TestRenderAutosignPolicy_ReservesTheOperatorCertname is the operator half of +// the escalation guard. The CA auth.conf grants admin rights to this certname, +// so the rendered policy has to tell the autosign binary never to hand it out. +func TestRenderAutosignPolicy_ReservesTheOperatorCertname(t *testing.T) { + ca := newCertificateAuthority("production-ca") + r := newConfigReconciler(setupTestClient(ca)) + + out, err := r.renderAutosignPolicyConfig(testCtx(), testNamespace, ca, nil) + if err != nil { + t.Fatalf("rendering the policy: %v", err) + } + + if !strings.Contains(out, "reservedCertnames:") { + t.Fatalf("the rendered policy carries no reservation:\n%s", out) + } + // Must match what the operator actually issues to itself, not a literal + // spelled out twice. + want := operatorSigningCertname(ca.Name) + if !strings.Contains(out, want) { + t.Errorf("expected %q to be reserved, got:\n%s", want, out) + } +} diff --git a/internal/controller/config_controller.go b/internal/controller/config_controller.go index 968df28e..01773dec 100644 --- a/internal/controller/config_controller.go +++ b/internal/controller/config_controller.go @@ -5,7 +5,7 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -51,7 +51,7 @@ func (r *ConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr cfg := &openvoxv1alpha1.Config{} if err := r.Get(ctx, req.NamespacedName, cfg); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return ctrl.Result{}, nil } return ctrl.Result{}, err @@ -291,7 +291,7 @@ func (r *ConfigReconciler) findCertificateAuthority(ctx context.Context, cfg *op } ca := &openvoxv1alpha1.CertificateAuthority{} if err := r.Get(ctx, types.NamespacedName{Name: cfg.Spec.AuthorityRef, Namespace: cfg.Namespace}, ca); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return nil, nil } return nil, fmt.Errorf("getting CertificateAuthority %s: %w", cfg.Spec.AuthorityRef, err) diff --git a/internal/controller/config_controller_test.go b/internal/controller/config_controller_test.go index 74d9ca93..b7d0ed1f 100644 --- a/internal/controller/config_controller_test.go +++ b/internal/controller/config_controller_test.go @@ -82,7 +82,7 @@ func TestConfigReconcile_PuppetConfRendering(t *testing.T) { { name: "storeconfigs enabled", opts: []configOption{withPuppetSpec(openvoxv1alpha1.PuppetSpec{ - Storeconfigs: boolPtr(true), + Storeconfigs: new(true), StoreBackend: "puppetdb", Reports: "puppetdb", })}, @@ -91,7 +91,7 @@ func TestConfigReconcile_PuppetConfRendering(t *testing.T) { { name: "storeconfigs disabled", opts: []configOption{withPuppetSpec(openvoxv1alpha1.PuppetSpec{ - Storeconfigs: boolPtr(false), + Storeconfigs: new(false), Reports: "puppetdb", })}, excludes: []string{"storeconfigs = true"}, @@ -212,11 +212,17 @@ func TestConfigReconcile_PuppetConfWithCA(t *testing.T) { if !strings.Contains(puppetConf, "autosign = ") { t.Errorf("puppet.conf missing autosign\n---\n%s", puppetConf) } + // The policy lives in a directory mount, so the binary has to be told where + // to look. A wrong path here denies every CSR and would otherwise surface + // only in an end-to-end run. + if !strings.Contains(puppetConf, "--config "+autosignPolicyPath) { + t.Errorf("puppet.conf must point the binary at %s\n---\n%s", autosignPolicyPath, puppetConf) + } } func TestConfigReconcile_PuppetConfWithENC(t *testing.T) { nc := newNodeClassifier("my-enc", "https://enc.example.com") - cfg := newConfig("production", withNodeClassifierRef("my-enc")) + cfg := newConfig("production", withNodeClassifierRef()) c := setupTestClient(cfg, nc) r := newConfigReconciler(c) @@ -273,7 +279,7 @@ func TestConfigReconcile_AutosignCommandOverride(t *testing.T) { func TestConfigReconcile_ExternalNodesCommandOverride(t *testing.T) { cfg := newConfig("production", - withNodeClassifierRef("my-enc"), + withNodeClassifierRef(), withExternalNodesCommand("/usr/local/bin/custom-enc"), ) nc := newNodeClassifier("my-enc", "https://enc.example.com") @@ -309,7 +315,7 @@ func TestConfigReconcile_ExternalNodesCommandOverride(t *testing.T) { func TestConfigReconcile_PuppetConfWithReports(t *testing.T) { cfg := newConfig("production") - rp := newReportProcessor("webhook-rp", "production", "https://reports.example.com") + rp := newReportProcessor("webhook-rp", "https://reports.example.com") c := setupTestClient(cfg, rp) r := newConfigReconciler(c) @@ -359,8 +365,8 @@ func TestConfigReconcile_PuppetserverConf(t *testing.T) { name: "http-client settings", ps: openvoxv1alpha1.PuppetServerSpec{ HTTPClient: &openvoxv1alpha1.HTTPClientSpec{ - ConnectTimeoutMs: int32Ptr(5000), - IdleTimeoutMs: int32Ptr(30000), + ConnectTimeoutMs: new(int32(5000)), + IdleTimeoutMs: new(int32(30000)), }, }, contains: []string{ @@ -581,7 +587,7 @@ func TestConfigReconcile_ENCSecret(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cfg := newConfig("production", withNodeClassifierRef("my-enc")) + cfg := newConfig("production", withNodeClassifierRef()) c := setupTestClient(cfg, tt.nc) r := newConfigReconciler(c) @@ -606,8 +612,8 @@ func TestConfigReconcile_ENCSecret(t *testing.T) { func TestConfigReconcile_ReportWebhookSecret(t *testing.T) { cfg := newConfig("production") - rp1 := newReportProcessor("beta-webhook", "production", "https://beta.example.com/reports") - rp2 := newReportProcessor("alpha-webhook", "production", "https://alpha.example.com/reports") + rp1 := newReportProcessor("beta-webhook", "https://beta.example.com/reports") + rp2 := newReportProcessor("alpha-webhook", "https://alpha.example.com/reports") c := setupTestClient(cfg, rp1, rp2) r := newConfigReconciler(c) @@ -752,7 +758,3 @@ func TestConfigReconcile_UpdateExistingConfigMap(t *testing.T) { t.Error("ConfigMap puppet.conf missing expected content") } } - -func int32Ptr(v int32) *int32 { - return &v -} diff --git a/internal/controller/config_enc.go b/internal/controller/config_enc.go index c6f30d5b..c2e85eaa 100644 --- a/internal/controller/config_enc.go +++ b/internal/controller/config_enc.go @@ -4,7 +4,7 @@ import ( "context" "fmt" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -33,7 +33,7 @@ func (r *ConfigReconciler) reconcileENCSecret(ctx context.Context, cfg *openvoxv nc := &openvoxv1alpha1.NodeClassifier{} if err := r.Get(ctx, types.NamespacedName{Name: cfg.Spec.NodeClassifierRef, Namespace: cfg.Namespace}, nc); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return nil } return fmt.Errorf("getting NodeClassifier %s: %w", cfg.Spec.NodeClassifierRef, err) diff --git a/internal/controller/config_rendering.go b/internal/controller/config_rendering.go index e877cbaf..ce6ad216 100644 --- a/internal/controller/config_rendering.go +++ b/internal/controller/config_rendering.go @@ -52,7 +52,7 @@ func (r *ConfigReconciler) renderPuppetConf(ctx context.Context, cfg *openvoxv1a if reports == "" { reports = "webhook" } else if !strings.Contains(reports, "webhook") { - reports = reports + ",webhook" + reports += ",webhook" } } if reports != "" { @@ -76,12 +76,12 @@ func (r *ConfigReconciler) renderPuppetConf(ctx context.Context, cfg *openvoxv1a } // Autosign: by default point to the built-in binary, which reads the policy - // Secret (mounted by the server controller) and decides sign/deny. A policy - // change rewrites the Secret and the server controller rolls the CA pod via - // the autosign-policy-secret-hash annotation, so it applies without a manual - // restart. A custom autosignCommand replaces the built-in binary and disables - // the SigningPolicy-driven flow (the policy Secret is not mounted). - autosignCmd := autosignBinaryPath + // Secret (mounted by the server controller as a directory) and decides + // sign/deny. The binary re-reads the file on every CSR and the kubelet keeps + // the mount in sync, so a policy change applies without restarting the CA. + // A custom autosignCommand replaces the built-in binary and disables the + // SigningPolicy-driven flow (the policy Secret is not mounted). + autosignCmd := fmt.Sprintf("%s --config %s", autosignBinaryPath, autosignPolicyPath) if cfg.Spec.Puppet.AutosignCommand != "" { autosignCmd = cfg.Spec.Puppet.AutosignCommand } @@ -270,7 +270,7 @@ func (r *ConfigReconciler) renderAuthConf(cfg *openvoxv1alpha1.Config, ca *openv // Derive the operator-signing certname when an internal CA exists. var operatorCertname string if ca != nil && ca.Spec.External == nil { - operatorCertname = fmt.Sprintf("%s-operator", ca.Name) + operatorCertname = operatorSigningCertname(ca.Name) } var sb strings.Builder @@ -297,11 +297,12 @@ func (r *ConfigReconciler) renderAuthConf(cfg *openvoxv1alpha1.Config, ca *openv } } sb.WriteString(" }\n") - if rule.AllowUnauthenticated { + switch { + case rule.AllowUnauthenticated: sb.WriteString(" allow-unauthenticated: true\n") - } else if rule.Allow != "" { + case rule.Allow != "": fmt.Fprintf(&sb, " allow: %q\n", rule.Allow) - } else if rule.Deny != "" { + case rule.Deny != "": fmt.Fprintf(&sb, " deny: %q\n", rule.Deny) } sortOrder := rule.SortOrder diff --git a/internal/controller/config_rendering_test.go b/internal/controller/config_rendering_test.go index 44a04426..198819a0 100644 --- a/internal/controller/config_rendering_test.go +++ b/internal/controller/config_rendering_test.go @@ -35,7 +35,7 @@ func TestRenderRoutesYAML(t *testing.T) { { name: "wired up but backend is not puppetdb", cfg: newConfig("c", withDatabaseRef("db"), withPuppetSpec(openvoxv1alpha1.PuppetSpec{ - Storeconfigs: boolPtr(false), + Storeconfigs: new(false), StoreBackend: "", Reports: "store", })), @@ -44,7 +44,7 @@ func TestRenderRoutesYAML(t *testing.T) { { name: "wired up via reports=puppetdb only", cfg: newConfig("c", withDatabaseRef("db"), withPuppetSpec(openvoxv1alpha1.PuppetSpec{ - Storeconfigs: boolPtr(false), + Storeconfigs: new(false), Reports: "puppetdb", })), want: true, @@ -280,10 +280,10 @@ func TestRenderCAConf(t *testing.T) { name: "custom values", ca: &openvoxv1alpha1.CertificateAuthority{ Spec: openvoxv1alpha1.CertificateAuthoritySpec{ - AllowSubjectAltNames: boolPtr(false), - AllowAuthorizationExtensions: boolPtr(false), - EnableInfraCRL: boolPtr(false), - AllowAutoRenewal: boolPtr(false), + AllowSubjectAltNames: new(false), + AllowAuthorizationExtensions: new(false), + EnableInfraCRL: new(false), + AllowAutoRenewal: new(false), AutoRenewalCertTTL: "30d", }, }, @@ -299,10 +299,10 @@ func TestRenderCAConf(t *testing.T) { name: "empty autoRenewalCertTTL uses default", ca: &openvoxv1alpha1.CertificateAuthority{ Spec: openvoxv1alpha1.CertificateAuthoritySpec{ - AllowSubjectAltNames: boolPtr(true), - AllowAuthorizationExtensions: boolPtr(true), - EnableInfraCRL: boolPtr(true), - AllowAutoRenewal: boolPtr(true), + AllowSubjectAltNames: new(true), + AllowAuthorizationExtensions: new(true), + EnableInfraCRL: new(true), + AllowAutoRenewal: new(true), AutoRenewalCertTTL: "", }, }, diff --git a/internal/controller/config_serviceaccount.go b/internal/controller/config_serviceaccount.go index 2d356891..1bda7b9d 100644 --- a/internal/controller/config_serviceaccount.go +++ b/internal/controller/config_serviceaccount.go @@ -5,7 +5,7 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -19,7 +19,7 @@ func (r *ConfigReconciler) reconcileServerServiceAccount(ctx context.Context, cf sa := &corev1.ServiceAccount{} err := r.Get(ctx, types.NamespacedName{Name: saName, Namespace: cfg.Namespace}, sa) - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { sa = &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: saName, diff --git a/internal/controller/database_controller.go b/internal/controller/database_controller.go index 3bb8aea9..1c3fee6f 100644 --- a/internal/controller/database_controller.go +++ b/internal/controller/database_controller.go @@ -8,7 +8,7 @@ import ( corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" policyv1 "k8s.io/api/policy/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -63,7 +63,7 @@ func (r *DatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c db := &openvoxv1alpha1.Database{} if err := r.Get(ctx, req.NamespacedName, db); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return ctrl.Result{}, nil } return ctrl.Result{}, fmt.Errorf("getting Database %s: %w", req.NamespacedName, err) @@ -90,7 +90,7 @@ func (r *DatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c // Resolve Certificate -- wait until phase is Signed cert := &openvoxv1alpha1.Certificate{} if err := r.Get(ctx, types.NamespacedName{Name: db.Spec.CertificateRef, Namespace: db.Namespace}, cert); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("waiting for Certificate", "certificateRef", db.Spec.CertificateRef) return ctrl.Result{RequeueAfter: RequeueIntervalShort}, nil } @@ -110,7 +110,7 @@ func (r *DatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c // Resolve CertificateAuthority via Certificate's authorityRef ca := &openvoxv1alpha1.CertificateAuthority{} if err := r.Get(ctx, types.NamespacedName{Name: cert.Spec.AuthorityRef, Namespace: db.Namespace}, ca); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("waiting for CertificateAuthority", "authorityRef", cert.Spec.AuthorityRef) return ctrl.Result{RequeueAfter: RequeueIntervalShort}, nil } @@ -120,7 +120,7 @@ func (r *DatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c // Validate PG credentials Secret exists pgSecret := &corev1.Secret{} if err := r.Get(ctx, types.NamespacedName{Name: db.Spec.Postgres.CredentialsSecretRef, Namespace: db.Namespace}, pgSecret); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("waiting for PostgreSQL credentials Secret", "secretRef", db.Spec.Postgres.CredentialsSecretRef) return ctrl.Result{RequeueAfter: RequeueIntervalShort}, nil } @@ -312,7 +312,7 @@ func (r *DatabaseReconciler) reconcileService(ctx context.Context, db *openvoxv1 func (r *DatabaseReconciler) getReadyReplicas(ctx context.Context, db *openvoxv1alpha1.Database) (int32, error) { deploy := &appsv1.Deployment{} if err := r.Get(ctx, types.NamespacedName{Name: db.Name, Namespace: db.Namespace}, deploy); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return 0, nil } return 0, fmt.Errorf("getting Deployment %s: %w", db.Name, err) @@ -332,11 +332,11 @@ func (r *DatabaseReconciler) reconcilePDB(ctx context.Context, db *openvoxv1alph return guardErr } logger.Info("deleting Database PDB (disabled)", "name", pdbName) - if err := r.Delete(ctx, existing); err != nil && !errors.IsNotFound(err) { + if err := r.Delete(ctx, existing); err != nil && !apierrors.IsNotFound(err) { return fmt.Errorf("deleting PodDisruptionBudget %s: %w", pdbName, err) } r.Recorder.Eventf(db, nil, corev1.EventTypeNormal, EventReasonDatabasePDBDeleted, "Reconcile", "PodDisruptionBudget %s deleted", pdbName) - } else if !errors.IsNotFound(err) { + } else if !apierrors.IsNotFound(err) { return fmt.Errorf("getting PodDisruptionBudget %s: %w", pdbName, err) } return nil @@ -386,11 +386,12 @@ func (r *DatabaseReconciler) buildPDB(db *openvoxv1alpha1.Database) (*policyv1.P }, }, } - if db.Spec.PDB.MinAvailable != nil { + switch { + case db.Spec.PDB.MinAvailable != nil: pdb.Spec.MinAvailable = db.Spec.PDB.MinAvailable - } else if db.Spec.PDB.MaxUnavailable != nil { + case db.Spec.PDB.MaxUnavailable != nil: pdb.Spec.MaxUnavailable = db.Spec.PDB.MaxUnavailable - } else { + default: minAvailable := intstrInt(DefaultPDBMinAvailable) pdb.Spec.MinAvailable = &minAvailable } @@ -412,11 +413,11 @@ func (r *DatabaseReconciler) reconcileNetworkPolicy(ctx context.Context, db *ope return guardErr } logger.Info("deleting Database NetworkPolicy (disabled)", "name", npName) - if err := r.Delete(ctx, existing); err != nil && !errors.IsNotFound(err) { + if err := r.Delete(ctx, existing); err != nil && !apierrors.IsNotFound(err) { return fmt.Errorf("deleting NetworkPolicy %s: %w", npName, err) } r.Recorder.Eventf(db, nil, corev1.EventTypeNormal, EventReasonDatabaseNetworkPolicyDeleted, "Reconcile", "NetworkPolicy %s deleted", npName) - } else if !errors.IsNotFound(err) { + } else if !apierrors.IsNotFound(err) { return fmt.Errorf("getting NetworkPolicy %s: %w", npName, err) } return nil diff --git a/internal/controller/database_deployment.go b/internal/controller/database_deployment.go index 1ac39edf..b8cd4e5e 100644 --- a/internal/controller/database_deployment.go +++ b/internal/controller/database_deployment.go @@ -6,7 +6,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -68,7 +68,7 @@ func (r *DatabaseReconciler) reconcileDeployment(ctx context.Context, db *openvo deploy := &appsv1.Deployment{} err = r.Get(ctx, types.NamespacedName{Name: deployName, Namespace: db.Namespace}, deploy) - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("creating Database Deployment", "name", deployName, "replicas", replicas) deploy = &appsv1.Deployment{ @@ -239,8 +239,8 @@ chmod 640 /ssl/private_keys/%s.pem`, certname, certname, certname) } containerSecurityContext := &corev1.SecurityContext{ - AllowPrivilegeEscalation: boolPtr(false), - ReadOnlyRootFilesystem: boolPtr(true), + AllowPrivilegeEscalation: new(false), + ReadOnlyRootFilesystem: new(true), Capabilities: &corev1.Capabilities{ Drop: []corev1.Capability{"ALL"}, }, diff --git a/internal/controller/helpers.go b/internal/controller/helpers.go index d071c70c..fccda393 100644 --- a/internal/controller/helpers.go +++ b/internal/controller/helpers.go @@ -10,7 +10,7 @@ import ( "time" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -112,7 +112,7 @@ func parseCertNotAfter(ctx context.Context, certPEM []byte) *metav1.Time { func isSecretReady(ctx context.Context, reader client.Reader, name, namespace, requiredKey string) bool { secret := &corev1.Secret{} if err := reader.Get(ctx, client.ObjectKey{Name: name, Namespace: namespace}, secret); err != nil { - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { log.FromContext(ctx).Error(err, "failed to get Secret", "name", name, "namespace", namespace) } return false @@ -342,6 +342,16 @@ func resolveReadOnlyRootFilesystem(server *openvoxv1alpha1.Server, cfg *openvoxv return openvoxv1alpha1.BoolValue(cfg.Spec.ReadOnlyRootFilesystem, true) } +// operatorSigningCertname is the certname the operator issues to itself for +// mTLS against the CA API. +// +// The CA auth.conf grants admin rights to this name, so it is also the name the +// autosign policy reserves. Both derive from here: a drift between them would +// reserve one name while granting admin to another. +func operatorSigningCertname(caName string) string { + return fmt.Sprintf("%s-operator", caName) +} + // serverRoleEnabled reports whether the Server runs the catalog server role. // The spec field defaults to true, so an unset value enables the role. func serverRoleEnabled(server *openvoxv1alpha1.Server) bool { diff --git a/internal/controller/helpers_test.go b/internal/controller/helpers_test.go index 5f14cb3d..67158ee5 100644 --- a/internal/controller/helpers_test.go +++ b/internal/controller/helpers_test.go @@ -6,7 +6,7 @@ import ( "sync/atomic" "testing" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" @@ -212,25 +212,6 @@ func TestResolveCodeMounts(t *testing.T) { } } -func TestInt64Ptr(t *testing.T) { - val := int64Ptr(42) - if val == nil || *val != 42 { - t.Errorf("int64Ptr(42) = %v, want pointer to 42", val) - } -} - -func TestBoolPtr(t *testing.T) { - val := boolPtr(true) - if val == nil || !*val { - t.Errorf("boolPtr(true) = %v, want pointer to true", val) - } - - val = boolPtr(false) - if val == nil || *val { - t.Errorf("boolPtr(false) = %v, want pointer to false", val) - } -} - func TestUpdateStatusWithRetry(t *testing.T) { cfg := &openvoxv1alpha1.Config{ ObjectMeta: metav1.ObjectMeta{ @@ -273,7 +254,7 @@ func TestUpdateStatusWithRetry_ConflictRetry(t *testing.T) { WithInterceptorFuncs(interceptor.Funcs{ SubResourceUpdate: func(ctx context.Context, client client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error { if calls.Add(1) == 1 { - return errors.NewConflict(schema.GroupResource{Group: "openvox.voxpupuli.org", Resource: "configs"}, obj.GetName(), fmt.Errorf("conflict")) + return apierrors.NewConflict(schema.GroupResource{Group: "openvox.voxpupuli.org", Resource: "configs"}, obj.GetName(), fmt.Errorf("conflict")) } return client.SubResource(subResourceName).Update(ctx, obj, opts...) }, diff --git a/internal/controller/image_pull_settings_test.go b/internal/controller/image_pull_settings_test.go index 6226867f..82effeb9 100644 --- a/internal/controller/image_pull_settings_test.go +++ b/internal/controller/image_pull_settings_test.go @@ -240,7 +240,7 @@ func TestSSLBootstrapped_TrueOnceSigned(t *testing.T) { // TestPullSecrets_ReachTheCASetupJob covers the third workload. func TestPullSecrets_ReachTheCASetupJob(t *testing.T) { ca := newCertificateAuthority("test-ca") - cfg := caPrereqs("test-ca") + cfg := caPrereqs() cfg.Spec.Image.PullSecrets = []corev1.LocalObjectReference{{Name: "registry-creds"}} r := newCertificateAuthorityReconciler(setupTestClient(ca, cfg)) diff --git a/internal/controller/labels.go b/internal/controller/labels.go index 9aa4ffca..56dd1201 100644 --- a/internal/controller/labels.go +++ b/internal/controller/labels.go @@ -1,7 +1,5 @@ package controller -import corev1 "k8s.io/api/core/v1" - const ( // Label keys used across all resources. LabelConfig = "openvox.voxpupuli.org/config" @@ -60,10 +58,3 @@ func databaseLabels(dbName string) map[string]string { LabelDatabase: dbName, } } - -func int64Ptr(i int64) *int64 { return &i } -func boolPtr(b bool) *bool { return &b } - -func fsGroupChangePolicyPtr(p corev1.PodFSGroupChangePolicy) *corev1.PodFSGroupChangePolicy { - return &p -} diff --git a/internal/controller/observed_generation_test.go b/internal/controller/observed_generation_test.go index 366931d7..8f15140e 100644 --- a/internal/controller/observed_generation_test.go +++ b/internal/controller/observed_generation_test.go @@ -125,7 +125,7 @@ func TestCondition_LastTransitionTimeIsStable(t *testing.T) { } first := readTransitionTime("the first reconcile") - for i := 0; i < 3; i++ { + for i := range 3 { if _, err := r.Reconcile(testCtx(), testRequest("production")); err != nil { t.Fatalf("reconcile %d: %v", i+2, err) } diff --git a/internal/controller/ownership_test.go b/internal/controller/ownership_test.go index 84216574..5acf69d4 100644 --- a/internal/controller/ownership_test.go +++ b/internal/controller/ownership_test.go @@ -78,7 +78,7 @@ func TestReconcilePDB_NoEventWhenUnchanged(t *testing.T) { } drain(rec) - for i := 0; i < 3; i++ { + for i := range 3 { if err := r.reconcilePDB(testCtx(), server); err != nil { t.Fatalf("reconcile %d: %v", i+2, err) } @@ -165,7 +165,7 @@ func TestPoolRouteHostname_IsDerivedNotWritten(t *testing.T) { server.Spec.CertificateRef = "web-cert" server.Spec.PoolRefs = []string{"puppet"} - pool := newPool("puppet", withRoute(true, "puppet.example.com", "gw")) + pool := newPool("puppet", withRoute("puppet.example.com", "gw")) pool.Spec.Route.InjectDNSAltName = true c := setupTestClient(ca, cert, server, pool) diff --git a/internal/controller/pause_test.go b/internal/controller/pause_test.go index 7c237404..76acd4a9 100644 --- a/internal/controller/pause_test.go +++ b/internal/controller/pause_test.go @@ -198,7 +198,7 @@ func TestPause_DoesNotWriteStatusRepeatedly(t *testing.T) { t.Fatalf("reading Config: %v", err) } - for i := 0; i < 3; i++ { + for i := range 3 { if _, err := r.Reconcile(testCtx(), testRequest("production")); err != nil { t.Fatalf("reconcile %d: %v", i+2, err) } diff --git a/internal/controller/pool_controller.go b/internal/controller/pool_controller.go index ca9041b7..5ba043c0 100644 --- a/internal/controller/pool_controller.go +++ b/internal/controller/pool_controller.go @@ -3,10 +3,11 @@ package controller import ( "context" "fmt" + "maps" corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -57,7 +58,7 @@ func (r *PoolReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. pool := &openvoxv1alpha1.Pool{} if err := r.Get(ctx, req.NamespacedName, pool); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return ctrl.Result{}, nil } return ctrl.Result{}, fmt.Errorf("getting Pool %s: %w", req.NamespacedName, err) @@ -242,7 +243,7 @@ func (r *PoolReconciler) deleteOwnedTLSRoute(ctx context.Context, pool *openvoxv existing := &gwapiv1.TLSRoute{} if err := r.Get(ctx, types.NamespacedName{Name: pool.Name, Namespace: pool.Namespace}, existing); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return nil } return fmt.Errorf("getting TLSRoute %s: %w", pool.Name, err) @@ -252,7 +253,7 @@ func (r *PoolReconciler) deleteOwnedTLSRoute(ctx context.Context, pool *openvoxv } logger.Info("deleting orphaned TLSRoute", "name", pool.Name) - if err := r.Delete(ctx, existing); err != nil && !errors.IsNotFound(err) { + if err := r.Delete(ctx, existing); err != nil && !apierrors.IsNotFound(err) { return fmt.Errorf("deleting orphaned TLSRoute: %w", err) } r.Recorder.Eventf(pool, nil, corev1.EventTypeNormal, EventReasonTLSRouteDeleted, "Reconcile", "TLSRoute %s deleted", pool.Name) @@ -358,9 +359,7 @@ func (r *PoolReconciler) reconcileService(ctx context.Context, pool *openvoxv1al "app.kubernetes.io/managed-by": "openvox-operator", poolLabel(pool.Name): "true", } - for k, v := range pool.Spec.Service.Labels { - labels[k] = v - } + maps.Copy(labels, pool.Spec.Service.Labels) svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{Name: svcName, Namespace: pool.Namespace}, @@ -432,7 +431,7 @@ func (r *PoolReconciler) reconcileTLSRoute(ctx context.Context, pool *openvoxv1a port := gwapiv1.PortNumber(8140) if pool.Spec.Service.Port > 0 { - port = gwapiv1.PortNumber(pool.Spec.Service.Port) + port = pool.Spec.Service.Port } parentRef := gwapiv1.ParentReference{ diff --git a/internal/controller/pool_controller_test.go b/internal/controller/pool_controller_test.go index 1eb76a1a..be85028b 100644 --- a/internal/controller/pool_controller_test.go +++ b/internal/controller/pool_controller_test.go @@ -148,7 +148,7 @@ func TestPoolReconcile_EndpointCount(t *testing.T) { } func TestPoolReconcile_TLSRouteCreation(t *testing.T) { - pool := newPool("puppet", withRoute(true, "puppet.example.com", "my-gateway")) + pool := newPool("puppet", withRoute("puppet.example.com", "my-gateway")) c := setupTestClient(pool) r := newPoolReconciler(c, true) @@ -187,7 +187,7 @@ func TestPoolReconcile_TLSRouteDisabled(t *testing.T) { func TestPoolReconcile_TLSRouteHostnameConflict(t *testing.T) { // Create pool-a first and reconcile it - pool1 := newPool("puppet-a", withRoute(true, "puppet.example.com", "gw")) + pool1 := newPool("puppet-a", withRoute("puppet.example.com", "gw")) c := setupTestClient(pool1) r := newPoolReconciler(c, true) @@ -196,7 +196,7 @@ func TestPoolReconcile_TLSRouteHostnameConflict(t *testing.T) { } // Now add pool-b with the same hostname - pool2 := newPool("puppet-b", withRoute(true, "puppet.example.com", "gw")) + pool2 := newPool("puppet-b", withRoute("puppet.example.com", "gw")) if err := c.Create(testCtx(), pool2); err != nil { t.Fatalf("failed to create pool-b: %v", err) } @@ -241,7 +241,7 @@ func TestPoolReconcile_UpdateExistingService(t *testing.T) { } func TestPoolReconcile_TLSRouteDirectUpdate(t *testing.T) { - pool := newPool("puppet", withRoute(true, "puppet.example.com", "my-gateway")) + pool := newPool("puppet", withRoute("puppet.example.com", "my-gateway")) c := setupTestClient(pool) r := newPoolReconciler(c, true) @@ -266,7 +266,7 @@ func TestPoolReconcile_TLSRouteDirectUpdate(t *testing.T) { } func TestPoolReconcile_TLSRouteWithSectionName(t *testing.T) { - pool := newPool("puppet", withRoute(true, "puppet.example.com", "my-gateway")) + pool := newPool("puppet", withRoute("puppet.example.com", "my-gateway")) pool.Spec.Route.GatewayRef.SectionName = "https" c := setupTestClient(pool) r := newPoolReconciler(c, true) @@ -285,7 +285,7 @@ func TestPoolReconcile_TLSRouteWithSectionName(t *testing.T) { } func TestPoolReconcile_TLSRouteCustomPort(t *testing.T) { - pool := newPool("puppet", withRoute(true, "puppet.example.com", "my-gateway"), withServicePort(9140)) + pool := newPool("puppet", withRoute("puppet.example.com", "my-gateway"), withServicePort(9140)) c := setupTestClient(pool) r := newPoolReconciler(c, true) @@ -297,7 +297,7 @@ func TestPoolReconcile_TLSRouteCustomPort(t *testing.T) { if err := c.Get(testCtx(), types.NamespacedName{Name: "puppet", Namespace: testNamespace}, route); err != nil { t.Fatalf("TLSRoute not created: %v", err) } - if route.Spec.Rules[0].BackendRefs[0].Port == nil || int32(*route.Spec.Rules[0].BackendRefs[0].Port) != 9140 { + if route.Spec.Rules[0].BackendRefs[0].Port == nil || *route.Spec.Rules[0].BackendRefs[0].Port != 9140 { t.Errorf("expected port 9140 on backend ref") } } diff --git a/internal/controller/pool_hostname_conflict_test.go b/internal/controller/pool_hostname_conflict_test.go index 86d0c857..d5b46cec 100644 --- a/internal/controller/pool_hostname_conflict_test.go +++ b/internal/controller/pool_hostname_conflict_test.go @@ -25,8 +25,8 @@ const conflictHostname = "puppet.example.com" // controller-runtime retry it with exponential backoff forever, which produced // nothing but a growing backlog of identical events. func TestPoolConflict_ReportedAsConditionNotRetried(t *testing.T) { - winner := newPool("pool-a", withRoute(true, conflictHostname, "gw")) - loser := newPool("pool-b", withRoute(true, conflictHostname, "gw")) + winner := newPool("pool-a", withRoute(conflictHostname, "gw")) + loser := newPool("pool-b", withRoute(conflictHostname, "gw")) c := setupTestClient(winner, loser) r := newPoolReconciler(c, true) @@ -60,9 +60,9 @@ func TestPoolConflict_ReportedAsConditionNotRetried(t *testing.T) { // the same list, so both must reach the same verdict, otherwise they take turns // creating and deleting the TLSRoute. func TestPoolConflict_OlderPoolKeepsTheHostname(t *testing.T) { - older := newPool("zzz-first", withRoute(true, conflictHostname, "gw")) + older := newPool("zzz-first", withRoute(conflictHostname, "gw")) older.CreationTimestamp = metav1.NewTime(time.Unix(1_700_000_000, 0)) - younger := newPool("aaa-second", withRoute(true, conflictHostname, "gw")) + younger := newPool("aaa-second", withRoute(conflictHostname, "gw")) younger.CreationTimestamp = metav1.NewTime(time.Unix(1_700_000_100, 0)) c := setupTestClient(older, younger) @@ -99,7 +99,7 @@ func TestPoolConflict_OlderPoolKeepsTheHostname(t *testing.T) { // TLSRoutes for one hostname: a Pool that held the route before an older // claimant enabled its own must release it. func TestPoolConflict_LoserGivesUpItsRoute(t *testing.T) { - loser := newPool("pool-b", withRoute(true, conflictHostname, "gw")) + loser := newPool("pool-b", withRoute(conflictHostname, "gw")) c := setupTestClient(loser) r := newPoolReconciler(c, true) @@ -112,7 +112,7 @@ func TestPoolConflict_LoserGivesUpItsRoute(t *testing.T) { } // pool-a takes the hostname on the tie-break. - winner := newPool("pool-a", withRoute(true, conflictHostname, "gw")) + winner := newPool("pool-a", withRoute(conflictHostname, "gw")) if err := c.Create(testCtx(), winner); err != nil { t.Fatalf("creating pool-a: %v", err) } @@ -130,10 +130,10 @@ func TestPoolConflict_LoserGivesUpItsRoute(t *testing.T) { // from holding a hostname hostage. func TestPoolConflict_TerminatingPoolReleasesTheHostname(t *testing.T) { now := metav1.Now() - leaving := newPool("pool-a", withRoute(true, conflictHostname, "gw")) + leaving := newPool("pool-a", withRoute(conflictHostname, "gw")) leaving.DeletionTimestamp = &now leaving.Finalizers = []string{"example.com/keep-around"} - successor := newPool("pool-b", withRoute(true, conflictHostname, "gw")) + successor := newPool("pool-b", withRoute(conflictHostname, "gw")) c := setupTestClient(leaving, successor) r := newPoolReconciler(c, true) @@ -150,15 +150,15 @@ func TestPoolConflict_TerminatingPoolReleasesTheHostname(t *testing.T) { // TestPoolConflict_EventFiresOncePerTransition guards against the event noise // the old backoff loop produced. func TestPoolConflict_EventFiresOncePerTransition(t *testing.T) { - winner := newPool("pool-a", withRoute(true, conflictHostname, "gw")) - loser := newPool("pool-b", withRoute(true, conflictHostname, "gw")) + winner := newPool("pool-a", withRoute(conflictHostname, "gw")) + loser := newPool("pool-b", withRoute(conflictHostname, "gw")) c := setupTestClient(winner, loser) r := newPoolReconciler(c, true) rec := events.NewFakeRecorder(100) r.Recorder = rec - for i := 0; i < 3; i++ { + for i := range 3 { if _, err := r.Reconcile(testCtx(), testRequest("pool-b")); err != nil { t.Fatalf("reconcile %d: %v", i, err) } @@ -189,8 +189,8 @@ func countEvents(rec *events.FakeRecorder, reason string) int { // hostname is free the Pool must pick it up, which is what the sibling watch is // there to trigger. func TestPoolConflict_ResolvedWhenTheWinnerGivesUp(t *testing.T) { - winner := newPool("pool-a", withRoute(true, conflictHostname, "gw")) - loser := newPool("pool-b", withRoute(true, conflictHostname, "gw")) + winner := newPool("pool-a", withRoute(conflictHostname, "gw")) + loser := newPool("pool-b", withRoute(conflictHostname, "gw")) c := setupTestClient(winner, loser) r := newPoolReconciler(c, true) @@ -223,9 +223,9 @@ func TestPoolConflict_ResolvedWhenTheWinnerGivesUp(t *testing.T) { // TestPoolsSharingHostname checks the watch that makes the resolution // above happen without polling. func TestPoolsSharingHostname(t *testing.T) { - changed := newPool("pool-a", withRoute(true, conflictHostname, "gw")) - sibling := newPool("pool-b", withRoute(true, conflictHostname, "gw")) - unrelated := newPool("pool-c", withRoute(true, "other.example.com", "gw")) + changed := newPool("pool-a", withRoute(conflictHostname, "gw")) + sibling := newPool("pool-b", withRoute(conflictHostname, "gw")) + unrelated := newPool("pool-c", withRoute("other.example.com", "gw")) noRoute := newPool("pool-d") c := setupTestClient(changed, sibling, unrelated, noRoute) @@ -242,8 +242,8 @@ func TestPoolsSharingHostname(t *testing.T) { // flight comes back with a nil Spec.Route. Reporting the conflict must not // reach into it. func TestPoolConflict_SurvivesRouteRemovedMidReconcile(t *testing.T) { - winner := newPool("pool-a", withRoute(true, conflictHostname, "gw")) - loser := newPool("pool-b", withRoute(true, conflictHostname, "gw")) + winner := newPool("pool-a", withRoute(conflictHostname, "gw")) + loser := newPool("pool-b", withRoute(conflictHostname, "gw")) gets := 0 c := fake.NewClientBuilder(). diff --git a/internal/controller/reportprocessor_controller.go b/internal/controller/reportprocessor_controller.go index cd43d025..eab02ca3 100644 --- a/internal/controller/reportprocessor_controller.go +++ b/internal/controller/reportprocessor_controller.go @@ -3,10 +3,11 @@ package controller import ( "context" "fmt" + "slices" "strings" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -44,7 +45,7 @@ func (r *ReportProcessorReconciler) Reconcile(ctx context.Context, req ctrl.Requ rp := &openvoxv1alpha1.ReportProcessor{} if err := r.Get(ctx, req.NamespacedName, rp); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return ctrl.Result{}, nil } return ctrl.Result{}, fmt.Errorf("getting ReportProcessor %s: %w", req.NamespacedName, err) @@ -100,7 +101,7 @@ func (r *ReportProcessorReconciler) observe(ctx context.Context, rp *openvoxv1al cfg := &openvoxv1alpha1.Config{} if err := r.Get(ctx, types.NamespacedName{Name: rp.Spec.ConfigRef, Namespace: rp.Namespace}, cfg); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return openvoxv1alpha1.ReportProcessorPhaseError, "ConfigNotFound", fmt.Sprintf("Config %s does not exist", rp.Spec.ConfigRef) } @@ -110,7 +111,7 @@ func (r *ReportProcessorReconciler) observe(ctx context.Context, rp *openvoxv1al secretName := fmt.Sprintf("%s-report-webhook", cfg.Name) secret := &corev1.Secret{} if err := r.Get(ctx, types.NamespacedName{Name: secretName, Namespace: rp.Namespace}, secret); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return openvoxv1alpha1.ReportProcessorPhaseError, "NotRendered", fmt.Sprintf("Secret %s has not been rendered yet", secretName) } @@ -122,11 +123,9 @@ func (r *ReportProcessorReconciler) observe(ctx context.Context, rp *openvoxv1al return openvoxv1alpha1.ReportProcessorPhaseError, "RenderedConfigUnreadable", fmt.Sprintf("Secret %s does not contain a readable report-webhook.yaml: %v", secretName, err) } - for _, name := range rendered { - if name == rp.Name { - return openvoxv1alpha1.ReportProcessorPhaseActive, "Rendered", - fmt.Sprintf("Endpoint is present in Secret %s", secretName) - } + if slices.Contains(rendered, rp.Name) { + return openvoxv1alpha1.ReportProcessorPhaseActive, "Rendered", + fmt.Sprintf("Endpoint is present in Secret %s", secretName) } return openvoxv1alpha1.ReportProcessorPhaseError, "NotRendered", diff --git a/internal/controller/reportprocessor_controller_status_test.go b/internal/controller/reportprocessor_controller_status_test.go index 5be0d668..592d02b1 100644 --- a/internal/controller/reportprocessor_controller_status_test.go +++ b/internal/controller/reportprocessor_controller_status_test.go @@ -23,7 +23,7 @@ func webhookSecret(cfgName string, endpointNames ...string) *corev1.Secret { } func TestReportProcessorReconcile_Status(t *testing.T) { - rp := newReportProcessor("test-rp", "production", "https://puppetdb.example.invalid") + rp := newReportProcessor("test-rp", "https://puppetdb.example.invalid") cfg := newConfig("production") key := types.NamespacedName{Name: "test-rp", Namespace: testNamespace} diff --git a/internal/controller/reportprocessor_controller_test.go b/internal/controller/reportprocessor_controller_test.go index e1e31860..9a494abf 100644 --- a/internal/controller/reportprocessor_controller_test.go +++ b/internal/controller/reportprocessor_controller_test.go @@ -18,7 +18,7 @@ func TestReportProcessorReconcile_NotFound(t *testing.T) { } func TestReportProcessorReconcile_BasicReconcile(t *testing.T) { - rp := newReportProcessor("test-rp", "production", "https://reports.example.com") + rp := newReportProcessor("test-rp", "https://reports.example.com") c := setupTestClient(rp) r := newReportProcessorReconciler(c) diff --git a/internal/controller/securitycontext.go b/internal/controller/securitycontext.go index f859785e..eaf2cea2 100644 --- a/internal/controller/securitycontext.go +++ b/internal/controller/securitycontext.go @@ -15,13 +15,15 @@ import ( // root. Callers pass their workload-specific uid/gid/fsGroup constants; users can // override individual fields via the CRD (e.g. on OpenShift or PSA-restricted // namespaces that assign their own UID/GID ranges). +// +//nolint:unparam // every workload currently runs as uid 1001; the uid stays a parameter alongside group and fsGroup so per-workload constants remain possible func buildPodSecurityContext(defaultUser, defaultGroup, defaultFSGroup int64, override *openvoxv1alpha1.PodSecurityContextSpec) *corev1.PodSecurityContext { psc := &corev1.PodSecurityContext{ - RunAsUser: int64Ptr(defaultUser), - RunAsGroup: int64Ptr(defaultGroup), - RunAsNonRoot: boolPtr(true), - FSGroup: int64Ptr(defaultFSGroup), - FSGroupChangePolicy: fsGroupChangePolicyPtr(corev1.FSGroupChangeOnRootMismatch), + RunAsUser: new(defaultUser), + RunAsGroup: new(defaultGroup), + RunAsNonRoot: new(true), + FSGroup: new(defaultFSGroup), + FSGroupChangePolicy: new(corev1.FSGroupChangeOnRootMismatch), SeccompProfile: &corev1.SeccompProfile{ Type: corev1.SeccompProfileTypeRuntimeDefault, }, diff --git a/internal/controller/server_controller.go b/internal/controller/server_controller.go index 860dfca1..2830a896 100644 --- a/internal/controller/server_controller.go +++ b/internal/controller/server_controller.go @@ -3,13 +3,14 @@ package controller import ( "context" "fmt" + "math" appsv1 "k8s.io/api/apps/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" policyv1 "k8s.io/api/policy/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -66,7 +67,7 @@ func (r *ServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr server := &openvoxv1alpha1.Server{} if err := r.Get(ctx, req.NamespacedName, server); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { // The Server carries no finalizer, so this is the only point at // which its gauges can be retired. forgetServerMetrics(req.Name, req.Namespace) @@ -96,7 +97,7 @@ func (r *ServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Resolve Config cfg := &openvoxv1alpha1.Config{} if err := r.Get(ctx, types.NamespacedName{Name: server.Spec.ConfigRef, Namespace: server.Namespace}, cfg); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("waiting for Config", "configRef", server.Spec.ConfigRef) return ctrl.Result{RequeueAfter: RequeueIntervalShort}, nil } @@ -106,7 +107,7 @@ func (r *ServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Resolve Certificate -- wait until phase is Signed cert := &openvoxv1alpha1.Certificate{} if err := r.Get(ctx, types.NamespacedName{Name: server.Spec.CertificateRef, Namespace: server.Namespace}, cert); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("waiting for Certificate", "certificateRef", server.Spec.CertificateRef) r.reportSSLBootstrapped(ctx, server, metav1.ConditionFalse, "CertificateNotFound", fmt.Sprintf("Certificate %s does not exist", server.Spec.CertificateRef)) @@ -135,7 +136,7 @@ func (r *ServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Resolve CertificateAuthority (needed for CA PVC name when ca: true) ca := &openvoxv1alpha1.CertificateAuthority{} if err := r.Get(ctx, types.NamespacedName{Name: cert.Spec.AuthorityRef, Namespace: server.Namespace}, ca); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("waiting for CertificateAuthority", "authorityRef", cert.Spec.AuthorityRef) return ctrl.Result{RequeueAfter: RequeueIntervalShort}, nil } @@ -251,11 +252,11 @@ func (r *ServerReconciler) reconcilePDB(ctx context.Context, server *openvoxv1al return guardErr } logger.Info("deleting PDB (disabled)", "name", pdbName) - if err := r.Delete(ctx, existing); err != nil && !errors.IsNotFound(err) { + if err := r.Delete(ctx, existing); err != nil && !apierrors.IsNotFound(err) { return fmt.Errorf("deleting PodDisruptionBudget %s: %w", pdbName, err) } r.Recorder.Eventf(server, nil, corev1.EventTypeNormal, EventReasonPDBDeleted, "Reconcile", "PodDisruptionBudget %s deleted", pdbName) - } else if !errors.IsNotFound(err) { + } else if !apierrors.IsNotFound(err) { return fmt.Errorf("getting PodDisruptionBudget %s: %w", pdbName, err) } return nil @@ -308,11 +309,12 @@ func (r *ServerReconciler) buildPDB(server *openvoxv1alpha1.Server) (*policyv1.P }, }, } - if server.Spec.PDB.MinAvailable != nil { + switch { + case server.Spec.PDB.MinAvailable != nil: pdb.Spec.MinAvailable = server.Spec.PDB.MinAvailable - } else if server.Spec.PDB.MaxUnavailable != nil { + case server.Spec.PDB.MaxUnavailable != nil: pdb.Spec.MaxUnavailable = server.Spec.PDB.MaxUnavailable - } else { + default: // Default: minAvailable: 1 minAvailable := intstrInt(DefaultPDBMinAvailable) pdb.Spec.MinAvailable = &minAvailable @@ -336,11 +338,11 @@ func (r *ServerReconciler) reconcileHPA(ctx context.Context, server *openvoxv1al return guardErr } logger.Info("deleting HPA (disabled)", "name", hpaName) - if err := r.Delete(ctx, existing); err != nil && !errors.IsNotFound(err) { + if err := r.Delete(ctx, existing); err != nil && !apierrors.IsNotFound(err) { return fmt.Errorf("deleting HorizontalPodAutoscaler %s: %w", hpaName, err) } r.Recorder.Eventf(server, nil, corev1.EventTypeNormal, EventReasonHPADeleted, "Reconcile", "HorizontalPodAutoscaler %s deleted", hpaName) - } else if !errors.IsNotFound(err) { + } else if !apierrors.IsNotFound(err) { return fmt.Errorf("getting HorizontalPodAutoscaler %s: %w", hpaName, err) } return nil @@ -436,11 +438,11 @@ func (r *ServerReconciler) reconcileNetworkPolicy(ctx context.Context, server *o return guardErr } logger.Info("deleting NetworkPolicy (disabled)", "name", npName) - if err := r.Delete(ctx, existing); err != nil && !errors.IsNotFound(err) { + if err := r.Delete(ctx, existing); err != nil && !apierrors.IsNotFound(err) { return fmt.Errorf("deleting NetworkPolicy %s: %w", npName, err) } r.Recorder.Eventf(server, nil, corev1.EventTypeNormal, EventReasonNetworkPolicyDeleted, "Reconcile", "NetworkPolicy %s deleted", npName) - } else if !errors.IsNotFound(err) { + } else if !apierrors.IsNotFound(err) { return fmt.Errorf("getting NetworkPolicy %s: %w", npName, err) } return nil @@ -534,6 +536,11 @@ func (r *ServerReconciler) SetupWithManager(mgr ctrl.Manager) error { } func intstrInt(val int) intstr.IntOrString { + if val > math.MaxInt32 { + val = math.MaxInt32 + } else if val < math.MinInt32 { + val = math.MinInt32 + } return intstr.FromInt32(int32(val)) } @@ -546,7 +553,7 @@ func intstrInt(val int) intstr.IntOrString { func (r *ServerReconciler) getReadyReplicas(ctx context.Context, server *openvoxv1alpha1.Server) (int32, error) { deploy := &appsv1.Deployment{} if err := r.Get(ctx, types.NamespacedName{Name: server.Name, Namespace: server.Namespace}, deploy); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return 0, nil } return 0, fmt.Errorf("getting Deployment %s: %w", server.Name, err) diff --git a/internal/controller/server_controller_test.go b/internal/controller/server_controller_test.go index d0505b55..acf07784 100644 --- a/internal/controller/server_controller_test.go +++ b/internal/controller/server_controller_test.go @@ -6,6 +6,7 @@ import ( appsv1 "k8s.io/api/apps/v1" autoscalingv2 "k8s.io/api/autoscaling/v2" + corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" policyv1 "k8s.io/api/policy/v1" "k8s.io/apimachinery/pkg/types" @@ -180,7 +181,12 @@ func TestServerReconcile_AnnotationHashes(t *testing.T) { } } -func TestServerReconcile_AutosignPolicyHashAnnotation(t *testing.T) { +// TestServerReconcile_AutosignPolicyIsLiveMounted replaces the former hash +// annotation test. A policy edit must not roll the CA pod: the CA Deployment +// uses the Recreate strategy, so a restart is a short outage with no signing +// and no CRL - for a change that alters no running state. The binary re-reads +// the file on every CSR, so nothing downstream needs the restart either. +func TestServerReconcile_AutosignPolicyIsLiveMounted(t *testing.T) { objs := append(serverPrereqs(), newSecret("production-ca-autosign-policy", map[string][]byte{ "autosign-policy.yaml": []byte("policies:\n"), @@ -199,9 +205,26 @@ func TestServerReconcile_AutosignPolicyHashAnnotation(t *testing.T) { t.Fatalf("Deployment not found: %v", err) } - // The CA pod must carry the autosign-policy hash so a SigningPolicy change rolls it. - if v, ok := deploy.Spec.Template.Annotations["openvox.voxpupuli.org/autosign-policy-secret-hash"]; !ok || v == "" { - t.Error("CA pod should carry the autosign-policy-secret-hash annotation") + if _, ok := deploy.Spec.Template.Annotations["openvox.voxpupuli.org/autosign-policy-secret-hash"]; ok { + t.Error("the policy hash must not be in the pod template any more, it would roll the CA on every edit") + } + + // A SubPath mount is never refreshed by the kubelet, which is what forced + // the restart. The mount has to stay a directory for the sync to happen. + var mount *corev1.VolumeMount + for i := range deploy.Spec.Template.Spec.Containers[0].VolumeMounts { + if deploy.Spec.Template.Spec.Containers[0].VolumeMounts[i].Name == "autosign-policy" { + mount = &deploy.Spec.Template.Spec.Containers[0].VolumeMounts[i] + } + } + if mount == nil { + t.Fatal("the CA pod must mount the autosign policy") + } + if mount.SubPath != "" { + t.Errorf("the policy must not be mounted with SubPath, got %q", mount.SubPath) + } + if mount.MountPath != autosignPolicyDir { + t.Errorf("expected the policy directory %q, got %q", autosignPolicyDir, mount.MountPath) } } diff --git a/internal/controller/server_deployment.go b/internal/controller/server_deployment.go index 558a01e4..66764eb3 100644 --- a/internal/controller/server_deployment.go +++ b/internal/controller/server_deployment.go @@ -7,7 +7,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" @@ -118,17 +118,9 @@ func (r *ServerReconciler) reconcileDeployment(ctx context.Context, server *open // SigningPolicy changes. The policy Secret is subPath-mounted, so kubelet does not // live-sync it; hashing it into the pod template rolls the CA pod when the rendered // policy changes, so a SigningPolicy edit applies without a manual restart. - // Skipped when a custom autosignCommand disables the SigningPolicy-driven flow. - if server.Spec.CA && cfg.Spec.Puppet.AutosignCommand == "" { - autosignSecretName := fmt.Sprintf("%s-autosign-policy", ca.Name) - if autosignHash, err := r.secretHash(ctx, autosignSecretName, server.Namespace); err == nil { - annotations["openvox.voxpupuli.org/autosign-policy-secret-hash"] = autosignHash - } - } - deploy := &appsv1.Deployment{} err = r.Get(ctx, types.NamespacedName{Name: deployName, Namespace: server.Namespace}, deploy) - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { logger.Info("creating Server Deployment", "name", deployName, "role", role, "replicas", replicas) deploy = &appsv1.Deployment{ @@ -312,10 +304,15 @@ func (r *ServerReconciler) buildPodSpec(server *openvoxv1alpha1.Server, cfg *ope // flow: the command is then responsible for its own signing decision. if cfg.Spec.Puppet.AutosignCommand == "" { autosignSecretName := fmt.Sprintf("%s-autosign-policy", ca.Name) + // Mounted as a directory rather than with SubPath: a SubPath mount is + // never refreshed by the kubelet, which is why this used to need a + // pod restart on every policy change. The CA Deployment uses the + // Recreate strategy, so that restart was a short CA outage - no + // signing, no CRL - for an edit that changes no running state. The + // CRL Secret is mounted the same way for the same reason. volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: "autosign-policy", - MountPath: "/etc/puppetlabs/puppet/autosign-policy.yaml", - SubPath: "autosign-policy.yaml", + MountPath: autosignPolicyDir, ReadOnly: true, }) volumes = append(volumes, corev1.Volume{ @@ -436,7 +433,7 @@ func (r *ServerReconciler) buildPodSpec(server *openvoxv1alpha1.Server, cfg *ope VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ SecretName: reportSecretName, - Optional: boolPtr(true), + Optional: new(true), }, }, }, @@ -448,9 +445,8 @@ func (r *ServerReconciler) buildPodSpec(server *openvoxv1alpha1.Server, cfg *ope volumes = append(volumes, server.Spec.ExtraVolumes...) volumeMounts = append(volumeMounts, server.Spec.ExtraVolumeMounts...) - env := []corev1.EnvVar{ - {Name: "JAVA_ARGS", Value: javaArgs}, - } + env := make([]corev1.EnvVar, 0, 1+len(server.Spec.ExtraEnv)) + env = append(env, corev1.EnvVar{Name: "JAVA_ARGS", Value: javaArgs}) env = append(env, server.Spec.ExtraEnv...) container := corev1.Container{ @@ -518,8 +514,8 @@ chmod 640 /ssl/private_keys/puppet.pem` } containerSecurityContext := &corev1.SecurityContext{ - AllowPrivilegeEscalation: boolPtr(false), - ReadOnlyRootFilesystem: boolPtr(resolveReadOnlyRootFilesystem(server, cfg)), + AllowPrivilegeEscalation: new(false), + ReadOnlyRootFilesystem: new(resolveReadOnlyRootFilesystem(server, cfg)), Capabilities: &corev1.Capabilities{ Drop: []corev1.Capability{"ALL"}, }, diff --git a/internal/controller/server_deployment_test.go b/internal/controller/server_deployment_test.go index 9005fafd..95776584 100644 --- a/internal/controller/server_deployment_test.go +++ b/internal/controller/server_deployment_test.go @@ -220,7 +220,7 @@ func TestBuildPodSpec_AutosignCommandSkipsPolicyMount(t *testing.T) { func TestBuildPodSpec_ExternalNodesCommandSkipsENCMount(t *testing.T) { cfg := newConfig("production", - withNodeClassifierRef("my-enc"), + withNodeClassifierRef(), withExternalNodesCommand("/usr/local/bin/custom-enc"), ) server := newServer("test-server", withServerRole(true)) @@ -465,7 +465,7 @@ func TestBuildPodSpec_SecurityContextOverride(t *testing.T) { } func TestBuildPodSpec_ENCVolumes(t *testing.T) { - cfg := newConfig("production", withNodeClassifierRef("my-enc")) + cfg := newConfig("production", withNodeClassifierRef()) server := newServer("test-server", withServerRole(true)) podSpec := testBuildPodSpec(server, cfg) diff --git a/internal/controller/testutil_test.go b/internal/controller/testutil_test.go index b5ea1951..5f932fd8 100644 --- a/internal/controller/testutil_test.go +++ b/internal/controller/testutil_test.go @@ -86,7 +86,7 @@ func newConfig(name string, opts ...configOption) *openvoxv1alpha1.Config { Namespace: testNamespace, }, Spec: openvoxv1alpha1.ConfigSpec{ - ReadOnlyRootFilesystem: boolPtr(true), + ReadOnlyRootFilesystem: new(true), Image: openvoxv1alpha1.ImageSpec{ Repository: "ghcr.io/slauger/openvox-server-8", Tag: "latest", @@ -96,7 +96,7 @@ func newConfig(name string, opts ...configOption) *openvoxv1alpha1.Config { EnvironmentTimeout: "unlimited", EnvironmentPath: "/etc/puppetlabs/code/environments", HieraConfig: "$confdir/hiera.yaml", - Storeconfigs: boolPtr(true), + Storeconfigs: new(true), StoreBackend: "puppetdb", Reports: "puppetdb", }, @@ -114,15 +114,15 @@ func withAuthorityRef(ref string) configOption { } } -func withNodeClassifierRef(ref string) configOption { +func withNodeClassifierRef() configOption { return func(c *openvoxv1alpha1.Config) { - c.Spec.NodeClassifierRef = ref + c.Spec.NodeClassifierRef = "my-enc" } } func withReadOnlyRootFS(v bool) configOption { return func(c *openvoxv1alpha1.Config) { - c.Spec.ReadOnlyRootFilesystem = boolPtr(v) + c.Spec.ReadOnlyRootFilesystem = new(v) } } @@ -201,7 +201,7 @@ func newServer(name string, opts ...serverOption) *openvoxv1alpha1.Server { Spec: openvoxv1alpha1.ServerSpec{ ConfigRef: "production", CertificateRef: "production-cert", - Server: boolPtr(true), + Server: new(true), CA: false, Replicas: &replicas, }, @@ -216,14 +216,14 @@ func withCA(ca bool) serverOption { return func(s *openvoxv1alpha1.Server) { s.Spec.CA = ca if ca && !serverRoleEnabled(s) { - s.Spec.Server = boolPtr(false) + s.Spec.Server = new(false) } } } func withServerRole(server bool) serverOption { return func(s *openvoxv1alpha1.Server) { - s.Spec.Server = boolPtr(server) + s.Spec.Server = new(server) } } @@ -320,10 +320,10 @@ func withServiceAnnotations(a map[string]string) poolOption { } } -func withRoute(enabled bool, hostname, gwName string) poolOption { +func withRoute(hostname, gwName string) poolOption { return func(p *openvoxv1alpha1.Pool) { p.Spec.Route = &openvoxv1alpha1.PoolRouteSpec{ - Enabled: enabled, + Enabled: true, Hostname: hostname, GatewayRef: openvoxv1alpha1.GatewayReference{ Name: gwName, @@ -395,10 +395,10 @@ func newCertificateAuthority(name string, opts ...caOption) *openvoxv1alpha1.Cer }, Spec: openvoxv1alpha1.CertificateAuthoritySpec{ TTL: "5y", - AllowSubjectAltNames: boolPtr(true), - AllowAuthorizationExtensions: boolPtr(true), - EnableInfraCRL: boolPtr(true), - AllowAutoRenewal: boolPtr(true), + AllowSubjectAltNames: new(true), + AllowAuthorizationExtensions: new(true), + EnableInfraCRL: new(true), + AllowAutoRenewal: new(true), AutoRenewalCertTTL: "90d", }, } @@ -449,14 +449,14 @@ func newNodeClassifier(name, url string) *openvoxv1alpha1.NodeClassifier { } } -func newReportProcessor(name, configRef, url string) *openvoxv1alpha1.ReportProcessor { +func newReportProcessor(name, url string) *openvoxv1alpha1.ReportProcessor { return &openvoxv1alpha1.ReportProcessor{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: testNamespace, }, Spec: openvoxv1alpha1.ReportProcessorSpec{ - ConfigRef: configRef, + ConfigRef: "production", URL: url, TimeoutSeconds: 30, }, @@ -495,7 +495,7 @@ func newEndpointSlice(name, serviceName string, readyCount int) *discoveryv1.End AddressType: discoveryv1.AddressTypeIPv4, } ready := true - for i := 0; i < readyCount; i++ { + for range readyCount { eps.Endpoints = append(eps.Endpoints, discoveryv1.Endpoint{ Conditions: discoveryv1.EndpointConditions{ Ready: &ready, diff --git a/internal/puppet/oids.go b/internal/puppet/oids.go index a8c7fc80..e6e37765 100644 --- a/internal/puppet/oids.go +++ b/internal/puppet/oids.go @@ -36,14 +36,46 @@ var PuppetOIDs = map[string]asn1.ObjectIdentifier{ "challengePassword": {1, 2, 840, 113549, 1, 9, 7}, } +// AuthorizationArc is the OID prefix for Puppet's privileged authorization +// extensions (pp_authorization, pp_auth_token, pp_cli_auth). An extension whose +// OID falls under this arc grants elevated trust (e.g. pp_cli_auth=true is +// honored by the CA admin auth.conf rules) and must never be auto-signed unless +// a policy explicitly allows it. +var AuthorizationArc = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 34380, 1, 3} + // OIDByName returns the ASN.1 OID for a known Puppet extension name. func OIDByName(name string) (asn1.ObjectIdentifier, bool) { oid, ok := PuppetOIDs[name] return oid, ok } +// NameByOID returns the Puppet extension name for a known OID. +func NameByOID(oid asn1.ObjectIdentifier) (string, bool) { + for name, candidate := range PuppetOIDs { + if candidate.Equal(oid) { + return name, true + } + } + return "", false +} + // IsKnownOID reports whether name is a recognized Puppet extension name. func IsKnownOID(name string) bool { _, ok := PuppetOIDs[name] return ok } + +// IsAuthorizationOID reports whether oid falls under the privileged +// authorization arc. The check is prefix-based so that authorization OIDs not +// (yet) present in PuppetOIDs are still recognized as privileged. +func IsAuthorizationOID(oid asn1.ObjectIdentifier) bool { + if len(oid) < len(AuthorizationArc) { + return false + } + for i := range AuthorizationArc { + if oid[i] != AuthorizationArc[i] { + return false + } + } + return true +} diff --git a/internal/webhook/certificateauthority_delete_test.go b/internal/webhook/certificateauthority_delete_test.go index 5231ccaa..16b6ad82 100644 --- a/internal/webhook/certificateauthority_delete_test.go +++ b/internal/webhook/certificateauthority_delete_test.go @@ -7,7 +7,6 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" openvoxv1alpha1 "github.com/slauger/openvox-operator/api/v1alpha1" ) @@ -64,7 +63,7 @@ func TestValidateStorageTransition(t *testing.T) { Spec: openvoxv1alpha1.CertificateAuthoritySpec{Storage: &openvoxv1alpha1.StorageSpec{StorageClass: class}}, } if size != "" { - ca.Spec.Storage.Size = ptr.To(resource.MustParse(size)) + ca.Spec.Storage.Size = new(resource.MustParse(size)) } return ca } diff --git a/internal/webhook/certificateauthority_webhook_test.go b/internal/webhook/certificateauthority_webhook_test.go index 098c081f..c315d8f1 100644 --- a/internal/webhook/certificateauthority_webhook_test.go +++ b/internal/webhook/certificateauthority_webhook_test.go @@ -6,7 +6,6 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" openvoxv1alpha1 "github.com/slauger/openvox-operator/api/v1alpha1" ) @@ -39,7 +38,7 @@ func TestCertificateAuthorityValidator(t *testing.T) { TTL: "5y", AutoRenewalCertTTL: "90d", CRLRefreshInterval: "5m", - Storage: &openvoxv1alpha1.StorageSpec{Size: ptr.To(resource.MustParse("1Gi"))}, + Storage: &openvoxv1alpha1.StorageSpec{Size: new(resource.MustParse("1Gi"))}, }, } _, err := v.ValidateCreate(context.Background(), ca) diff --git a/internal/webhook/helpers.go b/internal/webhook/helpers.go index d8aa3b22..3e41f777 100644 --- a/internal/webhook/helpers.go +++ b/internal/webhook/helpers.go @@ -5,7 +5,7 @@ import ( "fmt" "net/url" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" @@ -35,7 +35,7 @@ func validateCodeList(code []openvoxv1alpha1.CodeSpec, path *field.Path) field.E func refExists[T client.Object](ctx context.Context, c client.Reader, ns, name string, obj T) error { key := types.NamespacedName{Namespace: ns, Name: name} if err := c.Get(ctx, key, obj); err != nil { - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return fmt.Errorf("referenced %T %q not found", obj, name) } return fmt.Errorf("looking up %T %q: %w", obj, name, err) diff --git a/internal/webhook/signingpolicy_webhook.go b/internal/webhook/signingpolicy_webhook.go index 3d6a753a..479fe384 100644 --- a/internal/webhook/signingpolicy_webhook.go +++ b/internal/webhook/signingpolicy_webhook.go @@ -2,6 +2,7 @@ package webhook import ( "context" + "net" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" @@ -36,18 +37,51 @@ func (v *SigningPolicyValidator) validate(ctx context.Context, sp *openvoxv1alph errs = append(errs, field.Invalid(specPath.Child("certificateAuthorityRef"), sp.Spec.CertificateAuthorityRef, err.Error())) } - if sp.Spec.Pattern != nil { - for i, pattern := range sp.Spec.Pattern.Allow { + if sp.Spec.Certnames != nil { + for i, pattern := range sp.Spec.Certnames.Allow { if pattern == "" { - errs = append(errs, field.Invalid(specPath.Child("pattern", "allow").Index(i), pattern, "pattern must not be empty")) + errs = append(errs, field.Invalid(specPath.Child("certnames", "allow").Index(i), pattern, "certname pattern must not be empty")) } } } - if sp.Spec.DNSAltNames != nil { - for i, pattern := range sp.Spec.DNSAltNames.Allow { + for _, f := range []struct { + name string + spec *openvoxv1alpha1.PatternSpec + }{ + {"dnsAltNames", sp.Spec.DNSAltNames}, + {"uriAltNames", sp.Spec.URIAltNames}, + {"emailAltNames", sp.Spec.EmailAltNames}, + } { + if f.spec == nil { + continue + } + for i, pattern := range f.spec.Allow { if pattern == "" { - errs = append(errs, field.Invalid(specPath.Child("dnsAltNames", "allow").Index(i), pattern, "pattern must not be empty")) + errs = append(errs, field.Invalid(specPath.Child(f.name, "allow").Index(i), pattern, "pattern must not be empty")) + } + } + } + + if sp.Spec.IPAltNames != nil { + for i, cidr := range sp.Spec.IPAltNames.Allow { + if _, _, err := net.ParseCIDR(cidr); err != nil { + errs = append(errs, field.Invalid(specPath.Child("ipAltNames", "allow").Index(i), cidr, "must be a valid CIDR, e.g. 10.0.0.0/16")) + } + } + } + + // Extensions listed here can only take effect if they are known Puppet OIDs + // (the autosign guard resolves CSR extensions to names). Reject unknown names + // early so a typo doesn't silently fail to allow a privileged extension. + if sp.Spec.Extensions != nil { + for i, name := range sp.Spec.Extensions.Allow { + extPath := specPath.Child("extensions", "allow").Index(i) + switch { + case name == "": + errs = append(errs, field.Invalid(extPath, name, "extension name must not be empty")) + case !puppet.IsKnownOID(name): + errs = append(errs, field.Invalid(extPath, name, "unknown Puppet extension name")) } } } diff --git a/internal/webhook/signingpolicy_webhook_test.go b/internal/webhook/signingpolicy_webhook_test.go index 8c2d5146..5fbe72a4 100644 --- a/internal/webhook/signingpolicy_webhook_test.go +++ b/internal/webhook/signingpolicy_webhook_test.go @@ -45,7 +45,7 @@ func TestSigningPolicyValidator(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, Spec: openvoxv1alpha1.SigningPolicySpec{ CertificateAuthorityRef: "my-ca", - Pattern: &openvoxv1alpha1.PatternSpec{ + Certnames: &openvoxv1alpha1.PatternSpec{ Allow: []string{"*.example.com"}, }, }, @@ -78,7 +78,7 @@ func TestSigningPolicyValidator(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, Spec: openvoxv1alpha1.SigningPolicySpec{ CertificateAuthorityRef: "my-ca", - Pattern: &openvoxv1alpha1.PatternSpec{ + Certnames: &openvoxv1alpha1.PatternSpec{ Allow: []string{"*.example.com", ""}, }, }, @@ -158,6 +158,52 @@ func TestSigningPolicyValidator(t *testing.T) { } }) + t.Run("valid ipAltNames CIDR and extensions", func(t *testing.T) { + c := setupTestClient(ca) + v := &SigningPolicyValidator{Client: c} + sp := &openvoxv1alpha1.SigningPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: openvoxv1alpha1.SigningPolicySpec{ + CertificateAuthorityRef: "my-ca", + IPAltNames: &openvoxv1alpha1.PatternSpec{Allow: []string{"10.0.0.0/16", "::1/128"}}, + Extensions: &openvoxv1alpha1.PatternSpec{Allow: []string{"pp_cli_auth"}}, + }, + } + if _, err := v.ValidateCreate(context.Background(), sp); err != nil { + t.Errorf("expected no error, got %v", err) + } + }) + + t.Run("invalid ipAltNames CIDR is rejected", func(t *testing.T) { + c := setupTestClient(ca) + v := &SigningPolicyValidator{Client: c} + sp := &openvoxv1alpha1.SigningPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: openvoxv1alpha1.SigningPolicySpec{ + CertificateAuthorityRef: "my-ca", + IPAltNames: &openvoxv1alpha1.PatternSpec{Allow: []string{"10.0.0.5"}}, // not a CIDR + }, + } + if _, err := v.ValidateCreate(context.Background(), sp); err == nil { + t.Error("expected error for non-CIDR ipAltNames entry") + } + }) + + t.Run("unknown extension name is rejected", func(t *testing.T) { + c := setupTestClient(ca) + v := &SigningPolicyValidator{Client: c} + sp := &openvoxv1alpha1.SigningPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: openvoxv1alpha1.SigningPolicySpec{ + CertificateAuthorityRef: "my-ca", + Extensions: &openvoxv1alpha1.PatternSpec{Allow: []string{"not_a_real_extension"}}, + }, + } + if _, err := v.ValidateCreate(context.Background(), sp); err == nil { + t.Error("expected error for unknown extension name") + } + }) + t.Run("delete always succeeds", func(t *testing.T) { v := &SigningPolicyValidator{Client: setupTestClient()} _, err := v.ValidateDelete(context.Background(), &openvoxv1alpha1.SigningPolicy{}) diff --git a/mkdocs.yml b/mkdocs.yml index d4e6d0c7..156cba3e 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -66,6 +66,7 @@ nav: - Installation: getting-started/installation.md - Quick Start: getting-started/quickstart.md - Guides: + - Connecting Agents: guides/connecting-agents.md - CA Import & External CA: guides/ca-import.md - Monitoring: guides/monitoring.md - Pausing Reconciliation: guides/pausing-reconciliation.md diff --git a/tests/e2e/autosign-policy/chainsaw-test.yaml b/tests/e2e/autosign-policy/chainsaw-test.yaml index f8bb7e1d..f6629fa6 100644 --- a/tests/e2e/autosign-policy/chainsaw-test.yaml +++ b/tests/e2e/autosign-policy/chainsaw-test.yaml @@ -124,6 +124,83 @@ spec: kubectl wait --for=jsonpath='{.status.failed}'=1 job/puppet-agent-nomatch \ -n e2e-autosign-policy --timeout=120s + - name: Agent requesting the reserved operator certname (expect failure) + description: | + The CA auth.conf grants admin rights to the operator-signing certname as + well as to the pp_cli_auth extension, so an agent that obtained a + certificate under that name would be a CA admin. + + This agent presents the *correct* preshared key, so the policy itself + would approve it. + + What this step can and cannot show: the operator already holds a + certificate under that name, so puppetserver would refuse a second CSR + for it regardless. The job failing is therefore a regression guard, not + proof of the reservation. The reservation matters for the window before + the operator issues its own certificate, and after that certificate is + removed - neither of which is reachable from a chainsaw step. It is + proven by unit tests in cmd/autosign; what is asserted end to end here + is that the operator wires the reservation into the rendered policy. + try: + - apply: + resource: + apiVersion: batch/v1 + kind: Job + metadata: + name: puppet-agent-reserved + namespace: e2e-autosign-policy + spec: + activeDeadlineSeconds: 60 + backoffLimit: 0 + template: + spec: + restartPolicy: Never + containers: + - name: puppet-agent + image: (join('', [$registry, '/openvox-agent-', $major, ':', $imageTag])) + imagePullPolicy: Always + command: ["sh", "-c"] + args: + - | + mkdir -p /etc/puppetlabs/puppet + cat > /etc/puppetlabs/puppet/csr_attributes.yaml << 'CSRATTR' + extension_requests: + pp_preshared_key: e2e-correct-key + CSRATTR + puppet agent --test \ + --server e2e-autosign-policy-server \ + --waitforcert 5 \ + --certname e2e-autosign-policy-ca-operator; + EXIT=$?; + if [ $EXIT -eq 0 ] || [ $EXIT -eq 2 ]; then exit 0; else exit $EXIT; fi + securityContext: + runAsUser: 0 + - script: + timeout: 3m + content: | + kubectl wait --for=jsonpath='{.status.failed}'=1 job/puppet-agent-reserved \ + -n e2e-autosign-policy --timeout=120s + - script: + timeout: 1m + content: | + set -eu + # Assert the operator actually wired the reservation into the + # policy the autosign binary reads. Checking the CA's signed + # directory would prove nothing here: the operator legitimately + # holds a certificate under this name, so the file exists either + # way - and because the name is taken, puppetserver would refuse a + # second CSR for it even without the reservation. + POLICY=$(kubectl get secret e2e-autosign-policy-ca-autosign-policy \ + -n e2e-autosign-policy -o jsonpath='{.data.autosign-policy\.yaml}' | base64 -d) + + echo "${POLICY}" | grep -q "reservedCertnames:" \ + || { echo "ERROR: the rendered policy carries no reservation"; echo "${POLICY}"; exit 1; } + echo "${POLICY}" | grep -q "e2e-autosign-policy-ca-operator" \ + || { echo "ERROR: the operator certname is not reserved"; echo "${POLICY}"; exit 1; } + echo "OK: the operator certname is reserved in the rendered policy" + catch: + - events: {} + - name: Change the SigningPolicy and assert the CA pod rolls try: - script: