Skip to content

K8SPG-1045: add operatorProvidedOnly to the spec.tls.certManagementPolicy - #1771

Open
pooknull wants to merge 2 commits into
mainfrom
K8SPG-1045-improvement
Open

K8SPG-1045: add operatorProvidedOnly to the spec.tls.certManagementPolicy#1771
pooknull wants to merge 2 commits into
mainfrom
K8SPG-1045-improvement

Conversation

@pooknull

Copy link
Copy Markdown
Contributor

https://perconadev.atlassian.net/browse/K8SPG-1045

DESCRIPTION

This PR adds the operatorProvidedOnly value to spec.tls.certManagementPolicy.

When used, the operator generates and manages TLS certificates without cert-manager

CHECKLIST

Jira

  • Is the Jira ticket created and referenced properly?
  • Does the Jira ticket have the proper statuses for documentation (Needs Doc) and QA (Needs QA)?
  • Does the Jira ticket link to the proper milestone (Fix Version field)?

Tests

  • Is an E2E test/test case added for the new feature/change?
  • Are unit tests added where appropriate?

Config/Logging/Testability

  • Are all needed new/changed options added to default YAML files?
  • Are all needed new/changed options added to the Helm Chart?
  • Did we add proper logging messages for operator actions?
  • Did we ensure compatibility with the previous version or cluster upgrade process?
  • Does the change support oldest and newest supported PG version?
  • Does the change support oldest and newest supported Kubernetes version?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new TLS certificate management mode, operatorProvidedOnly, to the PostgresCluster API so the operator always generates/manages TLS using internal PKI and ignores cert-manager even when cert-manager is installed.

Changes:

  • Extends spec.tls.certManagementPolicy enum with operatorProvidedOnly and propagates it through generated CRDs/bundles.
  • Updates cert-manager integration points to treat only auto as “cert-manager enabled” (issuerConf is ignored for non-auto policies).
  • Adds/extends unit tests and a KUTTL E2E scenario to validate operator-provided TLS behavior and absence of cert-manager resources.

Reviewed changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_types.go Adds operatorProvidedOnly to the API enum/validation.
percona/certmanager/certmanager.go Makes ResolveIssuerMode short-circuit for non-auto policies (issuerConf ignored).
percona/certmanager/certmanager_test.go Adds tests ensuring issuerConf is ignored for non-auto policies.
internal/pgbouncer/reconcile_test.go Extends PgBouncer secret policy tests for operatorProvidedOnly.
internal/controller/postgrescluster/pki.go Gates cert-manager usage behind policy via shouldUseCertManager and updates stale-certificate recovery checks.
internal/controller/postgrescluster/pki_test.go Adds coverage for operatorProvidedOnly internal PKI behavior and skipping cert-manager checks.
internal/controller/postgrescluster/pgbouncer.go Uses new stale-certificate reconciliation signature (cluster-aware).
internal/controller/postgrescluster/pgbackrest.go Uses new stale-certificate reconciliation signature (cluster-aware).
internal/controller/postgrescluster/patroni.go Uses new stale-certificate reconciliation signature (cluster-aware).
internal/controller/postgrescluster/instance.go Uses new stale-certificate reconciliation signature (cluster-aware).
e2e-tests/tests/cert-management-policy/12-recreate-cluster.yaml Recreates cluster with operatorProvidedOnly and issuerConf set (expected ignored).
e2e-tests/tests/cert-management-policy/13-assert.yaml Asserts policy value and status condition messaging for operatorProvidedOnly.
e2e-tests/tests/cert-management-policy/14-verify-operator-provided-only.yaml Verifies operator-generated TLS secrets and absence of cert-manager resources/annotations.
deploy/cw-bundle.yaml Updates published bundle schema enum to include operatorProvidedOnly.
deploy/crd.yaml Updates published CRD schema enum to include operatorProvidedOnly.
deploy/bundle.yaml Updates published bundle schema enum to include operatorProvidedOnly.
config/crd/bases/upstream.pgv2.percona.com_postgresclusters.yaml Updates CRD base schema enum to include operatorProvidedOnly.
config/crd/bases/pgv2.percona.com_perconapgclusters.yaml Updates CRD base schema enum to include operatorProvidedOnly.
build/crd/percona/generated/pgv2.percona.com_perconapgclusters.yaml Updates generated CRD schema enum to include operatorProvidedOnly.
Suppressed comments (1)

internal/controller/postgrescluster/pki.go:618

    1. Problem: The log message for ErrCertManagerNotReady says the operator is “falling back to internal PKI”, but in some reconcile paths (e.g., when spec.tls.issuerConf requires cert-manager) the operator will not actually fall back and will instead block with an error.
  1. Why it matters: Misleading logs make it harder to diagnose why TLS reconciliation is stalled when cert-manager is installed but temporarily unavailable.
  2. Fix: Log a neutral “cert-manager is not ready” message here, and let the caller decide whether it can fall back or must wait/error.
		case errors.Is(err, certmanager.ErrCertManagerNotFound):
			return false, nil
		case errors.Is(err, certmanager.ErrCertManagerNotReady):
			logging.FromContext(ctx).Info("cert-manager is not ready, falling back to internal PKI")
			return false, nil

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@egegunes egegunes added this to the v3.1.0 milestone Aug 21, 2026
func ResolveIssuerMode(ctx context.Context, cl client.Client, cluster *v1beta1.PostgresCluster) (IssuerMode, error) {
// Only auto uses cert-manager
if cluster.Spec.TLS.GetCertManagementPolicy() != v1beta1.CertManagementAuto {
return IssuerModeManagedNamespaced, nil

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we return an error here instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants