feat: support GCP Private CA as TLS automation cert issuer - #522
Merged
Merged
Conversation
BREAKING CHANGE: the downstream TLS certificate settings moved from tls.certificateFile/tls.privateKeyFile to tls.static.certificateFile/ tls.static.privateKeyFile. Behavior is unchanged; the Helm chart renders the new layout. Prepares the schema for the mutually exclusive tls.dynamic mode (#423). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X9Z4JjodvSrpkMw964Lcgx
- add tls.automation.issuer.vault, issuing leaf certificates through Vault's PKI secrets engine alongside the existing local CA issuer - share the Vault client, auth and token-renewal code between the TLS and SSH CAs in a new internal/vault package - issue DNS SANs only; the no-SNI local-address fallback still serves probes, which skip verification Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Health probes dial the gateway by IP without SNI, so the local-address fallback requests a certificate for a bare IP. Split IP aliases into ip_sans when issuing through Vault so those certificates verify, matching the self-sign issuer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
internal/config/config.go:153
- The YAML loading path is not covered for these new optional fields: the existing
TestLoad_TLSAutomationonly unmarshals a local issuer, while the added tests constructTLSGCPPrivateCAIssuerConfigdirectly. A regression in either tag would silently dropissuingCertificateAuthorityIDorcredentialsFile, causing issuance to use a different CA or ADC instead of the operator's configured credentials. Add aLoadtest that asserts both values are populated from YAML.
IssuingCertificateAuthorityID string `yaml:"issuingCertificateAuthorityID,omitempty"`
CredentialsFile string `yaml:"credentialsFile,omitempty"` // Defaults to Application Default Credentials
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (3)
internal/config/config.go:131
- Please add a YAML load test for this new issuer. The current config tests only construct
TLSGCPPrivateCAIssuerConfigvalues directly, so the user-facinggcpPrivateCAand nested tags (caPoolID, the optional issuing-CA ID, and credentials file) are not exercised; a YAML tag typo would pass the suite. ExtendTestLoad_TLSAutomationwith a GCP block and assert all fields.
GCPPrivateCA *TLSGCPPrivateCAIssuerConfig `yaml:"gcpPrivateCA,omitempty"`
internal/connect/cert/issuer.go:304
- Please add coverage for the empty
CredentialsFilebranch. The new tests exercise a service-account file and a missing file, but none runsrunwithout a credentials file, so a regression that disables the documented ADC/workload-identity fallback would go unnoticed. Use a controlled ADC fixture (for exampleGOOGLE_APPLICATION_CREDENTIALS) rather than relying on the test runner's ambient credentials.
if g.credentialsFile != "" {
opts = append(opts, option.WithAuthCredentialsFile(option.ServiceAccount, g.credentialsFile))
}
internal/connect/cert/issuer.go:348
pem_certificate_chainfrom Certificate Authority Service already contains the leaf certificate (the API documents it as the full chain, leaf first). PrependingPemCertificatehere therefore returns the leaf twice;automation.issuewill send that duplicate as an intermediate, which can make downstream TLS clients reject the chain. Parse the returned chain directly (and update the fake response in the test to include the leaf first).
chain, err := parseCertificateChain(append([]string{issued.GetPemCertificate()}, issued.GetPemCertificateChain()...))
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Tickets
Issue: #423
Changes
tls.automation.issuer.gcpPrivateCAconfig. When configured, Gateway issues downstream TLS certs through a Google Cloud CA Service pool, and verifies each issued cert the same way the Vault issuer does: exact requested DNS/IP SANs, TTL and public key.