Skip to content

feat: add cert-manager TLS support#125

Open
GatewayJ wants to merge 1 commit into
rustfs:mainfrom
GatewayJ:feat/cert-manager-tls
Open

feat: add cert-manager TLS support#125
GatewayJ wants to merge 1 commit into
rustfs:mainfrom
GatewayJ:feat/cert-manager-tls

Conversation

@GatewayJ
Copy link
Copy Markdown
Member

Type of Change

  • New Feature
  • Bug Fix
  • Documentation
  • Performance Improvement
  • Test/CI
  • Refactor
  • Other: N/A

Related Issues

N/A

Summary of Changes

  • Adds an optional Tenant spec.tls API for cert-manager-backed RustFS TLS, including managed Certificate creation, external TLS Secret support, CA trust configuration, rollout hashing, status reasons, and Console API exposure.
  • Wires TLS Secrets and CA bundles into generated StatefulSets and switches RustFS services to HTTPS naming when TLS is enabled.
  • Updates CRDs, Helm/dev RBAC, and sample manifests for cert-manager TLS usage.
  • Adds unit and live e2e coverage for managed cert-manager certificates, external TLS Secrets, negative validation paths, secret rotation, and artifact redaction.

Checklist

  • I have read and followed the CONTRIBUTING.md guidelines
  • Passed make pre-commit (fmt-check + clippy + test + console-lint + console-fmt-check)
  • Added/updated necessary tests
  • Documentation updated (if needed)
  • CHANGELOG.md updated under [Unreleased] (if user-visible change) - N/A: this repository does not currently contain a CHANGELOG.md
  • CI/CD passed (if applicable) - N/A: pending upstream GitHub Actions

Impact

  • Breaking change (CRD/API compatibility)
  • Requires doc/config/deployment update
  • Other impact: Adds optional Tenant TLS fields and cert-manager RBAC; TLS remains disabled by default.

Verification

make pre-commit
RUSTFS_E2E_LIVE=1 make e2e-live-delete
RUSTFS_E2E_LIVE=1 make e2e-live-create
RUSTFS_E2E_LIVE=1 make e2e-live-run

Local verification completed:

  • make pre-commit: passed
  • live delete/create/run sequence: passed; e2e-live-run ended with configured live e2e suites passed.

Additional Notes

The branch is based on current origin/main to avoid carrying the already-merged image pull secret update commit.


Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 608ad7b796

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/reconcile/phases.rs
Comment on lines +478 to +480
if let Some(tls_status) = tls_plan.status {
builder.set_tls_status(tls_status);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Clear TLS status when TLS is no longer enabled

When TLS is disabled (spec.tls removed or mode=Disabled), reconcile_tls returns a disabled plan with status=None, and this block only updates status when tls_plan.status is present. That leaves the previous status.certificates.tls payload (certificate refs/hash/errors) in place after a successful reconcile, so the API/console can continue to report stale TLS state even though TLS is off. Please explicitly clear TLS certificate status (and corresponding condition state) on the disabled path.

Useful? React with 👍 / 👎.

Comment thread src/lib.rs
Comment on lines +167 to +170
if let Some(labels) = labels
&& let Some(tenant_ref) = tenant_ref_from_labels(namespace, labels)
{
push_unique_tenant_ref(&mut refs, tenant_ref);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Trigger reconcile for external TLS Secret rotations

Secret events are mapped to Tenants only via owner references or the rustfs.tenant label. In the manageCertificate=false external-secret flow, user-provided TLS Secrets commonly have neither metadata link, so rotating the referenced Secret does not enqueue a reconcile. Because rollouts are hash-driven during reconcile and steady-state uses Action::await_change(), pods can keep serving old cert/key material indefinitely after external Secret updates. The watch mapping should include Tenant spec secret references (or enforce/validate the required label) so external rotations always trigger reconciliation.

Useful? React with 👍 / 👎.

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.

1 participant