Skip to content

Conversation

abelhoula
Copy link
Contributor

…back to self-signed

@abelhoula abelhoula requested a review from a team as a code owner August 8, 2025 13:42
Copy link

netlify bot commented Aug 8, 2025

Deploy Preview for docs-kargo-io canceled.

Name Link
🔨 Latest commit 4b129b7
🔍 Latest deploy log https://app.netlify.com/projects/docs-kargo-io/deploys/6896018a2051af00082246af

Copy link

codecov bot commented Aug 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.17%. Comparing base (27769a2) to head (4b129b7).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4802   +/-   ##
=======================================
  Coverage   54.17%   54.17%           
=======================================
  Files         395      395           
  Lines       33792    33792           
=======================================
  Hits        18306    18306           
  Misses      14558    14558           
  Partials      928      928           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: abelhoula <[email protected]>
@krancour
Copy link
Member

This was already possible by setting api.ingress.tls.selfSignedCert to false and providing the Secret named kargo-api-ingress-cert in any manner you wish to do so.

Is there a specific reason that existing capability wasn't working for you?

@krancour krancour added area/charts needs-discussion kind/enhancement kind/proposal priority/low more-information-needed Kargo maintainers have responded to the issue, and more information is needed to address the issue and removed needs-discussion labels Aug 11, 2025
@abelhoula
Copy link
Contributor Author

Many platforms pre-provision TLS secrets with standardized names (e.g., from external PKI/ACME controllers). Allowing secretName lets us point to those directly without renaming or duplicating secrets.

@krancour
Copy link
Member

krancour commented Aug 12, 2025

I think the case wherein one has no control over the naming convention of the Secrets containing one's certs, outlying though I think it may be, seems like rational impetus for this.

So I'm on board with the idea, but the following things need to be addressed for this PR to be merged:

  1. I think there is a total of six places where you can either use a self-signed cert or bring your own. For consistency's sake, this change should be applied consistently in all six places instead of just the one place where you care about it.

  2. I think some more discussion is in order over how exactly this is to work and needs to be documented more thoroughly. The following bit of inline doc in values.yaml leaves me with questions that I need to go digging in the templates to answer:

    Name of the existing TLS secret to use for the API server's Ingress.

    It's not clear from that what relationship, if any, it has to selfSignedCert. Is it applicable only when selfSignedCert is false? This does not appear to be the case, but I think it ought to be. What happens if selfSignedCert is true and a value is specified for secretName regardless? The current implementation doesn't appear to deal with that scenario correctly.

Bottom line is that I am perfectly amenable to this change, but need it to be more thorough.

@krancour krancour removed kind/proposal more-information-needed Kargo maintainers have responded to the issue, and more information is needed to address the issue labels Aug 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants