feat(infisical): add caBundle and caProvider support#5770
feat(infisical): add caBundle and caProvider support#5770Skarlso merged 2 commits intoexternal-secrets:mainfrom
Conversation
WalkthroughAdds optional CA configuration to the Infisical provider: an inline PEM Changes
Pre-merge checks✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🔇 Additional comments (3)
Comment |
Add support for custom CA certificates when connecting to self-hosted Infisical instances that use self-signed or private CA certificates. - Add CABundle and CAProvider fields to InfisicalProvider API types - Implement CA certificate fetching using esutils.FetchCACertFromSource - Pass CA certificate to Infisical SDK's CaCertificate config - Add validation for ClusterSecretStore CAProvider namespace requirement - Add validation for SecretStore CAProvider namespace restriction - Update documentation with CA configuration examples - Regenerate CRDs and API spec documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Adam Winstanley <adam@winstanley.industries>
fdbd1bf to
2c02d82
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apis/externalsecrets/v1/secretsstore_infisical_types.go (1)
168-177: LGTM! Well-designed API fields with clear documentation.The new
CABundleandCAProviderfields are properly defined with:
- Appropriate types (
[]bytefor PEM data,*CAProviderfor optional reference)- Clear documentation including mutual exclusivity
- Correct kubebuilder markers (
+optional)The mutual exclusivity is documented in comments and validated at runtime (per the test coverage mentioned in the PR). For Kubernetes 1.25+, you could optionally add a CEL validation rule to enforce this at the CRD level:
// +kubebuilder:validation:XValidation:rule="!(has(self.caBundle) && has(self.caProvider))",message="caBundle and caProvider are mutually exclusive"However, this is purely optional since runtime validation is already in place.
config/crds/bases/external-secrets.io_clustersecretstores.yaml (1)
3270-3314: Infisical CA bundle / CA provider schema looks correct and consistentThe
caBundleandcaProviderfields for the Infisical provider match the existing CA patterns used elsewhere in this CRD (formatbytefor the bundle, standardCAProvidershape with requirednameandtype, and the same namespace semantics). Descriptions are clear and align with the new controller behavior. Nothing blocking here.If you ever want stronger schema-level guarantees, you could optionally mirror the Doppler-style
x-kubernetes-validationsto enforce mutual exclusivity betweencaBundleandcaProvider, but given existing providers also rely on controller-side validation, leaving it as-is is perfectly reasonable.config/crds/bases/external-secrets.io_secretstores.yaml (1)
3270-3314: Infisical CA bundle schema looks consistent with existing providers
caBundleandcaProviderare typed and structured consistently with other CA-enabled providers (e.g., Akeyless, GitLab, Vault):caBundleis astringwithformat: byte, andcaProviderhaskey/name/namespace/typewithnameandtyperequired. The namespace doc matches the existing pattern (“Can only be defined when used in a ClusterSecretStore”), which aligns with your stated validation rules.If you want absolute wording parity, you could tweak the
caBundledescription to mention “PEM/base64 encoded” like some other providers, but this is purely cosmetic and not blocking.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apis/externalsecrets/v1/secretsstore_infisical_types.goapis/externalsecrets/v1/zz_generated.deepcopy.goconfig/crds/bases/external-secrets.io_clustersecretstores.yamlconfig/crds/bases/external-secrets.io_secretstores.yamldeploy/crds/bundle.yamldocs/api/spec.mddocs/provider/infisical.mddocs/snippets/infisical-generic-secret-store.yamlproviders/v1/infisical/provider.goproviders/v1/infisical/provider_test.go
👮 Files not reviewed due to content moderation or server errors (1)
- docs/api/spec.md
🧰 Additional context used
🧬 Code graph analysis (2)
apis/externalsecrets/v1/zz_generated.deepcopy.go (1)
apis/externalsecrets/v1/secretstore_types.go (1)
CAProvider(241-266)
providers/v1/infisical/provider_test.go (2)
providers/v1/infisical/provider.go (1)
Provider(53-58)providers/v1/vault/util/vault.go (1)
Auth(31-33)
🪛 LanguageTool
docs/provider/infisical.md
[style] ~149-~149: Using many exclamation marks might seem excessive (in this case: 15 exclamation marks for a text that’s 2635 characters long)
Context: ...oject environmentSlug: dev ``` !!! note For ClusterSecretStore, be s...
(EN_EXCESSIVE_EXCLAMATION)
🔇 Additional comments (10)
apis/externalsecrets/v1/zz_generated.deepcopy.go (1)
2424-2433: LGTM! Deep copy implementation is correct.The auto-generated deep copy logic properly handles the new
CABundleandCAProviderfields with appropriate nil checks and follows the same pattern used by other providers in this file (e.g., AkeylessProvider, GitlabProvider).docs/provider/infisical.md (1)
88-151: LGTM! Excellent documentation for the CA certificate feature.The documentation is clear, well-structured, and provides practical examples for both configuration approaches (
caBundleandcaProvider). The note about settingnamespaceforClusterSecretStoreis helpful and aligns with the validation rules mentioned in the PR.providers/v1/infisical/provider.go (2)
339-355: LGTM - CA certificate fetching implementation is correct.The implementation correctly uses the shared
esutils.FetchCACertFromSource()helper and properly handles the case where a certificate is returned. Error handling provides good context for debugging.
360-361: LGTM - CA certificate integration with SDK.The CA certificate is correctly passed to the Infisical SDK configuration.
docs/snippets/infisical-generic-secret-store.yaml (1)
12-24: LGTM - Clear documentation for CA configuration options.The commented examples clearly illustrate both
caBundleandcaProvideroptions, with appropriate notes about mutual exclusivity and namespace requirements forClusterSecretStore.providers/v1/infisical/provider_test.go (4)
236-236: Good addition of TypeMeta for validation testing.Adding
TypeMetawith theKindfield is essential for theValidateStoremethod to correctly identify the store type. This ensures the CAProvider namespace validation works correctly.
280-290: LGTM - Well-structured test helper for SecretStore CAProvider.The
withSecretStoreCAProviderhelper follows the existing pattern and correctly sets the CAProvider on SecretStore.
292-350: LGTM - Comprehensive test scaffolding for ClusterSecretStore.The
makeClusterSecretStorefactory and associated modifiers (withCAProvider,withClusterClientID,withClusterClientSecret) follow the established pattern from the SecretStore helpers and provide good test infrastructure.
401-469: Excellent test coverage for CAProvider namespace validation.The test cases comprehensively cover all four validation scenarios for CAProvider namespace requirements. Using
esv1.GenericStorein the test struct is a good approach for testing both store types.deploy/crds/bundle.yaml (1)
16805-16846: Expected duplication for multiple CRD definitions.This hunk is identical to the previous one (lines 5139-5180), which is expected in a CRD bundle that includes both
SecretStoreandClusterSecretStoredefinitions. The schema correctly replicates the samecaBundleandcaProviderfields for consistency across both resource types.
Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
|
…s#5770) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Summary
caBundleandcaProviderfields toInfisicalProviderAPI types for custom CA certificate supportesutils.FetchCACertFromSource()helperCaCertificateconfig fieldcaProvider.namespaceforClusterSecretStoreand rejecting it forSecretStoreThis enables users with self-hosted Infisical instances using self-signed or private CA certificates to configure trust.
Fixes #3793
Test plan
TestValidateStoreCAProviderwith 4 test cases covering ClusterSecretStore and SecretStore validationmake test)make lint)Documentation
docs/provider/infisical.mdwith examples for bothcaBundleandcaProviderdocs/snippets/infisical-generic-secret-store.yamlwith commented examplesAdds support for custom CA certificates to the Infisical provider to allow connections to self‑hosted Infisical instances using self-signed or private CA certificates.
Key changes:
Testing & docs:
Fixes #3793.